test(contract): pin the max borsh size of a stored NodeAttestation - #3856
Draft
pbeza wants to merge 44 commits into
Draft
test(contract): pin the max borsh size of a stored NodeAttestation#3856pbeza wants to merge 44 commits into
NodeAttestation#3856pbeza wants to merge 44 commits into
Conversation
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.
- 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)
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
- 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)
A Dstack attestation is stored on the contract only after its async verification succeeds. The node confirms a submission by checking whether the attestation is stored; while verification is still in flight it isn't, so the node observed NotExecuted and resubmitted, hitting VerificationAlreadyPending until the verifier resolved. Add an is_verification_pending contract view and consult it in observe_tx_result: when the attestation isn't stored yet but a verification is pending for the submitter, count the submission as executed so the node does not resubmit mid-verification. The ABI snapshot (abi__abi_has_not_changed.snap) needs regenerating for the new view method; the local toolchain cannot build the contract wasm, so it must be updated in CI or on a 1.86-compatible toolchain.
Co-authored-by: Patryk Bęza <patryk.beza@gmail.com>
…ttestation_storage
`stored_attestations` is a `near_sdk::store::IterableMap`, which buffers writes and only serializes them on flush/Drop. Both attestation charge paths read the `env::storage_usage()` delta immediately after the insert, before the buffered write reaches the trie, so the delta was always zero: `submit_participant_info` accepted a new attestation with zero deposit and refunded it in full, letting any account grow contract storage for free. Flush the map in the shared `store_verified_attestation` tail so the insert is visible to the delta measured by `charge_attestation_storage`, covering both the synchronous mock path and the async dstack path.
The node, tee-context, and e2e/test harnesses previously submitted submit_participant_info with a zero deposit, so the contract's storage charge would fail once submissions started paying for their own storage. Add a shared SUBMIT_PARTICIPANT_INFO_DEPOSIT_YOCTONEAR in the interface crate's new deposits module and use it everywhere: - node: ChainSendTransactionRequest::deposit_required(), threaded through submit_tx and the TransactionSigner - tee-context: submit_attestation attaches it - e2e: init_contract submits with gas + deposit - contract tests: sandbox and inprocess consts derive from the shared value The contract charges the measured storage cost and refunds the excess.
…onear Prefer from_near for legible NEAR amounts: - rename SUBMIT_PARTICIPANT_INFO_DEPOSIT_YOCTONEAR (= 10^24) to SUBMIT_PARTICIPANT_INFO_DEPOSIT_NEAR (= 1), and switch its call sites from from_yoctonear to from_near; the 25-digit yocto literal was unreadable - convert every zero deposit from from_yoctonear(0) to from_near(0) Reverses the direction of the earlier from_near(0) -> from_yoctonear(0) unification: from_near reads more clearly for whole-NEAR and zero amounts. Non-zero yocto values (from_yoctonear(1) minimum-deposit sentinels, storage cost math) are unchanged. Behavior is identical.
The function flushes the insert to storage before returning so a caller's subsequent env::storage_usage() reflects it, which the mock and dstack storage-delta charges rely on. Surface this as a doc comment since it is caller-relevant, and trim the inline comment to the IterableMap mechanic.
…n-core # Conflicts: # crates/contract/src/lib.rs # crates/contract/src/tee/tee_state.rs
Replace the runtime storage-delta measurement in submit_participant_info with a fixed MINIMUM_ATTESTATION_STORAGE_DEPOSIT (0.1 NEAR). The stored attestation entry is bounded (worst-case ~520 bytes), so metering the exact cost at runtime buys nothing and forces the insert-then-charge-then- maybe-revert machinery. The node now attaches exactly the fee, so there is no excess to refund. Removes charge_attestation_storage, revert_dstack_store, the ParticipantInsertion payload, both env::storage_usage() snapshots, and the measurement-coupled flush(). Adds an up-front deposit guard that rejects an under-funded submission before any store or the verifier promise, and a drift-guard unit test asserting the fee covers the worst-case entry at today's storage price. The verdict-failure refund path in resolve_verification is unchanged.
…n-core # Conflicts: # crates/node/src/indexer/types.rs
…n-core # Conflicts: # crates/contract/tests/inprocess/attestation_submission.rs
…ests The merged shared `common::participant_context` attaches no deposit, but `submit_participant_info` now requires the flat storage fee, so every attestation-submitting test failed with InsufficientDeposit. Build the submission context with ATTESTATION_STORAGE_DEPOSIT attached; vote calls keep using the deposit-free context.
…n-core # Conflicts: # crates/e2e-tests/src/cluster.rs
Guarantee the storage budget the flat attestation fee is sized against by asserting the exact worst-case borsh size of a stored NodeAttestation per verified-attestation variant (Dstack 445 bytes, Mock 450 bytes), with the account id at NEAR's 64-byte cap and every other field at its maximum. Every field is fixed-size or hard-capped, so the sizes are deterministic; an exact match fails on any growth or shrink and forces a re-check of the fee coverage test.
pbeza
changed the title
NodeAttestation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to the flat-fee discussion on #3714 (r3613125473): kevindeforth asked whether the "worst-case ~520 bytes" bound the flat attestation storage fee is sized against is actually guaranteed.
This adds a test that pins the exact worst-case borsh size of a stored
NodeAttestationperVerifiedAttestationvariant, with the account id at NEAR's 64-byte cap and every other field at its maximum:WithConstraints, all fieldsSome): 450 bytesThe bound is guaranteed because every stored field is fixed-size except the account id, which NEAR hard-caps at 64 bytes. The heavy DCAP data (quote, collateral, advisory IDs, app-compose) lives on the input attestation and is discarded after verification, so it is never persisted. The measured worst case is 450 bytes, not ~520; the higher figure in the deposit test includes NEAR's per-record
storage_usageoverhead on top of the borsh bytes.Exact
assert_eq!catches growth and shrink, and the comment points back tominimum_attestation_storage_deposit__should_cover_worst_case_entryso a future size change forces a re-check of the fee coverage.🤖 Generated with Claude Code