-
Notifications
You must be signed in to change notification settings - Fork 688
feat: add unlock all option to allow unknown accounts send transaction #3710
base: develop
Are you sure you want to change the base?
Changes from all commits
e153154
6ab1a2a
060c5ff
aa0d7f4
45d8820
71853a3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1949,13 +1949,22 @@ export default class EthereumApi implements Api { | |
|
||
const wallet = this.#wallet; | ||
const isKnownAccount = wallet.knownAccounts.has(fromString); | ||
const privateKey = wallet.unlockedAccounts.get(fromString); | ||
let privateKey = wallet.unlockedAccounts.get(fromString); | ||
|
||
if (privateKey === undefined) { | ||
const msg = isKnownAccount | ||
? "authentication needed: passphrase or unlock" | ||
: "sender account not recognized"; | ||
throw new Error(msg); | ||
// if unlock all option is set to true | ||
// then create a fake private key for unknown account | ||
if(this.#options.wallet.unlockAll) | ||
{ | ||
privateKey = wallet.createFakePrivateKey(fromString) | ||
} | ||
Comment on lines
+1957
to
+1960
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
else | ||
{ | ||
const msg = isKnownAccount | ||
? "authentication needed: passphrase or unlock" | ||
: "sender account not recognized"; | ||
throw new Error(msg); | ||
} | ||
Comment on lines
+1957
to
+1967
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the styling here is not consistent with the code base - space after |
||
} | ||
|
||
await autofillDefaultTransactionValues( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -202,6 +202,16 @@ export type WalletConfig = { | |
hd_path: string; | ||
}; | ||
}; | ||
|
||
/** | ||
* Unlock all accounts irrespective of known or unknown status. | ||
* | ||
* @defaultValue false | ||
*/ | ||
unlockAll: { | ||
type: boolean; | ||
hasDefault: true; | ||
}; | ||
}; | ||
exclusiveGroups: [ | ||
["accounts", "totalAccounts"], | ||
|
@@ -326,5 +336,13 @@ export const WalletOptions: Definitions<WalletConfig> = { | |
default: () => ["m", "44'", "60'", "0'", "0"], | ||
legacyName: "hd_path", | ||
cliType: "string" | ||
} | ||
}, | ||
unlockAll: { | ||
normalize, | ||
cliDescription: | ||
"Unlock all accounts irrespective of known or unknown status.", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
default: () => false, | ||
cliAliases: ["unlockAll"], | ||
cliType: "boolean" | ||
}, | ||
}; |
Uh oh!
There was an error while loading. Please reload this page.