-
Notifications
You must be signed in to change notification settings - Fork 46
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
various configurations of empty / nonempty blocks, in particular E EEEE EEEN NEEN NEEE ENNE EENENE where E = empty block, N = nonempty block
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. |
| BesuExecutionTools besuExecTools = | ||
| new BesuExecutionTools( | ||
| Optional.of(testInfo), | ||
| MAINNET_TESTCONFIG(CANCUN), // TODO: make configurable ? |
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.
You can use the variable testsChain, it's the ChainConfig set in this class when we build the MultiBlockEnv
arithmetization/src/test/java/net/consensys/linea/zktracer/EmptyBlockTests.java
Outdated
Show resolved
Hide resolved
| } | ||
|
|
||
| public void run() { | ||
| if (System.getenv().containsKey("RUN_WITH_BESU_NODE")) { |
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.
I think the way it's going to go, is that when we run our unit tests, this env variable RUN_WITH_BESU_NODE will be at false, as usually we don't run the tests with a besu node. Maybe what can work is to use a property like in ToyExecEnvV2 like so
@Builder.Default private final Boolean runWithBesuNode = false;
replace the if with if (runWithBesuNode || System.getenv().containsKey("RUN_WITH_BESU_NODE"))
and then when you run a multi block test
final MultiBlockExecutionEnvironment env = builder.build()
.runWithBesuNode(true)
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.
Done (I believe)
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.
Yes, perfect 👌
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.
Note
Adds empty-block test scenarios and updates Besu execution tooling to model empty blocks (null txs) and trace correct block ranges; minor Osaka import fix.
EmptyBlockTestswith multiple empty/non-empty block patterns (EENENE,NEEN,NEEE,ENNE,EEEE,EEEN,E) using Besu-backed execution, sharedBlockTypeenum, and a builder helper.BesuExecutionTools:nulltransactions to represent empty blocks; handle leading/all-empty sequences.[firstBlockNumber, finalBlockNumber]for tracing; add precondition/state checks.MultiBlockExecutionEnvironment:runWithBesuNodeoption; when enabled, translate empty blocks tonulltxs and execute viaBesuExecutionToolswithMAINNET_TESTCONFIG(fork).TraceOsaka.EIP_7825_TRANSACTION_GAS_LIMIT_CAPimport.Written by Cursor Bugbot for commit b477825. This will update automatically on new commits. Configure here.