diff --git a/crates/contract/src/dto_mapping.rs b/crates/contract/src/dto_mapping.rs index 41d904d9d..7f7465906 100644 --- a/crates/contract/src/dto_mapping.rs +++ b/crates/contract/src/dto_mapping.rs @@ -25,7 +25,7 @@ use crate::{ key_state::{AuthenticatedAccountId, AuthenticatedParticipantId, KeyForDomain, Keyset}, participants::{ParticipantInfo, Participants}, threshold_votes::ThresholdParametersVotes, - thresholds::ThresholdParameters, + thresholds::{ProposedThresholdParameters, ThresholdParameters}, }, state::{ ProtocolContractState, @@ -214,10 +214,28 @@ impl IntoContractType for dtos::Participants { impl IntoContractType for dtos::ThresholdParameters { fn into_contract_type(self) -> ThresholdParameters { + // This conversion is intentionally infallible: `new_unvalidated` skips the + // absolute/relative (>= 60%) threshold checks. Validation is not the job of the + // DTO mapping — every contract entry point that accepts these parameters + // (`init`, `init_running`, `vote_new_parameters`) calls `validate()` / + // `validate_incoming_proposal` downstream, so production proposals are still + // rejected if they violate the threshold bounds. Deferring validation here also + // lets tests construct parameters with sub-production thresholds. ThresholdParameters::new_unvalidated(self.participants.into_contract_type(), self.threshold) } } +impl IntoContractType for dtos::ProposedThresholdParameters { + fn into_contract_type(self) -> ProposedThresholdParameters { + // Infallible for the same reason as `ThresholdParameters` above: the + // proposal is validated downstream in `process_new_parameters_proposal`. + ProposedThresholdParameters::new( + self.parameters.into_contract_type(), + self.per_domain_thresholds, + ) + } +} + impl TryIntoContractType for dtos::EventLog { type Error = Error; fn try_into_contract_type(self) -> Result { @@ -636,6 +654,12 @@ mod to_dto { } } + impl From for dtos::ProposedThresholdParameters { + fn from(params: ProposedThresholdParameters) -> Self { + (¶ms).into_dto_type() + } + } + impl From for dtos::ParticipantInfo { fn from(info: ParticipantInfo) -> Self { dtos::ParticipantInfo { @@ -729,6 +753,15 @@ impl IntoInterfaceType for &ThresholdParameters { } } +impl IntoInterfaceType for &ProposedThresholdParameters { + fn into_dto_type(self) -> dtos::ProposedThresholdParameters { + dtos::ProposedThresholdParameters { + parameters: self.parameters().into_dto_type(), + per_domain_thresholds: self.per_domain_thresholds().clone(), + } + } +} + // --- Voting types --- impl IntoInterfaceType for &ThresholdParametersVotes { @@ -853,6 +886,7 @@ impl IntoInterfaceType for &ResharingContractState .iter() .map(|a| a.into_dto_type()) .collect(), + per_domain_thresholds: self.per_domain_thresholds.clone(), } } } diff --git a/crates/contract/src/errors.rs b/crates/contract/src/errors.rs index bfdec9e44..a87ed8726 100644 --- a/crates/contract/src/errors.rs +++ b/crates/contract/src/errors.rs @@ -261,6 +261,10 @@ pub enum DomainError { "Reconstruction threshold {threshold} overflowed when computing the DamgardEtAl bound." )] ReconstructionThresholdOverflow { threshold: u64 }, + #[error( + "Resharing proposal references domain ID {domain_id}, which is not in the current registry." + )] + UnknownDomainInProposal { domain_id: DomainId }, #[error("CaitSith threshold mismatch: expected {expected}, found {found}.")] CaitsithThresholdMismatch { expected: u64, found: u64 }, } diff --git a/crates/contract/src/lib.rs b/crates/contract/src/lib.rs index 05a9fdff5..167c7cc02 100644 --- a/crates/contract/src/lib.rs +++ b/crates/contract/src/lib.rs @@ -79,7 +79,7 @@ use primitives::{ domain::DomainRegistry, key_state::{AuthenticatedParticipantId, EpochId, KeyEventId, Keyset}, signature::{SignRequestArgs, SignatureRequest, YieldIndex}, - thresholds::{Threshold, ThresholdParameters}, + thresholds::{ProposedThresholdParameters, Threshold, ThresholdParameters}, }; use tee::measurements::{ContractExpectedMeasurements, MeasurementVoteAction, MeasurementVotes}; use tee::proposal::{CodeHashesVotes, LauncherHashVotes}; @@ -873,10 +873,10 @@ impl MpcContract { pub fn vote_new_parameters( &mut self, prospective_epoch_id: EpochId, - proposal: dtos::ThresholdParameters, + proposal: dtos::ProposedThresholdParameters, ) -> Result<(), Error> { Self::assert_caller_is_signer(); - let proposal: ThresholdParameters = proposal.into_contract_type(); + let proposal: ProposedThresholdParameters = proposal.into_contract_type(); log!( "vote_new_parameters: signer={}, proposal={:?}", env::signer_account_id(), @@ -1604,7 +1604,11 @@ impl MpcContract { ) .expect("Require valid threshold parameters"); // this should never happen. current_params.validate_incoming_proposal(&threshold_parameters)?; - let res = running_state.transition_to_resharing_no_checks(&threshold_parameters); + // TEE-driven resharing only changes the participant set, so the + // per-domain reconstruction-threshold overlay is empty. + let proposed_parameters = + ProposedThresholdParameters::new(threshold_parameters, BTreeMap::new()); + let res = running_state.transition_to_resharing_no_checks(&proposed_parameters); if let Some(resharing) = res { self.protocol_state = ProtocolContractState::Resharing(resharing); } @@ -3676,7 +3680,10 @@ mod tests { .build(); testing_env!(voting_context); - let proposal = ThresholdParameters::new(participants, threshold).unwrap(); + let proposal = ProposedThresholdParameters::new( + ThresholdParameters::new(participants, threshold).unwrap(), + BTreeMap::new(), + ); contract.vote_new_parameters(EpochId::new(1), (&proposal).into_dto_type()) } @@ -3781,7 +3788,10 @@ mod tests { // so signer_account_id (the participant) != predecessor_account_id (the forwarder). let (mut contract, participants, first_participant_id) = setup_tee_test_contract(3, 2); let threshold = Threshold::new(2); - let proposal = ThresholdParameters::new(participants, threshold).unwrap(); + let proposal = ProposedThresholdParameters::new( + ThresholdParameters::new(participants, threshold).unwrap(), + BTreeMap::new(), + ); let ctx = VMContextBuilder::new() .signer_account_id(first_participant_id) @@ -5169,6 +5179,7 @@ mod tests { expected_params, ), cancellation_requests: HashSet::new(), + per_domain_thresholds: BTreeMap::new(), }; assert_eq!(*resharing_state, expected_resharing_state); diff --git a/crates/contract/src/primitives/domain.rs b/crates/contract/src/primitives/domain.rs index 1be238819..c8ccec7e9 100644 --- a/crates/contract/src/primitives/domain.rs +++ b/crates/contract/src/primitives/domain.rs @@ -1,7 +1,9 @@ use super::key_state::AuthenticatedParticipantId; use crate::errors::{DomainError, Error}; use crate::primitives::participants::Participants; -use near_mpc_contract_interface::types::{Curve, DomainConfig, DomainId, DomainPurpose, Protocol}; +use near_mpc_contract_interface::types::{ + Curve, DomainConfig, DomainId, DomainPurpose, Protocol, ReconstructionThreshold, +}; use near_sdk::{log, near}; use std::collections::BTreeMap; @@ -69,6 +71,30 @@ pub fn validate_domain_threshold( Ok(()) } +/// Enforces the 3.11-transition lock: every CaitSith domain (ForeignTx +/// included) must share a single `reconstruction_threshold` so the legacy +/// unprefixed `DBCol::Triple` mirror (#3292) can't collide. Domains using +/// other protocols are unconstrained. The first CaitSith domain encountered +/// fixes the expected value; any later CaitSith domain with a different +/// threshold is rejected. Remove once #3298 drops the mirror. +pub fn validate_caitsith_uniform_threshold(domains: &[DomainConfig]) -> Result<(), Error> { + let mut expected: Option = None; + for domain in domains { + if domain.protocol != Protocol::CaitSith { + continue; + } + let found = domain.reconstruction_threshold.inner(); + match expected { + None => expected = Some(found), + Some(expected) if expected != found => { + return Err(DomainError::CaitsithThresholdMismatch { expected, found }.into()); + } + Some(_) => {} + } + } + Ok(()) +} + /// All the domains present in the contract, as well as the next domain ID which is kept to ensure /// that we never reuse domain IDs. (Domains may be deleted in only one case: when we decided to /// add domains but ultimately canceled that process.) @@ -177,6 +203,49 @@ impl DomainRegistry { pub fn next_domain_id(&self) -> u64 { self.next_domain_id } + + /// Returns a new registry whose domains have their + /// `reconstruction_threshold` rewritten from `overlay`. Domain IDs in + /// `overlay` that are not present in the registry are rejected with + /// [`DomainError::UnknownDomainInProposal`]. Domains absent from + /// `overlay` retain their existing threshold. An empty overlay returns a + /// structurally identical clone. + /// + /// The resulting registry is re-checked against the 3.11-transition lock + /// (see [`validate_caitsith_uniform_threshold`]): because the overlay can + /// rewrite per-domain thresholds, it could otherwise leave CaitSith + /// domains with differing thresholds. This is the authoritative chokepoint + /// — no resharing transition can produce a registry that violates the + /// invariant. + pub fn with_overlaid_thresholds( + &self, + overlay: &BTreeMap, + ) -> Result { + for id in overlay.keys() { + if !self.domains.iter().any(|d| d.id == *id) { + return Err(DomainError::UnknownDomainInProposal { domain_id: *id }.into()); + } + } + let domains: Vec = self + .domains + .iter() + .map(|d| { + let reconstruction_threshold = overlay + .get(&d.id) + .copied() + .unwrap_or(d.reconstruction_threshold); + DomainConfig { + reconstruction_threshold, + ..d.clone() + } + }) + .collect(); + validate_caitsith_uniform_threshold(&domains)?; + Ok(DomainRegistry { + domains, + next_domain_id: self.next_domain_id, + }) + } } /// Tracks votes to add domains. Each participant can at any given time vote for a list of domains @@ -229,6 +298,7 @@ impl AddDomainsVotes { } #[cfg(test)] +#[expect(non_snake_case)] pub mod tests { use super::{ AddDomainsVotes, Curve, DomainConfig, DomainId, DomainPurpose, DomainRegistry, @@ -242,6 +312,7 @@ pub mod tests { use near_sdk::test_utils::VMContextBuilder; use near_sdk::testing_env; use rstest::rstest; + use std::collections::BTreeMap; #[test] fn test_add_domains() { @@ -567,4 +638,169 @@ pub mod tests { assert_eq!(remaining.proposal_by_account[&auth_ids[0]], proposal_a); assert_eq!(remaining.proposal_by_account[&auth_ids[1]], proposal_b); } + + fn registry_of(domains: Vec) -> DomainRegistry { + let next_domain_id = domains.iter().map(|d| d.id.0).max().map_or(0, |m| m + 1); + DomainRegistry::from_raw_validated(domains, next_domain_id).unwrap() + } + + #[test] + fn with_overlaid_thresholds__should_be_identity_when_overlay_is_empty() { + // Given a non-empty registry and an empty overlay + let registry = registry_of(vec![ + DomainConfig { + id: DomainId(0), + protocol: Protocol::CaitSith, + reconstruction_threshold: ReconstructionThreshold::new(3), + purpose: DomainPurpose::Sign, + }, + DomainConfig { + id: DomainId(1), + protocol: Protocol::Frost, + reconstruction_threshold: ReconstructionThreshold::new(2), + purpose: DomainPurpose::Sign, + }, + ]); + let overlay = BTreeMap::new(); + + // When applying the overlay + let result = registry.with_overlaid_thresholds(&overlay).unwrap(); + + // Then the registry is structurally identical + assert_eq!(result, registry); + } + + #[test] + fn with_overlaid_thresholds__should_apply_per_domain_updates() { + // Given a registry with two domains and an overlay targeting one + let registry = registry_of(vec![ + DomainConfig { + id: DomainId(0), + protocol: Protocol::CaitSith, + reconstruction_threshold: ReconstructionThreshold::new(3), + purpose: DomainPurpose::Sign, + }, + DomainConfig { + id: DomainId(1), + protocol: Protocol::Frost, + reconstruction_threshold: ReconstructionThreshold::new(2), + purpose: DomainPurpose::Sign, + }, + ]); + let mut overlay = BTreeMap::new(); + overlay.insert(DomainId(0), ReconstructionThreshold::new(5)); + + // When applying the overlay + let result = registry.with_overlaid_thresholds(&overlay).unwrap(); + + // Then only the targeted domain's threshold changes + assert_eq!( + result.domains()[0].reconstruction_threshold, + ReconstructionThreshold::new(5) + ); + assert_eq!( + result.domains()[1].reconstruction_threshold, + ReconstructionThreshold::new(2) + ); + } + + #[test] + fn with_overlaid_thresholds__should_reject_unknown_domain_id() { + // Given a registry with one domain and an overlay referencing a different ID + let registry = registry_of(vec![DomainConfig { + id: DomainId(0), + protocol: Protocol::CaitSith, + reconstruction_threshold: ReconstructionThreshold::new(3), + purpose: DomainPurpose::Sign, + }]); + let mut overlay = BTreeMap::new(); + overlay.insert(DomainId(42), ReconstructionThreshold::new(5)); + + // When applying the overlay + let err = registry.with_overlaid_thresholds(&overlay).unwrap_err(); + + // Then unknown-domain guard rejects + assert!( + err.to_string().contains("not in the current registry"), + "Expected UnknownDomainInProposal, got: {err}" + ); + } + + #[test] + fn with_overlaid_thresholds__should_reject_overlay_that_diverges_caitsith_thresholds() { + // Given a registry with two CaitSith domains sharing one threshold + // (the 3.11-transition lock invariant) and an overlay that rewrites + // only one of them to a different value. + let registry = registry_of(vec![ + DomainConfig { + id: DomainId(0), + protocol: Protocol::CaitSith, + reconstruction_threshold: ReconstructionThreshold::new(3), + purpose: DomainPurpose::Sign, + }, + DomainConfig { + id: DomainId(1), + protocol: Protocol::CaitSith, + reconstruction_threshold: ReconstructionThreshold::new(3), + purpose: DomainPurpose::Sign, + }, + ]); + let mut overlay = BTreeMap::new(); + overlay.insert(DomainId(0), ReconstructionThreshold::new(5)); + + // When applying the overlay + let err = registry.with_overlaid_thresholds(&overlay).unwrap_err(); + + // Then the 3.11-transition lock rejects the divergence + assert!( + err.to_string().contains("CaitSith threshold mismatch"), + "Expected CaitsithThresholdMismatch, got: {err}" + ); + } + + #[test] + fn with_overlaid_thresholds__should_accept_overlay_that_keeps_caitsith_thresholds_uniform() { + // Given two CaitSith domains and a Frost domain, with an overlay + // that moves both CaitSith domains to the same new threshold. + let registry = registry_of(vec![ + DomainConfig { + id: DomainId(0), + protocol: Protocol::CaitSith, + reconstruction_threshold: ReconstructionThreshold::new(3), + purpose: DomainPurpose::Sign, + }, + DomainConfig { + id: DomainId(1), + protocol: Protocol::CaitSith, + reconstruction_threshold: ReconstructionThreshold::new(3), + purpose: DomainPurpose::Sign, + }, + DomainConfig { + id: DomainId(2), + protocol: Protocol::Frost, + reconstruction_threshold: ReconstructionThreshold::new(2), + purpose: DomainPurpose::Sign, + }, + ]); + let mut overlay = BTreeMap::new(); + overlay.insert(DomainId(0), ReconstructionThreshold::new(5)); + overlay.insert(DomainId(1), ReconstructionThreshold::new(5)); + + // When applying the overlay + let result = registry.with_overlaid_thresholds(&overlay).unwrap(); + + // Then both CaitSith domains move together and Frost is untouched + assert_eq!( + result.domains()[0].reconstruction_threshold, + ReconstructionThreshold::new(5) + ); + assert_eq!( + result.domains()[1].reconstruction_threshold, + ReconstructionThreshold::new(5) + ); + assert_eq!( + result.domains()[2].reconstruction_threshold, + ReconstructionThreshold::new(2) + ); + } } diff --git a/crates/contract/src/primitives/test_utils.rs b/crates/contract/src/primitives/test_utils.rs index a1c6dda45..43d6bd312 100644 --- a/crates/contract/src/primitives/test_utils.rs +++ b/crates/contract/src/primitives/test_utils.rs @@ -3,7 +3,7 @@ use crate::{ crypto_shared::types::{PublicKeyExtended, serializable::SerializableEdwardsPoint}, primitives::{ participants::{ParticipantInfo, Participants}, - thresholds::{Threshold, ThresholdParameters}, + thresholds::{ProposedThresholdParameters, Threshold, ThresholdParameters}, }, }; use curve25519_dalek::edwards::CompressedEdwardsY; @@ -163,6 +163,12 @@ pub fn gen_threshold_params(max_n: usize) -> ThresholdParameters { ThresholdParameters::new(gen_participants(n), Threshold::new(k as u64)).unwrap() } +/// Like [`gen_threshold_params`] but wrapped as a proposal with an empty +/// (no-change) per-domain overlay — the shape `vote_new_parameters` accepts. +pub fn gen_proposed_threshold_params(max_n: usize) -> ProposedThresholdParameters { + ProposedThresholdParameters::new(gen_threshold_params(max_n), BTreeMap::new()) +} + /// Infer a default purpose from the protocol. /// Used during migration from old state that lacks the `purpose` field. pub fn infer_purpose_from_protocol(protocol: Protocol) -> DomainPurpose { diff --git a/crates/contract/src/primitives/threshold_votes.rs b/crates/contract/src/primitives/threshold_votes.rs index d5b8e4748..60090f3cc 100644 --- a/crates/contract/src/primitives/threshold_votes.rs +++ b/crates/contract/src/primitives/threshold_votes.rs @@ -1,21 +1,25 @@ -use crate::primitives::thresholds::ThresholdParameters; +use crate::primitives::thresholds::ProposedThresholdParameters; use crate::primitives::{key_state::AuthenticatedAccountId, participants::Participants}; use near_sdk::{log, near}; use std::collections::BTreeMap; -/// Tracks votes for ThresholdParameters (new participants and threshold). -/// Each current participant can maintain one vote. +/// Tracks votes for proposed threshold parameters (new participants, threshold, +/// and per-domain overlay). Each current participant can maintain one vote. // TODO(#2825): Replace with Votes from votes.rs // once this type is moved out of RunningContractState (which requires Clone + PartialEq + JSON). #[near(serializers=[borsh, json])] #[derive(Clone, Debug, Default, PartialEq, Eq)] pub struct ThresholdParametersVotes { - pub(crate) proposal_by_account: BTreeMap, + pub(crate) proposal_by_account: BTreeMap, } impl ThresholdParametersVotes { /// return the number of votes for `proposal` cast by members of `participants` - pub fn n_votes(&self, proposal: &ThresholdParameters, participants: &Participants) -> u64 { + pub fn n_votes( + &self, + proposal: &ProposedThresholdParameters, + participants: &Participants, + ) -> u64 { u64::try_from( self.proposal_by_account .iter() @@ -36,7 +40,7 @@ impl ThresholdParametersVotes { /// vote). pub fn vote( &mut self, - proposal: &ThresholdParameters, + proposal: &ProposedThresholdParameters, participant: AuthenticatedAccountId, ) -> u64 { if self @@ -62,9 +66,11 @@ mod tests { use crate::primitives::{ key_state::AuthenticatedAccountId, participants::Participants, - test_utils::{gen_participant, gen_threshold_params}, + test_utils::{gen_participant, gen_proposed_threshold_params}, }; + use near_mpc_contract_interface::types::{DomainId, ReconstructionThreshold}; use near_sdk::{test_utils::VMContextBuilder, testing_env}; + use std::collections::BTreeMap; #[test] fn test_voting_and_removal() { @@ -76,11 +82,11 @@ mod tests { testing_env!(ctx.build()); let participant = AuthenticatedAccountId::new(&participants).expect("expected authentication"); - let params = gen_threshold_params(30); + let params = gen_proposed_threshold_params(30); let mut votes = ThresholdParametersVotes::default(); assert_eq!(votes.vote(¶ms, participant.clone()), 1); assert_eq!(votes.n_votes(¶ms, &participants), 1); - let params2 = gen_threshold_params(30); + let params2 = gen_proposed_threshold_params(30); assert_eq!(votes.vote(¶ms2, participant), 1); assert_eq!(votes.n_votes(¶ms2, &participants), 1); assert_eq!(votes.n_votes(¶ms, &participants), 0); @@ -100,6 +106,45 @@ mod tests { assert_eq!(votes.n_votes(¶ms, &participants), 0); } + #[test] + #[expect(non_snake_case)] + fn vote__should_tally_distinct_per_domain_overlays_separately() { + // Given two voters and two proposals identical except for per_domain_thresholds + let mut participants = Participants::default(); + let (p0, p1) = (gen_participant(0), gen_participant(1)); + participants.insert(p0.0.clone(), p0.1).unwrap(); + participants.insert(p1.0.clone(), p1.1).unwrap(); + + let mut ctx = VMContextBuilder::new(); + let auth_p0 = { + ctx.signer_account_id(p0.0); + testing_env!(ctx.build()); + AuthenticatedAccountId::new(&participants).unwrap() + }; + let auth_p1 = { + ctx.signer_account_id(p1.0); + testing_env!(ctx.build()); + AuthenticatedAccountId::new(&participants).unwrap() + }; + + let base = gen_proposed_threshold_params(30); + let mut overlay_a = BTreeMap::new(); + overlay_a.insert(DomainId(0), ReconstructionThreshold::new(2)); + let mut overlay_b = BTreeMap::new(); + overlay_b.insert(DomainId(0), ReconstructionThreshold::new(3)); + let proposal_a = base.clone().with_per_domain_thresholds(overlay_a); + let proposal_b = base.with_per_domain_thresholds(overlay_b); + + // When each voter casts a different proposal + let mut votes = ThresholdParametersVotes::default(); + votes.vote(&proposal_a, auth_p0); + votes.vote(&proposal_b, auth_p1); + + // Then the two proposals are tallied independently + assert_eq!(votes.n_votes(&proposal_a, &participants), 1); + assert_eq!(votes.n_votes(&proposal_b, &participants), 1); + } + #[test] fn test_non_participant_votes_not_counted() { // given: two participants vote for a proposal @@ -120,7 +165,7 @@ mod tests { AuthenticatedAccountId::new(&old_participants).unwrap() }; - let params = gen_threshold_params(30); + let params = gen_proposed_threshold_params(30); let mut votes = ThresholdParametersVotes::default(); votes.vote(¶ms, auth_p0); votes.vote(¶ms, auth_p1); diff --git a/crates/contract/src/primitives/thresholds.rs b/crates/contract/src/primitives/thresholds.rs index 117781cea..97b2a3e23 100644 --- a/crates/contract/src/primitives/thresholds.rs +++ b/crates/contract/src/primitives/thresholds.rs @@ -1,6 +1,7 @@ use super::participants::{ParticipantId, ParticipantInfo, Participants}; use crate::errors::{Error, InvalidCandidateSet, InvalidThreshold}; use near_account_id::AccountId; +use near_mpc_contract_interface::types::{DomainId, ReconstructionThreshold}; use near_sdk::near; use std::collections::BTreeMap; @@ -9,9 +10,11 @@ pub use near_mpc_contract_interface::types::Threshold; /// Minimum absolute threshold required. const MIN_THRESHOLD_ABSOLUTE: u64 = 2; -/// Stores information about the threshold key parameters: -/// - owners of key shares -/// - cryptographic threshold +/// Stores the threshold key parameters: the owners of key shares +/// (`participants`) and the cryptographic `threshold`. This is the stored, +/// always-current shape. Per-domain reconstruction-threshold *proposals* are +/// carried separately by [`ProposedThresholdParameters`], so this type can +/// never hold a meaningless overlay. #[near(serializers=[borsh, json])] #[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Clone)] pub struct ThresholdParameters { @@ -165,6 +168,69 @@ impl ThresholdParameters { } } +/// A proposed set of threshold parameters submitted to `vote_new_parameters`: +/// the new [`ThresholdParameters`] plus an optional per-domain +/// `ReconstructionThreshold` overlay for the resharing it would trigger. +/// +/// The overlay is meaningful only while a proposal is in flight — it is applied +/// to the [`super::domain::DomainRegistry`] when resharing completes and the +/// stored [`ThresholdParameters`] is taken from [`Self::parameters`], so the +/// overlay is structurally dropped rather than scrubbed at runtime. +/// +/// An empty overlay means "keep current per-domain thresholds"; a populated map +/// must reference only existing domains (validated in +/// `RunningContractState::process_new_parameters_proposal`). +#[near(serializers=[borsh, json])] +#[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Clone)] +pub struct ProposedThresholdParameters { + parameters: ThresholdParameters, + #[serde(default)] + per_domain_thresholds: BTreeMap, +} + +impl ProposedThresholdParameters { + pub fn new( + parameters: ThresholdParameters, + per_domain_thresholds: BTreeMap, + ) -> Self { + ProposedThresholdParameters { + parameters, + per_domain_thresholds, + } + } + + /// Builder-style helper: replace the per-domain reconstruction-threshold + /// overlay. Convenient for constructing proposals (notably in tests). + #[cfg(test)] + pub fn with_per_domain_thresholds( + mut self, + per_domain_thresholds: BTreeMap, + ) -> Self { + self.per_domain_thresholds = per_domain_thresholds; + self + } + + /// The proposed stored parameters (participants + threshold). + pub fn parameters(&self) -> &ThresholdParameters { + &self.parameters + } + + /// The proposed per-domain reconstruction-threshold overlay. + pub fn per_domain_thresholds(&self) -> &BTreeMap { + &self.per_domain_thresholds + } + + /// Delegates to the proposed parameters' participants. + pub fn participants(&self) -> &Participants { + self.parameters.participants() + } + + /// Delegates to the proposed parameters' threshold. + pub fn threshold(&self) -> Threshold { + self.parameters.threshold() + } +} + #[cfg(test)] mod tests { use crate::{ @@ -240,7 +306,7 @@ mod tests { let params = gen_threshold_params(10); let proposal = gen_valid_params_proposal(¶ms); params - .validate_incoming_proposal(&proposal) + .validate_incoming_proposal(proposal.parameters()) .expect("Valid proposal should validate"); // Random proposals should not validate. @@ -395,10 +461,8 @@ mod tests { .insert_with_id(account_id, participant_info, ParticipantId(wrong_id)) .unwrap(); - let tampered_params = ThresholdParameters { - participants: tampered_participants, - threshold: params.threshold, - }; + let tampered_params = + ThresholdParameters::new_unvalidated(tampered_participants, params.threshold); assert_eq!( params diff --git a/crates/contract/src/snapshots/mpc_contract__tests__mpc_contract_borsh_schema_has_not_changed.snap b/crates/contract/src/snapshots/mpc_contract__tests__mpc_contract_borsh_schema_has_not_changed.snap index 91cd80f2a..de815783d 100644 --- a/crates/contract/src/snapshots/mpc_contract__tests__mpc_contract_borsh_schema_has_not_changed.snap +++ b/crates/contract/src/snapshots/mpc_contract__tests__mpc_contract_borsh_schema_has_not_changed.snap @@ -15,10 +15,10 @@ BorshSchemaContainer { "ParticipantInfo", ], }, - "(AuthenticatedAccountId, ThresholdParameters)": Tuple { + "(AuthenticatedAccountId, ProposedThresholdParameters)": Tuple { elements: [ "AuthenticatedAccountId", - "ThresholdParameters", + "ProposedThresholdParameters", ], }, "(AuthenticatedParticipantId, DockerImageHash)": Tuple { @@ -45,6 +45,12 @@ BorshSchemaContainer { "Vec", ], }, + "(DomainId, ReconstructionThreshold)": Tuple { + elements: [ + "DomainId", + "ReconstructionThreshold", + ], + }, "AccountId": Struct { fields: UnnamedFields( [ @@ -151,10 +157,10 @@ BorshSchemaContainer { ], ), }, - "BTreeMap": Sequence { + "BTreeMap": Sequence { length_width: 4, length_range: 0..=4294967295, - elements: "(AuthenticatedAccountId, ThresholdParameters)", + elements: "(AuthenticatedAccountId, ProposedThresholdParameters)", }, "BTreeMap": Sequence { length_width: 4, @@ -176,6 +182,11 @@ BorshSchemaContainer { length_range: 0..=4294967295, elements: "(AuthenticatedParticipantId, Vec)", }, + "BTreeMap": Sequence { + length_width: 4, + length_range: 0..=4294967295, + elements: "(DomainId, ReconstructionThreshold)", + }, "BTreeSet": Sequence { length_width: 4, length_range: 0..=4294967295, @@ -826,6 +837,20 @@ BorshSchemaContainer { ], ), }, + "ProposedThresholdParameters": Struct { + fields: NamedFields( + [ + ( + "parameters", + "ThresholdParameters", + ), + ( + "per_domain_thresholds", + "BTreeMap", + ), + ], + ), + }, "ProposedUpdates": Struct { fields: NamedFields( [ @@ -1030,6 +1055,10 @@ BorshSchemaContainer { "cancellation_requests", "HashSet", ), + ( + "per_domain_thresholds", + "BTreeMap", + ), ], ), }, @@ -1173,7 +1202,7 @@ BorshSchemaContainer { [ ( "proposal_by_account", - "BTreeMap", + "BTreeMap", ), ], ), diff --git a/crates/contract/src/state.rs b/crates/contract/src/state.rs index e4cb3810a..4fb03837f 100644 --- a/crates/contract/src/state.rs +++ b/crates/contract/src/state.rs @@ -11,7 +11,7 @@ use crate::primitives::{ domain::DomainRegistry, key_state::{AuthenticatedParticipantId, EpochId, KeyEventId}, participants::Participants, - thresholds::{Threshold, ThresholdParameters}, + thresholds::{ProposedThresholdParameters, Threshold, ThresholdParameters}, }; use initializing::InitializingContractState; use near_account_id::AccountId; @@ -126,7 +126,7 @@ impl ProtocolContractState { pub fn vote_new_parameters( &mut self, prospective_epoch_id: EpochId, - proposed_parameters: &ThresholdParameters, + proposed_parameters: &ProposedThresholdParameters, ) -> Result, Error> { match self { ProtocolContractState::Running(state) => { diff --git a/crates/contract/src/state/resharing.rs b/crates/contract/src/state/resharing.rs index 0bb1fcc70..23f552f69 100644 --- a/crates/contract/src/state/resharing.rs +++ b/crates/contract/src/state/resharing.rs @@ -1,4 +1,4 @@ -use std::collections::HashSet; +use std::collections::{BTreeMap, HashSet}; use super::key_event::KeyEvent; use super::running::RunningContractState; @@ -6,8 +6,9 @@ use crate::errors::{Error, InvalidParameters}; use crate::primitives::key_state::{ AuthenticatedAccountId, EpochId, KeyEventId, KeyForDomain, Keyset, }; -use crate::primitives::thresholds::ThresholdParameters; +use crate::primitives::thresholds::ProposedThresholdParameters; use near_account_id::AccountId; +use near_mpc_contract_interface::types::{DomainId, ReconstructionThreshold}; use near_sdk::near; /// In this state, we reshare the key of every domain onto a new set of participants and threshold. @@ -31,6 +32,10 @@ pub struct ResharingContractState { pub reshared_keys: Vec, pub resharing_key: KeyEvent, pub cancellation_requests: HashSet, + /// Per-domain `ReconstructionThreshold` overlay carried from the accepted + /// proposal. Applied to the [`DomainRegistry`](crate::primitives::domain::DomainRegistry) + /// when resharing completes; empty means "keep current per-domain thresholds". + pub per_domain_thresholds: BTreeMap, } impl ResharingContractState { @@ -51,7 +56,7 @@ impl ResharingContractState { pub fn vote_new_parameters( &mut self, prospective_epoch_id: EpochId, - proposal: &ThresholdParameters, + proposal: &ProposedThresholdParameters, ) -> Result, Error> { let expected_prospective_epoch_id = self.prospective_epoch_id().next(); if prospective_epoch_id != expected_prospective_epoch_id { @@ -80,9 +85,10 @@ impl ResharingContractState { .get_domain_by_index(0) .unwrap() .clone(), - proposal.clone(), + proposal.parameters().clone(), ), cancellation_requests: HashSet::new(), + per_domain_thresholds: proposal.per_domain_thresholds().clone(), })); } Ok(None) @@ -138,8 +144,16 @@ impl ResharingContractState { self.resharing_key.proposed_parameters().clone(), ); } else { + // Resharing complete: fold the per-domain overlay into the + // registry and store the proposed parameters. The overlay lives + // only on this resharing state, so it is structurally dropped + // here rather than scrubbed off the stored parameters. + let new_domains = self + .previous_running_state + .domains + .with_overlaid_thresholds(&self.per_domain_thresholds)?; return Ok(Some(RunningContractState::new( - self.previous_running_state.domains.clone(), + new_domains, Keyset::new(self.prospective_epoch_id(), self.reshared_keys.clone()), self.resharing_key.proposed_parameters().clone(), self.previous_running_state.add_domains_votes.clone(), @@ -194,21 +208,24 @@ impl ResharingContractState { #[cfg(test)] pub mod tests { use crate::primitives::test_utils::NUM_PROTOCOLS; - use crate::state::{key_event::tests::find_leader, running::RunningContractState}; + use crate::state::{ + key_event::tests::{Environment, find_leader}, + running::RunningContractState, + }; use crate::{ primitives::{ domain::AddDomainsVotes, key_state::{AttemptId, KeyEventId}, test_utils::gen_account_id, threshold_votes::ThresholdParametersVotes, - thresholds::{Threshold, ThresholdParameters}, + thresholds::{ProposedThresholdParameters, Threshold, ThresholdParameters}, }, - state::test_utils::gen_resharing_state, + state::test_utils::{gen_resharing_state, gen_running_state}, }; use near_account_id::AccountId; - use near_mpc_contract_interface::types::DomainId; + use near_mpc_contract_interface::types::{DomainId, ReconstructionThreshold}; use rstest::rstest; - use std::collections::BTreeSet; + use std::collections::{BTreeMap, BTreeSet}; fn test_resharing_contract_state_for(num_domains: usize) { println!("Testing with {} domains", num_domains); @@ -405,6 +422,9 @@ pub mod tests { .subset(new_participants_1.len() - old_participants.len()..new_participants_1.len()); let new_params_1 = ThresholdParameters::new(new_participants_1, new_threshold).unwrap(); let new_params_2 = ThresholdParameters::new(new_participants_2, new_threshold).unwrap(); + // Proposals carry an empty (no-change) per-domain overlay. + let proposed_1 = ProposedThresholdParameters::new(new_params_1.clone(), BTreeMap::new()); + let proposed_2 = ProposedThresholdParameters::new(new_params_2.clone(), BTreeMap::new()); state .previous_running_state .parameters @@ -423,10 +443,10 @@ pub mod tests { { env.set_signer(&old_participants.participants()[0].0); let _ = state - .vote_new_parameters(state.prospective_epoch_id(), &new_params_1) + .vote_new_parameters(state.prospective_epoch_id(), &proposed_1) .unwrap_err(); let _ = state - .vote_new_parameters(state.prospective_epoch_id().next().next(), &new_params_1) + .vote_new_parameters(state.prospective_epoch_id().next().next(), &proposed_1) .unwrap_err(); } @@ -436,7 +456,7 @@ pub mod tests { env.set_signer(account); assert!(new_state.is_none()); new_state = state - .vote_new_parameters(state.prospective_epoch_id().next(), &new_params_1) + .vote_new_parameters(state.prospective_epoch_id().next(), &proposed_1) .unwrap(); } // We should've gotten a new resharing state. @@ -462,7 +482,90 @@ pub mod tests { // Repropose with new_params_2. That should fail. env.set_signer(&old_participants.participants()[0].0); let _ = new_state - .vote_new_parameters(new_state.prospective_epoch_id().next(), &new_params_2) + .vote_new_parameters(new_state.prospective_epoch_id().next(), &proposed_2) .unwrap_err(); } + + /// On successful resharing transition, the proposal's + /// `per_domain_thresholds` overlay must be applied to the new + /// `DomainRegistry`. The overlay lives only on the proposal / + /// resharing state, so the stored `RunningContractState.parameters` + /// (a plain `ThresholdParameters`) cannot carry it at all. + /// + /// The fixture is deterministic on purpose: it keeps the participant + /// set unchanged (a key-refresh resharing) so the proposed participant + /// count equals the running count (`n >= 3`, guaranteed by + /// `gen_running_state`). That guarantees `n` is itself a valid + /// reconstruction threshold and always differs from the default `2` — + /// avoiding the flakiness of deriving the new value from the random + /// cluster threshold, which lands on `2` whenever the proposal has 2 or + /// 3 participants. + #[expect(non_snake_case)] + #[test] + fn vote_reshared__final_transition__should_apply_overlay_to_registry() { + // Given a running state with a single CaitSith domain at the default + // reconstruction threshold (2), and a resharing proposal over the + // same participant set carrying an overlay that moves that domain to + // `n` — a value valid for `n` participants and distinct from 2. + let mut env = Environment::new(Some(100), None, None); + let mut running = gen_running_state(1); + let current_params = running.parameters.clone(); + let n = current_params.participants().len() as u64; + assert!( + n >= 3, + "gen_running_state guarantees at least 3 participants" + ); + let domain_id = running.domains.domains()[0].id; + let original_threshold = running.domains.domains()[0].reconstruction_threshold; + let new_threshold = ReconstructionThreshold::new(n); + assert_ne!(new_threshold, original_threshold); + let mut overlay = BTreeMap::new(); + overlay.insert(domain_id, new_threshold); + let proposal = ProposedThresholdParameters::new(current_params.clone(), overlay); + + // Drive the proposal to acceptance so we transition into Resharing + // through the real vote path (which also exercises the fail-fast + // overlay validation in `process_new_parameters_proposal`). + let prospective_epoch_id = running.prospective_epoch_id(); + let mut state = None; + for (account, _, _) in proposal.participants().participants() { + env.set_signer(account); + state = running + .vote_new_parameters(prospective_epoch_id, &proposal) + .unwrap(); + } + let mut state = state.expect("Should've transitioned into resharing"); + + // When all candidates vote-reshared for the (single) domain + let leader = find_leader(&state.resharing_key); + env.set_signer(&leader.0); + let key_event_id = KeyEventId { + attempt_id: AttemptId::new(), + domain_id, + epoch_id: state.prospective_epoch_id(), + }; + state.start(key_event_id, 0).unwrap(); + let mut new_running = None; + let candidates: Vec<_> = state + .resharing_key + .proposed_parameters() + .participants() + .participants() + .iter() + .map(|(acc, _, _)| acc.clone()) + .collect(); + for account in &candidates { + env.set_signer(account); + new_running = state.vote_reshared(key_event_id).unwrap(); + } + + // Then the new running state's registry carries the overlay's + // threshold. (The stored parameters are a plain `ThresholdParameters` + // and structurally cannot carry an overlay.) + let new_running = new_running.expect("resharing should have transitioned to Running"); + assert_eq!( + new_running.domains.domains()[0].reconstruction_threshold, + new_threshold, + ); + } } diff --git a/crates/contract/src/state/running.rs b/crates/contract/src/state/running.rs index 7f94659ed..e135c2d15 100644 --- a/crates/contract/src/state/running.rs +++ b/crates/contract/src/state/running.rs @@ -1,15 +1,18 @@ use super::initializing::InitializingContractState; use super::key_event::KeyEvent; use super::resharing::ResharingContractState; -use crate::errors::{DomainError, Error, InvalidParameters, VoteError}; +use crate::errors::{ConversionError, DomainError, Error, InvalidParameters, VoteError}; use crate::primitives::{ - domain::{AddDomainsVotes, DomainRegistry}, + domain::{ + AddDomainsVotes, DomainRegistry, validate_caitsith_uniform_threshold, + validate_domain_threshold, + }, key_state::{AuthenticatedAccountId, AuthenticatedParticipantId, EpochId, Keyset}, threshold_votes::ThresholdParametersVotes, - thresholds::ThresholdParameters, + thresholds::{ProposedThresholdParameters, ThresholdParameters}, }; use near_account_id::AccountId; -use near_mpc_contract_interface::types::{DomainConfig, Protocol}; +use near_mpc_contract_interface::types::DomainConfig; use near_sdk::near; use std::collections::{BTreeSet, HashSet}; @@ -62,7 +65,7 @@ impl RunningContractState { pub fn transition_to_resharing_no_checks( &mut self, - proposal: &ThresholdParameters, + proposal: &ProposedThresholdParameters, ) -> Option { if let Some(first_domain) = self.domains.get_domain_by_index(0) { let epoch_id = self.prospective_epoch_id(); @@ -75,16 +78,23 @@ impl RunningContractState { self.add_domains_votes.clone(), ), reshared_keys: Vec::new(), - resharing_key: KeyEvent::new(epoch_id, first_domain.clone(), proposal.clone()), + resharing_key: KeyEvent::new( + epoch_id, + first_domain.clone(), + proposal.parameters().clone(), + ), cancellation_requests: HashSet::new(), + per_domain_thresholds: proposal.per_domain_thresholds().clone(), }) } else { - // A new ThresholdParameters was proposed, but we have no keys, so directly - // transition into Running state but bump the EpochId. + // New parameters were proposed, but we have no keys, so directly + // transition into Running state but bump the EpochId. With no + // domains the per-domain overlay has nothing to apply to and is + // dropped. *self = RunningContractState::new( self.domains.clone(), Keyset::new(self.keyset.epoch_id.next(), Vec::new()), - proposal.clone(), + proposal.parameters().clone(), self.add_domains_votes.clone(), ); None @@ -96,7 +106,7 @@ impl RunningContractState { pub fn vote_new_parameters( &mut self, prospective_epoch_id: EpochId, - proposal: &ThresholdParameters, + proposal: &ProposedThresholdParameters, ) -> Result, Error> { let expected_prospective_epoch_id = self.prospective_epoch_id(); @@ -132,24 +142,57 @@ impl RunningContractState { /// Returns true if all participants of the proposed parameters voted for it. pub(super) fn process_new_parameters_proposal( &mut self, - proposal: &ThresholdParameters, + proposal: &ProposedThresholdParameters, ) -> Result { // ensure the proposal is valid against the current parameters - self.parameters.validate_incoming_proposal(proposal)?; - - // TODO(#3169): once resharing votes carry per-domain `t` (and the node - // honors per-domain reconstruction thresholds, #3164), this path must - // (1) reject proposals that would leave CaitSith domains with differing - // reconstruction_thresholds — the 3.11-transition lock that - // `vote_add_domains` already applies — and (2) re-enable the per-domain - // threshold check below. Both hold trivially today: the proposal - // carries only `ThresholdParameters` (per-domain `t` untouched) and the - // node signs every domain at the cluster threshold, so enabling (2) now - // would wrongly block legitimate resharings. - // let new_num_participants = proposal.participants().len() as u64; - // for domain in self.domains.domains() { - // crate::primitives::domain::validate_domain_threshold(domain, new_num_participants)?; - // } + self.parameters + .validate_incoming_proposal(proposal.parameters())?; + + // Validate effective per-domain thresholds against the proposed new + // participant count. Domains not present in the overlay keep their + // existing threshold; overlay entries override. An overlay entry + // referencing an unknown domain ID is rejected. + let new_num_participants = u64::try_from(proposal.participants().len()).map_err(|e| { + ConversionError::DataConversion { + reason: format!("participant count does not fit in u64: {e}"), + } + })?; + let overlay = proposal.per_domain_thresholds(); + // `with_overlaid_thresholds` re-runs this same guard at the final + // resharing transition, so this is intentionally redundant. We check it + // here too to fail fast at vote acceptance: the threshold-validation + // loop below iterates the existing domains (not the overlay keys), so + // without this guard an entry for an unknown domain ID would be + // silently ignored now and only rejected at transition time. + for id in overlay.keys() { + if self.domains.get_domain_by_domain_id(*id).is_none() { + return Err(DomainError::UnknownDomainInProposal { domain_id: *id }.into()); + } + } + let effective_domains: Vec = self + .domains + .domains() + .iter() + .map(|domain| { + let effective_threshold = overlay + .get(&domain.id) + .copied() + .unwrap_or(domain.reconstruction_threshold); + DomainConfig { + reconstruction_threshold: effective_threshold, + ..domain.clone() + } + }) + .collect(); + for domain in &effective_domains { + validate_domain_threshold(domain, new_num_participants)?; + } + // 3.11-transition lock: the overlay can rewrite per-domain thresholds, + // so it could leave CaitSith domains with differing thresholds — which + // `vote_add_domains` forbids and the legacy `DBCol::Triple` mirror + // (#3292) requires. Fail fast here; `with_overlaid_thresholds` re-runs + // this same check at the final resharing transition. + validate_caitsith_uniform_threshold(&effective_domains)?; // ensure the signer is a proposed participant let candidate = AuthenticatedAccountId::new(proposal.participants())?; @@ -167,7 +210,12 @@ impl RunningContractState { // finally, vote. let n_votes = self.parameters_votes.vote(proposal, candidate); - Ok(proposal.participants().len() as u64 == n_votes) + let num_participants = u64::try_from(proposal.participants().len()).map_err(|e| { + ConversionError::DataConversion { + reason: format!("participant count does not fit in u64: {e}"), + } + })?; + Ok(num_participants == n_votes) } /// Casts a vote for the signer participant to add new domains, replacing any previous vote. @@ -186,32 +234,20 @@ impl RunningContractState { crate::primitives::domain::validate_domain_purpose(domain)?; crate::primitives::domain::validate_domain_threshold(domain, num_participants)?; } - // TODO(#3164): remove this single-threshold lock once each domain can - // carry its own `t`. All CaitSith domains (ForeignTx included) must - // share one `reconstruction_threshold` today, because the node only - // runs background triple generation for a single `t`. If no CaitSith - // domain exists yet the first one is free to pick any valid `t`; any - // later CaitSith — in the same proposal or in future calls — must - // match it. - let mut expected_caitsith_t = self + // 3.11-transition lock: all CaitSith domains (ForeignTx included) must + // share one `reconstruction_threshold` so the legacy unprefixed + // `DBCol::Triple` mirror (#3292) can't collide. If no CaitSith domain + // exists yet the first one is free to pick any valid `t`; any later + // CaitSith — already present or in this proposal — must match it. + // Tracked for removal in #3306. + let existing_and_new: Vec = self .domains .domains() .iter() - .find(|d| d.protocol == Protocol::CaitSith) - .map(|d| d.reconstruction_threshold.inner()); - for domain in &domains { - if domain.protocol != Protocol::CaitSith { - continue; - } - let found = domain.reconstruction_threshold.inner(); - match expected_caitsith_t { - None => expected_caitsith_t = Some(found), - Some(expected) if expected != found => { - return Err(DomainError::CaitsithThresholdMismatch { expected, found }.into()); - } - Some(_) => {} - } - } + .cloned() + .chain(domains.iter().cloned()) + .collect(); + validate_caitsith_uniform_threshold(&existing_and_new)?; let participant = AuthenticatedParticipantId::new(self.parameters.participants())?; let n_votes = self.add_domains_votes.vote(domains.clone(), &participant); if self.parameters.participants().len() as u64 == n_votes { @@ -246,7 +282,7 @@ pub mod running_tests { use super::RunningContractState; use crate::primitives::domain::AddDomainsVotes; - use crate::primitives::test_utils::{NUM_PROTOCOLS, gen_threshold_params}; + use crate::primitives::test_utils::{NUM_PROTOCOLS, gen_proposed_threshold_params}; use crate::primitives::threshold_votes::ThresholdParametersVotes; use crate::state::key_event::tests::Environment; use crate::state::test_utils::{gen_running_state, gen_valid_params_proposal}; @@ -265,7 +301,7 @@ pub mod running_tests { let participants = state.parameters.participants().clone(); // Assert that random proposals get rejected. for (account_id, _, _) in participants.participants() { - let ksp = gen_threshold_params(30); + let ksp = gen_proposed_threshold_params(30); env.set_signer(account_id); let _ = state .vote_new_parameters(state.keyset.epoch_id.next(), &ksp) @@ -369,7 +405,14 @@ pub mod running_tests { resharing.prospective_epoch_id(), state.keyset.epoch_id.next(), ); - assert_eq!(resharing.resharing_key.proposed_parameters(), &proposal); + assert_eq!( + resharing.resharing_key.proposed_parameters(), + proposal.parameters() + ); + assert_eq!( + resharing.per_domain_thresholds, + *proposal.per_domain_thresholds() + ); } } @@ -515,6 +558,65 @@ pub mod running_tests { ); } + use std::collections::BTreeMap; + + #[test] + fn process_new_parameters_proposal__should_accept_empty_per_domain_overlay() { + // Given a running state where existing thresholds are valid under the + // proposed participant count + let mut state = gen_running_state(1); + let mut env = Environment::new(None, None, None); + let proposal = gen_valid_params_proposal(&state.parameters); + // Sign as a participant present in BOTH the current and proposed sets: + // `gen_valid_params_proposal` keeps only a random subset of the current + // participants, so an arbitrary current participant may be absent from + // the proposal (rejected as a non-participant) and a freshly added one + // would be deferred as a pending newcomer. The retained overlap is + // non-empty (at least `threshold` current participants are kept). + let signer = proposal + .participants() + .participants() + .iter() + .map(|(account_id, _, _)| account_id.clone()) + .find(|account_id| { + state + .parameters + .participants() + .is_participant_given_account_id(account_id) + }) + .expect("proposal must retain at least one current participant"); + env.set_signer(&signer); + + // When voting with an empty per_domain_thresholds map (legacy shape) + let res = state.vote_new_parameters(state.keyset.epoch_id.next(), &proposal); + + // Then the vote is recorded without error + assert!(res.is_ok(), "Expected accept with empty overlay: {res:?}"); + } + + #[test] + fn process_new_parameters_proposal__should_reject_overlay_with_unknown_domain_id() { + // Given a running state with one domain + let mut state = gen_running_state(1); + let mut env = Environment::new(None, None, None); + env.set_signer(&state.parameters.participants().participants()[0].0); + let proposal = gen_valid_params_proposal(&state.parameters); + + // When voting with an overlay referencing a non-existent domain ID + let mut overlay = BTreeMap::new(); + overlay.insert(DomainId(9999), ReconstructionThreshold::new(2)); + let proposal = proposal.with_per_domain_thresholds(overlay); + let err = state + .vote_new_parameters(state.keyset.epoch_id.next(), &proposal) + .unwrap_err(); + + // Then the unknown-domain guard rejects it + assert!( + err.to_string().contains("not in the current registry"), + "Expected UnknownDomainInProposal, got: {err}" + ); + } + /// Builds a `DomainConfig` for the next domain id with the given protocol, /// purpose, and reconstruction threshold. fn caitsith_lock_test_proposal( @@ -602,6 +704,33 @@ pub mod running_tests { ); } + #[test] + fn process_new_parameters_proposal__should_apply_overlay_to_threshold_validation() { + // Given a running state with one domain whose existing threshold would + // remain valid under the new participants, but the overlay swaps it + // for an invalid (too-low) value. + let mut state = gen_running_state(1); + let mut env = Environment::new(None, None, None); + env.set_signer(&state.parameters.participants().participants()[0].0); + let proposal = gen_valid_params_proposal(&state.parameters); + + // When voting with an overlay that violates the universal lower bound + let domain_id = state.domains.domains()[0].id; + let mut overlay = BTreeMap::new(); + overlay.insert(domain_id, ReconstructionThreshold::new(1)); + let proposal = proposal.with_per_domain_thresholds(overlay); + let err = state + .vote_new_parameters(state.keyset.epoch_id.next(), &proposal) + .unwrap_err(); + + // Then the overlay's value (not the stored value) is validated and rejected + assert!( + err.to_string() + .contains("Reconstruction threshold must be at least"), + "Expected ReconstructionThresholdTooLow on overlay value, got: {err}" + ); + } + #[test] fn vote_add_domains__should_accept_caitsith_threshold_matching_existing() { // Given a Running state with a CaitSith domain at t = 2 (fixture diff --git a/crates/contract/src/state/test_utils.rs b/crates/contract/src/state/test_utils.rs index 7a37c3067..5f2b558a8 100644 --- a/crates/contract/src/state/test_utils.rs +++ b/crates/contract/src/state/test_utils.rs @@ -11,11 +11,12 @@ use crate::primitives::{ key_state::{EpochId, KeyForDomain, Keyset}, participants::{ParticipantId, Participants}, test_utils::{gen_participant, gen_threshold_params}, - thresholds::{Threshold, ThresholdParameters}, + thresholds::{ProposedThresholdParameters, Threshold, ThresholdParameters}, }; use rand::Rng; +use std::collections::BTreeMap; -pub fn gen_valid_params_proposal(params: &ThresholdParameters) -> ThresholdParameters { +pub fn gen_valid_params_proposal(params: &ThresholdParameters) -> ProposedThresholdParameters { let mut rng = rand::thread_rng(); let current_k = params.threshold().value() as usize; let current_n = params.participants().len(); @@ -56,7 +57,10 @@ pub fn gen_valid_params_proposal(params: &ThresholdParameters) -> ThresholdParam } let threshold = ((new_participants.len() as f64) * 0.6).ceil() as u64; - ThresholdParameters::new(new_participants, Threshold::new(threshold)).unwrap() + let parameters = ThresholdParameters::new(new_participants, Threshold::new(threshold)).unwrap(); + // Valid proposals carry an empty (no-change) per-domain overlay by default; + // tests that exercise overlays attach one via `with_per_domain_thresholds`. + ProposedThresholdParameters::new(parameters, BTreeMap::new()) } /// Generates a resharing state with the given number of domains. diff --git a/crates/contract/src/v3_11_2_state.rs b/crates/contract/src/v3_11_2_state.rs index 088f07a2d..790b70e64 100644 --- a/crates/contract/src/v3_11_2_state.rs +++ b/crates/contract/src/v3_11_2_state.rs @@ -7,6 +7,8 @@ //! However, this approach (a) requires manual effort from a developer and (b) increases the binary size. //! A better approach: only copy the structures that have changed and import the rest from the existing codebase. +use std::collections::BTreeMap; + use borsh::{BorshDeserialize, BorshSerialize}; use near_mpc_contract_interface::types::{Metrics, VerifyForeignTransactionRequest}; use near_sdk::{env, store::LookupMap}; @@ -17,19 +19,99 @@ use crate::{ node_migrations::NodeMigrations, primitives::{ ckd::CKDRequest, + domain::{AddDomainsVotes, DomainRegistry}, + key_state::{AuthenticatedAccountId, EpochId, Keyset}, signature::{SignatureRequest, YieldIndex}, + threshold_votes::ThresholdParametersVotes, + thresholds::{ProposedThresholdParameters, ThresholdParameters}, + }, + state::{ + ProtocolContractState, initializing::InitializingContractState, + resharing::ResharingContractState, running::RunningContractState, }, - state::ProtocolContractState, tee::tee_state::TeeState, update::ProposedUpdates, }; +/// `3.11.2` layout of `ThresholdParametersVotes`. The stored +/// `ThresholdParameters` (`{ participants, threshold }`) is byte-identical +/// between 3.11.2 and the current layout, so no shadow is needed for it — the +/// real type decodes old bytes directly. Only the vote *value* type changed: +/// votes now carry [`ProposedThresholdParameters`], which appends a +/// `per_domain_thresholds` overlay. Borsh is positional, so old vote bytes are +/// decoded into the real `ThresholdParameters` and mapped to a proposal with an +/// empty (no-change) overlay. +#[derive(Debug, BorshSerialize, BorshDeserialize)] +struct OldThresholdParametersVotes { + proposal_by_account: BTreeMap, +} + +impl From for ThresholdParametersVotes { + fn from(old: OldThresholdParametersVotes) -> Self { + ThresholdParametersVotes { + proposal_by_account: old + .proposal_by_account + .into_iter() + // Pre-migration votes didn't carry per-domain thresholds, so the + // migrated proposal gets an empty (no-change) overlay. + .map(|(acc, params)| { + ( + acc, + ProposedThresholdParameters::new(params, BTreeMap::new()), + ) + }) + .collect(), + } + } +} + +/// `3.11.2` layout of `RunningContractState`. The stored `parameters` use the +/// real `ThresholdParameters` (byte-identical to 3.11.2); only `parameters_votes` +/// needs the [`OldThresholdParametersVotes`] shadow. +#[derive(Debug, BorshSerialize, BorshDeserialize)] +struct OldRunningContractState { + domains: DomainRegistry, + keyset: Keyset, + parameters: ThresholdParameters, + parameters_votes: OldThresholdParametersVotes, + add_domains_votes: AddDomainsVotes, + previously_cancelled_resharing_epoch_id: Option, +} + +impl From for RunningContractState { + fn from(old: OldRunningContractState) -> Self { + RunningContractState { + domains: old.domains, + keyset: old.keyset, + parameters: old.parameters, + parameters_votes: old.parameters_votes.into(), + add_domains_votes: old.add_domains_votes, + previously_cancelled_resharing_epoch_id: old.previously_cancelled_resharing_epoch_id, + } + } +} + +/// `3.11.2` layout of `ProtocolContractState`. Only the `Running` variant +/// has a verified shadow — Initializing/Resharing reuse current types and +/// would fail to deserialize old data, which matches the pre-existing +/// "migration panics if not Running" policy. +#[derive(Debug, BorshSerialize, BorshDeserialize)] +enum OldProtocolContractState { + NotInitialized, + Initializing(InitializingContractState), + Running(OldRunningContractState), + Resharing(ResharingContractState), +} + /// Keep this module in lock-step with [`crate::MpcContract`]: the moment a field's borsh /// layout diverges, shadow the old type here (see this module's history for examples) so /// state written by the `3.11.2` contract still deserializes during migration. +/// +/// `protocol_state` carries the per-domain-threshold layout shift (#3169) and is shadowed +/// by `OldProtocolContractState`; every other field is byte-identical to `3.11.2`. #[derive(Debug, BorshSerialize, BorshDeserialize)] pub struct MpcContract { - protocol_state: ProtocolContractState, + protocol_state: OldProtocolContractState, pending_signature_requests: LookupMap>, pending_ckd_requests: LookupMap>, pending_verify_foreign_tx_requests: LookupMap>, @@ -45,12 +127,12 @@ pub struct MpcContract { impl From for crate::MpcContract { fn from(old: MpcContract) -> Self { - if !matches!(old.protocol_state, ProtocolContractState::Running(_)) { + let OldProtocolContractState::Running(running) = old.protocol_state else { env::panic_str("Contract must be in running state when migrating."); - } + }; crate::MpcContract { - protocol_state: old.protocol_state, + protocol_state: ProtocolContractState::Running(running.into()), pending_signature_requests: old.pending_signature_requests, pending_ckd_requests: old.pending_ckd_requests, pending_verify_foreign_tx_requests: old.pending_verify_foreign_tx_requests, @@ -65,3 +147,55 @@ impl From for crate::MpcContract { } } } + +#[cfg(test)] +#[expect(non_snake_case)] +mod tests { + use super::*; + use crate::primitives::test_utils::{NUM_PROTOCOLS, gen_participants}; + use crate::primitives::thresholds::Threshold; + use near_sdk::{test_utils::VMContextBuilder, testing_env}; + + /// Borsh round-trip *through the shadow type*: write a `ThresholdParametersVotes` + /// in the OLD 3.11.2 layout (vote values are bare `ThresholdParameters`, with no + /// `per_domain_thresholds`), decode it via [`OldThresholdParametersVotes`], run the + /// real [`From`] migration, and assert each migrated vote becomes a + /// `ProposedThresholdParameters` carrying an empty (no-change) overlay. + /// + /// This exercises both the shadow's `BorshDeserialize` and the conversion impl, so + /// it fails if either the old layout or the overlay-defaulting logic regresses. + #[test] + fn old_threshold_parameter_votes__should_migrate_into_empty_overlay() { + // Given a participant set with one member installed as the signer, so we can + // mint an `AuthenticatedAccountId` to key the vote by. + let participants = gen_participants(NUM_PROTOCOLS); + let n = participants.len() as u64; + let voter_account = participants.participants()[0].0.clone(); + + let mut ctx = VMContextBuilder::new(); + ctx.signer_account_id(voter_account); + testing_env!(ctx.build()); + let voter = AuthenticatedAccountId::new(&participants).unwrap(); + + // and old-layout vote bytes: a single vote whose value is a bare + // `ThresholdParameters` (the 3.11.2 vote shape). + let params = ThresholdParameters::new(participants, Threshold::new(n)).unwrap(); + let old = OldThresholdParametersVotes { + proposal_by_account: BTreeMap::from([(voter.clone(), params)]), + }; + let bytes = borsh::to_vec(&old).unwrap(); + + // When decoding through the shadow type and running the real migration. + let decoded: OldThresholdParametersVotes = borsh::from_slice(&bytes).unwrap(); + let migrated: ThresholdParametersVotes = decoded.into(); + + // Then the single migrated vote retains the original threshold and gains an + // empty per-domain overlay. + let proposal = migrated + .proposal_by_account + .get(&voter) + .expect("migrated vote should be keyed by the original voter"); + assert!(proposal.per_domain_thresholds().is_empty()); + assert_eq!(proposal.threshold().value(), n); + } +} diff --git a/crates/contract/tests/inprocess/attestation_submission.rs b/crates/contract/tests/inprocess/attestation_submission.rs index cb02d3e81..937611a95 100644 --- a/crates/contract/tests/inprocess/attestation_submission.rs +++ b/crates/contract/tests/inprocess/attestation_submission.rs @@ -8,7 +8,7 @@ use mpc_contract::{ key_state::{AttemptId, EpochId, KeyForDomain, Keyset}, participants::{ParticipantId, ParticipantInfo}, test_utils::{bogus_ed25519_public_key, gen_participants}, - thresholds::{Threshold, ThresholdParameters}, + thresholds::{ProposedThresholdParameters, Threshold, ThresholdParameters}, }, tee::tee_state::NodeId, }; @@ -17,6 +17,7 @@ use near_mpc_contract_interface::types::{ ReconstructionThreshold, }; use near_mpc_contract_interface::types::{DomainConfig, DomainId, DomainPurpose}; +use std::collections::BTreeMap; use assert_matches::assert_matches; use near_account_id::AccountId; @@ -186,9 +187,11 @@ impl TestSetupBuilder { let context = create_context_for_participant(&node_id.account_id); testing_env!(context); + let proposal = + ProposedThresholdParameters::new(parameters.clone(), BTreeMap::new()); setup .contract - .vote_new_parameters(EpochId::new(6), parameters.clone().into()) + .vote_new_parameters(EpochId::new(6), proposal.into()) .unwrap(); } 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 5955805da..199b77052 100644 --- a/crates/contract/tests/snapshots/abi__abi_has_not_changed.snap +++ b/crates/contract/tests/snapshots/abi__abi_has_not_changed.snap @@ -1716,7 +1716,7 @@ expression: abi { "name": "proposal", "type_schema": { - "$ref": "#/definitions/ThresholdParameters" + "$ref": "#/definitions/ProposedThresholdParameters" } } ] @@ -3862,6 +3862,31 @@ expression: abi } } }, + "ProposedThresholdParameters": { + "description": "A proposed set of threshold parameters submitted to `vote_new_parameters`. Carries the proposed [`ThresholdParameters`] (participant set and threshold) plus an optional per-domain `ReconstructionThreshold` overlay for the resharing it would trigger.", + "type": "object", + "required": [ + "participants", + "threshold" + ], + "properties": { + "participants": { + "$ref": "#/definitions/Participants" + }, + "per_domain_thresholds": { + "default": {}, + "type": "object", + "additionalProperties": { + "type": "integer", + "format": "uint64", + "minimum": 0.0 + } + }, + "threshold": { + "$ref": "#/definitions/Threshold" + } + } + }, "ProposedUpdates": { "type": "object", "required": [ @@ -4160,6 +4185,16 @@ expression: abi }, "uniqueItems": true }, + "per_domain_thresholds": { + "description": "Per-domain `ReconstructionThreshold` overlay carried from the accepted proposal. Applied to the `DomainRegistry` when resharing completes.", + "default": {}, + "type": "object", + "additionalProperties": { + "type": "integer", + "format": "uint64", + "minimum": 0.0 + } + }, "previous_running_state": { "$ref": "#/definitions/RunningContractState" }, @@ -4579,7 +4614,7 @@ expression: abi "minimum": 0.0 }, "ThresholdParameters": { - "description": "Threshold parameters for distributed key operations.", + "description": "Threshold parameters for distributed key operations: the current participant set and the governance threshold. This is the stored, always-current shape; per-domain reconstruction-threshold *proposals* live on [`ProposedThresholdParameters`].", "type": "object", "required": [ "participants", @@ -4604,7 +4639,7 @@ expression: abi "proposal_by_account": { "type": "object", "additionalProperties": { - "$ref": "#/definitions/ThresholdParameters" + "$ref": "#/definitions/ProposedThresholdParameters" } } } diff --git a/crates/devnet/src/cli.rs b/crates/devnet/src/cli.rs index 3dddd2a5f..5d49d5388 100644 --- a/crates/devnet/src/cli.rs +++ b/crates/devnet/src/cli.rs @@ -298,6 +298,21 @@ pub struct MpcVoteNewParametersCmd { /// The indices of the voters; leave empty to vote from every other participant. #[clap(long, value_delimiter = ',')] pub voters: Vec, + /// Optional per-domain reconstruction-threshold overlay, given as a + /// comma-separated list of `DOMAIN_ID:THRESHOLD` pairs (e.g. + /// `--per-domain-threshold 0:6,1:5`). + /// Omitting it preserves every domain's existing threshold; supplying it + /// changes those listed and re-validates the rest against the new + /// participant count. + #[clap(long = "per-domain-threshold", value_delimiter = ',', value_parser = parse_per_domain_threshold)] + pub per_domain_thresholds: Vec<(u64, u64)>, +} + +fn parse_per_domain_threshold(s: &str) -> anyhow::Result<(u64, u64)> { + let (id, t) = s + .split_once(':') + .ok_or_else(|| anyhow::anyhow!("expected DOMAIN_ID:THRESHOLD, got `{s}`"))?; + Ok((id.parse()?, t.parse()?)) } #[derive(clap::Parser)] diff --git a/crates/devnet/src/mpc.rs b/crates/devnet/src/mpc.rs index e41c7e403..7f2ad37b1 100644 --- a/crates/devnet/src/mpc.rs +++ b/crates/devnet/src/mpc.rs @@ -25,8 +25,8 @@ use near_jsonrpc_primitives::types::query::QueryResponseKind; use near_mpc_contract_interface::method_names; use near_mpc_contract_interface::types::{ DomainConfig, DomainPurpose, EpochId, NodeImageHash, ParticipantId, ParticipantInfo, - Participants, Protocol, ProtocolContractState, ReconstructionThreshold, Threshold, - ThresholdParameters, protocol_state_to_string, + Participants, ProposedThresholdParameters, Protocol, ProtocolContractState, + ReconstructionThreshold, Threshold, ThresholdParameters, protocol_state_to_string, }; use near_primitives::types::{BlockReference, Finality, FunctionArgs}; use near_primitives::views::QueryRequest; @@ -732,9 +732,22 @@ impl MpcVoteNewParametersCmd { } else { parameters.threshold }; - let proposal = ThresholdParameters { - participants, - threshold, + let per_domain_thresholds: std::collections::BTreeMap<_, _> = self + .per_domain_thresholds + .iter() + .map(|(id, t)| { + ( + near_mpc_contract_interface::types::DomainId(*id), + near_mpc_contract_interface::types::ReconstructionThreshold::new(*t), + ) + }) + .collect(); + let proposal = ProposedThresholdParameters { + parameters: ThresholdParameters { + participants, + threshold, + }, + per_domain_thresholds, }; let from_accounts = get_voter_account_ids(mpc_setup, &self.voters); @@ -884,7 +897,7 @@ pub async fn read_contract_state( #[derive(Serialize)] struct VoteNewParametersArgs { prospective_epoch_id: EpochId, - proposal: ThresholdParameters, + proposal: ProposedThresholdParameters, } #[derive(Serialize)] diff --git a/crates/e2e-tests/src/cluster.rs b/crates/e2e-tests/src/cluster.rs index 1155dc835..9f82d819c 100644 --- a/crates/e2e-tests/src/cluster.rs +++ b/crates/e2e-tests/src/cluster.rs @@ -9,8 +9,9 @@ use near_kit::AccountId; use near_mpc_contract_interface::method_names; use near_mpc_contract_interface::types::{ AccountId as ContractAccountId, CKDAppPublicKey, DomainConfig, DomainId, DomainPurpose, - Ed25519PublicKey, EpochId, ParticipantId, ParticipantInfo, Participants, Protocol, - ProtocolContractState, ReconstructionThreshold, Threshold, ThresholdParameters, + Ed25519PublicKey, EpochId, ParticipantId, ParticipantInfo, Participants, + ProposedThresholdParameters, Protocol, ProtocolContractState, ReconstructionThreshold, + Threshold, ThresholdParameters, }; use rand::SeedableRng; use rand::rngs::StdRng; @@ -507,9 +508,12 @@ impl MpcCluster { let participants = build_participants_from_nodes(new_participants, &self.nodes, current_participants); - let proposal = ThresholdParameters { - threshold: Threshold(new_threshold as u64), - participants, + let proposal = ProposedThresholdParameters { + parameters: ThresholdParameters { + threshold: Threshold(new_threshold as u64), + participants, + }, + per_domain_thresholds: std::collections::BTreeMap::new(), }; tracing::info!(?prospective_epoch_id, new_threshold, "voting for resharing"); diff --git a/crates/near-mpc-contract-interface/src/lib.rs b/crates/near-mpc-contract-interface/src/lib.rs index d5a189b4d..1b1ae9c89 100644 --- a/crates/near-mpc-contract-interface/src/lib.rs +++ b/crates/near-mpc-contract-interface/src/lib.rs @@ -22,9 +22,10 @@ pub mod types { pub use state::{ AddDomainsVotes, AttemptId, AuthenticatedAccountId, AuthenticatedParticipantId, Curve, DomainConfig, DomainPurpose, DomainRegistry, EpochId, InitializingContractState, KeyEvent, - KeyEventId, KeyEventInstance, KeyForDomain, Keyset, Protocol, ProtocolContractState, - ReconstructionThreshold, ResharingContractState, RunningContractState, Threshold, - ThresholdParameters, ThresholdParametersVotes, protocol_state_to_string, + KeyEventId, KeyEventInstance, KeyForDomain, Keyset, ProposedThresholdParameters, Protocol, + ProtocolContractState, ReconstructionThreshold, ResharingContractState, + RunningContractState, Threshold, ThresholdParameters, ThresholdParametersVotes, + protocol_state_to_string, }; pub use tee::NodeId; pub use updates::{ProposedUpdates, UpdateHash}; diff --git a/crates/near-mpc-contract-interface/src/types/state.rs b/crates/near-mpc-contract-interface/src/types/state.rs index a7f897f7a..447bdcfd2 100644 --- a/crates/near-mpc-contract-interface/src/types/state.rs +++ b/crates/near-mpc-contract-interface/src/types/state.rs @@ -130,7 +130,10 @@ pub use near_mpc_crypto_types::{KeyForDomain, Keyset}; // Threshold/Participants Types // ============================================================================= -/// Threshold parameters for distributed key operations. +/// Threshold parameters for distributed key operations: the current +/// participant set and the governance threshold. This is the stored, +/// always-current shape; per-domain reconstruction-threshold *proposals* live +/// on [`ProposedThresholdParameters`]. #[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize, BorshSerialize, BorshDeserialize)] #[cfg_attr( all(feature = "abi", not(target_arch = "wasm32")), @@ -141,6 +144,61 @@ pub struct ThresholdParameters { pub threshold: Threshold, } +/// A proposed set of threshold parameters submitted to `vote_new_parameters`. +/// Carries the proposed [`ThresholdParameters`] (participant set and threshold) +/// plus an optional per-domain `ReconstructionThreshold` overlay for the +/// resharing it would trigger. +// +// `per_domain_thresholds` proposes an updated `ReconstructionThreshold` for the +// listed domains. An empty map means "keep current per-domain thresholds"; a +// populated map must reference only existing domains (validated by the +// contract). The overlay is applied to the `DomainRegistry` when resharing +// completes and never persists onto the stored `ThresholdParameters`. +// +// ## Wire contract and the `serde(flatten)` migration path +// +// The frozen wire contract is the flat object `{ participants, threshold, +// per_domain_thresholds }` (and the positional borsh layout `[participants, +// threshold, per_domain_thresholds]`). `serde(flatten)` is merely *how* that +// flat JSON is produced today — by reusing `ThresholdParameters` as a named +// sub-field — and is an implementation detail, not part of the contract. +// `serde(default)` parses a payload lacking `per_domain_thresholds` as empty, +// so pre-3.11 callers keep submitting `{ participants, threshold }` unchanged. +// +// To drop `serde(flatten)` later (e.g. after 3.12, to allow +// `#[serde(deny_unknown_fields)]` or to escape flatten's `Content`-buffering +// quirks), inline the two `parameters` fields: +// +// pub participants: Participants, +// pub threshold: Threshold, +// #[serde(default)] +// pub per_domain_thresholds: BTreeMap, +// +// That is byte-identical for JSON, borsh, and the generated ABI: borsh is +// positional and ignores serde attributes, and the inlined JSON keys match. So +// it needs no compat struct and no migration — only the conversion impls in +// `dto_mapping.rs` that read `.parameters` must follow the field move. The +// `proposed_threshold_parameters__*` wire-lock tests below pin this shape so a +// non-equivalent change fails loudly. Changing the *shape itself* (e.g. nesting +// the params under a `parameters` key) WOULD be wire-breaking and would instead +// require a compat deserializer accepting both the old flat and new nested +// forms across a transition window. +#[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize, BorshSerialize, BorshDeserialize)] +#[cfg_attr( + all(feature = "abi", not(target_arch = "wasm32")), + derive(schemars::JsonSchema) +)] +pub struct ProposedThresholdParameters { + #[serde(flatten)] + pub parameters: ThresholdParameters, + // TODO(#3495): drop `serde(default)` after the 3.12 release. It exists only + // so pre-3.11 `vote_new_parameters` payloads (which omit this field) still + // deserialize as an empty overlay; once all callers populate it explicitly + // the field should be mandatory so legacy/malformed payloads fail loudly. + #[serde(default)] + pub per_domain_thresholds: BTreeMap, +} + // ============================================================================= // Voting Types // ============================================================================= @@ -152,7 +210,7 @@ pub struct ThresholdParameters { derive(schemars::JsonSchema) )] pub struct ThresholdParametersVotes { - pub proposal_by_account: BTreeMap, + pub proposal_by_account: BTreeMap, } /// Votes for adding new domains. @@ -241,6 +299,10 @@ pub struct ResharingContractState { pub reshared_keys: Vec, pub resharing_key: KeyEvent, pub cancellation_requests: HashSet, + /// Per-domain `ReconstructionThreshold` overlay carried from the accepted + /// proposal. Applied to the `DomainRegistry` when resharing completes. + #[serde(default)] + pub per_domain_thresholds: BTreeMap, } /// The main protocol contract state enum. @@ -393,6 +455,7 @@ pub fn protocol_state_to_string(contract_state: &ProtocolContractState) -> Strin #[cfg(test)] mod tests { use super::*; + use crate::types::participants::{ParticipantId, ParticipantInfo}; #[test] fn test_protocol_state_serialization() { @@ -413,4 +476,96 @@ mod tests { // Then assert_eq!(output, "Contract is not initialized\n"); } + + /// A proposal carrying a non-empty per-domain overlay, used by the + /// `ProposedThresholdParameters` wire-lock tests. + fn sample_proposal() -> ProposedThresholdParameters { + let participants = Participants { + next_id: ParticipantId(1), + participants: vec![( + "alice.near".parse().unwrap(), + ParticipantId(0), + ParticipantInfo { + url: "https://alice.com".to_string(), + tls_public_key: "ed25519:6E8sCci9badyRkXb3JoRpBj5p8C6Tw41ELDZoiihKEtp" + .parse() + .unwrap(), + }, + )], + }; + ProposedThresholdParameters { + parameters: ThresholdParameters { + participants, + threshold: Threshold::new(1), + }, + per_domain_thresholds: BTreeMap::from([(DomainId(0), ReconstructionThreshold::new(1))]), + } + } + + /// Wire-format lock: the public contract for `ProposedThresholdParameters` is + /// the flat JSON object `{ participants, threshold, per_domain_thresholds }`, + /// not the `#[serde(flatten)]` mechanism that currently produces it. Pinning + /// the shape here means dropping `flatten` later (by inlining the `parameters` + /// fields) can be proven byte-identical rather than taken on faith. + #[test] + #[expect(non_snake_case)] + fn proposed_threshold_parameters__serializes_to_flat_keys() { + // Given a proposal carrying a non-empty per-domain overlay. + let proposal = sample_proposal(); + + // When serialized to JSON. + let value: serde_json::Value = + serde_json::from_str(&serde_json::to_string(&proposal).unwrap()).unwrap(); + + // Then the object is flat: `participants`/`threshold` sit at the top level + // alongside `per_domain_thresholds`, with no nested `parameters` key. + let mut keys: Vec<&str> = value + .as_object() + .unwrap() + .keys() + .map(String::as_str) + .collect(); + keys.sort_unstable(); + assert_eq!(keys, ["participants", "per_domain_thresholds", "threshold"]); + } + + /// TODO(#3495): Pre-3.11 callers submit `{ participants, threshold }` with no + /// `per_domain_thresholds`. `serde(default)` must keep parsing that as an + /// empty (no-change) overlay — the backward-compat guarantee that let the + /// field be added without a wire break. Removed alongside the + /// `serde(default)` it guards. + #[test] + #[expect(non_snake_case)] + fn proposed_threshold_parameters__legacy_payload_omitting_overlay__parses_as_empty() { + // Given a legacy proposal value with the overlay field absent. + let mut legacy = serde_json::to_value(sample_proposal()).unwrap(); + legacy + .as_object_mut() + .unwrap() + .remove("per_domain_thresholds"); + + // When deserialized. + let parsed: ProposedThresholdParameters = serde_json::from_value(legacy).unwrap(); + + // Then the overlay defaults to empty. + assert!(parsed.per_domain_thresholds.is_empty()); + } + + /// borsh is positional and ignores serde attributes, so the stored layout is + /// `[participants, threshold, per_domain_thresholds]` regardless of `flatten`. + /// A round-trip locks that the type stays borsh-stable across the eventual + /// inlining. + #[test] + #[expect(non_snake_case)] + fn proposed_threshold_parameters__borsh_round_trips() { + // Given a proposal carrying a non-empty per-domain overlay. + let proposal = sample_proposal(); + + // When borsh round-tripped. + let bytes = borsh::to_vec(&proposal).unwrap(); + let decoded: ProposedThresholdParameters = borsh::from_slice(&bytes).unwrap(); + + // Then it survives unchanged. + assert_eq!(decoded, proposal); + } } diff --git a/crates/node/src/indexer/fake.rs b/crates/node/src/indexer/fake.rs index a0bea32f5..24534a779 100644 --- a/crates/node/src/indexer/fake.rs +++ b/crates/node/src/indexer/fake.rs @@ -229,6 +229,7 @@ impl FakeMpcContractState { participants_config_to_threshold_parameters(&new_participants), ), cancellation_requests: HashSet::new(), + per_domain_thresholds: std::collections::BTreeMap::new(), }); }