diff --git a/crates/contract/src/lib.rs b/crates/contract/src/lib.rs index dbe6699f44..dba93afa9d 100644 --- a/crates/contract/src/lib.rs +++ b/crates/contract/src/lib.rs @@ -117,12 +117,6 @@ const MINIMUM_CKD_REQUEST_DEPOSIT: NearToken = NearToken::from_yoctonear(1); /// node key cannot invoke these methods. pub const MINIMUM_NODE_MANAGEMENT_DEPOSIT: NearToken = NearToken::from_yoctonear(1); -/// Flat fee a node attaches to [`MpcContract::submit_participant_info`] for its -/// stored attestation entry. The entry is bounded, so the fee is fixed and -/// nothing is refunded; its margin over the true cost absorbs storage-price and -/// layout changes. A unit test asserts it covers the worst-case entry. -const MINIMUM_ATTESTATION_STORAGE_DEPOSIT: NearToken = NearToken::from_millinear(100); - /// Entries to scan in the post-reshare `clean_invalid_attestations` sweep. External /// callers may pick a different value; this only governs the automatic invocation. const RESHARE_CLEAN_INVALID_ATTESTATIONS_MAX_SCAN: u32 = 100; @@ -784,9 +778,8 @@ impl MpcContract { /// `verify_quote` call, with [`Self::resolve_verification`] chained as its /// callback to run the post-DCAP checks and store the attestation. /// - /// The caller must attach a flat 0.1 NEAR fee for the stored entry; the whole - /// fee is kept on success and refunded if the attestation is not accepted. - #[payable] + /// Storage is funded by the contract's own balance, so a node submits with no deposit via its + /// function-call access key, for its first attestation and re-attestations alike. #[handle_result] pub fn submit_participant_info( &mut self, @@ -822,15 +815,6 @@ impl MpcContract { account_public_key, }; - let attached = env::attached_deposit(); - if attached < MINIMUM_ATTESTATION_STORAGE_DEPOSIT { - return Err(InvalidParameters::InsufficientDeposit { - attached: attached.as_yoctonear(), - required: MINIMUM_ATTESTATION_STORAGE_DEPOSIT.as_yoctonear(), - } - .into()); - } - match proposed_participant_attestation { Attestation::Mock(mock) => { let tee_upgrade_deadline_duration = @@ -871,7 +855,6 @@ impl MpcContract { .then( Self::ext(env::current_account_id()) .with_static_gas(Gas::from_tgas(self.config.resolve_verification_tera_gas)) - .with_attached_deposit(env::attached_deposit()) .resolve_verification(VerificationContext { node_id, attestation, @@ -2320,11 +2303,10 @@ impl MpcContract { } } - /// Verify-quote callback: on a verifier verdict it runs the post-DCAP - /// checks and stores the attestation, refunding the flat fee if the - /// attestation is not accepted. + /// Verify-quote callback: on a verifier verdict it runs the post-DCAP checks and stores the + /// attestation (storage funded by the contract's balance). On any failure it fails the + /// submitter's transaction. #[private] - #[payable] pub fn resolve_verification( &mut self, #[serializer(borsh)] context: VerificationContext, @@ -2356,9 +2338,8 @@ impl MpcContract { match attestation_result { Ok(()) => PromiseOrValue::Value(()), Err(err) => { - refund_to(&account_id, env::attached_deposit()); - // Fail the submitter's transaction from a separate receipt so - // the refund above commits (a panic here would roll it back) + // Fail the submitter's transaction from a separate receipt so any prior state + // commits (a panic here would roll it back) let promise = Promise::new(env::current_account_id()).function_call( method_names::FAIL_ATTESTATION_SUBMISSION.to_string(), borsh::to_vec(&err.to_string()) @@ -2372,9 +2353,7 @@ impl MpcContract { } /// Runs the post-DCAP checks and stores the attestation for a - /// [`VerificationResult::Verified`] response. The deposit was already - /// checked against the flat fee in [`Self::submit_participant_info`], so this - /// only verifies and stores. + /// [`VerificationResult::Verified`] response. Storage is funded by the contract's balance. fn verify_post_dcap_and_store( &mut self, context: &VerificationContext, @@ -2393,7 +2372,6 @@ impl MpcContract { log!("post-DCAP check failed for {account_id}: {err}"); return Err(err.into()); } - Ok(()) } @@ -4201,7 +4179,6 @@ mod tests { let participant_context = VMContextBuilder::new() .signer_account_id(account_id.clone()) .predecessor_account_id(account_id.clone()) - .attached_deposit(NearToken::from_near(1)) .build(); testing_env!(participant_context); @@ -4630,7 +4607,6 @@ mod tests { let ctx = VMContextBuilder::new() .signer_account_id(participant_id.clone()) .predecessor_account_id("outsider.near".parse().unwrap()) - .attached_deposit(NearToken::from_near(1)) .build(); testing_env!(ctx); @@ -4645,7 +4621,6 @@ mod tests { let context = VMContextBuilder::new() .current_account_id(contract_account_id.clone()) .predecessor_account_id(contract_account_id) - .attached_deposit(MINIMUM_ATTESTATION_STORAGE_DEPOSIT) .block_timestamp(VALID_ATTESTATION_TIMESTAMP * 1_000_000_000) .build(); testing_env!(context); @@ -4727,7 +4702,6 @@ mod tests { let ctx = VMContextBuilder::new() .signer_account_id(outsider_id.clone()) .predecessor_account_id(outsider_id.clone()) - .attached_deposit(NearToken::from_near(1)) .build(); testing_env!(ctx); @@ -4789,7 +4763,6 @@ mod tests { VMContextBuilder::new() .signer_account_id(outsider_id.clone()) .predecessor_account_id(outsider_id.clone()) - .attached_deposit(NearToken::from_near(1)) .build() ); @@ -8152,43 +8125,104 @@ mod tests { assert!(configs.contains_key(&tls_key_b), "node B config must exist"); } - // Catches only entry-size growth: fails if a schema change makes the stored entry - // cost more than the fee at today's storage_byte_cost. It cannot see a future - // storage_byte_cost increase on a live contract; the fee's margin covers that. - #[test] - fn minimum_attestation_storage_deposit__should_cover_worst_case_entry() { - // Given: the largest entry a submission can store. NEAR caps an account id - // at 64 bytes; every other field is fixed-size, so this is the worst case. + const MAX_HASH: [u8; 32] = [0xff; 32]; + + /// Charged storage of the largest attestation entry the contract can store, in bytes: + /// a 64-byte account id (NEAR's cap) plus fixed-width keys and the largest + /// [`VerifiedAttestation`] variant, including the `IterableMap` record overhead. + const WORST_CASE_ENTRY_BYTES: u64 = 604; + + /// Ceiling on one entry's storage cost at today's price, with headroom over + /// [`WORST_CASE_ENTRY_BYTES`] for storage-price changes. + const WORST_CASE_ENTRY_COST_CEILING: NearToken = NearToken::from_millinear(10); + + fn worst_case_dstack_attestation() -> VerifiedAttestation { + VerifiedAttestation::Dstack(ValidatedDstackAttestation { + mpc_image_hash: MAX_HASH.into(), + launcher_compose_hash: MAX_HASH.into(), + expiry_timestamp_seconds: u64::MAX, + measurements: default_measurements()[0], + }) + } + + fn worst_case_mock_attestation() -> VerifiedAttestation { + VerifiedAttestation::Mock(MpcMockAttestation::WithConstraints { + mpc_docker_image_hash: Some(MAX_HASH.into()), + launcher_docker_compose_hash: Some(MAX_HASH.into()), + expiry_timestamp_seconds: Some(u64::MAX), + expected_measurements: Some(default_measurements()[0]), + }) + } + + /// Storage the contract pays for one stored attestation entry, measured the way the runtime + /// charges it. NEAR caps an account id at 64 bytes; every other `NodeId` field is fixed-size, + /// so this is the worst case for the given attestation variant. + fn measure_stored_entry_bytes(verified_attestation: VerifiedAttestation) -> u64 { testing_env!(VMContextBuilder::new().build()); let node_id = create_node_id( &"a".repeat(64).parse().unwrap(), &bogus_ed25519_public_key(), ); - let worst_case = NodeAttestation { - node_id: node_id.clone(), - verified_attestation: VerifiedAttestation::Dstack(ValidatedDstackAttestation { - mpc_image_hash: [0xff; 32].into(), - launcher_compose_hash: [0xff; 32].into(), - expiry_timestamp_seconds: u64::MAX, - measurements: default_measurements()[0], - }), - }; - // When: the entry is inserted and flushed, so storage_usage reflects it. let mut tee_state = TeeState::default(); - let storage_before = env::storage_usage(); - tee_state - .stored_attestations - .insert(node_id.tls_public_key.clone(), worst_case); + let before = env::storage_usage(); + tee_state.stored_attestations.insert( + node_id.tls_public_key.clone(), + NodeAttestation { + node_id, + verified_attestation, + }, + ); tee_state.stored_attestations.flush(); - let bytes_grown = env::storage_usage() - storage_before; - let worst_case_cost = env::storage_byte_cost().saturating_mul(u128::from(bytes_grown)); - // Then: the flat fee covers the worst-case cost with headroom to spare. + env::storage_usage() - before + } + + /// Pins the exact stored size of the largest variant of each attestation kind. + /// + /// Do not update these numbers just to make a failing case pass. The contract funds every + /// entry from its own balance, so a size change alters what the contract pays per node, and + /// everything derived from it must be revisited in the same change — today + /// [`WORST_CASE_ENTRY_BYTES`] and [`WORST_CASE_ENTRY_COST_CEILING`]. + /// + /// TODO(#3972): the flat onboarding deposit will be derived from these sizes too. + #[rstest] + #[case::dstack(599, worst_case_dstack_attestation())] + #[case::mock(604, worst_case_mock_attestation())] + fn stored_attestation_entry__should_have_the_pinned_size( + #[case] expected_bytes: u64, + #[case] verified_attestation: VerifiedAttestation, + ) { + // Given / When + let bytes_stored = measure_stored_entry_bytes(verified_attestation); + + // Then + assert_eq!( + bytes_stored, expected_bytes, + "stored entry size changed; see this test's doc comment before updating the number" + ); + assert!( + bytes_stored <= WORST_CASE_ENTRY_BYTES, + "entry ({bytes_stored} bytes) exceeds the pinned worst case \ + ({WORST_CASE_ENTRY_BYTES} bytes)" + ); + } + + #[rstest] + #[case::dstack(worst_case_dstack_attestation())] + #[case::mock(worst_case_mock_attestation())] + fn stored_attestation_entry__should_stay_under_the_cost_ceiling( + #[case] verified_attestation: VerifiedAttestation, + ) { + // Given / When + let bytes_grown = measure_stored_entry_bytes(verified_attestation); + let cost = env::storage_byte_cost().saturating_mul(u128::from(bytes_grown)); + + // Then assert!( - MINIMUM_ATTESTATION_STORAGE_DEPOSIT >= worst_case_cost, - "flat fee {MINIMUM_ATTESTATION_STORAGE_DEPOSIT} must cover the worst-case entry \ - ({bytes_grown} bytes, {worst_case_cost}) at today's storage price" + cost <= WORST_CASE_ENTRY_COST_CEILING, + "worst-case entry cost ({bytes_grown} bytes, {cost}) must stay under \ + {WORST_CASE_ENTRY_COST_CEILING} at today's storage price" ); } } diff --git a/crates/contract/tests/inprocess/attestation_submission.rs b/crates/contract/tests/inprocess/attestation_submission.rs index 3d6f37f3e4..9c4c774e92 100644 --- a/crates/contract/tests/inprocess/attestation_submission.rs +++ b/crates/contract/tests/inprocess/attestation_submission.rs @@ -3,7 +3,7 @@ use super::common; use mpc_contract::{ MpcContract, - errors::{Error, InvalidParameters, InvalidState, TeeError}, + errors::{Error, InvalidState, TeeError}, primitives::{ key_state::EpochId, participants::{ParticipantId, ParticipantInfo}, @@ -15,15 +15,14 @@ use mpc_contract::{ }, tee::tee_state::{AttestationSubmissionError, NodeId}, }; -use near_mpc_contract_interface::{ - deposits::SUBMIT_PARTICIPANT_INFO_DEPOSIT_MILLINEAR, - types::{Attestation, InitConfig, MockAttestation, ProtocolContractState}, +use near_mpc_contract_interface::types::{ + Attestation, InitConfig, MockAttestation, ProtocolContractState, }; use std::collections::BTreeMap; use assert_matches::assert_matches; use near_account_id::AccountId; -use near_sdk::{NearToken, test_utils::VMContextBuilder, testing_env}; +use near_sdk::{test_utils::VMContextBuilder, testing_env}; use rstest::rstest; use std::time::Duration; use test_utils::attestation::mock_dto_dstack_attestation; @@ -31,9 +30,6 @@ use test_utils::attestation::mock_dto_dstack_attestation; const SECOND: Duration = Duration::from_secs(1); const NANOS_IN_SECOND: u64 = SECOND.as_nanos() as u64; -const ATTESTATION_STORAGE_DEPOSIT: NearToken = - NearToken::from_millinear(SUBMIT_PARTICIPANT_INFO_DEPOSIT_MILLINEAR); - const DEFAULT_PARTICIPANT_COUNT: usize = 3; const DEFAULT_THRESHOLD_SIZE: u64 = 2; const DEFAULT_CONTRACT_PROTOCOL_STATE: ContractProtocolState = ContractProtocolState::Running; @@ -163,16 +159,7 @@ impl TestSetup { node_id: &NodeId, attestation: Attestation, ) -> Result<(), mpc_contract::errors::Error> { - // `submit_participant_info` requires the flat storage fee, unlike the - // deposit-free calls `common::participant_context` is built for. - testing_env!( - VMContextBuilder::new() - .signer_account_id(node_id.account_id.clone()) - .predecessor_account_id(node_id.account_id.clone()) - .block_timestamp(near_sdk::env::block_timestamp()) - .attached_deposit(ATTESTATION_STORAGE_DEPOSIT) - .build() - ); + testing_env!(common::participant_context(&node_id.account_id)); self.contract .submit_participant_info(attestation, node_id.tls_public_key.clone()) .map(|_| ()) @@ -278,39 +265,67 @@ fn submit_participant_info__should_reject_overwrite_from_other_account() { assert_eq!(stored_before, stored_after); } -/// Rejects a submission whose attached deposit is below the storage cost, so a caller -/// cannot store an attestation without paying for it. +/// A newcomer stores its first attestation with no deposit (contract-funded storage), so the +/// node's function-call access key can self-onboard. #[test] -fn submit_participant_info__should_reject_when_deposit_is_below_storage_cost() { +fn submit_participant_info__should_store_new_entry_with_zero_deposit() { // Given let mut setup = TestSetupBuilder::new().build(); - let node = setup.get_participant_node_ids()[0].clone(); - let attached_deposit = NearToken::from_yoctonear(1); - testing_env!( - VMContextBuilder::new() - .signer_account_id(node.account_id.clone()) - .predecessor_account_id(node.account_id.clone()) - .attached_deposit(attached_deposit) - .build() - ); + let newcomer = node_id_for(&"newcomer.near".parse().unwrap()); + testing_env!(common::participant_context(&newcomer.account_id)); // When let result = setup .contract .submit_participant_info( Attestation::Mock(MockAttestation::Valid), - node.tls_public_key.clone(), + newcomer.tls_public_key.clone(), ) .map(|_| ()); // Then - assert_matches!( - &result, - Err(Error::InvalidParameters(InvalidParameters::InsufficientDeposit { attached, required })) - if *attached == attached_deposit.as_yoctonear() && required > attached + assert_matches!(&result, Ok(())); + assert!( + setup + .contract + .get_attestation(newcomer.tls_public_key) + .unwrap() + .is_some() ); } +/// A current participant re-attesting an existing entry succeeds with no attached deposit, so the +/// node's function-call access key can re-attest. +#[test] +fn submit_participant_info__should_reattest_with_zero_deposit() { + // Given + let mut setup = TestSetupBuilder::new().build(); + let node = setup.get_participant_node_ids()[0].clone(); + let attestation = Attestation::Mock(MockAttestation::Valid); + setup.submit_attestation_for_node(&node, attestation.clone()); + let stored_before = setup + .contract + .get_attestation(node.tls_public_key.clone()) + .unwrap() + .expect("participant attestation should be stored"); + + // When: the same participant re-attests with no attached deposit. + testing_env!(common::participant_context(&node.account_id)); + let result = setup + .contract + .submit_participant_info(attestation, node.tls_public_key.clone()) + .map(|_| ()); + + // Then: the submission succeeds and the stored entry is unchanged. + assert_matches!(&result, Ok(())); + let stored_after = setup + .contract + .get_attestation(node.tls_public_key) + .unwrap() + .expect("participant attestation should still be stored"); + assert_eq!(stored_before, stored_after); +} + /// Test that a `Dstack` submission is rejected when no verifier is configured. #[test] fn submit_participant_info__should_reject_dstack_when_verifier_not_configured() { diff --git a/crates/contract/tests/sandbox/tee.rs b/crates/contract/tests/sandbox/tee.rs index 3255d4cbe8..b717b5c45f 100644 --- a/crates/contract/tests/sandbox/tee.rs +++ b/crates/contract/tests/sandbox/tee.rs @@ -3,12 +3,12 @@ use crate::sandbox::{ common::{SandboxTestSetup, build_sandbox_node_ids, gen_accounts, submit_tee_attestations}, utils::{ - consts::{ALL_PROTOCOLS, SUBMIT_PARTICIPANT_INFO_DEPOSIT}, + consts::ALL_PROTOCOLS, interface::IntoContractType, mpc_contract::{ assert_running_return_participants, assert_running_return_threshold, get_participant_attestation, get_state, get_tee_accounts, submit_participant_info, - total_gas_fee, vote_add_launcher_hash, vote_for_hash, + vote_add_launcher_hash, vote_for_hash, }, resharing_utils::conclude_resharing, sign_utils::DomainResponseTest, @@ -20,8 +20,8 @@ use mpc_primitives::hash::{LauncherDockerComposeHash, LauncherImageHash, NodeIma use near_mpc_contract_interface::method_names; use near_mpc_contract_interface::types::Protocol; use near_mpc_contract_interface::types::{self as dtos, Attestation, MockAttestation}; -use near_workspaces::Contract; -use near_workspaces::types::NearToken; +use near_workspaces::types::{KeyType, NearToken, SecretKey}; +use near_workspaces::{AccessKey, Account, Contract}; use rand::SeedableRng; use test_utils::attestation::{image_digest, p2p_tls_key}; @@ -279,6 +279,52 @@ async fn test_submit_participant_info_succeeds_with_mock_attestation() -> Result Ok(()) } +#[tokio::test] +async fn submit_participant_info__should_accept_zero_deposit_via_function_call_key() -> Result<()> { + // Given + let SandboxTestSetup { + worker, + contract, + mpc_signer_accounts, + .. + } = SandboxTestSetup::builder() + .with_protocols(ALL_PROTOCOLS) + .build() + .await; + + let account = &mpc_signer_accounts[0]; + let participants = assert_running_return_participants(&contract).await?; + let tls_key = participants + .participants + .iter() + .find(|(id, _, _)| id == account.id()) + .map(|(_, _, info)| info.tls_public_key.clone()) + .expect("participant must exist"); + let attestation = Attestation::Mock(MockAttestation::Valid); + + let fc_sk = SecretKey::from_random(KeyType::ED25519); + account + .batch(account.id()) + .add_key( + fc_sk.public_key(), + AccessKey::function_call_access(contract.id(), &[], None), + ) + .transact() + .await? + .into_result()?; + let fc_account = Account::from_secret_key(account.id().clone(), fc_sk, &worker); + + // When + let outcome = submit_participant_info(&fc_account, &contract, &attestation, &tls_key).await?; + + // Then + assert!( + outcome.is_success(), + "zero-deposit fc submission must succeed: {outcome:?}" + ); + Ok(()) +} + /// **Access control validation** - Tests that external accounts cannot call the private clean_tee_status contract method. /// This verifies the security boundary: only the contract itself should be able to perform internal cleanup operations. #[tokio::test] @@ -980,10 +1026,10 @@ async fn verify_tee__should_keep_participants_and_stop_signing_when_kickout_drop Ok(()) } -/// A submission attaching less than the flat storage fee is rejected before the -/// entry is stored. +/// A new attestation entry is stored with no attached deposit; the contract's own balance funds +/// the storage. #[tokio::test] -async fn submit_participant_info__should_reject_new_attestation_below_flat_fee() -> Result<()> { +async fn submit_participant_info__should_store_new_entry_with_zero_deposit() -> Result<()> { // Given let SandboxTestSetup { worker, contract, .. @@ -993,50 +1039,31 @@ async fn submit_participant_info__should_reject_new_attestation_below_flat_fee() .await; let outsider = worker.dev_create_account().await?; let fresh_tls_key = bogus_ed25519_public_key(); - let storage_before = worker.view_account(contract.id()).await?.storage_usage; - let below_fee = SUBMIT_PARTICIPANT_INFO_DEPOSIT.saturating_sub(NearToken::from_yoctonear(1)); // When - let result = outsider - .call(contract.id(), method_names::SUBMIT_PARTICIPANT_INFO) - .args_json(( - Attestation::Mock(MockAttestation::Valid), - fresh_tls_key.clone(), - )) - .deposit(below_fee) - .max_gas() - .transact() - .await?; + let result = submit_participant_info( + &outsider, + &contract, + &Attestation::Mock(MockAttestation::Valid), + &fresh_tls_key, + ) + .await?; // Then assert!( - !result.is_success(), - "submission below the flat fee must fail: {result:?}" - ); - let error_msg = format!("{:?}", result.into_result()); - assert!( - error_msg.contains("Attached deposit is lower than required"), - "expected an insufficient-deposit error, got: {error_msg}" + result.is_success(), + "zero-deposit submission must succeed: {result:?}" ); let stored = get_participant_attestation(&contract, &fresh_tls_key).await?; - assert!( - stored.is_none(), - "no attestation should be stored when the deposit is rejected" - ); - let storage_after = worker.view_account(contract.id()).await?.storage_usage; - assert_eq!( - storage_after, storage_before, - "contract storage must not grow when the submission is rejected" - ); + assert!(stored.is_some(), "the entry should be stored on-chain"); Ok(()) } -/// A submission attaching exactly the flat fee is stored, and the caller is -/// charged the whole fee with no excess refunded (the fee far exceeds the true -/// storage cost by design). +/// `submit_participant_info` is not payable: an attached deposit is rejected outright rather than +/// accepted and refunded, so a caller following stale guidance fails loudly instead of silently +/// no-opping. Pins the behaviour the removed `#[payable]` marker used to allow. #[tokio::test] -async fn submit_participant_info__should_store_new_attestation_and_charge_the_flat_fee() --> Result<()> { +async fn submit_participant_info__should_reject_an_attached_deposit() -> Result<()> { // Given let SandboxTestSetup { worker, contract, .. @@ -1046,30 +1073,33 @@ async fn submit_participant_info__should_store_new_attestation_and_charge_the_fl .await; let outsider = worker.dev_create_account().await?; let fresh_tls_key = bogus_ed25519_public_key(); - let balance_before = outsider.view_account().await?.balance; // When - let result = submit_participant_info( - &outsider, - &contract, - &Attestation::Mock(MockAttestation::Valid), - &fresh_tls_key, - ) - .await?; + let result = outsider + .call(contract.id(), method_names::SUBMIT_PARTICIPANT_INFO) + .args_json(( + Attestation::Mock(MockAttestation::Valid), + fresh_tls_key.clone(), + )) + .deposit(NearToken::from_yoctonear(1)) + .max_gas() + .transact() + .await?; // Then assert!( - result.is_success(), - "submission attaching the flat fee should succeed: {result:?}" + !result.is_success(), + "a submission attaching a deposit must fail: {result:?}" + ); + let error_msg = format!("{:?}", result.into_result()); + assert!( + error_msg.contains("doesn't accept deposit"), + "expected a non-payable rejection, got: {error_msg}" ); let stored = get_participant_attestation(&contract, &fresh_tls_key).await?; assert!( - stored.is_some(), - "the attestation entry should be stored on-chain" + stored.is_none(), + "nothing should be stored when the submission is rejected" ); - let balance_after = outsider.view_account().await?.balance; - let net_spent = balance_before.saturating_sub(balance_after); - let non_gas_spent = net_spent.saturating_sub(total_gas_fee(&result)); - assert_eq!(non_gas_spent, SUBMIT_PARTICIPANT_INFO_DEPOSIT); Ok(()) } diff --git a/crates/contract/tests/sandbox/tee_verifier.rs b/crates/contract/tests/sandbox/tee_verifier.rs index b0d221079d..b4403ba478 100644 --- a/crates/contract/tests/sandbox/tee_verifier.rs +++ b/crates/contract/tests/sandbox/tee_verifier.rs @@ -67,7 +67,7 @@ async fn submit_dstack(submitter: &Account, contract: &Contract) -> ExecutionFin /// 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 deposit was refunded. +/// attestation was stored, and the caller spent only gas. async fn assert_submission_failed_cleanly( result: &ExecutionFinalResult, contract: &Contract, @@ -93,11 +93,12 @@ async fn assert_submission_failed_cleanly( .await .unwrap(); assert!(stored.is_none(), "nothing should be stored on failure"); - assert_deposit_refunded(submitter, balance_before, result).await; + assert_only_gas_spent(submitter, balance_before, result).await; } -/// Asserts the deposit was fully refunded: with nothing stored, the caller spends only gas. -async fn assert_deposit_refunded( +/// Asserts the caller spent only gas: no deposit is attached, so a failed submission costs nothing +/// beyond gas. +async fn assert_only_gas_spent( account: &Account, balance_before: NearToken, result: &ExecutionFinalResult, @@ -165,7 +166,7 @@ async fn tee_verifier_account_id__should_return_none_until_a_verifier_is_voted_i } #[tokio::test] -async fn submit_participant_info__should_refund_and_store_nothing_on_verifier_rejection() { +async fn submit_participant_info__should_store_nothing_on_verifier_rejection() { // Given let SandboxTestSetup { worker, diff --git a/crates/contract/tests/sandbox/utils/consts.rs b/crates/contract/tests/sandbox/utils/consts.rs index bd9d862484..b122dad2f6 100644 --- a/crates/contract/tests/sandbox/utils/consts.rs +++ b/crates/contract/tests/sandbox/utils/consts.rs @@ -1,8 +1,6 @@ use std::time::Duration; -use near_mpc_contract_interface::{ - deposits::SUBMIT_PARTICIPANT_INFO_DEPOSIT_MILLINEAR, types::Protocol, -}; +use near_mpc_contract_interface::types::Protocol; use near_sdk::{Gas, NearToken}; /* --- Protocol defaults --- */ @@ -47,9 +45,4 @@ pub const MAX_GAS_FOR_THRESHOLD_VOTE: Gas = Gas::from_tgas(190); /// TODO(#2756): Reduce this to the minimal value possible pub const CURRENT_CONTRACT_DEPLOY_DEPOSIT: NearToken = NearToken::from_millinear(17000); -/// Attached to `submit_participant_info`; the contract requires exactly this flat -/// fee to store the bounded attestation entry, with no refund. -pub const SUBMIT_PARTICIPANT_INFO_DEPOSIT: NearToken = - NearToken::from_millinear(SUBMIT_PARTICIPANT_INFO_DEPOSIT_MILLINEAR); - pub const DEFAULT_MAX_TIMEOUT_TX_INCLUDED: Duration = Duration::from_secs(3); diff --git a/crates/contract/tests/snapshots/abi__abi_has_not_changed.snap b/crates/contract/tests/snapshots/abi__abi_has_not_changed.snap index 0cd4cb6018..0ec9c8ab76 100644 --- a/crates/contract/tests/snapshots/abi__abi_has_not_changed.snap +++ b/crates/contract/tests/snapshots/abi__abi_has_not_changed.snap @@ -1317,10 +1317,9 @@ expression: abi }, { "name": "resolve_verification", - "doc": " Verify-quote callback: on a verifier verdict it runs the post-DCAP\n checks and stores the attestation, refunding the flat fee if the\n attestation is not accepted.", + "doc": " Verify-quote callback: on a verifier verdict it runs the post-DCAP checks and stores the\n attestation (storage funded by the contract's balance). On any failure it fails the\n submitter's transaction.", "kind": "call", "modifiers": [ - "payable", "private" ], "params": { @@ -2342,11 +2341,8 @@ expression: abi }, { "name": "submit_participant_info", - "doc": " Submit a TEE attestation for a current or prospective participant.\n\n - [`Attestation::Mock`] is verified synchronously.\n - [`Attestation::Dstack`] is verified asynchronously via a cross-contract\n `verify_quote` call, with [`Self::resolve_verification`] chained as its\n callback to run the post-DCAP checks and store the attestation.\n\n The caller must attach a flat 0.1 NEAR fee for the stored entry; the whole\n fee is kept on success and refunded if the attestation is not accepted.", + "doc": " Submit a TEE attestation for a current or prospective participant.\n\n - [`Attestation::Mock`] is verified synchronously.\n - [`Attestation::Dstack`] is verified asynchronously via a cross-contract\n `verify_quote` call, with [`Self::resolve_verification`] chained as its\n callback to run the post-DCAP checks and store the attestation.\n\n Storage is funded by the contract's own balance, so a node submits with no deposit via its\n function-call access key, for its first attestation and re-attestations alike.", "kind": "call", - "modifiers": [ - "payable" - ], "params": { "serialization_type": "json", "args": [ diff --git a/crates/near-mpc-contract-interface/src/client.rs b/crates/near-mpc-contract-interface/src/client.rs index 42d602c61a..811b6f86a2 100644 --- a/crates/near-mpc-contract-interface/src/client.rs +++ b/crates/near-mpc-contract-interface/src/client.rs @@ -12,7 +12,7 @@ use crate::call_args::{ }; use crate::deposits::{ DepositOverflowError, SIGN_DEPOSIT_YOCTONEAR, STORAGE_BYTE_COST_YOCTONEAR, - SUBMIT_PARTICIPANT_INFO_DEPOSIT_MILLINEAR, propose_update_required_deposit_yoctonear, + propose_update_required_deposit_yoctonear, }; use crate::method_names::{ PROPOSE_UPDATE, REQUEST_APP_PRIVATE_KEY, SIGN, SUBMIT_PARTICIPANT_INFO, @@ -174,7 +174,9 @@ impl MpcContractHandle { method_name: SUBMIT_PARTICIPANT_INFO.to_string(), args, gas: MAX_GAS, - deposit: NearToken::from_millinear(SUBMIT_PARTICIPANT_INFO_DEPOSIT_MILLINEAR), + // The node's function-call key cannot attach a deposit; attestation storage is + // funded by the contract's own balance. + deposit: NearToken::from_yoctonear(0), }, ) .await diff --git a/crates/near-mpc-contract-interface/src/deposits.rs b/crates/near-mpc-contract-interface/src/deposits.rs index 45b6b4c8f5..49c6125c05 100644 --- a/crates/near-mpc-contract-interface/src/deposits.rs +++ b/crates/near-mpc-contract-interface/src/deposits.rs @@ -1,10 +1,6 @@ //! Deposit amounts to attach to contract methods. One shared value for node, //! tests, and e2e. -/// Deposit for `submit_participant_info`. The contract requires exactly this -/// flat fee to store the bounded attestation entry; nothing is refunded. -pub const SUBMIT_PARTICIPANT_INFO_DEPOSIT_MILLINEAR: u128 = 100; - pub const SIGN_DEPOSIT_YOCTONEAR: u128 = 1; pub const STORAGE_BYTE_COST_YOCTONEAR: u128 = 10_000_000_000_000_000_000; diff --git a/crates/near-mpc-contract-interface/src/snapshots/near_mpc_contract_interface__client__tests__mpc_contract_handle__should_match_the_wire_format_catalog.snap b/crates/near-mpc-contract-interface/src/snapshots/near_mpc_contract_interface__client__tests__mpc_contract_handle__should_match_the_wire_format_catalog.snap index 766e07136c..99faa9a595 100644 --- a/crates/near-mpc-contract-interface/src/snapshots/near_mpc_contract_interface__client__tests__mpc_contract_handle__should_match_the_wire_format_catalog.snap +++ b/crates/near-mpc-contract-interface/src/snapshots/near_mpc_contract_interface__client__tests__mpc_contract_handle__should_match_the_wire_format_catalog.snap @@ -41,7 +41,7 @@ args: {"id":7} contract: mpc.near method: submit_participant_info gas: 300.0 Tgas -deposit: 0.1 NEAR +deposit: 0 NEAR args: {"proposed_participant_attestation":{"Mock":"Valid"},"tls_public_key":"ed25519:US517G5965aydkZ46HS38QLi7UQiSojurfbQfKCELFx"} contract: mpc.near diff --git a/crates/node/src/indexer/tx_sender.rs b/crates/node/src/indexer/tx_sender.rs index da378427cc..7390b76ba0 100644 --- a/crates/node/src/indexer/tx_sender.rs +++ b/crates/node/src/indexer/tx_sender.rs @@ -10,7 +10,7 @@ use crate::types::{ use anyhow::Context; use ed25519_dalek::SigningKey; use near_account_id::AccountId; -use near_indexer_primitives::types::{Balance, Gas}; +use near_indexer_primitives::types::Gas; use near_mpc_contract_interface::types::{Attestation, Ed25519PublicKey, VerifiedAttestation}; use near_time::Clock; use std::future::Future; @@ -148,7 +148,6 @@ async fn submit_tx( method: String, params_ser: String, gas: Gas, - deposit: Balance, ) -> anyhow::Result { let block = indexer_state.view_client.latest_final_block().await?; @@ -157,7 +156,6 @@ async fn submit_tx( method, params_ser.into(), gas, - deposit, block.header.hash, block.header.height, ); @@ -343,7 +341,6 @@ async fn ensure_send_transaction( method.to_string(), params_ser.clone(), request.gas_required(), - request.deposit_required(), ) .await; diff --git a/crates/node/src/indexer/tx_signer.rs b/crates/node/src/indexer/tx_signer.rs index 1152fc6d72..0fe2b99888 100644 --- a/crates/node/src/indexer/tx_signer.rs +++ b/crates/node/src/indexer/tx_signer.rs @@ -36,22 +36,21 @@ impl TransactionSigner { new_nonce } - #[expect(clippy::too_many_arguments)] pub(crate) fn create_and_sign_function_call_tx( &self, receiver_id: AccountId, method_name: String, args: Vec, gas: Gas, - deposit: Balance, block_hash: CryptoHash, block_height: u64, ) -> SignedTransaction { + // The node signs with a function-call access key, which cannot attach a deposit. let action = FunctionCallAction { method_name, args, gas, - deposit, + deposit: Balance::from_near(0), }; let verifying_key = self.signing_key.verifying_key(); diff --git a/crates/node/src/indexer/types.rs b/crates/node/src/indexer/types.rs index b47402041e..663cf0e586 100644 --- a/crates/node/src/indexer/types.rs +++ b/crates/node/src/indexer/types.rs @@ -6,9 +6,8 @@ use k256::{ ecdsa::RecoveryId, elliptic_curve::{Curve, CurveArithmetic, ops::Reduce, point::AffineCoordinates}, }; -use near_indexer_primitives::types::{Balance, Gas}; +use near_indexer_primitives::types::Gas; use near_mpc_contract_interface::call_args as contract_args; -use near_mpc_contract_interface::deposits::SUBMIT_PARTICIPANT_INFO_DEPOSIT_MILLINEAR; use near_mpc_contract_interface::method_names::{ CONCLUDE_NODE_MIGRATION, REGISTER_FOREIGN_CHAINS_CONFIG, RESPOND, RESPOND_CKD, RESPOND_VERIFY_FOREIGN_TX, START_KEYGEN_INSTANCE, START_RESHARE_INSTANCE, @@ -127,15 +126,6 @@ impl ChainSendTransactionRequest { | Self::VerifyForeignTransactionRespond(_) => MAX_GAS, } } - - pub fn deposit_required(&self) -> Balance { - match self { - Self::SubmitParticipantInfo { .. } => { - Balance::from_millinear(SUBMIT_PARTICIPANT_INFO_DEPOSIT_MILLINEAR) - } - _ => Balance::from_near(0), - } - } } /// Extension trait for constructing SignatureRespond arguments from node-internal types. diff --git a/docs/design/attestation-verifier-contract.md b/docs/design/attestation-verifier-contract.md index 0346ce6c69..77917da90e 100644 --- a/docs/design/attestation-verifier-contract.md +++ b/docs/design/attestation-verifier-contract.md @@ -1,5 +1,7 @@ # Attestation Verifier Contract Breakout +**Status:** Partially superseded — TODO(#3825): rewrite the `mpc-contract`-side sections below. That side shipped as a no-yield promise chain, so every mention of yield-resume, `pending_attestations` / `PendingAttestation`, `data_id`, the "already pending" guard, `on_attestation_verified`, and the attached deposit and its refunds describes a design that was not built. The verifier-contract split, governance, crate layout, and the verifier's own API are accurate. + This document outlines the design for moving on-chain TDX quote verification out of `mpc-contract`'s WASM into a standalone verifier contract. It supersedes [#3160](https://github.com/near/mpc/pull/3160), which sketched a three-contract architecture (shared verifier + per-team policy contract + TEE-agnostic application contract) for Defuse, Proximity, and other teams. That direction was deferred: a shared policy contract presumes shared lifecycle conventions (the [launcher pattern][launcher-pattern] `mpc-contract` uses), and aligning the other teams on those conventions is a separate, longer [conversation][slack-launcher-discussion] that has not yet converged. @@ -105,7 +107,7 @@ The only caller of `submit_participant_info` in production is `mpc-node`'s `peri #### Handling failures -The first thing `submit_participant_info` does is insert a `PendingAttestation` entry, and that entry has to come back out once verification finishes — successfully or not. If a failure leaves the entry behind, the submitter's account is wedged: every future `submit_participant_info` call panics on the "already pending" guard, and the deposit stays locked because the refund is part of the cleanup the contract never got around to. +The first thing `submit_participant_info` does is insert a `PendingAttestation` entry, and that entry has to come back out once verification finishes — successfully or not. If a failure leaves the entry behind, the submitter's account is wedged: every future `submit_participant_info` call panics on the "already pending" guard. That makes *where* the cleanup runs the central question, because NEAR offers two natural homes for "do something when the verifier responds" and they have very different failure modes. @@ -139,7 +141,7 @@ Each [`PendingAttestation`](#mpc-contractsubmit_participant_info) holds: - **The submitter's `Attestation::Dstack` payload** — the RTMR3 event log, app-compose, and report-data the post-DCAP checks consume. - **The submitter's TLS public key** — the callback hashes it with the submitter's account public key and compares to the quote's `report_data` field, proving the enclave produced the quote for this specific submitter. -- **The attached deposit** — covers storage staking on success, refunded to the signer of the original `submit_participant_info` transaction on failure. `env::attached_deposit()` is not visible from the callback receipt, so the value is stashed at submit time and the recipient `AccountId` is the same one used to key the entry (set by `Self::assert_caller_is_signer()`). +- No deposit is stashed: attestation storage is funded by the contract's own balance, so `submit_participant_info` attaches nothing and there is no deposit to forward to the callback or refund. - **`data_id: CryptoHash`** — the yield handle returned by [`env::promise_yield_create`][promise-yield-create]. The intermediate `.then` callback (`resolve_verification`) reads this back to call [`env::promise_yield_resume`][promise-yield-resume] with a `FinalOutcome` after the post-DCAP checks have run. Entries are removed by `resolve_verification` on every branch where the verifier returned an answer — `Verified` (post-DCAP success or failure) and `Rejected`; only when the verifier gave no verdict (`Err(PromiseError::Failed)` — unreachable or crashed) does it return early and leave the entry in place. `on_attestation_verified` then removes the entry on its `Err(PromiseError::Failed)` branch, which covers that unreachable case and the no-response timeout. diff --git a/docs/running-an-mpc-node-in-tdx-external-guide.md b/docs/running-an-mpc-node-in-tdx-external-guide.md index 33084a79c3..f507e8d827 100644 --- a/docs/running-an-mpc-node-in-tdx-external-guide.md +++ b/docs/running-an-mpc-node-in-tdx-external-guide.md @@ -2067,11 +2067,10 @@ Invalid TEE Remote Attestation: TeeQuoteStatus is invalid: the submitted attestation failed verification, reason: Custom("...") ``` -The `reason` is the same `VerificationError` the client-side WARN reports (see section 1) — for example `Custom("the allowed mpc image hashes list is empty")`. Errors that **only** surface on-chain (because they're checked against the contract's allowed-measurements list, the contract's deposit logic, or the contract's caller assertion): +The `reason` is the same `VerificationError` the client-side WARN reports (see section 1) — for example `Custom("the allowed mpc image hashes list is empty")`. Errors that **only** surface on-chain (because they're checked against the contract's allowed-measurements list or the contract's caller assertion): - **`MeasurementsNotAllowed`** — your boot measurements (MRTD / RTMR0–2) are not in the contract's allowed set. Vote them in (see [OS measurement voting](#os-measurement-voting)). - **`EmptyMeasurementsList`** — the contract has no allowed measurements yet; the first set must be voted in before any node can attest. -- **`Attached deposit is lower than required. Attached: X, required: Y`** — first-time joiners must attach enough yoctoNEAR for storage; the node attaches `0`, so call `submit_participant_info` manually with `--deposit` once. Exact amount tracked in [#903](https://github.com/near/mpc/issues/903). - **`Caller is not the signer account.`** — the access key used to sign does not match the node's `my_near_account_id`. If you see no error logs at all but `get_attestation` still returns `null`, the node has not yet generated a quote. Check `mpc_tee_attestation_attempts_total` on the `/metrics` endpoint. diff --git a/docs/securing-mpc-with-tee-design-doc.md b/docs/securing-mpc-with-tee-design-doc.md index 128092e9d1..999416ef80 100644 --- a/docs/securing-mpc-with-tee-design-doc.md +++ b/docs/securing-mpc-with-tee-design-doc.md @@ -386,7 +386,7 @@ pub struct Contract { } ``` -_Note_: submit_participant_info - can be called either by the node or by the operator. +_Note_: submit_participant_info must be called by the node itself - the quote's report_data binds hash(tls_public_key, account_public_key) to env::signer_account_pk(), so a submission signed by any other key fails verification. Attestation storage is funded by the contract's own balance, so submissions attach no deposit and the node's function-call access key works for both a first-time (join) submission and re-attestations. ## MPC Node changes: