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

Put hardware wallets behind an HARDWARE_WALLETS_MV3 flag #17354

Merged
merged 5 commits into from
Jan 25, 2023

Conversation

segun
Copy link
Contributor

@segun segun commented Jan 23, 2023

Disable Hardware connect buttons if the flag is set.

Signed-off-by: Akintayo A. Olusegun [email protected]

update to metamask/eth-keyring-controller

Signed-off-by: Akintayo A. Olusegun [email protected]

revert to eth-keyring-controller v8

Signed-off-by: Akintayo A. Olusegun [email protected]

LAvamost after rebase

Signed-off-by: Akintayo A. Olusegun [email protected]

lock file.

Signed-off-by: Akintayo A. Olusegun [email protected]

Added toaster for removed NFTs (#17297)

  • added notification for remove nfts

  • reverted names for tabs

  • updated default key

  • updated snapshot

  • updated remove nft toast to danger

Setup network controller mocks per-test (#17250)

The network controller unit test network mocks are now setup for each test. This makes modifying network behavior on a per-test basis easier, and makes it more clear which test relies upon which mocks.

Security provider check (OpenSea) (#16584)

chore: copy update for metamask fee on swaps (#17133)

Explanation

Screenshots/Screencaps

Before

After

Manual Testing Steps

Pre-merge author checklist

  • I've clearly explained:
    • What problem this PR is solving
    • How this problem was solved
    • How reviewers can test my changes
  • Sufficient automated test coverage has been added

Pre-merge reviewer checklist

  • Manual testing (e.g. pull and build branch, run in browser, test code being changed)
  • PR is linked to the appropriate GitHub issue
  • IF this PR fixes a bug in the release milestone, add this PR to the release milestone

If further QA is required (e.g. new feature, complex testing steps, large refactor), add the Extension QA Board label.

In this case, a QA Engineer approval will be be required.

Disable Hardware connect buttons if the flag is set.

Signed-off-by: Akintayo A. Olusegun <[email protected]>

update to metamask/eth-keyring-controller

Signed-off-by: Akintayo A. Olusegun <[email protected]>

revert to eth-keyring-controller v8

Signed-off-by: Akintayo A. Olusegun <[email protected]>

LAvamost after rebase

Signed-off-by: Akintayo A. Olusegun <[email protected]>

lock file.

Signed-off-by: Akintayo A. Olusegun <[email protected]>

Added toaster for removed NFTs (#17297)

* added notification for remove nfts

* reverted names for tabs

* updated default key

* updated snapshot

* updated remove nft toast to danger

Setup network controller mocks per-test (#17250)

The network controller unit test network mocks are now setup for each
test. This makes modifying network behavior on a per-test basis easier,
and makes it more clear which test relies upon which mocks.

Security provider check (OpenSea) (#16584)

chore: copy update for metamask fee on swaps (#17133)
@segun segun requested a review from a team as a code owner January 23, 2023 17:08
@segun segun requested a review from ryanml January 23, 2023 17:08
@github-actions
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.

@metamaskbot
Copy link
Collaborator

Builds ready [90dee4d]
Page Load Metrics (1427 ± 124 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint107175134178
domContentLoaded104119391398269129
load104119391427259124
domInteractive104119391397269129
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 304 bytes
  • ui: 345 bytes
  • common: 0 bytes

@codecov-commenter
Copy link

Codecov Report

Merging #17354 (90dee4d) into develop (51518c4) will decrease coverage by 0.01%.
The diff coverage is 16.67%.

@@             Coverage Diff             @@
##           develop   #17354      +/-   ##
===========================================
- Coverage    60.05%   60.04%   -0.01%     
===========================================
  Files          936      936              
  Lines        36047    36057      +10     
  Branches      9261     9268       +7     
===========================================
+ Hits         21646    21648       +2     
- Misses       14401    14409       +8     
Impacted Files Coverage Δ
...create-account/connect-hardware/select-hardware.js 0.00% <0.00%> (ø)
app/scripts/metamask-controller.js 60.54% <33.33%> (+0.05%) ⬆️
app/scripts/migrations/023.js 93.75% <0.00%> (-3.12%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@danjm
Copy link
Contributor

danjm commented Jan 24, 2023

Copying over my comment from the other PR:

setLedgerTransportPreference in the metamask-controller should not be called, or should return immediately, if canUseHardwareWallets() is false

Copy link
Contributor

@danjm danjm left a comment

Choose a reason for hiding this comment

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

one change needed

… called, or should return immediately, if canUseHardwareWallets() is false

Signed-off-by: Akintayo A. Olusegun <[email protected]>
Copy link
Contributor

@danjm danjm 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

@jpuri jpuri left a comment

Choose a reason for hiding this comment

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

👍

@metamaskbot
Copy link
Collaborator

Builds ready [14e99fa]
Page Load Metrics (1284 ± 116 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint971971182211
domContentLoaded100016671260233112
load102017451284241116
domInteractive100016671260233112
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 354 bytes
  • ui: 345 bytes
  • common: 0 bytes

@danjm danjm merged commit 16ea4a8 into develop Jan 25, 2023
@danjm danjm deleted the dev-olu-hardware-wallet-feature-flag-2 branch January 25, 2023 21:12
@github-actions github-actions bot locked and limited conversation to collaborators Jan 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants