feat: add unlock all option to allow unknown accounts send transaction#3710
feat: add unlock all option to allow unknown accounts send transaction#3710satyajeetkolhapure wants to merge 6 commits into
Conversation
| unlockAll: { | ||
| normalize, | ||
| cliDescription: | ||
| "Unlock all accounts irrespective of known or unknown status.", |
There was a problem hiding this comment.
I can't think of better/clearer alternatives, but I wonder if this help text will make sense to our users.
I'm curious on other reviewers' thoughts on this
There was a problem hiding this comment.
I agree - the term "known or unknown status" seems to be an internal concern. We could probably get more information in there, but at the very least, just "Unlock all accounts" would be clearer.
MicaiahReid
left a comment
MicaiahReid
left a comment
There was a problem hiding this comment.
Approved on the code changes, though the PR description needs to be updated to be "release ready"
| if(this.#options.wallet.unlockAll) | ||
| { | ||
| privateKey = wallet.createFakePrivateKey(fromString) | ||
| } | ||
| else | ||
| { | ||
| const msg = isKnownAccount | ||
| ? "authentication needed: passphrase or unlock" | ||
| : "sender account not recognized"; | ||
| throw new Error(msg); | ||
| } |
There was a problem hiding this comment.
the styling here is not consistent with the code base - space after if, { should be on preceding line
| unlockAll: { | ||
| normalize, | ||
| cliDescription: | ||
| "Unlock all accounts irrespective of known or unknown status.", |
There was a problem hiding this comment.
I agree - the term "known or unknown status" seems to be an internal concern. We could probably get more information in there, but at the very least, just "Unlock all accounts" would be clearer.
| if(this.#options.wallet.unlockAll) | ||
| { | ||
| privateKey = wallet.createFakePrivateKey(fromString) | ||
| } |
There was a problem hiding this comment.
Should this be implemented in the wallet, rather than the api? I can imagine us chasing different cases where we need to unlock the account.
4 participants
Fixes: #3620 Adds wallet.unlockAll option to wallet options. This allows unknown accounts send transaction when the value is true. Default value is false.