-
Notifications
You must be signed in to change notification settings - Fork 47
Prover tests for empty blocks #2443
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: arith-dev
Are you sure you want to change the base?
Conversation
Bug: Transaction List Nulls Cause Block Number ErrorsThe |
Bug: Block Number Calculation Incorrect for Non-Standard Genesis BlocksWhen |
Bug: Null Transaction Handling MismatchThe |
Bug: Test Initialization Order Causes Null ReferencesInstance fields (senderKeyPair, receivingAccount, storingNumber, logging, storing, reading) are initialized with references to chainConfig, but chainConfig is only initialized by the test framework after instance field initialization. This will cause these fields to use a null or uninitialized chainConfig, leading to NullPointerException or incorrect bytecode compilation. These fields should either be initialized lazily in a @beforeeach method or moved into the test methods themselves. |
testing/src/main/java/net/consensys/linea/testing/MultiBlockExecutionEnvironment.java
Outdated
Show resolved
Hide resolved
arithmetization/src/test/java/net/consensys/linea/zktracer/EmptyBlockTests.java
Outdated
Show resolved
Hide resolved
testing/src/main/java/net/consensys/linea/testing/MultiBlockExecutionEnvironment.java
Outdated
Show resolved
Hide resolved
arithmetization/src/test/java/net/consensys/linea/zktracer/EmptyBlockTests.java
Show resolved
Hide resolved
|
Thanks for fixing the daily ref tests !! |
| String txHash = | ||
| besuNode.execute(ethTransactions.sendRawTransaction(tx.encoded().toHexString())); | ||
| txHashes.add(txHash); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: NullPointer on Null Transactions in Else Branch
NullPointerException when oneTxPerBlock is false and transactions list contains null values. The else branch (lines 216-223) attempts to call .encoded() on txs.next() without checking if the transaction is null first. When a null transaction is encountered (representing an empty block), this will throw a NullPointerException. The if branch (lines 207-214) correctly handles null transactions with a null check, but the else branch is missing this protection.
testing/src/main/java/net/consensys/linea/testing/MultiBlockExecutionEnvironment.java
Outdated
Show resolved
Hide resolved
| null); | ||
| besuExecTools.executeTest(); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Early Exit Prevents Tracer Initialization
When runWithBesuNode is true, the run() method returns early without initializing the tracer field, leaving it null. Tests that call getHub() after running with Besu node will encounter a NullPointerException since getHub() returns tracer.getHub() and tracer is null.
testing/src/main/java/net/consensys/linea/testing/MultiBlockExecutionEnvironment.java
Show resolved
Hide resolved
arithmetization/src/test/java/net/consensys/linea/zktracer/EmptyBlockTests.java
Outdated
Show resolved
Hide resolved
various configurations of empty / nonempty blocks, in particular E EEEE EEEN NEEN NEEE ENNE EENENE where E = empty block, N = nonempty block
testing/src/main/java/net/consensys/linea/testing/MultiBlockExecutionEnvironment.java
Show resolved
Hide resolved
419045b to
7a66335
Compare
7a66335 to
5342cec
Compare
…erized with and without besu nodes
| for (State.HubTransactionState state : hub.getState().getAll()) { | ||
| for (TraceSection section : state.traceSections().trace()) { | ||
| if (section instanceof LogSection) { | ||
| nbOfLog += 1; |
Check failure
Code scanning / CodeQL
Implicit narrowing conversion in compound assignment High test
short
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 5 days ago
The best way to fix this problem is to widen the type of nbOfLog from short to int. This prevents any implicit narrowing conversion when performing compound assignment operations like nbOfLog += 1;. The code's logic and surrounding assumptions are not altered: the only change is the variable's type. You should update the declaration at line 180 from short to int. No additional imports or refactoring are required, and no other code changes are necessary.
-
Copy modified line R180
| @@ -177,7 +177,7 @@ | ||
|
|
||
| if (!runWithBesu) { | ||
| final State hub = env.getHub().state(); | ||
| short nbOfLog = 0; | ||
| int nbOfLog = 0; | ||
| for (State.HubTransactionState state : hub.getState().getAll()) { | ||
| for (TraceSection section : state.traceSections().trace()) { | ||
| if (section instanceof LogSection) { |
| long startBlockNumber = Collections.min(blockNumbers); | ||
| long endBlockNumber = Collections.max(blockNumbers); | ||
| TraceFile traceFile = traceAndCheckTracer(startBlockNumber, endBlockNumber, currentFork); | ||
| checkState(blockNumbers.isEmpty() == allTransactionsAreNull); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Assertion failure for mixed empty blocks
The assertion checkState(blockNumbers.isEmpty() == allTransactionsAreNull) fails when processing an empty block in a test suite containing a mix of empty and non-empty blocks. For an empty block, blockNumbers is empty (no receipts), but allTransactionsAreNull is false, causing an IllegalStateException.
testing/src/main/java/net/consensys/linea/testing/BesuExecutionTools.java
Show resolved
Hide resolved
| long startBlockNumber = Collections.min(blockNumbers); | ||
| long endBlockNumber = Collections.max(blockNumbers); | ||
| TraceFile traceFile = traceAndCheckTracer(startBlockNumber, endBlockNumber, currentFork); | ||
| checkState(blockNumbers.isEmpty() == allTransactionsAreNull); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Invalid assertion for mixed empty blocks
The checkState condition enforces that empty blocks (empty blockNumbers) can only occur if every transaction in the test is null. This causes an IllegalStateException when running tests with a mix of empty and non-empty blocks, as processing a single empty block results in blockNumbers being empty while allTransactionsAreNull is false.
| long firstBlockNumber = blockNumbers.isEmpty() ? 1 : Collections.min(blockNumbers); | ||
| long finalBlockNumber = | ||
| blockNumbers.isEmpty() ? transactions.size() : Collections.max(blockNumbers); | ||
| TraceFile traceFile = traceAndCheckTracer(firstBlockNumber, finalBlockNumber, currentFork); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Trace generation requests non-existent blocks
When handling an empty block, finalBlockNumber defaults to the total number of transactions (transactions.size()). In sequential execution, this triggers a trace request for the entire planned block range before subsequent blocks have been mined, which will cause RPC errors by requesting blocks that do not yet exist.
| long startBlockNumber = Collections.min(blockNumbers); | ||
| long endBlockNumber = Collections.max(blockNumbers); | ||
| TraceFile traceFile = traceAndCheckTracer(startBlockNumber, endBlockNumber, currentFork); | ||
| checkState(blockNumbers.isEmpty() == allTransactionsAreNull); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Null transaction causes NullPointerException when oneTxPerBlock false
When oneTxPerBlock is false, the code calls txs.next().encoded() without checking if the transaction is null. Null transactions represent empty blocks in this implementation, so calling .encoded() on null will throw a NullPointerException. The null check added for the oneTxPerBlock == true case is missing here.
Note
Adds parameterized empty-block test scenarios and enables Besu-based execution/tracing with null-transaction placeholders, plus a minor Osaka import fix.
EmptyBlockTeststo run with/without Besu across multiple empty/non-empty sequences (EENENE,NEEN,NEEE,ENNE,EEEE,EEEN,E).builderFromBlockTypeList(...); conditionally validate hub/logs only when not using Besu.MultiBlockExecutionEnvironmentgainsrunWithBesuNode; when enabled, flattens blocks into a per-block stream includingnullfor empty blocks and executes viaBesuExecutionTools.BesuExecutionToolsacceptsnulltransactions to represent empty blocks, requires at least one entry, computes trace start/end when all are empty, and aligns proof requests accordingly.TraceOsaka.EIP_7825_TRANSACTION_GAS_LIMIT_CAPinOsakaUserTransaction.Written by Cursor Bugbot for commit dfadcae. This will update automatically on new commits. Configure here.