This repository has been archived by the owner on Feb 26, 2024. It is now read-only.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
feat: add unlock all option to allow unknown accounts send transaction #3710
base: develop
Are you sure you want to change the base?
feat: add unlock all option to allow unknown accounts send transaction #3710
Changes from all commits
e153154
6ab1a2a
060c5ff
aa0d7f4
45d8820
71853a3
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The 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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the styling here is not consistent with the code base - space after
if
,{
should be on preceding lineThere was a problem hiding this comment.
Choose a reason for hiding this comment
The 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 comment
The 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.