Skip to content

refactor: simplifying storage by giving it to the client directly - #3851

Open
SimonRastikian wants to merge 140 commits into
mainfrom
1680-storage-takes-client
Open

refactor: simplifying storage by giving it to the client directly#3851
SimonRastikian wants to merge 140 commits into
mainfrom
1680-storage-takes-client

Conversation

@SimonRastikian

Copy link
Copy Markdown
Contributor

Simplifies the triple generation part following #3164. Concretely, it refactors TripleStorage::new so the storage takes the network client directly, instead of being handed the two pieces of state it used to derive from that client.

Net effect: TripleStorage depends on one cohesive object (the mesh client) rather than being passed several client-derived values separately.

Reference #1680 (comment)

SimonRastikian and others added 30 commits June 15, 2026 23:12
…inator)

Colleagues did not agree on an 80% upper bound for the GovernanceThreshold, so
set MAX_THRESHOLD_NUMERATOR = MAX_THRESHOLD_DENOMINATOR (5/5 = 100%). The relative
upper cap structure is kept but never binds below the absolute `k <= n` check, so
the GovernanceThreshold may again go up to the participant count. The cross-domain
rule (GovernanceThreshold >= max(ReconstructionThreshold)) is unchanged.

Revert the test changes that were only needed to satisfy the 80% cap (dropping
thresholds / raising participant counts) and remove the now-meaningless dedicated
upper-cap tests:
- thresholds.rs: restore 5/5-participant thresholds; drop reject-above-cap test
- dto_mapping.rs / lib.rs: drop the upper-cap rejection tests
- lib.rs verify_tee: rework the kickout-refusal fixture to break the relation via
  the participant-count ceiling instead of the cap
- running.rs: make the reconstruction>governance test regenerate until gov < n
- sandbox + e2e + node resharing tests: restore original participant/threshold values
- docs/design/domain-separation.md: describe the cap as disabled (100%)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…hreshold' of github.com:near/mpc into 3499-correlate-governancethreshold-with-reconstructionthreshold
…hreshold' of github.com:near/mpc into 3499-correlate-governancethreshold-with-reconstructionthreshold

# Conflicts:
#	crates/contract/src/dto_mapping.rs
Co-authored-by: Mårten Blankfors <marten@blankfors.se>
@claude

claude Bot commented Jul 17, 2026

Copy link
Copy Markdown

Pull request overview

Refactors TripleStorage::new to accept a &Arc<MeshNetworkClient> in place of two client-derived arguments (my_participant_id and an alive_participant_ids_query closure). This aligns TripleStorage::new's signature with PresignatureStorage::new, which already takes the client directly, and centralizes the "who is alive?" closure construction inside the storage constructor via the shared active_participants_query helper.

Changes:

  • TripleStorage::new now takes &Arc<MeshNetworkClient> and internally derives my_participant_id() and calls active_participants_query(client) (crates/node/src/providers/ecdsa/triple.rs:57)
  • EcdsaSignatureProvider::new simplified: no longer builds the alive-query closure at the call site (crates/node/src/providers/ecdsa.rs:70)
  • New test-only helper network::testing::new_test_client constructs a fully-alive MeshNetworkClient for unit tests (crates/node/src/network.rs:952)
  • Three cleanup tests and the local new_triple_store helper migrated to the new signature via new_test_client

Reviewed changes

Per-file summary
File Description
crates/node/src/providers/ecdsa/triple.rs New TripleStorage::new signature; new_triple_store test helper uses new_test_client
crates/node/src/providers/ecdsa.rs Call site simplified to pass &client
crates/node/src/network.rs Adds new_test_client helper in the testing module
crates/node/src/assets/cleanup.rs Three cleanup tests rewired to construct the store from new_test_client

Findings

No blocking issues. Purely mechanical refactor with no behavior change:

  • active_participants_query(client) reproduces the exact closure previously built inline (client.all_alive_participant_ids()), so runtime behavior is preserved.
  • All prior test call sites passed all_participants as the "alive" list, which matches the new new_test_client behavior (TestConnectivityInterface::is_bidirectionally_connected() returns true and indexer heights all start at 0, so every participant appears alive).
  • MeshNetworkClient::new is private, but network::testing is a submodule of network, so super::MeshNetworkClient::new remains accessible.
  • Matches the existing PresignatureStorage::new idiom (crates/node/src/providers/ecdsa_common.rs:46), improving consistency.

Non-blocking (nits, optional):

  • crates/node/src/network.rs:952new_test_client duplicates ~5 lines of the transport/tracker plumbing that already appears in new_test_transports and new_task_channel_for_test. Not worth extracting on its own, but if a fourth variant appears, consolidating into a single TestMeshTransportSender/IndexerHeightTracker builder would pay off.

✅ Approved

@SimonRastikian SimonRastikian self-assigned this Jul 17, 2026
Base automatically changed from 3164-adding-ReconstructionThreshold-in-node to main July 21, 2026 13:03
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