-
Notifications
You must be signed in to change notification settings - Fork 170
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add symbol name to the x/uibc QueryInflowsResponse #2377
Conversation
WalkthroughThe changes involve refining the representation and retrieval of financial data in the 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? TipsChat with CodeRabbit Bot (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to filter (2)
- swagger/swagger.yaml
- x/uibc/query.pb.go
Files selected for processing (2)
- proto/umee/uibc/v1/query.proto (2 hunks)
- x/uibc/quota/grpc_query.go (1 hunks)
Additional comments: 6
x/uibc/quota/grpc_query.go (4)
- 59-59: Verify the implementation of
coinsWithTokenSymbols
method to ensure it correctly populates theoutflows
array with token symbols.Verification successful
The
coinsWithTokenSymbols
method does exist in the codebase and is defined in the filex/uibc/quota/token.go
with the receiver typeKeeper
. This confirms that the method is implemented and should be populating theoutflows
array with token symbols as intended in theAllOutflows
method.* 67-67: Ensure that the renaming of `inflows` to `inflowCoins` is consistent throughout the codebase and does not cause any issues.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for the definition of coinsWithTokenSymbols within the codebase. ast-grep --lang go --pattern $'func ($_ Querier) coinsWithTokenSymbols($_, $_) $$$'Length of output: 84
Script:
#!/bin/bash # Broaden the search for the coinsWithTokenSymbols method without assuming the receiver type or the exact parameters. ast-grep --lang go --pattern $'func $$$ coinsWithTokenSymbols($$$) $$$'Length of output: 72
Script:
#!/bin/bash # Use ripgrep to search for the coinsWithTokenSymbols method in the codebase. rg 'func .*coinsWithTokenSymbols'Length of output: 155
Verification successful
The search results indicate that the term
inflows
is still present in various parts of the codebase. However, it is important to note that the context of the usage matters. The terminflows
appears in comments, method names, and variable names in different files. The renaming in theInflows
method toinflowCoins
seems to be specific to thegrpc_query.go
file and does not necessarily imply that all instances ofinflows
throughout the codebase would be renamed.The presence of
inflows
in thequery.pb.go
,quota_test.go
,quota.go
,grpc_query.go
,genesis.go
,query.go
,quota.pb.go
,query.pb.gw.go
, andgenesis.pb.go
files suggests thatinflows
is a term used in various contexts, such as protobuf definitions, test cases, keeper methods, and genesis state.The specific change mentioned in the review comment refers to the
Inflows
method in thegrpc_query.go
file, where the variable was renamed toinflowCoins
. The search results show that within thegrpc_query.go
file, the variableinflows
has been updated toinflowCoins
on line 80, which is consistent with the change described in the review comment.Given the information provided, it appears that the renaming of
inflows
toinflowCoins
within theInflows
method is consistent and does not cause issues within the scope of the changes made in thegrpc_query.go
file. However, the terminflows
is still used in other contexts within the codebase, which is expected and does not indicate an issue.* 69-69: Confirm that the new method of initializing the keeper with `q.Keeper(&ctx)` is correctly implemented and consistent throughout the codebase.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for the usage of the old inflows variable to ensure it has been updated everywhere. rg --type go 'inflows' --vimgrepLength of output: 3284
Verification successful
The search results show that the
q.Keeper(&ctx)
initialization is used consistently across various methods in thegrpc_query.go
file within thex/uibc/quota
directory, as well as in thegrpc_query.go
file within thex/metoken/keeper
directory. This indicates that the new method of initializing the keeper withq.Keeper(&ctx)
is indeed implemented consistently throughout the codebase.Based on the provided output, the change to initialize the keeper with
q.Keeper(&ctx)
is:
- : The verification is successful, and the new method of initializing the keeper is used consistently across the codebase.
* 80-80: Verify the correct usage of `coinsWithTokenSymbols` method in the `Inflows` method to ensure inflows are populated with token symbols accurately.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for the usage of q.Keeper to ensure the new initialization is used consistently. rg --type go 'q.Keeper\(&ctx\)' --vimgrepLength of output: 900
Verification successful
The
ast-grep
command has successfully matched theInflows
method in thegrpc_query.go
file and provided the relevant code snippet. From the output, we can see the usage of thecoinsWithTokenSymbols
method:inflows := k.coinsWithTokenSymbols(ctx, inflowCoins)This line of code is within the
Inflows
method and shows thatinflowCoins
is passed to thecoinsWithTokenSymbols
method of thek
(keeper) object, withctx
as the context. The result is assigned to theinflows
variable, which is then used to construct theQueryInflowsResponse
object returned by the method.Based on the provided output, the
coinsWithTokenSymbols
method is indeed used within theInflows
method to populate theinflows
with token symbols, which aligns with the review comment.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for the usage of coinsWithTokenSymbols within the Inflows method. ast-grep --lang go --pattern $'func ($_ Querier) Inflows($_, $_) $$$ { $$$ coinsWithTokenSymbols($_, $_) $$$ }'Length of output: 112
Script:
#!/bin/bash # Search for the Inflows method to inspect its implementation and verify the usage of coinsWithTokenSymbols. ast-grep --lang go --pattern $'func ($_ Querier) Inflows($_, $_) $$$ { $$$ }'Length of output: 1251
proto/umee/uibc/v1/query.proto (2)
1-6: Removal of the import statement for "cosmos/base/v1beta1/coin.proto" is appropriate if it's no longer used after the changes.
70-70: The change to the
inflows
field type in theQueryInflowsResponse
message torepeated DecCoinSymbol
aligns with the PR objectives. Ensure this new type is consistently used and properly handled throughout the codebase.Verification successful
The search results confirm that the
cosmos.base.v1beta1.DecCoin
type has been replaced byDecCoinSymbol
in theuibc
module'squery.proto
file as intended by PR 2377. The type is still present in other.proto
files, but this is outside the scope of the PR in question.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for the usage of the old DecCoin type to ensure it has been replaced by DecCoinSymbol. rg --type proto 'cosmos.base.v1beta1.DecCoin' --vimgrepLength of output: 85
Script:
#!/bin/bash # Search for the usage of the old DecCoin type to ensure it has been replaced by DecCoinSymbol. # Removing the --type flag to include all files in the search. rg 'cosmos.base.v1beta1.DecCoin' --vimgrepLength of output: 1365
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files selected for processing (2)
- CHANGELOG.md (1 hunks)
- x/uibc/quota/token.go (1 hunks)
Files skipped from review due to trivial changes (1)
- CHANGELOG.md
Additional comments: 1
x/uibc/quota/token.go (1)
- 11-12: The function assumes that
GetAllRegisteredTokens
will always provide a complete mapping of base denoms to symbol denoms. Verify thatGetAllRegisteredTokens
has proper error handling and that there are no scenarios where a token could be registered without a symbol.Verification successful
The
GetAllRegisteredTokens
method is designed to either return a non-nil list of tokens or panic if it encounters an error, as indicated by its implementation usingstore.MustLoadAll
. Therefore, the assumption in thetoken.go
file that the method will always provide a complete mapping is consistent with the method's behavior. There is no need for additional error handling in the calling function since the method does not return an error.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that GetAllRegisteredTokens handles errors appropriately and that all registered tokens have symbols. rg --type go 'GetAllRegisteredTokens' --context 5Length of output: 17739
Script:
#!/bin/bash # Look at the implementation of GetAllRegisteredTokens in iter.go and any other potential implementations. rg --type go 'func \(.*Keeper\) GetAllRegisteredTokens' --context 5Length of output: 1355
Description
Following the outlfows update, here we update inflows
Summary by CodeRabbit
Refactor
Bug Fixes