Skip to content

Conversation

@segun
Copy link
Contributor

@segun segun commented Nov 29, 2022

Explanation

Put the hardware wallet behind a feature flag and disable/remove ledger, trezor and gridplus connect buttons

Screenshots/Screencaps

Before

Screenshot 2022-11-29 at 17 39 27

After

Screenshot 2022-11-29 at 17 44 53

Manual Testing Steps

  1. Start MetaMask in MV3 mode
  2. Login
  3. Click on Connect Hardware wallet
  4. You should see the screen like in Before
  5. Edit .metamaskrc
  6. Add HARDWARE_WALLETS_MV3=1
  7. Restart MetaMask in MV3 mode
  8. Login
  9. Click Connect Hardware wallet
  10. You should see screen like in After
  11. Restart MetaMask in MV2 mode
  12. Login
  13. Click Connect Hardware wallet
  14. You should see screen like in Before

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.

@segun segun requested a review from a team as a code owner November 29, 2022 13:43
@segun segun requested a review from ryanml November 29, 2022 13:43
@segun segun self-assigned this Nov 29, 2022
@segun segun marked this pull request as draft November 29, 2022 13:43
@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 [3857747]
Page Load Metrics (2136 ± 102 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint963361334923
domContentLoaded176424882110211101
load179924892136212102
domInteractive176424882110211101
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 290 bytes
  • ui: 345 bytes
  • common: 0 bytes

highlights:

storybook

@metamaskbot
Copy link
Collaborator

Builds ready [944c0c8]
Page Load Metrics (1702 ± 261 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint1021692213340163
domContentLoaded117037361683549264
load119537361702544261
domInteractive117037361683549264
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 269 bytes
  • ui: 345 bytes
  • common: 0 bytes

highlights:

storybook

@segun segun force-pushed the dev-olu-hardware-wallet-feature-flag branch from 944c0c8 to d9af3fe Compare January 19, 2023 13:06
@metamaskbot
Copy link
Collaborator

Builds ready [d9af3fe]
Page Load Metrics (1259 ± 84 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint94140117126
domContentLoaded10411603122417283
load10461604125917584
domInteractive10411603122417283
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 269 bytes
  • ui: 345 bytes
  • common: 0 bytes

highlights:

storybook

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.

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

segun added 4 commits January 23, 2023 12:05
Disable Hardware connect buttons if the flag is set.

Signed-off-by: Akintayo A. Olusegun <[email protected]>
Signed-off-by: Akintayo A. Olusegun <[email protected]>
Signed-off-by: Akintayo A. Olusegun <[email protected]>
Signed-off-by: Akintayo A. Olusegun <[email protected]>
@segun segun force-pushed the dev-olu-hardware-wallet-feature-flag branch from 044c1a6 to c6c330e Compare January 23, 2023 12:55
Signed-off-by: Akintayo A. Olusegun <[email protected]>
@segun segun force-pushed the dev-olu-hardware-wallet-feature-flag branch from 1100784 to ee6652a Compare January 24, 2023 04:24
@danjm
Copy link
Contributor

danjm commented Jan 27, 2023

This was resolved in #17354

@danjm danjm closed this Jan 27, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Jan 27, 2023
@jpuri jpuri deleted the dev-olu-hardware-wallet-feature-flag branch September 11, 2024 16:14
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.

Put hardware wallet code and features in MV3 behind a feature flag

4 participants