feat: add proposer selection - #228
Open
tac0turtle wants to merge 11 commits into
Open
Conversation
📝 WalkthroughWalkthroughAdds an optional proposer-control precompile with chainspec configuration, activation-height handling, admin-authorized rotation, proposer JSON-RPC access, node registration, integration tests, and documentation. ChangesProposer Rotation Precompile
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
randygrok
marked this pull request as ready for review
June 8, 2026 09:04
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@bin/ev-dev/src/main.rs`:
- Around line 305-315: The TUI startup path fails to register the proposer RPC,
so evolve_getNextProposer is unavailable when run_with_tui is used; fix by
creating and merging the proposer API in the TUI branch the same way as the
non-TUI branch: construct EvolvePayloadBuilderConfig (or reuse proposer_cfg),
derive initial_next_proposer, instantiate EvolveProposerApiImpl (proposer_api)
with ctx.provider() and the initial value, then call
ctx.modules.merge_configured(proposer_api.into_rpc()) alongside merging
evolve_txpool in the run_with_tui code path so the proposer RPC is registered
for TUI mode as well.
In `@crates/ev-precompiles/README.md`:
- Around line 202-204: The fenced code block containing the address
0x000000000000000000000000000000000000f101 should include a language tag to
satisfy MD040; update the fence opening from ``` to ```text so the block is
fenced as text (locate the fenced block containing the address string and add
the language tag).
- Around line 208-252: The README docs and example ABI need to use bytes32 for
proposer values to match the precompile: update the IProposerControl interface
declaration (replace address with bytes32 for nextProposer() and
setNextProposer(bytes32)), change the JSON config keys (proposerControlAdmin
stays address but initialNextProposer must be a bytes32 value), and update all
example CLI commands and cast calls to pass/expect a bytes32 (and the
corresponding calldata/signature) instead of an address; verify references to
nextProposer and setNextProposer in the prose reflect bytes32 semantics to avoid
mismatched calldata with crates/ev-precompiles/src/proposer.rs.
In `@docs/adr/ADR-0004-proposer-rotation-precompile.md`:
- Around line 93-99: The ADR currently declares proposer identity as address (in
the IProposerControl interface and related prose) but the implementation uses
bytes32 proposer IDs; update the ADR to use bytes32 for nextProposer() and
setNextProposer(bytes32 proposer) in the IProposerControl symbol references and
change all prose/semantics that mention “zero address” to reference the bytes32
zero value (bytes32::ZERO) instead; apply the same change to the other affected
sections noted (around the prose covering lines 134-156 and 166-167) so the API
signatures and semantics match the implemented bytes32 proposer identity while
leaving admin() as an address if implementation uses address for admin.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 74e3a819-0c8e-4041-a931-d5e6a76d39e8
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (15)
bin/ev-dev/src/main.rsbin/ev-reth/src/main.rscrates/ev-precompiles/README.mdcrates/ev-precompiles/src/lib.rscrates/ev-precompiles/src/proposer.rscrates/ev-revm/src/factory.rscrates/ev-revm/src/lib.rscrates/node/Cargo.tomlcrates/node/src/config.rscrates/node/src/executor.rscrates/node/src/lib.rscrates/node/src/payload_service.rscrates/node/src/proposer_rpc.rscrates/tests/src/common.rsdocs/adr/ADR-0004-proposer-rotation-precompile.md
Comment on lines
305
to
315
| let proposer_cfg = | ||
| EvolvePayloadBuilderConfig::from_chain_spec(ctx.config().chain.as_ref())?; | ||
| let initial_next_proposer = proposer_cfg | ||
| .proposer_control_precompile_settings() | ||
| .map(|(_, _, initial_next_proposer)| initial_next_proposer) | ||
| .unwrap_or_default(); | ||
| let proposer_api = | ||
| EvolveProposerApiImpl::new(ctx.provider().clone(), initial_next_proposer); | ||
| ctx.modules.merge_configured(evolve_txpool.into_rpc())?; | ||
| ctx.modules.merge_configured(proposer_api.into_rpc())?; | ||
| Ok(()) |
There was a problem hiding this comment.
Proposer RPC is not registered in TUI mode.
You register proposer RPC in the non-TUI startup path, but Line 380-385 (run_with_tui) still only merges txpool. This makes evolve_getNextProposer unavailable whenever --tui is used.
Suggested fix
@@
let _handle = builder
.node(EvolveNode::new())
.extend_rpc_modules(move |ctx| {
let evolve_cfg = EvolveConfig::default();
let evolve_txpool =
EvolveTxpoolApiImpl::new(ctx.pool().clone(), evolve_cfg.max_txpool_bytes);
+ let proposer_cfg =
+ EvolvePayloadBuilderConfig::from_chain_spec(ctx.config().chain.as_ref())?;
+ let initial_next_proposer = proposer_cfg
+ .proposer_control_precompile_settings()
+ .map(|(_, _, initial_next_proposer)| initial_next_proposer)
+ .unwrap_or_default();
+ let proposer_api =
+ EvolveProposerApiImpl::new(ctx.provider().clone(), initial_next_proposer);
ctx.modules.merge_configured(evolve_txpool.into_rpc())?;
+ ctx.modules.merge_configured(proposer_api.into_rpc())?;
Ok(())
})
.launch_with_debug_capabilities()
.await?;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@bin/ev-dev/src/main.rs` around lines 305 - 315, The TUI startup path fails to
register the proposer RPC, so evolve_getNextProposer is unavailable when
run_with_tui is used; fix by creating and merging the proposer API in the TUI
branch the same way as the non-TUI branch: construct EvolvePayloadBuilderConfig
(or reuse proposer_cfg), derive initial_next_proposer, instantiate
EvolveProposerApiImpl (proposer_api) with ctx.provider() and the initial value,
then call ctx.modules.merge_configured(proposer_api.into_rpc()) alongside
merging evolve_txpool in the run_with_tui code path so the proposer RPC is
registered for TUI mode as well.
…oser # Conflicts: # bin/ev-dev/src/main.rs
- Fix README/ADR interface and cast examples to the implemented setNextProposer(bytes32)/nextProposer() -> bytes32 ABI; the documented address-typed selectors would halt in the precompile. Document why the value is an opaque 32-byte word and that only zero is rejected. - Fix config examples: initialNextProposer is a B256, the 20-byte example value would fail chainspec parsing. - Register evolve_getNextProposer only when proposerControlAdmin is configured, so unconfigured chains get method-not-found instead of a meaningless zero. - Document the pre-activation initialNextProposer fallback semantics of the RPC in the ADR and at the read site. - Mark ADR-0004 as accepted/implemented. Co-Authored-By: Claude Fable 5 <[email protected]>
There was a problem hiding this comment.
🧹 Nitpick comments (2)
docs/adr/ADR-0004-proposer-rotation-precompile.md (1)
302-304: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd a static-call rejection test.
setNextProposerrejects state changes during static execution. Add this behavior to the required test coverage and verify that the call halts without changing storage.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/adr/ADR-0004-proposer-rotation-precompile.md` around lines 302 - 304, Add a required test covering static execution of setNextProposer, verifying the call is rejected and storage remains unchanged after the attempted state change.crates/tests/src/e2e_tests.rs (1)
2232-2240: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for a non-zero activation height.
The test always passes
Noneas the activation height, so the precompile activates at genesis. The delayed-activation path stays untested, and that path decides whether the precompile is installed at a given block height. Add a case withSome(height)that assertssetNextProposerfails before the height and succeeds after it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/tests/src/e2e_tests.rs` around lines 2232 - 2240, Extend the test around create_test_chain_spec_with_proposer_control and proposer_control_precompile_settings to use a non-zero activation height via Some(height), then exercise setNextProposer at a block before the height and assert failure, followed by a block at or after the height and assert success. Preserve the existing chainspec settings assertion while covering delayed precompile installation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@crates/tests/src/e2e_tests.rs`:
- Around line 2232-2240: Extend the test around
create_test_chain_spec_with_proposer_control and
proposer_control_precompile_settings to use a non-zero activation height via
Some(height), then exercise setNextProposer at a block before the height and
assert failure, followed by a block at or after the height and assert success.
Preserve the existing chainspec settings assertion while covering delayed
precompile installation.
In `@docs/adr/ADR-0004-proposer-rotation-precompile.md`:
- Around line 302-304: Add a required test covering static execution of
setNextProposer, verifying the call is rejected and storage remains unchanged
after the attempted state change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 6b849d8d-c992-4aac-a859-0d84ff725f00
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (10)
bin/ev-reth/src/main.rscrates/ev-precompiles/README.mdcrates/node/Cargo.tomlcrates/node/src/config.rscrates/node/src/lib.rscrates/node/src/node.rscrates/node/src/proposer_rpc.rscrates/tests/src/common.rscrates/tests/src/e2e_tests.rsdocs/adr/ADR-0004-proposer-rotation-precompile.md
🚧 Files skipped from review as they are similar to previous changes (5)
- crates/node/src/lib.rs
- crates/ev-precompiles/README.md
- crates/node/Cargo.toml
- crates/node/src/proposer_rpc.rs
- crates/node/src/config.rs
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.
Description
this pr adds proposer selection to ev-reth based on evstack/ev-node#3282
Type of Change
Related Issues
Fixes #(issue)
Checklist
Testing
Additional Notes
Summary by CodeRabbit
New Features
Documentation
Tests