Integrate PollingController mixin with GasFeeController - #1673

Merged

adonesky1 merged 7 commits into

mainfrom

networkclientid-polling

Sep 29, 2023

adonesky1 commented Sep 13, 2023•
edited
Loading

Contributor

Integrates recently introduced PollingController mixin with GasFeeController. Leaves old polling pattern in pace for now so as to not force a ton of breaking changes for mobile, but with intention to activate new pattern in both clients ASAP.

Addresses: https://github.com/MetaMask/MetaMask-planning/issues/1314

adonesky1 added the team-wallet-api-platform label

jiexi reviewed

View reviewed changes

packages/gas-fee-controller/src/GasFeeControllerPolling.ts Outdated

jiexi reviewed

View reviewed changes

packages/gas-fee-controller/src/GasFeeControllerPolling.ts Outdated

BelfordZ reviewed

View reviewed changes

packages/gas-fee-controller/src/GasFeeControllerPolling.ts Outdated

BelfordZ reviewed

View reviewed changes

packages/gas-fee-controller/src/GasFeeControllerPolling.ts Outdated

BelfordZ reviewed

View reviewed changes

packages/gas-fee-controller/src/GasFeeControllerPolling.ts Outdated

adonesky1 requested review from Gudahtt and mcmire

September 15, 2023 17:02

shanejonas force-pushed the networkclientid-polling branch from 900c3d5 to 9fa253aCompare

September 15, 2023 20:10

adonesky1 force-pushed the networkclientid-polling branch from e936134 to 1844af4Compare

September 18, 2023 16:29

adonesky1 requested review from BelfordZ and jiexi

September 18, 2023 16:29

adonesky1 marked this pull request as ready for review

September 18, 2023 16:46

adonesky1 requested a review from a team as a code owner

September 18, 2023 16:46

Gudahtt commented Sep 18, 2023

Member

Generally this looks good! Will give it a more detailed review later

adonesky1 force-pushed the networkclientid-polling branch 4 times, most recently from 0ef971b to 41ccfc8Compare

September 20, 2023 02:15

adonesky1 commented

View reviewed changes

packages/controller-utils/package.json Outdated

adonesky1 commented

View reviewed changes

packages/controller-utils/package.json Outdated

shanejonas force-pushed the networkclientid-polling branch from 290ac3b to 20199c3Compare

September 21, 2023 19:52

socket-securityBot commented Sep 21, 2023•
edited
Loading

👍 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.

Gudahtt commented Sep 21, 2023

Member

I suspect that CI will not let us use the new package in the gas fee controller package until we merge and release it separately

Gudahtt reviewed

View reviewed changes

Gudahtt left a comment

Member

Looks great! Maybe we can move the new package into a separate PR and get that merged and released to unblock the gas fee controller changes.

packages/polling-controller/src/PollingController.test.ts Outdated

packages/polling-controller/src/PollingController.ts Outdated

packages/polling-controller/src/PollingController.ts Outdated

packages/polling-controller/src/PollingController.ts Outdated

packages/polling-controller/src/PollingController.ts Outdated

shanejonas mentioned this pull request

Added polling-controller #1703

Merged

shanejonas added a commit that referenced this pull request

Added polling-controller (#1703)

9698b0a

## Explanation
> Originally was in #1673 but
pulled out to get this in on its own.
Adds an abstract class (currently named `PollingController`. The start
and stop methods are parameterized by `networkClientId`'s and
`pollingToken`'s and polling intervals are stored in class variables by
`chainId` so that multiple `networkClients`/`chainIds` can poll
simultaneously. `executePoll` is an abstract method to be implemented by
the controller itself so that this pattern could be generalized and be
agnostic to what is being executed on the polling interval.
## References
Related to #1673
Related to https://github.com/MetaMask/MetaMask-planning/issues/1314
---------
Co-authored-by: Alex Donesky <adonesky@gmail.com>

shanejonas force-pushed the networkclientid-polling branch from 7bb1699 to 9bd377bCompare

September 26, 2023 14:47

shanejonas reviewed

View reviewed changes

packages/gas-fee-controller/src/GasFeeController.ts Outdated

shanejonas reviewed

View reviewed changes

packages/gas-fee-controller/src/GasFeeController.ts Outdated

adonesky1 changed the title ~~Integrate PollingController Mixing with GasFeeController~~Integrate PollingController mixin with GasFeeController

adonesky1 requested review from jiexi and shanejonas

September 28, 2023 15:51

jiexi reviewed

View reviewed changes

packages/gas-fee-controller/src/GasFeeController.ts Outdated

jiexi previously approved these changes

View reviewed changes

shanejonas reviewed

View reviewed changes

packages/network-controller/src/NetworkController.ts Outdated

adonesky1 dismissed jiexi’s stale review via 9b53d8c

September 28, 2023 18:27

adonesky1 requested review from jiexi and shanejonas

September 28, 2023 18:28

adonesky1 added 5 commits

September 28, 2023 14:39

Integrate PollingController mixin with GasFeeController

a0d92ad

small adaptations to new mixin pattern

58f62a4

address feedback

f52a48b

address type name feedback

312415b

update start polling method name in test to reflect change on polling…

20e9ea3

…Controller mixin

adonesky1 force-pushed the networkclientid-polling branch from 283b09b to 20e9ea3Compare

September 28, 2023 19:50

shanejonas approved these changes

View reviewed changes

adonesky1 added 2 commits

September 29, 2023 10:08

Merge branch 'main' into networkclientid-polling

a695cd8

Merge branch 'main' into networkclientid-polling

2eae9bb

adonesky1 merged commit fd03dc6 into main

adonesky1 deleted the networkclientid-polling branch

September 29, 2023 15:30

legobeat mentioned this pull request

fix(gas-fee-controller): add missing @metamask/polling-controller #1748

Merged

3 tasks

legobeat commented Oct 2, 2023

Contributor

@adonesky1 @shanejonas:

fix(gas-fee-controller): add missing @metamask/polling-controller #1748

adonesky1 added a commit that referenced this pull request

fix(gas-fee-controller): add missing @metamask/polling-controller (#1748

94175cd

)
## Explanation
Adds missing dependency introduced in #1673
## References
- Follow-up to: #1673 ## 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
---------
Co-authored-by: Alex <adonesky@gmail.com>

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

team-wallet-api-platform