rip out tenderly (prereq to #1767) #1863
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 1 minutes and 52 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 project has undergone a significant refactoring, with a focus on simplifying the backend infrastructure by removing dependencies on 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 (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1863 +/- ##
===================================================
+ Coverage 51.08388% 51.61072% +0.52684%
===================================================
Files 397 409 +12
Lines 27171 28062 +891
Branches 307 307
===================================================
+ Hits 13880 14483 +603
- Misses 11936 12159 +223
- Partials 1355 1420 +65
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 (18)
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:!**/*.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/Makefile (1 hunks)
- ethergo/backends/base/base.go (5 hunks)
- ethergo/backends/geth/geth_test.go (1 hunks)
- ethergo/parser/hardhat/export_test.go (1 hunks)
- ethergo/parser/hardhat/parser.go (6 hunks)
- ethergo/parser/hardhat/parser_test.go (4 hunks)
- ethergo/parser/hardhat/types.go (1 hunks)
Files skipped from review due to trivial changes (1)
- ethergo/backends/geth/geth_test.go
Additional comments: 15
ethergo/parser/hardhat/export_test.go (3)
5-6: The changes to
GetCompilerandGetSourcesmethods to returnContractCompilerandContractSourcestypes respectively are consistent with the removal of thetenderly-clidependency and the refactoring of the codebase to use internal types.18-18: The
GetMetadatafunction now accepts aHardhatContractinstead ofhardhat.HardhatContract. Ensure that all references to this function have been updated to pass the correct type.22-22: The
GetNetworksfunction has been updated to acceptHardhatContractand return a map ofContractNetwork. This change should be verified across the codebase to ensure compatibility.Verification successful
The broader ast-grep search has successfully located the definition of the
GetNetworksfunction withinethergo/parser/hardhat/export_test.go. This confirms the presence of the function in the codebase, and since no calls to this function were found in the previous searches, it can be inferred that the update to theGetNetworksfunction's signature does not affect any existing code.Based on the information gathered, the review comment concerning the need to verify compatibility across the codebase for the
GetNetworksfunction update appears to be addressed. There are no instances of the function being called with either the old or new type, which means there are no compatibility issues detected within the current scope of the codebase.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for instances where GetNetworks is called with the old type. ast-grep --lang go --pattern $'GetNetworks(hardhat.HardhatContract($_), $_)'Length of output: 77
Script:
#!/bin/bash # Search for any instances where GetNetworks is called, regardless of the type passed. ast-grep --lang go --pattern $'GetNetworks($_, $_)'Length of output: 52
Script:
#!/bin/bash # Verify the presence of the GetNetworks function in the codebase. ast-grep --lang go --pattern $'func GetNetworks($_, $_) { $$$ }'Length of output: 65
Script:
#!/bin/bash # Broaden the search to find the definition of the GetNetworks function without specifying the function body structure. ast-grep --lang go --pattern $'func GetNetworks($_, $_)'Length of output: 326
ethergo/Makefile (2)
1-1: The inclusion of
../make/go.Makefileat the top of theMakefilesuggests that common make functionalities are being centralized. This is a good practice for maintainability and reuse of make logic.1-1: > Note: This review was outside the patches, and no patch overlapping with it was found. Original lines [78-82]
The initialization of the
Backendstruct has been refactored to remove thetenderlyanddebugfields. This aligns with the PR's objective to remove dependencies ontenderly-cli.ethergo/parser/hardhat/parser_test.go (3)
8-8: The import path has been corrected to reference the
hardhatpackage directly. This change is likely part of the refactoring process and should be consistent throughout the codebase.18-18: The call to
GetDeploymentshas been updated to use thehardhatpackage. This change should be verified to ensure that all instances ofGetDeploymentsare updated accordingly.Verification successful
Given the results of the ast-grep searches, which did not find any instances of
GetDeploymentsbeing used either withhardhatParserorhardhat, and considering the broader search forGetDeploymentsalso returned no results, it appears that the method is not present in the codebase outside of the instance mentioned in the review comment. This suggests that the change to use thehardhatpackage for theGetDeploymentscall has been implemented correctly and there are no remaining references to an old package qualifier.Based on the information gathered, the review comment has been addressed properly in the codebase.
* 37-38: The `GetNetworks` function is now being called with the `hardhat.HardhatContract` type. This change should be verified to ensure that all instances of `GetNetworks` are updated accordingly.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for instances where GetDeployments is called with the old package. ast-grep --lang go --pattern $'hardhatParser.GetDeployments($_)'Length of output: 65
Script:
#!/bin/bash # Search for instances where GetDeployments is called with the new package. ast-grep --lang go --pattern $'hardhat.GetDeployments($_)'Length of output: 59
Script:
#!/bin/bash # Broad search for any usage of GetDeployments to ensure it's not being called with a different qualifier. ast-grep --lang go --pattern $'GetDeployments($_)'Length of output: 51
Verification successful
The ast-grep search for the
GetNetworksfunction usage has returned results fromethergo/parser/hardhat/parser_test.go, showing that the function is indeed being called with the newhardhat.HardhatContracttype. Since no other instances were found, it suggests that either this is the only place whereGetNetworksis used or all instances have been updated to the new type.Based on the provided context and the results of the scripts, it appears that the review comment has been addressed correctly in the codebase.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for instances where GetNetworks is called with the old type. ast-grep --lang go --pattern $'hardhatParser.GetNetworks($_, $_)'Length of output: 66
Script:
#!/bin/bash # Search for any usage of the GetNetworks function to ensure all instances have been updated. ast-grep --lang go --pattern $'hardhat.GetNetworks($_, $_)'Length of output: 374
ethergo/parser/hardhat/types.go (1)
- 8-23: The
Contractstruct has been updated with new fields and types. This change is likely part of the refactoring process to remove thetenderly-clidependency and should be consistent with the new data structures used throughout the codebase.ethergo/parser/hardhat/parser.go (4)
16-17: The
hardhatMetadatastruct has been updated to useContractCompilerandContractSourcestypes. This change is consistent with the refactoring of the codebase to use internal types instead of those provided bytenderly-cli.21-21: The
getMetadatafunction's parameter and return types have been changed to useHardhatContractandhardhatMetadatatypes. This change should be verified across the codebase to ensure compatibility.32-32: The
getNetworksfunction's parameter and return types have been changed to useHardhatContractandContractNetworktypes. This change should be verified across the codebase to ensure compatibility.62-62: The
GetDeploymentsfunction's return type has been changed to useContracttype. This change should be verified across the codebase to ensure compatibility.Verification successful
The search results indicate that the
GetDeploymentsfunction is indeed being used in multiple places within the codebase:
services/scribe/config/generate_test.goservices/scribe/config/generate.goethergo/parser/hardhat/parser_test.goIn each of these instances, the function is called and the return value is being handled. Since the function signature in
ethergo/parser/hardhat/parser.gohas been updated to return[]Contract, any code that calls this function must be compatible with the new return type.Given that the search results do not indicate any issues with the usage of the updated
GetDeploymentsfunction, and there are no errors or mismatches in the return type handling, it appears that the change to theGetDeploymentsfunction's return type has been successfully integrated into the codebase.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for instances where GetDeployments is called with the old return type. ast-grep --lang go --pattern $'GetDeployments($_)'Length of output: 51
Script:
#!/bin/bash # Broaden the search to find any usage of GetDeployments to verify the impact of the return type change. rg 'GetDeployments' --goLength of output: 453
Script:
#!/bin/bash # Correct the search to find any usage of GetDeployments within Go files. rg 'GetDeployments' -g '*.go'Length of output: 538
ethergo/backends/base/base.go (2)
78-82: The
Backendstruct has been refactored to remove thetenderlyanddebugfields, which is in line with the PR's objective to remove thetenderly-clidependency.78-82: The logic within the
VerifyContractandWaitForConfirmationmethods has been modified. Ensure that these changes do not introduce any regressions or unexpected behaviors.
| Chain: chn, | ||
| ctx: ctx, | ||
| t: t, | ||
| Manager: nonce.NewNonceManager(ctx, chn, chn.GetBigChainID()), | ||
| store: NewInMemoryKeyStore(), |
There was a problem hiding this comment.
The search using rg for the string "EnableTenderly" has returned several results. It appears that the EnableTenderly method is still present in the codebase in various files:
ethergo/backends/mocks/simulated_test_backend.go: Contains a mock function definition and implementation ofEnableTenderly.ethergo/backends/simulated/simulated_test.go: Calls theEnableTenderlymethod.ethergo/backends/simulated/simulated.go: Contains a comment explaining the purpose ofEnableTenderlyand its implementation for the simulated backend.ethergo/backends/backend.go: Contains an interface definition that includesEnableTenderly.
These findings indicate that there are indeed remaining references and calls to the EnableTenderly method in the codebase. This contradicts the review comment's expectation that the method should have been completely removed.
Analysis chain
The EnableTenderly method has been removed from the Backend. This is expected as part of the removal of the tenderly-cli dependency. Ensure that there are no remaining calls to this method in the codebase.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for instances where EnableTenderly is called.
ast-grep --lang go --pattern $'EnableTenderly()'
Length of output: 49
Script:
#!/bin/bash
# Perform a more general search for the string "EnableTenderly" to ensure it's not being called anywhere.
rg "EnableTenderly"
Length of output: 734
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to path filters (7)
agents/go.sumis excluded by:!**/*.sumcontrib/promexporter/go.sumis excluded by:!**/*.sumservices/cctp-relayer/go.sumis excluded by:!**/*.sumservices/explorer/go.sumis excluded by:!**/*.sumservices/scribe/go.modis excluded by:!**/*.modservices/scribe/go.sumis excluded by:!**/*.sumservices/sinner/go.sumis excluded by:!**/*.sum
Files selected for processing (4)
- ethergo/backends/backend.go (1 hunks)
- ethergo/backends/mocks/simulated_test_backend.go (1 hunks)
- ethergo/backends/simulated/simulated.go (1 hunks)
- services/scribe/config/generate_test.go (2 hunks)
Additional comments: 4
ethergo/backends/backend.go (1)
- 27-32: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [1-59]
The removal of the
EnableTenderlymethod from theSimulatedTestBackendinterface aligns with the PR's objective to removetenderly-clidependencies. Ensure that all implementations of this interface and any code that relied on this method have been appropriately refactored.services/scribe/config/generate_test.go (1)
- 11-16: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [1-82]
The removal of the Tenderly import and the change in the
getDeploymentsTxHashfunction parameter type from[]providers.Contractto[]hardhat.Contractare consistent with the PR's goal of removing Tenderly dependencies. Verify that the newhardhat.Contracttype provides all necessary fields and methods that were used from the oldproviders.Contracttype.Verification successful
The
hardhat.Contracttype has been successfully located inethergo/parser/hardhat/types.goand contains several fields such asName,Abi,Bytecode,DeployedBytecode,SourceMap,DeployedSourceMap,Source,SourcePath,Ast,Compiler,Networks,SchemaVersion, andUpdatedAt. These fields are likely to provide the necessary data previously obtained from theproviders.Contracttype.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the hardhat.Contract type has the necessary fields and methods. ast-grep --lang go --pattern $'type Contract struct { $$$ }' src/ethergo/parser/hardhat/types.goLength of output: 180
Script:
#!/bin/bash # Search for the file that defines the hardhat.Contract type fd 'types.go' --exec ast-grep --lang go --pattern $'type Contract struct { $$$ }'Length of output: 1510
ethergo/backends/simulated/simulated.go (1)
- 74-79: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [1-261]
The removal of the
EnableTenderlymethod from theBackendtype is in line with the PR's objectives. Confirm that no other parts of the codebase are attempting to call this method on instances ofBackend.ethergo/backends/mocks/simulated_test_backend.go (1)
- 370-375: > Note: This review was outside the patches, and no patch overlapping with it was found. Original lines [1-366]
The removal of the
EnableTenderlyfunction from theSimulatedTestBackendmock is consistent with the removal of the method from the interface. Ensure that any tests that might have used this mock function are updated accordingly.
Description
Rremoves tenderly-cli dependency
Summary by CodeRabbit
Refactor
Bug Fixes
Documentation
Chores