test(contract): address review on the cross-contract attestation tests

- assert nothing-stored through the typed view: the untyped one cannot
  deserialize a stored Dstack entry, so the exact failure it guards for
  surfaced as a serde panic instead of the assertion
- take the verifier gas budget from the contract's config view and assert
  headroom; a receipt cannot outspend its own prepaid gas, so comparing
  against a hand-copied 200 Tgas asserted nothing and could drift from
  DEFAULT_VERIFIER_TERA_GAS
- match failure substrings that survive Debug formatting and workspaces'
  quote escaping
- bounds-check initial_participant_indices in MpcClusterConfig::validate,
  which also covers the pre-existing indexing in init_contract and
  add_initial_domains
- name the build task in the tee-verifier wasm panic, spell the intra-doc
  link in full so plain cargo doc resolves it, and fix a stale comment in
  Makefile.toml
- state the current design in the verifier design doc rather than narrating
  how it was reached, and say plainly that the store-path coverage is
  inactive until the fixture key lands

Also drops the TODO token from two prose references to #3787, which
scripts/check-todo-format.sh requires to carry a colon.

Author: pbeza

Makefile.toml

@@ -165,7 +165,7 @@ args = ["scripts/check-sandbox-image-version.sh"]
 
 # These build tasks are the single source of truth for both local and CI builds.
 # CI's `mpc-e2e-tests` job invokes them via `cargo make`.
-# All three are skipped when `E2E_SKIP_BUILD` is set (used by `e2e-tests-skip-build`).
+# Each is skipped when `E2E_SKIP_BUILD` is set (used by `e2e-tests-skip-build`).
 
 [tasks.build-mpc-node-network-hardship-simulation]
 description = "Build the mpc-node binary used by the E2E tests"
@@ -203,6 +203,9 @@ args = [
 
 [tasks.build-tee-verifier-optimized]
 description = "Build the tee-verifier WASM for localnet and the E2E tests"
+# `scripts/launch-localnet.sh` tells operators to run this task by hand; with
+# `E2E_SKIP_BUILD` exported it no-ops (cargo-make logs "Skipping Task"), and the E2E
+# loader's panic names this task so a missing WASM points back here.
 condition = { env_not_set = ["E2E_SKIP_BUILD"] }
 command = "cargo"
 args = [

crates/contract/tests/sandbox/tee_verifier.rs

@@ -11,7 +11,7 @@
 //! Verified-path tests that store an attestation must sign as the fixture
 //! account (the quote's report_data binds the fixture account key, and the
 //! contract reads that key from the transaction signer). They are ignored
-//! until the fixture secret key asset lands; see TODO(#3787).
+//! until the fixture secret key asset lands; see #3787.
 #![allow(non_snake_case)]
 
 use crate::sandbox::{
@@ -20,7 +20,7 @@ use crate::sandbox::{
         consts::ALL_PROTOCOLS,
         contract_build::{tee_verifier_contract, tee_verifier_contract_with_sandbox_test_hooks},
         mpc_contract::{
-            get_participant_attestation, get_tee_accounts, get_verified_attestation,
+            get_config, get_participant_attestation, get_tee_accounts, get_verified_attestation,
             submit_participant_info, tee_verifier_account_id, total_gas_fee,
             vote_add_launcher_hash, vote_add_os_measurement, vote_for_hash,
             vote_tee_verifier_change,
@@ -162,15 +162,15 @@ async fn submit_dstack(submitter: &Account, contract: &Contract) -> ExecutionFin
     .unwrap()
 }
 
-/// Asserts a Dstack submission failed cleanly: a receipt failed carrying
-/// `expected_error` (`fail_attestation_submission` panics in its own receipt), no
-/// attestation was stored, and the caller spent only gas.
+/// Asserts a Dstack submission failed cleanly: a receipt failed mentioning every
+/// string in `expected_error` (`fail_attestation_submission` panics in its own
+/// receipt), no attestation was stored, and the caller spent only gas.
 async fn assert_submission_failed_cleanly(
     result: &ExecutionFinalResult,
     contract: &Contract,
     submitter: &Account,
     balance_before: NearToken,
-    expected_error: &str,
+    expected_error: &[&str],
 ) {
     let failures = result.failures();
     assert!(
@@ -180,12 +180,16 @@ async fn assert_submission_failed_cleanly(
     // Substring-match: near-workspaces keeps `ExecutionOutcome.status`
     // `pub(crate)`, so the error is only reachable via the Debug dump.
     let rendered = format!("{failures:?}");
-    assert!(
-        rendered.contains(expected_error),
-        "expected a receipt failure containing {expected_error:?}, got: {rendered}"
-    );
+    for expected in expected_error {
+        assert!(
+            rendered.contains(expected),
+            "expected a receipt failure containing {expected:?}, got: {rendered}"
+        );
+    }
 
-    let stored = get_participant_attestation(contract, &p2p_tls_key().into())
+    // Typed as `VerifiedAttestation`: a wrongly stored Dstack entry must surface as
+    // this assertion, not as a deserialization panic in the view helper.
+    let stored = get_verified_attestation(contract, &p2p_tls_key().into())
         .await
         .unwrap();
     assert!(stored.is_none(), "nothing should be stored on failure");
@@ -291,10 +295,10 @@ async fn submit_participant_info__should_store_nothing_on_verifier_rejection() {
         &contract,
         &submitter,
         balance_before,
-        &TeeError::QuoteRejected {
+        &[&TeeError::QuoteRejected {
             reason: String::new(),
         }
-        .to_string(),
+        .to_string()],
     )
     .await;
 }
@@ -321,15 +325,11 @@ async fn submit_participant_info__should_fail_and_store_nothing_when_verifier_un
         &contract,
         &submitter,
         balance_before,
-        &TeeError::VerifierUnavailable.to_string(),
+        &[&TeeError::VerifierUnavailable.to_string()],
     )
     .await;
 }
 
-/// Mirrors the contract's default `verifier_tera_gas`; the config constants are
-/// crate-private, so the production budget is pinned here by value.
-const VERIFIER_TERA_GAS_BUDGET: u64 = 200;
-
 /// Tolerance for comparing an on-chain expiry stamp against this process's
 /// wall clock (sandbox block time tracks it loosely).
 const EXPIRY_SLACK_SECONDS: u64 = 600;
@@ -354,7 +354,7 @@ async fn submit_participant_info__should_run_dcap_within_verifier_gas_budget() {
     // Then: the real DCAP run succeeds within the production gas budget and its
     // Verified verdict reaches the callback. The submission then fails at the
     // post-DCAP report_data binding, because the submitter does not hold the
-    // fixture account key (TODO(#3787)); that terminal error is asserted to pin
+    // fixture account key (see #3787); that terminal error is asserted to pin
     // that the verdict was Verified, not Rejected.
     let outcomes = result.outcomes();
     let verify_quote_outcome = outcomes
@@ -365,19 +365,28 @@ async fn submit_participant_info__should_run_dcap_within_verifier_gas_budget() {
         verify_quote_outcome.is_success(),
         "verify_quote must succeed, got: {verify_quote_outcome:#?}"
     );
+    // The receipt is created with exactly `verifier_tera_gas` of static gas, so
+    // succeeding already proves it fit the budget. Assert headroom instead, which
+    // is the regression that matters: `dcap-qvl` growing until it OOGs in
+    // production. Read the budget from the contract so it cannot drift from
+    // `DEFAULT_VERIFIER_TERA_GAS`.
+    let budget = Gas::from_tgas(get_config(&contract).await.unwrap().verifier_tera_gas);
+    let headroom = Gas::from_gas(budget.as_gas() / 10);
     assert!(
-        verify_quote_outcome.gas_burnt <= Gas::from_tgas(VERIFIER_TERA_GAS_BUDGET),
-        "verify_quote burnt {} but the production budget is {VERIFIER_TERA_GAS_BUDGET} Tgas",
+        verify_quote_outcome.gas_burnt <= budget.saturating_sub(headroom),
+        "verify_quote burnt {} of the configured {budget}, leaving less than the {headroom} \
+         headroom this test exists to protect. Raise DEFAULT_VERIFIER_TERA_GAS (config.rs) \
+         before the real cost reaches the budget",
         verify_quote_outcome.gas_burnt,
     );
-    // The receipt carries the error's Debug form, with the inner panic message
-    // quote-escaped once by the outer dump.
     assert_submission_failed_cleanly(
         &result,
         &contract,
         &submitter,
         balance_before,
-        r#"WrongHash { name: \"report_data\""#,
+        // Substrings that survive the error's Debug formatting and
+        // near-workspaces' quote escaping.
+        &["failed verification", "report_data"],
     )
     .await;
 }
@@ -412,7 +421,7 @@ async fn submit_participant_info__should_fail_cleanly_when_verifier_gas_budget_t
         &contract,
         &submitter,
         balance_before,
-        &TeeError::VerifierUnavailable.to_string(),
+        &[&TeeError::VerifierUnavailable.to_string()],
     )
     .await;
 }
@@ -494,7 +503,7 @@ async fn submit_participant_info__should_fail_and_store_nothing_when_resolve_ver
         &contract,
         &submitter,
         balance_before,
-        "Exceeded the prepaid gas",
+        &["Exceeded the prepaid gas"],
     )
     .await;
 }

crates/contract/tests/sandbox/utils/mpc_contract.rs

@@ -7,8 +7,8 @@ use near_mpc_contract_interface::{
     client::MpcContractHandle,
     method_names,
     types::{
-        Attestation, Ed25519PublicKey, GovernanceThreshold, Participants, ProtocolContractState,
-        VerifiedAttestation,
+        Attestation, Config, Ed25519PublicKey, GovernanceThreshold, Participants,
+        ProtocolContractState, VerifiedAttestation,
     },
 };
 use near_workspaces::{
@@ -23,6 +23,10 @@ pub fn total_gas_fee(result: &ExecutionFinalResult) -> NearToken {
         .fold(NearToken::from_yoctonear(0), NearToken::saturating_add)
 }
 
+pub async fn get_config(contract: &Contract) -> anyhow::Result<Config> {
+    Ok(contract.view(method_names::CONFIG).await?.json()?)
+}
+
 pub async fn get_state(contract: &Contract) -> ProtocolContractState {
     contract
         .view(method_names::STATE)

crates/e2e-tests/src/cluster.rs

@@ -231,6 +231,15 @@ impl MpcClusterConfig {
                 self.num_nodes,
             );
         }
+        // Startup indexes the key vectors by participant index, so an out-of-range
+        // entry here would otherwise surface as a panic mid-startup.
+        for (i, &participant_idx) in self.initial_participant_indices.iter().enumerate() {
+            anyhow::ensure!(
+                participant_idx < self.num_nodes,
+                "initial_participant_indices[{i}]: index {participant_idx} must be < num_nodes ({})",
+                self.num_nodes,
+            );
+        }
         Ok(())
     }
 }
@@ -245,7 +254,8 @@ pub fn must_load_tee_verifier_wasm() -> Vec<u8> {
         let wasm_path = PathBuf::from(&path);
         return std::fs::read(&wasm_path).unwrap_or_else(|e| {
             panic!(
-                "failed to read tee-verifier WASM at {}: {e}",
+                "failed to read tee-verifier WASM at {}: {e}. Build it with \
+                 `cargo make build-tee-verifier-optimized` (skipped when E2E_SKIP_BUILD is set)",
                 wasm_path.display()
             )
         });
@@ -263,7 +273,8 @@ pub fn must_load_tee_verifier_wasm() -> Vec<u8> {
     }
 
     tracing::info!(
-        "MPC_TEE_VERIFIER_WASM not set and pre-built WASM not found — building tee-verifier"
+        "MPC_TEE_VERIFIER_WASM not set and pre-built WASM not found — building tee-verifier. \
+         Build it up front with `cargo make build-tee-verifier-optimized` to skip this."
     );
     test_utils::contract_build::ContractBuilder::new("crates/tee-verifier/Cargo.toml").build()
 }

crates/tee-verifier/src/lib.rs

@@ -67,7 +67,8 @@ impl TeeVerifier {
 }
 
 /// The timestamp quotes are verified against: block time, unless a sandbox test
-/// pinned one under [`SANDBOX_TEST_PINNED_NOW_STORAGE_KEY`]. The pin exists
+/// pinned one under [`tee_verifier_interface::SANDBOX_TEST_PINNED_NOW_STORAGE_KEY`]
+/// (spelled in full because the import is feature-gated). The pin exists
 /// because sandbox chain time is wall-clock and forward-only, so it can never
 /// fall inside the validity window of a checked-in collateral fixture.
 fn now_seconds() -> u64 {

docs/design/attestation-verifier-contract.md

@@ -611,7 +611,11 @@ The yield-resume split adds four resolution branches the synchronous version nev
 
 The verifier-rotation design changes the test surface in three ways. First, the expiration window itself: an entry whose `expiry_timestamp_seconds` is in the past must be rejected by `re_verify` even when every post-DCAP allowlist invariant still holds, and an entry within the (shortened) window must still pass — this is the existing expiry check, now exercised against the lowered `DEFAULT_EXPIRATION_DURATION_SECONDS`. Second, rotation routing: after `vote_tee_verifier_change` crosses threshold, the next `submit_participant_info` must call `verify_quote` on the new `tee_verifier_account_id`, and existing stored entries must remain present (no purge) until they expire. Third, the in-flight case: a verification scheduled against the old verifier that resolves after the vote crosses threshold must still be stored as a normal entry — it is not treated specially and ages out via the same expiration window as any other entry.
 
-What shipped instead of the stub verifier this section originally proposed: the sandbox tests in `crates/contract/tests/sandbox/tee_verifier.rs` deploy the real `tee-verifier` WASM and drive each verdict through `vote_tee_verifier_change` + `submit_participant_info`: `Rejected` with a malformed quote, no-verdict with an undeployed verifier account, and `Verified` with the fixture quote against a verifier built with the `sandbox-test-hooks` feature, which lets the test pin the timestamp `verify_quote` verifies against (the fixture collateral is expired against live chain time, and sandbox time cannot be wound back). A stub verifier was briefly built and then dropped: it duplicated the real verifier and added maintenance surface for little gain. Verified-path tests that store an attestation additionally sign as the fixture account, because the quote's report_data binds the fixture account key (issue #3787 tracks the key asset).
+Status: no stub verifier exists. The design below was superseded during implementation, because a second contract mirroring the real one duplicated it for little gain.
+
+Sandbox tests in `crates/contract/tests/sandbox/tee_verifier.rs` deploy the real `tee-verifier` WASM and drive each verdict through `vote_tee_verifier_change` + `submit_participant_info`: `Rejected` with a malformed quote, no-verdict with an undeployed verifier account, and `Verified` with the fixture quote. `Verified` needs the verifier built with the `sandbox-test-hooks` feature, which lets the test pin the timestamp `verify_quote` verifies against: the fixture collateral is valid only inside a fixed window, while sandbox time is wall-clock and forward-only.
+
+Tests that assert the *store* additionally sign as the fixture account, because the quote's report_data binds that account key. They are inactive until the fixture secret key asset lands (#3787); until then, the store path is covered in-process instead.
 
 E2E tests in `crates/e2e-tests` deploy the real `tee-verifier` and vote it in during cluster startup for topology parity; nodes there submit mock attestations, which the MPC contract verifies without calling the verifier, so the cross-contract flow is covered at the sandbox layer.