Skip to content

Commit 82c126e

Browse files
committed
fix(contract): size max_scan and the sweep budget together
Scanning an entry costs gas whether or not it is removed, so RESHARE_CLEAN_INVALID_ATTESTATIONS_MAX_SCAN and clean_invalid_attestations_tera_gas have to be sized against each other. They were not: max_scan 100 needs ~28 TGas to walk the list with zero removals, against a 10 TGas budget. The promise is detached, so exceeding it rolls back every removal silently, which a live sweep on v1.signer-prod.testnet confirmed by burning 13.1 TGas. Lower max_scan 100 -> 30 and raise the budget 10 -> 15 TGas. A 30-entry scan costs ~9.9 TGas before any removal; 15 TGas covers ~7 removals on top, measured with #4016's storage-grant return in place (which took the per-removal cost from 0.362 to 0.588 TGas). Reset that field during migration rather than carrying the deployed value forward, so it applies on upgrade without a governance config vote. Every scanned entry being removable stays unfunded (~22 TGas); the permissionless entry point is the recovery path. Closes #4035
1 parent 0a31d96 commit 82c126e

10 files changed

Lines changed: 888 additions & 9 deletions

File tree

crates/contract/src/bench.rs

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,10 @@
66
77
use crate::MpcContract;
88
use crate::primitives::participants::ParticipantInfo;
9+
use crate::tee::tee_state::{NodeAttestation, NodeId};
10+
use mpc_attestation::attestation::{
11+
MockAttestation, ValidatedDstackAttestation, VerifiedAttestation, default_measurements,
12+
};
913
use near_account_id::AccountId;
1014
use near_mpc_contract_interface::types as dtos;
1115
use near_sdk::near;
@@ -123,4 +127,53 @@ impl MpcContract {
123127
.update_info(account_id, new_info)
124128
.is_ok()
125129
}
130+
131+
/// Benchmark: seeds `count` attestations keyed by a synthetic TLS key derived from
132+
/// `offset + i`, so successive calls with disjoint offsets accumulate. Returns the
133+
/// resulting map size.
134+
///
135+
/// Bypasses `add_participant`, and so DCAP verification, which is what makes seeding a
136+
/// backlog of arbitrary size affordable.
137+
/// `expired` picks an entry [`crate::tee::tee_state::TeeState::clean_invalid_attestations`]
138+
/// removes over one it keeps, isolating the per-entry scan cost from the removal cost.
139+
pub fn bench_seed_attestations(&mut self, offset: u32, count: u32, expired: bool) -> u32 {
140+
let end = offset
141+
.checked_add(count)
142+
.expect("bench seeding range must not overflow");
143+
let measurements = default_measurements()[0];
144+
let verified_attestation = if expired {
145+
VerifiedAttestation::Dstack(ValidatedDstackAttestation {
146+
mpc_image_hash: [7u8; 32].into(),
147+
launcher_compose_hash: [8u8; 32].into(),
148+
expiry_timestamp_seconds: 1,
149+
measurements,
150+
})
151+
} else {
152+
VerifiedAttestation::Mock(MockAttestation::WithConstraints {
153+
mpc_docker_image_hash: None,
154+
launcher_docker_compose_hash: None,
155+
expiry_timestamp_seconds: Some(u64::MAX),
156+
expected_measurements: None,
157+
})
158+
};
159+
160+
for i in offset..end {
161+
let mut key_bytes = [0u8; 32];
162+
key_bytes[..4].copy_from_slice(&i.to_le_bytes());
163+
let tls_public_key: dtos::Ed25519PublicKey = key_bytes.into();
164+
let account_id: AccountId = format!("bench-attested-{i}.near").parse().unwrap();
165+
self.tee_state.stored_attestations.insert(
166+
tls_public_key.clone(),
167+
NodeAttestation {
168+
node_id: NodeId {
169+
account_id,
170+
tls_public_key: tls_public_key.clone(),
171+
account_public_key: tls_public_key,
172+
},
173+
verified_attestation: verified_attestation.clone(),
174+
},
175+
);
176+
}
177+
self.tee_state.stored_attestations.len()
178+
}
126179
}

crates/contract/src/config.rs

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,18 @@ const DEFAULT_FAIL_ON_TIMEOUT_TERA_GAS: u64 = 2;
2525
const DEFAULT_FAIL_ATTESTATION_SUBMISSION_TERA_GAS: u64 = 2;
2626
/// Prepaid gas for a `clean_tee_status` call
2727
const DEFAULT_CLEAN_TEE_STATUS_TERA_GAS: u64 = 10;
28-
/// Prepaid gas for the reshare-time `clean_invalid_attestations` promise.
29-
const DEFAULT_CLEAN_INVALID_ATTESTATIONS_TERA_GAS: u64 = 10;
28+
/// Prepaid gas for the reshare-time [`crate::MpcContract::clean_invalid_attestations`] promise.
29+
///
30+
/// Sized against [`crate::RESHARE_CLEAN_INVALID_ATTESTATIONS_MAX_SCAN`]. Measured on
31+
/// `v1.signer-prod.testnet`: ~3.5 TGas of fixed overhead and ~0.215 TGas per entry
32+
/// re-verified, so a full 30-entry scan costs ~9.9 TGas before a single removal. Each removal
33+
/// adds ~0.5 TGas on top, dominated by returning that entry's storage grant, leaving this
34+
/// budget room for ~10 of them.
35+
///
36+
/// It does **not** cover every scanned entry being removable (~18 TGas). The promise is
37+
/// detached, so exceeding it rolls back every removal with no partial progress; the
38+
/// permissionless entry point is the recovery path, clearing ~750 entries in one call.
39+
const DEFAULT_CLEAN_INVALID_ATTESTATIONS_TERA_GAS: u64 = 15;
3040
/// Prepaid gas for a `cleanup_orphaned_node_migrations` call
3141
/// TODO(#1164): benchmark
3242
const DEFAULT_CLEANUP_ORPHANED_NODE_MIGRATIONS_TERA_GAS: u64 = 4;

crates/contract/src/lib.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,11 @@ pub const MINIMUM_NODE_MANAGEMENT_DEPOSIT: NearToken =
121121

122122
/// Entries to scan in the post-reshare `clean_invalid_attestations` sweep. External
123123
/// callers may pick a different value; this only governs the automatic invocation.
124-
const RESHARE_CLEAN_INVALID_ATTESTATIONS_MAX_SCAN: u32 = 100;
124+
///
125+
/// Kept in step with `clean_invalid_attestations_tera_gas`: scanning an entry costs gas
126+
/// whether or not it is removed, so this limit and that budget must be sized together
127+
/// (#4035). `clean_invalid_attestations__budget_covers_max_scan` pins the relation.
128+
const RESHARE_CLEAN_INVALID_ATTESTATIONS_MAX_SCAN: u32 = 30;
125129

126130
/// Checks that the caller attached at least `minimum_deposit` and refunds any excess.
127131
///

crates/contract/src/tee/tee_state.rs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -274,9 +274,12 @@ impl TeeState {
274274
}
275275

276276
/// Evicts expired entries from the allowed docker-image and launcher-image sets, then
277-
/// reverifies stored participant attestations, removing any that fail reverification
278-
/// (e.g. the MPC image hash the attestation was tied to is no longer allowed, or a
279-
/// certificate expired).
277+
/// reverifies stored participant attestations and returns the subset that still passes.
278+
/// Reverification fails when, for example, the MPC image hash the attestation was tied
279+
/// to is no longer allowed, or a certificate expired.
280+
///
281+
/// Despite the name, `stored_attestations` is only read here, never pruned —
282+
/// reclaiming entries is [`Self::clean_invalid_attestations`]'s job alone.
280283
pub fn reverify_and_cleanup_participants(
281284
&mut self,
282285
participants: &Participants,

crates/contract/src/v3_14_0_state.rs

Lines changed: 136 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,12 @@ impl From<OldConfig> for Config {
5858
fn from(old: OldConfig) -> Self {
5959
// Carry the deployed values; the attestation-storage fee is new in this release, so
6060
// it takes its default.
61+
//
62+
// `clean_invalid_attestations_tera_gas` is the deliberate exception: it is reset to
63+
// the new default rather than carried over. The deployed 10 TGas cannot fund the scan
64+
// `vote_reshared` requests, and the promise is detached, so every removal rolls back
65+
// unnoticed (#4035). Carrying it forward would leave the fix unreachable without a
66+
// governance config vote.
6167
Config {
6268
key_event_timeout_blocks: old.key_event_timeout_blocks,
6369
tee_upgrade_deadline_duration_seconds: old.tee_upgrade_deadline_duration_seconds,
@@ -73,7 +79,8 @@ impl From<OldConfig> for Config {
7379
fail_on_timeout_tera_gas: old.fail_on_timeout_tera_gas,
7480
fail_attestation_submission_tera_gas: old.fail_attestation_submission_tera_gas,
7581
clean_tee_status_tera_gas: old.clean_tee_status_tera_gas,
76-
clean_invalid_attestations_tera_gas: old.clean_invalid_attestations_tera_gas,
82+
clean_invalid_attestations_tera_gas: Config::default()
83+
.clean_invalid_attestations_tera_gas,
7784
cleanup_orphaned_node_migrations_tera_gas: old
7885
.cleanup_orphaned_node_migrations_tera_gas,
7986
remove_non_participant_update_votes_tera_gas: old
@@ -153,3 +160,131 @@ struct Metrics {
153160
sign_with_v1_payload_count: u64,
154161
sign_with_v2_payload_count: u64,
155162
}
163+
164+
#[cfg(test)]
165+
#[expect(non_snake_case)]
166+
mod tests {
167+
use super::*;
168+
169+
/// Every field distinguishable from [`Config::default()`], so a carried-forward value
170+
/// cannot be mistaken for a defaulted one.
171+
fn deployed_config() -> OldConfig {
172+
OldConfig {
173+
key_event_timeout_blocks: 1,
174+
tee_upgrade_deadline_duration_seconds: 2,
175+
contract_upgrade_deposit_tera_gas: 3,
176+
sign_call_gas_attachment_requirement_tera_gas: 4,
177+
ckd_call_gas_attachment_requirement_tera_gas: 5,
178+
return_signature_and_clean_state_on_success_call_tera_gas: 6,
179+
return_ck_and_clean_state_on_success_call_tera_gas: 7,
180+
fail_on_timeout_tera_gas: 8,
181+
fail_attestation_submission_tera_gas: 9,
182+
clean_tee_status_tera_gas: 10,
183+
clean_invalid_attestations_tera_gas: 11,
184+
cleanup_orphaned_node_migrations_tera_gas: 12,
185+
remove_non_participant_update_votes_tera_gas: 13,
186+
clean_foreign_chain_data_tera_gas: 14,
187+
remove_non_participant_tee_verifier_votes_tera_gas: 15,
188+
verifier_tera_gas: 16,
189+
resolve_verification_tera_gas: 17,
190+
launcher_hash_unused_ttl_seconds: 18,
191+
}
192+
}
193+
194+
/// The deployed budget cannot fund the scan [`crate::MpcContract::vote_reshared`] requests,
195+
/// and the promise is detached, so the sweep rolls back silently. Migration resets this one
196+
/// field so the fix lands on upgrade rather than needing a governance vote (#4035).
197+
#[test]
198+
fn config_migration__should_reset_clean_invalid_attestations_gas_to_the_new_default() {
199+
// given
200+
let old = deployed_config();
201+
assert_ne!(
202+
old.clean_invalid_attestations_tera_gas,
203+
Config::default().clean_invalid_attestations_tera_gas
204+
);
205+
206+
// when
207+
let migrated = Config::from(old);
208+
209+
// then
210+
assert_eq!(
211+
migrated.clean_invalid_attestations_tera_gas,
212+
Config::default().clean_invalid_attestations_tera_gas
213+
);
214+
}
215+
216+
/// The reset must not leak into any other field an operator may have voted in.
217+
#[test]
218+
fn config_migration__should_carry_every_other_deployed_value_forward() {
219+
// given / when
220+
let old = deployed_config();
221+
let migrated = Config::from(deployed_config());
222+
223+
// then
224+
assert_eq!(
225+
migrated.key_event_timeout_blocks,
226+
old.key_event_timeout_blocks
227+
);
228+
assert_eq!(
229+
migrated.tee_upgrade_deadline_duration_seconds,
230+
old.tee_upgrade_deadline_duration_seconds
231+
);
232+
assert_eq!(
233+
migrated.contract_upgrade_deposit_tera_gas,
234+
old.contract_upgrade_deposit_tera_gas
235+
);
236+
assert_eq!(
237+
migrated.sign_call_gas_attachment_requirement_tera_gas,
238+
old.sign_call_gas_attachment_requirement_tera_gas
239+
);
240+
assert_eq!(
241+
migrated.ckd_call_gas_attachment_requirement_tera_gas,
242+
old.ckd_call_gas_attachment_requirement_tera_gas
243+
);
244+
assert_eq!(
245+
migrated.return_signature_and_clean_state_on_success_call_tera_gas,
246+
old.return_signature_and_clean_state_on_success_call_tera_gas
247+
);
248+
assert_eq!(
249+
migrated.return_ck_and_clean_state_on_success_call_tera_gas,
250+
old.return_ck_and_clean_state_on_success_call_tera_gas
251+
);
252+
assert_eq!(
253+
migrated.fail_on_timeout_tera_gas,
254+
old.fail_on_timeout_tera_gas
255+
);
256+
assert_eq!(
257+
migrated.fail_attestation_submission_tera_gas,
258+
old.fail_attestation_submission_tera_gas
259+
);
260+
assert_eq!(
261+
migrated.clean_tee_status_tera_gas,
262+
old.clean_tee_status_tera_gas
263+
);
264+
assert_eq!(
265+
migrated.cleanup_orphaned_node_migrations_tera_gas,
266+
old.cleanup_orphaned_node_migrations_tera_gas
267+
);
268+
assert_eq!(
269+
migrated.remove_non_participant_update_votes_tera_gas,
270+
old.remove_non_participant_update_votes_tera_gas
271+
);
272+
assert_eq!(
273+
migrated.clean_foreign_chain_data_tera_gas,
274+
old.clean_foreign_chain_data_tera_gas
275+
);
276+
assert_eq!(
277+
migrated.remove_non_participant_tee_verifier_votes_tera_gas,
278+
old.remove_non_participant_tee_verifier_votes_tera_gas
279+
);
280+
assert_eq!(migrated.verifier_tera_gas, old.verifier_tera_gas);
281+
assert_eq!(
282+
migrated.resolve_verification_tera_gas,
283+
old.resolve_verification_tera_gas
284+
);
285+
assert_eq!(
286+
migrated.launcher_hash_unused_ttl_seconds,
287+
old.launcher_hash_unused_ttl_seconds
288+
);
289+
}
290+
}

0 commit comments

Comments
 (0)