-
-
Notifications
You must be signed in to change notification settings - Fork 83
feat: Add useCache optional param to getLatestBlock()
#340
Changes from all commits
3da8732
f3ef6e6
22006e4
194e855
6453182
51b24f8
c1842b9
f59c095
20b6c10
83e8a15
9a516e8
ce63eb1
e23bc9c
30a2cfd
3d63b9d
8e41b21
a71ca3d
32a138e
2407ab3
e49d5bc
70c87f5
e3d13f1
17ae0e0
f233852
aa0f910
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -174,7 +174,7 @@ | |
|
|
||
| await withPollingBlockTracker(async ({ blockTracker }) => { | ||
| expect(blockTracker.isRunning()).toBe(false); | ||
| blockTracker.getLatestBlock(); | ||
|
Check warning on line 177 in src/PollingBlockTracker.test.ts
|
||
| expect(blockTracker.isRunning()).toBe(false); | ||
| }); | ||
| }); | ||
|
|
@@ -984,6 +984,171 @@ | |
| ); | ||
| }); | ||
| }); | ||
|
|
||
| describe('with useCache: false', () => { | ||
| describe('when the block tracker is not running', () => { | ||
| it('should not fetch a new block even if a block is already cached and less than the polling interval time has passed since the last call', async () => { | ||
| recordCallsToSetTimeout(); | ||
|
|
||
| await withPollingBlockTracker( | ||
| { | ||
| provider: { | ||
| stubs: [ | ||
| { | ||
| methodName: 'eth_blockNumber', | ||
| result: '0x1', | ||
| }, | ||
| { | ||
| methodName: 'eth_blockNumber', | ||
| result: '0x2', | ||
| }, | ||
| ], | ||
| }, | ||
| }, | ||
| async ({ blockTracker }) => { | ||
| await blockTracker.getLatestBlock(); | ||
| const block = await blockTracker.getLatestBlock({ | ||
| useCache: false, | ||
| }); | ||
| expect(block).toBe('0x1'); | ||
| expect(blockTracker.isRunning()).toBe(false); | ||
| }, | ||
| ); | ||
| }); | ||
|
|
||
| it('should fetch a new block even if a block is already cached and more than the polling interval time has passed since the last call', async () => { | ||
| recordCallsToSetTimeout({ | ||
| numAutomaticCalls: 1, | ||
| }); | ||
|
|
||
| await withPollingBlockTracker( | ||
| { | ||
| provider: { | ||
| stubs: [ | ||
| { | ||
| methodName: 'eth_blockNumber', | ||
| result: '0x1', | ||
| }, | ||
| { | ||
| methodName: 'eth_blockNumber', | ||
| result: '0x2', | ||
| }, | ||
| ], | ||
| }, | ||
| }, | ||
| async ({ blockTracker }) => { | ||
| await blockTracker.getLatestBlock(); | ||
| const block = await blockTracker.getLatestBlock({ | ||
| useCache: false, | ||
| }); | ||
| expect(block).toBe('0x2'); | ||
| expect(blockTracker.isRunning()).toBe(false); | ||
| }, | ||
| ); | ||
| }); | ||
| }); | ||
|
|
||
| describe('when the block tracker is already started', () => { | ||
| it('should wait for the next block event even if a block is already cached', async () => { | ||
| const setTimeoutRecorder = recordCallsToSetTimeout(); | ||
|
|
||
| await withPollingBlockTracker( | ||
| { | ||
| provider: { | ||
| stubs: [ | ||
| { | ||
| methodName: 'eth_blockNumber', | ||
| result: '0x1', | ||
| }, | ||
| { | ||
| methodName: 'eth_blockNumber', | ||
| result: '0x2', | ||
| }, | ||
| { | ||
| methodName: 'eth_blockNumber', | ||
| result: '0x3', | ||
| }, | ||
| ], | ||
| }, | ||
| }, | ||
|
|
||
| async ({ blockTracker }) => { | ||
| blockTracker.on('latest', EMPTY_FUNCTION); | ||
| await new Promise((resolve) => { | ||
| blockTracker.once('_waitingForNextIteration', resolve); | ||
| }); | ||
|
|
||
| const blockPromise1 = blockTracker.getLatestBlock({ | ||
| useCache: false, | ||
| }); | ||
| const pollingLoopPromise1 = new Promise((resolve) => { | ||
| blockTracker.once('_waitingForNextIteration', resolve); | ||
| }); | ||
| await setTimeoutRecorder.next(); | ||
| await pollingLoopPromise1; | ||
| const block1 = await blockPromise1; | ||
| expect(block1).toBe('0x2'); | ||
|
|
||
| const pollingLoopPromise2 = new Promise((resolve) => { | ||
| blockTracker.once('_waitingForNextIteration', resolve); | ||
| }); | ||
| const blockPromise2 = blockTracker.getLatestBlock({ | ||
| useCache: false, | ||
| }); | ||
| await setTimeoutRecorder.next(); | ||
| await pollingLoopPromise2; | ||
| const block2 = await blockPromise2; | ||
| expect(block2).toBe('0x3'); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm having trouble understanding how this test asserts the description. i.e. if the block tracker didn't wait for the next block, what is stopping this test from passing?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because of the
but if the getLatestBlock() call wasn't waiting for the next block, we'd have 2 more setTimeouts we'd need to await for, which would cause the test to fail since it would need to allow 3 setTimeouts to pass.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. actually, I shouldn't need to call
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. cleaned up here 17ae0e0
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. added explicit
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I was thinking of the opposite case. i.e. what if it incorrectly fetches immediately instead of waiting for the next loop. This test would still pass despite that behavior contradicting the description
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually no, the |
||
| }, | ||
| ); | ||
| }); | ||
|
|
||
| it('should handle concurrent calls', async () => { | ||
| const setTimeoutRecorder = recordCallsToSetTimeout(); | ||
|
|
||
| await withPollingBlockTracker( | ||
| { | ||
| provider: { | ||
| stubs: [ | ||
| { | ||
| methodName: 'eth_blockNumber', | ||
| result: '0x1', | ||
| }, | ||
| { | ||
| methodName: 'eth_blockNumber', | ||
| result: '0x2', | ||
| }, | ||
| ], | ||
| }, | ||
| }, | ||
| async ({ blockTracker }) => { | ||
| blockTracker.on('latest', EMPTY_FUNCTION); | ||
| await new Promise((resolve) => { | ||
| blockTracker.once('_waitingForNextIteration', resolve); | ||
| }); | ||
|
|
||
| const blockPromise1 = blockTracker.getLatestBlock({ | ||
| useCache: false, | ||
| }); | ||
| const blockPromise2 = blockTracker.getLatestBlock({ | ||
| useCache: false, | ||
| }); | ||
|
|
||
| const pollingLoopPromise = new Promise((resolve) => { | ||
| blockTracker.once('_waitingForNextIteration', resolve); | ||
| }); | ||
| await setTimeoutRecorder.next(); | ||
| await pollingLoopPromise; | ||
|
|
||
| const block1 = await blockPromise1; | ||
| const block2 = await blockPromise2; | ||
| expect(block1).toBe('0x2'); | ||
| expect(block2).toBe('0x2'); | ||
| }, | ||
| ); | ||
| }); | ||
| }); | ||
| }); | ||
| }); | ||
|
|
||
| describe('checkForLatestBlock', () => { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -111,9 +111,11 @@ export class PollingBlockTracker | |
| return this._currentBlock; | ||
| } | ||
|
|
||
| async getLatestBlock(): Promise<string> { | ||
| async getLatestBlock({ | ||
| useCache = true, | ||
| }: { useCache?: boolean } = {}): Promise<string> { | ||
| // return if available | ||
| if (this._currentBlock) { | ||
| if (this._currentBlock && useCache) { | ||
| return this._currentBlock; | ||
| } | ||
|
|
||
|
|
@@ -126,31 +128,42 @@ export class PollingBlockTracker | |
| }); | ||
| this.#pendingLatestBlock = { reject, promise }; | ||
|
|
||
| try { | ||
| if (this._isRunning) { | ||
cryptodev-2s marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| try { | ||
| // If tracker is running, wait for next block with timeout | ||
| const onLatestBlock = (value: string) => { | ||
| this.#removeInternalListener(onLatestBlock); | ||
| this.removeListener('latest', onLatestBlock); | ||
| resolve(value); | ||
| }; | ||
|
|
||
| this.#addInternalListener(onLatestBlock); | ||
| this.once('latest', onLatestBlock); | ||
|
|
||
| return await promise; | ||
| } catch (error) { | ||
| reject(error); | ||
| throw error; | ||
| } finally { | ||
| this.#pendingLatestBlock = undefined; | ||
| } | ||
| } else { | ||
| // If tracker isn't running, just fetch directly | ||
| if (!this._isRunning) { | ||
| const latestBlock = await this._fetchLatestBlock(); | ||
| this._newPotentialLatest(latestBlock); | ||
| try { | ||
| const latestBlock = await this._updateLatestBlock(); | ||
jiexi marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: Stale Block Data in Latest Block RetrievalThe Additional Locations (1) |
||
| resolve(latestBlock); | ||
| return latestBlock; | ||
| } catch (error) { | ||
| reject(error); | ||
| throw error; | ||
| } finally { | ||
| // We want to rate limit calls to this method if we made a direct fetch | ||
| // for the block number because the BlockTracker was not running. We | ||
| // achieve this by delaying the unsetting of the #pendingLatestBlock promise. | ||
| setTimeout(() => { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, this is how we are ensuring we aren't making a new request when calling |
||
| this.#pendingLatestBlock = undefined; | ||
| }, this._pollingInterval); | ||
| } | ||
|
|
||
| // If tracker is running, wait for next block with timeout | ||
| const onLatestBlock = (value: string) => { | ||
| this.#removeInternalListener(onLatestBlock); | ||
| this.removeListener('latest', onLatestBlock); | ||
| resolve(value); | ||
| }; | ||
|
|
||
| this.#addInternalListener(onLatestBlock); | ||
| this.once('latest', onLatestBlock); | ||
|
|
||
| return await promise; | ||
| } catch (error) { | ||
| reject(error); | ||
| throw error; | ||
| } finally { | ||
| this.#pendingLatestBlock = undefined; | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -287,6 +300,13 @@ export class PollingBlockTracker | |
| this._currentBlock = null; | ||
| } | ||
|
|
||
| /** | ||
| * Checks for the latest block, updates the internal state, and returns the | ||
| * value immediately rather than waiting for the next polling interval. | ||
| * | ||
| * @deprecated Use {@link getLatestBlock} instead. | ||
| * @returns A promise that resolves to the latest block number. | ||
| */ | ||
| async checkForLatestBlock() { | ||
| await this._updateLatestBlock(); | ||
| return await this.getLatestBlock(); | ||
|
|
@@ -302,10 +322,13 @@ export class PollingBlockTracker | |
| this._clearPollingTimeout(); | ||
| } | ||
|
|
||
| private async _updateLatestBlock(): Promise<void> { | ||
| private async _updateLatestBlock(): Promise<string> { | ||
| // fetch + set latest block | ||
| const latestBlock = await this._fetchLatestBlock(); | ||
| this._newPotentialLatest(latestBlock); | ||
| // _newPotentialLatest() ensures that this._currentBlock is not null | ||
| // eslint-disable-next-line @typescript-eslint/no-non-null-assertion | ||
| return this._currentBlock!; | ||
jiexi marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| private async _fetchLatestBlock(): Promise<string> { | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.