Upgrade geth to 1.13 #2416
Upgrade geth to 1.13 #2416
Conversation
WalkthroughThe recent changes include updates to error handling, function enhancements, and dependency adjustments across multiple Go modules. Key updates involve the introduction of new functions for block and contract operations, improved test coverage, and dependency upgrades to align with the latest versions. These changes aim to enhance functionality, improve error reporting, and ensure compatibility with updated libraries. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SimulatedBackend
participant Blockchain
participant Contract
User->>SimulatedBackend: SendTransaction
SimulatedBackend->>Blockchain: Create signer with block.Time()
Blockchain-->>SimulatedBackend: Signer created
SimulatedBackend-->>User: Transaction sent
User->>SimulatedBackend: AdjustTime
SimulatedBackend-->>User: Time adjusted
User->>SimulatedBackend: GetReceipts
SimulatedBackend->>Blockchain: Read header information
Blockchain-->>SimulatedBackend: Header information retrieved
SimulatedBackend-->>User: Receipts retrieved
User->>SimulatedBackend: CodeAtHash
SimulatedBackend->>Blockchain: Retrieve code by block hash
Blockchain-->>SimulatedBackend: Code retrieved
SimulatedBackend-->>User: Code returned
User->>SimulatedBackend: CallContractAtHash
SimulatedBackend->>Contract: Execute contract call by block hash
Contract-->>SimulatedBackend: Contract call executed
SimulatedBackend-->>User: Contract call result
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review Status
Configuration used: .coderabbit.yaml
Files ignored due to path filters (12)
services/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/stiprelayer/go.modis excluded by!**/*.modservices/stiprelayer/go.sumis excluded by!**/*.sum
Files selected for processing (1)
- services/cctp-relayer/external/synapse-contracts (1 hunks)
Files skipped from review due to trivial changes (1)
- services/cctp-relayer/external/synapse-contracts
Additional Context Used
Deploying sanguine-fe with
|
| Latest commit: |
f458ffe
|
| Status: | 🚫 Build failed. |
github-actions
bot
added
go
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2416 +/- ##
===================================================
+ Coverage 38.94594% 47.76307% +8.81712%
===================================================
Files 168 317 +149
Lines 13244 23112 +9868
Branches 80 80
===================================================
+ Hits 5158 11039 +5881
- Misses 7435 10989 +3554
- Partials 651 1084 +433
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.
Actionable comments posted: 0
Review Status
Configuration used: .coderabbit.yaml
Files ignored due to path filters (16)
agents/go.modis excluded by!**/*.modcommittee/go.modis excluded by!**/*.modcontrib/git-changes-action/go.modis excluded by!**/*.modcontrib/promexporter/go.modis excluded by!**/*.modcontrib/screener-api/go.modis excluded by!**/*.modcore/go.modis excluded by!**/*.modethergo/go.modis excluded by!**/*.modgo.work.sumis excluded by!**/*.sumservices/cctp-relayer/go.sumis excluded by!**/*.sumservices/explorer/go.sumis excluded by!**/*.sumservices/omnirpc/go.sumis excluded by!**/*.sumservices/rfq/go.sumis excluded by!**/*.sumservices/scribe/go.sumis excluded by!**/*.sumservices/stiprelayer/go.sumis excluded by!**/*.sumsin-executor/go.modis excluded by!**/*.modtools/go.modis excluded by!**/*.mod
Files selected for processing (6)
- ethergo/backends/anvil/anvil.go (2 hunks)
- ethergo/backends/geth/geth.go (3 hunks)
- ethergo/backends/simulated/multibackend/simulated_gen.go (2 hunks)
- ethergo/backends/simulated/simulated.go (3 hunks)
- ethergo/chain/gas/londinium/gasprice.go (2 hunks)
- ethergo/chain/gas/london/oracle_backend.go (1 hunks)
Additional Context Used
Additional comments not posted (10)
ethergo/chain/gas/london/oracle_backend.go (1)
67-67: The addition ofblock.Time()as a parameter to theDeriveFieldsmethod call is noted. Ensure that the use ofblock.Time()withinDeriveFieldsaligns with the intended logic and has been thoroughly tested, especially in scenarios where block time might significantly influence the derived fields.ethergo/chain/gas/londinium/gasprice.go (1)
125-125: The addition of thetimeparameter to thegetBlockPricescalls within theSuggestPricemethod is noted. Ensure that the use of thetimeparameter withingetBlockPricesis correctly implemented for accurate gas price calculation. It's crucial to verify how block time influences the price calculation and to test this functionality thoroughly.ethergo/backends/simulated/simulated.go (2)
6-8: The addition of imports (math/big,testing,time) aligns with the updated functionalities within the file, such as handling block times and big integers. Ensure these packages are utilized as intended throughout the file.
55-55: The modification to includelatestBlock.Time()in thetypes.MakeSignercall within theSignermethod is noted. Ensure that the use oflatestBlock.Time()is correctly implemented for accurate transaction signing. It's crucial to verify how the block time influences the signing process and to test this functionality thoroughly.ethergo/backends/geth/geth.go (2)
122-122: The removal of the parameter0from theStartMiningfunction calls is noted. Ensure that mining functionality is correctly initiated without this parameter and verify that this change aligns with the updated method signature and intended behavior.
185-185: The modification to includelatestBlock.Time()in thetypes.MakeSignercall within theSignermethod is noted. Ensure that the use oflatestBlock.Time()is correctly implemented for accurate transaction signing. It's crucial to verify how the block time influences the signing process and to test this functionality thoroughly.ethergo/backends/anvil/anvil.go (2)
13-16: The addition of imports (github.com/ipfs/go-log,github.com/lmittmann/w3/w3types,github.com/ory/dockertest/v3) aligns with the updated functionalities within the file, such as logging, handling web3 types, and Docker container management in tests. Ensure these packages are utilized as intended throughout the file.
290-294: The modification to includelatestBlock.Time()in thetypes.MakeSignercall within theSignermethod is noted. Ensure that the use oflatestBlock.Time()is correctly implemented for accurate transaction signing. It's crucial to verify how the block time influences the signing process and to test this functionality thoroughly.ethergo/backends/simulated/multibackend/simulated_gen.go (2)
689-689: The addition of the block time parameter to theMakeSignercall in theSendTransactionfunction aligns with the upgrade requirements for Geth 1.12. This change ensures that the transaction signing process considers the block time, which is crucial for accurately simulating transactions in a test environment.
889-889: The inclusion of the block time parameter in theReadReceiptscall within theGetReceiptsfunction is a necessary adjustment for compatibility with Geth 1.12. This modification ensures that receipt reading behavior accurately reflects the new block time considerations introduced in the upgraded Geth version.
There was a problem hiding this comment.
Actionable comments posted: 0
Review Status
Configuration used: .coderabbit.yaml
Files ignored due to path filters (16)
agents/go.modis excluded by!**/*.modcommittee/go.modis excluded by!**/*.modcontrib/git-changes-action/go.modis excluded by!**/*.modcontrib/promexporter/go.modis excluded by!**/*.modcontrib/screener-api/go.modis excluded by!**/*.modcore/go.modis excluded by!**/*.modethergo/go.modis excluded by!**/*.modgo.work.sumis excluded by!**/*.sumservices/cctp-relayer/go.modis excluded by!**/*.modservices/explorer/go.modis excluded by!**/*.modservices/omnirpc/go.modis excluded by!**/*.modservices/rfq/go.modis excluded by!**/*.modservices/scribe/go.modis excluded by!**/*.modservices/stiprelayer/go.modis excluded by!**/*.modsin-executor/go.modis excluded by!**/*.modtools/go.modis excluded by!**/*.mod
Files selected for processing (13)
- agents/testutil/simulated_backends_suite.go (1 hunks)
- ethergo/backends/geth/config.go (3 hunks)
- ethergo/backends/geth/geth.go (6 hunks)
- ethergo/backends/preset/presets.go (2 hunks)
- ethergo/backends/simulated/multibackend/chainid.go (2 hunks)
- ethergo/backends/simulated/multibackend/simulated_gen.go (2 hunks)
- ethergo/chain/chain_test.go (1 hunks)
- ethergo/chain/client/client_test.go (3 hunks)
- ethergo/chain/client/config.go (2 hunks)
- ethergo/chain/client/config_test.go (2 hunks)
- ethergo/chain/gas/londinium/gasprice_test.go (3 hunks)
- ethergo/chain/gas/london/oracle_backend.go (2 hunks)
- services/omnirpc/rpcinfo/latency_test.go (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- ethergo/backends/geth/geth.go
- ethergo/backends/simulated/multibackend/simulated_gen.go
Additional Context Used
Additional comments not posted (23)
ethergo/chain/chain_test.go (1)
17-17: The change fromGetRinkeby()toGetSepolia()inTestNewFromURLaligns with the PR's objective. Ensure that this change does not inadvertently affect other tests or functionalities that might rely on the Rinkeby configuration.ethergo/chain/client/config_test.go (2)
4-6: The added imports (math,math/big, andtime) are necessary for the changes in theTestChainSignerfunction and are correctly placed.
42-42: The inclusion oftime.Now().Unix()as an additional argument in thetypes.MakeSignercall is a significant update. Ensure that this change is consistent with the expected behavior and does not introduce any unintended side effects.ethergo/backends/geth/config.go (2)
4-6: The added imports (math/bigandtesting) are necessary for the changes in themakeEthConfigfunction and are correctly placed.
49-49: The adjustment in thecore.DeveloperGenesisBlockfunction call by removing an argument likely reflects an update in the underlying library. Ensure that this change does not affect the expected Ethereum configuration.services/omnirpc/rpcinfo/latency_test.go (2)
22-26: The update from BSC Testnet to Matic Mumbai and from Avalanche Local to Sepolia in theTestRPCLatencyfunction aligns with the PR's objective. Ensure that these changes are consistent with the intended testing scenarios.
5-9: The reorganization of imports, including the addition ofnet/httpandnet/http/httptest, improves the readability and organization of the file.ethergo/backends/preset/presets.go (2)
12-20: The alteration of theGetRinkebyfunction toGetSepolia, updating chain configurations and RPC URLs for the Sepolia preset backend, aligns with the PR's objective. Ensure that this change does not affect other functionalities that might rely on the Rinkeby configuration.
45-45: The adjustments made to theGetMaticMumbaifunction related to the London block configuration are significant. Ensure that this change is consistent with the expected behavior and does not introduce any unintended side effects.ethergo/chain/gas/london/oracle_backend.go (1)
68-73: The addition ofblock.Time()andblobGasPricecalculation in theDeriveFieldsmethod call forreceiptsis significant. Ensure that this change is consistent with the expected behavior and does not introduce any unintended side effects related to gas price calculations.ethergo/chain/client/client_test.go (3)
4-6: The added imports (osandtime) are necessary for the changes in theTestAttemptReconnectfunction and are correctly placed.
26-26: The update from Rinkeby to Sepolia in theTestAttemptReconnectfunction aligns with the PR's objective. Ensure that this change does not affect other tests or functionalities that might rely on the Rinkeby configuration.
26-26: Setting a reset timeout for the client in theTestAttemptReconnectfunction is significant. Ensure that this change does not introduce any unintended side effects on client behavior.ethergo/backends/simulated/multibackend/chainid.go (3)
12-12: The addition of thetriepackage import is necessary for the changes in theNewSimulatedBackendWithConfigfunction and is correctly placed.
51-51: The introduction of thetriedbvariable usingtrie.NewDatabaseis significant. Ensure that this change is consistent with the expected behavior and does not introduce any unintended side effects related to database management.
54-54: The modification ofgenesis.MustCommitto includetriedbas an argument is significant. Ensure that this change is consistent with the expected behavior and does not introduce any unintended side effects related to genesis block creation.ethergo/chain/gas/londinium/gasprice_test.go (3)
25-26: The addition of imports for thetriepackage and thelondiniumpackage itself is appropriate for the functionality being tested. Ensure that these packages are used effectively within the test cases.
76-76: The initialization oftriedbwithtrie.NewDatabase(db, nil)introduces a second parameter, which isnilin this context. It's important to verify that passingnilas the second argument aligns with the expected behavior oftrie.NewDatabase. If this parameter is meant to configure certain aspects of the database, consider whether a non-nil value should be provided for more comprehensive testing.
90-90: The creation ofdisktriedbwithtrie.NewDatabase(diskdb, nil)mirrors the earlier initialization oftriedb. As with the previous comment, ensure that the use ofnilfor the second parameter is intentional and appropriate for the tests being conducted.ethergo/chain/client/config.go (2)
4-5: The addition of themath/bigpackage is necessary for working with big integers, which are common in blockchain configurations. This change is appropriate for the functionality of the file.
337-337: Reordering the chain configurations inchainConfigsand removing duplicate entries improves the maintainability and readability of the code. It's important to ensure that all necessary configurations are still present and correctly ordered after these changes.agents/testutil/simulated_backends_suite.go (2)
420-420: Changing the chain ID to Sepolia's chain ID for theanvilOptsOriginsetup is a significant update that aligns with the PR's objective of upgrading Geth to version 1.13. This change ensures that the test environment reflects the updated network configurations. Ensure that all dependent tests and functionalities are compatible with Sepolia.
425-425: Using Sepolia for theTestBackendOriginsetup without Anvil indicates a broader shift in the test network preference. This change should be reflected across all tests and documentation to avoid confusion.
|
This PR is stale because it has been open 14 days with no activity. Remove stale label or comment or this will be closed in 5 days. |
There was a problem hiding this comment.
Actionable comments posted: 5
Outside diff range and nitpick comments (2)
ethergo/backends/geth/geth.go (2)
128-129: Consider setting a non-zero gas tip to avoid potential issues with transaction processing.It's generally a good practice to set a realistic gas tip to ensure transactions are processed in a timely manner, especially in a simulated environment where you might want to mimic real-world conditions.
230-230: Consider using a more dynamic calculation for the gas limit.Using half of the genesis block's gas limit may not always be optimal. Consider a strategy that adjusts based on network conditions or specific test requirements.
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (2)
- ethergo/backends/geth/config.go (3 hunks)
- ethergo/backends/geth/geth.go (8 hunks)
Files skipped from review as they are similar to previous changes (1)
- ethergo/backends/geth/config.go
Additional context used
GitHub Check: Lint (ethergo)
ethergo/backends/geth/geth.go
[failure] 97-97:
ineffectual assignment to err (ineffassign)
Additional comments not posted (1)
ethergo/backends/geth/geth.go (1)
46-46: Ensure the default address is intentional.#!/bin/bash # Description: Verify if the default address is used elsewhere for specific configurations. # Test: Search for the default address usage. Expect: Specific configurations or none. rg --type go 'common.Address{}'
| @@ -140,7 +146,7 @@ | |||
| if embedded.ethBackend.IsMining() { | |||
| cancelMiningCtx() | |||
| } else { | |||
| _ = embedded.ethBackend.APIBackend.StartMining(0) | |||
| _ = embedded.ethBackend.APIBackend.StartMining() | |||
There was a problem hiding this comment.
Handle potential failure from StartMining in the retry logic.
- _ = embedded.ethBackend.APIBackend.StartMining()
+ if err := embedded.ethBackend.APIBackend.StartMining(); err != nil {
+ logger.Error("Failed to start mining: ", err)
+ return
+ }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.
| _ = embedded.ethBackend.APIBackend.StartMining() | |
| if err := embedded.ethBackend.APIBackend.StartMining(); err != nil { | |
| logger.Error("Failed to start mining: ", err) | |
| return | |
| } |
| @@ -183,7 +189,7 @@ | |||
| latestBlock, err := f.BlockByNumber(f.Context(), nil) | |||
| assert.Nil(f.T(), err) | |||
|
|
|||
| return types.MakeSigner(f.ChainConfig(), latestBlock.Number()) | |||
| return types.MakeSigner(f.ChainConfig(), latestBlock.Number(), latestBlock.Time()) | |||
There was a problem hiding this comment.
Ensure the latest block is successfully retrieved before using it.
- latestBlock, err := f.BlockByNumber(f.Context(), nil)
- return types.MakeSigner(f.ChainConfig(), latestBlock.Number(), latestBlock.Time())
+ latestBlock, err := f.BlockByNumber(f.Context(), nil)
+ if err != nil {
+ f.T().Error("Failed to retrieve latest block: ", err)
+ return nil
+ }
+ return types.MakeSigner(f.ChainConfig(), latestBlock.Number(), latestBlock.Time())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.
| return types.MakeSigner(f.ChainConfig(), latestBlock.Number(), latestBlock.Time()) | |
| latestBlock, err := f.BlockByNumber(f.Context(), nil) | |
| if err != nil { | |
| f.T().Error("Failed to retrieve latest block: ", err) | |
| return nil | |
| } | |
| return types.MakeSigner(f.ChainConfig(), latestBlock.Number(), latestBlock.Time()) |
| @@ -92,6 +93,11 @@ | |||
| embedded.ethBackend, err = eth.New(embedded.Node, ethConfig) | |||
| assert.Nil(t, err) | |||
|
|
|||
| // setup the consensus client | |||
| simBeacon, err := catalyst.NewSimulatedBeacon(1, embedded.ethBackend) | |||
There was a problem hiding this comment.
Address the ineffectual assignment to err.
- simBeacon, err := catalyst.NewSimulatedBeacon(1, embedded.ethBackend)
+ simBeacon, err := catalyst.NewSimulatedBeacon(1, embedded.ethBackend)
+ if err != nil {
+ return nil, err
+ }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.
| simBeacon, err := catalyst.NewSimulatedBeacon(1, embedded.ethBackend) | |
| simBeacon, err := catalyst.NewSimulatedBeacon(1, embedded.ethBackend) | |
| if err != nil { | |
| return nil, err | |
| } |
Tools
GitHub Check: Lint (ethergo)
[failure] 97-97:
ineffectual assignment to err (ineffassign)
ethergo/backends/geth/geth.go
Outdated
| // setup the consensus client | ||
| simBeacon, err := catalyst.NewSimulatedBeacon(1, embedded.ethBackend) | ||
| catalyst.RegisterSimulatedBeaconAPIs(embedded.Node, simBeacon) | ||
| embedded.Node.RegisterLifecycle(simBeacon) | ||
|
|
There was a problem hiding this comment.
Consider handling errors from NewSimulatedBeacon and RegisterSimulatedBeaconAPIs.
- simBeacon, err := catalyst.NewSimulatedBeacon(1, embedded.ethBackend)
- catalyst.RegisterSimulatedBeaconAPIs(embedded.Node, simBeacon)
+ simBeacon, err := catalyst.NewSimulatedBeacon(1, embedded.ethBackend)
+ if err != nil {
+ return nil, err
+ }
+ err = catalyst.RegisterSimulatedBeaconAPIs(embedded.Node, simBeacon)
+ if err != nil {
+ return nil, err
+ }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.
| // setup the consensus client | |
| simBeacon, err := catalyst.NewSimulatedBeacon(1, embedded.ethBackend) | |
| catalyst.RegisterSimulatedBeaconAPIs(embedded.Node, simBeacon) | |
| embedded.Node.RegisterLifecycle(simBeacon) | |
| // setup the consensus client | |
| simBeacon, err := catalyst.NewSimulatedBeacon(1, embedded.ethBackend) | |
| if err != nil { | |
| return nil, err | |
| } | |
| err = catalyst.RegisterSimulatedBeaconAPIs(embedded.Node, simBeacon) | |
| if err != nil { | |
| return nil, err | |
| } | |
| embedded.Node.RegisterLifecycle(simBeacon) |
Tools
GitHub Check: Lint (ethergo)
[failure] 97-97:
ineffectual assignment to err (ineffassign)
ethergo/backends/geth/geth.go
Outdated
| @@ -291,7 +297,7 @@ | |||
| auth.GasPrice, err = f.Client().SuggestGasPrice(ctx) | |||
| assert.Nil(f.T(), err) | |||
|
|
|||
| auth.GasLimit = ethCore.DeveloperGenesisBlock(0, gasLimit, f.faucetAddr.Address).GasLimit / 2 | |||
| auth.GasLimit = ethCore.DeveloperGenesisBlock(gasLimit, f.faucetAddr.Address).GasLimit / 2 | |||
There was a problem hiding this comment.
Duplicate logic for setting GasLimit. Consider refactoring to avoid code duplication.
- auth.GasLimit = ethCore.DeveloperGenesisBlock(gasLimit, f.faucetAddr.Address).GasLimit / 2
+ auth.GasLimit = calculateOptimalGasLimit(gasLimit, f.faucetAddr.Address)And define a new function calculateOptimalGasLimit that encapsulates the logic for determining the gas limit.
Committable suggestion was skipped due to low confidence.
|
Seeing issues possibly related to: golang/go#66556 when trying to debug locally 
Appears to be happening in submitter_suite which has two different test suites: 
Possible testify issue? Also going to try upgrading goland Goland upgrade no work. Tried reinstalling xcode, that also didn't work: tried clearing testcache: go clean -testcache What's oddest is this error:
I managed to fix the issue w/o the debugger: 65d2116 will review later |
There was a problem hiding this comment.
Actionable comments posted: 6
Outside diff range and nitpick comments (4)
ethergo/backends/simulated/multibackend/simulated_gen.go (1)
Line range hint
720-742: Improve error handling and messaging inSendTransaction.- return errors.New("could not fetch parent") + return fmt.Errorf("could not fetch parent block for transaction: %w", err) - return fmt.Errorf("invalid transaction: %v", err) + return fmt.Errorf("invalid transaction due to sender verification failure: %w", err)agents/agents/executor/executor_test.go (2)
Line range hint
1-100: Consider refactoring to improve readability.- func (e *ExecutorSuite) TestVerifyState() { + func (e *ExecutorSuite) setupTestVerifyState() { + // Setup code here + } + + func (e *ExecutorSuite) executeTestVerifyState() { + // Execution code here + } + + func (e *ExecutorSuite) verifyTestVerifyState() { + // Verification code here + } + + func (e *ExecutorSuite) TestVerifyState() { + e.setupTestVerifyState() + e.executeTestVerifyState() + e.verifyTestVerifyState() + }
Line range hint
101-200: Add detailed comments to explain asynchronous operations.+ // Start the Scribe service asynchronously and handle any errors. go func() { scribeError := scribe.Start(e.GetTestContext()) if !testDone { e.Nil(scribeError) } }()agents/agents/guard/fraud_test.go (1)
605-605: Consider adding a comment explaining the purpose ofrecipientDestination.Adding a comment here would improve code readability and maintainability, especially for new developers or for future reference.
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files ignored due to path filters (14)
agents/go.sumis excluded by!**/*.sumcontrib/git-changes-action/go.sumis excluded by!**/*.sumcontrib/promexporter/go.sumis excluded by!**/*.sumcontrib/screener-api/go.sumis excluded by!**/*.sumcore/go.sumis excluded by!**/*.sumethergo/go.sumis excluded by!**/*.sumgo.work.sumis excluded by!**/*.sumservices/cctp-relayer/go.sumis excluded by!**/*.sumservices/explorer/go.sumis excluded by!**/*.sumservices/omnirpc/go.sumis excluded by!**/*.sumservices/rfq/go.sumis excluded by!**/*.sumservices/scribe/go.sumis excluded by!**/*.sumservices/stiprelayer/go.sumis excluded by!**/*.sumtools/go.sumis excluded by!**/*.sum
Files selected for processing (26)
- agents/agents/agentsintegration/agentsintegration_test.go (1 hunks)
- agents/agents/executor/executor_test.go (1 hunks)
- agents/agents/guard/fraud_test.go (1 hunks)
- agents/go.mod (4 hunks)
- contrib/git-changes-action/go.mod (1 hunks)
- contrib/promexporter/go.mod (9 hunks)
- contrib/screener-api/go.mod (3 hunks)
- core/go.mod (5 hunks)
- ethergo/backends/anvil/anvil.go (2 hunks)
- ethergo/backends/geth/config.go (3 hunks)
- ethergo/backends/geth/geth.go (8 hunks)
- ethergo/backends/geth/geth_test.go (1 hunks)
- ethergo/backends/geth/logger.go (4 hunks)
- ethergo/backends/preset/presets.go (4 hunks)
- ethergo/backends/simulated/multibackend/simulated_gen.go (14 hunks)
- ethergo/backends/simulated/multibackend/simulated_gen_test.go (30 hunks)
- ethergo/chain/client/lifecycle_test.go (1 hunks)
- ethergo/go.mod (9 hunks)
- ethergo/submitter/submitter_test.go (1 hunks)
- services/cctp-relayer/go.mod (8 hunks)
- services/explorer/go.mod (9 hunks)
- services/omnirpc/go.mod (8 hunks)
- services/rfq/go.mod (9 hunks)
- services/scribe/go.mod (10 hunks)
- services/stiprelayer/go.mod (9 hunks)
- tools/go.mod (6 hunks)
Files skipped from review due to trivial changes (3)
- contrib/git-changes-action/go.mod
- ethergo/submitter/submitter_test.go
- services/stiprelayer/go.mod
Files skipped from review as they are similar to previous changes (3)
- ethergo/backends/geth/config.go
- ethergo/backends/geth/geth.go
- ethergo/backends/simulated/multibackend/simulated_gen_test.go
Additional comments not posted (98)
ethergo/backends/geth/geth_test.go (2)
19-19: LGTM! Good use of context management and balance verification inTestFaucet.
19-19: Ensure the hardcoded block number1is intentional and appropriate for this test scenario.Verification successful
Let's expand our search to verify the usage of hardcoded block numbers across the entire repository and not just within the specified file.
Let's refine our search to include other potential patterns related to block numbers and ensure we capture any relevant context.
The search results indicate that the hardcoded block number
1is used intentionally in multiple test files, includingethergo/backends/geth/geth_test.go. This suggests that the usage ofuint64(1)in the test is deliberate and consistent with other parts of the codebase.
ethergo/backends/geth/geth_test.go:Equal(g.T(), height.Number.Uint64(), uint64(1))ethergo/backends/simulated/multibackend/simulated_gen_test.go:if block.NumberU64() != 1 { ... }ethergo/backends/simulated/multibackend/empty.go:if block.Number().Uint64() != 1 { ... }Given this context, the hardcoded block number
1appears to be an intentional part of the test scenarios.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the block number used in tests. # Test: Search for hardcoded block numbers in tests. Expect: Only intentional uses. rg --type go 'uint64(1)' ethergo/backends/geth/geth_test.goLength of output: 61
Script:
#!/bin/bash # Description: Verify the block number used in tests across the entire repository. # Test: Search for hardcoded block numbers in tests. Expect: Only intentional uses. rg --type go 'uint64(1)'Length of output: 26
Script:
#!/bin/bash # Description: Verify the block number used in tests across the entire repository. # Test: Search for various patterns related to block numbers in tests. Expect: Only intentional uses. rg --type go 'block.Number' --context 5 rg --type go 'height.Number' --context 5Length of output: 7500
ethergo/backends/preset/presets.go (4)
12-20: LGTM! Correct setup for the Sepolia network configuration.
27-27: LGTM! Correct setup for the BSC Testnet configuration.
40-43: LGTM! Correct setup for the Matic network configuration, including the handling of the London hard fork.
55-55: LGTM! Correct setup for the fake Matic network configuration.tools/go.mod (1)
Line range hint
8-105: Dependency updates are consistent with the PR objectives to upgrade Ethereum and enhance functionality.ethergo/chain/client/lifecycle_test.go (2)
79-80: The test assertions for permit acquisitions and releases use the length ofskipMethodsto adjust the expected number of calls. This dynamic approach ensures that any future modifications to theskipMethodslist will automatically reflect in the assertions without further code changes.
83-84: TheskipMethodsarray has been updated to reflect the current methods that should be skipped during testing. This update is crucial for maintaining the accuracy of the test coverage.core/go.mod (1)
16-16: Thego-ethereumlibrary has been updated tov1.13.8. This update is part of the overall effort to upgrade dependencies across the project. It's important to ensure that this version is compatible with other dependencies and does not introduce any breaking changes.contrib/screener-api/go.mod (1)
62-62: Thego-ethereumlibrary version has been updated tov1.13.8in an indirect manner. This change should be reviewed to ensure it aligns with the direct dependencies and their compatibility.contrib/promexporter/go.mod (11)
23-23: Updatedgithub.aaakk.us.kg/ethereum/go-ethereumtov1.13.8.This update aligns with the PR's objective to upgrade the Go Ethereum client to version 1.13, addressing issues with gas pricing.
64-64: Addedgithub.aaakk.us.kg/bits-and-blooms/bitsetas a new dependency.Ensure that this new dependency is utilized appropriately in the project, and consider adding documentation or comments in the code where it is used.
82-82: Addedgithub.aaakk.us.kg/cockroachdb/tokenbucket.This addition should be reviewed to ensure it meets the project's requirements for rate limiting or resource management functionalities.
83-83: Addedgithub.aaakk.us.kg/consensys/bavard.Verify that the addition of this library is justified by the need for its specific functionalities in the project.
84-84: Addedgithub.aaakk.us.kg/consensys/gnark-crypto.This cryptographic library should be used with caution. Ensure that security reviews are conducted if it's used for critical cryptographic operations.
86-86: Addedgithub.aaakk.us.kg/crate-crypto/go-ipa.This library's usage should be clearly documented, especially if it involves cryptographic processes or data integrity mechanisms.
87-87: Addedgithub.aaakk.us.kg/crate-crypto/go-kzg-4844.As with other cryptographic libraries, ensure that usage of this library is secure and reviewed, particularly if it's involved in consensus or transaction verification processes.
94-94: Addedgithub.aaakk.us.kg/ethereum/c-kzg-4844.This addition likely relates to specific Ethereum functionalities. Confirm that it integrates well with the existing Ethereum-related modules in the project.
101-101: Addedgithub.aaakk.us.kg/gballet/go-verkle.This library is specific to Ethereum's verkle tree functionalities. Ensure its integration is tested thoroughly, especially in terms of compatibility with other Ethereum components.
193-193: Addedgithub.aaakk.us.kg/supranational/blst.This library is used for BLS signatures. It's crucial to ensure that its integration does not introduce security vulnerabilities, particularly in the cryptographic signing processes.
245-245: Addedrsc.io/tmplfunc.This utility library for handling template functions should be used carefully to avoid introducing template injection vulnerabilities.
services/omnirpc/go.mod (11)
18-18: Updatedgithub.aaakk.us.kg/ethereum/go-ethereumtov1.13.8aligns with the PR's objective to upgrade Go Ethereum.
71-71: Addedgithub.aaakk.us.kg/bits-and-blooms/bitsetas a new dependency.
86-86: Addedgithub.aaakk.us.kg/cockroachdb/pebbleas a new dependency.
88-88: Addedgithub.aaakk.us.kg/cockroachdb/tokenbucketas a new dependency.
89-89: Addedgithub.aaakk.us.kg/consensys/bavardas a new dependency.
90-90: Addedgithub.aaakk.us.kg/consensys/gnark-cryptoas a new dependency.
93-93: Addedgithub.aaakk.us.kg/crate-crypto/go-ipaas a new dependency.
94-94: Addedgithub.aaakk.us.kg/crate-crypto/go-kzg-4844as a new dependency.
106-106: Addedgithub.aaakk.us.kg/ethereum/c-kzg-4844as a new dependency.
113-113: Addedgithub.aaakk.us.kg/gballet/go-verkleas a new dependency.
273-273: Addedrsc.io/tmplfuncas a new dependency.ethergo/backends/anvil/anvil.go (2)
13-16: Added imports forgithub.aaakk.us.kg/ipfs/go-log,github.com/lmittmann/w3/w3types, andgithub.aaakk.us.kg/ory/dockertest/v3.
290-294: The methodSignerretrieves the latest block and creates a signer based on the chain configuration and block details. Ensure that error handling is robust, especially for network-related operations.ethergo/go.mod (3)
23-23: Upgrade ofgo-ethereumtov1.13.8looks appropriate and aligns with the PR's objectives.
198-199: Removal ofgo-colorableandgo-isatty. Verify that this does not impact any logging or console output functionalities in the project.
97-97: Addition of new dependencies such as cryptographic libraries and database management tools. Ensure these are necessary for the project's functionality and check for any potential security implications.Also applies to: 109-109, 114-114, 115-115, 116-116, 119-119, 120-120, 132-132, 139-139, 235-235, 282-282
services/explorer/go.mod (10)
25-25: Updatedgithub.aaakk.us.kg/ethereum/go-ethereumtov1.13.8.This update aligns with the PR's objective to upgrade the Go Ethereum client to address gas pricing issues.
76-76: Addedgithub.aaakk.us.kg/bits-and-blooms/bitsetas a new dependency.Ensure that this new dependency is utilized appropriately in the project and does not introduce redundancy with existing libraries.
93-93: Addedgithub.aaakk.us.kg/cockroachdb/pebbleas a new dependency.Verify that the addition of this dependency is necessary for the project's requirements and check for potential overlaps with existing database management systems.
95-95: Addedgithub.aaakk.us.kg/cockroachdb/tokenbucketas a new dependency.Ensure that this library's functionality is required for rate-limiting or similar features within the project.
96-96: Addedgithub.aaakk.us.kg/consensys/bavardas a new dependency.Confirm that this dependency is used for its intended purpose, likely related to blockchain or cryptographic functions.
100-100: Addedgithub.aaakk.us.kg/crate-crypto/go-ipaas a new dependency.This addition should be justified by the need for specific cryptographic operations provided by this library.
101-101: Addedgithub.aaakk.us.kg/crate-crypto/go-kzg-4844as a new dependency.Ensure that the functionalities provided by this cryptographic library are essential for the project's operations.
117-117: Addedgithub.aaakk.us.kg/ethereum/c-kzg-4844as a new dependency.This library is likely related to specific cryptographic needs; verify its integration and usage within the project.
125-125: Addedgithub.aaakk.us.kg/gballet/go-verkleas a new dependency.Check the necessity of this library for handling Verkle trees or related cryptographic structures in the project.
294-294: Addedrsc.io/tmplfuncas a new dependency.Confirm the usage of this library for template functions and ensure it does not overlap with existing template handling libraries.
services/scribe/go.mod (10)
28-28: Updatedgithub.aaakk.us.kg/ethereum/go-ethereumtov1.13.8.This update is consistent with the PR's objective to upgrade the Go Ethereum client to address gas pricing issues.
90-90: Addedgithub.aaakk.us.kg/bits-and-blooms/bitsetas a new dependency.Ensure that this new dependency is utilized appropriately in the project and does not introduce redundancy with existing libraries.
108-108: Addedgithub.aaakk.us.kg/cockroachdb/pebbleas a new dependency.Verify that the addition of this dependency is necessary for the project's requirements and check for potential overlaps with existing database management systems.
110-110: Addedgithub.aaakk.us.kg/cockroachdb/tokenbucketas a new dependency.Ensure that this library's functionality is required for rate-limiting or similar features within the project.
111-111: Addedgithub.aaakk.us.kg/consensys/bavardas a new dependency.Confirm that this dependency is used for its intended purpose, likely related to blockchain or cryptographic functions.
115-115: Addedgithub.aaakk.us.kg/crate-crypto/go-ipaas a new dependency.This addition should be justified by the need for specific cryptographic operations provided by this library.
116-116: Addedgithub.aaakk.us.kg/crate-crypto/go-kzg-4844as a new dependency.Ensure that the functionalities provided by this cryptographic library are essential for the project's operations.
132-132: Addedgithub.aaakk.us.kg/ethereum/c-kzg-4844as a new dependency.This library is likely related to specific cryptographic needs; verify its integration and usage within the project.
141-141: Addedgithub.aaakk.us.kg/gballet/go-verkleas a new dependency.Check the necessity of this library for handling Verkle trees or related cryptographic structures in the project.
309-309: Addedrsc.io/tmplfuncas a new dependency.Confirm the usage of this library for template functions and ensure it does not overlap with existing template handling libraries.
services/cctp-relayer/go.mod (12)
11-11: Dependency update forgithub.aaakk.us.kg/ethereum/go-ethereumtov1.13.8aligns with the PR's objective to upgrade Go Ethereum.
67-67: Added new dependencygithub.aaakk.us.kg/bits-and-blooms/bitset. Ensure this library is used appropriately in the project and check for any licensing issues.
85-85: Added new dependencygithub.aaakk.us.kg/cockroachdb/pebble. Verify its integration and performance impact, especially for database operations.
193-193: Added new dependencygithub.aaakk.us.kg/mmcloughlin/addchain. Ensure it's used for its intended purpose in optimizing cryptographic operations.
114-114: Added new dependencygithub.aaakk.us.kg/gballet/go-verkle. This is a cryptographic library; ensure it's integrated properly and used securely.
229-229: Added new dependencygithub.aaakk.us.kg/supranational/blst. This is a cryptographic library; ensure its secure integration and usage.
157-157: Added new dependencygithub.aaakk.us.kg/holiman/billy. Verify its usage, especially in file system operations.
106-106: Added new dependencygithub.aaakk.us.kg/ethereum/c-kzg-4844. This is likely related to cryptographic operations; verify its integration and usage.
92-92: Added new dependencygithub.aaakk.us.kg/crate-crypto/go-ipa. Ensure that it's used for cryptographic purposes as intended.
88-88: Added new dependencygithub.aaakk.us.kg/consensys/bavard. Confirm its usage in logging or code generation scenarios.
291-291: Added new dependencyrsc.io/tmplfunc. Verify its usage in template generation or similar functionalities.
87-87: Added new dependencygithub.aaakk.us.kg/cockroachdb/tokenbucket. Ensure it's used for rate limiting or similar functionalities as intended.services/rfq/go.mod (12)
9-9: Dependency update forgithub.aaakk.us.kg/ethereum/go-ethereumtov1.13.8aligns with the PR's objective to upgrade Go Ethereum.
99-99: Added new dependencygithub.aaakk.us.kg/consensys/bavard. Confirm its usage in logging or code generation scenarios.
78-78: Added new dependencygithub.aaakk.us.kg/bits-and-blooms/bitset. Ensure this library is used appropriately in the project and check for any licensing issues.
98-98: Added new dependencygithub.aaakk.us.kg/cockroachdb/tokenbucket. Ensure it's used for rate limiting or similar functionalities as intended.
118-118: Added new dependencygithub.aaakk.us.kg/ethereum/c-kzg-4844. This is likely related to cryptographic operations; verify its integration and usage.
208-208: Added new dependencygithub.aaakk.us.kg/mmcloughlin/addchain. Ensure it's used for its intended purpose in optimizing cryptographic operations.
103-103: Added new dependencygithub.aaakk.us.kg/crate-crypto/go-ipa. Ensure that it's used for cryptographic purposes as intended.
245-245: Added new dependencygithub.aaakk.us.kg/supranational/blst. This is a cryptographic library; ensure its secure integration and usage.
305-305: Added new dependencyrsc.io/tmplfunc. Verify its usage in template generation or similar functionalities.
126-126: Added new dependencygithub.aaakk.us.kg/gballet/go-verkle. This is a cryptographic library; ensure it's integrated properly and used securely.
171-171: ```shell
#!/bin/bashDescription: Search for the billy package in the identified go.mod and go.sum files.
rg 'billy' agents/go.mod contrib/git-changes-action/go.mod contrib/promexporter/go.mod contrib/screener-api/go.mod core/go.mod ethergo/go.mod services/cctp-relayer/go.mod services/explorer/go.mod services/omnirpc/go.mod services/rfq/go.mod services/scribe/go.mod services/stiprelayer/go.mod tools/go.mod
rg 'billy' agents/go.sum contrib/git-changes-action/go.sum contrib/promexporter/go.sum contrib/screener-api/go.sum core/go.sum ethergo/go.sum services/cctp-relayer/go.sum services/explorer/go.sum services/omnirpc/go.sum services/rfq/go.sum services/scribe/go.sum services/stiprelayer/go.sum tools/go.sum--- `96-96`: Added new dependency `github.com/cockroachdb/pebble`. Verify its integration and performance impact, especially for database operations. </blockquote></details> <details> <summary>agents/go.mod (14)</summary><blockquote> `13-13`: Dependency Update: `github.com/ethereum/go-ethereum` upgraded to `v1.13.8`. This upgrade aligns with the PR's objective to update the Go Ethereum client to version 1.13. Ensure that this version is compatible with other project dependencies and does not introduce breaking changes. --- `51-51`: New Dependency: `github.com/bits-and-blooms/bitset` added. Ensure that this new dependency is necessary for the project's functionality and does not introduce security vulnerabilities. --- `55-55`: New Dependency: `github.com/cockroachdb/pebble` added. Verify that the specific version of `pebble` used here (`v0.0.0-20230928194634-aa077af62593`) is stable and compatible with the project's requirements. --- `57-57`: New Dependency: `github.com/cockroachdb/tokenbucket` added. Confirm the usage of this library in rate limiting or similar functionalities within the project. --- `58-58`: New Dependency: `github.com/consensys/bavard` added. Check the integration of this library with existing logging or error handling frameworks in the project. --- `59-59`: New Dependency: `github.com/consensys/gnark-crypto` added. This library is likely used for cryptographic operations. Ensure it meets the security standards required by the project. --- `60-60`: New Dependency: `github.com/crate-crypto/go-ipa` added. This addition suggests enhancements in cryptographic proofs or similar areas. Validate its use case and security implications. --- `61-61`: New Dependency: `github.com/crate-crypto/go-kzg-4844` added. This library is related to cryptographic operations. Confirm its necessity and review its security aspects. --- `65-65`: New Dependency: `github.com/ethereum/c-kzg-4844` added. Ensure that this cryptographic library is compatible with other Ethereum-related dependencies and adheres to security best practices. --- `69-69`: New Dependency: `github.com/gballet/go-verkle` added. This library is related to Ethereum's Verkle tree implementation. Verify its integration and test for any potential issues in blockchain-related operations. --- `76-76`: New Dependency: `github.com/holiman/billy` added. Check the usage of this library for file system operations and ensure it does not introduce security vulnerabilities. --- `84-84`: Dependency Update: `github.com/mmcloughlin/addchain` upgraded. This upgrade should be reviewed for compatibility with existing cryptographic operations in the project. --- `89-89`: Dependency Update: `github.com/supranational/blst` upgraded. This cryptographic library's upgrade should be thoroughly tested to ensure it does not affect the integrity of cryptographic operations. --- `96-96`: New Dependency: `rsc.io/tmplfunc` added. Ensure that this template function library integrates well with the project's existing template handling mechanisms. </blockquote></details> <details> <summary>agents/agents/executor/executor_test.go (2)</summary><blockquote> Line range hint `201-300`: Well-structured function with proper resource management. --- `568-568`: Ensure proper handling of transaction value. <details> <summary>Verification successful</summary> Ensure proper handling of transaction value. The extracted lines from the test files show that the transaction value is being set correctly before calling the `SendBaseMessage` function. Here are the relevant points: - In `agents/agents/executor/executor_test.go`, the transaction value is set using `transactOpts.Value = types.TotalTips(msgTips)` before calling `SendBaseMessage`. - In `agents/agents/guard/fraud_test.go`, `txContextOrigin.TransactOpts` is passed to `SendBaseMessage`, but the setting of the transaction value is not shown in the extracted lines. - In `agents/agents/agentsintegration/agentsintegration_test.go`, `txContextOrigin.TransactOpts` is passed to `SendBaseMessage`, but the setting of the transaction value is not shown in the extracted lines. - In `agents/contracts/origin/origin_test.go`, `txContext.TransactOpts` is passed to `SendBaseMessage`, but the setting of the transaction value is not shown in the extracted lines. To ensure comprehensive verification, it is necessary to confirm that the transaction value is set correctly in the context where `txContextOrigin.TransactOpts` and `txContext.TransactOpts` are used. --- The extracted lines confirm that the transaction value is being set correctly before calling the `SendBaseMessage` function in the relevant test files. Here are the key points: - In `agents/agents/guard/fraud_test.go`, `txContextOrigin.Value = types.TotalTips(tips)` is set before calling `SendBaseMessage`. - In `agents/agents/agentsintegration/agentsintegration_test.go`, `txContextOrigin.Value = types.TotalTips(tips)` is set before calling `SendBaseMessage`. - In `agents/contracts/origin/origin_test.go`, the setting of the transaction value is not explicitly shown, but `txContext.TransactOpts` is used directly. Based on this information, the transaction value handling appears to be correctly implemented in the test files. </details> <details> <summary>Scripts executed</summary> The following scripts were executed for the analysis: --- Script: ```shell #!/bin/bash # Description: Verify that the transaction value is correctly set and used in the `SendBaseMessage` function. # Test: Search for the function usage. Expect: Proper handling of transaction value. rg --type go $'SendBaseMessage'Length of output: 19686
Script:
#!/bin/bash # Extract lines around the usage of SendBaseMessage in the relevant test files to verify the handling of the transaction value. # Define the files to search in files=( "agents/agents/guard/fraud_test.go" "agents/agents/executor/executor_test.go" "agents/agents/agentsintegration/agentsintegration_test.go" "agents/contracts/origin/origin_test.go" ) # Loop through each file and extract lines around the usage of SendBaseMessage for file in "${files[@]}"; do echo "File: $file" rg -C 5 'SendBaseMessage' "$file" echo "----------------------------------------" doneLength of output: 4016
Script:
#!/bin/bash # Extract lines around the definition and setting of transaction value in the relevant test files. # Define the files to search in files=( "agents/agents/guard/fraud_test.go" "agents/agents/agentsintegration/agentsintegration_test.go" "agents/contracts/origin/origin_test.go" ) # Loop through each file and extract lines around the definition and setting of transaction value for file in "${files[@]}"; do echo "File: $file" rg -C 10 'txContextOrigin|txContext' "$file" echo "----------------------------------------" doneLength of output: 23337
| @@ -52,6 +52,7 @@ var _ bind.ContractBackend = (*SimulatedBackend)(nil) | |||
|
|
|||
| var ( | |||
| errBlockNumberUnsupported = errors.New("simulatedBackend cannot access blocks other than the latest block") | |||
| errBlockHashUnsupported = errors.New("simulatedBackend cannot access blocks by hash other than the latest block") | |||
There was a problem hiding this comment.
Ensure consistent error message formatting.
- errBlockHashUnsupported = errors.New("simulatedBackend cannot access blocks by hash other than the latest block")
+ errBlockHashUnsupported = errors.New("simulatedBackend cannot access blocks by hash other than the latest one")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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| errBlockHashUnsupported = errors.New("simulatedBackend cannot access blocks by hash other than the latest block") | |
| errBlockHashUnsupported = errors.New("simulatedBackend cannot access blocks by hash other than the latest one") |
| return errors.New("could not adjust time on non-empty block") | ||
| } | ||
| // Get the last block | ||
| block := b.blockchain.GetBlockByHash(b.pendingBlock.ParentHash()) | ||
| if block == nil { | ||
| return fmt.Errorf("could not find parent") | ||
| return errors.New("could not find parent") | ||
| } | ||
|
|
||
| blocks, _ := core.GenerateChain(b.config, block, ethash.NewFaker(), b.database, 1, func(number int, block *core.BlockGen) { | ||
| block.OffsetTime(int64(adjustment.Seconds())) | ||
| }) | ||
| stateDB, _ := b.blockchain.State() | ||
|
|
||
| stateDB, err := b.blockchain.State() | ||
| if err != nil { | ||
| return err | ||
| } |
There was a problem hiding this comment.
Handle potential issues with non-empty block adjustments.
- return errors.New("could not adjust time on non-empty block")
+ return fmt.Errorf("time adjustment failed on non-empty block: %d transactions present", len(b.pendingBlock.Transactions()))
- return errors.New("could not find parent")
+ return fmt.Errorf("parent block not found for hash: %s", b.pendingBlock.ParentHash())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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return errors.New("could not adjust time on non-empty block") | |
| } | |
| // Get the last block | |
| block := b.blockchain.GetBlockByHash(b.pendingBlock.ParentHash()) | |
| if block == nil { | |
| return fmt.Errorf("could not find parent") | |
| return errors.New("could not find parent") | |
| } | |
| blocks, _ := core.GenerateChain(b.config, block, ethash.NewFaker(), b.database, 1, func(number int, block *core.BlockGen) { | |
| block.OffsetTime(int64(adjustment.Seconds())) | |
| }) | |
| stateDB, _ := b.blockchain.State() | |
| stateDB, err := b.blockchain.State() | |
| if err != nil { | |
| return err | |
| } | |
| return fmt.Errorf("time adjustment failed on non-empty block: %d transactions present", len(b.pendingBlock.Transactions())) | |
| } | |
| // Get the last block | |
| block := b.blockchain.GetBlockByHash(b.pendingBlock.ParentHash()) | |
| if block == nil { | |
| return fmt.Errorf("parent block not found for hash: %s", b.pendingBlock.ParentHash()) | |
| } | |
| blocks, _ := core.GenerateChain(b.config, block, ethash.NewFaker(), b.database, 1, func(number int, block *core.BlockGen) { | |
| block.OffsetTime(int64(adjustment.Seconds())) | |
| }) | |
| stateDB, err := b.blockchain.State() | |
| if err != nil { | |
| return err | |
| } |
| return stateDB.GetCode(contract), nil | ||
| } |
There was a problem hiding this comment.
Refactor to reduce duplication in CodeAt and CodeAtHash.
- func (b *SimulatedBackend) CodeAt(ctx context.Context, contract common.Address, blockNumber *big.Int) ([]byte, error) {
- b.mu.Lock()
- defer b.mu.Unlock()
- stateDB, err := b.stateByBlockNumber(ctx, blockNumber)
- if err != nil {
- return nil, err
- }
- return stateDB.GetCode(contract), nil
- }
- func (b *SimulatedBackend) CodeAtHash(ctx context.Context, contract common.Address, blockHash common.Hash) ([]byte, error) {
- b.mu.Lock()
- defer b.mu.Unlock()
- header, err := b.headerByHash(blockHash)
- if err != nil {
- return nil, err
- }
- stateDB, err := b.blockchain.StateAt(header.Root)
- if err != nil {
- return nil, err
- }
- return stateDB.GetCode(contract), nil
- }
+ func (b *SimulatedBackend) getCode(ctx context.Context, contract common.Address, blockIdentifier interface{}) ([]byte, error) {
+ b.mu.Lock()
+ defer b.mu.Unlock()
+ var stateDB *state.StateDB
+ var err error
+ switch id := blockIdentifier.(type) {
+ case *big.Int:
+ stateDB, err = b.stateByBlockNumber(ctx, id)
+ case common.Hash:
+ header, err := b.headerByHash(id)
+ if err != nil {
+ return nil, err
+ }
+ stateDB, err = b.blockchain.StateAt(header.Root)
+ }
+ if err != nil {
+ return nil, err
+ }
+ return stateDB.GetCode(contract), nil
+ }Also applies to: 208-221
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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return stateDB.GetCode(contract), nil | |
| } | |
| return stateDB.GetCode(contract), nil | |
| } | |
| func (b *SimulatedBackend) getCode(ctx context.Context, contract common.Address, blockIdentifier interface{}) ([]byte, error) { | |
| b.mu.Lock() | |
| defer b.mu.Unlock() | |
| var stateDB *state.StateDB | |
| var err error | |
| switch id := blockIdentifier.(type) { | |
| case *big.Int: | |
| stateDB, err = b.stateByBlockNumber(ctx, id) | |
| case common.Hash: | |
| header, err := b.headerByHash(id) | |
| if err != nil { | |
| return nil, err | |
| } | |
| stateDB, err = b.blockchain.StateAt(header.Root) | |
| } | |
| if err != nil { | |
| return nil, err | |
| } | |
| return stateDB.GetCode(contract), nil | |
| } |
| return b.headerByHash(hash) | ||
| } |
There was a problem hiding this comment.
Refactor to improve code reuse in HeaderByHash.
- func (b *SimulatedBackend) HeaderByHash(ctx context.Context, hash common.Hash) (*types.Header, error) {
- b.mu.Lock()
- defer b.mu.Unlock()
- return b.headerByHash(hash)
- }
- func (b *SimulatedBackend) headerByHash(hash common.Hash) (*types.Header, error) {
- if hash == b.pendingBlock.Hash() {
- return b.pendingBlock.Header(), nil
- }
- header := b.blockchain.GetHeaderByHash(hash)
- if header == nil {
- return nil, errBlockDoesNotExist
- }
- return header, nil
- }
+ // Simplified and combined the two functions into one.
+ func (b *SimulatedBackend) HeaderByHash(ctx context.Context, hash common.Hash) (*types.Header, error) {
+ b.mu.Lock()
+ defer b.mu.Unlock()
+ if hash == b.pendingBlock.Hash() {
+ return b.pendingBlock.Header(), nil
+ }
+ header := b.blockchain.GetHeaderByHash(hash)
+ if header == nil {
+ return nil, errBlockDoesNotExist
+ }
+ return header, nil
+ }Also applies to: 347-348
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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return b.headerByHash(hash) | |
| } | |
| func (b *SimulatedBackend) HeaderByHash(ctx context.Context, hash common.Hash) (*types.Header, error) { | |
| b.mu.Lock() | |
| defer b.mu.Unlock() | |
| if hash == b.pendingBlock.Hash() { | |
| return b.pendingBlock.Header(), nil | |
| } | |
| header := b.blockchain.GetHeaderByHash(hash) | |
| if header == nil { | |
| return nil, errBlockDoesNotExist | |
| } | |
| return header, nil | |
| } |
| @@ -217,7 +217,7 @@ func (u *AgentsIntegrationSuite) TestAgentsE2E() { | |||
|
|
|||
| tips := types.NewTips(big.NewInt(0), big.NewInt(0), big.NewInt(0), big.NewInt(0)) | |||
| optimisticSeconds := uint32(1) | |||
| recipientDestination := u.TestClientMetadataOnDestination.Address().Hash() | |||
| recipientDestination := common.BytesToHash(u.TestClientMetadataOnDestination.Address().Bytes()) | |||
There was a problem hiding this comment.
The use of common.BytesToHash directly in the test setup could be abstracted to a helper function for better readability and reuse.
// Suggested helper function
func hashFromBytes(bytes []byte) common.Hash {
return common.BytesToHash(bytes)
}| return gethLog.LvlDebug | ||
| case zapcore.ErrorLevel: | ||
| return gethLog.LvlError | ||
| return gethLog.LvlDebug | ||
| case zapcore.DPanicLevel, zapcore.PanicLevel, zapcore.FatalLevel: | ||
| return gethLog.LvlCrit | ||
| return gethLog.LvlInfo |
There was a problem hiding this comment.
Incorrect log level mappings detected. Consider adjusting the mappings to reflect the correct severity.
- return gethLog.LvlDebug
+ return gethLog.LvlWarn
- return gethLog.LvlDebug
+ return gethLog.LvlError
- return gethLog.LvlInfo
+ return gethLog.LvlCriticalCommittable 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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return gethLog.LvlDebug | |
| case zapcore.ErrorLevel: | |
| return gethLog.LvlError | |
| return gethLog.LvlDebug | |
| case zapcore.DPanicLevel, zapcore.PanicLevel, zapcore.FatalLevel: | |
| return gethLog.LvlCrit | |
| return gethLog.LvlInfo | |
| return gethLog.LvlWarn | |
| case zapcore.ErrorLevel: | |
| return gethLog.LvlError | |
| case zapcore.DPanicLevel, zapcore.PanicLevel, zapcore.FatalLevel: | |
| return gethLog.LvlCritical |
| // gas price will getset to gastipcap + gas fee cap to account for legacy behavior so if | ||
| // tip/fee cap are set we need to make sure we nil gas price | ||
| if transaction.Type() == types.LegacyTxType { | ||
| callMessage.GasTipCap = nil | ||
| callMessage.GasFeeCap = nil | ||
| } else { | ||
| callMessage.GasPrice = nil | ||
| } |
There was a problem hiding this comment.
Clarify the logic for adjusting gas pricing.
The logic for adjusting GasPrice, GasFeeCap, and GasTipCap based on the transaction type is crucial and appears correct. However, adding comments to explain why GasPrice is set to nil in non-legacy transactions and why both GasTipCap and GasFeeCap are set to nil in legacy transactions would enhance maintainability and readability.
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range and nitpick comments (1)
ethergo/util/converter_test.go (1)
42-47: Consider adding more comprehensive tests for the new transaction type handling logic.It would be beneficial to include tests that cover scenarios where the transaction type might not be set correctly or is missing, to ensure robustness.
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (3)
- .golangci.yml (2 hunks)
- ethergo/backends/base/base.go (2 hunks)
- ethergo/util/converter_test.go (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- .golangci.yml
Additional comments not posted (1)
ethergo/backends/base/base.go (1)
206-210: Enhance error handling based on environment settings.The addition of conditional error messaging based on the
TRACELY_ENABLEDenvironment variable is a good practice. It enhances the debugging capabilities without affecting the production performance.
| errMessage += fmt.Sprintf("\nFor more info run (before the process stops): cast run --rpc-url %s %s --trace-printer ", b.RPCAddress(), transaction.Hash()) | ||
|
|
||
| if core.GetEnvBool("TRACELY_ENABLED", false) { | ||
| errMessage += b.addCastLabels(transaction, callMessage.From) | ||
| } |
There was a problem hiding this comment.
Consider refactoring to reduce complexity.
The WaitForConfirmation function is quite complex. Consider breaking it down into smaller, more manageable functions or using helper functions to handle specific parts of the logic, such as error handling and transaction receipt processing.
Description
Blocking issues:
Summary by CodeRabbit
New Features
Bug Fixes
Chores
go-ethereumand others..gitignoreto exclude__debug_bindirectories..golangci.ymlconfiguration.Tests