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.

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

feat: Enable hardware wallets for smart transactions, sign a transaction only once #26251

Merged
merged 3 commits into from
Aug 21, 2024

Conversation

dan437
Copy link
Contributor

@dan437 dan437 commented Jul 31, 2024

Description

This PR enables hardware wallets for smart transactions and uses an already signed transaction from the TransactionController when submitting a smart transaction.

Open in GitHub Codespaces

Related issues

Fixes:

Manual testing steps

  1. Enable smart transactions
  2. For example the Send feature now works with a HW wallet + smart transaction
  3. After confirming a tx on a HW device we show the smart transaction status page

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

@dan437 dan437 requested a review from a team as a code owner July 31, 2024 15:26
@dan437 dan437 added the team-transactions Transactions team label Jul 31, 2024
Copy link
Contributor

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@dan437 dan437 marked this pull request as draft July 31, 2024 15:27
Copy link

codecov bot commented Jul 31, 2024

Codecov Report

Attention: Patch coverage is 66.66667% with 4 lines in your changes missing coverage. Please review.

Project coverage is 69.96%. Comparing base (dd4c2b4) to head (e68fea0).
Report is 8 commits behind head on develop.

Files Patch % Lines
ui/pages/swaps/prepare-swap-page/review-quote.js 50.00% 2 Missing ⚠️
app/scripts/metamask-controller.js 0.00% 1 Missing ⚠️
ui/ducks/swaps/swaps.js 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #26251      +/-   ##
===========================================
- Coverage    69.96%   69.96%   -0.00%     
===========================================
  Files         1405     1405              
  Lines        48996    48998       +2     
  Branches     13697    13698       +1     
===========================================
+ Hits         34280    34281       +1     
- Misses       14716    14717       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@metamaskbot
Copy link
Collaborator

Builds ready [de9053e]
Page Load Metrics (522 ± 374 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint6891017717885
domContentLoaded10128463618
load402573522778374
domInteractive10128463618
Bundle size diffs [🚀 Bundle size reduced!]
  • background: -52 Bytes (-0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@metamaskbot
Copy link
Collaborator

Builds ready [ab541df]
Page Load Metrics (243 ± 229 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint763771196129
domContentLoaded10172323316
load421813243477229
domInteractive10172323316
Bundle size diffs [🚀 Bundle size reduced!]
  • background: -52 Bytes (-0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@metamaskbot
Copy link
Collaborator

Builds ready [4a78888]
Page Load Metrics (83 ± 11 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint712471154019
domContentLoaded43136792512
load49136832311
domInteractive105731136
Bundle size diffs [🚀 Bundle size reduced!]
  • background: -52 Bytes (-0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@metamaskbot
Copy link
Collaborator

Builds ready [40e73e7]
Page Load Metrics (61 ± 5 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint6611183126
domContentLoaded418958105
load478961105
domInteractive9432294
Bundle size diffs [🚀 Bundle size reduced!]
  • background: -52 Bytes (-0.00%)
  • ui: 21 Bytes (0.00%)
  • common: 10 Bytes (0.00%)

Copy link

@dan437 dan437 marked this pull request as ready for review August 20, 2024 12:31
@dan437 dan437 requested a review from a team as a code owner August 20, 2024 12:31
@dan437 dan437 requested review from antonydenyer and dbrans August 20, 2024 12:38
@metamaskbot
Copy link
Collaborator

Builds ready [e68fea0]
Page Load Metrics (72 ± 14 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint792791064220
domContentLoaded46191683115
load53191723014
domInteractive138028168
Bundle size diffs [🚀 Bundle size reduced!]
  • background: -52 Bytes (-0.00%)
  • ui: 21 Bytes (0.00%)
  • common: 10 Bytes (0.00%)

Copy link
Contributor

@davibroc davibroc left a comment

Choose a reason for hiding this comment

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

QA Approved

Copy link
Contributor

@dbrans dbrans left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Contributor

@martahj martahj left a comment

Choose a reason for hiding this comment

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

lgtm!

@dan437 dan437 merged commit 590f0a9 into develop Aug 21, 2024
78 checks passed
@dan437 dan437 deleted the hw-for-stx branch August 21, 2024 09:46
@github-actions github-actions bot locked and limited conversation to collaborators Aug 21, 2024
@metamaskbot metamaskbot added the release-12.5.0 Issue or pull request that will be included in release 12.5.0 label Aug 21, 2024
@gauthierpetetin gauthierpetetin added the release-12.4.0 Issue or pull request that will be included in release 12.4.0 label Sep 11, 2024
@gauthierpetetin gauthierpetetin removed the release-12.5.0 Issue or pull request that will be included in release 12.5.0 label Sep 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
release-12.4.0 Issue or pull request that will be included in release 12.4.0 team-transactions Transactions team
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

6 participants