refactor: execute PoL in normal transaction pipeline#272
refactor: execute PoL in normal transaction pipeline#272NikhilSharmaWe wants to merge 3 commits into
Conversation
Signed-off-by: Nikhil Sharma <nikhilsharma230303@gmail.com>
Signed-off-by: Nikhil Sharma <nikhilsharma230303@gmail.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughPoL execution now happens in the payload builder as transaction ChangesPoL transaction execution relocation
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
Pull request overview
Refactors Proof-of-Liquidity (PoL) execution so it runs as the first transaction (tx #0) in the normal block transaction pipeline during payload building, eliminating prior special-casing and assembly-time injection workarounds.
Changes:
- Executes PoL as
tx #0during payload building and removes executor-side PoL bypass/skip logic. - Updates block assembly to validate PoL presence at index
0(post-Prague1) instead of injecting it. - Removes the custom block builder wrapper previously used to patch sender/transaction mismatches from PoL injection, and ensures PoL gas-used is zero even on revert/halt.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/node/evm/mod.rs | Removes the now-unneeded builder module wiring. |
| src/node/evm/executor.rs | Drops PoL system-call execution + receipt forging and removes PoL commit-skips/dummy results. |
| src/node/evm/error.rs | Removes an error variant tied to the deleted PoL execution path. |
| src/node/evm/config.rs | Stops wrapping BasicBlockBuilder with BerachainBlockBuilder; returns the basic builder directly. |
| src/node/evm/builder.rs | Deletes the PoL injection sender-fix wrapper and its tests. |
| src/node/evm/assembler.rs | Stops injecting PoL; validates PoL is present at index 0 post-Prague1. |
| src/evm/mod.rs | Forces PoL gas used to 0 even on revert/halt in the PoL execution path. |
| src/engine/builder.rs | Executes PoL as tx #0 during payload building (post-Prague1). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| transaction::{BerachainTxEnvelope, pol::create_pol_transaction}, | ||
| }; | ||
| use alloy_consensus::Transaction; | ||
| use alloy_eips::eip7002::SYSTEM_ADDRESS; |
Signed-off-by: Nikhil Sharma <nikhilsharma230303@gmail.com>
|
@calbera PTAL, thanks |
Closes #64
Summary
PoL was previously executed in
apply_pre_execution_changes, then handled again during import with dummy results and commit skips, and injected into the block during assembly. That required extra workarounds, includingBerachainBlockBuilderandfix_pol_senders(#129).This PR moves PoL into the normal transaction pipeline:
execute_transactionduring payload buildingBerachainBlockBuilderandfix_pol_senderstransact_rawTest plan
cargo buildcargo +nightly clippy --all-targets --all-features -- -D warningscargo nextest run --lockedSummary by CodeRabbit
New Features
Bug Fixes