Skip to content

Add nonRPCGasFeeApisDisabled property to the gas fee controller#4094

Merged
danjm merged 3 commits intomainfrom
gas-fee-disable-nonRPC-apis
Apr 24, 2024
Merged

Add nonRPCGasFeeApisDisabled property to the gas fee controller#4094
danjm merged 3 commits intomainfrom
gas-fee-disable-nonRPC-apis

Conversation

@danjm
Copy link
Copy Markdown
Contributor

@danjm danjm commented Mar 20, 2024

Explanation

This PR allows the user of this controller to control whether it makes any network requests to extermal APIs other than the rpc provider. To do this it adds a nonRPCGasFeeApisDisabled property to the controller state. This property defaults to false. Methods to set that property to true and false are added

If that property is true, then determineGasFeeCalculations will not make requests to the legacyAPIEndpoint/fetchLegacyGasPriceEstimatesUrl nor the EIP1559APIEndpoint/fetchGasEstimatesUrl. It will, in this case, only get gas data via eth_feeHistory or eth_gasPrice requests to the rpc provider.

The goal is to allow the user of the controller to prevent network requests to additional parties, which may be desired to allow end users greater control over privacy.

References

This is part of resolving https://github.com/MetaMask/MetaMask-planning/issues/2076

Changelog

@metamask/gas-fee-controller

  • ADDED: Adds nonRPCGasFeeApisDisabled, property to the controller, allowing the user to prevent requests for gas fees to the external 'APIEndpoints'

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

@danjm danjm requested a review from a team as a code owner March 20, 2024 19:36
darkwing
darkwing previously approved these changes Mar 21, 2024
Copy link
Copy Markdown
Contributor

@darkwing darkwing left a comment

Choose a reason for hiding this comment

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

This looks great!

DDDDDanica
DDDDDanica previously approved these changes Mar 21, 2024
"peerDependencies": {
"@metamask/network-controller": "^18.0.0"
"@metamask/network-controller": "^18.0.0",
"@metamask/preferences-controller": "^9.0.0"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this dependency necessary? I'm not seeing it used below. Specifically adding a peer dependency would be a breaking change since clients would now have to use this version for compatibility. But if we don't need to have this then it doesn't need to be a breaking change.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

oh, yeah, that was included by mistake

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed in ffc64cc

@danjm danjm dismissed stale reviews from DDDDDanica and darkwing via ffc64cc March 21, 2024 17:49
mcmire
mcmire previously approved these changes Mar 21, 2024
Copy link
Copy Markdown
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.

Thanks for the reply! Looks good.

danjm added 2 commits April 10, 2024 16:09
…e user to prevent requests for gas fees to the external 'APIEndpoints'
@danjm danjm force-pushed the gas-fee-disable-nonRPC-apis branch from ffc64cc to 0a7c524 Compare April 10, 2024 18:40
@danjm
Copy link
Copy Markdown
Contributor Author

danjm commented Apr 10, 2024

I just pushed changes to resolve merge conflicts

Copy link
Copy Markdown
Contributor

@cryptodev-2s cryptodev-2s left a comment

Choose a reason for hiding this comment

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

LGTM!

@danjm danjm requested a review from a team April 24, 2024 13:06
@danjm danjm merged commit d78e50d into main Apr 24, 2024
@danjm danjm deleted the gas-fee-disable-nonRPC-apis branch April 24, 2024 13:51
danjm added a commit that referenced this pull request May 7, 2024
## Explanation

This should have been part of #4094

The absence of this metadata results in an error being thrown from here:
https://github.com/MetaMask/core/blob/main/packages/base-controller/src/BaseControllerV2.ts#L295-L297

## Changelog

FIXED: Add metadata for the `nonRPCGasFeeApisDisabled` of
gas-fee-controller state

### `@metamask/gas-fee-controller`

- **FIXED**: Add metadata for the `nonRPCGasFeeApisDisabled` of
gas-fee-controller state

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

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

6 participants