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

patch(gas-fee-controller@15.1.2): revert "use hardcoded Infura gas API urls" (#4068) #4403

Draft
wants to merge 2 commits into
base: base/patch/extension-gas-api-endpoint
Choose a base branch
from

Conversation

dbrans
Copy link
Contributor

@dbrans dbrans commented Jun 11, 2024

Note

This PR was used to create a patch for extension and is not intended to be merged.

  • Link to issue once it gets created
  • Link to commit where patch was created.

This PR reverts commit 850461d (#4068) and is based on d30f7df (gas-fee-controller@15.1.2)
The revert had merge conflicts:

  • determineGasFeeCalculations.ts
  • GasFeeController.ts
  • GasFeeController.test.ts

Explanation

References

Changelog

Checklist

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
  • I've highlighted breaking changes using the "BREAKING" category above as appropriate

This reverts commit 850461d.

merge conflicts:
- determineGasFeeCalculations.ts
- GasFeeController.ts
- GasFeeController.test.ts
@dbrans dbrans force-pushed the patch/extension-gas-api-endpoint branch from 0622203 to ca38df4 Compare June 11, 2024 12:40
@dbrans dbrans added the team-transactions Transactions team label Jun 11, 2024
@dbrans dbrans changed the base branch from main to base/patch/extension-gas-api-endpoint June 11, 2024 12:59
@dbrans dbrans changed the title patch for extension: revert "use hardcoded Infura gas API urls" (#4068) patch(gas-fee-controller@15.1.2): revert "use hardcoded Infura gas API urls" (#4068) Jun 11, 2024
dbrans added a commit to MetaMask/metamask-extension that referenced this pull request Jun 11, 2024
dbrans added a commit to MetaMask/metamask-extension that referenced this pull request Jun 13, 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](http://infura.io/) endpoint. The [API team
has
asked](https://consensys.slack.com/archives/C05B78N1T9B/p1717783170599949)
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:
- apply patch using core patch branch MetaMask/core/pull/4403
- revert #23717 

### 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](https://consensys.slack.com/archives/CTQAGKY5V/p1718123930090259?thread_ts=1718123750.012709&cid=CTQAGKY5V).

### Re: Transitive dependencies on `gas-fee-controller@17.0.0`
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](https://github.com/MetaMask/core/blob/dcc1d9291297ca2106cd2a461c51ce2f4667d84d/packages/gas-fee-controller/src/GasFeeController.ts#L61-L66),
([example1](https://github.com/MetaMask/core/blob/dcc1d9291297ca2106cd2a461c51ce2f4667d84d/packages/user-operation-controller/src/utils/gas-fees.ts#L224),
[example2](https://github.com/MetaMask/core/blob/dcc1d9291297ca2106cd2a461c51ce2f4667d84d/packages/transaction-controller/src/gas-flows/DefaultGasFeeFlow.ts#L41-L62))
which hasn't be touched in [3
years](https://github.com/MetaMask/core/pull/494/files#diff-ac8f6d6d8ff039810f56d99da8735d4eb8c2978eed2685b1741c9124c7b8bb6fR47-R52).

### 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](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/25230?quickstart=1)

## **Related issues**

- Related: #25194

## **Manual testing steps**

1. Go to this page...
2.
3.

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**

- [ ] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [ ] I've completed the PR template to the best of my ability
- [ ] I’ve included tests if applicable
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **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.

---------

Co-authored-by: MetaMask Bot <metamaskbot@users.noreply.github.com>
dbrans added a commit to MetaMask/metamask-extension that referenced this pull request Jun 13, 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.

We need to move quickly on a hotfix for the Extension due to a caching
issue with the [infura.io](http://infura.io/) endpoint. The [API team
has
asked](https://consensys.slack.com/archives/C05B78N1T9B/p1717783170599949)
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:
- apply patch using core patch branch MetaMask/core/pull/4403
- revert #23717

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](https://consensys.slack.com/archives/CTQAGKY5V/p1718123930090259?thread_ts=1718123750.012709&cid=CTQAGKY5V).

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](https://github.com/MetaMask/core/blob/dcc1d9291297ca2106cd2a461c51ce2f4667d84d/packages/gas-fee-controller/src/GasFeeController.ts#L61-L66),
([example1](https://github.com/MetaMask/core/blob/dcc1d9291297ca2106cd2a461c51ce2f4667d84d/packages/user-operation-controller/src/utils/gas-fees.ts#L224),
[example2](https://github.com/MetaMask/core/blob/dcc1d9291297ca2106cd2a461c51ce2f4667d84d/packages/transaction-controller/src/gas-flows/DefaultGasFeeFlow.ts#L41-L62))
which hasn't be touched in [3
years](https://github.com/MetaMask/core/pull/494/files#diff-ac8f6d6d8ff039810f56d99da8735d4eb8c2978eed2685b1741c9124c7b8bb6fR47-R52).

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](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/25230?quickstart=1)

- Related: #25194

1. Go to this page...
2.
3.

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

<!-- [screenshots/recordings] -->

<!-- [screenshots/recordings] -->

- [ ] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [ ] I've completed the PR template to the best of my ability
- [ ] I’ve included tests if applicable
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

- [ ] 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.

---------

Co-authored-by: MetaMask Bot <metamaskbot@users.noreply.github.com>
dbrans added a commit to MetaMask/metamask-extension that referenced this pull request Jun 18, 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.

We need to move quickly on a hotfix for the Extension due to a caching
issue with the [infura.io](http://infura.io/) endpoint. The [API team
has
asked](https://consensys.slack.com/archives/C05B78N1T9B/p1717783170599949)
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:
- apply patch using core patch branch MetaMask/core/pull/4403
- revert #23717

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](https://consensys.slack.com/archives/CTQAGKY5V/p1718123930090259?thread_ts=1718123750.012709&cid=CTQAGKY5V).

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](https://github.com/MetaMask/core/blob/dcc1d9291297ca2106cd2a461c51ce2f4667d84d/packages/gas-fee-controller/src/GasFeeController.ts#L61-L66),
([example1](https://github.com/MetaMask/core/blob/dcc1d9291297ca2106cd2a461c51ce2f4667d84d/packages/user-operation-controller/src/utils/gas-fees.ts#L224),
[example2](https://github.com/MetaMask/core/blob/dcc1d9291297ca2106cd2a461c51ce2f4667d84d/packages/transaction-controller/src/gas-flows/DefaultGasFeeFlow.ts#L41-L62))
which hasn't be touched in [3
years](https://github.com/MetaMask/core/pull/494/files#diff-ac8f6d6d8ff039810f56d99da8735d4eb8c2978eed2685b1741c9124c7b8bb6fR47-R52).

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](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/25230?quickstart=1)

- Related: #25194

1. Go to this page...
2.
3.

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

<!-- [screenshots/recordings] -->

<!-- [screenshots/recordings] -->

- [ ] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [ ] I've completed the PR template to the best of my ability
- [ ] I’ve included tests if applicable
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

- [ ] 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.

---------

Co-authored-by: MetaMask Bot <metamaskbot@users.noreply.github.com>
tommasini pushed a commit to MetaMask/metamask-mobile that referenced this pull request Jun 20, 2024
## **Description**

1. There is a caching issue with some of the GasFeeController API URLs.
The decision has been made to patch it for the time being.

Mobile Core issue:
https://github.com/orgs/MetaMask/projects/60/views/6?pane=issue&itemId=67345155
Slack conversation:
https://consensys.slack.com/archives/C01V1L10W2E/p1718117049087139
Similar PR for Extension:
MetaMask/metamask-extension#25230
Core PR: MetaMask/core#4403
Core branch: `patch/extension-gas-api-endpoint`
## **Related issues**

Fixes:
https://github.com/orgs/MetaMask/projects/60/views/6?pane=issue&itemId=67345155

## **Manual testing steps**

1. Go to app and make sure gas fees are being calculated correctly.

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**


https://github.com/MetaMask/metamask-mobile/assets/6249205/bd45fcee-b64c-41c1-903e-1671c4a59392

### **After**


https://github.com/MetaMask/metamask-mobile/assets/6249205/0099be03-fb5a-465c-9734-37451b6f5019

## **Pre-merge author checklist**

- [X] I’ve followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile
Coding
Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [X] I've completed the PR template to the best of my ability
- [X] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant