Skip to content
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.

Sign up for GitHub

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

Update the MsgMarketCommitmentSettle cli stuff use the keyring if a from is provided. #2001

Merged
merged 3 commits into from
May 23, 2024
Merged

Update the MsgMarketCommitmentSettle cli stuff use the keyring if a from is provided. #2001

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
724343a
Update the MsgMarketCommitmentSettle cli stuff use the keyring if a f…
SpicyLemon May 23, 2024
3963b80
Add changelog entry.
SpicyLemon May 23, 2024
89eb7d1
Add test case for unknown keyring name.
SpicyLemon May 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
* Add `UpdateParams` and `Params` query rpc endpoints to modules.
* `ibcratelimit` add `UpdateParams` endpoint and deprecate `GovUpdateParams` [#1984](https://github.com/provenance-io/provenance/pull/1984).
* `attribute` add `UpdateParams` endpoint and cli [#1987](https://github.com/provenance-io/provenance/pull/1987).
* Update the exchange `commitment-settlement-fee-calc` cli query to utilize the keyring [#2001](https://github.com/provenance-io/provenance/pull/2001).

### Client Breaking

Expand Down
30 changes: 26 additions & 4 deletions x/exchange/client/cli/cli_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1312,9 +1312,11 @@ func getEncodingConfig(t *testing.T) params.EncodingConfig {
return *encodingConfig
}

// newClientContextWithCodec returns a new client.Context that has a useful Codec.
func newClientContextWithCodec(t *testing.T) client.Context {
return clientContextWithCodec(t, client.Context{})
// newClientContext returns a new client.Context that has a codec and keyring.
func newClientContext(t *testing.T) client.Context {
ctx := client.Context{}
ctx = clientContextWithCodec(t, ctx)
return clientContextWithKeyring(t, ctx)
}

// clientContextWithCodec adds a useful Codec to the provided client context.
Expand All @@ -1326,6 +1328,26 @@ func clientContextWithCodec(t *testing.T, clientCtx client.Context) client.Conte
WithTxConfig(encCfg.TxConfig)
}

const (
keyringName = "keyringaddr"
keyringMnemonic = "pave kit dust loop forest symptom lobster tape note attitude aim cloth cat welcome basic head wet pistol hair funny library dove oak drift"
keyringAddr = "cosmos177zjzs7mh79j0cx6wcxq8dayetkftkx2crt4u6"
)

// clientContextWithKeyring returns a client context that has a keyring and an entry for keyringName=>keyringAddr.
func clientContextWithKeyring(t *testing.T, clientCtx client.Context) client.Context {
kr, err := client.NewKeyringFromBackend(clientCtx, keyring.BackendMemory)
require.NoError(t, err, "NewKeyringFromBackend")
clientCtx = clientCtx.WithKeyring(kr)

// This mnemonic was generated by using Keyring.NewAccount().
// I then logged it (and the address) with t.Logf(...) and copy/pasted it into the constants above.
_, err = clientCtx.Keyring.NewAccount(keyringName, keyringMnemonic, keyring.DefaultBIP39Passphrase, hd.CreateHDPath(118, 0, 0).String(), hd.Secp256k1)
require.NoError(t, err, "adding %s to the keyring using NewAccount(...)", keyringName)

return clientCtx
}

// newGovProp creates a new MsgSubmitProposal containing the provided messages, requiring it to not error.
func newGovProp(t *testing.T, msgs ...sdk.Msg) *govv1.MsgSubmitProposal {
rv := &govv1.MsgSubmitProposal{}
Expand Down Expand Up @@ -1354,7 +1376,7 @@ func newTx(t *testing.T, msgs ...sdk.Msg) *txtypes.Tx {

// writeFileAsJson writes the provided proto message as a json file, requiring it to not error.
func writeFileAsJson(t *testing.T, filename string, content proto.Message) {
clientCtx := newClientContextWithCodec(t)
clientCtx := newClientContext(t)
bz, err := clientCtx.Codec.MarshalJSON(content)
require.NoError(t, err, "MarshalJSON(%T)", content)
writeFile(t, filename, bz)
Expand Down
12 changes: 6 additions & 6 deletions x/exchange/client/cli/flags_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2541,7 +2541,7 @@ func TestReadTxFileFlag(t *testing.T) {
err := tc.flagSet.Parse(args)
require.NoError(t, err, "flagSet.Parse(%q)", args)

clientCtx := newClientContextWithCodec(t)
clientCtx := newClientContext(t)

var actFilename string
var actTx *txtypes.Tx
Expand Down Expand Up @@ -2765,7 +2765,7 @@ func TestReadProposalFlag(t *testing.T) {
err := tc.flagSet.Parse(args)
require.NoError(t, err, "flagSet.Parse(%q)", args)

clientCtx := newClientContextWithCodec(t)
clientCtx := newClientContext(t)

var actPropFN string
var actAnys []*codectypes.Any
Expand Down Expand Up @@ -2893,7 +2893,7 @@ func TestReadMsgGovCreateMarketRequestFromProposalFlag(t *testing.T) {
err := flagSet.Parse(args)
require.NoError(t, err, "flagSet.Parse(%q)", args)

clientCtx := newClientContextWithCodec(t)
clientCtx := newClientContext(t)

var actual *exchange.MsgGovCreateMarketRequest
testFunc := func() {
Expand Down Expand Up @@ -3010,7 +3010,7 @@ func TestReadMsgGovManageFeesRequestFromProposalFlag(t *testing.T) {
err := flagSet.Parse(args)
require.NoError(t, err, "flagSet.Parse(%q)", args)

clientCtx := newClientContextWithCodec(t)
clientCtx := newClientContext(t)

var actual *exchange.MsgGovManageFeesRequest
testFunc := func() {
Expand Down Expand Up @@ -3173,7 +3173,7 @@ func TestReadMsgMarketCommitmentSettleFromFileFlag(t *testing.T) {
err := flagSet.Parse(args)
require.NoError(t, err, "flagSet.Parse(%q)", args)

clientCtx := newClientContextWithCodec(t)
clientCtx := newClientContext(t)

var actual *exchange.MsgMarketCommitmentSettleRequest
testFunc := func() {
Expand Down Expand Up @@ -3376,7 +3376,7 @@ func TestReadPaymentFromFileFlag(t *testing.T) {
err := flagSet.Parse(args)
require.NoError(t, err, "flagSet.Parse(%q)", args)

clientCtx := newClientContextWithCodec(t)
clientCtx := newClientContext(t)

var actual exchange.Payment
testFunc := func() {
Expand Down
6 changes: 5 additions & 1 deletion x/exchange/client/cli/query_setup.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -474,7 +474,11 @@ func MakeQueryCommitmentSettlementFeeCalc(clientCtx client.Context, flagSet *pfl
errs := make([]error, 4)
clientCtx.From, errs[0] = flagSet.GetString(flags.FlagFrom)
if len(clientCtx.From) > 0 {
clientCtx.FromAddress, errs[1] = sdk.AccAddressFromBech32(clientCtx.From)
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)
}
Comment on lines +477 to +481
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

}
rv.Settlement, errs[2] = MakeMsgMarketCommitmentSettle(clientCtx, flagSet, args)
rv.IncludeBreakdownFields, errs[3] = flagSet.GetBool(FlagDetails)
Expand Down
19 changes: 18 additions & 1 deletion x/exchange/client/cli/query_setup_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ func runQueryMakerTest[R any](t *testing.T, td queryMakerTestDef[R], tc queryMak
err := cmd.Flags().Parse(tc.flags)
require.NoError(t, err, "cmd.Flags().Parse(%q)", tc.flags)

clientCtx := newClientContextWithCodec(t)
clientCtx := newClientContext(t)

var req *R
testFunc := func() {
Expand Down Expand Up @@ -1196,6 +1196,23 @@ func TestMakeQueryCommitmentSettlementFeeCalc(t *testing.T) {
},
},
},
{
name: "admin from keyring",
flags: []string{"--from", keyringName},
expReq: &exchange.QueryCommitmentSettlementFeeCalcRequest{
Settlement: &exchange.MsgMarketCommitmentSettleRequest{
Admin: keyringAddr,
},
},
},
{
name: "from flag is unknown name",
flags: []string{"--from", "notknown"},
expReq: &exchange.QueryCommitmentSettlementFeeCalcRequest{
Settlement: &exchange.MsgMarketCommitmentSettleRequest{},
},
expErr: joinErrs("notknown.info: key not found", "no <admin> provided"),
},
{
name: "admin as authority",
flags: []string{"--authority"},
Expand Down
4 changes: 2 additions & 2 deletions x/exchange/client/cli/tx_setup_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -732,7 +732,7 @@ func TestMakeMsgMarketCommitmentSettle(t *testing.T) {
},
{
name: "from file",
clientCtx: newClientContextWithCodec(t),
clientCtx: newClientContext(t),
flags: []string{"--file", filename},
expMsg: fileMsg,
},
Expand All @@ -742,7 +742,7 @@ func TestMakeMsgMarketCommitmentSettle(t *testing.T) {
"--file", filename, "--tag", "new-thang", "--authority",
"--outputs", "monroe:87plum",
},
clientCtx: newClientContextWithCodec(t),
clientCtx: newClientContext(t),
expMsg: &exchange.MsgMarketCommitmentSettleRequest{
Admin: cli.AuthorityAddr.String(),
MarketId: 4,
Expand Down
Loading