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

chore: revert recent gas api endpoint changes #25230

Merged
merged 5 commits into from
Jun 13, 2024

Conversation

dbrans
Copy link
Contributor

@dbrans dbrans commented Jun 11, 2024

Note

Once this PR is merged into develop, it will be cherry-picked into the next 11.16 hotfix as well as v12.0.0.

Description

We need to move quickly on a hotfix for the Extension due to a caching issue with the infura.io endpoint. The API team has asked us to revert a recent gas API endpoint change until the issue is resolved, which could take months.

Here is what is in this PR:

Re: Large gas-fee-controller patch file

The renaming of chunk files in the dist folder of the gas-fee-controller are the cause of the large .patch file. For more context, see this slack thread.

Re: Transitive dependencies on [email protected]

Although transaction-controller and user-operation-controller depend on v17, they can be safely ignored. The only runtime dependency those packages have is on the enum-like GAS_ESTIMATE_TYPES, (example1, example2) which hasn't be touched in 3 years.

After the hotfix we need to:

  1. Revert MetaMask/core/pulls/4068 in core on top of the latest gas-fee-controller (v17)
  2. Upgrade gas-fee-controller in Extension and Mobile

Open in GitHub Codespaces

Related issues

Manual testing steps

  1. Go to this page...

Screenshots/Recordings

Before

After

image

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.

@dbrans
Copy link
Contributor Author

dbrans commented Jun 11, 2024

@metamaskbot update-policies

@dbrans dbrans added the team-transactions Transactions team label Jun 11, 2024
@dbrans dbrans requested a review from OGPoyraz June 11, 2024 20:08
@dbrans dbrans marked this pull request as ready for review June 11, 2024 20:08
@dbrans dbrans requested review from a team as code owners June 11, 2024 20:08
@dbrans dbrans requested a review from blurpesec June 11, 2024 20:09
@metamaskbot
Copy link
Collaborator

Policy update failed. You can review the logs or retry the policy update here

blurpesec
blurpesec previously approved these changes Jun 11, 2024
@dbrans dbrans force-pushed the dbrans/revert-gas-api-endpoint branch from 9e44361 to b81bb5d Compare June 12, 2024 01:57
@dbrans
Copy link
Contributor Author

dbrans commented Jun 12, 2024

@metamaskbot update-policies

@metamaskbot
Copy link
Collaborator

Policy update failed. You can review the logs or retry the policy update here

1 similar comment
@metamaskbot
Copy link
Collaborator

Policy update failed. You can review the logs or retry the policy update here

@metamaskbot
Copy link
Collaborator

Policies updated

@metamaskbot metamaskbot requested review from a team as code owners June 12, 2024 12:59
@metamaskbot
Copy link
Collaborator

No policy changes

@micaelae micaelae force-pushed the dbrans/revert-gas-api-endpoint branch 2 times, most recently from 4a72af4 to 418ecfc Compare June 12, 2024 19:42
@metamaskbot
Copy link
Collaborator

Builds ready [418ecfc]
Page Load Metrics (133 ± 162 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint711098694
domContentLoaded9241342
load421606133338162
domInteractive9241342
Bundle size diffs [🚀 Bundle size reduced!]
  • background: 111 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: -703 Bytes (-0.01%)

Copy link

codecov bot commented Jun 12, 2024

Codecov Report

Attention: Patch coverage is 66.66667% with 3 lines in your changes missing coverage. Please review.

Project coverage is 65.65%. Comparing base (d8e0cd1) to head (f011d0a).
Report is 12 commits behind head on develop.

Files Patch % Lines
app/scripts/metamask-controller.js 50.00% 3 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #25230      +/-   ##
===========================================
+ Coverage    65.61%   65.65%   +0.04%     
===========================================
  Files         1372     1375       +3     
  Lines        54519    54530      +11     
  Branches     14282    14299      +17     
===========================================
+ Hits         35770    35800      +30     
+ Misses       18749    18730      -19     

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

@dbrans dbrans force-pushed the dbrans/revert-gas-api-endpoint branch from 0b6ff56 to 4a1b4e8 Compare June 12, 2024 21:26
@dbrans dbrans mentioned this pull request Jun 12, 2024
7 tasks
micaelae
micaelae previously approved these changes Jun 12, 2024
@dbrans dbrans added the DO-NOT-MERGE Pull requests that should not be merged label Jun 13, 2024
@metamaskbot
Copy link
Collaborator

Builds ready [63bf6c1]
Page Load Metrics (45 ± 4 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint64967784
domContentLoaded9201021
load39684574
domInteractive9201021
Bundle size diffs [🚀 Bundle size reduced!]
  • background: 111 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: -703 Bytes (-0.01%)

@dbrans dbrans removed the DO-NOT-MERGE Pull requests that should not be merged label Jun 13, 2024
@metamaskbot
Copy link
Collaborator

Builds ready [f011d0a]
Page Load Metrics (265 ± 284 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint76146105157
domContentLoaded11381463
load452052265592284
domInteractive11381463

@blurpesec blurpesec self-requested a review June 13, 2024 16:34
@dbrans dbrans requested a review from a team June 13, 2024 17:15
Copy link
Contributor

@mcmire mcmire left a comment

Choose a reason for hiding this comment

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

Had one question, but otherwise LGTM.

package.json Show resolved Hide resolved
@dbrans dbrans merged commit 04642d2 into develop Jun 13, 2024
86 of 87 checks passed
@dbrans dbrans deleted the dbrans/revert-gas-api-endpoint branch June 13, 2024 17:48
@github-actions github-actions bot locked and limited conversation to collaborators Jun 13, 2024
@metamaskbot metamaskbot added the release-12.1.0 Issue or pull request that will be included in release 12.1.0 label Jun 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
release-12.1.0 Issue or pull request that will be included in release 12.1.0 team-transactions Transactions team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants