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: support creation of Bitcoin testnet accounts #25772

Merged
merged 11 commits into from
Jul 16, 2024

Conversation

ccharly
Copy link
Contributor

@ccharly ccharly commented Jul 11, 2024

Description

Adds support of testnet account within the extension following the same pattern than for mainnet accounts with a feature flag.

Open in GitHub Codespaces

Related issues

Fixes: https://github.com/MetaMask/accounts-planning/issues/511

Manual testing steps

  1. Run yarn start:flask
  2. Go to the experimental settings tab
  3. Enable the "Bitcoin testnet support" toggle
  4. Click on the account menu list (on top)
  5. You should now see a "Add a new Bitcoin account (Testnet)"
  6. Click this button and follow the flow
  7. You should now see a testnet account (starting with tb1q...)

Screenshots/Recordings

Screen.Recording.2024-07-11.at.16.37.19.mov

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.

@ccharly ccharly self-assigned this Jul 11, 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.

@ccharly ccharly force-pushed the feature/bitcoin-testnet-account branch from 079451d to 0bc54c5 Compare July 11, 2024 14:42
@ccharly ccharly marked this pull request as ready for review July 11, 2024 14:53
@ccharly ccharly requested a review from a team as a code owner July 11, 2024 14:53
@ccharly ccharly force-pushed the feature/bitcoin-testnet-account branch from 75a338b to 805ac7b Compare July 12, 2024 15:27
@metamaskbot
Copy link
Collaborator

Builds ready [805ac7b]
Page Load Metrics (65 ± 7 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint6612895178
domContentLoaded96228147
load4710165147
domInteractive96228147
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 198 Bytes (0.01%)
  • ui: 86 Bytes (0.00%)
  • common: 610 Bytes (0.01%)

Copy link

codecov bot commented Jul 12, 2024

Codecov Report

Attention: Patch coverage is 40.54054% with 22 lines in your changes missing coverage. Please review.

Project coverage is 69.95%. Comparing base (5ee57a6) to head (42ebe44).
Report is 47 commits behind head on develop.

Files Patch % Lines
.../multichain/account-list-menu/account-list-menu.js 29.41% 12 Missing ⚠️
ui/store/actions.ts 0.00% 4 Missing ⚠️
...gs/experimental-tab/experimental-tab.component.tsx 25.00% 3 Missing ⚠️
app/scripts/controllers/preferences.js 0.00% 2 Missing ⚠️
...ngs/experimental-tab/experimental-tab.container.ts 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #25772      +/-   ##
===========================================
+ Coverage    69.77%   69.95%   +0.18%     
===========================================
  Files         1376     1394      +18     
  Lines        48403    48978     +575     
  Branches     13348    13476     +128     
===========================================
+ Hits         33773    34261     +488     
- Misses       14630    14717      +87     

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

ui/selectors/accounts.ts Outdated Show resolved Hide resolved
@ccharly ccharly force-pushed the feature/bitcoin-testnet-account branch from 1ec1120 to bf2d25f Compare July 15, 2024 12:25
danroc
danroc previously approved these changes Jul 15, 2024
@metamaskbot
Copy link
Collaborator

Builds ready [4763e46]
Page Load Metrics (399 ± 387 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint6972113813665
domContentLoaded107030168
load422852399806387
domInteractive107030168
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 198 Bytes (0.01%)
  • ui: 67 Bytes (0.00%)
  • common: 613 Bytes (0.01%)

Copy link

@metamaskbot
Copy link
Collaborator

Builds ready [42ebe44]
Page Load Metrics (162 ± 203 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint633221085225
domContentLoaded9112312311
load412003162423203
domInteractive9112312311
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 198 Bytes (0.01%)
  • ui: 67 Bytes (0.00%)
  • common: 678 Bytes (0.01%)

@danroc danroc changed the title feat: bitcoin testnet account feat: allow creation of Bitcoin testnet accounts Jul 16, 2024
@danroc danroc changed the title feat: allow creation of Bitcoin testnet accounts feat: support creation of Bitcoin testnet accounts Jul 16, 2024
Copy link
Contributor

@montelaidev montelaidev left a comment

Choose a reason for hiding this comment

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

Tested and works well on 42ebe44

}

export function hasCreatedBtcTestnetAccount(state: AccountsState) {
return hasCreatedBtcAccount(state, isBtcTestnetAddress);
Copy link
Contributor

Choose a reason for hiding this comment

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

This isBtcTestnetAddress would return true for a non-EVM and non-BTC address. We can probably address this later when we support other non-EVM chains

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, and yes we would need to address this! Let's keep note of that for later

@ccharly ccharly merged commit 2d416bf into develop Jul 16, 2024
85 checks passed
@ccharly ccharly deleted the feature/bitcoin-testnet-account branch July 16, 2024 10:12
@github-actions github-actions bot locked and limited conversation to collaborators Jul 16, 2024
@metamaskbot metamaskbot added the release-12.3.0 Issue or pull request that will be included in release 12.3.0 label Jul 16, 2024
@metamaskbot metamaskbot added release-12.2.0 Issue or pull request that will be included in release 12.2.0 and removed release-12.3.0 Issue or pull request that will be included in release 12.3.0 labels Aug 30, 2024
@metamaskbot
Copy link
Collaborator

Missing release label release-12.2.0 on PR. Adding release label release-12.2.0 on PR and removing other release labels(release-12.3.0), as PR was cherry-picked in branch 12.2.0.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
release-12.2.0 Issue or pull request that will be included in release 12.2.0 team-accounts
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants