feat(contract): TEE verifier contract-account voting - #3626
Conversation
…feature Separate Intel TDX DCAP cryptographic verification from the post-DCAP policy checks so the two can run in different places, and make dcap-qvl an optional dependency. Attestation::verify is split into: - verify_with_report: pure post-DCAP checks against an already-produced VerifiedReport, no dcap-qvl - verify_locally: full local DCAP + post-DCAP, behind the new local-verify feature (used off-chain by node, tee-authority, attestation-cli) - verify_mock_only: the Mock path, always compiled dcap-qvl moves behind local-verify, out of the attestation crate's default dependency graph. A new dcap_conversions module translates between the tee-verifier-interface DTOs and dcap-qvl types, pinned by byte-equal borsh-layout tests. The duplicate attestation Collateral and QuoteBytes newtypes collapse into re-exports of the tee-verifier-interface types (single source of truth); the interface crate gains an off-by-default serde feature for the node's /public_data payload. This is groundwork: mpc-contract's behavior is unchanged. Its synchronous attestation path now calls verify_locally (a byte-identical replacement for the old verify), so it still links dcap-qvl for now. Routing DCAP through a separate verifier contract, and dropping dcap-qvl from mpc-contract, is a follow-up.
…me test; drop a clone Address self-review/Copilot/claude[bot] doc-drift nits on the DCAP split: - verify_locally docs no longer claim mpc-contract calls the verifier contract for DCAP; the contract calls verify_locally today, and the verifier-contract switch is a follow-up - drop the stale 'now 1 day' note on MAX_COLLATERAL_AGE; the contract expiry constant is still 7 days - rename the new integration test to the mandated __should_ form - dcap_report borrows the quote bytes instead of cloning them
verify_mock_only is only valid for Mock attestations; a Dstack input is caller-side misuse, not a verification failure. Add a debug_assert so it fails loudly in debug/test builds, keeping the release error path unchanged.
Extract the duplicated dcap_qvl <-> tee-verifier-interface conversions (previously a deliberate sibling copy in attestation/src/dcap_conversions.rs and tee-verifier/src/conversions.rs) into a new tee-verifier-conversions crate. Both the on-chain verifier contract and the off-chain attestation crate now re-export from it, so the Borsh-layout pin tests live in one place. Replace Attestation::verify_mock_only (which carried a debug_assert! for the impossible Dstack arm) with per-variant verification: MockAttestation::verify and a DstackVerify extension trait. Attestation::verify_with_report and verify_locally dispatch to these, so the invalid "Dstack on the mock path" case no longer exists in the type system.
- Inline verify_mock_attestation into MockAttestation::verify; route re_verify's Mock arm through it. - Add AcceptedAttestation::dstack / ::mock constructors, replacing the free fn accepted_dstack_attestation and the inline struct literals. - Rename DstackAttestation::dcap_report to verify_dcap_quote (it runs DCAP verification and returns the report, rather than being a getter). - TODO(#3264) at the contract's verify_locally call site and on the local-verify feature flags / docs, marking the transitional in-WASM DCAP path the verifier-contract follow-up removes. - Tighten doc comments across the attestation/tee-verifier crates; drop function names and external party names from config-level comments so they don't go stale.
Split the mock constraint check out of `verify` into a private `verify_constraints` returning `Result<(), _>`. `verify` calls it and wraps the result into an `AcceptedAttestation`; `re_verify`'s Mock arm calls it directly, so the periodic on-chain re-verification no longer builds (and discards) an `AcceptedAttestation`.
…DstackVerify::verify The Dstack arm of `Attestation::verify_locally` re-inlined `verify_dstack_mpc_hashes` + the `AcceptedAttestation::dstack` assembly that `DstackVerify::verify` already does. Route it through `verify_dcap_quote` + `DstackVerify::verify` instead, removing the duplication and running DCAP before the post-DCAP checks (matching the async verifier flow). Drop verify_with_report__should_agree_with_verify_locally: after the dedup, verify_locally is verify_dcap_quote plus the same path verify_with_report dispatches to, so the agreement holds by construction.
Adds the on-chain governance for choosing the trusted `tee-verifier` account, ahead of the async verification flow that will use it. Verification stays synchronous (`verify_locally`), so this is behavior-preserving. - New `tee::verifier_votes` module: `VerifierChangeProposal` (candidate account + audited code hash) and `TeeVerifierVotes` on the existing `Votes` primitive, mirroring the foreign-chain provider vote. - `MpcContract` gains `tee_verifier_account_id` (starts at an unset placeholder) and `tee_verifier_votes`. `vote_tee_verifier_change` / `withdraw_tee_verifier_vote` let participants vote one in by threshold; the placeholder is rejected as a candidate so a quorum can't roll the verifier back to unconfigured. Stale votes are swept post-resharing in `clean_foreign_chain_data`. - Migration starts deployed contracts from the placeholder; fresh deploys may set `tee_verifier_account_id` via `InitConfig`. - Lower `DEFAULT_EXPIRATION_DURATION_SECONDS` 7d -> 1d to bound how long a wrongly-accepted attestation stays trusted after a verifier rotation. Regenerates the ABI and borsh-schema snapshots.
There was a problem hiding this comment.
Pull request overview
Adds governance plumbing to let MPC participants choose a trusted tee-verifier contract account (with votes bucketed by (account_id, code_hash)), and reduces the default attestation expiry window from 7 days to 1 day to limit trust duration after verifier rotation.
Changes:
- Introduces
tee::verifier_voteswithVerifierChangeProposal+TeeVerifierVotes, and wires new vote/withdraw methods intoMpcContract(incl. placeholder-candidate rejection + post-reshare vote retention). - Adds
tee_verifier_account_idinitialization/migration support (init-time optional config + migration defaulting to an “unset” placeholder). - Lowers
DEFAULT_EXPIRATION_DURATION_SECONDSto 1 day and updates affected tests/snapshots (ABI + borsh schema).
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/near-mpc-contract-interface/src/types/config.rs | Adds optional tee_verifier_account_id to InitConfig and updates serde round-trip tests. |
| crates/near-mpc-contract-interface/src/method_names.rs | Adds method-name constants for verifier voting/withdraw. |
| crates/mpc-attestation/src/attestation.rs | Lowers default attestation expiry from 7d to 1d with rationale comment. |
| crates/contract/tests/snapshots/abi__abi_has_not_changed.snap | Regenerates ABI snapshot to include new methods and init field. |
| crates/contract/tests/sandbox/contract_configuration.rs | Adjusts sandbox init config construction to include the new optional field. |
| crates/contract/src/v3_12_0_state.rs | Migration: initializes new verifier fields for pre-existing deployed state. |
| crates/contract/src/tee/verifier_votes.rs | New module implementing verifier-account vote tracking on top of Votes. |
| crates/contract/src/tee.rs | Exposes the new verifier_votes module. |
| crates/contract/src/storage_keys.rs | Adds new storage prefixes for verifier votes. |
| crates/contract/src/snapshots/mpc_contract__tests__mpc_contract_borsh_schema_has_not_changed.snap | Updates borsh schema snapshot for new MpcContract fields and types. |
| crates/contract/src/lib.rs | Adds verifier fields + init helper, public vote/withdraw methods, and post-reshare vote retention. |
| crates/contract/src/errors.rs | Adds TeeError::VerifierCandidateIsPlaceholder. |
| crates/attestation-cli/tests/test_verification.rs | Updates expected expiry calculation to use DEFAULT_EXPIRATION_DURATION_SECONDS. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- vote_tee_verifier_change: short-circuit when the candidate is already the current verifier, so a no-op re-vote can't clear an in-flight rotation. - Add a contract-level unit test driving vote_tee_verifier_change through threshold and asserting the verifier account flips. - Add a strict-subset case to the TeeVerifierVotes::retain test. - Use rustdoc intra-doc links for non-public item references; note the expiry constant's node-side round-trip; fix two doc typos; tighten the clear-on-threshold comment. - Regenerate the ABI snapshot for the reworded method doc.
Replace the raw CryptoHash on vote_tee_verifier_change with a dedicated TeeVerifierCodeHash newtype defined in mpc-primitives, so the verifier code hash a participant audited is typed end to end. Regenerate the contract ABI snapshot for the new signature, deduplicate the verifier_votes tests, and clarify the DEFAULT_EXPIRATION_DURATION_SECONDS doc comment
…shold lookup Replace the unset.tee-verifier.invalid placeholder with Option<AccountId>: None now denotes the unconfigured state, so the rollback-to-placeholder guard and the VerifierCandidateIsPlaceholder error are gone (a vote only ever carries a real AccountId, so it can never unset the verifier). The migration starts from None. Collapsing the Option back to a plain AccountId once a verifier is voted in is tracked in TODO(#3639). Extract the copy-pasted "threshold_parameters() or panic on NotInitialized" block from the seven vote methods into ProtocolContractState::threshold_parameters_or_panic.
The tee_verifier_account_id doc comment on InitConfig was shortened, which flows into the JSON ABI description via JsonSchema. Update the abi snapshot to match so test_abi_has_not_changed passes.
The field let a fresh deploy seed the trusted verifier account, but nothing used it and it was the only InitConfig field that doesn't round-trip through config() (Config omits it: the verifier is governance state set by the audited-code vote, not a tunable knob). Remove it so the verifier can only be set via vote_tee_verifier_change, and InitConfig and Config become field-identical. The contract's own tee_verifier_account_id field and its migration default (None) are unchanged.
…leanup The tee_verifier_votes.retain sweep was hidden inside clean_foreign_chain_data, whose name and doc are about foreign-chain data. Move it into a dedicated remove_non_participant_tee_verifier_votes cleanup method, spawned as its own detached promise after resharing alongside the sibling cleanups, with its own gas config knob. Each cleanup concern is now self-contained.
Adding the verifier-vote cleanup broke two things: - The sixth detached cleanup promise in vote_reshared pushed total cleanup gas to 39 TGas, over the fixed GAS_FOR_VOTE_RESHARED test budget. Raise it to 50 TGas (production attaches max gas, so it is unaffected). - The new remove_non_participant_tee_verifier_votes_tera_gas field changed Config's borsh layout, so migrate() could no longer deserialize production state written with the old layout. Shadow the old 13-field Config as OldConfig in the v3.12.0 migration state and convert it into the current Config, defaulting the new field.
gilcu3
left a comment
There was a problem hiding this comment.
Thank you!
Left some minor comments. A test that covers the new cleanup step after resharing might be missing, but to do that we need a way to observe these votes, which IIUC is now missing as well, and could be a good addition
netrome
left a comment
There was a problem hiding this comment.
Shallow review, looks good to me - thanks!
…and design doc DEFAULT_EXPIRATION_DURATION_SECONDS was changed 7d -> 1d in #3626. Update the stale '7 days' wording; reference the constant instead of hardcoding a day-count in test comments to avoid future drift. The MPC docker-image grace period (DEFAULT_TEE_UPGRADE_DEADLINE_DURATION_SECONDS) remains 7 days.
Closes #3625. Next slice of #3540, stacked on #3590; each slice leaves
mainrunnable.Why
Moving DCAP verification off the
mpc-contractWASM means offloading it to a standalonetee-verifiercontract over a cross-contract call — which needs a trusted verifier contract account chosen by participants, not hardcoded. This PR adds that governance layer (the voting that picks the verifier contract account).What changed
Verifier-account voting was added (new
tee::verifier_votes, built on the existingVotesprimitive):vote_tee_verifier_change(candidate, expected_code_hash)mpc/crates/contract/src/lib.rs
Lines 1688 to 1736 in 46d04a0
expected_code_hashmakes each voter commit to the code they audited — same account, different hash ⇒ different buckets, so neither reaches threshold alone. Crossing threshold updatestee_verifier_account_idand clears pending votes; stale votes are swept after resharing.withdraw_tee_verifier_vote()mpc/crates/contract/src/lib.rs
Lines 1738 to 1756 in 46d04a0
MpcContractgainstee_verifier_account_idandtee_verifier_votes.mpc/crates/contract/src/lib.rs
Lines 185 to 190 in 46d04a0
Migration and fresh deploys start unconfigured (
tee_verifier_account_id: None); a verifier is set only byvote_tee_verifier_changeonce participants vote one in. Since a vote only ever carries a real account, it can never reset the verifier back to unconfigured.Lower attestation expiry 7d → 1d (
DEFAULT_EXPIRATION_DURATION_SECONDS) — bounds how long a wrongly-accepted attestation stays trusted after a verifier rotation, while staying well above the node's hourly resubmit cadence.No behavior change yet
Verification still runs synchronously in-WASM via
verify_locally.tee_verifier_account_idis stored and voted on, but not yet used for dispatch.The async flow that consumes it, cross-contract
verify_quoteplus the post-DCAP callback, and actually removes DCAP from the WASM, is the next slice of #3540, which will be submitted after this PR.