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.

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

BugFix: Adding Client side check For Requiring threshold < Noof public keys in update-account #2677

Closed

Conversation

spidey-169
Copy link

@spidey-169 spidey-169 commented Feb 21, 2024

Adding Client side check For Requiring threshold < Noof public keys in update-account cli call.

Describe your changes

Throwing a panic error if condition isn't met so the user accidentally doesn't end up changing threshold to be above the number of public keys. This addresses the existing issue #2624 where the user was able to change threshold to be greater than number of public keys and that caused the wallet to become unusable.

Currently only added the check in update-account, as that is mainly used to update the threshold value.

Also minor grammatical error corrections to correctly use "than" instead of "then"

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

Based on the Bug pointed out in #2624

Checklist before merging to draft

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

…ring updateAccount

Throwing a panic error if condition isn't met so the user accidently doesn't end up changing threshold to be above the number of public keys.

Also minor grammatical error corrections
@spidey-169 spidey-169 changed the title Adding Client side check For Requiring threshold < Noof public keys BugFix: Adding Client side check For Requiring threshold < Noof public keys Feb 21, 2024
@spidey-169 spidey-169 changed the title BugFix: Adding Client side check For Requiring threshold < Noof public keys BugFix: Adding Client side check For Requiring threshold < Noof public keys in update-account Feb 21, 2024
@phy-chain
Copy link

@spidey-169 I think you are missing a crate import and is not correct I think, unless you are implementing some generic.
Try running make build on the project, it should give you pointer on how to fix

@spidey-169
Copy link
Author

spidey-169 commented Feb 27, 2024

@spidey-169 I think you are missing a crate import and is not correct I think, unless you are implementing some generic.

Try running make build on the project, it should give you pointer on how to fix

Thanks, I think the clippy build tools gave suggestions which I was trying to incorporate but yeah might not be needed here. Let me try using make build and see if I can identify the issues. Have gone through a few iterations to fix this already, let me see if I can finish it.

@sug0
Copy link
Collaborator

sug0 commented Feb 28, 2024

favoring #2747 over this pr, therefore I'm closing it

@sug0 sug0 closed this Feb 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants