Update the MsgMarketCommitmentSettle cli stuff use the keyring if a from is provided. #2001
Conversation
WalkthroughThe recent updates primarily focus on enhancing 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.
Actionable comments posted: 1
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (6)
- CHANGELOG.md (1 hunks)
- x/exchange/client/cli/cli_test.go (3 hunks)
- x/exchange/client/cli/flags_test.go (6 hunks)
- x/exchange/client/cli/query_setup.go (1 hunks)
- x/exchange/client/cli/query_setup_test.go (2 hunks)
- x/exchange/client/cli/tx_setup_test.go (2 hunks)
Files skipped from review due to trivial changes (1)
- x/exchange/client/cli/flags_test.go
Additional comments not posted (27)
x/exchange/client/cli/cli_test.go (3)
1315-1319: RefactorednewClientContextto include keyring setup aligns with PR objectives.
1337-1349: Properly sets up keyring in client context with error handling.
1331-1335: Ensure sensitive keyring constants are securely managed.#!/bin/bash # Description: Verify that keyring constants are not exposed inappropriately. # Test: Search for the keyring constants usage. Expect: No exposure in logs or other insecure places. rg --type go $'keyringName|keyringMnemonic|keyringAddr'x/exchange/client/cli/query_setup_test.go (1)
71-71: Change fromnewClientContextWithCodectonewClientContextaligns with the PR's objective to simplify client context setup.x/exchange/client/cli/tx_setup_test.go (22)
Line range hint
73-91: The test functionTestSetupCmdTxCreateAskcorrectly sets up and validates the command flags for creating an ask in the market exchange context.
Line range hint
93-126: The test functionTestMakeMsgCreateAskthoroughly tests the message creation logic for an "ask" order under various scenarios, including error handling.
Line range hint
128-146: The test functionTestSetupCmdTxCreateBidcorrectly sets up and validates the command flags for creating a bid in the market exchange context.
Line range hint
148-181: The test functionTestMakeMsgCreateBidthoroughly tests the message creation logic for a "bid" order under various scenarios, including error handling.
Line range hint
183-201: The test functionTestSetupCmdTxCommitFundscorrectly sets up and validates the command flags for committing funds in the market exchange context.
Line range hint
203-236: The test functionTestMakeMsgCommitFundsthoroughly tests the message creation logic for committing funds under various scenarios, including error handling.
Line range hint
238-256: The test functionTestSetupCmdTxCancelOrdercorrectly sets up and validates the command flags for canceling an order in the market exchange context.
Line range hint
258-291: The test functionTestMakeMsgCancelOrderthoroughly tests the message creation logic for canceling an order under various scenarios, including error handling.
Line range hint
293-311: The test functionTestSetupCmdTxFillBidscorrectly sets up and validates the command flags for filling bids in the market exchange context.
Line range hint
313-346: The test functionTestMakeMsgFillBidsthoroughly tests the message creation logic for filling bids under various scenarios, including error handling.
Line range hint
348-366: The test functionTestSetupCmdTxFillAskscorrectly sets up and validates the command flags for filling asks in the market exchange context.
Line range hint
368-401: The test functionTestMakeMsgFillAsksthoroughly tests the message creation logic for filling asks under various scenarios, including error handling.
Line range hint
403-421: The test functionTestSetupCmdTxMarketSettlecorrectly sets up and validates the command flags for market settlement in the market exchange context.
Line range hint
423-456: The test functionTestMakeMsgMarketSettlethoroughly tests the message creation logic for market settlement under various scenarios, including error handling.
Line range hint
458-476: The test functionTestSetupCmdTxMarketCommitmentSettlecorrectly sets up and validates the command flags for market commitment settlement in the market exchange context.
Line range hint
478-735: The test functionTestMakeMsgMarketCommitmentSettlethoroughly tests the message creation logic for market commitment settlement under various scenarios, including error handling.
Line range hint
737-755: The test functionTestSetupCmdTxMarketReleaseCommitmentscorrectly sets up and validates the command flags for releasing market commitments in the market exchange context.
Line range hint
757-794: The test functionTestMakeMsgMarketReleaseCommitmentsthoroughly tests the message creation logic for releasing market commitments under various scenarios, including error handling.
Line range hint
796-814: The test functionTestSetupCmdTxMarketSetOrderExternalIDcorrectly sets up and validates the command flags for setting an external ID on a market order in the market exchange context.
Line range hint
816-849: The test functionTestMakeMsgMarketSetOrderExternalIDthoroughly tests the message creation logic for setting an external ID on a market order under various scenarios, including error handling.
Line range hint
851-869: The test functionTestSetupCmdTxMarketWithdrawcorrectly sets up and validates the command flags for withdrawing from a market in the market exchange context.
Line range hint
871-904: The test functionTestMakeMsgMarketWithdrawthoroughly tests the message creation logic for withdrawing from a market under various scenarios, including error handling.CHANGELOG.md (1)
95-95: The update to theCHANGELOG.mdcorrectly reflects the integration of keyring functionality into thecommitment-settlement-fee-calcCLI query as described in the PR summary and AI-generated summary.
| if addr, err := sdk.AccAddressFromBech32(clientCtx.From); err == nil { | ||
| clientCtx.FromAddress = addr | ||
| } else { | ||
| clientCtx.FromAddress, clientCtx.From, _, errs[1] = client.GetFromFields(clientCtx, clientCtx.Keyring, clientCtx.From) | ||
| } |
There was a problem hiding this comment.
Enhance error handling in address conversion logic.
The current implementation attempts to convert the from address from Bech32 format and falls back to the keyring if it fails. Consider adding more detailed error handling or logging to provide clearer feedback on what went wrong in case of an error. This could improve debugging and user experience.
- clientCtx.FromAddress, clientCtx.From, _, errs[1] = client.GetFromFields(clientCtx, clientCtx.Keyring, clientCtx.From)
+ clientCtx.FromAddress, clientCtx.From, _, errs[1] = client.GetFromFields(clientCtx, clientCtx.Keyring, clientCtx.From)
+ if errs[1] != nil {
+ // Log or handle the error appropriately
+ }Committable suggestion was skipped due low confidence.
…rom is provided. (#2001) * Update the MsgMarketCommitmentSettle cli stuff use the keyring if a from is provided. * Add changelog entry. * Add test case for unknown keyring name.
Description
This PR makes the
provenanced query exchange commitment-settlement-fee-calccommand use the keyring to get the from address (just like the related tx command would do).Prior to this PR, you could provide the admin address using the '--from' flag, but it didn't use the keyring, so it had to be an address. That means that, if you wanted to use a named keyring entry when doing the tx, you couldn't provide the same flags to the query command (to check it before you try it). After this PR, you can provide the same
--fromvalue to both commands.Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.
docs/) or specification (x/<module>/spec/)godoccomments.Unreleasedsection inCHANGELOG.mdFiles changedin the Github PR explorerCodecov Reportin the comment section below once CI passesSummary by CodeRabbit
New Features
Bug Fixes
Refactor
Tests