Unit of min-self-delegation

[youngjoon-lee]

Summary of Bug

From the create-validator guide, it seems that the unit of min-self-delegation is atom (not uatom).

But, the ValidatorBasic() was implemented as if its unit is uatom.

cosmos-sdk/x/staking/types/msg.go

Line 139 in 45265b1

if msg.Value.Amount.LT(msg.MinSelfDelegation) {

If the unit should be uatom, I would suggest to force users to put a denominator to the --min-self-delegation option as discussed in #3594.

gaiad tx staking create-validator --min-self-delegation="1uatom"

If not, the ValidatorBasic() should be fixed as below, I guess.

if msg.Value.Amount.LT(sdk.TokensFromConsensusPower(msg.MinSelfDelegation.Int64())) {
    return ErrSelfDelegationBelowMinimum
}

If you guys confirm which way is correct, I can open a PR (if it's ok).

Version

Cosmos SDK v0.42.4

Steps to Reproduce

No step to reproduce. I just read source codes and found this issue.

---

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate contributors tagged
• Contributor assigned/self-assigned

[alexanderbez]

The min self delegation passed to the CLI and thus the message should be in the base unit. e.g. for the hub, it's uatom, i.e. 10^6. I think we just need to update the docs.

[youngjoon-lee]

Cool. Thanks for your quick reply.
Yeah. The doc needs to be changed. Plus, what do you think to update the CLI as well? to get a denom as well as a number, as discussed in #3594.

--min-self-delegation=1uatom

[ryanchristo]

Thanks @youngjoon-lee for reporting the issue!

The document that you are referring to lives in the Gaia repository if you would like to submit a fix there. I think adding denom requirements at the client level is something we would still like to do so a pull request here would be welcome as well.

[aaronc]

@shahankhatch can you take a look at this from the gaia side?

[ryanchristo]

@youngjoon-lee Are you still interested in submitting a pull request to make the denom required?

[youngjoon-lee]

@youngjoon-lee Are you still interested in submitting a pull request to make the denom required?

Thank you for your reply. Sure! I will submit a PR this week regards to the CLI option, and another PR for Gaia documents to the Gaia repo.

[ryanchristo]

Awesome! I think a fix for the documentation has already been submitted but updating the CLI is all yours.

[ryanchristo]

Hey @youngjoon-lee! I just wanted to check in and see if you are still planning on tackling the CLI change.

[youngjoon-lee]

@ryanchristo Yes. Sorry for late. I will open a PR this week.

[ryanchristo]

No worries! I was just checking. Thanks for letting us know.