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: upgrade network controller to v20 #26150

Merged
merged 108 commits into from
Aug 23, 2024
Merged

Conversation

bergeron
Copy link
Contributor

@bergeron bergeron commented Jul 26, 2024

Description

Upgrades the network controller to v20, where providerConfig is removed from state.

To handle this, the getProviderConfig selector is shimmed to return the same data as before. Places that were accessing state directly are now sent through this selector.

A migration is added to remove providerConfig from state.

A helper function mockNetworkState is added to abstract the network schema from unit + e2e tests. This will simplify test writing and the next network controller upgrade, where this state will change again.

Open in GitHub Codespaces

Related issues

Manual testing steps

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.

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.

Copy link

socket-security bot commented Jul 26, 2024

👍 Dependency issues cleared. Learn more about Socket for GitHub ↗︎

This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored.

View full report↗︎

@MetaMask MetaMask deleted a comment from metamaskbot Jul 27, 2024
@MetaMask MetaMask deleted a comment from metamaskbot Jul 27, 2024
@MetaMask MetaMask deleted a comment from metamaskbot Jul 27, 2024
@MetaMask MetaMask deleted a comment from metamaskbot Jul 27, 2024
@MetaMask MetaMask deleted a comment from sonarqubecloud bot Jul 27, 2024
@metamaskbot
Copy link
Collaborator

Builds ready [3c0373d]
Page Load Metrics (131 ± 137 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint752081023115
domContentLoaded1710331189
load471364131284137
domInteractive1710331189
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 1.11 KiB (0.03%)
  • ui: 291 Bytes (0.00%)
  • common: 294 Bytes (0.00%)

mikesposito
mikesposito previously approved these changes Aug 22, 2024
@bergeron
Copy link
Contributor Author

#26147 from develop breaks when merged in since it usses more provider config. Fixing which will reset votes

@bergeron bergeron dismissed stale reviews from mikesposito, hmalik88, and shane-t via 7767a4a August 22, 2024 23:38
@metamaskbot
Copy link
Collaborator

Builds ready [304ebfd]
Page Load Metrics (84 ± 9 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint821751162512
domContentLoaded5212581199
load5913284189
domInteractive185631136
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 2.86 KiB (0.08%)
  • ui: 310 Bytes (0.00%)
  • common: 37.04 KiB (0.44%)

hmalik88
hmalik88 previously approved these changes Aug 23, 2024
@legobeat

This comment was marked as resolved.

@bergeron
Copy link
Contributor Author

bergeron commented Aug 23, 2024

Thanks @legobeat, I've used those steps to regenerate yarn.lock #26150 (comment)

@bergeron
Copy link
Contributor Author

@metamaskbot update-policies

@metamaskbot
Copy link
Collaborator

Policies updated

Copy link

@metamaskbot
Copy link
Collaborator

Builds ready [d1e86fa]
Page Load Metrics (80 ± 26 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint664021076833
domContentLoaded41312755526
load47312805426
domInteractive9110292010
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: -843 Bytes (-0.02%)
  • ui: 310 Bytes (0.00%)
  • common: 37.04 KiB (0.44%)

__classPrivateFieldGet(this, _SmartTransactionsController_instances, "m", _SmartTransactionsController_ensureUniqueSmartTransactions).call(this);
- onNetworkStateChange(({ providerConfig: newProvider }) => {
- const { chainId } = newProvider;
+ onNetworkStateChange((state) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like this change is now part of released @metamask/smart-transactions-controller@12.0.0, while also bumping the transitive dep on @metamask/network-controller 19->20.

Is there a reason to not update to (seemingly equivalent, considering this resolutions entry) @metamask/smart-transactions-controller v12.0.0, or even v12.0.1?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes that can be bumped now. I'll do that as quick follow up so i can get the bulk of this merged

Copy link
Contributor Author

Choose a reason for hiding this comment

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

bumped here #26644

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.

Changes in files owned by confirmations team look good 👍

@bergeron bergeron merged commit 800a9d3 into develop Aug 23, 2024
78 checks passed
@bergeron bergeron deleted the brian/network-controller-v20 branch August 23, 2024 15:30
@github-actions github-actions bot locked and limited conversation to collaborators Aug 23, 2024
@metamaskbot metamaskbot added the release-12.5.0 Issue or pull request that will be included in release 12.5.0 label Aug 23, 2024
@gauthierpetetin gauthierpetetin added release-12.4.0 Issue or pull request that will be included in release 12.4.0 and removed release-12.5.0 Issue or pull request that will be included in release 12.5.0 labels Sep 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
release-12.4.0 Issue or pull request that will be included in release 12.4.0 team-assets
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.