feat(api): improve web3_clientVersion
#1404
Conversation
WalkthroughThe changes involve a rebranding in the Cosmos build scripts, minor tweaks in the Cosmos runtime, and significant updates in the Ethereum compatibility layer to include a host chain version. The Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (7)
- build/scripts/cosmos.mk (1 hunks)
- cosmos/runtime/miner/miner.go (1 hunks)
- cosmos/x/evm/keeper/host.go (2 hunks)
- eth/core/host.go (1 hunks)
- eth/polar/api/web3.go (1 hunks)
- eth/polar/api_backend.go (4 hunks)
- eth/polar/backend.go (2 hunks)
Files skipped from review due to trivial changes (1)
- cosmos/runtime/miner/miner.go
Additional comments: 11
eth/polar/api/web3.go (2)
- 46-48: The
NewWeb3APIfunction now returns theWeb3Backendinterface instead of a concrete type. This change could affect how the function is used throughout the codebase. Ensure that all calls to this function have been updated to handle the interface type correctly.Verification successful
The search results indicate that the
NewWeb3APIfunction is referenced ineth/polar/backend.goand is defined ineth/polar/api/web3.go. There are no other files listed, which suggests that the usage of this function might be limited within the codebase, or that the function is not used outside of theeth/polarpackage.Given this information, it seems that the change in the function signature of
NewWeb3APIto accept and return aWeb3Backendtype is localized and may not have widespread implications across the entire codebase. However, it is still important to ensure that the change is properly handled ineth/polar/backend.gowhere the function is used.* 43-51: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [50-53]Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for files that use the NewWeb3API function. Expecting list of dependent files. rg --type go "NewWeb3API"Length of output: 187
The
ClientVersionandSha3methods remain unchanged and correctly implemented.build/scripts/cosmos.mk (1)
- 72-73: The
ldflagsvariable has been updated to change theNameandAppNamevalues to "polard". Verify that this renaming is consistent throughout the entire codebase and that no references to the old names remain.Verification successful
The search results indicate that the terms "sim" and "simd" are still present in the
main.mkfile, but they do not appear to be related to theNameandAppNamevalues that were changed incosmos.mk. Instead, they seem to be associated with simulation paths and commands. Therefore, the renaming of application identifiers to "polard" incosmos.mkdoes not seem to be inconsistent with these findings.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for the old application identifiers "sim" and "simd" to ensure they have been fully replaced. rg --type make "sim" rg --type make "simd"Length of output: 577
cosmos/x/evm/keeper/host.go (2)
36-36: The import of
github.com/cosmos/cosmos-sdk/versionhas been added to support the newVersion()method.116-119: The
Version()method has been added to theHoststruct to return the version of the host chain. Ensure that this new method is being used correctly wherever the host chain version is required.Verification successful
The output from the
rgcommand shows several files where theHoststruct or the term "host" is mentioned. To verify the correct usage of the newVersion()method, we need to focus on instances where theHoststruct from thecosmos/x/evm/keeperpackage is used, as this is where the method is defined.From the output, we can see that the
Hoststruct is referenced in thecosmos/x/evm/keeperpackage files, such askeeper.go,host.go,genesis.go, andgenesis_test.go. We need to check if theVersion()method is being called on instances of theHoststruct in these files.To proceed, we should create a script to search for the usage of the
Version()method in the context of theHoststruct within thecosmos/x/evm/keeperpackage.
To verify the usage of the
Version()method across the entire codebase, we will create a script to search for calls to this method on instances of theHoststruct in all.gofiles.
To further investigate the usage of the
Version()method, we will create scripts to extract more context around the calls ineth/polar/backend.goandeth/polar/api/net.go.
Based on the new context provided:
The
host.Version()call ineth/polar/backend.gois used correctly to pass the host chain version information to theNewAPIBackendfunction, which aligns with the intended use of theVersion()method added to theHoststruct.The
api.b.Version()call ineth/polar/api/net.gois part of a method that exposes the version information through thenetAPIstruct. Although it's not explicitly clear ifapi.bis an instance of theHoststruct, the usage pattern is consistent with common practices of delegation or proxying, suggesting that this is also a correct usage of theVersion()method.Therefore, the review comment has been addressed, and the
Version()method appears to be used correctly in the provided contexts.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for files that use the Host struct to ensure the new Version method is being utilized correctly. rg --type go "Host"Length of output: 4951
Script:
#!/bin/bash # Search for the usage of the Version() method on instances of the Host struct within the cosmos/x/evm/keeper package. rg --type go -A 3 "func \(h \*Host\) Version\(\)" cosmos/x/evm/keeper/Length of output: 355
Script:
#!/bin/bash # Search for calls to the Version() method on instances of the Host struct across all .go files in the codebase. rg --type go "\.Version\(\)"Length of output: 217
Script:
#!/bin/bash # Extract more context around the Version() call in eth/polar/backend.go rg --type go -C 5 "host\.Version\(\)" eth/polar/backend.go # Extract more context around the Version() call in eth/polar/api/net.go rg --type go -C 5 "api\.b\.Version\(\)" eth/polar/api/net.goLength of output: 608
eth/core/host.go (1)
- 45-46: The
PolarisHostChaininterface has been extended with a newVersion()method. This change is consistent with the addition of theVersion()method in theHoststruct in the Cosmos-related files.eth/polar/backend.go (1)
- 180-182: The
APIsmethod has been reformatted to passpl.apiBackendas an argument topolarapi.NewWeb3API, aligning with the updated function signature. Ensure that theAPIsmethod is correctly updated wherever it is used.Verification successful
The output from the
eth/eth.gofile shows an interface methodRegisterAPIs([]rpc.API)which indicates that it expects an array ofrpc.APIobjects. This is consistent with the usage ofpl.APIs()ineth/polar/backend.go, wherestack.RegisterAPIs(pl.APIs())is called. Sincepl.APIs()returns an array ofrpc.APIand this is whatRegisterAPIsexpects, the usage is correct and aligns with the updated method signature.Based on the information gathered, the
APIsmethod ineth/polar/backend.gois correctly updated and is being used properly in the codebase. The review comment is verified as the changes align with the updated function signature, and the method is used correctly wherever it is called.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for files that use the APIs method of the Polaris type. Expecting list of dependent files. rg --type go "APIs\("Length of output: 397
Script:
#!/bin/bash # Check the usage of `APIs()` in `eth/polar/backend.go` where it is used with `stack.RegisterAPIs(pl.APIs())` cat eth/polar/backend.go | grep -A 5 -B 5 "stack.RegisterAPIs(pl.APIs())" # Check the usage of `APIs()` in `eth/eth.go` to ensure compatibility with the updated `APIs()` method signature. cat eth/eth.go | grep -A 5 -B 5 "RegisterAPIs([]rpc.API)"Length of output: 554
Script:
#!/bin/bash # Correcting the syntax error and re-running the script to check the usage of `APIs()` in `eth/eth.go`. cat eth/eth.go | grep -A 5 -B 5 "RegisterAPIs(\[\]rpc.API"Length of output: 455
eth/polar/api_backend.go (4)
26-28: The addition of the "fmt" and "strings" packages is appropriate for the string formatting required in the
ClientVersionmethod.83-83: A new field
hostChainVersionhas been added to thebackendstruct. Ensure that all instances wherebackendis instantiated are updated to initialize this new field.98-105: The
NewAPIBackendfunction now accepts an additional parameterhostChainVersion. This change will require updates whereverNewAPIBackendis called to include the new argument.Verification successful
The verification script has confirmed that the
NewAPIBackendfunction is being called with the newhostChainVersionparameter ineth/polar/backend.go. This suggests that the necessary updates to include the new argument have been made in this part of the codebase.* 649-651: The `ClientVersion` method has been updated to return a formatted string that includes the `hostChainVersion` and the client name. This change seems to align with the PR's objective to include host chain version information in the API response. However, ensure that the formatting of the version string meets the expected standards and that the `hostChainVersion` is properly validated before use.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for instances where NewAPIBackend is called to ensure the new parameter is being passed. ast-grep --lang go --pattern $'NewAPIBackend($$$)'Length of output: 244
| pl.apiBackend = NewAPIBackend( | ||
| pl, stack.ExtRPCEnabled(), allowUnprotectedTxs, pl.config, host.Version(), |
There was a problem hiding this comment.
The New function call in eth.go does not include the host.Version() argument. This needs to be updated to match the new function signature in eth/polar/backend.go.
Analysis chain
The New function has been updated to include the host chain version when creating a new API backend. Verify that the New function is correctly updated wherever it is used to ensure the host chain version is passed as expected.
This script should help us identify the specific New function for the Polaris type by looking for the function definition and then checking for the presence of Polaris in the surrounding lines.
This script will search for the usage of the New function within the eth.go file, providing some context lines to understand how the function is being called. We're looking to confirm that the host.Version() argument is included in the call.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for files that use the New function of the Polaris type. Expecting list of dependent files.
rg --type go "New\("
Length of output: 11353
Script:
#!/bin/bash
# Filter the output to find usages of the New function specifically for the Polaris type.
rg --type go -A 2 "func New\(" | rg "Polaris"
Length of output: 160
Script:
#!/bin/bash
# Verify the usage of the New function in the eth.go file to ensure it includes the host.Version() argument.
rg --type go -C 3 "polar.New\(" eth/eth.go
Length of output: 277
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #1404 +/- ##
==========================================
- Coverage 47.36% 47.32% -0.04%
==========================================
Files 85 85
Lines 5061 5065 +4
==========================================
Hits 2397 2397
- Misses 2487 2491 +4
Partials 177 177
|
Summary by CodeRabbit
New Features
Refactor
Style
Documentation