docs: align TEE attestation docs with the no-yield design - #3826
Conversation
Dstack attestations are no longer verified in-WASM. submit_participant_info now branches: Mock is verified synchronously, Dstack yields and offloads DCAP quote verification to a separate tee-verifier contract via a cross-contract call, resuming through resolve_verification / on_attestation_verified. This lets the contract drop the mpc-attestation local-verify feature (and with it dcap-qvl). Adds the pending-attestation state, the yield/resume + refund/revert machinery, the verifier-call gas config, and the related error variants. This is the contract-side wiring only; the verifier interface (tee-verifier-interface, DstackAttestation::verify) already exists. Sandbox coverage (stub verifier + tests) and the design doc follow in a separate PR.
- refund the attached deposit when a participant refreshes an existing attestation (charge_attestation_storage early-return kept it silently) - add v3_13_0_state migration shadow so migrate() can upgrade from a deployed 3.13.0 layout, not just 3.12.0 - collapse the two per-arm debug_asserts in on_attestation_verified into one guarding both resolved paths, with a note on why cleanup stays in resolve_verification (its remove-before-store gates the timeout race) - nits: from_yoctonear(0) on the fail-call; drop a redundant yield comment; reframe the promise_yield_resume comment; PendingAttestation fields pub(crate); document the verify_quote wire args; benchmark TODOs on the new gas defaults - TODO(#3720): stash only tcb_info in PendingAttestation (follow-up)
Rename the attestation verify-and-store helpers so they describe what they do rather than when they run: - add_mock_participant -> verify_and_store_mock - finish_dstack_verify -> verify_and_store_dstack - finish_verified_attestation -> verify_post_dcap_and_store Also tighten the resolve_verification and on_attestation_verified comments: scope the no-verdict note to the Err arm, name the ~200-block yield-resume timeout explicitly, and trim the debug_assert rationale to the single invariant it rests on
- Fix broken rustdoc intra-doc link in tee_state.rs (Attestation was not in scope; use the in-scope DstackAttestation), which the workspace broken_intra_doc_links=deny lint would fail CI on - Correct the v3_13_0_state module doc field count (four Config gas fields plus the pending_attestations map, not two) - Rename stale add_participant_* tests to the verify_and_store_mock SUT and the <sut>__should_<assertion> form - Fix a/an grammar in an Attestation::Dstack doc comment
The a->an grammar fix in on_attestation_verified's doc comment changed the doc string embedded in the ABI, which test_abi_has_not_changed compares against the committed snapshot. Update the snapshot to match
The fail_on_timeout callback sites used NearToken::from_near(0) while the new attestation code uses from_yoctonear(0); both are zero. Unify on from_yoctonear(0)
A Dstack attestation is stored on the contract only after its async verification succeeds. The node confirms a submission by checking whether the attestation is stored; while verification is still in flight it isn't, so the node observed NotExecuted and resubmitted, hitting VerificationAlreadyPending until the verifier resolved. Add an is_verification_pending contract view and consult it in observe_tx_result: when the attestation isn't stored yet but a verification is pending for the submitter, count the submission as executed so the node does not resubmit mid-verification. The ABI snapshot (abi__abi_has_not_changed.snap) needs regenerating for the new view method; the local toolchain cannot build the contract wasm, so it must be updated in CI or on a 1.86-compatible toolchain.
Co-authored-by: Patryk Bęza <patryk.beza@gmail.com>
…ttestation_storage
`stored_attestations` is a `near_sdk::store::IterableMap`, which buffers writes and only serializes them on flush/Drop. Both attestation charge paths read the `env::storage_usage()` delta immediately after the insert, before the buffered write reaches the trie, so the delta was always zero: `submit_participant_info` accepted a new attestation with zero deposit and refunded it in full, letting any account grow contract storage for free. Flush the map in the shared `store_verified_attestation` tail so the insert is visible to the delta measured by `charge_attestation_storage`, covering both the synchronous mock path and the async dstack path.
The node, tee-context, and e2e/test harnesses previously submitted submit_participant_info with a zero deposit, so the contract's storage charge would fail once submissions started paying for their own storage. Add a shared SUBMIT_PARTICIPANT_INFO_DEPOSIT_YOCTONEAR in the interface crate's new deposits module and use it everywhere: - node: ChainSendTransactionRequest::deposit_required(), threaded through submit_tx and the TransactionSigner - tee-context: submit_attestation attaches it - e2e: init_contract submits with gas + deposit - contract tests: sandbox and inprocess consts derive from the shared value The contract charges the measured storage cost and refunds the excess.
- revert_dstack_store unit tests: reuse create_node_id, compare the whole NodeAttestation (add PartialEq/Eq to NodeAttestation, VerifiedAttestation, MockAttestation, ValidatedDstackAttestation) instead of a field + a _ match - add a verified_report() test-utils fixture (mints the real report via verify_dcap_quote) and rework the OOG test onto the Verified path; it stays #[ignore]d pending the allowlist fixture setup (TODO(#3730)) - compress the sandbox tee_verifier tests: submit_dstack + setup_with_stub + assert_submission_cleaned_up helpers remove the repeated preamble/asserts; inline the trivial dstack_attestation()/tls_key() wrappers - switch the sandbox tests to unwrap() style (repo-wide majority) instead of Result<()>/? - assert the exact VerifierNotConfigured message via the error Display - gate/name has_pending_attestation via method_names::HAS_PENDING_ATTESTATION - extract TEST_COLLATERAL_STRING / SUBMIT_DEPOSIT consts; doc + intra-doc-link cleanups
The tests were written against the old yield-resume resolve_verification (pending_attestations + on_attestation_verified yield-callback + ~200-block timeout). #3766 replaced that with a plain promise chain (verify_quote -> .then(resolve_verification)) whose failures refund and fire a separate fail_attestation_submission receipt, with no pending state and no timeout. Rewrite the sandbox tests to match: - tee_verifier.rs: drop YIELD_TIMEOUT_BLOCKS / fast_forward and the pending-entry assertions; observe failures via the chain's receipt outcomes (ExecutionFinalResult::failures) instead of a queryable pending state. The verifier-crash test now expects an immediate VerifierUnavailable rather than a timeout cleanup. Keep the Verified happy-path and the OOG-resolve test #[ignore]d (they need fixture-allowlist + signer-key setup to reach a successful store) and point them at #3738 rather than the wrong #3730. - Remove has_pending_attestation (it read the removed pending_attestations map) from sandbox_test_methods.rs, its sandbox helper, and the HAS_PENDING_ATTESTATION method-name constant. - docs/design/attestation-verifier-contract.md: rewrite the submission-flow, handling-failures, state, API, and testing sections to the promise-chain design. Also reflow one pre-existing rustfmt violation in participants_gas.rs.
editorconfig-checker requires a final newline (insert_final_newline); the doc rewrite left the file without one, failing Fast CI checks.
- test-utils/Cargo.toml: sort tee-verifier-interface into dependency order (fixes the cargo-sort Fast CI failure) - test-tee-verifier/Cargo.toml: ignore borsh in cargo-shear (used only via the abi feature, like the sibling tee-verifier crate), avoiding a --deny-warnings failure - test-tee-verifier-types: reword the StubResponse::Panic doc comment to the no-yield flow (it described the removed yield timeout) - tee_verifier.rs: bound the ignored happy-path test's net spend on both sides so a wrongly-retained deposit fails; note why the failure assertion substring-matches; retarget the ignored tests at the fixture follow-up (#3787)
Extract helpers and reuse existing ones to cut boilerplate, with no change to test coverage: - tee_state.rs: reuse create_node_id; add node_id_for for filler-TLS-key nodes; add attestation_expiring_at, store_valid_attestations, and authenticate_as; collapse per-test TEE_UPGRADE_DURATION into one const; reuse gen_participants; merge crate::primitives imports; drop a test whose assertion is subsumed by preserve_node_id_integrity and internal_storage_distinguishes_participants_by_tls_key - attestation_submission.rs: reuse get_participant_node_ids; collapse Running-state matches into assert_matches!; add with_tee_upgrade_grace_period_seconds builder method; remove dead pre-build testing_env! blocks that TestSetupBuilder::build overwrites; fix typos
…osit guard Fill async-attestation coverage gaps that opened after the base PR settled on the no-yield design: - sandbox: a Verified verdict that fails the post-DCAP checks (empty allowlist) still refunds and fails the submission in a separate receipt, storing nothing — the resolve_verification failure mode not blocked by the fixture-allowlist work - unit: verify_and_store_dstack rejects and stores nothing when the post-DCAP checks fail - inprocess: submit_participant_info rejects a deposit below the storage cost with InsufficientDeposit - sandbox: extend the mock-success test to assert the excess deposit is refunded
- verify_and_store_mock/revert tests: compare whole NodeAttestation values via a shared mock_valid_attestation helper instead of single fields; drop .expect()/.unwrap() lookups in favor of assert_eq!(map.get(k), Some(&entry)) - bind error payloads instead of discarding them (Invalid(msg), InvalidState, VerificationError variants); replace let-else+panic! and Ok(_)=>panic! with assert_matches! binding blocks / expect_err - rename attestation_expiring_at -> mock_attestation_with_expiry - derive PartialEq/Eq on ParticipantInsertion and Clone on NodeAttestation so insertions compare as whole values - deposit/refund tests: measure the storage stake from the contract's byte growth times env::storage_byte_cost() and assert net_spent == storage_stake + total_gas_fee(result) exactly, replacing hardcoded refund_floor/gas_ceiling and STORAGE_COST_PER_BYTE constants; add total_gas_fee helper
Condense the tee_verifier sandbox module doc, per-test `// Then:` notes, and the two TODO(#3787) blocks; drop the inaccurate "runtime refunds the deposit to the predecessor" note on the OOG TODO. Trim the duplicated non-`#[near]` rationale on the test-tee-verifier-types crate (Cargo.toml + lib.rs) and shorten the test-tee-verifier stub doc.
Revert the TEE attestation doc edits (docs/design/attestation-verifier-contract.md, docs/localnet/tee-localnet.md, docs/running-an-mpc-node-in-tdx-external-guide.md) to their base-branch content so this PR is test-only. The doc updates move to a stacked PR tracked by #3825.
Rewrite docs/design/attestation-verifier-contract.md to describe the no-yield promise-chain submit_participant_info flow (submit_dstack_attestation + resolve_verification, deposit forwarded on the callback, fail_attestation_submission on failure) instead of the superseded yield-resume / pending_attestations / timeout design, and fix the re_verify source reference. Correct stale on-chain error strings in docs/localnet/tee-localnet.md and docs/running-an-mpc-node-in-tdx-external-guide.md. Closes #3825
There was a problem hiding this comment.
Pull request overview
Aligns the repository’s TEE attestation documentation with the shipped “no-yield” promise-chain submit_participant_info flow, replacing the superseded yield/resume + timeout design and updating operator-facing troubleshooting examples accordingly.
Changes:
- Rewrites
docs/design/attestation-verifier-contract.mdto describe the no-yieldverify_quote -> resolve_verificationpromise chain, including deposit-forwarding and failure handling. - Updates the
re_verifyreference to its current source location. - Refreshes localnet/TDX guide error-message examples to match current on-chain error strings.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| docs/running-an-mpc-node-in-tdx-external-guide.md | Updates the on-chain receipt error example to match the current attestation failure message. |
| docs/localnet/tee-localnet.md | Updates the localnet ExecutionError(...) example to match the current on-chain attestation failure message. |
| docs/design/attestation-verifier-contract.md | Replaces yield/resume design details with the no-yield promise-chain verifier-contract design and updates source references. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Pull request overviewThis PR splits the TEE-attestation documentation changes out of the tests-only PR #3715 and rewrites Changes:
Reviewed changesPer-file summary
FindingsSpot-checked the shipped implementation against the rewritten prose and code snippets:
No blocking issues found. Non-blocking (nits):
✅ Approved |
The localnet troubleshooting snippet dropped the closing `))`, so the pasted `ExecutionError(...)` example read as unbalanced.
6afae04 to
b21cc1e
Compare
|
Heads-up: #3940 (drop the deposit for This PR correctly removes the yield-resume drift — the new §Handling failures ("nothing is inserted into contract storage at submit time, so there is no pending entry that a failure could leave wedged") is exactly right. But the rewrite is built on the deposit-forwarding model, which #3940 deletes: Symbols this rewrite documents that don't exist on #3940's head: Passages that need stripping (line numbers from this PR's version of
Line 120 is worth a second look rather than a delete: the underlying observation — that the callback receipt commits even when it returns In the meantime I've added a |
b444cf6 to
93c9ab5
Compare
|
PR title type suggestion: This PR changes editor config, dependencies, and assets alongside documentation, so the type should probably be |
93c9ab5 to
b444cf6
Compare
Splits the TEE attestation doc changes out of the async-attestation tests PR (#3715) so that PR stays test-only. Stacked on top of #3715; retarget to the base branch once #3715 merges.
Closes #3825