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.

Sign up for GitHub

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

Jump to bottom

Sign messages by Trezor #10903

Merged
merged 1 commit into from
Nov 12, 2021
Merged

Sign messages by Trezor #10903

merged 1 commit into from
Nov 12, 2021

Conversation

spylogsster
Copy link
Contributor

@spylogsster spylogsster commented Nov 5, 2021 •
edited
Loading

Resolves brave/brave-browser#19219

  • Added support for signing messages with Trezor

Submitter Checklist:

  • I confirm that no security/privacy review is needed, or that I have requested one
  • There is a ticket for my issue
  • Used Github auto-closing keywords in the PR description above
  • Wrote a good PR/commit description
  • Added appropriate labels (QA/Yes or QA/No; release-notes/include or release-notes/exclude; OS/...) to the associated issue
  • Checked the PR locally: npm run test -- brave_browser_tests, npm run test -- brave_unit_tests, npm run lint, npm run gn_check, npm run tslint
  • Ran git rebase master (if needed)

Reviewer Checklist:

  • A security review is not needed, or a link to one is included in the PR description
  • New files have MPL-2.0 license header
  • Adequate test coverage exists to prevent regressions
  • Major classes, functions and non-trivial code blocks are well-commented
  • Changes in component dependencies are properly reflected in gn
  • Code follows the style guide
  • Test plan is specified in PR before merging

After-merge Checklist:

Test Plan:

  • Call eth_sign with hardware transactions

@spylogsster spylogsster requested review from a team and Douglashdaniel November 5, 2021 15:09
@spylogsster spylogsster force-pushed the issue-19029-sign-transaction branch from 3daf64c to deb02a1 Compare November 5, 2021 21:06
@spylogsster spylogsster requested review from a team November 5, 2021 21:06
@spylogsster spylogsster force-pushed the issue-19029-sign-transaction branch from deb02a1 to cc5be49 Compare November 6, 2021 07:16
@spylogsster spylogsster force-pushed the issue-19219-sign-messages-trezor branch from 2b0e640 to 576ace2 Compare November 6, 2021 07:17
@spylogsster spylogsster removed request for a team November 6, 2021 07:17
@spylogsster spylogsster force-pushed the issue-19029-sign-transaction branch from cc5be49 to 44e461e Compare November 6, 2021 19:09
@spylogsster spylogsster force-pushed the issue-19219-sign-messages-trezor branch from 576ace2 to d03f3b3 Compare November 6, 2021 19:13
@spylogsster spylogsster force-pushed the issue-19029-sign-transaction branch from 44e461e to d4c0461 Compare November 7, 2021 09:53
@spylogsster spylogsster force-pushed the issue-19219-sign-messages-trezor branch from d03f3b3 to a9aec12 Compare November 7, 2021 09:54
@spylogsster spylogsster force-pushed the issue-19029-sign-transaction branch from d4c0461 to f40759c Compare November 8, 2021 14:12
@spylogsster spylogsster force-pushed the issue-19219-sign-messages-trezor branch from a9aec12 to 97d6464 Compare November 8, 2021 14:15
@spylogsster spylogsster force-pushed the issue-19029-sign-transaction branch from f40759c to 8c31a6e Compare November 9, 2021 05:27
@spylogsster spylogsster force-pushed the issue-19219-sign-messages-trezor branch from 97d6464 to 794ad9c Compare November 9, 2021 07:29
@spylogsster spylogsster force-pushed the issue-19029-sign-transaction branch 3 times, most recently from 267e1d2 to 766a10e Compare November 9, 2021 08:49
@spylogsster spylogsster force-pushed the issue-19219-sign-messages-trezor branch from 794ad9c to a459d89 Compare November 9, 2021 08:50
@spylogsster spylogsster force-pushed the issue-19029-sign-transaction branch from cfb9020 to 8e2e123 Compare November 9, 2021 15:47
@spylogsster spylogsster force-pushed the issue-19219-sign-messages-trezor branch from a459d89 to 39f385e Compare November 9, 2021 15:56
@spylogsster spylogsster force-pushed the issue-19029-sign-transaction branch from 8e2e123 to 55db922 Compare November 9, 2021 19:38
@spylogsster spylogsster force-pushed the issue-19219-sign-messages-trezor branch from 39f385e to 37e8aec Compare November 9, 2021 19:43
@spylogsster spylogsster force-pushed the issue-19029-sign-transaction branch from e4760c5 to 636a2ad Compare November 10, 2021 06:43
@spylogsster spylogsster force-pushed the issue-19219-sign-messages-trezor branch from 37e8aec to eaf1f21 Compare November 10, 2021 06:59
petemill
petemill approved these changes Nov 11, 2021
View reviewed changes
Copy link
Member

@petemill petemill left a comment

Choose a reason for hiding this comment

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

Looks good. Please fix string spelling before merge, anything else is just a comment

components/brave_wallet_ui/panel/async/wallet_panel_async_handler.ts
Comment on lines +258 to 262
const signMessageRequest = await getPendingSignMessageRequest()
if (signMessageRequest) {
store.dispatch(PanelActions.signMessage(signMessageRequest))
return
}
Copy link
Member

Choose a reason for hiding this comment

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

Just a question (and maybe warrants a code comment in a follow-up): What does this do? Attempt to re-sign the request again? Or sign the next request if there is one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sign next if there is one

components/resources/wallet_strings.grdp Outdated Show resolved Hide resolved
@spylogsster spylogsster force-pushed the issue-19219-sign-messages-trezor branch from 41f15b3 to c3ede61 Compare November 12, 2021 05:26
@spylogsster spylogsster requested a review from a team as a code owner November 12, 2021 05:26
@spylogsster spylogsster force-pushed the issue-19219-sign-messages-trezor branch from c3ede61 to 0d41cc5 Compare November 12, 2021 05:26
@spylogsster spylogsster force-pushed the issue-19219-sign-messages-trezor branch from 0d41cc5 to d2ff731 Compare November 12, 2021 05:29
@spylogsster spylogsster merged commit a375958 into master Nov 12, 2021
@spylogsster spylogsster deleted the issue-19219-sign-messages-trezor branch November 12, 2021 07:17
spylogsster added a commit that referenced this pull request Nov 12, 2021
@spylogsster
Sign messages by Trezor (#10903)
6eeed77 
(cherry picked from commit a375958)
@spylogsster spylogsster mentioned this pull request Nov 12, 2021
Merged
7 tasks
@spylogsster spylogsster added this to the 1.33.x - Beta milestone Nov 12, 2021
@spylogsster spylogsster mentioned this pull request Nov 12, 2021
Merged
7 tasks
@spylogsster spylogsster mentioned this pull request Nov 12, 2021
Merged
7 tasks
spylogsster added a commit that referenced this pull request Nov 12, 2021
@spylogsster
Sign messages by Trezor (#10903)
bb78c70 
(cherry picked from commit a375958)
spylogsster added a commit that referenced this pull request Nov 12, 2021
@spylogsster
Sign messages by Trezor (#10903) (#11036)
4901c56 
(cherry picked from commit a375958)
kjozwiak added a commit that referenced this pull request Nov 12, 2021
@kjozwiak
Merge pull request #11035 from brave/pr10902_sign_trezor_transactions
8c20070 
(uplift to 1.32.x) Signing transactions and messages with Trezor accounts (#10902 #10903)
kjozwiak added a commit that referenced this pull request Nov 12, 2021
@kjozwiak
Merge pull request #11026 from brave/pr10902_sign_trezor_transactions…
5ae00f4 
…_to_33

(uplift to 1.33.x) Signing transactions and messages with Trezor accounts (#10902 #10903)
@srirambv srirambv mentioned this pull request Nov 16, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@darkdh darkdh darkdh approved these changes

@petemill petemill petemill approved these changes

@Douglashdaniel Douglashdaniel Awaiting requested review from Douglashdaniel

Assignees

@spylogsster spylogsster

Labels
None yet
Projects
None yet
Milestone
1.34.x - Release
Development

Successfully merging this pull request may close these issues.

Add support for signing messages by Trezor
4 participants
@spylogsster @petemill @darkdh @kjozwiak