Draft
Conversation
Signed-off-by: Ameziane H. <ameziane.hamlat@consensys.net>
Signed-off-by: Ameziane H. <ameziane.hamlat@consensys.net>
Signed-off-by: Ameziane H. <ameziane.hamlat@consensys.net>
Signed-off-by: Ameziane H. <ameziane.hamlat@consensys.net>
Signed-off-by: Ameziane H. <ameziane.hamlat@consensys.net>
Signed-off-by: Ameziane H. <ameziane.hamlat@consensys.net>
Signed-off-by: Ameziane H. <ameziane.hamlat@consensys.net>
Signed-off-by: Simon Dudley <simon.dudley@consensys.net> # Conflicts: # evm/src/main/java/org/hyperledger/besu/evm/v2/StackArithmetic.java
Signed-off-by: Simon Dudley <simon.dudley@consensys.net>
… infrastructure - Expand StackArithmetic.java with all arithmetic, bitwise, comparison, unary, ternary, stack manipulation and boundary-helper methods from StackMath - Add VarHandle fields (LONG_BE, INT_BE) for zero-allocation byte array access - Add stackHasSpace(int n) to MessageFrame for overflow checking in push/dup ops - Add static OVERFLOW_RESPONSE to AbstractFixedCostOperationV2 - Add UnaryOperationBenchmarkV2 and ImmediateByteOperationBenchmarkV2 base classes for benchmark suites covering unary ops and DUPN/SWAPN/EXCHANGE respectively Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Simon Dudley <simon.dudley@consensys.net>
Adds ~90 V2 operation implementations across all EVM opcode categories (arithmetic, bitwise, comparison, stack manipulation, environment push, memory, storage, data copy/hash/account, control flow, calls, creates) plus 25+ JMH benchmarks and two new benchmark base classes (UnaryOperationBenchmarkV2, ImmediateByteOperationBenchmarkV2). All operations follow the staticOperation(MessageFrame, long[]) pattern and are wired into the runToHaltV2() switch in EVM.java with appropriate fork guards (Constantinople, London, Shanghai, Cancun, Amsterdam, Osaka). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Simon Dudley <simon.dudley@consensys.net>
Expand single-line Javadoc on all 45 public StackArithmetic methods to include @param and @return tags. Fix three HTML entity errors (&, <, <s). Add missing @param to MulModOperationV2.staticOperation and restore /** on ExtCodeHashOperationV2.staticOperation so -Xdoclint:all passes. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Simon Dudley <simon.dudley@consensys.net>
runToHalt gated on evmConfiguration.enableEvmV2() globally, but system call frames (and transaction frames) built without .enableEvmV2(true) on the builder had stackDataV2 == null, causing NPE in StackArithmetic. Fix: make stackDataV2 non-final with lazy init via ensureV2Stack(), called from runToHalt before entering runToHaltV2. No frame builder changes needed. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Simon Dudley <simon.dudley@consensys.net>
The batch migration missed Unit 4 (bitwise binary ops). This adds the 5 missing V2 operation files and wires them into the runToHaltV2 switch (opcodes 0x0B, 0x16–0x18, 0x1A). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Simon Dudley <simon.dudley@consensys.net>
…ad-only V2 ops Add AbstractOperationV2 as a standalone V2 base class that implements Operation directly (no V1 AbstractOperation dependency), enabling future removal of V1 code. Provides protected static helpers (getAccount, getMutableAccount, getOrCreateAccount, getSenderAccount, getStorageValue) callable from both instance and staticOperation() methods. Migrate all 26 V2 operations from extends AbstractOperation to extends AbstractOperationV2 (AbstractFixedCostOperationV2, AbstractCallOperationV2, and AbstractCreateOperationV2 also updated, covering the remaining 61 ops transitively). Fix BALANCE, EXTCODESIZE, EXTCODEHASH, EXTCODECOPY, and SELFBALANCE to use the new getAccount(address, frame) static helper instead of getWorldUpdater().getAccount(), which was incorrectly wrapping cold accounts in UpdateTrackingAccount and adding them to updatedAccounts. This caused empty accounts to be deleted by clearAccountsThatAreEmpty() purely from read operations, producing a different state root and failing EEST benchmark tests with chain header mismatches. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Simon Dudley <simon.dudley@consensys.net>
- AbstractCallOperationV2/AbstractCreateOperationV2: replace frame.stackSize() (reads V1 OperandStack, always 0 in V2 mode) with frame.stackHasItems() (checks V2 stackTopV2). This was causing every CALL, STATICCALL, CALLCODE, DELEGATECALL, CREATE, and CREATE2 to immediately return UNDERFLOW_RESPONSE. - SStoreOperationV2: make 4-arg staticOperation() public so callers can pass an explicit minimumGasRemaining. - EVM.java V2 SSTORE dispatch: pass EIP_1706_MINIMUM (2300L) instead of FRONTIER_MINIMUM (0L); V2 only targets Istanbul+ forks. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Simon Dudley <simon.dudley@consensys.net>
Replace direct frame.getWorldUpdater().get/getOrCreate() calls with the static helpers from AbstractOperationV2: - beneficiary read: getAccount() (no side effects, EIP-7928 tracking) - originator mutable: getMutableAccount() (matches V1 behaviour; returns null for deleted accounts rather than creating a ghost entry) - beneficiary mutable: getOrCreateAccount() (must create if absent to receive balance transfer) Using getOrCreate() for the originator was the likely cause of the two remaining test_selfdestruct_existing failures: it manufactured a phantom zero-balance account and then called decrementBalance on it, producing incorrect world-state. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Simon Dudley <simon.dudley@consensys.net>
gasAvailableForChildCall() in TangerineWhistleGasCalculator mutates the frame by calling frame.decrementRemainingGas(gasCap) internally. V2 was calling it before gas cost checks, so all subsequent checks saw remainingGas - gasCap instead of the full remainingGas. This caused incorrect gas accounting near loop boundaries with cold addresses (e.g. the selfdestruct_existing benchmark's CREATE2→CALL→SELFDESTRUCT loop where every CALL hits a cold address at 2600 gas cost). Fix mirrors V1 AbstractCallOperation: call gasAvailableForChildCall after frame.decrementRemainingGas(cost), so gas checks see the correct remaining gas before the child cap is deducted. This fixes the 2 remaining test_selfdestruct_existing failures, bringing the EEST benchmark suite to 940/940 with --Xevm-go-fast=true. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Simon Dudley <simon.dudley@consensys.net>
Allows running reference tests with the experimental EVM v2 (long[] stack) via: ./gradlew referenceTests -Dtest.evm.v2=true Also forwards test.ethereum.state.eips, test.ethereum.blockchain.eips, and test.ethereum.include to the forked test JVM, which previously were read by Java code but never propagated. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Simon Dudley <simon.dudley@consensys.net>
- Fix TLOAD/TSTORE gas cost from 3 to 100 (EIP-1153 WARM_STORAGE_READ_COST) - Add DifficultyOperationV2 for pre-Paris DIFFICULTY opcode - Gate RETURNDATASIZE/RETURNDATACOPY on enableByzantium in V2 dispatch - Fix SELFBALANCE gate from enableConstantinople to enableIstanbul - Gate DIFFICULTY/PREVRANDAO on enableParis in V2 dispatch - Add EIP-8037 state gas charging to AbstractCallOperationV2, AbstractCreateOperationV2, and SelfDestructOperationV2 Resolves ~1305 of 1524 reference test failures when running with -Dtest.evm.v2=true. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Simon Dudley <simon.dudley@consensys.net>
When s[off+3] is a large positive long (e.g. 0x80000000L), the old check 's[off+3] < 0' does not trigger because the value is positive as a long. Casting it to int then wraps to a negative int, making 'data.size() - offset' overflow to a large positive value, and tuweni's slice() throws IndexOutOfBoundsException. Replace the check with '(s[off+3] >>> 31) != 0' which catches both negative longs and longs > Integer.MAX_VALUE, correctly treating any offset that doesn't fit in a non-negative int as "beyond calldata = zero". Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Simon Dudley <simon.dudley@consensys.net>
- Gate REVERT (0xFD) on enableByzantium in V2 dispatch - REVERT was introduced in EIP-140 (Byzantium) but was dispatched unconditionally, causing Frontier/Homestead reference test failures - Fix SLOTNUM (0x4B) gate from enableOsaka to enableAmsterdam - SLOTNUM is registered in registerAmsterdamOperations, not registerOsakaOperations, causing Osaka reference test failures Resolves 11 remaining reference test failures with -Dtest.evm.v2=true. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Simon Dudley <simon.dudley@consensys.net>
Contributor
Author
|
re: stack underflow/overflow
// PushOperationV2: // DupOperationV2: ★ Insight ─────────────────────────────────────
|
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
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.
PR description
Reference branch of EVM V2 that attempts to migrate the code from https://github.com/ahamlat/besu/tree/optimize-wei-operations into the EVM V2 skeleton structure.
Adds
-Dtest.evm.v2=trueto enable v2 for referenceTests and referenceTestsDevnet../gradlew referenceTests -Dtest.evm.v2=truepassingevmtool --Xevm-v2=true block-test ...for eest benchmarks passingGets in sync on mainnet, still measuring performance.
Not tested with glamsterdam devnets/tests
Key Architectural Insights
The dual-stack trap: V1
OperandStackand V2long[]are completely independent. Any V1 stack method (stackSize(),popStackItem()) returns wrong results in V2 mode. Every V2 operation must exclusively use V2 stack methods (stackTopV2,stackHasItems).The shadow registry problem: V1 uses a fork-aware
OperationRegistry(each fork registers only valid opcodes). V2's staticswitchbypasses this — it must manually replicate every fork gate. Two patterns coexist: inline ternary (enableByzantium ? ... : Invalid) and internal checks instaticOperation(). The inconsistency makes omissions hard to spot.Read-only vs mutable account access:
WorldUpdater.getAccount()wraps inUpdateTrackingAccount, marking as "touched".WorldUpdater.get()is pure read. V1'sAbstractOperation.getAccount(addr, frame)helper does the right thing; V2 operations initially called the mutable path directly.Gas mutation ordering matters:
gasAvailableForChildCall()mutates frame gas internally. Must be called AFTERdecrementRemainingGas(cost), not before. A 1-line reorder fixed 2/940 test failures.0-input operations are the canary: When testing opcode validity per fork, only operations that need zero stack items (STOP, PUSH0, RETURNDATASIZE, SLOTNUM, etc.) produce different outcomes when incorrectly enabled. Stack-consuming operations fail via underflow regardless.
V2 Dispatch Exception Handling
The V2 dispatch loop only catches
OverflowExceptionandUnderflowException— it assumes all operations are crash-proof. Raw JVM exceptions (likeIndexOutOfBoundsException) propagate all the way to the block processor, which rejects the block as invalid. In V1, each operation'sexecute()has its own try-catch and returnsOperationResult— the dispatch never sees raw exceptions.The CALLDATALOAD
IndexOutOfBoundsExceptionexploited this design: when(int)0x80000000Lwraps to negative, tuweni'sslice()throws. Fixed with(s[off + 3] >>> 31) != 0which catches any long whose bit-31+ is set.