feat(core): wire up validatorapi Component subscribe/await/verify hooks#458
Merged
Conversation
Adds the plumbing every subsequent submit/await handler needs without implementing any of the unimplemented!() arms. Mirrors Charon's core/validatorapi/validatorapi.go:196-256 (subscriber list + six Register* hooks) plus :1352 (verifyPartialSig). - New Component fields: subs, await_proposal_fn, await_agg_attestation_fn, await_sync_contribution_fn, await_agg_sig_db_fn, duty_def_fn, pub_key_by_att_fn. All Option<Arc<…>> so registration before the Component is shared in an Arc, then read-only thereafter. - subscribe() wraps the user closure with a set-clone step so each subscriber receives its own ParSignedDataSet — matches Go's Subscribe clone-before-fanout at validatorapi.go:249-256. - register_* methods replace any prior registration, matching Go's single-function input semantics. - verify_partial_sig() honours insecure_test, looks up this node's public share from pub_share_by_pubkey, then delegates to pluto_eth2util::signing::verify. Unlike Go — which projects domain / epoch / message-root through the core.Eth2SignedData interface — the Rust hook takes those three values directly so we don't have to port the Eth2SignedData trait in this plumbing PR; submit handlers in PRs 3-6 will derive the triple from their concrete signed-data wrapper. Tests: subscribe fanout clones per subscriber; the six register hooks all overwrite on re-register; unregistered hooks default to None; verify_partial_sig accepts a real BLS signature, rejects a tampered one, rejects an unknown DV pubkey, and short-circuits in insecure_test mode. Co-Authored-By: Bohdan Ohorodnii <35969035+varex83@users.noreply.github.com>
SubscriberFn took ParSignedDataSet by value, so the wrapper closure had
to call set.clone() before invoking the user fn even though the caller
had already cloned to produce the owned argument. The fanout loop that
future submit handlers will run (`for sub in &subs { sub(&duty,
set.clone()).await? }`) would thus pay two HashMap-backed clones per
subscriber where Charon's `Subscribe` (validatorapi.go:249-256) pays one.
Take the set by reference: the wrapper now performs the sole clone
before handing the owned copy to the user closure. Fanout sites pass
`&set` to every subscriber, matching Go's per-subscriber cost.
The existing subscribe_fanouts_clones_to_every_subscriber test (which
exercises the clone-isolation invariant via a mutating first subscriber
and a pristine-set assertion on the second) is updated to the new
call-by-reference pattern and still passes — confirming each subscriber
continues to observe its own copy.
…ck aliases The validatorapi Component callback type aliases (SubscriberFn, AwaitProposalFn, AwaitAggAttestationFn, AwaitSyncContributionFn, AwaitAggSigDBFn, DutyDefFn, PubKeyByAttFn) used a locally-defined `CallbackFuture<'a, T> = Pin<Box<dyn Future<...>>>` alias instead of the workspace-standard `futures::future::BoxFuture`. The codebase already uses BoxFuture in pluto-core (consensus/qbft/transport.rs) and in pluto-dkg / pluto-peerinfo for the same shape. Replace inline with BoxFuture, drop the local CallbackFuture alias, and drop the now-unused `pin::Pin` import. No behaviour change.
Rust naming convention (RFC 430) treats acronyms in PascalCase as single capitalized words: `Db`, not `DB`. The companion error variant in this crate already uses `DutyDbError` (dutydb::Error). `BLSPubKey` is an unrelated upstream type from `pluto_eth2api` and stays as-is.
The tests module had a second `use` block in the middle of the file (line ~1259, just before the plumbing-tests section) for std::sync::Mutex, pluto_crypto/testutil/serde_json, and a re-import of SignedRandao / SyncContribution / VersionedAggregatedAttestation / PubKey from `crate`. Rust style places all `use` declarations at the top of each module. Fold the mid-block imports into the existing top-of-module `use` block. Drop the now-redundant `pluto_eth2util::signing::DomainName` test-mod import — the parent module already brings it in via `use super::*`.
varex83agent
force-pushed
the
bohdan/validatorapi-plumbing
branch
from
f5c3b49 to
8eedb3f
Compare
Collaborator
Author
/loop-review-pr summaryRan 2 review-and-fix iteration(s) against this PR. Pre-loop housekeepingThe branch was rebased onto Quality gates (final)
Resolved during the loopMajor (1)
Minor (3)
Deliberately deferred (documented in source / state file)
OutstandingNone — internal review pipeline (pluto-review / security / rust-style / code-quality, four agents in parallel) returned zero VerdictPR is ideal — all bug/major findings resolved, gates green. |
Removes the "Mirrors Go's foo" / `validatorapi.go:N` / `router.go:N` pointers from doc comments throughout component.rs. The descriptions of what each callback / method / error variant does remain; only the Go-source references are dropped. Internal cross-references (e.g. `upstream_status_error`) and the test-fixture spec note remain — those don't point at the Charon Go codebase.
emlautarom1
approved these changes
Jun 2, 2026
Collaborator
emlautarom1
left a comment
emlautarom1
left a comment
There was a problem hiding this comment.
Beyond verify_partial_sig everything is plumbing so it's hard to get the full picture (and thus deciding what makes sense/what not).
I particularly do not like the callback mechanism where we have non trivial type signatures and we store them into structs. Once we start with the wiring I think thinks will get clearer.
@therustmonk wdyt about this? Any pattern you suggest?
| /// [`ParSignedDataSet`] by reference; the registered wrapper clones the | ||
| /// set exactly once before invoking the user closure so every subscriber | ||
| /// observes an independent copy. | ||
| pub type SubscriberFn = Arc< |
Collaborator
There was a problem hiding this comment.
nit: Callbacks don't need to be Arc, Box should suffice.
| /// `Subscribe` invokes every registered subscriber, each receiving its | ||
| /// own clone of the set. Mutating one clone does not affect the others. | ||
| #[tokio::test] | ||
| async fn subscribe_fanouts_clones_to_every_subscriber() { |
Collaborator
There was a problem hiding this comment.
I'm not sure if we need these kind of test: subscribers get a ParSignedDataSet (which we could make a type alias instead of a separate type), so it's impossible for subscribers to alter each other from the type system perspective.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
unimplemented!()arms are filled in — this PR only exposes the hooks. Mirrors Charon'score/validatorapi/validatorapi.go:196-256and:1352.Componentcallback fields (six total —await_proposal_fn,await_agg_attestation_fn,await_sync_contribution_fn,await_agg_sig_db_fn,duty_def_fn,pub_key_by_att_fn) plus asubs: Vec<SubscriberFn>subscriber list. All stored asOption<Arc<…>>so registration happens before theComponentis shared.subscribe()wraps the user closure with a set-clone step so each subscriber receives its ownParSignedDataSet, mirroring Go's clone-before-fanout atvalidatorapi.go:249-256.register_*methods overwrite on re-register, matching Go's single-function input semantics.verify_partial_sig()honoursinsecure_test, looks up this node's public share via the existingpub_share_by_pubkeymap, then delegates topluto_eth2util::signing::verify. The Rust hook takes(domain_name, epoch, message_root, signature)directly rather than projecting them throughcore.Eth2SignedData— the trait hasn't been ported yet, so submit handlers in PRs 3-6 will derive the triple from their concrete signed-data wrapper.Go reference
charon/core/validatorapi/validatorapi.go)crates/core/src/validatorapi/component.rs)subs []func(...) errorfield,Subscribe:249-259subs: Vec<SubscriberFn>,Component::subscribeawaitProposalFuncfield,RegisterAwaitProposal:208await_proposal_fn,Component::register_await_proposalawaitAggAttFuncfield,RegisterAwaitAggAttestation:238await_agg_attestation_fn,Component::register_await_agg_attestationawaitSyncContributionFuncfield,RegisterAwaitSyncContribution:220await_sync_contribution_fn,Component::register_await_sync_contributionawaitAggSigDBFuncfield,RegisterAwaitAggSigDB:243await_agg_sig_db_fn,Component::register_await_agg_sig_dbdutyDefFuncfield,RegisterGetDutyDefinition:232duty_def_fn,Component::register_get_duty_definitionpubKeyByAttFuncfield,RegisterPubKeyByAttestation:226pub_key_by_att_fn,Component::register_pub_key_by_attestationverifyPartialSig:1352Component::verify_partial_sigOut of scope
core.VerifyEth2SignedDataare not ported here.verify_partial_siginstead takes the BLS domain/epoch/message-root directly. Submit handlers (PRs 3-6) compute those from their concrete signed-data wrapper before calling the hook.Handlertrait signatures are unchanged. No other crates touched.pub_share_by_pubkey: HashMap<BLSPubKey, BLSPubKey>onComponentalready provides the DV-root → public-share lookup that Go'sgetVerifyShareFuncexposes, so nocrates/cluster/change was needed.Test plan
cargo +nightly fmt --all --checkcargo clippy -p pluto-core --all-targets --all-features -- -D warningscargo test -p pluto-core --all-features— 374/374 passing (8 new tests: subscribe fanout-clones, register_await_proposal overwrite, the remaining five register hooks overwrite, unregistered hooks default to None, verify_partial_sig accept/reject valid+tampered, reject unknown pubkey, short-circuit in insecure_test)