fix(contract): charge attestation storage only for new entries or non-participants

submit_participant_info required a deposit unconditionally, which broke nodes
signing with a function-call access key: such keys cannot attach any deposit, so
every re-attestation failed at tx validation with DepositWithFunctionCall.

Restore the pre-#3714 conditional: a participant re-attesting an existing entry
attaches no deposit and is charged nothing (so the node's function-call key can
re-attest), while a new entry or a non-participant caller still pays the storage
delta. The client and node stop attaching the deposit; an operator's full-access
key funds a first-time (join) submission.

Adds a sandbox regression test that drives submit_participant_info through a real
function-call access key (zero deposit succeeds; a deposit is rejected).

Closes #3925

Author: pbeza

crates/contract/src/lib.rs

@@ -161,19 +161,29 @@ fn refund_to(account_id: &AccountId, amount: NearToken) {
     }
 }
 
-/// Charges the storage growth since `initial_storage` and refunds the rest. The
-/// [`MINIMUM_ATTESTATION_STORAGE_DEPOSIT`] floor guarantees the refund never underflows.
-fn keep_storage_delta_and_refund_rest(account_id: &AccountId, initial_storage: u64) {
+/// Requires at least [`MINIMUM_ATTESTATION_STORAGE_DEPOSIT`], then keeps the storage delta since
+/// `initial_storage` and refunds the excess.
+fn charge_storage(account_id: &AccountId, initial_storage: u64) -> Result<(), Error> {
+    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());
+    }
+
     // saturating_sub: a shrink charges nothing rather than underflowing.
     let bytes_grown = env::storage_usage().saturating_sub(initial_storage);
     let cost = env::storage_byte_cost().saturating_mul(u128::from(bytes_grown));
-    match env::attached_deposit().checked_sub(cost) {
+    match attached.checked_sub(cost) {
         Some(refund) => refund_to(account_id, refund),
         // Unreachable given the MINIMUM_ATTESTATION_STORAGE_DEPOSIT floor, which
         // minimum_attestation_storage_deposit__should_cover_worst_case_entry pins to the
         // worst-case entry cost.
         None => log!("attestation storage cost {cost} exceeded deposit for {account_id}"),
     }
+    Ok(())
 }
 
 impl Default for MpcContract {
@@ -798,11 +808,12 @@ 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 at least [`MINIMUM_ATTESTATION_STORAGE_DEPOSIT`],
-    /// enough to cover the worst-case stored entry. On success only the actual
-    /// storage delta is kept and the excess is refunded, so a re-submission that
-    /// changes no stored bytes is charged nothing. The full deposit is refunded if
-    /// the attestation is not accepted.
+    /// Storage is charged to the caller only when a new entry is stored or the caller is not a
+    /// current participant. A participant re-attesting an existing entry is charged nothing and
+    /// need not attach any deposit, so the node's function-call access key can re-attest. When a
+    /// charge applies, the caller must attach at least [`MINIMUM_ATTESTATION_STORAGE_DEPOSIT`]
+    /// (enough to cover the worst-case stored entry); only the actual storage delta is kept and
+    /// the excess is refunded. The full deposit is refunded if the attestation is not accepted.
     #[payable]
     #[handle_result]
     pub fn submit_participant_info(
@@ -839,30 +850,30 @@ 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());
-        }
+        // Non-participants pay per entry so an outsider cannot drain the contract; participants
+        // re-attest for free.
+        let caller_is_not_participant = !self
+            .protocol_state
+            .is_existing_or_prospective_participant(&account_id)
+            .unwrap_or(false);
 
         match proposed_participant_attestation {
             Attestation::Mock(mock) => {
                 let tee_upgrade_deadline_duration =
                     Duration::from_secs(self.config.tee_upgrade_deadline_duration_seconds);
                 let initial_storage = env::storage_usage();
-                self.tee_state.verify_and_store_mock(
+                let insertion = self.tee_state.verify_and_store_mock(
                     node_id,
                     mock,
                     tee_upgrade_deadline_duration,
                 )?;
-                keep_storage_delta_and_refund_rest(&account_id, initial_storage);
+                if insertion.is_new() || caller_is_not_participant {
+                    charge_storage(&account_id, initial_storage)?;
+                }
                 Ok(PromiseOrValue::Value(()))
             }
             Attestation::Dstack(attestation) => Ok(PromiseOrValue::Promise(
-                self.submit_dstack_attestation(node_id, attestation)?,
+                self.submit_dstack_attestation(node_id, attestation, caller_is_not_participant)?,
             )),
         }
     }
@@ -874,6 +885,7 @@ impl MpcContract {
         &mut self,
         node_id: NodeId,
         attestation: DstackAttestation,
+        caller_is_not_participant: bool,
     ) -> Result<Promise, Error> {
         let Some(verifier_account_id) = self.tee_verifier_account_id.clone() else {
             return Err(TeeError::VerifierNotConfigured.into());
@@ -894,6 +906,7 @@ impl MpcContract {
                     .resolve_verification(VerificationContext {
                         node_id,
                         attestation,
+                        caller_is_not_participant,
                     }),
             ))
     }
@@ -2383,8 +2396,8 @@ impl MpcContract {
     }
 
     /// Runs the post-DCAP checks and stores the attestation for a
-    /// [`VerificationResult::Verified`] response, then keeps the storage delta
-    /// and refunds the excess deposit.
+    /// [`VerificationResult::Verified`] response, then charges the storage delta unless a
+    /// participant re-attested an existing entry.
     fn verify_post_dcap_and_store(
         &mut self,
         context: &VerificationContext,
@@ -2395,17 +2408,22 @@ impl MpcContract {
             Duration::from_secs(self.config.tee_upgrade_deadline_duration_seconds);
 
         let initial_storage = env::storage_usage();
-        if let Err(err) = self.tee_state.verify_and_store_dstack(
+        let insertion = match self.tee_state.verify_and_store_dstack(
             context.node_id.clone(),
             &context.attestation,
             report,
             tee_upgrade_deadline_duration,
         ) {
-            log!("post-DCAP check failed for {account_id}: {err}");
-            return Err(err.into());
-        }
+            Ok(insertion) => insertion,
+            Err(err) => {
+                log!("post-DCAP check failed for {account_id}: {err}");
+                return Err(err.into());
+            }
+        };
 
-        keep_storage_delta_and_refund_rest(account_id, initial_storage);
+        if insertion.is_new() || context.caller_is_not_participant {
+            charge_storage(account_id, initial_storage)?;
+        }
         Ok(())
     }
 
@@ -4679,6 +4697,9 @@ mod tests {
             VerificationContext {
                 node_id,
                 attestation,
+                // Fixed, not parametrized: this flag only affects charging, which the mock VM
+                // can't observe, so true and false would store identical state here.
+                caller_is_not_participant: true,
             },
         )
     }
@@ -8116,19 +8137,21 @@ mod tests {
         assert!(configs.contains_key(&tls_key_b), "node B config must exist");
     }
 
+    const MAX_HASH: [u8; 32] = [0xff; 32];
+
     // Catches entry-size growth: fails if a schema change makes the largest storable entry
     // cost more than the deposit at today's storage_byte_cost. It cannot see a future
     // storage_byte_cost increase on a live contract; the deposit's margin covers that.
     #[rstest]
     #[case::dstack(VerifiedAttestation::Dstack(ValidatedDstackAttestation {
-        mpc_image_hash: [0xff; 32].into(),
-        launcher_compose_hash: [0xff; 32].into(),
+        mpc_image_hash: MAX_HASH.into(),
+        launcher_compose_hash: MAX_HASH.into(),
         expiry_timestamp_seconds: u64::MAX,
         measurements: default_measurements()[0],
     }))]
     #[case::mock(VerifiedAttestation::Mock(MpcMockAttestation::WithConstraints {
-        mpc_docker_image_hash: Some([0xff; 32].into()),
-        launcher_docker_compose_hash: Some([0xff; 32].into()),
+        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]),
     }))]

crates/contract/src/tee/tee_state.rs

@@ -53,6 +53,12 @@ pub(crate) enum ParticipantInsertion {
     UpdatedExistingParticipant,
 }
 
+impl ParticipantInsertion {
+    pub(crate) fn is_new(&self) -> bool {
+        matches!(self, Self::NewlyInsertedParticipant)
+    }
+}
+
 #[derive(Debug)]
 pub enum TeeValidationResult {
     /// All participants are valid

crates/contract/src/tee/verification_context.rs

@@ -11,4 +11,5 @@ use super::tee_state::NodeId;
 pub struct VerificationContext {
     pub(crate) node_id: NodeId,
     pub(crate) attestation: DstackAttestation,
+    pub(crate) caller_is_not_participant: bool,
 }

crates/contract/tests/inprocess/attestation_submission.rs

@@ -278,18 +278,18 @@ 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 non-participant storing a new entry must cover its storage cost, so an outsider cannot
+/// store an attestation without paying for it.
 #[test]
-fn submit_participant_info__should_reject_when_deposit_is_below_storage_cost() {
+fn submit_participant_info__should_reject_when_non_participant_deposit_is_below_storage_cost() {
     // Given
     let mut setup = TestSetupBuilder::new().build();
-    let node = setup.get_participant_node_ids()[0].clone();
+    let newcomer = node_id_for(&"newcomer.near".parse().unwrap());
     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())
+            .signer_account_id(newcomer.account_id.clone())
+            .predecessor_account_id(newcomer.account_id.clone())
             .attached_deposit(attached_deposit)
             .build()
     );
@@ -299,7 +299,7 @@ fn submit_participant_info__should_reject_when_deposit_is_below_storage_cost() {
         .contract
         .submit_participant_info(
             Attestation::Mock(MockAttestation::Valid),
-            node.tls_public_key.clone(),
+            newcomer.tls_public_key.clone(),
         )
         .map(|_| ());
 
@@ -311,6 +311,38 @@ fn submit_participant_info__should_reject_when_deposit_is_below_storage_cost() {
     );
 }
 
+/// A current participant re-attesting an existing entry is charged nothing and need not attach
+/// any deposit, so the node's function-call access key can re-attest.
+#[test]
+fn submit_participant_info__should_charge_nothing_when_participant_reattests_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() {

crates/contract/tests/sandbox/tee.rs

@@ -8,8 +8,8 @@ use crate::sandbox::{
         mpc_contract::{
             assert_running_return_participants, assert_running_return_threshold,
             get_participant_attestation, get_state, get_tee_accounts, submit_participant_info,
-            submit_participant_info_and_measure_kept_deposit, vote_add_launcher_hash,
-            vote_for_hash,
+            submit_participant_info_and_measure_kept_deposit, submit_participant_info_with_deposit,
+            vote_add_launcher_hash, vote_for_hash,
         },
         resharing_utils::conclude_resharing,
         sign_utils::DomainResponseTest,
@@ -21,8 +21,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};
 
@@ -280,6 +280,75 @@ async fn test_submit_participant_info_succeeds_with_mock_attestation() -> Result
     Ok(())
 }
 
+/// Regression test for #3925: a participant re-attesting through a function-call access key (which
+/// cannot attach a deposit) must succeed with zero deposit, and the same call with a deposit must
+/// be rejected by the protocol.
+#[tokio::test]
+async fn submit_participant_info__should_accept_zero_deposit_via_function_call_key() -> Result<()> {
+    let SandboxTestSetup {
+        worker,
+        contract,
+        mpc_signer_accounts,
+        ..
+    } = SandboxTestSetup::builder()
+        .with_protocols(ALL_PROTOCOLS)
+        .build()
+        .await;
+
+    // Setup already stored an attestation for this participant, so re-attesting is the free path.
+    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);
+
+    // Zero deposit through the fc key succeeds.
+    let zero = submit_participant_info_with_deposit(
+        &fc_account,
+        &contract,
+        &attestation,
+        &tls_key,
+        NearToken::from_yoctonear(0),
+    )
+    .await?;
+    assert!(
+        zero.is_success(),
+        "zero-deposit fc submission must succeed: {zero:?}"
+    );
+
+    // A deposit through the fc key is rejected at tx validation (the original bug).
+    let with_deposit = submit_participant_info_with_deposit(
+        &fc_account,
+        &contract,
+        &attestation,
+        &tls_key,
+        SUBMIT_PARTICIPANT_INFO_DEPOSIT,
+    )
+    .await;
+    let err = format!("{with_deposit:?}");
+    assert!(
+        err.contains("DepositWithFunctionCall"),
+        "fc submission with a deposit must be rejected: {err}"
+    );
+    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]

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

@@ -47,8 +47,8 @@ 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.
+/// Attached by sandbox onboarding submissions to `submit_participant_info`, which store a new
+/// entry the contract charges for; the excess over the actual storage delta is refunded.
 pub const SUBMIT_PARTICIPANT_INFO_DEPOSIT: NearToken =
     NearToken::from_millinear(SUBMIT_PARTICIPANT_INFO_DEPOSIT_MILLINEAR);