Skip to content

Commit 808d024

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 808d024

10 files changed

Lines changed: 871 additions & 7 deletions

File tree

crates/contract/src/bench.rs

Lines changed: 50 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,50 @@ impl MpcContract {
123127
.update_info(account_id, new_info)
124128
.is_ok()
125129
}
130+
131+
/// Benchmark: seeds `count` entries into `stored_attestations`, keyed by a synthetic TLS
132+
/// key derived from `offset + i` so successive calls with disjoint offsets accumulate.
133+
///
134+
/// Bypasses `add_participant` (and thus DCAP verification) so a backlog of arbitrary
135+
/// size can be created in a few transactions. `expired` selects between a `Dstack`
136+
/// entry that `clean_invalid_attestations` removes and an unconstrained `Mock` entry it
137+
/// keeps — the latter isolates the per-entry scan cost paid for surviving entries.
138+
/// Returns the resulting map size.
139+
pub fn bench_seed_attestations(&mut self, offset: u32, count: u32, expired: bool) -> u32 {
140+
let measurements = default_measurements()[0];
141+
let verified_attestation = if expired {
142+
VerifiedAttestation::Dstack(ValidatedDstackAttestation {
143+
mpc_image_hash: [7u8; 32].into(),
144+
launcher_compose_hash: [8u8; 32].into(),
145+
expiry_timestamp_seconds: 1,
146+
measurements,
147+
})
148+
} else {
149+
VerifiedAttestation::Mock(MockAttestation::WithConstraints {
150+
mpc_docker_image_hash: None,
151+
launcher_docker_compose_hash: None,
152+
expiry_timestamp_seconds: Some(u64::MAX),
153+
expected_measurements: None,
154+
})
155+
};
156+
157+
for i in offset..offset + count {
158+
let mut key_bytes = [0u8; 32];
159+
key_bytes[..4].copy_from_slice(&i.to_le_bytes());
160+
let tls_public_key: dtos::Ed25519PublicKey = key_bytes.into();
161+
let account_id: AccountId = format!("bench-attested-{i}.near").parse().unwrap();
162+
self.tee_state.stored_attestations.insert(
163+
tls_public_key.clone(),
164+
NodeAttestation {
165+
node_id: NodeId {
166+
account_id,
167+
tls_public_key: tls_public_key.clone(),
168+
account_public_key: tls_public_key,
169+
},
170+
verified_attestation: verified_attestation.clone(),
171+
},
172+
);
173+
}
174+
self.tee_state.stored_attestations.len()
175+
}
126176
}

crates/contract/src/config.rs

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,17 @@ 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`]: ~3.7 TGas of fixed
31+
/// overhead, ~0.21 TGas per entry re-verified, and ~0.59 TGas more for each one removed
32+
/// (dominated by returning that entry's storage grant). A full 30-entry scan therefore costs
33+
/// ~9.9 TGas before any removal, and this budget covers roughly 7 removals on top.
34+
///
35+
/// It does **not** cover every scanned entry being removable (~22 TGas). The promise is
36+
/// detached, so exceeding it rolls back every removal with no partial progress; the
37+
/// permissionless entry point is the recovery path, clearing ~750 entries in one call.
38+
const DEFAULT_CLEAN_INVALID_ATTESTATIONS_TERA_GAS: u64 = 15;
3039
/// Prepaid gas for a `cleanup_orphaned_node_migrations` call
3140
/// TODO(#1164): benchmark
3241
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: 4 additions & 1 deletion
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
277+
/// reverifies stored participant attestations and returns the subset still passing
278278
/// (e.g. the MPC image hash the attestation was tied to is no longer allowed, or a
279279
/// certificate expired).
280+
///
281+
/// Despite the name, `stored_attestations` is only read here, never pruned — reclaiming
282+
/// entries is [`TeeState::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)