Skip to content

feat(node): chain-compatible presignature selection for verify-foreign-tx - #3960

Draft
anodar wants to merge 1 commit into
anodar/3569-5-node-available-chains-switchfrom
anodar/3569-6-chain-aware-presignatures
Draft

feat(node): chain-compatible presignature selection for verify-foreign-tx#3960
anodar wants to merge 1 commit into
anodar/3569-5-node-available-chains-switchfrom
anodar/3569-6-chain-aware-presignatures

Conversation

@anodar

@anodar anodar commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

Closes #3959

@anodar
anodar force-pushed the anodar/3569-6-chain-aware-presignatures branch from 4ec7b30 to 4c342b4 Compare July 24, 2026 11:33
@near near deleted a comment from claude Bot Jul 24, 2026
…n-tx

The leader now takes a presignature whose participants all support the
requested chain (alive participants intersected with the supporters
map), waiting for background generation to produce a compatible one
when none is buffered. DoubleQueue grows take_owned_matching: drain the
hot receiver into the cold queue, take the first matching asset, retry
as new assets arrive.
@anodar
anodar force-pushed the anodar/3569-6-chain-aware-presignatures branch from 4c342b4 to a4ddf52 Compare July 24, 2026 13:02
@near near deleted a comment from claude Bot Jul 24, 2026
@anodar

anodar commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator Author

@claude code review

@claude

claude Bot commented Jul 24, 2026

Copy link
Copy Markdown

Pull request overview

Adds a chain-aware presignature selection path for verify-foreign-tx signing. Instead of consuming the next available presignature, the leader now waits for one whose participants are all alive AND all support the requested foreign chain. To support this, DoubleQueue gains a take_owned_matching method (with ColdQueue::ingest and take_first_matching helpers) that scans the queue against a caller-supplied condition value, independent of the stored condition value. An early guard (ensure_enough_alive_supporters) rejects the request up-front when the intersection of alive participants and chain supporters is below the reconstruction threshold, and a new ecdsa.all_alive_participant_ids() shim exposes the network liveness view to the verify-foreign-tx provider.

Changes:

  • assets.rs: adds ColdQueue::ingest/take_first_matching and DoubleQueue/DistributedAssetStorage::take_owned_matching, plus five new unit tests covering matching-take semantics and barrier invariants.
  • providers/verify_foreign_tx/sign.rs: computes eligible = alive ∩ chain_supporters, enforces threshold gate, and switches from take_owned to take_owned_matching(eligible); introduces ensure_enough_alive_supporters and its error variant.
  • providers/ecdsa.rs: exposes all_alive_participant_ids() on the ECDSA provider.
  • metrics.rs: extends the doc for MPC_NUM_VERIFY_FOREIGN_TX_UNAVAILABLE_CHAIN_REJECTIONS to include the new gate reason.

Reviewed changes

Per-file summary
File Description
crates/node/src/assets.rs New ingest/take_first_matching on ColdQueue; new take_owned_matching on DoubleQueue and DistributedAssetStorage; five new tests.
crates/node/src/providers/verify_foreign_tx/sign.rs Leader flow now selects a chain-compatible presignature via take_owned_matching, with a new ensure_enough_alive_supporters gate.
crates/node/src/providers/ecdsa.rs Adds all_alive_participant_ids() delegate on EcdsaSignatureProvider.
crates/node/src/metrics.rs Updates rejection-counter description to include the too-few-alive-supporters case.

Findings

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

  • crates/node/src/providers/verify_foreign_tx/sign.rs:79-83 — Missing self-participation guard. Owned presignatures always contain my_participant_id in presignature.participants, so the matching predicate participants ⊆ eligible can only succeed if my_participant_id ∈ eligible, i.e. if this node itself supports the requested chain. ensure_enough_alive_supporters only checks eligible.len() >= threshold, and eligible_leaders_and_heights (queue.rs:534) does not filter leaders by chain support — so a node that does not support the requested chain can still become the leader, pass the threshold gate, and then have take_owned_matching spin (draining hot into cold on every iteration) until the outer signature.timeout_sec cancels it. Consider adding an explicit eligible.contains(&my_participant_id) check next to the threshold check and rejecting via the same metric, so the failure mode is fast + observable rather than a silent timeout.

  • crates/node/src/providers/verify_foreign_tx/sign.rs:59-61 — The rewrite of the pre-existing comment removes the "why". Before this PR it read: "checked early here because take_owned below irreversibly consumes a presignature. An availability flip between the two checks still costs one presignature." The new comment ("checked early here so an unavailable chain is rejected up front") restates what the code does without the reason. take_owned_matching still consumes a presignature on success, so the original rationale still applies — please restore something equivalent.

  • crates/node/src/assets.rs:283-311take_owned_matching scans the entire cold queue under the mutex on every iteration (both take_first_matching and the hot-drain while let run under the lock). For presignature stores that can accumulate a large backlog this is a per-call O(n) scan holding a mutex shared with the background triple/presig workers. Presumed low-volume for verify-foreign-tx, but worth noting; if it ever becomes hot, indexing supporters into a data structure that can short-circuit misses (or moving the scan out from under the mutex) would help.

  • crates/node/src/assets.rs:270-291 — Public API doc for DistributedAssetStorage::take_owned_matching doesn't mention that the wait is unbounded (bounded only by callers' timeout) and that it aggressively drains the hot queue into cold's "unknown" section. Would be helpful to consumers to note both, since it changes the profile of the cold queue while a matching take is in flight.

  • crates/node/src/providers/verify_foreign_tx/sign.rs:65-69 — Given ensure_chain_is_available (line 63) already asserts the entry is present, the follow-up .and_then(...).cloned().unwrap_or_default() is a redundant defensive fallback for a case that has just been ruled out. If the intent is to guard against a snapshot flip between the two borrow() calls (there's only one here), consider a single .expect_or_else with a note, or simplify to a direct get(...).cloned().unwrap() — either is more honest about what's true at this point.

  • crates/node/src/providers/verify_foreign_tx/sign.rs:88let participants: Vec<ParticipantId> = presignature.participants.clone(); — the explicit type annotation is unnecessary (was let participants = ... before), and the introduced use crate::primitives::ParticipantId is used elsewhere in the function so annotating this one binding just for the type name adds noise.

  • crates/node/src/assets.rs:270-291take_owned_matching and take_owned diverge slightly on freshness of the condition value: take_owned calls update_condition_value (unconditional) on entry so a just-changed alive-set is immediately reflected, while take_owned_matching only calls update_condition_value_if_due. Given the comment explaining "matching doesn't depend on stored condition value" that's intentional, but it means concurrent take_owned callers after a matching-take may observe a slightly staler barrier than before. Probably fine; flagging so it's a conscious choice.

⚠️ Issues found

@anodar
anodar marked this pull request as draft July 24, 2026 20:49
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