feat: integrate the TEE attestation verifier contract into mpc-contract#3540
feat: integrate the TEE attestation verifier contract into mpc-contract#3540pbeza wants to merge 26 commits into
Conversation
Move the heavy `dcap_qvl::verify::verify` call out of the always-compiled attestation path and behind an off-chain `local-verify` feature, so the post-DCAP checks can run against a `VerifiedReport` supplied by the verifier contract (on-chain) or by local DCAP (off-chain). - `DstackAttestation` / `Attestation` gain `verify_with_report` (pure, no dcap-qvl) and `verify_locally` (off-chain, `local-verify`). The contract temporarily keeps calling `verify_locally`; the cross-contract verifier call that removes dcap-qvl from the contract WASM lands in a later step. - Collapse the duplicate `Collateral` / `QuoteBytes` definitions: `attestation` now re-exports the `tee-verifier-interface` types instead of wrapping `dcap_qvl::QuoteCollateralV3`, removing dcap-qvl from `attestation`'s default dependency graph. The interface crate gains an off-by-default `serde` feature (input types only) for the node's `/public_data` payload. - dcap<->interface conversions get an `attestation`-local copy (kept out of the minimal verifier contract on purpose), pinned by a borsh-layout test. Behavior-neutral: off-chain callers (node, tee-authority, attestation-cli) and the contract verify exactly as before.
Bounds how long a wrongly-accepted attestation (e.g. one let through by a since-rotated, buggy verifier) stays trusted before it ages out via re_verify, without any sweep. The window stays far above the node's hourly resubmission cadence (ATTESTATION_RESUBMISSION_INTERVAL = 1h), so honest nodes refresh with comfortable margin. The node's storage-time heuristic in tx_sender reads the same constant and the checked_sub stays safe. Decouples the tee-authority MAX_COLLATERAL_AGE doc note, which no longer needs to match this window.
…rmant) Introduces the contract state and governance for the upcoming async attestation flow, without yet routing submit_participant_info through it: - New MpcContract fields: tee_verifier_account_id (the trusted verifier the contract will call verify_quote on), tee_verifier_votes, and an empty pending_attestations map. New append-only storage keys for each. - vote_tee_verifier_change(candidate, expected_code_hash) and withdraw_tee_verifier_vote, mirroring the foreign-chain provider vote on the generic Votes primitive. Crossing threshold sets tee_verifier_account_id and clears the round; expected_code_hash binds each yes-vote to audited code. - Post-resharing sweep of stale verifier votes via clean_foreign_chain_data. - Migration starts deployed contracts from a placeholder verifier account; participants vote in a real one. Fresh deploys may set it via InitConfig. - PendingAttestation / FinalOutcome types for the later async flow. Unit-tested: vote threshold crossing, same-account-different-hash isolation, re-vote replacement, withdraw, post-reshare retain, and borsh round-trips. The contract ABI snapshot and sandbox migration/upgrade tests must be regenerated in CI (the WASM build requires the contract toolchain).
submit_participant_info no longer runs dcap_qvl in the contract WASM. Mock attestations are still verified synchronously (no DCAP); Dstack attestations now go async: - submit_participant_info returns PromiseOrValue<()>. The Dstack arm rejects a duplicate in-flight submission and an unset (placeholder) verifier, stashes a PendingAttestation, registers a yield, and fires a cross-contract verify_quote call whose .then bridges into resolve_verification. - resolve_verification owns the answered outcomes: Verified runs the post-DCAP checks (via Attestation::verify_with_report) against fresh policy and stores + resumes Ok, or refunds + resumes Err on a post-DCAP failure; Rejected refunds + resumes Err immediately; a non-answer logs and defers to the yield timeout. - on_attestation_verified is the trivial yield-callback; its timeout branch does the deferred cleanup + refund. - tee_state gains add_mock_participant / finish_dstack_verify / store_verified_ attestation (split from add_participant, which is now a test-only Mock shim). - Storage charging is preserved (charge only when new or non-participant; actual delta; refund excess) and relocated into the verified path; the deposit and participant flag are stashed in PendingAttestation. - New Config gas knobs for the verifier call and the two callbacks. dcap-qvl is now absent from mpc-contract's normal dependency graph. The Dstack end-to-end paths move to sandbox tests (added next); Mock paths and the post-DCAP logic stay unit-tested. ABI snapshot + sandbox tests regenerate in CI.
…on flow Adds the test-tee-verifier stub contract (returns a test-chosen verify_quote answer instead of running dcap-qvl, speaking the same Borsh DTOs as the real verifier) and sandbox tests that drive the async submit_participant_info paths: verifier-not-configured rejection, verifier Rejected (refund, nothing stored), and verifier crash / no-verdict (yield-timeout cleanup). The Verified + post-DCAP-pass path needs a fixture-matching stub report and is a tracked follow-up; the post-DCAP logic stays unit-tested in mpc-attestation. Sandbox tests require the contract WASM toolchain and run in CI.
- attestation-cli full-verification test asserted the 7-day expiry literal; switch it to DEFAULT_EXPIRATION_DURATION_SECONDS so it tracks the constant (now 1 day) like the mpc-attestation integration tests. - expect(large_enum_variant) on the StubResponse enums (stub crate + sandbox mirror): the Verified(VerifiedReport) variant is large by design.
There was a problem hiding this comment.
Pull request overview
Integrates the standalone TEE attestation verifier contract into mpc-contract by moving DCAP quote verification out of the contract WASM and into a cross-contract verify_quote call, while keeping post-DCAP policy checks in the contract/attestation crates. This aligns the on-chain attestation flow with docs/design/attestation-verifier-contract.md and reduces the contract’s dependency closure.
Changes:
- Refactors attestation verification into a pure post-DCAP path (
verify_with_report) plus an off-chain-only local DCAP path (verify_locallyvialocal-verifyfeature). - Adds verifier governance + async Dstack submission flow in
mpc-contract(trusted verifier account, voting, pending attestation state, yield/callback resolution). - Adds a test-only stub verifier contract and sandbox tests covering key async branches (not configured / rejected / crash-timeout).
Reviewed changes
Copilot reviewed 49 out of 50 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/design/attestation-verifier-contract.md | Updates design doc status and remaining follow-ups. |
| crates/test-utils/src/contract_types.rs | Extends dummy config with verifier/yield callback gas fields. |
| crates/test-utils/src/attestation.rs | Updates quote/collateral test parsing to match new DTO ownership and serde constraints. |
| crates/test-tee-verifier/src/lib.rs | Adds a stub verifier contract returning canned Borsh results for sandbox tests. |
| crates/test-tee-verifier/Cargo.toml | Defines the stub verifier crate build (cdylib) and deps. |
| crates/tee-verifier-interface/src/lib.rs | Adds optional off-by-default serde derives for input DTOs only. |
| crates/tee-verifier-interface/Cargo.toml | Adds optional serde feature with no_std-compatible serde config. |
| crates/tee-authority/src/tee_authority.rs | Switches to new local-verify API and explicit DCAP/interface conversion helpers. |
| crates/tee-authority/Cargo.toml | Enables mpc-attestation/local-verify for off-chain verification. |
| crates/node/src/trait_extensions/convert_to_contract_dto.rs | Adapts collateral DTO conversion to new interface-owned type. |
| crates/node/src/tee/remote_attestation.rs | Switches node verification to verify_locally. |
| crates/node/Cargo.toml | Enables mpc-attestation/local-verify for node-side verification. |
| crates/near-mpc-contract-interface/src/types/config.rs | Adds init/config fields for verifier gas + optional verifier account id. |
| crates/near-mpc-contract-interface/src/method_names.rs | Adds method-name constants for verifier voting + async callbacks + verify_quote. |
| crates/mpc-attestation/tests/test_attestation_verification.rs | Updates integration tests to use verify_locally and adds equivalence test vs verify_with_report. |
| crates/mpc-attestation/src/report_data.rs | Gates quote-parsing unit test behind local-verify and simplifies fixture handling. |
| crates/mpc-attestation/src/lib.rs | Re-exports DCAP conversion helpers only under local-verify. |
| crates/mpc-attestation/src/attestation.rs | Splits verification entry points (verify_with_report, verify_mock_only, verify_locally) and adjusts expiry window to 1 day. |
| crates/mpc-attestation/Cargo.toml | Adds local-verify feature and wires dev-deps for local verification tests. |
| crates/contract/tests/sandbox/utils/mpc_contract.rs | Adds sandbox helper for voting in a verifier. |
| crates/contract/tests/sandbox/utils/contract_build.rs | Builds and caches the stub verifier WASM for sandbox tests. |
| crates/contract/tests/sandbox/upgrade_from_current_contract.rs | Updates config proposal struct to include new gas fields. |
| crates/contract/tests/sandbox/tee_verifier.rs | Adds sandbox tests for async Dstack submission branches using stub verifier. |
| crates/contract/tests/sandbox/mod.rs | Registers the new sandbox test module. |
| crates/contract/tests/sandbox/contract_configuration.rs | Updates init config test to include new gas fields and clarify verifier id behavior. |
| crates/contract/tests/inprocess/attestation_submission.rs | Adjusts in-process tests for PromiseOrValue return type (mock path remains synchronous). |
| crates/contract/src/v3_11_2_state.rs | Adds migration defaults for verifier account/votes/pending map. |
| crates/contract/src/tee/verifier_votes.rs | Implements threshold voting primitive for selecting the trusted verifier account. |
| crates/contract/src/tee/tee_state.rs | Splits participant insertion into mock-sync vs Dstack-post-verifier storage paths. |
| crates/contract/src/tee/pending_attestation.rs | Introduces pending attestation state + yield final outcome type. |
| crates/contract/src/tee.rs | Exposes new tee modules (pending attestation + verifier votes). |
| crates/contract/src/storage_keys.rs | Adds storage keys for verifier votes and pending attestations. |
| crates/contract/src/lib.rs | Implements async submit_participant_info for Dstack (yield + cross-contract verifier + callbacks) and adds verifier voting methods/state. |
| crates/contract/src/errors.rs | Adds new TEE errors for verifier configuration and in-flight verification. |
| crates/contract/src/dto_mapping.rs | Updates collateral mapping + config mapping for new gas fields. |
| crates/contract/src/config.rs | Introduces default gas constants for verifier call + callbacks. |
| crates/contract/Cargo.toml | Adds dependencies needed for verifier DTOs and refactored attestation usage. |
| crates/attestation/tests/collateral.rs | Updates collateral parsing tests to use new parsing helpers. |
| crates/attestation/src/quote.rs | Re-exports QuoteBytes from tee-verifier-interface. |
| crates/attestation/src/measurements.rs | Switches Measurements conversion to interface VerifiedReport. |
| crates/attestation/src/lib.rs | Adds dcap_conversions module behind local-verify. |
| crates/attestation/src/dcap_conversions.rs | Adds explicit interface↔dcap conversion helpers + Borsh layout pinning tests. |
| crates/attestation/src/collateral.rs | Re-exports interface Collateral and moves JSON/hex parsing into off-chain helpers. |
| crates/attestation/src/attestation.rs | Splits Dstack verification into verify_with_report + verify_locally + dcap_report behind local-verify. |
| crates/attestation/Cargo.toml | Makes dcap-qvl optional behind local-verify and enables interface serde for node /public_data. |
| crates/attestation-cli/tests/test_verification.rs | Updates expiry expectation to use the new constant. |
| crates/attestation-cli/src/verify.rs | Switches CLI to end-to-end local verification via verify_locally. |
| crates/attestation-cli/Cargo.toml | Enables mpc-attestation/local-verify for the CLI. |
| Cargo.toml | Adds the new test-tee-verifier crate to the workspace. |
| Cargo.lock | Updates lockfile for new crates/features/dependencies. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| match self.charge_attestation_storage( | ||
| &account_id, | ||
| initial_storage, | ||
| insertion, | ||
| caller_is_not_participant, |
There was a problem hiding this comment.
Good catch — fixed in 186eb40. store_verified_attestation now returns the displaced previous entry, and finish_verified_attestation reverts the store (tee_state.revert_dstack_store) when charge_attestation_storage fails, so the receipt no longer commits a free store. Added a sandbox assertion that a rejected submission stores nothing and refunds the deposit.
| let account_id = node_id.account_id.clone(); | ||
| let final_outcome = match result { | ||
| Err(promise_err) => { | ||
| // No verdict; let the yield timeout clean up. Do NOT resume, or | ||
| // we'd race the timeout for ownership of the cleanup path. | ||
| log!("verifier did not answer for {account_id}: {promise_err:?}"); | ||
| return; | ||
| } | ||
| Ok(VerificationResult::Rejected(reason)) => { | ||
| log!("verifier rejected quote for {account_id}: {reason}"); | ||
| FinalOutcome::Err(format!("verifier rejected quote: {reason}")) | ||
| } | ||
| Ok(VerificationResult::Verified(report)) => { | ||
| self.finish_verified_attestation(node_id, &report) | ||
| } | ||
| }; | ||
|
|
||
| let pending = self | ||
| .pending_attestations | ||
| .remove(&account_id) | ||
| .expect("PendingAttestation must exist while resolve_verification holds the yield"); |
There was a problem hiding this comment.
Fixed in 186eb40. resolve_verification now checks pending_attestations.contains_key up front and returns a logged no-op if the entry is gone (late response after the timeout already cleaned up), instead of .expect-panicking.
Pull request overviewSplits DCAP verification out of mpc-contract WASM by offloading the cryptographic step to a separate trusted tee-verifier contract. The post-DCAP checks remain in mpc-contract, which calls verify_quote cross-contract and resumes a yielded promise from the .then callback. Adds threshold-vote machinery (vote_tee_verifier_change + expected_code_hash commitment) so participants pick the verifier, drops the attestation expiry from 7d to 1d, and ships a test-only stub verifier for sandbox coverage. dcap-qvl is removed from mpc-contracts graph. Changes:
Blocking findings (must fix before merge):
Non-blocking (nits, follow-ups, suggestions):
Issues found. |
Under --all-features the contract enables `abi`, where #[near(serializers=[borsh])] also derives BorshSchema. PendingAttestation embeds DstackAttestation, which has no BorshSchema, so ABI generation failed to compile. These are internal-only state types never present in a public method signature, so they don't need a schema — switch them to plain #[derive(BorshSerialize, BorshDeserialize)].
…inalOutcome - FinalOutcome reaches ABI schema generation as on_attestation_verified's #[callback_result] arg, so derive BorshSchema under `abi`. - Give test-tee-verifier an `abi` feature (mirroring the real verifier) so --all-features enables borsh-schema for the wire DTOs, and derive BorshSchema on StubResponse under it. - Use #[expect(dead_code)] over #[allow] on the sandbox stub mirror's unused Verified variant.
Reflects the added MpcContract fields (tee_verifier_account_id, tee_verifier_votes, pending_attestations), the TeeVerifierVotes / Votes<AuthenticatedParticipantId> schema types, and the new Config gas knobs. Diff reviewed: only the intended additions.
The verifier integration added three gas fields to Config, changing its borsh layout. The migration shadow struct embedded the *current* Config, so deployed (pre-change) state failed to deserialize and migrate() panicked — the upgrade was rolled back (caught by the contract_upgrade_compatibility e2e test). Shadow the old 13-field Config as OldConfig and map it to the current Config, filling the new gas fields from defaults.
The check requires `TODO(#NNNN):` (with colon) or `TODO:`; two parenthetical prose mentions of issue numbers tripped it. Reworded to "see issue #NNNN".
The repo forbids `use` inside fn bodies. Move the dcap-conversion trait import to a feature-gated module-level use in `attestation`, hoist the report_data test's imports to the test module (aliasing the quote fixture to avoid shadowing), and drop a redundant in-fn import in the mpc-attestation integration test.
Reflects the new public/private methods (vote_tee_verifier_change, withdraw_tee_verifier_vote, resolve_verification, on_attestation_verified) and submit_participant_info's PromiseOrValue return, plus the InitConfig fields. Reconstructed from the CI-generated ABI (the local toolchain can't run `cargo near build`: the WASM build needs rustc <=1.86 while ABI generation needs >=1.88); every unchanged and deleted line was verified byte-for-byte against the prior snapshot, so the diff is exactly the intended additions.
CI runs `cargo fmt` under the nightly toolchain, which formats a few constructs differently from stable (import-block wrapping, comment spacing). Reformat the three affected files to match.
- resolve_verification no longer links the private submit_dstack_attestation. - AcceptedDstackAttestation / AcceptedAttestation docs point at verify_with_report (verify was split into verify_with_report / verify_locally). - dcap_conversions module doc uses plain code spans for the sibling file path and the private IntoDcapType/IntoInterfaceType traits instead of intra-doc links.
The doc-link fix to resolve_verification changed its rustdoc, which the ABI embeds; update the snapshot's matching doc string. (Only the doc text differs; no schema change.)
The verifier-rejection and verifier-crash sandbox tests asserted ExecutionFinalResult::is_failure() on the original submit_participant_info call, but those outcomes are resolved in the yield-resume receipt (not the outer transaction), so that flag is runtime-dependent. Assert the reliable invariant instead: a rejected/non-verdict quote is never stored. (The not-configured test keeps is_failure() — it rejects synchronously.)
|
@claude review |
| Err(promise_err) => { | ||
| // No verdict; let the yield timeout clean up. Do NOT resume, or | ||
| // we'd race the timeout for ownership of the cleanup path. | ||
| log!("verifier did not answer for {account_id}: {promise_err:?}"); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Kept the early-return here by design (rather than resuming immediately on Err(PromiseError)). Resuming from resolve_verification would race the runtime's yield timeout for ownership of the cleanup: the timeout always fires on_attestation_verified exactly once per data_id, and that callback now does the cleanup + refund itself (fixed in 186eb40). If resolve_verification also resumed on a non-answer, both paths could try to resolve/clean the same yield. So the non-answer case is intentionally routed to a single owner — the timeout callback. The trade-off you note is real (the PendingAttestation lingers up to ~200 blocks), but it's bounded and avoids the double-ownership hazard; this mirrors the existing sign-request flow. The genuine bug nearby — the late answer after timeout — is fixed separately by the contains_key guard.
| attached_deposit, | ||
| ) { | ||
| Ok(()) => FinalOutcome::Ok, | ||
| Err(err) => FinalOutcome::Err(format!("{err:?}")), |
There was a problem hiding this comment.
Fixed in 186eb40 — uses err.to_string() (Display) instead of {err:?}.
|
Pull request overview Splits DCAP verification out of Changes:
Findings Blocking (must fix before merge):
Non-blocking (nits, follow-ups, suggestions):
Issues found |
Addresses review findings on the async submit_participant_info flow (both Copilot and claude[bot] flagged these; confirmed against the code): - Timeout cleanup was rolled back: on_attestation_verified did remove + refund and then returned Err under #[handle_result], panicking the same receipt and undoing the cleanup — leaking the PendingAttestation, stranding the deposit, and locking the account out (the contains_key gate keeps firing). Mirror the sign-flow pattern: clean up in this receipt, then fail via a separate fail_on_attestation_timeout receipt so the cleanup commits. - Storage-staking bypass: finish_dstack_verify inserted into stored_attestations before the deposit check; on InsufficientDeposit the receipt still committed, so a valid attestation with too small a deposit got stored for free and fully refunded. store_verified_attestation now returns the displaced entry and the callback reverts the store (revert_dstack_store) when the charge fails. - Late-response panic: a verifier response arriving after the yield timeout cleaned up the pending entry hit .expect(...) and panicked. resolve_verification now bails gracefully when no pending entry exists. Nits: use Display (not Debug) for the charge error; consolidate the submit_participant_info doc above its attributes; relax the enqueue_yield_request doc; drop a wrong issue ref; document the frozen caller_is_not_participant and the no-min-deposit DoS surface. Tests: add a sandbox-only has_pending_attestation view; the rejection and crash tests now assert the pending entry is cleaned up and the deposit refunded (the crash test fast-forwards past the yield timeout), guarding the regressions above.
Reflects on_attestation_verified's new PromiseOrValue<()> return + doc, the new fail_on_attestation_timeout method, and the consolidated submit_participant_info doc. Reconstructed from the CI-generated ABI; unchanged/deleted lines verified byte-for-byte. (has_pending_attestation is sandbox-only, correctly absent.)
…rifier-contract # Conflicts: # crates/contract/src/lib.rs # crates/contract/src/v3_11_2_state.rs
|
@claude review |
| let (quote, collateral) = (dstack.quote.clone(), dstack.collateral.clone()); | ||
| let attached_deposit = env::attached_deposit(); | ||
| let tls_public_key = node_id.tls_public_key.clone(); | ||
|
|
||
| self.enqueue_yield_request( | ||
| method_names::ON_ATTESTATION_VERIFIED, | ||
| borsh::to_vec(&account_id).expect("borsh 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_not_participant, | ||
| data_id, | ||
| }, | ||
| ); | ||
| }, |
There was a problem hiding this comment.
Acknowledged — this is the known DoS surface, and it's already called out with an in-code comment at the top of submit_dstack_attestation (the one-in-flight-per-account guard bounds per-account concurrency but not aggregate gas burn). Charging for the temporary pending-entry storage at submit time and refunding on resolution, or imposing a minimum-deposit floor, is the right hardening, but the exact deposit policy is a deliberate team decision I'd rather not bake in unilaterally here. Leaving it as an explicit, documented follow-up rather than expanding this PR's scope.
| Ok(Collateral { | ||
| pck_crl_issuer_chain: get_str(&v, "pck_crl_issuer_chain")?, | ||
| root_ca_crl: get_hex(&v, "root_ca_crl")?, | ||
| pck_crl: get_hex(&v, "pck_crl")?, | ||
| tcb_info_issuer_chain: get_str(&v, "tcb_info_issuer_chain")?, | ||
| tcb_info: get_str(&v, "tcb_info")?, | ||
| tcb_info_signature: get_hex(&v, "tcb_info_signature")?, | ||
| qe_identity_issuer_chain: get_str(&v, "qe_identity_issuer_chain")?, | ||
| qe_identity: get_str(&v, "qe_identity")?, | ||
| qe_identity_signature: get_hex(&v, "qe_identity_signature")?, | ||
| pck_crl_issuer_chain: get_str(&v, "pck_crl_issuer_chain")?, | ||
| root_ca_crl: get_hex(&v, "root_ca_crl")?, | ||
| pck_crl: get_hex(&v, "pck_crl")?, | ||
| pck_certificate_chain: get_str(&v, "pck_certificate_chain").ok(), | ||
| }; | ||
| Ok(Self(quote_collateral)) | ||
| }) |
There was a problem hiding this comment.
Good observation, but the .ok() on the optional pck_certificate_chain predates this PR (it was already present in c8f8186); this PR only moved the collateral parser, preserving the behavior byte-for-byte. The field is informational/passthrough — it's carried through the DTO and conversions, not consumed by DCAP verification itself — so a wrong-typed value can't weaken quote validation, only get silently dropped. Tightening optional-field parsing to reject wrong JSON types (vs. missing/null) is a reasonable hardening, but it's a pre-existing behavior change to the attestation crate that's out of scope for this integration PR; better as its own follow-up so it can be reviewed and tested on its own.
Pull request overviewIntegrates the standalone Changes:
FindingsI verified that the two previously-flagged blocking issues (storage-staking bypass in Blocking (must fix before merge):
Non-blocking (nits, follow-ups, suggestions):
Issues found. |
…ome borsh layout Reject the placeholder account (unset.tee-verifier.invalid) as a vote_tee_verifier_change candidate so a quorum cannot roll the trusted verifier back to the unconfigured state. Add a byte-level borsh pin for FinalOutcome (serialized into the live yield-resume payload) so a future variant reorder fails loudly instead of silently breaking in-flight callback receipts across an upgrade. Tighten the resolve_verification expect message and cross-reference the rotation in-flight semantics from the design doc's status banner.
|
@claude review |
| @@ -765,10 +830,6 @@ impl MpcContract { | |||
| account_key | |||
| ); | |||
|
|
|||
| // Save the initial storage usage to know how much to charge the proposer for the storage | |||
| // used | |||
| let initial_storage = env::storage_usage(); | |||
|
|
|||
| let tee_upgrade_deadline_duration = | |||
| Duration::from_secs(self.config.tee_upgrade_deadline_duration_seconds); | |||
|
|
|||
| @@ -782,62 +843,329 @@ impl MpcContract { | |||
| } | |||
| })?; | |||
|
|
|||
| // Add the participant information to the contract state | |||
| let attestation_insertion_result = self | |||
| .tee_state | |||
| .add_participant( | |||
| NodeId { | |||
| account_id: account_id.clone(), | |||
| tls_public_key, | |||
| account_public_key, | |||
| }, | |||
| proposed_participant_attestation, | |||
| tee_upgrade_deadline_duration, | |||
| let node_id = NodeId { | |||
| account_id: account_id.clone(), | |||
| tls_public_key, | |||
| account_public_key, | |||
| }; | |||
| // Frozen at submit time and consumed later in the resolution callback. | |||
| // The callback receipt's predecessor is the contract itself, so participant | |||
| // status cannot be re-derived there — capture it now. A resharing that | |||
| // drops this submitter mid-flight therefore won't reclassify the storage | |||
| // charge, which is acceptable (the alternative is unavailable). | |||
| let caller_is_not_participant = self.voter_account().is_err(); | |||
|
|
|||
| match proposed_participant_attestation { | |||
| Attestation::Mock(mock) => { | |||
| // Synchronous path: no DCAP, store immediately. | |||
| let initial_storage = env::storage_usage(); | |||
| let insertion = self | |||
| .tee_state | |||
| .add_mock_participant(node_id, mock, tee_upgrade_deadline_duration) | |||
| .map_err(map_attestation_submission_error)?; | |||
| self.charge_attestation_storage( | |||
| &account_id, | |||
| initial_storage, | |||
| insertion, | |||
| caller_is_not_participant, | |||
| env::attached_deposit(), | |||
| )?; | |||
| Ok(PromiseOrValue::Value(())) | |||
| } | |||
| Attestation::Dstack(dstack) => { | |||
| Ok(self.submit_dstack_attestation(node_id, dstack, caller_is_not_participant)?) | |||
| } | |||
There was a problem hiding this comment.
Good instinct to scrutinize this, but I dug into the near-sdk codegen + nearcore VM semantics and the yield/timeout/callback flow is not broken — returning Ok(PromiseOrValue::Value(())) after enqueue_yield_request is safe here.
You're right about the first half: for a Call method, #[handle_result] codegen unwraps Ok(x), serializes x, and calls env::value_return(&result). For PromiseOrValue::Value(()) the bytes are empty, so a redundant value_return(&[]) does fire after our promise_return. And in nearcore (near-vm-runner .../logic.rs) both promise_return and value_return just overwrite return_data with no guard (last writer wins), so the method's return_data ends up Value([]) rather than the yield receipt index.
The part that makes it harmless: promise_yield_create enqueues the yield as a runtime receipt — with its ~200-block timeout and its callback — via ext.create_promise_yield_receipt(...), tracked independently of return_data. Overwriting return_data to Value([]) does not cancel that receipt. The yield still times out and fires on_attestation_verified, and resolve_verification's .then still resumes it via promise_yield_resume(data_id, ..). return_data only governs what the current execution returns to its caller; it has no bearing on receipts already queued by promise_yield_create.
This is also covered empirically. submit_participant_info__cleans_up_on_verifier_crash submits a Dstack attestation against a panicking stub verifier, fast_forward(250) past the timeout, and asserts the pending entry is cleaned up and the deposit refunded — which can only happen if the yield was created and its timeout callback fired. If value_return([]) had completed the call immediately and broken the yield, that callback would never run and the test would fail. It passes in CI, as does the rejection test (which depends on the .then resume landing). Those two are the regression guard for exactly this concern.
So the only residual is cosmetic: an empty value_return that gets overwritten-but-unused (nothing reads this method's tx-return today — the node observes the stored/pending state, not the return value). I considered refactoring the Dstack path to return PromiseOrValue::Promise(..) and drop the manual promise_return to match sign, but enqueue_yield_request is shared with sign and the rework has real blast radius for no functional gain, so I'm leaving it as-is.
| } | ||
|
|
||
| /// Result of a successful [`Attestation::verify`] call. | ||
| /// Result of a successful [`Attestation::verify_with_report`] call. |
Pull request overviewIntegrates the standalone Changes:
Reviewed changesPer-file summary (key files)
FindingsI re-verified that the three previously flagged blocking issues are correctly addressed by 186eb40 and cc18ebb:
Two prior nits also landed: Blocking (must fix before merge):
Non-blocking (nits, follow-ups, suggestions):
✅ Approved |
| /// `report` must be the verifier's output for *this* attestation's quote | ||
| /// and collateral; the checks below bind it to the expected report data, | ||
| /// the embedded TCB info, and the accepted measurement sets. | ||
| pub fn verify_with_report( |
There was a problem hiding this comment.
I wonder if there is a better name, I had to read the description to understand what "with_report" means.
but I don't have any suggestions :-(
| /// offloads to the verifier) and the post-DCAP checks | ||
| /// ([`verify_with_report`](Self::verify_with_report)). | ||
| #[cfg(feature = "local-verify")] | ||
| pub fn dcap_report(&self, timestamp_seconds: u64) -> Result<VerifiedReport, VerificationError> { |
There was a problem hiding this comment.
maybe call it dcap_verify_report or dcap_verify_quote?
|
@pbeza marking this as draft as I believe this was split in several PRs |
Overview
Integrates the standalone TEE attestation verifier (
tee-verifier#3237,tee-verifier-interface#3235) intompc-contract, followingdocs/design/attestation-verifier-contract.md.mpc-contractno longer runsdcap_qvl::verifyinside its own WASM — it offloads DCAP verification to the verifier contract over a cross-contract call, bringing the WASM under the NEP-509 transaction-size pressure.dcap-qvlis now absent frommpc-contract's normal dependency graph.This is one branch built in four conceptual layers; it can be reviewed/split layer by layer. Each layer leaves
mainrunnable.What changed
L1 — split DCAP from post-DCAP (
refactor(attestation))attestation/mpc-attestationgainverify_with_report(pure, nodcap-qvl, used by the contract) andverify_locally(off-chainlocal-verifyfeature, used by node / tee-authority / attestation-cli).attestation::Collateral/QuoteBytesinto re-exports of thetee-verifier-interfacetypes (single source of truth). The interface crate gains an off-by-defaultserdefeature for the node's/public_datapayload; external callers and the verifier contract still link onlyborsh+derive_more.dcap-qvlmoves behind thelocal-verifyfeature, out ofattestation's default graph. Newdcap_conversionsboundary translates between the interface DTOs anddcap-qvltypes, pinned by byte-equal borsh-layout tests.L2 — expiry window 7d → 1d (
feat(attestation))DEFAULT_EXPIRATION_DURATION_SECONDSto bound how long a wrongly-accepted attestation stays trusted after a verifier rotation, well above the node's hourly resubmit cadence.L3 — verifier state + voting (
feat(contract))tee_verifier_account_id,tee_verifier_votes,pending_attestations.vote_tee_verifier_change(candidate, expected_code_hash)/withdraw_tee_verifier_vote, on the existingVotesprimitive;expected_code_hashbinds each yes-vote to audited code. Stale votes swept post-resharing. The placeholder account is rejected as a vote candidate, so a quorum can't roll the verifier back to the unconfigured state.InitConfig.L4 — async
submit_participant_info+ tests (feat(contract)/test(contract))Mockattestations stay synchronous.Dstackattestations go async: yield + cross-contractverify_quotewhose.thenbridges intoresolve_verification, withon_attestation_verifiedas the yield-callback / timeout cleanup. Storage charging is preserved and relocated into the verified path.fail_on_attestation_timeoutreceipt, so cleanup is never rolled back; a failed storage charge reverts the just-stored attestation; a late verifier response after the timeout already cleaned up is a logged no-op rather than a panic.FinalOutcome's borsh wire layout is byte-pinned (it rides a live yield-resume payload).test-tee-verifierstub contract + sandbox tests for the rejection, crash/timeout, and not-configured branches.Behavior compatibility
The sync→async change is transparent to the node: its
submit_participant_infosuccess criterion polls contract state (get_participant_attestation), not the transaction return value, and tolerates "not yet stored" via existing exponential backoff. E2E usesTeeAuthority::Local(Mock attestations), which stay synchronous, so no verifier deploy is needed there.Verification
Locally green: unit + integration suites for
attestation,mpc-attestation,tee-verifier-interface,tee-verifier,tee-authority,attestation-cli,test-tee-verifier, the in-processmpc-contracttests, and the fullmpc-nodesuite.cargo treeconfirmsdcap-qvlis gone frommpc-contractandattestation, andtee-verifier-interfacedefault edges are onlyborsh+derive_more. Sandbox tests, the contract ABI + borsh-schema snapshots (regenerated by CI — the new vote methods, async callbacks, andInitConfigfields change them), and e2e run in CI.Follow-ups (deferred)
near-mpc-contract-interfacesuperseded bytee-verifier-interface#3494: collapse the JSON-facingnear-mpc-contract-interface::Collateralinto the interface type — an on-the-wire API migration that needs a coordinated node/contract rollout, kept separate.Verified+ post-DCAP-pass branch (needs a fixture-matching stub report); the other branches are covered and the post-DCAP logic is unit-tested.Dstacksubmit path (acknowledged in-code); a deposit floor to amortize the verifier round-trip is left as future hardening.🤖 Generated with Claude Code