fix(autocli): fix simapp enhancing #15906

julienrbrt · 2023-04-21T08:10:08Z

Description

Discovered a bug in AutoCLI that it does not enrich the command when the CLI isn't defined, this PR will fix this as well.

Fix AutoCLI enriching
Fixes typos and duplicated logic
Migrated as well NFT queries as a spike and for testing the fix. (Originally the PR was supposed to do only that 😅 ) - ref Add Query Module Configurations for AutoCLI #15901
~~Add client breaking changelog~~ Actually, we can make it non-breaking by tweaking autocli.go
Fix documentation (updates where the autocli.go file should be, add boilerplate for changelog and readme)
Fix bech32 address prefix (fixed by refactor: allow getting address prefix from address codec #15913 (comment))

I've tested the changes with simapp and hubl and it works.

Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.

I have...

confirmed the correct type prefix in the PR title
confirmed ! in the type prefix if API or client breaking change
confirmed all author checklist items have been addressed
reviewed state machine logic
reviewed API design and naming
reviewed documentation is accurate
reviewed tests and test coverage
manually tested (if applicable)

julienrbrt · 2023-04-21T10:44:00Z

simapp/simd/cmd/root.go

@@ -245,8 +245,6 @@ func queryCommand() *cobra.Command {
 		authcmd.QueryTxCmd(),
 	)

-	simapp.ModuleBasics.AddQueryCommands(cmd)


This is not necessary as autocli will enrich the commands that are not present

julienrbrt · 2023-04-21T10:44:46Z

client/v2/autocli/common.go

 		// if we have an existing command skip adding one here
-		if cmd.HasSubCommands() {
-			if _, _, err := cmd.Find([]string{moduleName}); err == nil {


This was always returning true for simapp. We already had a helper that was working.

julienrbrt · 2023-04-21T10:45:20Z

client/v2/autocli/app.go

 	if msgCmd := findSubCommand(rootCmd, "tx"); msgCmd != nil {
-		if err := builder.enhanceCommandCommon(msgCmd, moduleOptions, customQueryCmds, enhanceMsg); err != nil {


So here it was passing the custom query commands, which is incorrect.

julienrbrt · 2023-04-21T16:48:14Z

x/nft/module/autocli.go

Without this file, the command is not generated.
Still getting failure when running app err="client context not set" for all generated autocli apps.
Investigating the issue first. If this file is always needed then we need to update the docs as well.

This is done here: 1f01c04
I still have to dig the reason it was returning an error before, as the behavior of the commit above makes sense to me.

julienrbrt · 2023-04-21T16:52:01Z

client/v2/autocli/msg.go

 	msgCmd := topLevelCmd("tx", "Transaction subcommands")
-	enhanceMsg := func(cmd *cobra.Command, modOpts *autocliv1.ModuleOptions, moduleName string) error {
-		txCmdDesc := modOpts.Tx


this was duplicated.

julienrbrt · 2023-04-21T16:52:06Z

client/v2/autocli/query.go

 	queryCmd := topLevelCmd("query", "Querying subcommands")
 	queryCmd.Aliases = []string{"q"}

-	enhanceMsg := func(cmd *cobra.Command, modOpts *autocliv1.ModuleOptions, moduleName string) error {
-		txQueryDesc := modOpts.Query


this was duplicated.

client/v2/autocli/flag/address.go

aaronc · 2023-04-25T12:59:43Z

client/v2/autocli/flag/address.go

@@ -24,18 +27,20 @@ func (a addressStringType) NewValue(ctx context.Context, b *Builder) Value {
 		if resp == nil || resp.Config == nil {
 			panic("bech32 account address prefix is not set")
 		}
-		b.AddressPrefix = resp.Config.Bech32AccountAddressPrefix
+
+		b.AddressCodec = addresscodec.NewBech32Codec(resp.Config.Bech32AccountAddressPrefix)


We shouldn't call grpc every time to get the bech32 prefix. This should be done by hubl during unit and update and then cached

I agree, do you want me to do that here or in a follow-up? This PR does not touch hubl's code.

In a follow up

* solved lint issues * feat(x/tx): API improvements (cosmos#15871) * refactor(baseapp): set vote info to latest votes (cosmos#15930) * refactor(staking, slashing, signing, misc): remove aliases from types/math.go (cosmos#15902) * fix(autocli): fix simapp enhancing (cosmos#15906) * fix lint on armor.go * ran make lint-fix --------- Co-authored-by: Matt Kocubinski <[email protected]> Co-authored-by: Marko <[email protected]> Co-authored-by: samricotta <[email protected]> Co-authored-by: Julien Robert <[email protected]>

github-actions bot added C:CLI C:x/nft C:Confix Issues and PR related to Confix C:Cosmovisor Issues and PR related to Cosmovisor C:Hubl Tool: Hubl C:Rosetta Issues and PR related to Rosetta C:x/circuit labels Apr 21, 2023

julienrbrt commented Apr 21, 2023

View reviewed changes

julienrbrt changed the title ~~feat(nft)!: migrate command queries to autocli~~ feat(nft)!: migrate command queries to autocli and fix autocli enriching Apr 21, 2023

julienrbrt commented Apr 21, 2023

View reviewed changes

github-actions bot added the C:x/auth label Apr 21, 2023

julienrbrt commented Apr 21, 2023

View reviewed changes

client/v2/autocli/flag/address.go Outdated Show resolved Hide resolved

julienrbrt changed the title ~~feat(nft)!: migrate command queries to autocli and fix autocli enriching~~ fix(autocli): fix simapp enriching Apr 21, 2023

julienrbrt marked this pull request as ready for review April 21, 2023 21:57

julienrbrt requested a review from a team as a code owner April 21, 2023 21:57

julienrbrt assigned aaronc and JeancarloBarrios Apr 21, 2023

julienrbrt requested review from aaronc and JeancarloBarrios and removed request for mark-rushakoff and atheeshp April 21, 2023 22:08

This comment has been minimized.

Sign in to view

julienrbrt mentioned this pull request Apr 23, 2023

refactor: allow getting address prefix from address codec #15913

Closed

19 tasks

julienrbrt changed the base branch from main to julien/codec-add-get-prefix April 23, 2023 15:19

julienrbrt changed the base branch from julien/codec-add-get-prefix to main April 24, 2023 14:05

fix(autocli): fix simapp enriching and migrate nft queries to autocli

03a1c95

julienrbrt force-pushed the julien/nft-autocli branch from 50abaac to 03a1c95 Compare April 24, 2023 14:09

Merge branch 'main' into julien/nft-autocli

2d680de

Merge branch 'main' into julien/nft-autocli

c02a8bd

julienrbrt changed the title ~~fix(autocli): fix simapp enriching~~ fix(autocli): fix simapp enhancing Apr 24, 2023

julienrbrt added 2 commits April 25, 2023 01:03

Merge branch 'main' into julien/nft-autocli

39eccbb

Merge branch 'main' into julien/nft-autocli

b336aa8

aaronc approved these changes Apr 25, 2023

View reviewed changes

typo

0e377d8

julienrbrt added this pull request to the merge queue Apr 25, 2023

Merged via the queue into main with commit ebcad58 Apr 25, 2023

julienrbrt deleted the julien/nft-autocli branch April 25, 2023 14:10

julienrbrt mentioned this pull request Apr 26, 2023

feat(hubl): cache bech32 prefix #15954

Merged

19 tasks

tac0turtle mentioned this pull request May 3, 2023

Reduce CLI query boilerplate #10567

Closed

4 tasks

larry0x pushed a commit to larry0x/cosmos-sdk that referenced this pull request May 22, 2023

fix(autocli): fix simapp enhancing (cosmos#15906)

32e6056

julienrbrt mentioned this pull request Jun 21, 2023

Add Query Module Configurations for AutoCLI #15901

Closed

17 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(autocli): fix simapp enhancing #15906

fix(autocli): fix simapp enhancing #15906

julienrbrt commented Apr 21, 2023 •

edited

Loading

julienrbrt Apr 21, 2023

julienrbrt Apr 21, 2023

julienrbrt Apr 21, 2023 •

edited

Loading

julienrbrt Apr 21, 2023 •

edited

Loading

julienrbrt Apr 21, 2023

julienrbrt Apr 21, 2023

julienrbrt Apr 21, 2023

This comment has been minimized.

aaronc Apr 25, 2023

julienrbrt Apr 25, 2023

aaronc Apr 25, 2023

		if msgCmd := findSubCommand(rootCmd, "tx"); msgCmd != nil {
		if err := builder.enhanceCommandCommon(msgCmd, moduleOptions, customQueryCmds, enhanceMsg); err != nil {

fix(autocli): fix simapp enhancing #15906

fix(autocli): fix simapp enhancing #15906

Conversation

julienrbrt commented Apr 21, 2023 • edited Loading

Description

Author Checklist

Reviewers Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

julienrbrt Apr 21, 2023 • edited Loading

Choose a reason for hiding this comment

julienrbrt Apr 21, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment has been minimized.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

julienrbrt commented Apr 21, 2023 •

edited

Loading

julienrbrt Apr 21, 2023 •

edited

Loading

julienrbrt Apr 21, 2023 •

edited

Loading