Skip to content
This repository has been archived by the owner on Jan 11, 2024. It is now read-only.

FIX: Add an extra 2 rounds of delay before proposing a topdown finality #435

Closed
wants to merge 2 commits into from

Conversation

aakoshh
Copy link
Contributor

@aakoshh aakoshh commented Nov 15, 2023

Adds an FM_IPC__TOPDOWN__PROPOSAL_DELAY env var with a value of 2 set by the docker container scripts as an example (I think this doesn't appear in default.toml), with the effect that blocks are proposed when they are FM_IPC__TOPDOWN__CHAIN_HEAD_DELAY + FM_IPC__TOPDOWN__PROPOSAL_DELAY deep, but accepted if they are FM_IPC__TOPDOWN__CHAIN_HEAD_DELAY deep, so there is an extra minute for parents to sync with each other, which might help avoid lost rounds in the subnet.

@cryptoAtwill
Copy link
Contributor

@aakoshh Let's say if one of the validators, say A, died and the network comes to a halt, but other validators are still syncing with parent. Then that validator comes back and it starts syncing again but have not triggered the cometbft syncing. Assuming the validators have more or less the same syncing speed.

A is lagging others by around 100 blocks. If other validators keep on making proposals at latest height seen minus FM_IPC__TOPDOWN__CHAIN_HEAD_DELAY, it still won't reach consensus though.

Just an idea, why not we try proposing at a height that is half way between last_committed_height to latest_height, i.e. last_committed_height + (latest_height - last_committed_height) / 2.

@aakoshh
Copy link
Contributor Author

aakoshh commented Nov 17, 2023

@cryptoAtwill I'm trying to think through your question.

I'd like to assume that for the scenario we are discussing we will have already solved consensus-shipyard/ipc#157

If we did, then this wouldn't be entirely accurate:

If other validators keep on making proposals at latest height seen minus FM_IPC__TOPDOWN__CHAIN_HEAD_DELAY

because they should be making proposals at latest finalized block + 100.

A is lagging others by around 100 blocks.

With that fix, if A starts syncing at all, it should be able to catch up, because none of the others would venture further ahead.

If I assumed that this fix wasn't in, and A was constantly trying to catch up and constantly being behind because the others keep adding another and another 100 blocks on top, then surely at some point they will run out of new blocks to add? But I already said I don't think this constant piling up of blocks is desirable.

@aakoshh aakoshh force-pushed the delay-finality-proposal branch from 1016d48 to c6a0738 Compare November 20, 2023 09:29
@adlrocha
Copy link
Contributor

The implementation of this PR has been included as part of #447

@adlrocha adlrocha closed this Nov 29, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants