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

chore: validate wallet name #684

Merged
merged 2 commits into from
Nov 2, 2023
Merged

Conversation

theborakompanioni
Copy link
Collaborator

@theborakompanioni theborakompanioni commented Oct 20, 2023

Resolves #683.

Before this commit, the wallet name has been validated only on the backend.
After this commit is applied, it will also be validated by the frontend.

The validation is a bit more strict than "necessary", only numbers, letters and _ (underscore) will be allowed from now on.

@theborakompanioni theborakompanioni added concept Wild idea, or too many details unknown yet UI/UX Issue related to cosmetics, design, or user experience labels Oct 20, 2023
@theborakompanioni theborakompanioni self-assigned this Oct 20, 2023
@theborakompanioni theborakompanioni marked this pull request as ready for review October 20, 2023 09:47
@kristapsk
Copy link
Contributor

Why not also allow hyphen-minus (-)? I personally use that for JM wallet names instead of underscore.

@theborakompanioni
Copy link
Collaborator Author

Why not also allow hyphen-minus (-)? I personally use that for JM wallet names instead of underscore.

Now also allows hyphens. Error feedback message has been changed to

Please choose a valid wallet name: Use only letters, numbers, underscores or hyphens.

Additionally, the length of the wallet name has been limited to 234 chars.
(Notice that, while JM allows longer names, it will create a wallet but later fail loading it.)

Thanks for the feedback @kristapsk 🙏

@kristapsk
Copy link
Contributor

(Notice that, while JM allows longer names, it will create a wallet but later fail loading it.)

Interesting, something to test and likely fix in JM.

@kristapsk
Copy link
Contributor

Additionally, the length of the wallet name has been limited to 234 chars.
(Notice that, while JM allows longer names, it will create a wallet but later fail loading it.)

With which OS, filesystem and Python version do you experienced this? I cannot reproduce under Linux, ext4 and Python 3.11.4, at least not with cli tools. You had this both with scripts and RPC API or only RPC API?

@theborakompanioni
Copy link
Collaborator Author

Additionally, the length of the wallet name has been limited to 234 chars.
(Notice that, while JM allows longer names, it will create a wallet but later fail loading it.)

With which OS, filesystem and Python version do you experienced this? I cannot reproduce under Linux, ext4 and Python 3.11.4, at least not with cli tools. You had this both with scripts and RPC API or only RPC API?

Hey @kristapsk! (Mostly) everything I do and test is with the RPC API!
I am using the regtest docker image based on Debian GNU/Linux 12 (bookworm), fs ext4, python v3.11.6.

@theborakompanioni theborakompanioni merged commit 7375116 into master Nov 2, 2023
3 checks passed
@theborakompanioni theborakompanioni deleted the chore/validate-walletname branch November 2, 2023 11:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
concept Wild idea, or too many details unknown yet UI/UX Issue related to cosmetics, design, or user experience
Projects
None yet
Development

Successfully merging this pull request may close these issues.

/ char should not be allowed in wallet name
3 participants