feat: implement voting for RPC providers by anodar · Pull Request #3249 · near/mpc

anodar · 2026-05-15T15:06:09Z

Note: each vote is a full snapshot rather than diff.

Closes: #3215

…t-stores-rpc-providers

…c-providers' into 3215-implement-voting-mechanism-for-adding-removing-allowed-rpc-provider

…owed-rpc-provider' of github.com:near/mpc into 3215-implement-voting-mechanism-for-adding-removing-allowed-rpc-provider

claude · 2026-05-18T09:28:03Z

Pull request overview

Adds the vote_update_foreign_chain_providers entry point and supporting ProviderVotes storage. Switches from PR1's Add/Remove ops model to a per-chain full-snapshot vote: each ChainVote carries the proposed (providers, response_quorum) pair, and the chain's stored ChainEntry is replaced once the protocol's signing threshold of participants holds the same canonical pair. Adds clean_tee_status extension to drop votes from non-participants after resharing.

Changes:

ChainVote DTO and ChainEntry/ProviderVotes borsh-stored types
ForeignChainRpcWhitelist::vote with canonicalization (sort by provider_id, reject duplicate provider/chain in a batch, reject empty batch)
MpcContract::vote_update_foreign_chain_providers entry, gated by voter_or_panic and the protocol signing threshold
clean_tee_status extended to call retain_only(current_participants) on the pending votes
WASM size hard limit bumped 1490000→1540000
Borsh + ABI snapshots regenerated

Reviewed changes

Per-file summary

File	Description
crates/contract/src/foreign_chain_rpc.rs	Replaces `AllowedProviders` add/remove API with snapshot `replace`; adds `ProviderVotes` pending storage; adds `ForeignChainRpcWhitelist::vote` + `canonicalize` and a full unit-test suite.
crates/contract/src/lib.rs	New `vote_update_foreign_chain_providers` endpoint; `clean_tee_status` now also clears non-participant rows from `foreign_chain_rpc_whitelist.votes`.
crates/contract/tests/sandbox/user_views.rs	Adds sandbox test exercising the entry point across 6/10 participants.
crates/near-mpc-contract-interface/src/types/foreign_chain.rs	Adds `ChainVote` DTO.
crates/near-mpc-contract-interface/src/method_names.rs	Adds `VOTE_UPDATE_FOREIGN_CHAIN_PROVIDERS` constant.
crates/contract/src/snapshots/...snap, crates/contract/tests/snapshots/abi__abi_has_not_changed.snap	Regenerated borsh / ABI snapshots reflecting the new shape.
docs/design/allowing-per-node-foreign-chain-rpc-configuration.md	Updates the design summary to the snapshot model.
scripts/check-contract-wasm-size.sh	Raises hard limit 1490000→1540000.

Findings

Blocking (must fix before merge):

docs/foreign-chain-transactions.md:329, 373, 394-413, 449-455 — The canonical design doc (linked from docs/design/allowing-per-node-foreign-chain-rpc-configuration.md as the "source of truth for the implementation") still describes the superseded Add/Remove model: Vec<ProviderVoteAction> with Add { chain, entry } | Remove { chain, provider_id }, the old BTreeMap<ForeignChain, BTreeMap<ProviderId, ProviderEntry>> shape, per-chain chain_thresholds / DEFAULT_PROVIDER_VOTE_THRESHOLD = 2, and "entire pending vote map cleared on apply" — none of which match what this PR ships (full-snapshot ChainVote, BTreeMap<ForeignChain, ChainEntry>, protocol signing threshold via self.threshold(), only the applied chain's slots cleared via clear_chain). Per CLAUDE.md "Documentation alignment", doc drift is review-blocking, not a follow-up. Either rewrite §"Vote semantics (PR 2)", §"Whitelist storage shape", and the "Why" subsections to match the snapshot model, or front-load a Status: superseded by #3249 banner on the stale sections.
crates/near-mpc-contract-interface/src/types/foreign_chain.rs:1004, 1008-1009 — The ChainRouting doc-comments promise "segment MUST NOT contain '/' (validated when a vote applies)" and "name here MUST differ from auth_name (validated when a vote applies)". Neither check exists in canonicalize or the entry point in this PR, even though "when a vote applies" is precisely the contract-side path this PR adds. Either add the validations to canonicalize (returning InvalidParameters::MalformedPayload like the duplicate-provider check) or remove the "validated" claims from these doc-comments.

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

crates/contract/src/foreign_chain_rpc.rs:120-137 — ChainVote.threshold (the RPC response quorum) is stored verbatim without bounds checking. threshold = 0 (no agreement required) and threshold > providers.len() (impossible to satisfy) both make it through to ChainEntry and become the canonical state once threshold participants vote the same shape. An empty providers list is similarly accepted. These shapes are operationally meaningless and the contract is the natural place to reject them since the design doc bills canonicalize as where vote-time validation lives. Consider adding threshold >= 1 && threshold as usize <= providers.len() and !providers.is_empty() (unless empty is the intended "clear-this-chain" idiom — in which case worth documenting).
crates/contract/src/foreign_chain_rpc.rs:49-54 — count_for_chain counts every pending row regardless of whether the participant is still in the current set. vote_update (lib.rs:1245-1258) filters by current participants for exactly this reason, with the comment "This ensures correctness even if the cleanup promise in vote_reshared() fails." If the clean_tee_status promise fails (gas exhaustion, panic, etc.) after a resharing, stale rows from former participants remain in pending and count toward the threshold gate alongside current participants' votes. This matches the existing vote_add_os_measurement pattern, so it's consistent — but the vote_update gate is the safer precedent; consider filtering here too.
crates/contract/src/lib.rs:1489-1494 — The ContractNotInitialized → env::panic_str(...) branch is unreachable: voter_or_panic() runs three lines earlier and calls authenticate_update_vote, which already returns Err (→ panic via FunctionError) on NotInitialized. Not a bug, but the explicit branch suggests a guard that isn't actually doing work.
crates/contract/src/foreign_chain_rpc.rs:303-329 — vote__should_overwrite_only_mentioned_chain_slots_on_recast doesn't exercise overwriting: it shows that voting Ethereum doesn't disturb a prior Polygon vote (i.e. unmentioned slots are preserved), but never has a participant re-cast the same chain with a different ChainEntry. The "last write wins" property for a same-(participant, chain) key relies on BTreeMap::insert semantics and is worth pinning with a direct test.
crates/contract/tests/sandbox/user_views.rs:74-115 — The sandbox test asserts each of 6 calls succeeds but never verifies the chain entry was actually applied (and that pending votes were cleared). With no view function in this PR there's no clean way to inspect, but at minimum a 7th vote with a different shape would catch a regression where applying didn't reset the count.

⚠️ Issues found

…emoving-allowed-rpc-provider

kevindeforth

Some comments and questions.
Partial review only, didn't look at the tests yet.

kevindeforth · 2026-05-20T07:26:55Z

+        {
+            if routing_name == auth_name {
+                return Err(InvalidParameters::MalformedPayload {
+                    reason: format!(
+                        "ChainRouting::QueryParam.name collides with AuthScheme::Query.name {:?} for provider_id {:?}",
+                        auth_name, id.0
+                    ),
+                }
+                .into());
+            }


same here, why do we check for this specifically?

Some RPC providers take chain and auth parameter as query parameter, something like: ?network=ethereum, ?dkey=<TOKEN>. They can't be the same param obviously, it's a trivial sanity check for copy/pasting errors.

Makes sense, but are these checks exhaustive?
I.e. if a config passes these checks, is that a sufficient condition for the config to be valid?

If not, then the voter will still need to make some manual verification and we might as well remove these.

It's not exhaustive but I'm not sure that's a good reason for dropping sanity checks entirely. These are checks that can be easily checked, other validity checks can't be easily checked from the contract.

Interesting. Then, I have two follow-up questions:

where will the other validity checks take place?

what guarantees do the validity checks on the contract provide for the data stored on-chain?

where will the other validity checks take place?

Full validity check will be manual, when we add a new RPC provider we have to manually check how does one construct a query (how is chain specified, how is authentication done), then define a config according to it and do simple probe test. We can add e2e tests where one adds new config for new RPC provider and expected response to validate config of new RPC before proposing it. Although I'd rather do that in separate PR to not bloat this further.

what guarantees do the validity checks on the contract provide for the data stored on-chain?

It's more sanity check than validations. It currently guarantees that you don't make simple copy/paste error e.g. where you pass same query param name for auth and chain, or that you don't have backslash in chain name.

anodar · 2026-05-20T16:17:22Z

Regarding the high-level API, I might have missed it, but is there a way for a user to withdraw / cancel a vote?

There's no explicit withdraw vote, but you can submit different vote for that chain and it will override your current one.

…emoving-allowed-rpc-provider Resolve conflict in crates/contract/src/lib.rs by unifying the migration shims: main's placeholder v3_10_state.rs (added in #3280) is replaced with this branch's migration logic that converts the actual 3.10.0 nested-map ForeignChainRpcWhitelist shape to the new IterableMap + Votes shape. The duplicate v3_10_0_state.rs is removed.

…emoving-allowed-rpc-provider

kevindeforth

Thanks for the hard work!

Some things to check before merging:

error placement seems not consistent with the rest of the crate
missing unit test for validation method (& its placement)
we should check if the gas constants improved after merging #3280. They seem pretty high to me.

…emoving-allowed-rpc-provider Resolve conflicts: - crates/contract/src/v3_10_state.rs: combine main's `LegacyPendingRequests` shadow type (added in #3315 when the live field was dropped from `crate::MpcContract`) with this branch's `OldForeignChainRpcWhitelist` + `OldProviderEntry` shadows (needed to decode the 3.10.0 nested-map whitelist shape). The `From` impl now drops `legacy_pending_requests` (no longer a field on `crate::MpcContract`) and default-initializes `foreign_chain_rpc_whitelist` (reshape from this branch). Unified derive shape `Debug, BorshSerialize, BorshDeserialize` on every shadow struct — the derived serializer's per-field reads satisfy `dead_code`, so the prior `#[expect(dead_code)]` markers are gone. - crates/contract/src/snapshots/...borsh_schema...snap: drop the `assertion_line` metadata; tests regenerate it. - crates/near-mpc-contract-interface/Cargo.toml: re-add `thiserror` dep (main's #3314 dropped it; this branch still needs it for `ChainEntryValidationError`).

anodar · 2026-05-22T08:58:15Z

Thanks for the thorough review!

kevindeforth

Thanks for the changes!

anodar added 12 commits May 12, 2026 23:22

feat: implement foreign chain RPC providers in contract

3668905

update snapshot

8af7a5a

fix test

1c364a0

Address comments

29e5d74

Update doc about batch voting

f369fb4

cargo fmt

0b91eb4

update snap

6a9dcf7

Address claude's comments

bb8893a

shring contract size, address comments

8af299e

cargo fmt

680e173

Drop verbose comments

905d1ad

drop more

bad3c73

anodar changed the title ~~draft~~ feat: implement voting for RPC providers May 17, 2026

anodar added 5 commits May 17, 2026 23:28

Address nits

8d9eef1

update doc

470c43d

draft

6652ebf

better voting

b1db9db

rebase

af41e1e

anodar force-pushed the 3215-implement-voting-mechanism-for-adding-removing-allowed-rpc-provider branch from 53c8806 to af41e1e Compare May 17, 2026 21:52

anodar added 7 commits May 17, 2026 23:58

Update doc

383d9ad

Merge branch 'main' into 3214-design-and-implement-schema-how-contrac…

3d8a138

…t-stores-rpc-providers

bump up har limit

ed2fcdb

Merge branch '3214-design-and-implement-schema-how-contract-stores-rp…

f5681ae

…c-providers' into 3215-implement-voting-mechanism-for-adding-removing-allowed-rpc-provider

cleanup

05cc42b

Merge branch '3215-implement-voting-mechanism-for-adding-removing-all…

7f6a803

…owed-rpc-provider' of github.com:near/mpc into 3215-implement-voting-mechanism-for-adding-removing-allowed-rpc-provider

modify test

e5f8f95

anodar marked this pull request as ready for review May 18, 2026 09:20

Base automatically changed from 3214-design-and-implement-schema-how-contract-stores-rpc-providers to main May 18, 2026 10:20

Merge branch 'main' into 3215-implement-voting-mechanism-for-adding-r…

6dc58c2

…emoving-allowed-rpc-provider

anodar requested a review from kevindeforth May 19, 2026 22:49

kevindeforth reviewed May 20, 2026

View reviewed changes

Address comments

c753932

anodar requested review from DSharifi and kevindeforth May 20, 2026 16:55

anodar added 3 commits May 20, 2026 18:55

Merge branch 'main' into 3215-implement-voting-mechanism-for-adding-r…

59fd5ad

…emoving-allowed-rpc-provider

cargo clippy fix

9f3c722

cleanup

fc0f108

DSharifi previously approved these changes May 21, 2026

View reviewed changes

Comment thread crates/contract/src/v3_10_state.rs Outdated

Drop pub crate, cleanup

86d7ae6

anodar dismissed DSharifi’s stale review via 86d7ae6 May 21, 2026 13:26

DSharifi previously approved these changes May 21, 2026

View reviewed changes

Comment thread crates/contract/src/lib.rs

kevindeforth reviewed May 21, 2026

View reviewed changes

anodar added 3 commits May 22, 2026 03:16

lower gas, contract limit, switch to two types

e783286

add test, revert comment

dcec582

anodar dismissed DSharifi’s stale review via dcec582 May 22, 2026 08:56

Bump gas

0314645

kevindeforth approved these changes May 22, 2026

View reviewed changes

anodar enabled auto-merge May 22, 2026 09:38

anodar requested a review from DSharifi May 22, 2026 09:48

DSharifi approved these changes May 22, 2026

View reviewed changes

anodar added this pull request to the merge queue May 22, 2026

Merged via the queue into main with commit f83dd6d May 22, 2026
14 checks passed

anodar deleted the 3215-implement-voting-mechanism-for-adding-removing-allowed-rpc-provider branch May 22, 2026 13:53

barakeinav1 mentioned this pull request Jun 1, 2026

feat: fan out requests to all configured RPC providers #3225

Merged

Uh oh!

Conversation

anodar commented May 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

claude Bot commented May 18, 2026

Pull request overview

Reviewed changes

Findings

Uh oh!

kevindeforth left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kevindeforth May 20, 2026

Choose a reason for hiding this comment

Uh oh!

anodar May 20, 2026

Choose a reason for hiding this comment

Uh oh!

kevindeforth May 20, 2026

Choose a reason for hiding this comment

Uh oh!

anodar May 20, 2026

Choose a reason for hiding this comment

Uh oh!

kevindeforth May 21, 2026

Choose a reason for hiding this comment

Uh oh!

anodar May 21, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

anodar commented May 20, 2026

Uh oh!

Uh oh!

Uh oh!

kevindeforth left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

anodar commented May 22, 2026

Uh oh!

kevindeforth left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

anodar commented May 15, 2026 •

edited

Loading

kevindeforth left a comment •

edited

Loading