Upgrade Go-Ethereum to v1.11.6 #1865
Conversation
trajan0x
requested review from
dwasse,
aureliusbtc and
ChiTimesChi
as code owners
|
Warning Rate Limit Exceeded@trajan0x has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 2 minutes and 36 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. WalkthroughThe recent changes across various files seem to focus on updating the handling of Ethereum Improvement Proposals (EIPs), particularly the removal of Changes
Poem
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 (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1865 +/- ##
===================================================
+ Coverage 51.62991% 51.77254% +0.14262%
===================================================
Files 409 409
Lines 28069 28067 -2
Branches 307 307
===================================================
+ Hits 14492 14531 +39
+ Misses 12159 12125 -34
+ Partials 1418 1411 -7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
![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 (23)
agents/go.modis excluded by:!**/*.modagents/go.sumis excluded by:!**/*.sumcontrib/git-changes-action/go.sumis excluded by:!**/*.sumcontrib/promexporter/go.modis excluded by:!**/*.modcontrib/promexporter/go.sumis excluded by:!**/*.sumcontrib/terraform-provider-helmproxy/go.sumis excluded by:!**/*.sumcontrib/terraform-provider-kubeproxy/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:!**/*.sumtools/go.sumis excluded by:!**/*.sum
Files selected for processing (11)
- ethergo/backends/anvil/hardfork.go (1 hunks)
- ethergo/backends/simulated/multibackend/chainid.go (2 hunks)
- ethergo/backends/simulated/multibackend/simulated_gen.go (11 hunks)
- ethergo/backends/simulated/multibackend/simulated_gen_test.go (3 hunks)
- ethergo/chain/client/config.go (2 hunks)
- ethergo/chain/gas/londinium/config_test.go (1 hunks)
- ethergo/chain/gas/londinium/gasprice_test.go (1 hunks)
- ethergo/chain/gas/london/oracle_backend.go (1 hunks)
- ethergo/deployer/deployed_contract.go (2 hunks)
- ethergo/submitter/db/txdb/store.go (1 hunks)
- ethergo/util/converter.go (2 hunks)
Files skipped from review due to trivial changes (1)
- ethergo/chain/client/config.go
Additional comments: 14
ethergo/util/converter.go (1)
- 11-18: The
TxToCallfunction has been updated to accept a pointer totypes.Transactioninstead of theTransactioninterface. This change is consistent with the PR objectives and the AI-generated summary. The update fromrawMsg.From()torawMsg.Fromis also in line with the changes in the Ethereum library, whereFromis now a field rather than a method. The removal of theTransactioninterface and its methods is not shown in the diff, but it is mentioned in the summary and is a logical simplification if the interface is no longer needed.ethergo/chain/gas/londinium/config_test.go (1)
- 17-18: The change from
inttouint64forMaxHeaderHistoryandMaxBlockHistoryin theTestLondoniumConfigfunction is consistent with the PR objectives to update types for gas pricing configuration. This change likely reflects a necessary update to align with the newer version of Go-Ethereum where these fields are expected to beuint64.ethergo/deployer/deployed_contract.go (2)
6-6: The addition of the
ethCorepackage import is consistent with the changes in theNewDeployedContractfunction, whereethCore.TransactionToMessageis used.41-41: The change from
msg.From()tomsg.Fromis consistent with the updates in the Ethereum library, whereFromis now a field rather than a method. This change is necessary to align with the updated method signatures in the Go-Ethereum library.ethergo/chain/gas/london/oracle_backend.go (1)
- 67-67: The addition of
block.BaseFee()as an argument to theDeriveFieldsfunction call in theGetReceiptsmethod is consistent with the PR objectives to align with updated Ethereum protocol specifications, specifically the inclusion of the base fee in the derivation of receipt fields.ethergo/backends/simulated/multibackend/chainid.go (1)
- 64-64: The change in the
NewSimulatedBackendWithConfigfunction to useblockchain.GetBlockinstead ofblockchain.CurrentBlockfor therollbackmethod call is not explicitly mentioned in the PR objectives or the AI-generated summary. However, this change seems to be a logical update to ensure that the correct block is used for rollback operations, potentially improving the accuracy of the simulated backend.ethergo/chain/gas/londinium/gasprice_test.go (2)
45-45: The change in the
HeaderByNumbermethod to returnb.chain.CurrentBlock()directly is a simplification that removes unnecessary indirection. This change is not explicitly mentioned in the PR objectives or the AI-generated summary, but it seems to be a minor refactor for clarity.52-52: The change in the
BlockByNumbermethod to useb.chain.GetBlockByHash(b.chain.CurrentBlock().Hash())instead of directly returningb.chain.CurrentBlock()is not explicitly mentioned in the PR objectives or the AI-generated summary. This change seems to ensure that the method returns a block by its hash, which could be a necessary update for internal consistency within the test backend.ethergo/submitter/db/txdb/store.go (1)
- 264-267: The logic for handling transactions has been updated to unmarshal a new
types.Transactionobject and recover the signer from the transaction. This change is consistent with the PR objectives to improve transaction handling. The use ofutil.TxToCallto recover the signer suggests a more robust approach to transaction processing.ethergo/backends/simulated/multibackend/simulated_gen.go (1)
- 610-610: The
callContractmethod has been updated to accept aheader *types.Headerinstead of ablock *types.Block. This change aligns with the PR objectives to enhance backend simulation capabilities. Ensure that all calls to this method have been updated accordingly.ethergo/backends/simulated/multibackend/simulated_gen_test.go (4)
1194-1194: The test
TestForkis checking the chain length after a reorg, which is a valid test case. However, the actual implementation of the test is not visible in the provided code. Ensure that the logic within the test correctly handles the reorg scenario as described in the comments.1204-1204: Similar to the previous comment, the test
TestForkis mentioned, but the implementation details are not provided. Ensure that the test logic is correctly implemented to validate the chain length after a reorg.1349-1349: The comment at line 1349 indicates the start of a test case
TestCommitReturnValue. The test seems to be checking the return value of theCommitfunction. Ensure that the test case includes assertions to validate that theCommitfunction returns the correct block hash after a new block is committed to the blockchain.1191-1197: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [1194-1204]
The tests
TestFork,TestForkLogsReborn,TestForkResendTx,TestCommitReturnValue, andTestAdjustTimeAfterForkare all crucial for ensuring the correctness of the simulated backend after a fork. It is important to verify that these tests cover all edge cases and that the assertions within these tests are robust enough to catch any potential issues with the backend's behavior after a fork.Also applies to: 1349-1349
| header := b.blockchain.CurrentBlock() | ||
| block := b.blockchain.GetBlock(header.Hash(), header.Number.Uint64()) | ||
|
|
||
| b.rollback(block) |
There was a problem hiding this comment.
The Rollback method now takes additional parameters header *types.Header and block *types.Block, but the implementation still uses the current block from the blockchain. This change is inconsistent with the method signature and should be corrected to use the passed-in parameters.
func (b *SimulatedBackend) Rollback(header *types.Header, block *types.Block) {
b.mu.Lock()
defer b.mu.Unlock()
- header := b.blockchain.CurrentBlock()
- block := b.blockchain.GetBlock(header.Hash(), header.Number.Uint64())
b.rollback(block)
}Committable suggestion
❗ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| header := b.blockchain.CurrentBlock() | |
| block := b.blockchain.GetBlock(header.Hash(), header.Number.Uint64()) | |
| b.rollback(block) | |
| func (b *SimulatedBackend) Rollback(header *types.Header, block *types.Block) { | |
| b.mu.Lock() | |
| defer b.mu.Unlock() | |
| b.rollback(block) | |
| } |
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (3)
- services/scribe/api/data_test.go (2 hunks)
- services/scribe/graphql/server/graph/queries.resolvers.go (2 hunks)
- services/scribe/graphql/server/graph/utils.go (2 hunks)
Additional comments: 7
services/scribe/graphql/server/graph/utils.go (2)
6-6: The import of
ethCorefromgh.neting.cc/ethereum/go-ethereum/coreis correctly added to support the new usage ofTransactionToMessage.85-86: The change from
ethTx.Tx.AsMessagetoethCore.TransactionToMessageis consistent with the upgrade to Go-Ethereum v1.11.6, which likely deprecated the old method in favor of the new one. Ensure that the rest of the codebase has been updated to reflect this change where necessary.Verification successful
The search for the
TransactionToMessagemethod usage within Go files has yielded several results, indicating that the new method has been adopted in various parts of the codebase. This confirms that the transition from the oldTx.AsMessagemethod to the newTransactionToMessagemethod has been implemented in multiple locations.Given this information, it appears that the change from
ethTx.Tx.AsMessagetoethCore.TransactionToMessagehas been successfully applied where necessary in the codebase, in line with the upgrade to Go-Ethereum v1.11.6.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for the old `AsMessage` method usage to ensure it's been replaced everywhere. rg --type go 'Tx.AsMessage'Length of output: 27
Script:
#!/bin/bash # Search for the new `TransactionToMessage` method usage to confirm its adoption in the codebase. rg --type go 'TransactionToMessage'Length of output: 815
services/scribe/graphql/server/graph/queries.resolvers.go (3)
10-10: The import of
ethCorefromgh.neting.cc/ethereum/go-ethereum/coreis correctly added to support the new usage ofTransactionToMessage.178-178: The change from
Tx.AsMessagetoethCore.TransactionToMessageis consistent with the upgrade to Go-Ethereum v1.11.6. Ensure that the rest of the codebase has been updated to reflect this change where necessary.Verification successful
The search for the new
ethCore.TransactionToMessagemethod usage has yielded several results, indicating that this method is indeed being used in various parts of the codebase. This confirms that the change fromTx.AsMessagetoethCore.TransactionToMessagehas been adopted in multiple files, which is consistent with the upgrade to Go-Ethereum v1.11.6 as mentioned in the review comment.Given this information, the review comment suggesting to ensure that the rest of the codebase has been updated to reflect this change where necessary is consistent with the findings from the executed scripts.
* 183-183: The update from `msgFrom.From().String()` to `msgFrom.From.String()` is a minor change that reflects a change in the API. This should be correct assuming the `TransactionToMessage` now returns a `Message` struct with a direct `From` field instead of a method.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for the old `AsMessage` method usage to ensure it's been replaced everywhere. rg --type go 'Tx.AsMessage'Length of output: 27
Script:
#!/bin/bash # Search for the new `TransactionToMessage` method usage to confirm it has been adopted. rg --type go 'ethCore.TransactionToMessage'Length of output: 823
services/scribe/api/data_test.go (2)
6-6: The import of
ethCorefromgh.neting.cc/ethereum/go-ethereum/coreis correctly added to support the new usage ofTransactionToMessagein tests.178-179: The change from
tx.AsMessagetoethCore.TransactionToMessagein theTestTransactionDataEqualityfunction is consistent with the upgrade to Go-Ethereum v1.11.6. This change should be reflected in all tests that previously used the old method.Verification successful
The shell script executed a broader search for the
.AsMessagemethod usage in Go test files and did not produce any output. This indicates that there are no instances of.AsMessagebeing used in the test files, which supports the review comment that the method has been replaced consistently withethCore.TransactionToMessagein the context of the upgrade to Go-Ethereum v1.11.6.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for the old `AsMessage` method usage in tests to ensure it's been replaced everywhere. rg --type go 'Tx.AsMessage' --glob '*_test.go'Length of output: 46
Script:
#!/bin/bash # Broaden the search for the `AsMessage` method usage in tests to ensure it's been replaced everywhere. rg --type go '\.AsMessage' --glob '*_test.go'Length of output: 47
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- ethergo/submitter/db_test.go (1 hunks)
Additional comments: 1
ethergo/submitter/db_test.go (1)
- 102-102: The
util.TxToCallfunction has been updated to accepttx.Transactioninstead of justtx. Ensure that all calls toutil.TxToCallthroughout the codebase have been updated to match the new signature.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to path filters (7)
contrib/git-changes-action/go.modis excluded by:!**/*.modcontrib/git-changes-action/go.sumis excluded by:!**/*.sumcore/go.modis excluded by:!**/*.modcore/go.sumis excluded by:!**/*.sumgo.work.sumis excluded by:!**/*.sumtools/go.modis excluded by:!**/*.modtools/go.sumis excluded by:!**/*.sum
Files selected for processing (1)
- ethergo/submitter/db/txdb/store.go (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- ethergo/submitter/db/txdb/store.go
appears to be the issue right now, if I had to guess, this has something to do w/ tx receipt returned by simulated backend. |
Description
Upgrade part 2 (in pursuit of #1767)
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Documentation
Style
Tests
Chores
Revert