Fix bug that allowed for multiple fetch requests instantiate from getGasFeeEstimatesAndStartPolling #586
Conversation
src/gas/GasFeeController.test.ts
Outdated
| @@ -74,6 +76,9 @@ describe('GasFeeController', () => { | |||
| estimatedBaseFee: '28', | |||
| }) | |||
| .persist(); | |||
| mockGasFeeRequest.on('request', () => { | |||
There was a problem hiding this comment.
This was the best I could come up with for tracking number of requests. I look through nock documentation fairly extensively, tried a few other things, but this meets my exact need. Open to suggested alternative if there is a simpler way based on the nock api.
There was a problem hiding this comment.
I looked this up and it seems event is the way to go, however (nit) I'd change the event handler to a jest mock and use expect(handler).toHaveBeenCalledTimes instead of a counter
src/gas/GasFeeController.test.ts
Outdated
| expect(result2).toStrictEqual(pollToken); | ||
| }); | ||
|
|
||
| it('should fetch new estimates if the poll tokens are cleared, and then should not make additional new fetches', async () => { |
There was a problem hiding this comment.
I'm a bit confused on this one... I see it that it shouldn't make additional fetches on getGasFeeEstimatesAndStartPolling calls (until pollTokens are cleared again), but won't it make additional fetches on the polling interval?
There was a problem hiding this comment.
I guess this comment can be clarified. It means to say that subsequent calls to getGasFeeEstimatesAndStartPolling should not trigger new fetches
There was a problem hiding this comment.
@adonesky1 updated the comment in the latest commit
… while awaiting for a previous getGasFeeEstimatesAndStartPolling call
danjm
force-pushed
the
safer-gas-controller-polling
branch
from
eee033f
to
5595d22
Compare
…GasFeeEstimatesAndStartPolling (#586) * Ensure that the gas fee controller does not make new network requests while awaiting for a previous getGasFeeEstimatesAndStartPolling call * Use a jest function for handling events from nock calls in GasFeeController.test.ts * Improve unit test description'
…GasFeeEstimatesAndStartPolling (#586) * Ensure that the gas fee controller does not make new network requests while awaiting for a previous getGasFeeEstimatesAndStartPolling call * Use a jest function for handling events from nock calls in GasFeeController.test.ts * Improve unit test description'
There was a bug in
getGasFeeEstimatesAndStartPollingthat could result in multiple fetch requests to the gas api being made, despite thepollTokensrelated logic meant to prevent this.This PR fixes this by ensuring that the
pollTokensare set before the asyncronous call. Tests are added that fail without this change and succeed with it.