feat(app): wire the core node and implement the run command#536
Open
emlautarom1-agent[bot] wants to merge 29 commits into
Open
feat(app): wire the core node and implement the run command#536emlautarom1-agent[bot] wants to merge 29 commits into
emlautarom1-agent[bot] wants to merge 29 commits into
Conversation
Construct and connect the ten core duty-workflow components (scheduler, fetcher, consensus, dutydb, validatorapi, parsigdb, parsigex, sigagg, aggsigdb, broadcaster) into a runnable node — the Rust analog of Charon's app.go Run/wireCoreWorkflow + core.Wire. - node/behaviour.rs: compose parsigex + qbft + peerinfo into CoreBehaviour and build the libp2p Node (modeled on dkg/node.rs). - node/wire.rs: construct and stitch the components, including the three deadlock-critical fetcher back-edges (agg_sig_db -> aggsigdb.wait_for, await_att_data -> dutydb.await_attestation, plus vapi -> aggsigdb); the parsigex exchange is an injectable seam (Charon's TestConfig.ParSigExFunc analog) so the wiring is testable without a real swarm. - node/mod.rs: App::run with idiomatic-tokio lifecycle (JoinSet + ordered shutdown), cluster-lock loading, and the qbft broadcaster<->behaviour construction cycle resolved via OnceLock. - node/config.rs: minimal AppConfig (subset of Charon app.Config). - core/fetcher: re-export GraffitiBuilder so out-of-crate wiring can supply the required builder field. - tests/wiring.rs: Tier 1 single-node wiring test exercising the fetcher back-edges (blocks-then-unblocks deadlock guard) and the full sign path (real-BLS threshold aggregation -> broadcaster -> beacon mock). Minimal-runnable scope (issue #402 part A). Part-B items (tracker/priority/infosync/monitoring/sse/recaster/vmock, eth2-based verifiers, full proto encoders, real deadliners/gater) are marked with inline TODO(#402 part B). Refs: #402
- Unnecessary struct
- Rename vars - Change visibility
- Do not import functions - Extract common code
Adapt the core duty-workflow wiring to the pluto-core / pluto-consensus / pluto-featureset APIs on main: - SigAgg: the aggregate verifier is now an injectable `WireInputs.sigagg_verifier` (`sigagg::VerifyFn`). Production wires the eth2 verifier (`sigagg::new_verifier(eth2_cl)`); the Tier-1 wiring test injects a permissive verifier so it keeps proving the sign-path connectivity (real eth2 verification is exercised in part B). Mirrors Charon's `TestConfig` injection. - Consensus: thread an `Arc<FeatureSet>` into `qbft::Config` and `timer::get_round_timer_func` (featureset dropped its global state); a default set is used for now, sourced from CLI config in part B. - Cargo: add the `pluto-featureset` dependency; drop a duplicate `pluto-p2p` key. - Allow `clippy::too_many_arguments` on `wire_p2p` until its inputs are grouped in part B (relay/priority additions).
Resolve the beacon-derived duty-workflow inputs in `run` and pass them into `wire_core_workflow` via `WireInputs`, matching Charon `app.go:540-556`: - Real duty gater `DutyGater::new(ð2_cl)` (validates duties against the beacon chain) replaces the accept-any-valid-type stub. - Beacon-derived `DutyDeadlineCalculator::from_client` backs every component's deadliner, shared as an `Arc<dyn DeadlineCalculator>` (its `deadline` method is sync, and `Arc<dyn _>` satisfies the trait, so one instance backs all four). - Per-validator `GraffitiBuilder::new` from the new `AppConfig::graffiti` / `graffiti_disable_client_append` fields. - `electra_slot = fork_config[Electra].epoch * slots_per_epoch`, and `fetch_only_comm_idx0` / `compare_attestations` read from the `AppConfig` feature set (`FetchOnlyCommIdx0` / `ChainSplitHalt`). These inputs are injected via `WireInputs` so the Tier-1 test keeps its inert defaults (never-expiring deadlines, default graffiti) and never needs the extra beacon endpoints. `AppConfig` gains `graffiti`, `graffiti_disable_client_append` and `feature_set`.
Wire the validator API callbacks that have no dutydb fallback (Charon `core.Wire`): - `register_await_agg_attestation` / `register_await_sync_contribution` / `register_pub_key_by_attestation` -> the local dutydb (wrapping its results in the `VersionedAggregatedAttestation` / `SyncContribution` newtypes). - `register_get_duty_definition` -> the scheduler handle (type-erased across the callback boundary, downcast to `DutyDefinitionSet` by the component). To supply the scheduler handle, the scheduler is now built before the validator API (its handle is `Clone`); `await_proposal` continues to rely on the component's built-in dutydb fallback.
Wire `parsigex::new_eth2_verifier` into the P2P behaviour, replacing the always-accept stub. Inbound partial signatures are now verified against the sender's public share for the duty (Charon `parsigex.NewEth2Verifier`). `wire_p2p` takes the beacon client and a full public-share map (`DV pubkey -> 1-based share index -> public share`), built by the new `build_pub_shares_by_key` from every distributed validator's `pub_shares` (Part A only extracted this node's own share).
Verify the cluster lock's hashes and signatures during `run`, gated on `config.no_verify` (Charon `app.Run`): - `lock.verify_hashes()` checks the lock/definition hashes. - `lock.verify_signatures(ð1)` checks the BLS aggregate, node and operator signatures. Operator-signature verification uses an execution-layer client built from the new `AppConfig::eth1_endpoint`; when unset, a no-op eth1 client is used (EIP-1271 smart-contract operator signatures are then not checked). Adds the `pluto-eth1wrap` dependency. Manifest-file precedence is left as a TODO until pluto-cluster gains a manifest loader.
…ODOs The broadcaster already uses a separate submission client with the distinct `beacon_node_submit_timeout`, so drop the stale TODO that implied otherwise. Sharpen the two remaining TODOs to name the core-crate work they require, since neither can be wired against the current APIs: - Multi-`beacon_node_addrs` failover needs a multi-endpoint client in pluto-eth2api (`EthBeaconNodeApiClient` is single-endpoint). - Honoring the target gas limit needs a target-gas-limit parameter on `validatorapi::Component::new` (Charon passes it to `NewComponent`).
Replace the local attester-only proposal encoder with `pluto_core::unsigneddata::unsigned_data_set_to_proto` (all four duty types, SSZ-canonical, from #508). Together with the matching all-duty-type decode on the dutydb subscriber, proposer / sync-committee / aggregator duties now flow through `consensus.propose` end to end, not just attester duties.
…anups Complete the runnable-node wiring (#402 Part A): - Wire `privkeylock::Service` into `run`/`run_lifecycle`, gated on the new `AppConfig::priv_key_locking` flag: the run loop is spawned as a lifecycle task and `close()` is invoked at the start of shutdown so the `<priv_key_file>.lock` sentinel is removed before the task drain. Mirrors Charon `app.go:161`. - Extend the Tier-1 wiring test with the proposer and sync-committee sign paths (ParSigDB -> threshold -> SigAgg -> broadcaster -> beacon submit) and a negative test proving the real eth2 SigAgg verifier rejects a bad partial signature before broadcast — previously only the attester path was covered. - Error on an invalid/empty beacon-node URL instead of silently defaulting to `http://127.0.0.1:5052`. - Drop the dead `duty_gater` parameter from `wire_core_workflow`; the gater is still used by `wire_p2p` and consensus, just not the in-process core wiring.
- Remove invalid tasks
wire_core_workflow now builds one pubkey-scoped ValidatorCache and seeds it into the scheduler's beacon client and the submission client, reusing the same instance for the validator API (Charon app.go:481-482, 598). Previously both clients kept their default empty-pubkey cache, so a real run resolved duties against an empty (or unfiltered) validator set and scheduled nothing. The new wiring test serves POST /states/head/validators filtered by the request-body ids, which fails against an unseeded cache.
…aron - verify_lock now always runs hash + signature verification and only downgrades failures to warnings under no_verify, matching post-#4130 Charon (cluster/load.go) instead of skipping verification entirely. - parsigex broadcast failures surface as AppError::ParSigEx instead of being mislabeled as beacon-client errors. - drop the duplicate verify_p2p_key call in wire_p2p; run verifies once, matching Charon's single VerifyP2PKey in Run.
Mirrors Charon v1.7.1 wireP2P (app.go:341-400): CoreBehaviour gains the relay client transport, RelayManager (reservation lifecycle + relay-circuit peer routing, Charon's NewRelayReserver + NewRelayRouter) and ForceDirectBehaviour (Charon's ForceDirectConnections). Relay endpoints resolve in the background via bootnode::new_relays with the lock-hash-hex Charon-Cluster header, and the connection gater now admits the resolved relays alongside cluster peers. Without this the node was listen-only: lock ENRs carry no addresses, so relay routing is the only peer-discovery path. The multiaddr->URL resolution helper moves from pluto-dkg into pluto-p2p::bootnode (with a unit test) so the app and DKG share it. Post-#4130's Cluster-Uuid relay header remains a follow-up.
Replaces the inline load_lock/verify_lock/verify_lock_signatures trio with cluster::load::load_cluster_lock (#523), which mirrors Charon's cluster.LoadClusterLock (post-#4130): read + JSON-decode the lock, then verify hashes and signatures, downgrading failures to warnings under no_verify. The execution-layer client is now built once in run, matching Charon's app.Run ordering (app.go:177-178).
Replaces the unimplemented run_workflow stub with a RunConfig -> AppConfig mapping and App::new(config).run(ct). Correctness-affecting flags with no implementation yet (simnet mocks, custom testnets, beacon-node headers, VC TLS, consensus protocol preference, synthetic proposals, p2p fuzz) fail fast with a 'not yet supported' error; observability/availability-only flags (monitoring/debug addresses, OTLP, proc directory, fallback beacon endpoints) are ignored with a warning. An existing cluster manifest file is warned about and ignored, matching post-#4130 lock-only semantics. Feature flags resolve like Charon's featureset.Init: unknown min status is a hard error, unknown feature names are warned and skipped.
Replaces the stub-panics test with coverage of the bridge seam: field mapping (incl. hostname validator-api addresses and empty-to-None graffiti/eth1), fail-fast rejection of each unsupported flag group, warn-and-continue for observability flags and existing manifests, Charon-verbatim featureset errors, and an end-to-end run() that reaches App::run and fails cleanly on a missing cluster lock.
run_lifecycle logged a validator-API bind or serve failure and returned Ok(()), so the process exited 0 with the node silently degraded. Charon fails the run instead: any lifecycle start-hook error triggers shutdown and is returned from Manager.Run, and httpServeHook swallows only http.ErrServerClosed (graceful shutdown). The validator API server now runs as a fallible JoinSet task via serve_validator_api (graceful shutdown stays a clean exit), and the supervisor propagates the first failed task's error - constructing the previously-unused AppError::ValidatorApi variant.
![emlautarom1-agent[bot]](https://avatars.githubusercontent.com/u/31224949?s=60&v=4){kind=small}
Comment on lines
+61
to
+66
| /// Default `--monitoring-address`; shared by the flag definition and the | ||
| /// ignored-flag warning so they cannot drift. | ||
| const DEFAULT_MONITORING_ADDR: &str = "127.0.0.1:3620"; | ||
| /// Default `--simnet-validator-keys-dir`; shared by the flag definition and | ||
| /// the unsupported-flag check so they cannot drift. | ||
| const DEFAULT_SIMNET_KEYS_DIR: &str = ".charon/validator_keys"; |
Contributor
Author
There was a problem hiding this comment.
Deletion: two clap default_value string literals. The values are unchanged — they moved into consts shared by the flag definitions and the new unsupported/ignored-flag comparisons below, so a default and its check cannot drift apart. (The struct-level #[allow(dead_code)] on RunConfig/FeatureConfig/TestnetConfig is also gone because the bridge finally consumes the fields; only four genuinely-inert Charon-parity fields keep a field-level allow.)
| /// silently ignoring any of these would change duty or operator-facing | ||
| /// behavior (e.g. `--simnet-beacon-mock` permits empty beacon endpoints, which | ||
| /// the real beacon client cannot handle). | ||
| fn check_unsupported_flags(config: &RunConfig) -> Result<()> { |
Contributor
Author
There was a problem hiding this comment.
Policy: fail fast on correctness-affecting flags. Silently ignoring any of these would change duty or operator-facing behavior. The --simnet-beacon-mock rejection also closes a trap: it is the only way beacon_node_addrs can legally be empty (the CLI validation allows it), and without this check the app would build a beacon client from an empty URL and fail much later with an opaque parse error.
| let feature_set = build_feature_set(&config.feature)?; | ||
| let validator_api_addr = parse_validator_api_addr(&config.validator_api_addr)?; | ||
|
|
||
| // Exhaustive destructure: adding a `RunConfig` field without deciding its |
Contributor
Author
There was a problem hiding this comment.
Compile-time guard. The exhaustive destructure means adding a RunConfig field without deciding its bridge behavior (map / fail-fast / warn) is a compile error instead of a silently dropped flag.
Collaborator
There was a problem hiding this comment.
We'll eventually drop this check, but for now it's fine to have.
| // Post-#4130 Charon reads only the lock file and silently ignores an | ||
| // existing manifest; warn instead so migrated node directories cannot | ||
| // diverge unnoticed. | ||
| if Path::new(&config.manifest_file).exists() { |
Contributor
Author
There was a problem hiding this comment.
Deliberate deviation from post-#4130 Charon. After #4130, Charon reads only the lock file and silently ignores an existing cluster-manifest.pb (the flag survives as deprecated). We warn instead of staying silent so a node directory migrated from pre-#4130 Charon — where the manifest took precedence and may encode cluster edits — cannot diverge unnoticed. Warn-not-fail keeps every directory that runs on post-#4130 Charon runnable here.
| Ok((node, handlers)) | ||
| } | ||
|
|
||
| fn relay_addrs_for_resolution(relays: &[Multiaddr]) -> Vec<String> { |
Contributor
Author
There was a problem hiding this comment.
Moved, not deleted. relay_addrs_for_resolution/relay_addr_for_resolution now live byte-identical (and pub) in pluto_p2p::bootnode, so the new app node and the DKG node share one relay-resolution path. The DKG call site switched to bootnode::relay_addrs_for_resolution; DKG behavior is unchanged.
Comment on lines
+528
to
+543
| /// Serves the validator API until `ct` fires (graceful shutdown, `Ok`). A bind | ||
| /// or serve failure is returned and fails the run, matching Charon's | ||
| /// `httpServeHook`, which swallows only `http.ErrServerClosed`. | ||
| async fn serve_validator_api( | ||
| addr: std::net::SocketAddr, | ||
| router: axum::Router, | ||
| ct: CancellationToken, | ||
| ) -> Result<(), AppError> { | ||
| let listener = tokio::net::TcpListener::bind(addr) | ||
| .await | ||
| .map_err(AppError::ValidatorApi)?; | ||
|
|
||
| axum::serve(listener, router) | ||
| .with_graceful_shutdown(async move { ct.cancelled().await }) | ||
| .await | ||
| .map_err(AppError::ValidatorApi) |
Contributor
Author
There was a problem hiding this comment.
Charon lifecycle parity. A bind or serve failure returns an error, and the task supervisor above propagates the first failed task's error out of run — matching Charon, where any lifecycle start-hook error is returned from Manager.Run and httpServeHook swallows only http.ErrServerClosed. Graceful shutdown (ct fired) is the clean-exit equivalent of that carve-out. Without this, a validator-API bind failure logged an error and the process exited 0.
Comment on lines
+226
to
+232
| let validator_cache = ValidatorCache::new(eth2_cl.clone(), eth2_pubkeys); | ||
| beacon_client | ||
| .set_validator_cache(validator_cache.clone()) | ||
| .await; | ||
| submission_client | ||
| .set_validator_cache(validator_cache.clone()) | ||
| .await; |
Contributor
Author
There was a problem hiding this comment.
Load-bearing. Both beacon clients must share the single pubkey-scoped validator cache (Charon app.go:481-482 for the general client, app.go:598 for the submission client). Without this seeding, duty resolution queries POST /states/head/validators with ids: [], which per the beacon API returns the entire network's validator set (or zero on non-conforming clients) — either way the node schedules wrong duties or none. The regression test drives the scheduler path against a filtering beaconmock.
emlautarom1
approved these changes
Jul 8, 2026
emlautarom1
left a comment
emlautarom1
left a comment
Collaborator
There was a problem hiding this comment.
Code was manually re-reviewed and seems OK. There might be some excessive AI comments in some places but I'd rather keep them until we complete all of the associated tickets (this is the minimal work).
Verified also with Fable 5 in ultracode.
iamquang95
reviewed
Jul 8, 2026
| // Self-spawning actor: consensus expired-duty pruner. | ||
| let _consensus_task = consensus.start(ct.clone()); | ||
|
|
||
| let mut tasks: JoinSet<Result<(), AppError>> = JoinSet::new(); |
Collaborator
There was a problem hiding this comment.
This should watch the lifecycle of scheduler as well
iamquang95
reviewed
Jul 8, 2026
| // balancing — a pending follow-up in pluto-p2p's `new_relays`. | ||
| let relay_addrs = bootnode::relay_addrs_for_resolution(&p2p_config.relays); | ||
| let relays = | ||
| bootnode::new_relays(cancellation.clone(), &relay_addrs, &hex::encode(&lock_hash)).await?; |
Collaborator
There was a problem hiding this comment.
lockHashHex := Hex7(cluster.GetInitialMutationHash()) instead of &hex::encode(&lock_hash)
iamquang95
reviewed
Jul 8, 2026
| let fee_recipient = fee_recipients | ||
| .get(i) | ||
| .and_then(|s| parse_execution_address(s)) | ||
| .unwrap_or_default(); |
Collaborator
There was a problem hiding this comment.
Is this expected to have default here? It will return 0x0, or this is better to return an error?
iamquang95
reviewed
Jul 10, 2026
| if let Some(svc) = &priv_key_lock { | ||
| let svc = Arc::clone(svc); | ||
| tasks.spawn(async move { | ||
| let _ = svc.run().await; |
Collaborator
There was a problem hiding this comment.
Should not discard this error, Charon propagates this as an error
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
2 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.
Makes Pluto a runnable distributed-validator node: it ports the core of Charon's
app.Run/wireCoreWorkflow(v1.7.1 semantics, including the #4130 manifest-removal backport) and bridges the existingpluto runflag surface into it. After this PR,pluto runlaunches a node that discovers and dials its cluster peers via the configured relays and executes attester, aggregator, and sync-committee duties end-to-end in a real cluster.Related issues
Part of #402 (the
app/app.goport tracking issue). The remaining parity tail is broken out into granular follow-up issues — see Next steps below.Context
Everything was reviewed semantically against Charon v1.7.1 plus PR #4130. Three structural divergences are deliberate and behavior-neutral:
JoinSetsupervised against a sharedCancellationToken)CoreBehaviourinstead of Charon's registered services)core.WireOptionlayer (wrappers like tracking/retry are inlined at the wire seams instead).Two decisions worth calling out:
runflags fail fast or warn — never silently drop. Correctness-affecting flags with no implementation yet (simnet mocks, custom testnets, beacon-node headers, VC TLS, a preferred consensus protocol, synthetic proposals, p2p fuzz) abort startup withflag '--x' is not yet supported by pluto run; observability/availability-only flags (monitoring/debug addresses, OTLP, proc directory, fallback beacon endpoints) log a warning and continue. An existing cluster manifest file is warned about and ignored — post-#4130 Charon silently ignores it, so warning is strictly safer for migrated node directories.What's implemented
core.Wirestitches (hand-verified againstcore/interfaces.go), with all four duty types flowing throughconsensus.proposevia the core proto codec.cluster::load::load_cluster_lock(post-#4130no_verify= warn-not-skip), the eth1 client built once inrun, the private-key lock, a singleverify_p2p_key.app.go:481-482/:598) — without this a node resolves duties against the whole network's validator set (or none at all).wireP2Pparity: relay client +RelayManager+ force-direct upgrades + relay-aware connection gater; lock-hashCharon-Clusterheader on relay resolution.RunConfig → AppConfig → App::run, with Charon-parity featureset resolution (unknown min status is a hard error with Charon's exact message; unknown feature names warn and are ignored) and hostname-capable validator-API address parsing.run-command tests (flag surface, defaults, bridge mapping, fail-fast/warn policy, an end-to-endrun()that reaches the node and fails cleanly on a missing lock), plus unit coverage for the featureset parsing and validator-API serving.Out of scope
None of the pending items is a hard requirement for a working
run: the node starts, meshes over relays, and completes attester/aggregator/sync duties without them. Per-item justification (each verified against Charon v1.7.1 behavior during the porting review):runsetFeeRecipient)GnosisBlockHotfixauto-enable--testnet-*)LaggedhardeningNext steps
Filed as granular issues so they can proceed in parallel: #527 (eth2api
submit_proposal_preparations) followed by #528 (fee-recipient subscriber) is the only dependent chain; #529, #530, #531, #532, #533 are independent correctness items; #534 + #535 are the resilience pair (retry wrappers restore Charon's async duty dispatch, which also makes theLaggedfailure mode far less reachable).