search

MetaMask / eth-block-tracker

A JS module for keeping track of the latest Ethereum block by polling an ethereum provider
MIT License
131 stars 81 forks source link

Fix getLatestBlock to stop block tracker afterward #166

Closed mcmire closed 1 year ago

mcmire commented 1 year ago

getLatestBlock returns a promise that resolves to the latest block number. How it obtains the latest block number depends on whether the block tracker is already running and which type of block tracker is being accessed. If the block tracker is already started, then getLatestBlock will simply wait until the next iteration of the loop (in the case of PollingBlockTracker) or the next block to arrive (in the case of SubscribeBlockTracker). Otherwise, it needs to start the block tracker automatically, wait, and then stop it automatically after it has obtained the block number.

However, a problem lies with the second possibility. If there is a problem retrieving the block number — if the request fails in the case of PollingBlockTracker or if an error is thrown while waiting for the next block in the case of SubscribeBlockTracker — then the block tracker will never get stopped. This occurs even when the network is switched.

How do we fix this? Part of the issue is that the latest event is "magic". The block tracker does not run automatically but runs only when it's needed. As soon as anything listens to latest, the block tracker will start; and if nothing is listening to latest, the block tracker will stop. This applies even if the thing that's listening to latest is the block tracker itself.

So the main thing that we change about getLatestBlock is to create an internal event that is not magic, and we handle starting and stopping ourselves. The other thing is that we watch for errors to occur. Putting that together, we can ensure that whether or not there is an error, we stop the block tracker if it needs to be stopped once we're done.

Fixes #163.

Gudahtt commented 1 year ago

It looks like getLatestBlock will no longer work if the block tracker is stopped. That doesn't seem good. Today when we call getLatestBlock we assume we'll get a response.

mcmire commented 1 year ago

It looks like getLatestBlock will no longer work if the block tracker is stopped.

Sorry, can you clarify? The tests confirms that if the block tracker is already stopped, it will start itself.

Gudahtt commented 1 year ago

The tests confirms that if the block tracker is already stopped, it will start itself.

Ah OK, I think I just misunderstood how it worked. I'll take another look.

mcmire commented 1 year ago

I'm closing this since @Gudahtt and I had a discussion and we feel like we can solve this a different way that doesn't involve listening to latest in any form. Also, while we definitely shouldn't be making more requests to Infura than necessary, this bug is lower in priority than our other tickets.