Repository navigation
test: Add comprehensive EVM E2E tests #2394
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
Changes from 23 commits
fa4eee2
bd71544
eed64f6
065b7ef
ca15034
5091f89
c350346
9ed2542
9d20daf
303c580
c4bca3f
81afb2d
f9855c8
7b7be7e
9ee6b46
3b2827c
fd7ab38
c674f5f
e15477d
bea5d7e
86f9f69
23ee053
1df4ebc
2e76463
5171bed
0c8d343
d6aee4b
badfe37
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -181,16 +181,21 @@ func (c *EngineClient) ExecuteTxs(ctx context.Context, txs [][]byte, blockHeight | |
| txsPayload[i] = "0x" + hex.EncodeToString(tx) | ||
| } | ||
|
|
||
| _, _, prevGasLimit, _, err := c.getBlockInfo(ctx, blockHeight-1) | ||
| prevBlockHash, _, prevGasLimit, prevTimestamp, err := c.getBlockInfo(ctx, blockHeight-1) | ||
| if err != nil { | ||
| return nil, 0, fmt.Errorf("failed to get block info: %w", err) | ||
| } | ||
|
|
||
| ts := uint64(timestamp.Unix()) | ||
| if ts <= prevTimestamp { | ||
| ts = prevTimestamp + 1 // Subsequent blocks must have a higher timestamp. | ||
| } | ||
|
|
||
| c.mu.Lock() | ||
| args := engine.ForkchoiceStateV1{ | ||
| HeadBlockHash: c.currentHeadBlockHash, | ||
| SafeBlockHash: c.currentSafeBlockHash, | ||
| FinalizedBlockHash: c.currentFinalizedBlockHash, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. were these empty or why remove them?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is actually how the our execution evm implementation used to work before this PR: https://github.com/rollkit/rollkit/pull/2373/files#r2178116028 we already have a follow up issue for this: #2402 if we do not have this change, restarts for the evm do not work well since on startup the all the current hashes are set to the genesisHash, and it'll try to make a forkChoiceUpdatedV3 call with the genesis hash again on restart. Made this change back to make restarts work well when XO markets pointed out that restarts used to work fine but didn't work after this PR: #2373 |
||
| HeadBlockHash: prevBlockHash, | ||
| SafeBlockHash: prevBlockHash, | ||
| FinalizedBlockHash: prevBlockHash, | ||
| } | ||
| c.mu.Unlock() | ||
|
|
||
|
|
@@ -200,7 +205,7 @@ func (c *EngineClient) ExecuteTxs(ctx context.Context, txs [][]byte, blockHeight | |
| // Create rollkit-compatible payload attributes with flattened structure | ||
| rollkitPayloadAttrs := map[string]interface{}{ | ||
| // Standard Ethereum payload attributes (flattened) - using camelCase as expected by JSON | ||
| "timestamp": uint64(timestamp.Unix()), | ||
| "timestamp": ts, | ||
| "prevRandao": c.derivePrevRandao(blockHeight), | ||
| "suggestedFeeRecipient": c.feeRecipient, | ||
| "withdrawals": []*types.Withdrawal{}, | ||
|
|
||
This file was deleted.
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.
this seems entirely arbitrary, taking the old time stamp add adding one could mean that the execution timestamp could skew over time.
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 this is also a reversion to what was changed already in https://github.com/rollkit/rollkit/pull/2373/files#r2178117196
Note that all indexers etc use timestamp in seconds, and what's that Unix() time here does. Without this change, running a chain with a block time in milliseconds breaks since reth rejects building two blocks with the same timestamp, so this was a workaround for the time being. We should have a different way as a followup