latest fork watch feature

[nachog00]

Implements #263

• Added 'latest_fork_sender' in 'NonFinalisedState'
• initialised latest fork channel in BlockCache - included latest fork reciever in NonFinalisedState
• Added error handling in the latest fork sending
• New method 'get_latest_fork' for 'NonFinalisedStateSubscriber'

[idky137]

I think we could create the channel inside the spawn method to simplify the inputs.

eg. move:

let (channel_fork_tx, channel_fork_rx) = tokio::sync::watch::channel(BlockId {
            height: 0,
            hash: vec![0, 0],
        });

inside NonFinalisedState::spawn().

[nachog00]

btw, what do you think the correct default values should be? i went with 0 for no reason in particular. Maybe we want to make it Option<BlockId> and only set it if we already have actual values?

[AloeareV]

I'm adding this comment before I've finished reading the code in question so this may be obviously impossible, but can we do some sort of lazy initialization so that we don't start the channel until we have the first value?

[AloeareV]

Oh...except in theory we might never have one. I think an optional value makes the most sense, returning None if there are no known chain forks in the last 100 blocks (nonfinalized state)

[idky137]

Ye using an option should work well for lazy initialisation.

I think that technically when a new block is mined there is a chain fork at [chain tip - 1] so once we are running this should always return a chain-fork BlockId which was why I originally thought we didn't need the option. (maybe this is wrong though?)

[AloeareV]

In the case where the latest chain fork is >100 blocks old (and therefore no longer part of the nonfinalized state), should it still be reported, or should we report None in that case?

[idky137]

I think we still want to report it, it may actually be more important that we do for deep reorgs.

The reorg logic here can actually fix reorgs in the finalised state as well as the non- finalised state.

If the reorg height goes below [chain tip - 100 blocks] the non-finalised state will repopulate from that height, and as any blocks leave the non-finalised state they are pushed to the finalised state. Then the finalised state deals with any duplicate block heights.

EDIT: I've been thinking about this more and I'm not 100% what happens if the chain fork is deeper than 100 blocks. There is a chance that the non-finalised state will grow to [chain fork depth] in size. If this is the case we will need to add a prune method that flashes all blocks with a depth greater than 100 to the finalised state. I will look into this a bit more.

[AloeareV]

I think we still want to report it, it may actually be more important that we do for deep reorgs.

The reorg logic here can actually fix reorgs in the finalised state as well as the non- finalised state.

If the reorg height goes below [chain tip - 100 blocks] the non-finalised state will repopulate from that height, and as any blocks leave the non-finalised state they are pushed to the finalised state. Then the finalised state deals with any duplicate block heights.

EDIT: I've been thinking about this more and I'm not 100% what happens if the chain fork is deeper than 100 blocks. There is a chance that the non-finalised state will grow to [chain fork depth] in size. If this is the case we will need to add a prune method that flashes all blocks with a depth greater than 100 to the finalised state. I will look into this a bit more.

My understanding is that finalized state is intended to be 'trusted' as old enough that it won't be reorged away. I haven't fully thought through how that interacts with latest fork reporting

[zancas]

@nachog00 what's the state of this PR?

[nachog00]

This became stale after API redesign. Closing.