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

fix(wallet, cmd): adding ed25519_account in help and set as default #1485

Merged
merged 5 commits into from
Sep 14, 2024

Conversation

kehiy
Copy link
Contributor

@kehiy kehiy commented Aug 24, 2024

Description

The new address type was missed in the wallet cmd help command. Also ed25519 is faster and widely supported in different wallet's, so we can set it as default and new users will have ed25519 address.

cmd/wallet/address.go Outdated Show resolved Hide resolved
@kehiy
Copy link
Contributor Author

kehiy commented Aug 25, 2024

@b00f @akbariandev I reverted the default change. I tried to test the CLI and when I give an input as password, If it is not same as my wallet password it returns invalid password error.

I think it consider it as wallet password not a password provided for this address. or maybe I'm using this wrong?

@b00f
Copy link
Collaborator

b00f commented Aug 28, 2024

What is default now?

@akbariandev
Copy link
Contributor

@b00f @akbariandev I reverted the default change. I tried to test the CLI and when I give an input as password, If it is not same as my wallet password it returns invalid password error.

I think it consider it as wallet password not a password provided for this address. or maybe I'm using this wrong?

@kehiy It must compare provided password with wallet password. Actually this feature can create new Ed25519 address by wallet password.

@kehiy
Copy link
Contributor Author

kehiy commented Sep 4, 2024

@akbariandev This only affects the default and description of command, not the way that it inputs the password from user. maybe I understand your comment wrong?

@Ja7ad Ja7ad added this to the v.1.5.0 milestone Sep 8, 2024
@kehiy
Copy link
Contributor Author

kehiy commented Sep 10, 2024

@b00f @akbariandev @akbariandev Any updates? should we merge or close this PR?

@b00f b00f changed the title fix(wallet-cmd): adding ed25519_account in help and set as default fix(wallet, cmd): adding ed25519_account in help and set as default Sep 12, 2024
@b00f
Copy link
Collaborator

b00f commented Sep 12, 2024

@kehiy please add it but not make it as default now. Let's do it in the next release.

@kehiy
Copy link
Contributor Author

kehiy commented Sep 14, 2024

@b00f done.

@b00f b00f merged commit a1fd335 into pactus-project:main Sep 14, 2024
10 checks passed
@kehiy kehiy deleted the hotfix/ed25519_account branch September 15, 2024 09:53
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.

4 participants