feat: add unlock all option to allow unknown accounts send transaction #3710
Conversation
src/chains/ethereum/ethereum/tests/api/eth/sendTransaction.test.ts
Outdated
Show resolved
Hide resolved
src/chains/ethereum/ethereum/tests/api/eth/sendTransaction.test.ts
Outdated
Show resolved
Hide resolved
src/chains/ethereum/ethereum/tests/api/eth/sendTransaction.test.ts
Outdated
Show resolved
Hide resolved
src/chains/ethereum/ethereum/tests/api/eth/sendTransaction.test.ts
Outdated
Show resolved
Hide resolved
src/chains/ethereum/ethereum/tests/api/eth/sendTransaction.test.ts
Outdated
Show resolved
Hide resolved
| 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.
| 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.
Fixes: #3620 Adds wallet.unlockAll option to wallet options. This allows unknown accounts send transaction when the value is true. Default value is false.