Skip to content

chore: validate wallet name#684

Merged
theborakompanioni merged 2 commits intomasterfrom
chore/validate-walletname
Nov 2, 2023
Merged

chore: validate wallet name#684
theborakompanioni merged 2 commits intomasterfrom
chore/validate-walletname

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