-
Notifications
You must be signed in to change notification settings - Fork 12
Enable EVM Fusaka hard-fork for PreviewNet & Testnet #913
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: main
Are you sure you want to change the base?
Conversation
WalkthroughAdds dependency bumps in Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Requester
participant FlowNode as Flow node / emulator
Note over Client,Requester: EstimateGas request
Client->>Requester: eth_estimateGas(tx, blockOverrides?)
Requester->>Requester: compute initial passingGasLimit
alt passingGasLimit > MaxTxGas
Requester->>FlowNode: GetLatestEVMHeight()
FlowNode-->>Requester: height (or error)
Requester->>FlowNode: GetBlockByHeight(height)
FlowNode-->>Requester: block (or error)
Requester->>Requester: build emulator config with block + blockOverrides
alt block.detected_as_Osaka
Requester->>Requester: clamp passingGasLimit := MaxTxGas
Note right of Requester: Osaka cap applied (EIP-7825)
else not_Osaka
Note right of Requester: no clamp
end
end
Requester->>Requester: continue estimate simulation using passingGasLimit
Requester-->>Client: eth_estimateGas(result / error)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
9b59de4 to
d24234e
Compare
d24234e to
d55cd8a
Compare
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.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
go.sumis excluded by!**/*.sumtests/go.sumis excluded by!**/*.sum
📒 Files selected for processing (5)
go.mod(5 hunks)models/transaction_test.go(1 hunks)services/requester/requester.go(1 hunks)tests/go.mod(5 hunks)tests/web3js/estimate_gas_overrides_test.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- models/transaction_test.go
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Test
- GitHub Check: Lint
🔇 Additional comments (3)
tests/web3js/estimate_gas_overrides_test.js (1)
104-122: LGTM! Good test coverage for Osaka gas cap handling.The test validates that
eth_estimateGascontinues to work correctly when provided with gas limits exceeding the EIP-7825 cap (16,777,216), ensuring the new capping logic in the gateway doesn't break estimation behavior.go.mod (1)
7-7: LGTM! Dependency updates align with Fusaka hard-fork enablement.The dependency bumps, particularly
ethereum/go-ethereumto v1.16.5 and the updatedonflow/flow-gocommit, provide the necessary Osaka/Fusaka hard-fork support for PreviewNet and Testnet.Also applies to: 13-15, 37-37, 43-43, 92-92, 149-150, 222-223
tests/go.mod (1)
6-6: LGTM! Test dependency updates maintain consistency with main module.The test module dependencies are properly synchronized with the main
go.mod, ensuring the test environment has the necessary Osaka/Fusaka support.Also applies to: 8-8, 10-10, 12-13, 25-25, 31-31, 91-91, 160-161, 248-250
| // Cap the maximum gas allowance according to EIP-7825 if the estimation targets Osaka | ||
| if passingGasLimit > gethParams.MaxTxGas { | ||
| latestBlockHeight, err := e.blocks.LatestEVMHeight() | ||
| if err != nil { | ||
| return 0, err | ||
| } | ||
| latestBlock, err := e.blocks.GetByHeight(latestBlockHeight) | ||
| if err != nil { | ||
| return 0, err | ||
| } | ||
| blockNumber, blockTime := new(big.Int).SetUint64(latestBlock.Height), latestBlock.Timestamp | ||
| emulatorConfig := emulator.NewConfig( | ||
| emulator.WithChainID(e.config.EVMNetworkID), | ||
| emulator.WithBlockNumber(blockNumber), | ||
| emulator.WithBlockTime(blockTime), | ||
| ) | ||
|
|
||
| if blockOverrides != nil { | ||
| if blockOverrides.Number != nil { | ||
| blockNumber = blockOverrides.Number.ToInt() | ||
| } | ||
| if blockOverrides.Time != nil { | ||
| blockTime = uint64(*blockOverrides.Time) | ||
| } | ||
| } | ||
| if emulatorConfig.ChainConfig.IsOsaka(blockNumber, blockTime) { | ||
| passingGasLimit = gethParams.MaxTxGas | ||
| } | ||
| } |
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.
🧩 Analysis chain
Verify Osaka check uses correct block height.
The logic fetches the latest block to determine Osaka status (line 350), but the EstimateGas function receives a height parameter that specifies which block to estimate against. This creates a potential mismatch:
- If
heightpoints to a pre-Osaka block but the latest block is post-Osaka, the cap would be incorrectly applied - Conversely, if
heightpoints to a post-Osaka block but the latest block is pre-Osaka, the cap would be incorrectly skipped
The Osaka check should likely use the target height parameter instead of latestBlockHeight to ensure consistency between the block state being estimated and the hard-fork rules being applied.
Apply this diff to use the target height for Osaka detection:
- // Cap the maximum gas allowance according to EIP-7825 if the estimation targets Osaka
if passingGasLimit > gethParams.MaxTxGas {
- latestBlockHeight, err := e.blocks.LatestEVMHeight()
- if err != nil {
- return 0, err
- }
- latestBlock, err := e.blocks.GetByHeight(latestBlockHeight)
+ // Cap the maximum gas allowance according to EIP-7825 if the estimation targets Osaka
+ targetBlock, err := e.blocks.GetByHeight(height)
if err != nil {
return 0, err
}
- blockNumber, blockTime := new(big.Int).SetUint64(latestBlock.Height), latestBlock.Timestamp
+ blockNumber, blockTime := new(big.Int).SetUint64(targetBlock.Height), targetBlock.Timestamp
emulatorConfig := emulator.NewConfig(
emulator.WithChainID(e.config.EVMNetworkID),
emulator.WithBlockNumber(blockNumber),
emulator.WithBlockTime(blockTime),
)
if blockOverrides != nil {
if blockOverrides.Number != nil {
blockNumber = blockOverrides.Number.ToInt()
}
if blockOverrides.Time != nil {
blockTime = uint64(*blockOverrides.Time)
}
}
if emulatorConfig.ChainConfig.IsOsaka(blockNumber, blockTime) {
passingGasLimit = gethParams.MaxTxGas
}
}
---
🏁 Script executed:
```shell
#!/bin/bash
# Extract the EstimateGas function to understand the full context
# Look for the function definition and surrounding code
rg -n "func.*EstimateGas" services/requester/requester.go -A 100 | head -150
Length of output: 4201
🏁 Script executed:
#!/bin/bash
# Find where dryRunTx is called to see what parameters it receives
rg -n "dryRunTx" services/requester/requester.go -B 5 -A 5Length of output: 1040
🏁 Script executed:
#!/bin/bash
# Check the function signature and see what the height parameter is
rg -n "func \(e \*EVMRPCRequester\) EstimateGas" services/requester/requester.go -A 30Length of output: 0
Gas capping uses incorrect block height for Osaka detection.
The EstimateGas function receives a height parameter that specifies which block to estimate the transaction against (line 319), and dryRunTx is correctly called with this parameter (line 329). However, the Osaka fork detection at lines 348-376 fetches latestBlockHeight instead of using the target height, creating a mismatch.
If height and latestBlockHeight differ, the gas cap will be applied based on the wrong fork state:
- When height is pre-Osaka but latest is post-Osaka: cap incorrectly applied
- When height is post-Osaka but latest is pre-Osaka: cap incorrectly skipped
Use the target height parameter for Osaka detection to ensure consistency with the block being estimated:
- // Cap the maximum gas allowance according to EIP-7825 if the estimation targets Osaka
if passingGasLimit > gethParams.MaxTxGas {
- latestBlockHeight, err := e.blocks.LatestEVMHeight()
- if err != nil {
- return 0, err
- }
- latestBlock, err := e.blocks.GetByHeight(latestBlockHeight)
+ // Cap the maximum gas allowance according to EIP-7825 if the estimation targets Osaka
+ targetBlock, err := e.blocks.GetByHeight(height)
if err != nil {
return 0, err
}
- blockNumber, blockTime := new(big.Int).SetUint64(latestBlock.Height), latestBlock.Timestamp
+ blockNumber, blockTime := new(big.Int).SetUint64(targetBlock.Height), targetBlock.Timestamp🤖 Prompt for AI Agents
In services/requester/requester.go around lines 348 to 376, the Osaka fork
detection is using latestBlockHeight instead of the target height parameter —
fetch and use the block at the provided height (i.e., call
e.blocks.GetByHeight(height) and set blockNumber/blockTime from that block) and
build the emulator config after applying blockOverrides (or apply overrides to
the blockNumber/blockTime before creating the emulator.Config); keep existing
error handling for the GetByHeight call and then run IsOsaka against the target
blockNumber/blockTime so the gas cap decision matches the block being estimated.
Work towards: #912
Depends on: onflow/flow-go#8085
Description
Updates the flow-go version to onflow/flow-go#8085, which enables the Fusaka hard-fork for
PreviewNet(a.k.a Emulator) &Testnetnetworks.For contributor use:
masterbranchFiles changedin the Github PR explorerSummary by CodeRabbit