[SimonRastikian]

No description provided.

[github-actions]

Pull request overview

This PR finishes moving add-domains voting off the cloned/compared RunningContractState and onto MpcContract, mirroring what the base branch already did for parameters_votes. AddDomainsVotes becomes a thin wrapper over the storage-backed Votes<AuthenticatedParticipantId>, storing only the Borsh/SHA-256 proposal hash of the domain list instead of the full Vec<DomainConfig> per voter. Because the votes no longer travel inside RunningContractState::new (which used to filter them by the new participant set), the pruning of votes from dropped participants is now done explicitly at each MpcContract-level transition into Running.

Changes:

• AddDomainsVotes rewritten on top of Votes with vote/retain/clear/all; vote now returns the acceptance decision and clears on acceptance. ToProposalHash relaxed to ?Sized so [DomainConfig] can be hashed directly.
• RunningContractState loses its add_domains_votes field and the constructor parameter; vote_add_domains takes &self plus a &mut AddDomainsVotes.
• MpcContract gains add_domains_votes; two new StorageKey variants appended; pruning added in vote_reshared, the no-domain in-place path of vote_new_parameters, and the no-domain path of verify_tee.
• v3_17_0_state shadows the old AddDomainsVotes layout and drops pending votes on migration; borsh schema snapshot updated.
• state() view now passes a PendingVotes bundle (parameters + add-domains) into the DTO mapping.
• Tests updated for the new API; new unit tests for vote/retain and a contract-level test that pending add-domain votes survive a same-participant key refresh.

Reviewed changes

Per-file summary

File	Description
crates/contract/src/primitives/domain.rs	AddDomainsVotes reimplemented over Votes; ToProposalHash for [DomainConfig]; unit tests rewritten
crates/contract/src/primitives/proposal_hash.rs	SerializeProposal/ToProposalHash made ?Sized-friendly
crates/contract/src/state/running.rs	Field and constructor param removed; vote_add_domains(&self, domains, votes); tests adapted
crates/contract/src/state.rs	vote_add_domains threads the votes registry through
crates/contract/src/state/initializing.rs, resharing.rs	Drop the AddDomainsVotes argument at RunningContractState::new call sites and the related assertions
crates/contract/src/lib.rs	New add_domains_votes field on MpcContract with doc comment
crates/contract/src/api/governance.rs	vote_add_domains passes the registry; unconditional retain after vote_new_parameters; new/renamed tests
crates/contract/src/api/key_events.rs	retain after a resharing completes
crates/contract/src/api/attestation.rs	retain on the no-domain in-place kick-out path
crates/contract/src/api/lifecycle.rs	Initialise the new field; state() builds PendingVotes
crates/contract/src/dto_mapping.rs	PendingVotes bundle; DTO conversion for AddDomainsVotes uses all()
crates/contract/src/storage_keys.rs	Two new appended keys
crates/contract/src/v3_17_0_state.rs	OldAddDomainsVotes shadow; votes dropped at migration
crates/contract/src/snapshots/…borsh_schema… .snap	Schema moves AddDomainsVotes from RunningContractState to MpcContract
crates/contract/src/api/test_utils.rs, state/test_utils.rs, api/foreign_chain/support.rs	Test fixtures adapted
crates/node/src/indexer/fake.rs	Fake contract state adapted to the new constructor and struct shape

I traced every path that lands in Running with the votes now living outside the state: init/init_running (fresh), vote_pk and vote_cancel_keygen (votes already cleared when entering Initializing, and vote_add_domains is rejected outside Running so nothing can accumulate), vote_reshared (retain), vote_cancel_resharing (participants unchanged, votes still valid), the two no-domain in-place applications (retain), and migrate (default). The lifecycle is complete and matches the pre-refactor semantics. The Borsh encoding of [T] and Vec<T> is identical, so proposal hashes are unchanged for existing consumers; the ABI snapshot and backup-cli fixture are untouched as expected. Only archived docs describe the old layout, so there is no live-doc drift.

Findings

Non-blocking (nits, follow-ups, suggestions):

• crates/contract/src/api/governance.rs:63 — The comment says the retain is for the no-domain in-place case, but the block runs after every successful vote_new_parameters while Running, including ordinary votes that change nothing. The root cause is that RunningContractState::vote_new_parameters returns Ok(None) both for "vote recorded" and "proposal applied in place" (crates/contract/src/state/running.rs:84), so the caller cannot tell them apart. Functionally harmless (participants unchanged means the retain is a no-op), but it adds an O(voters) storage scan to every vote and the comment misleads. Consider making the in-place application observable (e.g. have transition_to_resharing_no_checks return an enum, or have the state-level vote_new_parameters return Some(ProtocolContractState::Running(..)) for the in-place case) so the retain is scoped to the actual transition, which would also let crates/contract/src/api/attestation.rs:351 share the same path.
• crates/contract/src/api/governance.rs:63, crates/contract/src/api/attestation.rs:351 — Neither in-place pruning path has a test; the only assertion that covered the no-domain case (assert_eq!(state.add_domains_votes, AddDomainsVotes::default()) in test_running_for) was removed in crates/contract/src/state/running.rs:366. Reverting either retain call would not fail any test, which the engineering standards ask to avoid. A contract-level test built from init (no domains), one pending vote_add_domains, then a unanimous vote_new_parameters that drops that voter and asserting add_domains_votes.is_empty() would cover the governance path; a similar fixture with a rejected attestation would cover verify_tee. The existing vote_reshared__should_retain_add_domains_votes_of_current_participants keeps the same participant set, so it only shows survival, not pruning (pruning on resharing is covered by the sandbox test).
• crates/contract/src/lib.rs:73 — Doc says votes from dropped participants are discarded "when a resharing completes", but they are also discarded on the in-place parameter application and the TEE kick-out. "whenever the participant set changes" would stay accurate.
• crates/contract/src/primitives/domain.rs:547 (also 564, 578, 601, 618) — These are newly written tests but use the test_… prefix rather than the <sut>__should_<assertion> form AGENTS.md requires for new tests; the module already carries #[expect(non_snake_case)].
• crates/contract/src/api/key_events.rs:111 — The pruning invariant that used to live in one place (RunningContractState::new) is now spread across three call sites with slightly different participant sources. A single MpcContract helper (e.g. prune_votes_of_dropped_participants) invoked wherever protocol_state becomes Running would restore local reasonability and make the next transition harder to forget.

✅ Approved