Skip to content

test(contract): cover the Verified attestation path cross-contract in sandbox - #4085

Closed
pbeza wants to merge 7 commits into
mainfrom
3787-sandbox-verified-attestation
Closed

test(contract): cover the Verified attestation path cross-contract in sandbox#4085
pbeza wants to merge 7 commits into
mainfrom
3787-sandbox-verified-attestation

Conversation

@pbeza

@pbeza pbeza commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Closes #4084

Copilot AI lite review requested due to automatic review settings August 7, 2026 08:31
@claude

claude Bot commented Aug 7, 2026

Copy link
Copy Markdown

PR title type suggestion: This PR modifies source code files in crates/tee-verifier/ and crates/tee-verifier-interface/, so the type prefix should probably be feat: instead of test:.

Suggested title: feat(contract): cover the Verified attestation path cross-contract in sandbox

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds end-to-end, gas-metered sandbox coverage for the async attestation flow reaching a Verified verdict through the real tee-verifier contract (including cross-contract callback propagation), using a cfg-gated hook to pin verifier time for fixture validity. Also updates E2E cluster startup to deploy and vote in tee-verifier for topology parity with production, and refreshes docs/build plumbing accordingly.

Changes:

  • Introduce tee-verifier sandbox-test-hooks feature to optionally read a pinned “now” timestamp from storage in sandbox-only builds.
  • Add sandbox (near-workspaces) tests that deploy/vote in the real verifier, whitelist fixture measurements, and assert Verified verdict propagation + gas budgeting (with store-path tests #[ignore] pending fixture secret key).
  • Update E2E cluster startup, CI, build tasks, and docs to consistently build/deploy/vote in the verifier.

Reviewed changes

Copilot reviewed 17 out of 18 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
Makefile.toml Ensures tee-verifier WASM is built for E2E runs and exported via env var.
.github/workflows/ci.yml Adds an explicit CI step to build the tee-verifier WASM.
crates/tee-verifier/Cargo.toml Adds sandbox-test-hooks feature flag.
crates/tee-verifier/src/lib.rs Implements pinned-time hook (feature-gated) and routes verify_quote timestamp via helper.
crates/tee-verifier-interface/src/lib.rs Exposes the shared storage key constant used to pin verifier time in sandbox tests.
crates/contract/tests/sandbox/utils/contract_build.rs Adds a builder path for verifier WASM with sandbox test hooks enabled.
crates/contract/tests/sandbox/utils/mpc_contract.rs Adds typed get_verified_attestation and a vote helper for OS measurements.
crates/contract/tests/sandbox/tee_verifier.rs Adds Verified cross-contract sandbox tests and related setup utilities (with store-path tests ignored pending fixture key).
crates/test-utils/src/attestation.rs Adds runtime-loaded fixture account secret key accessor for sandbox signing (pending asset).
crates/test-utils/assets/README.md Documents adding the fixture account secret key asset and how it’s used by sandbox tests.
crates/test-utils/assets/create-assets.sh Warns when the secret-key asset is missing during asset extraction.
crates/e2e-tests/src/cluster.rs Deploys and votes in tee-verifier at cluster startup; adds verifier WASM loading helper.
crates/e2e-tests/README.md Documents new cluster startup step and config field.
crates/e2e-tests/Cargo.toml Adds sha2 dependency for verifier code-hash computation.
Cargo.lock Records the new sha2 dependency resolution.
docs/localnet/localnet.md Updates verifier docs to reference new cross-contract sandbox coverage.
docs/deploy-tee-verifier.md Updates verifier deploy docs to reference cross-contract sandbox coverage.
docs/design/attestation-verifier-contract.md Updates design doc to reflect the shipped approach (real verifier + sandbox hook), not a stub verifier.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +1404 to +1407
for &i in participant_indices {
let account = format!("node{i}.{SANDBOX_ROOT_ACCOUNT}");
let client = blockchain.client_for(&account, &operator_keys[i])?;
let outcome = contract

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed at the root instead: MpcClusterConfig::validate() now bounds-checks initial_participant_indices, mirroring the existing migration_targets check. That also covers the same indexing in init_contract and add_initial_domains, which predate this PR.

@claude

claude Bot commented Aug 7, 2026

Copy link
Copy Markdown

PR title type suggestion: This PR adds new source code modules (tee-verifier-interface, tee-verifier). Since non-test source code files are changed, the type prefix should be feat instead of test. Suggested title: feat(tee-verifier): add attestation verifier interface and contract integration tests

@claude

claude Bot commented Aug 7, 2026

Copy link
Copy Markdown

Pull request overview

Closes the last gap in cross-contract attestation coverage: the Verified verdict. A new sandbox-test-hooks feature makes tee-verifier's verify_quote read its "now" from a well-known storage key (written by tests via patch_state) instead of block time, which is what lets the time-expired fixture collateral verify against a forward-only sandbox clock. Two sandbox tests run today (a gas-metered valid-quote run at the production verifier_tera_gas, and a too-low-budget clean failure); three store-path tests land #[ignore]d pending the fixture account secret key (#3787). E2E clusters also deploy and vote in the verifier for topology parity, and the design doc / runbooks are realigned with what shipped.

Changes:

  • tee-verifier: sandbox-test-hooks feature; verify_quote sources its timestamp from now_seconds(), preferring a pinned value read from SANDBOX_TEST_PINNED_NOW_STORAGE_KEY (constant added to tee-verifier-interface so caller test suites can share it).
  • Sandbox tests: 5 new tests in tee_verifier.rs (2 active, 3 ignored) plus helpers for the pinned verifier, fixture measurement whitelisting, and the fixture-key account; assert_submission_failed_cleanly widened to &str.
  • Test utils: account_secret_key() reads a not-yet-committed assets/near_account_secret_key; README/script document producing it.
  • E2E: MpcClusterConfig::tee_verifier_wasm + deploy_and_trust_tee_verifier() run unconditionally in MpcCluster::start; Makefile/CI wire MPC_TEE_VERIFIER_WASM.
  • Docs: design doc ## Testing rewritten (stub verifier dropped); localnet + deploy runbooks point at the new coverage.

Reviewed changes

Per-file summary
File Description
crates/tee-verifier/src/lib.rs, Cargo.toml now_seconds() reads a pinned timestamp under the new sandbox-test-hooks feature (absent from the reproducible build's --features abi)
crates/tee-verifier-interface/src/lib.rs Exports SANDBOX_TEST_PINNED_NOW_STORAGE_KEY
crates/contract/tests/sandbox/tee_verifier.rs 5 new tests + pinned-verifier / whitelist / fixture-account helpers
.../sandbox/utils/contract_build.rs tee_verifier_contract_with_sandbox_test_hooks() builds the feature-enabled WASM into its own out dir
.../sandbox/utils/mpc_contract.rs Adds get_verified_attestation, vote_add_os_measurement; documents that get_participant_attestation mistypes get_attestation
crates/test-utils/src/attestation.rs, assets/* account_secret_key() + docs/warning for the pending asset
crates/e2e-tests/src/cluster.rs, README, Cargo.toml must_load_tee_verifier_wasm(), tee_verifier_wasm field, deploy_and_trust_tee_verifier(); sha2 for the code hash
Makefile.toml, ci.yml, Cargo.lock Build/env wiring for MPC_TEE_VERIFIER_WASM
docs/design/attestation-verifier-contract.md, docs/localnet/localnet.md, docs/deploy-tee-verifier.md Replace the stub-verifier plan with the shipped sandbox coverage

Findings

Blocking (must fix before merge):

  • crates/contract/tests/sandbox/tee_verifier.rs:188assert_submission_failed_cleanly asserts "nothing was stored" via get_participant_attestation, which this same PR documents (utils/mpc_contract.rs:108) as unable to deserialize a stored Dstack entry. In exactly the failure mode the assertion exists to catch — a Verified quote wrongly stored despite the post-DCAP rejection — result.json()? returns Err, .unwrap() panics on a serde error, and the test fails with an opaque "missing field quote" rather than nothing should be stored. That now matters because a new test actually reaches a Verified verdict. Use the get_verified_attestation added a few lines below:
    let stored = get_verified_attestation(contract, &p2p_tls_key().into())
        .await
        .unwrap();
    assert!(stored.is_none(), "nothing should be stored on failure");
  • crates/contract/tests/sandbox/tee_verifier.rs:331,368-372 — the gas assertion cannot fail and its constant can drift. The verify_quote receipt is created with exactly Gas::from_tgas(config.verifier_tera_gas) (crates/contract/src/lib.rs:877), so gas_burnt <= 200 Tgas is already implied by the is_success() assert two lines above. Meanwhile VERIFIER_TERA_GAS_BUDGET = 200 is a hand-copied duplicate of DEFAULT_VERIFIER_TERA_GAS (crates/contract/src/config.rs:40): raise that default to 300 and this test keeps asserting 200, silently ceasing to pin the production budget. The contract exposes a config view returning dtos::Config with a public verifier_tera_gas (method_names::CONFIG, lib.rs:2243) — read the budget from the contract instead:
    let budget = Gas::from_tgas(get_config(&contract).await.unwrap().verifier_tera_gas);
    assert!(
        verify_quote_outcome.gas_burnt <= budget,
        "verify_quote burnt {} of the configured {budget} budget",
        verify_quote_outcome.gas_burnt,
    );

Non-blocking (nits, follow-ups, suggestions):

  • tee_verifier.rs:380r#"WrongHash { name: \"report_data\""# couples the test to three unrelated things: AttestationSubmissionError::InvalidAttestation's {:?} formatting (tee_state.rs:42), the Debug derive field order on VerificationError::WrongHash, and near-workspaces' single level of quote escaping. Neighbouring tests derive their expectation from the error type (TeeError::VerifierUnavailable.to_string()); consider the same, or match the escape-independent report_data plus the stable failed verification prefix.
  • crates/tee-verifier/src/lib.rs:70 — the intra-doc link to SANDBOX_TEST_PINNED_NOW_STORAGE_KEY only resolves with sandbox-test-hooks on, since the use at line 13 is cfg-gated. cargo make check-docs passes --all-features so CI is green, but a plain cargo doc --document-private-items trips the workspace rustdoc::broken_intra_doc_links = "deny". Writing the full path (tee_verifier_interface::SANDBOX_TEST_PINNED_NOW_STORAGE_KEY) resolves either way.
  • crates/e2e-tests/src/cluster.rs:359,1381 — the deploy + N votes + up-to-30s poll now runs for every cluster and, by the PR's own reasoning, buys no coverage ("the verifier stays idle"). It also hard-couples MpcCluster::start to the booted contract exporting vote_tee_verifier_change / tee_verifier_account_id. True for contract_history::current_mainnet() today (3.14.0 exports both), but contract_upgrade_compatibility boots whatever contract-history points at, so a future rollback turns a topology nicety into a hard failure of every cluster. Consider opt-in/opt-out on MpcClusterConfig (e.g. Option<Vec<u8>>).
  • Makefile.toml:168 — comment still says "All three are skipped when E2E_SKIP_BUILD is set"; already wrong at four tasks, now five. Also, build-tee-verifier-optimized is what scripts/launch-localnet.sh:91 tells users to run by hand, so it now silently no-ops for anyone with E2E_SKIP_BUILD exported — a condition_message, or leaving this task unconditioned, would avoid a confusing "missing WASM" later.
  • crates/e2e-tests/src/cluster.rs:243 — with E2E_SKIP_BUILD set, MPC_TEE_VERIFIER_WASM is still exported by [tasks._run-e2e-logic.env] while the build is skipped, so cargo make e2e-tests-skip-build before ever building the verifier fails every test with failed to read tee-verifier WASM at …. Naming the remedy in the panic message would save a round trip. (Same shape as the existing contract/parallel helpers, so purely additive.)
  • docs/design/attestation-verifier-contract.md:614 — two things. It narrates the PR's history rather than the design ("What shipped instead of the stub verifier this section originally proposed…", "A stub verifier was briefly built and then dropped"); per docs/engineering-standards.md state the current design, with a short Status: note if the supersession must be recorded. And it reads as if all three verdicts are covered today, while the Verified + store branches are #[ignore]d — say plainly that that coverage is inactive until the test(contract): exercise the Verified attestation store end-to-end in sandbox (fixture account key + measurements helper) #3787 asset lands.
  • tee_verifier.rs:420-563 — the three ignored tests have never been executed and cannot be until the asset exists, so ~140 lines of assertions (hyphenated fixture account id, the expiry-slack comparison, the two-accounts-one-key scenario) are unverified. Landing them with the asset in the test(contract): exercise the Verified attestation store end-to-end in sandbox (fixture account key + measurements helper) #3787 follow-up avoids merging test code whose first real run happens in someone else's PR; keeping them here is defensible too, just worth stating that they are unvalidated.
  • crates/test-utils/assets/README.md:52, crates/test-utils/src/attestation.rs:112 — before the follow-up commits near_account_secret_key, please verify the fixture account is genuinely unregistered/unfunded on mainnet and testnet (the file lives in git history forever, and the quote/collateral binding it are already public), and that GitHub secret scanning / push protection will not reject a file containing ed25519:<base58>. The README's justification is the right instinct; it needs confirming rather than asserting.
  • crates/tee-verifier-interface/src/lib.rs:193 — a production wire-types crate used by both contract and node now exports a sandbox-only constant unconditionally. Gating it behind a feature, or moving it into a clearly named sandbox_hooks module, keeps it off the production API surface while staying shareable with caller test suites.

⚠️ Issues found

@claude

claude Bot commented Aug 10, 2026

Copy link
Copy Markdown

PR title type suggestion: This PR modifies source code files across multiple crates (tee-verifier-interface, tee-verifier, test-utils, e2e-tests), so the type prefix should be feat: instead of test: — the test type is only for test-file-only changes.

Suggested title: feat(contract): cover the Verified attestation path cross-contract in sandbox

pbeza added 7 commits August 10, 2026 13:09
…tion

The MPC node generates its NEAR signer key inside the enclave and writes it
only to the CVM's encrypted disk. Collecting test assets needs that key
alongside the quote it is bound to, and an app-compose pre-launch script is
the only way in: the dev image ships no sshd and the guest agent serves only
node info and container logs.

vmm-cli already supports --prelaunch-script; this just plumbs an optional
PRELAUNCH_SCRIPT through. Attestation verification rejects any app-compose
carrying a script, so a CVM deployed this way cannot join a network - the
deploy guide, default.env and the script itself now say so loudly.
Collecting the attestation fixtures requires a pre-launch script to export
the node's in-enclave signer key, which makes the resulting app-compose one
that production verification rejects as arbitrary root code. The new
allow-pre-launch-script feature relaxes exactly that field for test builds;
init_script and bash_script stay rejected, and the production wasm never
enables it.

The check takes the policy as an argument so the strict behaviour keeps its
unit test in test builds too, and a further test pins the compiled-in policy
at the call site.
…ner key

Regenerated from a fresh localnet CVM on a TDX host, keeping the node's NEAR
signer key this time so tests can sign submit_participant_info as the fixture
node. The quote's report_data binds that key and the contract reads it from
the transaction signer, so without it the Verified store path is unreachable
(#3787).

create-assets.sh had been producing collateral our own parser cannot read
since the tee-verifier-interface refactor: nodes serialize the DER and
signature fields as JSON byte arrays while collateral_from_json expects hex
strings. It now hex-encodes those fields, and strips the NUL that terminates
the PEM chains so the fixture stays valid PEM. The regeneration reminder is
unconditional, since the committed key is normally present but stale.

.editorconfig no longer asserts the borsh fixture's final byte: it is
whatever the encoded collateral ends with.
…ation time

Sandbox chain time is wall-clock and forward-only, so it can never fall inside
the validity window of a checked-in collateral fixture. Under the new test-only
feature, verify_quote prefers a pinned timestamp read from a well-known storage
key that tests write via state patching; the production build reads no storage
at all. The key constant lives in tee-verifier-interface so caller test suites
can share it without depending on the contract crate.
… sandbox

Drives the async submit_participant_info flow against the real deployed
tee-verifier for every verdict: Rejected with a malformed quote, no-verdict
with an undeployed verifier account, and Verified with the fixture quote
against a verifier built with sandbox-test-hooks, whose clock the test pins to
the fixture's validity window.

Covers what the Verified path unlocks: the attestation stores with storage
charged to the contract, the TlsKeyOwnedByOtherAccount guard rejects a second
account claiming the same TLS key, and real dcap-qvl runs within the
production verifier_tera_gas budget with headroom to spare (measured at 175.7
of 200 Tgas). The budget is read from the contract's config view so it cannot
drift from DEFAULT_VERIFIER_TERA_GAS.

Storing a first attestation for a TLS key consumes a prepaid storage grant, so
the tests fund one from a separate payer, leaving the submitter's balance clean
for the only-gas-spent assertions.

Closes #3787
Mirrors the production topology (cf. scripts/launch-localnet.sh). Nodes in e2e
clusters submit mock attestations, which the MPC contract verifies without
calling the verifier, so the verifier stays idle; the cross-contract flow is
covered at the sandbox layer.

The vote-resolution check polls because views can briefly lag final votes, and
MpcClusterConfig::validate now bounds-checks initial_participant_indices, which
also covers the pre-existing indexing in init_contract and add_initial_domains.
The design doc's Testing section described a stub verifier that was built and
then dropped during implementation; it now states the shipped approach, with a
status note recording the supersession. The two verifier runbooks gain a pointer
to the cross-contract sandbox coverage alongside the existing unit test.
@pbeza
pbeza force-pushed the 3787-sandbox-verified-attestation branch from 63e2f8f to 48b09eb Compare August 10, 2026 11:42
@claude

claude Bot commented Aug 10, 2026

Copy link
Copy Markdown

PR title type suggestion: This PR changes core library source files in addition to tests (attestation, tee-verifier, etc.), so the type prefix should probably be feat: instead of test:.

Suggested title: feat(contract): implement Verified attestation path cross-contract in sandbox

@pbeza

pbeza commented Aug 10, 2026

Copy link
Copy Markdown
Contributor Author

Superseded by #4109 (fixtures, to main) and #4110 (cross-contract sandbox tests, stacked on it).

Re-sliced so the fixture regeneration lands first. That removes the awkward step this PR had: it landed three #[ignore]d tests that #4104 then enabled — and one of those, the out-of-gas test for resolve_verification, turned out to be unachievable (near-sdk forwards unused prepaid gas to the callback, so it cannot be starved) and would have been deleted one PR later. With fixtures first, every test lands live and that one never lands at all.

Same content otherwise, rebased onto current main and re-verified. Review comments here remain readable; the code they point at moved to #4109 or #4110.

@pbeza pbeza closed this Aug 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Sandbox tests for the attestation flow against the real tee-verifier contract

2 participants