Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Leader blocksync #1322

Merged
merged 2 commits into from
Feb 3, 2025
Merged

Leader blocksync #1322

merged 2 commits into from
Feb 3, 2025

Conversation

charithabandi
Copy link
Contributor

@charithabandi charithabandi commented Jan 29, 2025

Two mechanisms are used to prevent the leader from committing a block at an already finalized height:

  1. Manual checkpoints: The leader can be configured with a checkpoint height, which is the
    height the leader must sync to before proposing blocks. The node operator should set the
    checkpoint height based on the network's best height.
  2. Validator feedback: If the leader proposes a block at an already finalized height, validators
    will respond with a NACK, indicating the leader is out-of-sync. They can provide an OutOfSyncProof
    as evidence, including the BlockHeader and the leader's signature of the block the validator is on.
    This forces the leader to catch up to the validator's block height before proposing blocks again. This
    corrective mechanism allows leader to eventually converge to the best height.

However the nodes can still end up in these weird race scenarios where majority of Validators are in-sync with the leader, but not with the Validator with BestHeight, and the Ack's from the out of sync validators might end up reaching the leader faster than the OutOfSync NACK, eventually making the leader to commit the block. But the situation is not unrecoverable, leader and these Validators can always be wiped out and restarted to the correct state.

@charithabandi charithabandi marked this pull request as draft January 29, 2025 22:15
@charithabandi charithabandi linked an issue Jan 29, 2025 that may be closed by this pull request
@charithabandi charithabandi force-pushed the blocksync branch 4 times, most recently from 0f8a485 to 4956a9a Compare January 30, 2025 23:34
@charithabandi charithabandi marked this pull request as ready for review January 30, 2025 23:34
@charithabandi charithabandi changed the title POC: DO NOT MERGE: leader blocksync Leader blocksync Jan 30, 2025
@charithabandi charithabandi force-pushed the blocksync branch 2 times, most recently from 4a7b93b to 73d7cf5 Compare January 31, 2025 19:41
@jchappelow
Copy link
Member

When this merges we need a descriptive commit message, which actually would be good for the PR description too. ;)

node/consensus.go Outdated Show resolved Hide resolved
node/consensus/blocksync.go Show resolved Hide resolved
node/consensus/blocksync.go Outdated Show resolved Hide resolved
node/consensus/blocksync.go Outdated Show resolved Hide resolved
node/consensus/follower.go Outdated Show resolved Hide resolved
node/consensus/follower.go Show resolved Hide resolved
node/consensus/leader.go Outdated Show resolved Hide resolved
node/types/commit.go Outdated Show resolved Hide resolved
node/consensus/messages.go Outdated Show resolved Hide resolved
node/types/commit.go Outdated Show resolved Hide resolved
node/types/consensus.go Outdated Show resolved Hide resolved
@charithabandi charithabandi force-pushed the blocksync branch 3 times, most recently from 06294ea to bd9c175 Compare February 3, 2025 22:23
@jchappelow
Copy link
Member

LGTM, but might be a little last minute for a beta.2. What's your feeling about it @charithabandi ?

@charithabandi
Copy link
Contributor Author

I am fairly confident, ran lots of tests for various sync scenarios.

@jchappelow jchappelow merged commit e01f3c7 into kwilteam:main Feb 3, 2025
2 checks passed
@charithabandi charithabandi deleted the blocksync branch February 3, 2025 23:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature Request: Leader Blocksync
2 participants