feat: add symbol name to the x/uibc QueryAllOutflowsResponse #2374
Conversation
WalkthroughThe updates across various files in the Umee protocol reflect a focus on enhancing data structures and improving query functionalities. A new 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? TipsChat with CodeRabbit Bot (
|
![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: 4
Configuration used: CodeRabbit UI
Files ignored due to filter (2)
- x/uibc/query.pb.go
- x/uibc/uibc.pb.go
Files selected for processing (10)
- proto/umee/uibc/v1/query.proto (2 hunks)
- proto/umee/uibc/v1/uibc.proto (1 hunks)
- util/genmap/genmap.go (1 hunks)
- util/genmap/genmap_test.go (2 hunks)
- x/leverage/keeper/grpc_query.go (16 hunks)
- x/metoken/mocks/keepers.go (1 hunks)
- x/uibc/expected_keepers.go (1 hunks)
- x/uibc/mocks/keepers.go (1 hunks)
- x/uibc/quota/grpc_query.go (1 hunks)
- x/uibc/quota/mocks_test.go (2 hunks)
Files skipped from review due to trivial changes (1)
- x/metoken/mocks/keepers.go
Additional comments: 20
proto/umee/uibc/v1/uibc.proto (1)
- 18-27: The
DecCoinSymbolmessage is well-defined with appropriate field tags and types to include a symbol name with theDecCointype.x/uibc/expected_keepers.go (1)
- 19-19: The addition of the
GetAllRegisteredTokensmethod to theLeverageinterface is correctly implemented.util/genmap/genmap_test.go (1)
- 34-46: The
TestMapToSlicefunction correctly tests theMapToSlicefunction with various map inputs, including an empty map.x/uibc/quota/mocks_test.go (1)
- 26-28: The addition of the
GetAllRegisteredTokensmethod to theLeverageKeepermock is correctly implemented and returns a slice of tokens.x/uibc/quota/grpc_query.go (1)
- 54-69: The
AllOutflowsmethod is correctly updated to include token symbols in the response. Ensure that theGetAllRegisteredTokensmethod has proper error handling since it's used here without checking for errors.Verification successful
The
GetAllRegisteredTokensmethod inx/leverage/keeper/iter.gousesstore.MustLoadAll, which panics if an error occurs. There is no conventional error handling within this method; it relies on the panic-recover mechanism. This approach is consistent with the use ofutil.Panicin other iterator methods within the same file, indicating a pattern of error handling in this codebase.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that GetAllRegisteredTokens has proper error handling rg --type go "GetAllRegisteredTokens"Length of output: 2495
Script:
#!/bin/bash # Inspect the implementation of GetAllRegisteredTokens in x/leverage/keeper/iter.go cat x/leverage/keeper/iter.goLength of output: 6801
proto/umee/uibc/v1/query.proto (1)
- 106-106: The
QueryAllOutflowsResponsemessage is correctly updated to use theDecCoinSymboltype for theoutflowsfield, enhancing the response data structure.x/uibc/mocks/keepers.go (1)
- 115-127: The
GetAllRegisteredTokensmethod and its mock recorder are correctly added to theMockLeveragestruct, following the mock generation conventions.x/leverage/keeper/grpc_query.go (13)
34-34: The direct call to
q.GetParamsreplacesq.Keeper.GetParams. Ensure that theGetParamsmethod is correctly implemented on theQuerierstruct and that it maintains the same behavior as the previousKeepermethod.51-57: The logic for retrieving token settings based on
req.BaseDenomand getting all registered tokens has been refactored to use methods directly on theQuerierstruct. Verify that these methods are correctly implemented and handle errors as expected.Verification successful
The shell script has successfully located the
GetTokenSettingsandGetAllRegisteredTokensmethods in various files within the codebase. Here are the findings:
The
GetTokenSettingsmethod is implemented in:
x/uibc/quota/mocks_test.gox/uibc/mocks/keepers.gox/incentive/keeper/mock_test.gox/leverage/keeper/token.gox/metoken/mocks/keepers.gox/metoken/keeper/mocks_test.goThe
GetAllRegisteredTokensmethod is implemented in:
x/uibc/quota/mocks_test.gox/uibc/mocks/keepers.gox/leverage/keeper/iter.goThe presence of these methods in the
x/leverage/keeper/token.goandx/leverage/keeper/iter.gofiles suggests that the methods do exist and are likely implemented correctly, as they are part of the actualKeeperstruct and not just the mocks or tests.Given this information, the review comment asking to verify that these methods are correctly implemented and handle errors as expected on the
Querierstruct is not entirely accurate, as the methods are found on theKeeperstruct. However, since the PR's changes involve refactoring to use methods directly on theQuerierstruct, it's important to ensure that theQuerierstruct has these methods or appropriately delegates them to theKeeperstruct.Based on the script output, it seems that the methods are indeed part of the
Keeperstruct, and the PR may have refactored the code to call these methods directly on theQuerierstruct, which would be a correct approach if theQuerierstruct is set up to delegate to theKeeperstruct.* 77-80: The `GetAllSpecialAssetPairs` and `GetSpecialAssetPairs` methods are now called directly on the `Querier` struct. Confirm that these methods are present and correctly implemented in the `Querier` struct.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the GetTokenSettings and GetAllRegisteredTokens methods exist and are implemented correctly in the Querier struct. ast-grep --lang go --pattern $'func (q Querier) GetTokenSettings($_, $_) $_ { $$$ }' ast-grep --lang go --pattern $'func (q Querier) GetAllRegisteredTokens($_) $_ { $$$ }'Length of output: 173
Script:
#!/bin/bash # Search for the GetTokenSettings and GetAllRegisteredTokens methods in the entire codebase. ast-grep --lang go --pattern $'func ($_ $_) GetTokenSettings($_, $_) $_ { $$$ }' ast-grep --lang go --pattern $'func ($_ $_) GetAllRegisteredTokens($_) $_ { $$$ }'Length of output: 4924
100-120: The
MarketSummarymethod has been refactored to use direct calls toQueriermethods. Ensure that these methods are correctly implemented and that the logic for calculating market summary values is preserved.167-173: The
TokenPricemethod is used to fetch the oracle price and historic price. Ensure that theTokenPricemethod is correctly implemented in theQuerierstruct and that it handles errors appropriately.201-206: The methods
GetAllSupplied,GetBorrowerCollateral, andGetBorrowerBorrowsare now called directly on theQuerierstruct. Confirm that these methods are correctly implemented and that they handle errors as expected.233-267: The
AccountSummarymethod has been refactored to use direct calls toQueriermethods for fetching account information and calculating values. Ensure that these methods are correctly implemented and that the logic for calculating account summary values is preserved.283-286: The
GetAccountPositionmethod is now called directly on theQuerierstruct. Verify that this method is correctly implemented in theQuerierstruct and that it correctly calculates the account position.296-299: The
GetAccountPositionmethod is used again with a different parameter. Confirm that the method is correctly implemented and that it handles the different parameter correctly.317-323: The
GetEligibleLiquidationTargetsmethod is now called directly on theQuerierstruct. Verify that this method is correctly implemented in theQuerierstruct and that it correctly identifies eligible liquidation targets.375-375: The
GetAllRegisteredTokensmethod is used to fetch all registered tokens. Confirm that this method is correctly implemented in theQuerierstruct and that it handles the blacklist logic as expected.388-390: The
userMaxWithdrawandToTokenmethods are now called directly on theQuerierstruct. Verify that these methods are correctly implemented in theQuerierstruct and that they correctly calculate the maximum withdrawable amount.435-435: The
GetAllRegisteredTokensmethod is used again to fetch all registered tokens. Confirm that this method is correctly implemented in theQuerierstruct and that it handles the blacklist logic as expected.
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #2374 +/- ##
==========================================
- Coverage 75.38% 70.00% -5.39%
==========================================
Files 100 174 +74
Lines 8025 13006 +4981
==========================================
+ Hits 6050 9105 +3055
- Misses 1589 3294 +1705
- Partials 386 607 +221
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to filter (2)
- swagger/swagger.yaml
- x/uibc/uibc.pb.go
Files selected for processing (7)
- CHANGELOG.md (1 hunks)
- proto/umee/uibc/v1/query.proto (2 hunks)
- proto/umee/uibc/v1/uibc.proto (2 hunks)
- util/genmap/genmap.go (1 hunks)
- util/genmap/genmap_test.go (2 hunks)
- x/leverage/keeper/grpc_query.go (16 hunks)
- x/uibc/quota/mocks_test.go (2 hunks)
Files skipped from review as they are similar to previous changes (6)
- proto/umee/uibc/v1/query.proto
- proto/umee/uibc/v1/uibc.proto
- util/genmap/genmap.go
- util/genmap/genmap_test.go
- x/leverage/keeper/grpc_query.go
- x/uibc/quota/mocks_test.go
Additional comments: 1
CHANGELOG.md (1)
- 49-63: The entries added to the Unreleased section of the CHANGELOG.md are well-formatted and provide the necessary information, including PR links and descriptions for the changes. This follows the standard changelog conventions.
Description
We extend the
QueryAllOutflowsResponseby addingsymbolto each outflow entry.Summary by CodeRabbit
New Features
Improvements
Documentation
Tests
Refactor