Fix(store-sync): Handle unsynced RPC nodes #2901
Conversation
|
| const latestBlock$ = createBlockStream({ publicClient, blockTag: followBlockTag }).pipe(shareReplay(1)); | ||
| const latestBlockNumber$ = latestBlock$.pipe( | ||
| map((block) => block.number), | ||
| tap((blockNumber) => { | ||
| debug("on block number", blockNumber, "for", followBlockTag, "block tag"); | ||
| }), | ||
| filter((blockNumber) => { | ||
| return lastBlockNumberProcessed == null || blockNumber > lastBlockNumberProcessed; |
There was a problem hiding this comment.
unsure if we want to add a debug log here
| @@ -90,7 +90,18 @@ export async function* fetchLogs<TAbiEvents extends readonly AbiEvent[]>({ | |||
| try { | |||
| const toBlock = fromBlock + blockRange; | |||
| debug("getting logs", { fromBlock, toBlock }); | |||
| const logs = await publicClient.getLogs({ ...getLogsOpts, fromBlock, toBlock, strict: true }); | |||
|
|
|||
| const [latestBlockNumber, logs] = await Promise.all([ | |||
There was a problem hiding this comment.
This only works if batch is enabled in the HTTP transport. This is decided by clients though, so maybe this is put into docs?
There was a problem hiding this comment.
I wonder if we should have an option to enable the "validate unsynchronized nodes" behavior, where we require batch to be enabled or, ideally, do the batch call here directly rather than relying on some client configuration.
Since this approach isn't useful unless the call is atomic (going to the same RPC) and I suspect not all RPC providers support batching, seems like we should put this behind a flag to avoid doubling RPC calls for syncing data from RPC. Could make this the default approach with a way to opt out of this new behavior.
| publicClient.getLogs({ ...getLogsOpts, fromBlock, toBlock, strict: true }), | ||
| ]); | ||
| if (latestBlockNumber < toBlock) { | ||
| const blockTimeInSeconds = 2; |
There was a problem hiding this comment.
Not sure what the best way to handle this is, ie should it be a deploy config? I dont think there's a way to detect from the chosen chain what the block time is?
There was a problem hiding this comment.
we could use the client's pollingInterval here?
either way, we should probably align this with the other retry logic below
Context: https://indexsupply.com/shovel/docs/#unsynchronized-ethereum-nodes
3rd party Ethereum API providers may load balance RPC requests across a set of unsynchronized nodes. In rare cases, the eth_getLogs request will be routed to a node that doesn’t have the latest block.Solution: Batch fetch the latest block number with the logs, and ensure that the latest block number is past the
toBlock. Otherwise, wait for the RPC to catch up to the latest block.For the sync stack, the
fromBlockis decided from a local variable and thetoBlockis decided by a subscription to the latest block number. So we need an additional filter from the latest block number subscription to not emit when thetoBlockis behind thefromBlock, which happens with unsynchronized RPCs.toBlockis less thanfromBlock, no error is thrown, and no logs are fetched either, as this while loop will resolve false. So in this PR, we add a filter to not emit incorrecttoBlock's and also a safety error check to throw if this case occurs.