Make LogPoller more robust against local finality violations

[reductionista]

The main focus of this PR is to help LogPoller detect, prevent "local finality violations". These are cases where the multinode layer fails over from one rpc several to another and the second one is behind on its view of the chain. This can make it look like there is a finality violation even when there was not any global finality violation on the chain.

Primary change:

• Checks that all results from batchFetchBlocks() are finalized aside from "latest"
  batchFetchBlocks() will now fetch the "finalized" block along with the rest of each batch,
  and validate that all of the block numbers (aside from the special case when "latest" is requested)
  are <= the finalized block number returned.

Also in this PR are two refactors for reducing code complexity:

• Change backfill() to always save the last block of each batch of logs requested, rather than the last block of the logs returned.
  (This only makes a difference if the last block requested has no logs matching the filter, but this change will allow us to eliminate
  lastSafeBlockNumber = latestFinalizedBlock - 1 in an upcoming PR in favor of latestFinalizedBlock which simplifies the overall LogPoller
  implementation. It also gets us a step closer to being able to use "finalized" for the upper range of the final batch request for logs, but that
  comes with some additional complexities which still need to be worked out.)

• Start Backup LogPoller on lastProcessed.FinalizedBlockNumber instead of lastProcessed.FinalizedBlockNumber - 1. This was a harmless "bug" where it was starting one block too early, but entirely separate from the previous change.

• Minor refactor to remove code duplication (condensing 3 replicated code blocks calling getCurrentBlockMaybeHandleReorg down to 2)

[github-actions]

I see you updated files related to core. Please run pnpm changeset in the root directory to add a changeset as well as in the text include at least one of the following tags:

#nops : For any feature that is NOP facing and needs to be in the official Release Notes for the release.
#added : For any new functionality added.
#changed : For any change to the existing functionality. 
#removed : For any functionality/config that is removed.
#updated : For any functionality that is updated.
#deprecation_notice : For any upcoming deprecation functionality.
#breaking_change : For any functionality that requires manual action for the node to boot.
#db_update : For any feature that introduces updates to database schema.
#wip : For any change that is not ready yet and external communication about it should be held off till it is feature complete.
#bugfix - For bug fixes.
#internal - For changesets that need to be excluded from the final changelog.

[github-actions]

I see you added a changeset file but it does not contain a tag. Please edit the text include at least one of the following tags:

#nops : For any feature that is NOP facing and needs to be in the official Release Notes for the release.
#added : For any new functionality added.
#changed : For any change to the existing functionality. 
#removed : For any functionality/config that is removed.
#updated : For any functionality that is updated.
#deprecation_notice : For any upcoming deprecation functionality.
#breaking_change : For any functionality that requires manual action for the node to boot.
#db_update : For any feature that introduces updates to database schema.
#wip : For any change that is not ready yet and external communication about it should be held off till it is feature complete.
#bugfix - For bug fixes.
#internal - For changesets that need to be excluded from the final changelog.

[mateusz-sekara]

I'm not sure if I follow. How does validating whether blocks are finalized fix the issues with local finality violations? I think I miss the bigger picture here

[reductionista]

I'm not sure if I follow. How does validating whether blocks are finalized fix the issues with local finality violations? I think I miss the bigger picture here

Sorry, I should have provided more explanation on that in the PR description. The flow of PollAndSaveLogs() includes this sequence of calls:

1. latestBlock, latestFinalizedBlockNumber, err := lp.latestBlocks(lp.ctx)
2. currentBlock, err = lp.getCurrentBlockMaybeHandleReorg(ctx, currentBlockNumber, currentBlock)
3. lastSafeBackfillBlock := latestFinalizedBlockNumber - 1
4. lp.backfill(ctx, currentBlockNumber, lastSafeBackfillBlock)
   4a. gethLogs, err := lp.ec.FilterLogs(ctx, lp.Filter(big.NewInt(from), big.NewInt(to), nil))
   4b. blocks, err := lp.blocksFromLogs(ctx, gethLogs)
   4c. lp.orm.InsertLogsWithBlock(ctx, convertLogs(gethLogs, blocks, lp.lggr, lp.ec.ConfiguredChainID()), blocks[len(blocks)-1])

(omitting some inconsequential steps, these are the main ones)

The scenario where a local finality violation can cause problems is when the primary rpc server returns latestFinalizedBlockNumber in step 1, then there is a failover to an out-of-sync rpc server somewhere between steps 1 and 4b. In step 4b, blocksFromLogs() calls GetBlocksRange() to fetch all of the blocks associated with the list of logs passed to it in parallel. If it's connected to the out-of-sync rpc server at this point, then some or all of the blocks it gets back may be unfinalized blocks... they may not even be part of the canonical chain. If the last block get saved to the db as a finalized block in step 4c but it's actually not finalized, then we're in a situation where the db is corrupt and this can easily cause the node to get stuck. For example, if the primary rpc server comes back up, and now it's receiving blocks from the canonical chain, it may not be able to verify that those have any connection to the blocks in the db which it trusts because it thinks they are all "finalized".

The bottom line is, we only ever save finalized blocks to the db... the rpc failover scenario opened a loophole where we could save an unfinalized block for a block number we were previously told was finalized. This mostly closes that loophole. (The only potential edge case is if the requests sent in a single batch get executed out of order, and one or more of the blocks we're requesting gets re-org'd out and then finalized while the batch in being processed. That seems very unlikely, but we have another fix coming soon at the lower (multinode layer) level that should cover that scenario too.)

I was originally thinking mostly about fixing things in step 4a, where it requests the logs. If the failover happens between steps 1 and 4a then it can get back unfinalized logs, which is also a problem. But this fix should take care of both that situation and one where it fails over between steps 4a and 4b, because if blocksFromLogs returns an error then it won't proceed to step 4c so neither the unfinalized logs nor the unfinalized block will get saved to the db. It will just retry everything again next time around.

[cl-sonarqube-production]

Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
98.4% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube

[mateusz-sekara]

👍