Skip to content

feat(contract): report pending parameter votes by proposal hash in state() - #4872

Open
SimonRastikian wants to merge 14 commits into
2825-store-parameter-votes-hash-onlyfrom
2825-report-parameter-votes-tally
Open

SimonRastikian wants to merge 14 commits into
2825-store-parameter-votes-hash-onlyfrom
2825-report-parameter-votes-tally

Conversation

@SimonRastikian

@SimonRastikian SimonRastikian commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

This is part of the #2825

After taking out the GovernanceThresholdParametersVotes out of the RunningContractState (in PR 1), and after storing the only the proposalhashes and voters (via Votes struct), it's time to allow a proper migration of the state.

In this PR:

  • taking care of wire format to allow smooth rollout. (manually implementing deserialization instead of deriving due to the legacy type being casted into the new type)
  • updating the backup-cli
  • Small DTO edits to allow dto::GovernanceThresholdParametersVotes becoming a transparent type filled with the MpcContract.parameters_votes
  • tests

@SimonRastikian SimonRastikian self-assigned this Oct 20, 2026
@SimonRastikian
SimonRastikian added this pull request to stack #4871 October 20, 2026 23:48
@SimonRastikian
SimonRastikian marked this pull request as draft October 20, 2026 23:49
@github-actions

Copy link
Copy Markdown
Contributor

Pull request overview

Third PR in the #2825 stack (on top of #4869 and #4870). It makes state() report pending governance-parameter votes again, now as a map of proposal hash to voter set instead of the always-empty proposal_by_account placeholder the base PR left behind. The DTO becomes a transparent newtype mirroring MeasurementVotes, the contract-side Votes tally is exposed through a new all() accessor, and the Running/Resharing/Protocol DTO conversions take the converted tally as a tuple argument because the votes live on MpcContract rather than on the protocol state. The ABI snapshot, the backup-cli fixture, and two state() unit tests are updated accordingly.

Changes:

  • GovernanceThresholdParametersVotes DTO: {proposal_by_account: …} struct replaced by a #[serde(transparent)] newtype over BTreeMap<ProposalHash, BTreeSet<AuthenticatedAccountId>> with Default/Deref; the TODO(#2825) is removed.
  • Contract: new GovernanceThresholdParametersVotes::all(), an IntoInterfaceType impl for it, and the Running/Resharing/Protocol conversions reworked to (&State, dtos::GovernanceThresholdParametersVotes) tuples; state() threads the converted tally through. The test-utils From<ProtocolContractState> passes an empty tally.
  • Tests: state__should_report_pending_parameter_votes_by_proposal_hash and state__should_report_re_proposal_votes_while_resharing, plus an expected_tally helper.
  • ABI snapshot and backup-cli fixture updated to the new wire shape.

Reviewed changes

Per-file summary
File Description
crates/near-mpc-contract-interface/src/types/state.rs DTO becomes a transparent newtype map keyed by ProposalHash; adds Default, derive_more::Deref; drops the wire-compat placeholder and its TODO
crates/contract/src/primitives/threshold_votes.rs New public all() accessor returning the hash-keyed tally
crates/contract/src/dto_mapping.rs New IntoInterfaceType for the votes; Running/Resharing/Protocol conversions now take (state, tally) tuples; test-only From supplies an empty tally
crates/contract/src/api/lifecycle.rs state() converts MpcContract.parameters_votes and threads it into the state conversion
crates/contract/src/api/governance.rs Two state() tests (Running and Resharing re-proposal) and expected_tally helper
crates/backup-cli/assets/contract_state.json Fixture parameters_votes becomes {}
crates/contract/tests/snapshots/abi__abi_has_not_changed.snap GovernanceThresholdParametersVotes definition removed; parameters_votes rendered inline as a map of account arrays

Findings

Blocking (must fix before merge):

  • crates/near-mpc-contract-interface/src/types/state.rs:188 — Wire-compatibility break during the rolling upgrade. The deployed 3.17.0 contract returns "parameters_votes": {"proposal_by_account": {…}}. The new transparent map deserializes every key as a strict 32-byte hex ProposalHash, so "proposal_by_account" fails to parse and the whole ProtocolContractState read fails. The node's monitor_contract_state loop (crates/node/src/indexer/participants.rs:328-338) logs and retries forever on that error, and the first fetch blocks startup (participants.rs:357), so a node running this interface crate against the not-yet-upgraded contract never reads contract state. The operator guide documents that nodes are upgraded before the contract (docs/guide/operating-an-mpc-node.md:58), and the base PR kept proposal_by_account explicitly "for wire compatibility"; this PR removes that shim with no transition. The backup-cli has the same exposure via its state() read. The #4498 rollout (accepted_at_seconds) is the precedent: accept both shapes on the consumer side until the contract is live. Suggested fix: keep the Serialize/JsonSchema derives (so the ABI stays as in this PR) and hand-write Deserialize to accept both shapes, with a TODO(#…) to drop the legacy arm once the contract release is deployed, plus two unit tests (legacy object → empty tally; hex-keyed map → tally):
    impl<'de> Deserialize<'de> for GovernanceThresholdParametersVotes {
        fn deserialize<D: serde::Deserializer<'de>>(deserializer: D) -> Result<Self, D::Error> {
            #[derive(Deserialize)]
            #[serde(untagged)]
            enum Wire {
                // TODO(#…): drop once no deployed contract reports the pre-3.18 voter-keyed shape.
                Legacy { proposal_by_account: serde::de::IgnoredAny },
                Tally(BTreeMap<ProposalHash, BTreeSet<AuthenticatedAccountId>>),
            }
            Ok(match Wire::deserialize(deserializer)? {
                Wire::Legacy { .. } => Self::default(),
                Wire::Tally(tally) => Self(tally),
            })
        }
    }
    Note #[serde(from = …)] is not an option here: serde rejects it together with #[serde(transparent)], and schemars would also swap the ABI schema for the helper enum. If the team instead decides to upgrade the contract before the nodes for this release, say so in the PR description and the operator guide, since it inverts the documented order.

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

  • crates/near-mpc-contract-interface/src/types/state.rs:188 — No interface-crate test pins the JSON shape (hex key → array of account IDs) the way measurement_votes__should_serialize_as_a_transparent_map_keyed_by_proposal_hash does at crates/near-mpc-contract-interface/src/types/tee.rs:280. The contract tests compare DTO values in-process, so only the ABI snapshot currently guards the wire format. A mirror test is cheap and would also host the compat cases above.
  • crates/near-mpc-contract-interface/src/types/state.rs:274 — In Resharing, previous_running_state.parameters_votes now carries re-proposal votes for the prospective epoch (as the new test asserts). That is reasonable, but it is not obvious to an API consumer reading the DTO; a one-line doc on previous_running_state or on the votes type would prevent misreading them as stale votes from before the resharing.
  • crates/contract/src/primitives/threshold_votes.rs:68 — The doc comment "Pending votes keyed by proposal hash." paraphrases the return type and can be dropped per the comment standards.

⚠️ Issues found

@SimonRastikian
SimonRastikian marked this pull request as ready for review October 23, 2026 13:03

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant