fix: client side check to prevent threshold=0 on init/update

[phy-chain]

Adding Client side check to ensure threshold > 0 in init-account and update-account (also init-validator)

Describe your changes

Accounts (and validator accounts) should not be created/updated with less than 1 as threshold, otherwise this would allow transaction requiring to signature.

Indicate on which release or other PRs this topic is based on

I initially first reported this in #2671.

Checklist before merging to draft

• I have added a changelog
• Git history is in acceptable state

[Fraccaman]

Thanks for the PR! I think the logic is almost correct, but I would say it better to check if the threshold is <= than the number of public-keys associated with the new account. Would you be able to make the change?

[phy-chain]

@Fraccaman I think it's 2 differents issues. Checking if the threshold <= public-keys would not prevent it from begin 0, right ?
But I can add that logic too in the same PR if you want

[phy-chain]

Also, I can see that some of the CI check are not passing. Is it expected, or should I fix them ? (I've fixed the namada-release already, Im not used to Rust - make build works fine locally though)

[Fraccaman]

to get CI to work, just rebase this branch on base. For the logic I think we can do something like:

• at least 1 public key is definied
• threshold is <= number of public keys

This should solve both the issues, wdyt?

[Fraccaman]

for update-account the logic is more complex because you could modify only the threshold but we should check on the client that the new threshold won't "burn" the account. As an example, lets say that the current threshold for account X is 1, and the number of associated public keys is 1. If a user tries to modify the threshold of account X to 2, it should fail (without --force).

[spidey-169]

for update-account the logic is more complex because you could modify only the threshold but we should check on the client that the new threshold won't "burn" the account. As an example, lets say that the current threshold for account X is 1, and the number of associated public keys is 1. If a user tries to modify the threshold of account X to 2, it should fail (without --force).

Hi, looks like this is actually what I am trying to implement in my PR #2677. Sorry, am new to rust and hence taking a little longer. Will try to see I can get this implemented for update-account and currently planning to throw and exception and exit if threshold > num-public-keys is ever detected

[sug0]

also, please rebase on the latest base branch from upstream

$ git remote add upstream https://github.com/anoma/namada
$ git fetch -p upstream
$ git rebase upstream/base

[phy-chain]

git remote add upstream https://github.com/anoma/namada

Am i not supposed to continue working on my fork ? Im not used to open source contributions, so I'd rather not mess up my branche :D

[codecov]

Codecov Report

Attention: Patch coverage is 0% with 30 lines in your changes are missing coverage. Please review.

Project coverage is 53.94%. Comparing base (2535c9c) to head (35d90f5).

Files	Patch %	Lines
crates/apps/src/lib/cli.rs	0.00%	30 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2747      +/-   ##
==========================================
- Coverage   53.95%   53.94%   -0.02%     
==========================================
  Files         308      308              
  Lines      100018   100042      +24     
==========================================
- Hits        53967    53966       -1     
- Misses      46051    46076      +25     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[sug0]

git remote add upstream https://github.com/anoma/namada

Am i not supposed to continue working on my fork ? Im not used to open source contributions, so I'd rather not mess up my branche :D

you won't mess it up xd

your changes are pretty trivial, you shouldn't run into any conflicts by rebasing. the git remote-add command adds a new location to fetch git commits/references from, since your forked repository won't have our latest releases.

[phy-chain]

git remote add upstream https://github.com/anoma/namada

Am i not supposed to continue working on my fork ? Im not used to open source contributions, so I'd rather not mess up my branche :D

you won't mess it up xd

your changes are pretty trivial, you shouldn't run into any conflicts by rebasing. the git remote-add command adds a new location to fetch git commits/references from, since your forked repository won't have our latest releases.

Yeah rebase is already done. Im trying to figure out the NoneZeroU8 change now. But Im not so much of a Rust coder. More into typescript/frontend/react native :D But I'll keep hacking into it (unless you want to handle that your own way 😅 )

[brentstone]

Closing this in favor of #3154.