refactor: Fix confusing iavl command message

[williamchong]

Description

In #13540 iavl-disable-fastnode is added to cmd, however the description is confusing, since setting iavl-disable-fastnode=true disable fast node, instead of enabling it.

Closes: #XXXX

---

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...

• included the correct type prefix in the PR title
• added ! to the type prefix if API or client breaking change
• targeted the correct branch (see PR Targeting)
• provided a link to the relevant issue or specification
• followed the guidelines for building modules
• included the necessary unit and integration tests
• added a changelog entry to CHANGELOG.md
• included comments for documenting Go code
• updated the relevant documentation or specification
• reviewed "Files changed" and left comments if necessary
• confirmed all CI checks have passed

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)

[yihuang]

Suggested change

	cmd.Flags().Bool(FlagIAVLFastNode, true, "Disable fast node for IAVL tree, set to 'false' to enable")
	cmd.Flags().Bool(FlagIAVLDisableFastNode, false, "Disable fast node for IAVL tree, it's enabled by default")

[yihuang]

Should probably change the default value of IAVLDisableFastNode in DefaultConfig as well.

[williamchong]

IAVL fast node is disabled by default, so renaming to FlagIAVLDisableFastNode would do?

[yihuang]

will need core team members to chime in, but I think it's supposed to be enabled by default, which is the case with main branch and 0.46.

[nnkken]

The default value should remains true since current (v0.45.10) default behavior is disable Fast Node.

https://github.com/cosmos/cosmos-sdk/blob/v0.45.10/simapp/simd/cmd/root.go#L279

baseapp.SetIAVLDisableFastNode(cast.ToBool(appOpts.Get(server.FlagIAVLFastNode)))

https://github.com/cosmos/cosmos-sdk/blob/v0.45.10/store/rootmulti/store.go#L41

const iavlDisablefastNodeDefault = true

[nnkken]

BTW this CLI flag seems missing in 0.46, but the config is still there

[julienrbrt]

It is actually missing on main too. Could you set main as base branch, so then we can backport.

[nnkken]

But the default behavior is different for >= 0.46
0.46 enables Fast Node by default (i.e. disableFastNode == false)

[julienrbrt]

But the default behavior is different for >= 0.46
0.46 enables Fast Node by default (i.e. disableFastNode == false)

I don't see the issue:

0.45

cmd.Flags().Bool(FlagDisableIAVLFastNode, true, "Disable fast node for IAVL tree")

0.46+

cmd.Flags().Bool(FlagDisableIAVLFastNode, false, "Disable fast node for IAVL tree")

Not having the flag in version after 0.45 is a CLI breaking change, so I think it is better to still keep the flag (true and false keep the same meaning anyway). We don't need to precise the default value in the message, as cobra does that already.

In the PR backported to 0.45 we can just swap the false to true. This way we keep consistency.

[williamchong]

@julienrbrt #13656 Please check if I understand correctly, then we can close this PR and follow up in the new one

[williamchong]

Follow up in #13656