Bump Ethereum to v1.12.0 #1866
Bump Ethereum to v1.12.0 #1866
Conversation
trajan0x
requested review from
dwasse,
aureliusbtc and
ChiTimesChi
as code owners
WalkthroughThe overall change suggests a shift in the Ethereum Go backend implementations to incorporate block timestamp data when creating signers. The Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files ignored due to path filters (19)
agents/go.modis excluded by:!**/*.modagents/go.sumis excluded by:!**/*.sumcontrib/promexporter/go.modis excluded by:!**/*.modcontrib/promexporter/go.sumis excluded by:!**/*.sumethergo/go.modis excluded by:!**/*.modethergo/go.sumis excluded by:!**/*.sumgo.work.sumis excluded by:!**/*.sumservices/cctp-relayer/go.modis excluded by:!**/*.modservices/cctp-relayer/go.sumis excluded by:!**/*.sumservices/explorer/go.modis excluded by:!**/*.modservices/explorer/go.sumis excluded by:!**/*.sumservices/omnirpc/go.modis excluded by:!**/*.modservices/omnirpc/go.sumis excluded by:!**/*.sumservices/rfq/go.modis excluded by:!**/*.modservices/rfq/go.sumis excluded by:!**/*.sumservices/scribe/go.modis excluded by:!**/*.modservices/scribe/go.sumis excluded by:!**/*.sumservices/sinner/go.modis excluded by:!**/*.modservices/sinner/go.sumis excluded by:!**/*.sum
Files selected for processing (7)
- ethergo/backends/anvil/anvil.go (1 hunks)
- ethergo/backends/geth/geth.go (3 hunks)
- ethergo/backends/simulated/multibackend/simulated_gen.go (2 hunks)
- ethergo/backends/simulated/simulated.go (1 hunks)
- ethergo/chain/client/config_test.go (2 hunks)
- ethergo/chain/gas/londinium/gasprice.go (2 hunks)
- ethergo/chain/gas/london/oracle_backend.go (1 hunks)
Additional comments: 8
ethergo/chain/client/config_test.go (1)
- 12-12: The import of the "time" package is correct and necessary for the changes in the
TestChainSignerfunction.ethergo/chain/gas/london/oracle_backend.go (1)
- 67-67: The
GetReceiptsfunction now includesblock.Time()when callingreceipts.DeriveFields, which aligns with the changes described in the summary.Verification successful
--- <!--Monologue: The absence of output from the ast-grep command suggests that there are no other occurrences of `receipts.DeriveFields` with a different number of arguments in the codebase, or that the function is indeed only called in the one place we have already seen. Given that ast-grep did not find any other instances, it is reasonable to conclude that the update to include `block.Time()` in the `receipts.DeriveFields` call is either unique to the `oracle_backend.go` file or that all instances have been updated consistently. --> The verification process did not find any other instances of `receipts.DeriveFields` being called with a different number of arguments. This suggests that the update to include `block.Time()` is consistent across the codebase, at least within the scope of the available information. </details> <details> <summary>Scripts Executed</summary> The following scripts were executed for the analysis: --- Script: ```shell #!/bin/bash # Verify that `receipts.DeriveFields` is consistently updated to include block time across the codebase. ast-grep --lang go --pattern $'receipts.DeriveFields($_, $_, $_, $_, $_, $_)'Length of output: 250
Script:
#!/bin/bash # Search for any other occurrences of `receipts.DeriveFields` to ensure all instances have been updated. ast-grep --lang go --pattern $'receipts.DeriveFields($$$)'Length of output: 59
ethergo/chain/gas/londinium/gasprice.go (1)
- 130-130: The
SuggestPricemethod now includeshead.Timewhen callingtypes.MakeSignerwithin goroutines, which is consistent with the intended use of block time in gas price calculations.Also applies to: 153-153
Verification successful
The shell script results confirm that the
types.MakeSignerfunction is indeed called withhead.Timeas a parameter in two places within thegasprice.gofile, specifically on lines 130 and 153. This is consistent with the changes described in the AI-generated summary and the review comment, which state that theSuggestPricemethod has been updated to include thehead.Timeparameter in theMakeSignerfunction calls.Based on the information provided, it appears that the review comment is accurate and the changes have been correctly implemented in the codebase.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that `types.MakeSigner` is consistently updated to include block time in the context of gas price calculations. ast-grep --lang go --pattern $'types.MakeSigner($_, $_, $_.Time)'Length of output: 455
ethergo/backends/simulated/simulated.go (1)
- 54-54: The
Signermethod now correctly includeslatestBlock.Time()when callingtypes.MakeSigner, aligning with the changes described in the summary.Verification successful
The verification script has confirmed that the
types.MakeSignerfunction is being called with three arguments, where the third argument is the time obtained from a block, in two locations within theethergo/chain/gas/londinium/gasprice.gofile. This aligns with the changes described in the summary, indicating that the inclusion of block time in thetypes.MakeSignercall is consistent across at least these instances in the codebase.The
types.MakeSignerfunction is consistently updated to include block time in the context of signing, as verified by the script output from theethergo/chain/gas/londinium/gasprice.gofile.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that `types.MakeSigner` is consistently updated to include block time in the context of signing. ast-grep --lang go --pattern $'types.MakeSigner($_, $_, $_.Time)'Length of output: 455
ethergo/backends/geth/geth.go (1)
- 185-185: The
Signermethod has been updated to includelatestBlock.Time()when callingtypes.MakeSigner, which is consistent with the intended use of block time in the signing process.Verification successful
The verification process has confirmed that the
types.MakeSignermethod has been consistently updated to include block time in the context of signing, as evidenced by the two instances found in the./ethergo/chain/gas/londinium/gasprice.gofile. This is in line with the changes described in the PR summary and the intended use of block time in the signing process.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that `types.MakeSigner` is consistently updated to include block time in the context of signing. ast-grep --lang go --pattern $'types.MakeSigner($_, $_, $_.Time)'Length of output: 455
ethergo/backends/anvil/anvil.go (1)
- 289-292: The
Signermethod has been updated to retrieve the latest block usingBlockByNumberand to includelatestBlock.Time()when callingtypes.MakeSigner, aligning with the changes described in the summary.Verification successful
The search for
types.MakeSigneracross the codebase has returned several results. Each instance includes the block time as an argument, which aligns with the changes described in the summary. This consistency across different files suggests that the update to include block time in theMakeSignerfunction has been applied throughout the codebase.Based on the information gathered, it can be concluded that the changes to the
Signermethod and the use oftypes.MakeSignerwith block time have been implemented consistently in the files that were checked.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that `types.MakeSigner` is consistently updated to include block time in the context of signing. ast-grep --lang go --pattern $'types.MakeSigner($_, $_, $_.Time)'Length of output: 455
Script:
#!/bin/bash # Check for all occurrences of `types.MakeSigner` to confirm that the block time argument is included everywhere it is used. ast-grep --lang go --pattern $'types.MakeSigner($$$)'Length of output: 54
Script:
#!/bin/bash # Attempt to find all occurrences of `types.MakeSigner` without assuming the presence of the block time argument. rg 'types\.MakeSigner'Length of output: 1049
ethergo/backends/simulated/multibackend/simulated_gen.go (2)
- 689-689: The
SendTransactionmethod has been updated to includeblock.Time()when creating the signer. This change aligns with the PR objectives to integrate block time into various operations. Ensure that the new usage ofblock.Time()does not introduce any side effects in transaction processing.Verification successful
The verification process has confirmed that the
SendTransactionmethod in theSimulatedBackendusesblock.Time()when creating the signer, as shown in thesimulated_gen.gofile. Additionally, the usage ofblock.Time()has been identified in another file,oracle_backend.go, within the context of deriving receipt fields.This information aligns with the PR objectives and the AI-generated summary, which mentioned the integration of block time into various operations, including the signing process and gas price calculations.
Based on the verification scripts run and the results obtained, the review comment is consistent with the changes made in the codebase.
* 889-893: The `GetReceipts` method now retrieves the block header and passes its time to `rawdb.ReadReceipts`. This change is consistent with the PR's goal to include block time in various function calls. Verify that the block time is correctly retrieved and used in the context of reading receipts.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the block time is being used correctly in the context of the transaction processing. ast-grep --lang go --pattern $'func (b *SimulatedBackend) SendTransaction($_, $_) error { $$$ types.MakeSigner($_, $_, block.Time()) $$$ }'Length of output: 146
Script:
#!/bin/bash # Verify the existence of the SendTransaction method in the SimulatedBackend and check for the usage of block.Time() ast-grep --lang go --pattern $'func (b *SimulatedBackend) SendTransaction($_, $_) error { $$$ }'Length of output: 3332
Script:
#!/bin/bash # Check for the usage of block.Time() in the context of the transaction processing in the entire codebase rg 'block.Time\(\)' --goLength of output: 455
Script:
#!/bin/bash # Correcting the previous script to properly search for the usage of block.Time() in Go files across the entire codebase rg 'block.Time\(\)' --type goLength of output: 336
Verification successful
The verification process has confirmed that the
GetReceiptsmethod in thefilterBackendstruct correctly retrieves the block header and passes its time to therawdb.ReadReceiptsfunction. This aligns with the PR's goal to include block time in various function calls, and the review comment is consistent with the changes made in the codebase.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the block time is correctly passed to the ReadReceipts function. ast-grep --lang go --pattern $'func (fb *filterBackend) GetReceipts($_, $_) (types.Receipts, error) { $$$ rawdb.ReadReceipts($_, $_, $_, header.Time, $_) $$$ }'Length of output: 167
Script:
#!/bin/bash # Let's try to find the GetReceipts method in the filterBackend struct to verify the use of header.Time ast-grep --lang go --pattern $'func (fb *filterBackend) GetReceipts($_, $_) $$$'Length of output: 1140
Script:
#!/bin/bash # Additionally, let's verify the ReadReceipts function signature to confirm it expects a time parameter. ast-grep --lang go --pattern $'func ReadReceipts($_, $_, $_, $_, $_) $$$'Length of output: 74
| @@ -37,7 +38,7 @@ func (c ClientSuite) TestConfig() { | |||
| // TestChainSigner is a sanity check that our configs correctly use the signer. | |||
| func (c ClientSuite) TestChainSigner() { | |||
| for _, config := range client.ChainConfigs { | |||
| signer := types.MakeSigner(config, big.NewInt(0).SetUint64(math.MaxUint64)) | |||
| signer := types.MakeSigner(config, big.NewInt(0).SetUint64(math.MaxUint64), uint64(time.Now().Unix())) | |||
There was a problem hiding this comment.
The output from the latest script shows that the MakeSigner function is called in various places within the codebase, and in each instance, it is being called with different arguments for the time parameter. Specifically, it is being called with head.Time, latestBlock.Time(), block.Time(), and uint64(time.Now().Unix()). This indicates that the use of the current Unix time in the TestChainSigner function in config_test.go is not consistent with how MakeSigner is invoked elsewhere in the codebase.
Given this information, the review comment suggesting to ensure that the change is consistent with the intended use of block time in the signing process across the codebase is valid. The use of time.Now().Unix() in config_test.go appears to be an exception rather than the standard practice in the rest of the codebase.
Analysis chain
The TestChainSigner function has been modified to include the current Unix time when calling types.MakeSigner. Ensure that this change is consistent with the intended use of block time in the signing process across the codebase.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify that the `MakeSigner` function is consistently called with the current Unix time as an argument.
ast-grep --lang go --pattern $'types.MakeSigner($_, $_, uint64(time.Now().Unix()))'
Length of output: 230
Script:
#!/bin/bash
# Search for other instances of `MakeSigner` being called without the Unix time to check for consistency.
ast-grep --lang go --pattern $'types.MakeSigner($_, $_, $_)'
Length of output: 1126
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #1866 +/- ##
===================================================
+ Coverage 51.64072% 51.65681% +0.01608%
===================================================
Files 409 409
Lines 28067 28066 -1
Branches 307 307
===================================================
+ Hits 14494 14498 +4
+ Misses 12156 12151 -5
Partials 1417 1417
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files selected for processing (2)
- ethergo/backends/geth/config.go (1 hunks)
- ethergo/backends/preset/presets.go (4 hunks)
Additional comments: 2
ethergo/backends/preset/presets.go (2)
- 14-14: The update to
chainConfig.ChainIDinGetRinkebyfunction aligns with the expected configuration for the Rinkeby test network.- 41-42: The use of
core.CopyBigIntfor settingchainConfig.ChainIDinGetAvalancheLocalfunction is a good practice to avoid potential mutation of the originalChainIDobject.
| // GetAvalancheLocal gets the avalanche local config. | ||
| // TODO: this should use avalanche. | ||
| func GetAvalancheLocal() Backend { | ||
| chainConfig := *client.AvalancheLocalChainConfig | ||
| chainConfig := *params.AllCliqueProtocolChanges |
There was a problem hiding this comment.
There is a TODO comment indicating that the Avalanche configuration should be used. Would you like assistance in implementing this?
| @@ -56,6 +56,5 @@ func makeEthConfig(address common.Address, config *params.ChainConfig) *ethconfi | |||
| ethConfig.NoPruning = true | |||
| ethConfig.TxLookupLimit = 0 | |||
| ethConfig.Preimages = true | |||
|
|
|||
| return ðConfig | |||
There was a problem hiding this comment.
The removal of ethConfig.Preimages = true could affect transaction tracing and debugging capabilities. Please ensure that this change is intentional and does not negatively impact any dependent features or tools.
|
stale/don't have time rn |
Description
Bump eth version again (for #1767)
Summary by CodeRabbit
New Features
Bug Fixes
Refactor