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: replace experimental add solana account with remote flag #30487

Conversation

aganglada
Copy link
Contributor

@aganglada aganglada commented Feb 21, 2025

Description

  • Removes experimental Enable "Add Solana account" setting
  • Adds remote feature flag fence
  • Enable feature in flask - dev

Open in GitHub Codespaces

Related issues

Fixes:

Manual testing steps

  1. Add MANIFEST_OVERRIDES=.manifest-overrides.json to the .metamaskrc
  2. Create a .manifest-overrides.json file
  3. Add the following content
{
  "_flags": {
    "remoteFeatureFlags": { "addSolanaAccount": true }
  }
}
  1. Build
  2. Modify the value to test both cases (enabled/disabled)

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.

@aganglada aganglada self-assigned this Feb 21, 2025
@aganglada aganglada requested a review from a team as a code owner February 21, 2025 13:27
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 metamaskbot added the team-sol PRs from the Solana snap team label Feb 21, 2025
@aganglada aganglada requested a review from Gudahtt February 21, 2025 13:28
@aganglada aganglada requested a review from a team as a code owner February 21, 2025 14:44
@aganglada aganglada force-pushed the SOL-134-extension-replace-experimental-toggle-for-rc-with-remote-flags branch from ec34c73 to 21a7f01 Compare February 24, 2025 08:57
Copy link
Contributor

@ccharly ccharly left a comment

Choose a reason for hiding this comment

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

Also, we should either remove this selector here:

OR, we could keep it as and re-use the remote-ff-flag under the hood (I've never really used it, so IDK if that plays well with our selectors?)

@metamaskbot
Copy link
Collaborator

Builds ready [21a7f01]
Page Load Metrics (1754 ± 73 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint14351985174716077
domContentLoaded14241966171615172
load14331991175415173
domInteractive18125442612
backgroundConnect1194392411
firstReactRender147029199
getState572232311
initialActions01000
loadScripts10291500127913163
setupStore85917168
uiStartup16782298200417484
Bundle size diffs [🚀 Bundle size reduced!]
  • background: -47 Bytes (-0.00%)
  • ui: 0 Bytes (0.00%)
  • common: -442 Bytes (-0.01%)

@metamaskbot
Copy link
Collaborator

Builds ready [7991628]
Page Load Metrics (1640 ± 59 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint14421967163912158
domContentLoaded14371872160910852
load14411956164012259
domInteractive24603394
backgroundConnect984312110
firstReactRender1475342411
getState479192211
initialActions01000
loadScripts1010131611688641
setupStore76816178
uiStartup16662185187313263
Bundle size diffs [🚀 Bundle size reduced!]
  • background: -47 Bytes (-0.00%)
  • ui: 0 Bytes (0.00%)
  • common: -535 Bytes (-0.01%)

@aganglada aganglada requested review from a team as code owners February 24, 2025 14:31
@aganglada aganglada force-pushed the SOL-134-extension-replace-experimental-toggle-for-rc-with-remote-flags branch from da2ddf0 to 79feb37 Compare February 24, 2025 14:31
@metamaskbot
Copy link
Collaborator

Builds ready [79feb37]
Page Load Metrics (1658 ± 113 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint33621011472396190
domContentLoaded139724201627242116
load145824261658236113
domInteractive25143453215
backgroundConnect876372311
firstReactRender1490382713
getState5173213818
initialActions01000
loadScripts9681843118820598
setupStore77114147
uiStartup167327681889271130
Bundle size diffs [🚀 Bundle size reduced!]
  • background: -47 Bytes (-0.00%)
  • ui: 0 Bytes (0.00%)
  • common: -303 Bytes (-0.00%)

@DDDDDanica
Copy link
Contributor

The implementation of remote feature flag looks good to me, let me know when you finish resolving other comments 🙏🏻

@metamaskbot
Copy link
Collaborator

Builds ready [34e27a9]
Page Load Metrics (1683 ± 69 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint14222005167814771
domContentLoaded14151987164614770
load14842003168314469
domInteractive248638168
backgroundConnect1070392210
firstReactRender137525209
getState568212411
initialActions01000
loadScripts10221491120512359
setupStore75911115
uiStartup16832319192116479
Bundle size diffs [🚀 Bundle size reduced!]
  • background: -47 Bytes (-0.00%)
  • ui: 0 Bytes (0.00%)
  • common: -303 Bytes (-0.00%)

@metamaskbot
Copy link
Collaborator

Builds ready [29827e9]
Page Load Metrics (1847 ± 124 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint29524861483715343
domContentLoaded136823671811251121
load141224771847258124
domInteractive24158443014
backgroundConnect7104422813
firstReactRender1575292010
getState45113115
initialActions01000
loadScripts97118701337219105
setupStore871242211
uiStartup161428082129327157
Bundle size diffs [🚀 Bundle size reduced!]
  • background: -47 Bytes (-0.00%)
  • ui: 0 Bytes (0.00%)
  • common: -303 Bytes (-0.00%)

Copy link
Contributor

@DDDDDanica DDDDDanica left a comment

Choose a reason for hiding this comment

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

LGTM

@aganglada aganglada added this pull request to the merge queue Feb 25, 2025
Merged via the queue into main with commit 0ad093f Feb 25, 2025
79 checks passed
@aganglada aganglada deleted the SOL-134-extension-replace-experimental-toggle-for-rc-with-remote-flags branch February 25, 2025 10:48
@github-actions github-actions bot locked and limited conversation to collaborators Feb 25, 2025
@metamaskbot metamaskbot added the release-12.14.0 Issue or pull request that will be included in release 12.14.0 label Feb 25, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
release-12.14.0 Issue or pull request that will be included in release 12.14.0 team-sol PRs from the Solana snap team
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

5 participants