Skip to content

feat(app): implement simnet integration#553

Open
iamquang95 wants to merge 1 commit into
mainfrom
feat/simnet-integration
Open

feat(app): implement simnet integration#553
iamquang95 wants to merge 1 commit into
mainfrom
feat/simnet-integration

Conversation

@iamquang95

Copy link
Copy Markdown
Collaborator

Implement simnet integration for pluto run

Summary

Enables simnet mode in pluto run: --simnet-beacon-mock and --simnet-validator-mock (plus --simnet-beacon-mock-fuzz, --simnet-slot-duration, --simnet-validator-keys-dir) are now wired instead of failing fast, so a cluster of pluto nodes can run duties end-to-end without a real beacon node or validator client.

  • The run path builds an in-process BeaconMock seeded with the cluster's validators (fuzz mode supported) and a validator mock that signs with this node's key shares and drives the node's own validator API; both are supervised and shut down in order.
  • New per-slot seam (SlotTickFn): the scheduler forwards slot ticks to the validator mock; ticks race the cancellation token so no work outlives shutdown.
  • Consensus propose/participate are bounded by the duty deadline; deadline-calculator errors surface instead of being downgraded to "no deadline".
  • Simnet slot duration is normalized to whole seconds (integer SECONDS_PER_SLOT keeps the mock's head ticker and all consumer clocks aligned) and defaults zero to 1s.
  • CLI validation, Charon parity: --simnet-validator-mock requires --simnet-beacon-mock; --builder-api rejected with the simnet mocks (no blinded proposals yet); --p2p-relays="" means no relays.
  • pluto-testutil becomes a normal dependency of the app crate — the mocks compile into the binary, as in Charon.

Testing

  • tests/simnet.rs: boots the real App::run with both mocks against an on-disk cluster fixture; asserts load, API bind, and clean shutdown (readiness scope only).
  • tests/wiring.rs additions: slot-tick delivery through the scheduler; a 3-node partial-signature-exchange oracle (cross-node partials → SigAgg → beacon submit, through the real wired graph); deadline-error regression.
  • CLI tests for flag threading, validation combos, and error strings.
  • Full gates green: fmt, clippy -D warnings, workspace tests, cargo deny.
  • Verified live: 3-node cluster + local relay completing all duty types (attestations, proposals, aggregations, sync messages/contributions) with first-round consensus; killing one node keeps duties flowing on the surviving 2-of-3; clean SIGINT shutdown with zero errors.

Try yourself

Prep (once, from repo root)

cargo build -p pluto-cli
rm -rf /tmp/pluto-simnet
./target/debug/pluto create cluster \
  --nodes 4 --num-validators 1 --network holesky \
  --cluster-dir /tmp/pluto-simnet/cluster \
  --fee-recipient-addresses 0x000000000000000000000000000000000000dEaD \
  --withdrawal-addresses    0x000000000000000000000000000000000000dEaD \
  --insecure-keys

Terminal R — relay (from repo root)

./target/debug/pluto relay --data-dir /tmp/pluto-simnet/relay \
  --http-address 127.0.0.1:3640 --auto-p2pkey \
  --p2p-tcp-address 127.0.0.1:3641 --p2p-udp-address 127.0.0.1:3643 \
  --monitoring-address 127.0.0.1:3642 --p2p-advertise-private-addresses

Terminals A–D — one node each (from repo root; set i=0 / 1 / 2 / 3)

i=0   # ← change per terminal
./target/debug/pluto run \
  --lock-file        "/tmp/pluto-simnet/cluster/node$i/cluster-lock.json" \
  --private-key-file "/tmp/pluto-simnet/cluster/node$i/charon-enr-private-key" \
  --simnet-beacon-mock --simnet-validator-mock \
  --simnet-validator-keys-dir "/tmp/pluto-simnet/cluster/node$i/validator_keys" \
  --p2p-relays http://127.0.0.1:3640 \
  --p2p-tcp-address       "127.0.0.1:$((3610+i))" \
  --validator-api-address "127.0.0.1:$((3600+i))" \
  --monitoring-address    "127.0.0.1:$((3620+i))" \
  --nickname "node$i" --no-verify

@iamquang95

Copy link
Copy Markdown
Collaborator Author

@claude review this PR

@claude

claude Bot commented Jul 17, 2026

Copy link
Copy Markdown

Claude finished @iamquang95's task in 5m 50s —— View job


Review: simnet integration (PR #553)

  • Read changed files & gather context
  • Compare against Charon Go source (parity)
  • Review correctness, security, style
  • Post review feedback

Overall this is a high-quality, well-tested PR. The wiring is careful, the comments explain the why (not just the what), Charon parity is preserved in the subtle spots, and the test additions genuinely guard the new seams (deleting the subscribe_slot registration or the deadline bound would turn a test red). I verified the key parity points against charon/app/app.go. No blocking issues found — the notes below are confirmations and minor polish.


Correctness — verified, no blockers

1. Deadline-bounding is a real behavior change to the production duty flow, not just simnet.
run_bounded_by_duty_deadline now wraps consensus.propose (fetch stitch) and consensus.participate (consensus stitch) for all duties, in every mode. I traced this against the existing consensus component: its internal deadliner (cons_deadliner/expired_rx, node/mod.rs:366) only calls delete_instance_io — it drops cached instance I/O but does not cancel a running QBFT instance's context (qbft/component.rs:507, runner.rs:245 instance_ct = parent_ct.child_token()). So before this PR, a stuck instance ran until component shutdown; now it's cancelled at the duty deadline, which matches Charon's qbftDeadliner-cancels-context semantics (controller.go:44). This is more correct, and it's disclosed in the PR description — just flagging that reviewers should read it as a core-path change, not a mock-only one. The three deadline tests cover the past-deadline, no-deadline, and calc-error paths well. ✅

2. Charon parity on proposer duties is preserved indirectly. build_simnet_beacon_mock always adds deterministic_proposer_duties, whereas Charon gates it behind if !conf.SyntheticBlockProposals (app.go:952). This is equivalent only because check_unsupported_flags still rejects --synthetic-block-proposals in Pluto, so that condition is always true. Correct today; worth a one-line comment tying the two together so a future person enabling synthetic proposals remembers to gate this. ✅

3. Fuzz precedence matches app.go:913 (SimnetBMockFuzz short-circuits before SimnetBMock, uses WithForkVersion, and the 1s slot-duration default is applied). The CLI requiring an endpoint or --simnet-beacon-mock when only -fuzz is set (run.rs:747) is consistent with the parse_run test harness always injecting an endpoint. ✅

4. bounded_by_deadline timer task is not leaked — the drop_guard on the child token cancels the spawned timer on the happy (decided) path, not just at deadline. Good. ✅

5. vapi_dial loopback rewrite for unspecified bind addresses (0.0.0.0/[::]) is a nice correctness catch. ✅


Minor / optional

a. AppError::Simnet(String) drops the source-error chain. Every construction uses format!("...: {e}"), so the underlying error's source() is lost for downstream matching/logging. Consider a #[from]/#[source] variant (or a couple of typed variants) if any caller ever needs to distinguish causes. Low priority given these are fatal startup errors. Fix this →

b. Key-presence validation does a full BLS sign per validator (build_simnet_validator_mock, signer.sign(pubshare, b"test signing")). It's correct and startup-only, but if Signer exposes (or could expose) a cheap contains(pubkey) lookup over its HashMap, that would express intent more directly and skip the crypto. Purely optional.

c. config.builder_api is threaded into the validator mock but is guaranteed false here — the CLI rejects --builder-api with any simnet beacon mock, and the vmock is only built when simnet_validator_mock (which requires simnet_beacon_mock). Harmless, but a brief comment noting "always false today; kept for when blinded proposals land" would avoid a future reader assuming builder-API is exercised.

d. Nit: simnet.rs available_addr() TOCTOU is already acknowledged in-comment and is acceptable for a single-process test. The 30s/15s timeouts are generous enough for CI.


Tests

Coverage is strong: the 3-node multinode_parsig_exchange_reaches_submission oracle exercises the real cross-node partial→SigAgg→submit path through the wired graph, wiring_delivers_slot_ticks_to_subscriber guards the new slot seam, and the deadline-error regression test asserts consensus is skipped (not just that an error surfaces). The readiness-only scope of simnet.rs is clearly documented and appropriate.

CI: build/test + linter still in progress at review time; all other checks (semver, deny, DKG integration, create-cluster parity) green.
· branch feat/simnet-integration

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