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

Update Gas API to use MetaMask Gas API #25194

Closed
desi opened this issue Jun 11, 2024 · 2 comments
Closed

Update Gas API to use MetaMask Gas API #25194

desi opened this issue Jun 11, 2024 · 2 comments
Labels
INVALID-ISSUE-TEMPLATE Issue's body doesn't match any issue template. team-transactions Transactions team

Comments

@desi
Copy link
Contributor

desi commented Jun 11, 2024 •
edited by dbrans
Loading

What is this about?

We recently made a change to use a new Infura Gas API but we would like to go back to using the previous MetaMask Gas API which some additional changes are made to the Infura one. We should use gas.api.cx.metamask.io rather than gas.api.infura.io
@desi desi added the team-transactions Transactions team label Jun 11, 2024
@dbrans
Copy link
Contributor

dbrans commented Jun 11, 2024 •
edited
Loading

For context, here are the relevant changes:

  • Core gas-fee-controller PR #4068 – Released in gas-fee-controller@15.0.0
  • Extension PR #23717 – Extension is on gas-fee-controller@15.1.2 (both develop and production), hence the need for a hotfix.
  • Mobile PR #9693 – Mobile is on gas-fee-controller@13 in v7.24.0, so no urgency here. However, main is on gas-fee-controller@15.1.2.

Here's the plan for the hotfix:

  1. Create a single extension PR to patch the gas-fee-controller and revert the changes. Merge the PR into develop.
  2. Merge the PR into develop, cherry-pick it into 11.16.NEXT (the hotfix), and the Extension v12.0.0 RC.

After the hotfix, we'll need to regroup:

  1. Revert fix: use hardcoded Infura gas API urls core#4068 in core on top of the latest gas-fee-controller (v17)
  2. Upgrade gas-fee-controller in Extension and Mobile

@metamaskbot metamaskbot added the INVALID-ISSUE-TEMPLATE Issue's body doesn't match any issue template. label Jun 11, 2024
@dbrans dbrans mentioned this issue Jun 11, 2024
Merged
7 tasks
dbrans added a commit that referenced this issue Jun 13, 2024
@dbrans @metamaskbot
chore: revert recent gas api endpoint changes (#25230)
04642d2 
> [!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 that referenced this issue Jun 13, 2024
@dbrans @metamaskbot
chore: revert recent gas api endpoint changes (#25230)
46057c3 
> [!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 that referenced this issue Jun 18, 2024
@dbrans @metamaskbot
chore: revert recent gas api endpoint changes (#25230)
d6f65ef 
> [!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 dbrans mentioned this issue Jun 21, 2024
Merged
3 tasks
@desi
Copy link
Contributor Author

desi commented Jul 23, 2024

All PR's are merged for this.

@dbrans dbrans closed this as completed Jul 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
INVALID-ISSUE-TEMPLATE Issue's body doesn't match any issue template. team-transactions Transactions team
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@desi @dbrans @metamaskbot