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

cmd/geth: standardize the action of importing bls account #2020

Merged
merged 4 commits into from
Dec 5, 2023

Conversation

buddh0
Copy link
Collaborator

@buddh0 buddh0 commented Nov 30, 2023

Description

cmd/geth: standardize the action of importing bls account

Rationale

there are 3 issues when using geth bls account

  1. no trim '\r\n' when read password from file.
    when use vim or echo command to write password into a file, a '\r' or '\n' will be added automaticly
    this PR will do the trim by following geth account

  2. some related flags have not set field Category, so they will not be displayed in help info
    this PR fix it

  3. putting a bls private key into wallet viewed as a new action
    only putting a keystore file into wallet will be viewed as a import action
    but geth account can put a private key into wallet by a import action
    and others, in metamask, the import action both accept a file including a private key or a keystore file

    so in this PR, putting a bls private key into wallet will be viewed as a import action instead of a new action.

  4. when new a bls account, use the bls wallet password to encrypt it into a keystore file
    so no need a blsaccountpassword any more, bring much convenient

  5. when import a bls account with a keystore file, rename blsaccountpassword to importedaccountpassword,
    to reduce ambiguity.

to demostrate the usage of geth bls account, a shell is provided,
and using it to test these above features is easily

Example

run bash bls-account-usage-demo.sh in bsc/cmd/geth/testdata
image

Changes

Notable changes:

  • add each change in a bullet point here
  • ...

cmd/geth/blsaccountcmd.go Outdated Show resolved Hide resolved
cmd/geth/blsaccountcmd.go Outdated Show resolved Hide resolved
@buddh0 buddh0 force-pushed the improve_bls_account_import branch 5 times, most recently from 2d973de to 9c2a6ca Compare December 5, 2023 06:25
Copy link
Contributor

@keefel keefel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@brilliant-lx brilliant-lx merged commit 5db73c9 into bnb-chain:develop Dec 5, 2023
4 of 5 checks passed
@buddh0 buddh0 deleted the improve_bls_account_import branch December 12, 2023 02:20
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