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.

Sign up for GitHub

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

Jump to bottom

refactor: remove global network from transaction controller #28449

Merged
merged 13 commits into from
Nov 29, 2024
Merged

refactor: remove global network from transaction controller #28449

merged 13 commits into from
Nov 29, 2024

Conversation

matthewwalsh0
Copy link
Member

@matthewwalsh0 matthewwalsh0 commented Nov 13, 2024 •
edited
Loading

Description

Upgrade @metamask/transaction-controller to remove all usages of the global network.

Specifically:

  • Remove deleted constructor options.
  • Add getGlobalChainId and getGlobalNetworkClientId private methods in MetamaskController.
  • Remove TRANSACTION_MULTICHAIN environment variable.
  • Add networkClientId to test data.
  • Update calls to:
    • addTransaction
    • estimateGasBuffered
    • getNonceLock
    • getTransactions
    • startIncomingTransactionPolling
    • updateIncomingTransactions
    • wipeTransactions

Open in GitHub Codespaces

Related issues

Fixes: #3499

Manual testing steps

Full regression of all transaction flows.

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.

@matthewwalsh0
Use preview build
f77045d 
Remove multichain flag.
Remove getCurrentChainId usage in background.
@metamaskbot metamaskbot added the team-confirmations Push issues to confirmations team label Nov 13, 2024
@matthewwalsh0
Fix linting
5ca3702 
Fix unit tests.
Update Lavamoat policies.
@matthewwalsh0
Fix nonce lock call
cafd321
@metamaskbot
Copy link
Collaborator

Builds ready [cafd321]
Page Load Metrics (1826 ± 57 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint38621421765343165
domContentLoaded15912126179711455
load16092142182611957
domInteractive17104472210
backgroundConnect96831199
firstReactRender5511796147
getState55312105
initialActions00000
loadScripts1153153213289546
setupStore65917209
uiStartup17882350203912761
Bundle size diffs [🚀 Bundle size reduced!]
  • background: -241 Bytes (-0.00%)
  • ui: 0 Bytes (0.00%)
  • common: -2.73 KiB (-0.03%)

@metamaskbot
Copy link
Collaborator

Builds ready [8c6efbf]
Page Load Metrics (2015 ± 103 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint50926131960407196
domContentLoaded16682444197920297
load167924952015215103
domInteractive2095532010
backgroundConnect799363014
firstReactRender522981225125
getState497182311
initialActions01000
loadScripts12051736143114971
setupStore688222311
uiStartup188230032303306147
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: -254 Bytes (-0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 638 Bytes (0.01%)

@matthewwalsh0 matthewwalsh0 mentioned this pull request Nov 19, 2024
Merged
4 tasks
@matthewwalsh0 matthewwalsh0 marked this pull request as ready for review November 19, 2024 13:44
@matthewwalsh0 matthewwalsh0 requested review from a team as code owners November 19, 2024 13:44
@metamaskbot
Copy link
Collaborator

Builds ready [fcd95a7]
Page Load Metrics (1981 ± 143 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint32127611781549264
domContentLoaded156526861955291140
load157327661981298143
domInteractive15179533517
backgroundConnect786252512
firstReactRender871741112311
getState468202211
initialActions01000
loadScripts113620191448237114
setupStore55913157
uiStartup176832892229374180
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: -254 Bytes (-0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 709 Bytes (0.01%)

jpuri
jpuri previously approved these changes Nov 21, 2024
View reviewed changes
@metamaskbot
Copy link
Collaborator

Builds ready [815aaed]
Page Load Metrics (2029 ± 100 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint32224941853542260
domContentLoaded16352421199920799
load166424972029208100
domInteractive268349188
backgroundConnect1175352210
firstReactRender1774442211
getState667292311
initialActions01000
loadScripts11831875148117785
setupStore65213136
uiStartup190327202277219105
Bundle size diffs [🚀 Bundle size reduced!]
  • background: -278 Bytes (-0.00%)
  • ui: 0 Bytes (0.00%)
  • common: -2.75 KiB (-0.03%)

@metamaskbot
Copy link
Collaborator

Builds ready [c378718]
Page Load Metrics (1989 ± 147 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint49331151925447215
domContentLoaded162031011962300144
load163831101989307147
domInteractive2796512211
backgroundConnect8124262814
firstReactRender1978432210
getState55114126
initialActions00000
loadScripts11732036141718991
setupStore767272512
uiStartup186034052228326156
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: -278 Bytes (-0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 3.82 KiB (0.05%)

@metamaskbot
Copy link
Collaborator

Builds ready [7f8ea7d]
Page Load Metrics (1493 ± 23 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint28616031446271130
domContentLoaded1380157114714823
load1392158314934723
domInteractive227049188
backgroundConnect115825147
firstReactRender1569322211
getState64913126
initialActions00000
loadScripts1005120510825125
setupStore65113157
uiStartup1560176116415125
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: -278 Bytes (-0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 709 Bytes (0.01%)

@socket-security Socket Security
Copy link

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package New capabilities Transitives Size Publisher
npm/@metamask/transaction-controller@41.0.0 network 0 2.26 MB danfinlay, gudahtt, kumavis, ...6 more

🚮 Removed packages: npm/@metamask/transaction-controller@40.1.0

View full report↗︎

@metamaskbot
Copy link
Collaborator

Builds ready [96ce747]
Page Load Metrics (2045 ± 128 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint28026191874584280
domContentLoaded153126062021269129
load153926172045266128
domInteractive269844199
backgroundConnect888292311
firstReactRender156225126
getState471421012110
initialActions00000
loadScripts112621791543243116
setupStore65513115
uiStartup175928392326305147
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: -159 Bytes (-0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 709 Bytes (0.01%)

@matthewwalsh0 matthewwalsh0 added this pull request to the merge queue Nov 29, 2024
Merged via the queue into develop with commit 7d252e9 Nov 29, 2024
75 checks passed
@matthewwalsh0 matthewwalsh0 deleted the refactor/transaction-controller-remove-global-network branch November 29, 2024 16:48
@github-actions github-actions bot locked and limited conversation to collaborators Nov 29, 2024
@metamaskbot metamaskbot added the release-12.10.0 Issue or pull request that will be included in release 12.10.0 label Nov 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@pedronfigueiredo pedronfigueiredo pedronfigueiredo approved these changes

@OGPoyraz OGPoyraz OGPoyraz approved these changes

@jpuri jpuri Awaiting requested review from jpuri

@vinistevam vinistevam Awaiting requested review from vinistevam

Assignees
No one assigned
Labels
QA Passed release-12.10.0 Issue or pull request that will be included in release 12.10.0 team-confirmations Push issues to confirmations team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@matthewwalsh0 @metamaskbot @jpuri @OGPoyraz @pedronfigueiredo @sleepytanya