feat(contract): async TEE attestation verification, drop dcap-qvl#3714
feat(contract): async TEE attestation verification, drop dcap-qvl#3714pbeza wants to merge 10 commits into
dcap-qvl#3714Conversation
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.
There was a problem hiding this comment.
Pull request overview
This PR wires the MPC contract to verify Attestation::Dstack asynchronously by offloading DCAP quote verification to an external tee-verifier contract (via a cross-contract verify_quote call), while keeping Attestation::Mock synchronous. It also removes the dcap-qvl dependency from mpc-contract by dropping the mpc-attestation/local-verify feature and updates the interface/config/snapshots to reflect the new async flow.
Changes:
- Implement async Dstack attestation submission using yield/resume (
resolve_verification→on_attestation_verified) with refund + cleanup on rejection/timeout. - Add pending-attestation state and new config gas fields + interface constants for verifier/callback/failure calls.
- Remove contract-side local DCAP verification dependency path and regenerate ABI/Borsh schema snapshots and affected tests.
Reviewed changes
Copilot reviewed 19 out of 20 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/test-utils/src/contract_types.rs | Updates dummy config builder to populate newly added gas config fields. |
| crates/near-mpc-contract-interface/src/types/config.rs | Extends InitConfig/Config with verifier + callback + failure gas parameters and updates related tests. |
| crates/near-mpc-contract-interface/src/method_names.rs | Adds method-name constants for new callbacks and the verifier’s verify_quote entrypoint. |
| crates/mpc-attestation/src/attestation.rs | Updates docs around local verification usage after moving contract-side DCAP verification out-of-WASM. |
| crates/contract/tests/snapshots/abi__abi_has_not_changed.snap | Regenerates ABI snapshot to include new private callbacks, types, and config fields. |
| crates/contract/tests/sandbox/upgrade_from_current_contract.rs | Updates sandbox upgrade test config values to include new fields. |
| crates/contract/tests/sandbox/contract_configuration.rs | Updates initialization/configuration sandbox test to include new config fields. |
| crates/contract/tests/inprocess/attestation_submission.rs | Adjusts in-process tests to match new attestation-submission error plumbing. |
| crates/contract/src/v3_12_0_state.rs | Adds pending_attestations initialization to the v3.12.0 → current state migration. |
| crates/contract/src/tee/tee_state.rs | Splits mock vs Dstack verification paths, adds revert support for async storage-charge failures, and exposes attestation submission errors. |
| crates/contract/src/tee/pending_attestation.rs | Introduces new on-chain state/type for in-flight Dstack attestation verification and yield resume result. |
| crates/contract/src/tee.rs | Exposes the new pending_attestation module. |
| crates/contract/src/storage_keys.rs | Adds a storage key variant for the pending-attestations map. |
| crates/contract/src/snapshots/mpc_contract__tests__mpc_contract_borsh_schema_has_not_changed.snap | Regenerates Borsh schema snapshot for new config fields and added contract state. |
| crates/contract/src/lib.rs | Implements async Dstack submission flow, pending state management, refund/fail machinery, and storage charging refactor. |
| crates/contract/src/errors.rs | Adds new tee error variants and plumbs AttestationSubmissionError into the top-level Error. |
| crates/contract/src/dto_mapping.rs | Maps new config fields between DTOs and internal config. |
| crates/contract/src/config.rs | Adds default values for new gas config fields. |
| crates/contract/Cargo.toml | Drops mpc-attestation/local-verify (removing dcap-qvl from contract), adds tee-verifier-interface, and adjusts ABI features. |
| Cargo.lock | Updates dependency graph for the contract changes (new deps, removed feature linkage). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
test |
Pull request overviewSplits the contract-side async TEE attestation flow out of #3664. FindingsBlocking (must fix before merge):
Non-blocking (nits, follow-ups, suggestions):
|
- 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)
pbeza
changed the title
dcap-qvl
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
|
@claude review |
Pull request overviewSplits the async TEE attestation core out of #3664. Changes:
Reviewed changesPer-file summary
FindingsBlocking issues from my prior review have been addressed in this iteration:
Non-blocking (nits, follow-ups, suggestions):
✅ Approved |
- 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)
| let Some(pending) = self.pending_attestations.remove(&account_id) else { | ||
| log!( | ||
| "resolve_verification: no pending attestation for {account_id} (late response or already cleaned up); ignoring" | ||
| ); |
There was a problem hiding this comment.
nit(doc): can you please align the design doc (#3715) here — its resolve_verification does .remove(..).expect(..), which would panic on exactly this late-response path (verifier answers after the ~200-block timeout already cleaned up). The graceful return here is the correct behavior; the doc should match.
| // Fail the submitter's transaction from a separate receipt so the | ||
| // cleanup above commits (a panic here would roll it back) | ||
| let promise = Promise::new(env::current_account_id()).function_call( | ||
| method_names::FAIL_ATTESTATION_SUBMISSION.to_string(), | ||
| borsh::to_vec(&reason).expect("borsh serialization of reason must succeed"), | ||
| NearToken::from_yoctonear(0), | ||
| Gas::from_tgas(self.config.fail_attestation_submission_tera_gas), | ||
| ); | ||
| PromiseOrValue::Promise(promise.as_return()) |
There was a problem hiding this comment.
nit(doc): can you please align the design doc (#3715) — it has on_attestation_verified -> Result<(), String> returning Err(reason), but the impl fails the caller's tx via a separate fail_attestation_submission receipt (with its own gas knob). Worth reflecting the actual mechanism so a future reader doesn't implement the return Err version.
| env::promise_yield_resume( | ||
| &pending.data_id, | ||
| serde_json::to_vec(&attestation_result) | ||
| .expect("json serialization of AttestationResult must succeed"), |
There was a problem hiding this comment.
nit(doc): can you please align the design doc (#3715) — it resumes with borsh::to_vec(&FinalOutcome), but the impl uses JSON AttestationResult. Reconcile the name + serializer.
| fn submit_dstack_attestation( | ||
| &mut self, | ||
| node_id: NodeId, | ||
| dstack: DstackAttestation, | ||
| caller_is_participant: bool, | ||
| ) -> Result<(), Error> { | ||
| let account_id = node_id.account_id.clone(); | ||
|
|
||
| if self.pending_attestations.contains_key(&account_id) { | ||
| return Err(TeeError::VerificationAlreadyPending.into()); | ||
| } | ||
|
|
||
| let Some(verifier_account_id) = self.tee_verifier_account_id.clone() else { | ||
| return Err(TeeError::VerifierNotConfigured.into()); |
There was a problem hiding this comment.
nit(doc): can you please align the design doc (#3715) — a few names/fields drifted: finish_verify → verify_post_dcap_and_store, three → four gas knobs, PendingAttestation.caller_is_participant (absent from the doc's field list), and the guards here return VerificationAlreadyPending/VerifierNotConfigured rather than panicking as the doc describes.
|
Node-side follow-up (not this PR): the node polls |
| self.enqueue_yield_request( | ||
| method_names::ON_ATTESTATION_VERIFIED, | ||
| serde_json::to_vec(&(&account_id,)) | ||
| .expect("json serialization of account_id must succeed"), | ||
| Gas::from_tgas(self.config.on_attestation_verified_tera_gas), | ||
| |this, data_id| { | ||
| this.pending_attestations.insert( | ||
| account_id.clone(), | ||
| PendingAttestation { | ||
| dstack, | ||
| tls_public_key, | ||
| attached_deposit, | ||
| caller_is_participant, | ||
| data_id, | ||
| }, | ||
| ); | ||
| }, | ||
| ); | ||
|
|
||
| let attestation_storage_must_be_paid_by_caller = | ||
| is_new_attestation || caller_is_not_participant; | ||
| Ok(()) |
There was a problem hiding this comment.
Using Claude for this one, so take it with a grain of salt — but it flagged something worth verifying.
Issue: submit_participant_info yields here (via enqueue_yield_request → env::promise_return) yet returns Result<(), Error>. On the Ok(()) path, near-sdk's #[handle_result] codegen emits env::value_return(b"null") after the body's promise_return, and last-write-wins — so the yield gets clobbered and the caller's tx resolves immediately as success. The reject/timeout outcome computed later in on_attestation_verified then has nowhere to go. Note the other three yielding methods (sign, request_app_private_key, verify_foreign_transaction) all return () precisely to avoid this — a () method emits no value_return, so promise_return survives.
Net effect: a Dstack submission would always look like immediate success to the caller, regardless of the eventual verifier result.
Suggested fix (mirror sign): make submit_participant_info return () instead of Result<(), Error>, and env::panic_str on the early errors (VerifierNotConfigured, VerificationAlreadyPending, and the Mock-path InsufficientDeposit). A panic already fails the tx with a message — exactly what sign does ("It's important we fail here … This allows users to get the error message"). The on_attestation_verified → fail_attestation_submission half already mirrors sign's fail_on_timeout, so no callback changes are needed.
On the removed tests: a unit test wouldn't have caught this — unit tests call the method directly and bypass the generated wasm entrypoint where value_return/promise_return actually live. This needs a sandbox test that submits a Dstack attestation and asserts the caller's tx parks and then fails on rejection/timeout. Worth confirming #3715 adds exactly that.
There was a problem hiding this comment.
Requesting changes on one blocker (flagged with Claude's help, so sanity-check me).
submit_participant_info yields but returns Result<(), Error>. On Ok(()), near-sdk emits value_return after the yield's promise_return and clobbers it — so the caller's tx returns immediate success and never sees a reject/timeout. The other yield methods (sign, etc.) return () to avoid exactly this. Fix: mirror sign — return () and panic on the early errors. And add a sandbox test; a unit test can't catch this.
Details: #3714 (comment)
Closes #3642.
Splits the first half of #3664 out for focused review: the contract-side async TEE attestation flow and the removal of
dcap-qvlfrommpc-contract. The stub verifier, sandbox tests, and design doc land in a stacked PR on top of this branch.Today the MPC contract verifies a node's Intel TDX attestation synchronously, inside its own WASM, by linking the
dcap-qvllibrary. This PR moves that crypto verification out to a separatetee-verifiercontract and makessubmit_participant_infoasynchronous (yield/resume via a cross-contract call, with refund/revert on failure or the ~200-block timeout). The MPC contract stops linkingdcap-qvlentirely.WASM size impact
Dropping
dcap-qvlshrinks the contract WASM by ~296 KB (-19.6%). Both binaries built identically (cargo near build non-reproducible-wasm --features abi --profile=release-contract --locked, samewasm-opt -Opost-step):main(v3.13.0)cargo tree -p mpc-contract -i dcap-qvlreturns no match on this branch (it resolves onmain), confirmingdcap-qvlis no longer in the contract's build graph.