Chainindex passthrough when behind backing validator

[AloeareV]

Fixes: #769

This PR adds a validator_finalized_height field to a snapshot. If this height is higher than the snapshot's tip, most APIs will pass requests through to the backing validator.

[idky137]

Hey I've been mulling over the problem with testing this functionality and came up with a possible solution (you may have already fixed this but thought I would add this in case it has not been possible to implement in the "MockChainSource".

I think one way we could implement the test only "Stop finalised state syncing at block X" is to add a "test" feature flagged field ("pause_sync_at_height?) to the "BlockCacheConfig" struct (or we could define a "TestVariables" struct to be added as this new field?):

zaino/zaino-state/src/config.rs

Line 136 in 31d0d85

pub struct BlockCacheConfig {

Then under the same "test" feature flag stop writing to the finalised state at the given block height in "start_sync_loop":

zaino/zaino-state/src/chain_index.rs

Line 520 in 31d0d85

fs.write_block(next_finalized_block.clone())

It may still be cleaner to implement this in the "MockChainSource" if possible but that could be more intrusive than just adding here.

[AloeareV]

Hey I've been mulling over the problem with testing this functionality and came up with a possible solution (you may have already fixed this but thought I would add this in case it has not been possible to implement in the "MockChainSource".

I think one way we could implement the test only "Stop finalised state syncing at block X" is to add a "test" feature flagged field ("pause_sync_at_height?) to the "BlockCacheConfig" struct (or we could define a "TestVariables" struct to be added as this new field?):

zaino/zaino-state/src/config.rs

Line 136 in 31d0d85

pub struct BlockCacheConfig {

Then under the same "test" feature flag stop writing to the finalised state at the given block height in "start_sync_loop":

zaino/zaino-state/src/chain_index.rs

Line 520 in 31d0d85

fs.write_block(next_finalized_block.clone())

It may still be cleaner to implement this in the "MockChainSource" if possible but that could be more intrusive than just adding here.

I just thought of something, which I'm gonna try out...if we add a customizable delay to MockchainSource methods, we could do some experimentation with simulated network latency...and if we make the delay long enough, it could slow sync down to a crawl, and still allow passthroughs to go through (as a passthrough requires one or maybe two calls to the source, a one-second delay would mean that the passthrough takes a couple seconds, and syncing a block would also take seconds, so if we make the chain a few hundred blocks we won't catch up to the finalized boundary before the request comes in.)

[AloeareV]

An open question: When a passthrough request is made, and the needed information is in the nonfinalized state, is this:

1. An Ok(None) return, as if we and the validator were synced to the top of the finalized range
2. An Error return, indicating that the information requested exists but isn't synced yet
3. Something else?

If 2, do we just return this error whenever it's convenient (i.e., when we don't need to additional work to distinguish between the requested information being nonfinalized or nonexistent), or do we make the guarantee of correctly identifying this error in all cases?
Also, if 2, if the request covers multiple blocks and some of them are finalized, do we consider this a success case and return everything finalized, or an error case as the request covered information in the nonfinalized state that we can't safely serve until we've caught up to it? Possibly even vary by specific case/method?

[AloeareV]

Still to be done:

• Make sure passthrough only triggers when the validator's finalized tip is ahead of ChainIndexes best (nonfinalized) tip.
• Some error handling
• Fix passthrough triggering before local lookup in some RPCs

[fluidvanadium]

Why do self.get_fullblock_bytes_from_node and self.blockchain_source.get_block look so different? shouldnt they be sister functions?

[AloeareV]

Why do self.get_fullblock_bytes_from_node and self.blockchain_source.get_block look so different? shouldnt they be sister functions?

How do you mean 'so different'? They're not sisters, one calls the other. get_fullblock_bytes_from_node calls get_block and then serializes it to a vec.

(Originally, I somehow edited your message with my answer. I've turned it into a reply now)