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

doc: Spec of finality vote submission #120

Merged
merged 7 commits into from
Nov 19, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)

## Unreleased

### Documentation

[#120](https://github.com/babylonlabs-io/finality-provider/pull/120) Spec of
finality vote submission

## v0.10.0

### Improvements
Expand Down
82 changes: 82 additions & 0 deletions docs/send-finality-vote.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
# Finality Votes Submission Specification

## Overview

Finality providers submit votes to finalize blocks on the consumer chain.
This document specifies the process of submitting finality votes.

## Internal State

The finality provider maintains two critical persistent states:
Copy link
Collaborator

@bap2pecs bap2pecs Nov 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think it's simpler to just keep one persistent state in DB (i.e. last processed height)

last voted height can be easily retrieved from remote

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually do you think we can get rid of both from DB and only keeps one in-memory state (i.e. fp.nextHeightToProcess)?

I feel it's doable

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. I think we maybe can get rid of lastVotedHeight but why do you say it's easy to retrieve lastVotedHeight from the remote? It seems that we need to save this info in the finality provider on Babylon

actually do you think we can get rid of both from DB and only keeps one in-memory state (i.e. fp.nextHeightToProcess)?

if we get rid of fp.nextHeightToProcess, we will waste some resources processing blocks that have already been processed, won't we?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but why do you say it's easy to retrieve lastVotedHeight from the remote? It seems that we need to save this info in the finality provider on Babylon

b/c if we store it in a local DB, there will be two source of truth. for the OP consumer FP, we can easily add a query in the CosmWasm contract for it e.g. getFpLastVotedHeight

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we get rid of fp.nextHeightToProcess, we will waste some resources processing blocks that have already been processed, won't we?

no I mean to get rid of the local DB states but keep nextHeightToProcess

how it works is: at startup, the FP will get everything it needs from the Babylon Chain and/or the CosmWasm contract to set nextHeightToProcess

but nextHeightToProcess is not a DB state, it's an in-memory field value of the FinalityProvider object

Copy link
Collaborator

@bap2pecs bap2pecs Nov 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

presisting lastProcessedHeight means engineers will need to have it in their mental model to make sure this data is updated and think about edge cases where it can go out of sync. and also need to think about when and where to compare with lastVotedHeight and what the results imply.

that's why I think it's not worth it if the edge case discussed will almost never gonna happen in the real world. and even if it happens, the impact is negligible.

but I don't have a strong preference either. will leave it to your call

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. Will bring it to the team to make the decision. Appreciate the discussion!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After a second thought, I feel that we can get rid of lastProcessedHeight completely (leaving unharmful edge case open) but we probably need to keep the lastVotedHeight both in db and memory. If we only have it in memory, there would be an attack I can think of:

Consider a fp is crashed right after sending a vote for height 100. After restart, it syncs with the remote and initialize the last voted height. However, the last voted height of the fp from the remote could be still 99 due to the late excution of the block. Therefore, it is possible for the fp to send another vote for 100. This puts the fp under the risk of being slashed due to signing the same height twice.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

However, the last voted height of the fp from the remote could be still 99 due to the late excution of the block.

do u mean the babylon chain didn't execute the block containing the vote in time?

but as long as the FP signs the same block at height 100, it won't be slashed.

the assumption is the FP is connected to a trusted RPC that won't send forks to it.

but i guess it's probably safer to have some redundancy considering FP is crucial

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, I don't think it's safe to assume that FP is connected to a trusted RPC as we can not control it. The bottom line is that fp should not sign twice for the same height.


- Last voted height
- Last processed height

### Last voted height

Tracks the most recent height for which a finality vote was successfully
submitted. This prevents duplicate voting on previously voted heights.

### Last processed height
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's the difference between last voted height and last processed height?

Copy link
Member Author

@gitferry gitferry Nov 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

last processed height additionally covers the case where the fp does not have voting power


Tracks the most recent height for which a voting decision was made
(whether the decision was to vote or not).
By definition, `LastProcessedHeight >= LastVotedHeight` always holds.

## Submission Process

### Bootstraping

To determine the initial processing height:

1. Query consumer chain for `lastFinalizedHeight`
2. If no finalized blocks exist:
- Start from `finalityActivationHeight`
Copy link
Collaborator

@bap2pecs bap2pecs Nov 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is a problem here.

imagine there is no finalized block, but this FP already voted for the first activated block and it's just waiting for the rest FPs to vote so the block can get finalized

starting at finalityActivationHeight means a duplicate vote

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. See updates in ab8913a

3. Query `lastVotedHeightRemote` from consumer chain
4. If no previous votes from this provider:
- Start from `lastFinalizedHeight + 1`
5. Synchronize local state:
- Ensure local `lastVotedHeight` and `lastProcessedHeight` ≥ `lastVotedHeightRemote`
6. Begin processing at:
- `max(lastProcessedHeight + 1, lastFinalizedHeight + 1)`

### Normal Submission Loop

After the finality provider is bootstraped, it continuously monitors for
new blocks. For each new block, it performs these validation checks:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think it is worth mentioning that finality provider receives block from trusted full node of consumer chain (or at least light client ) ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. Fixed in 8732f15


1. Block hasn't been previously processed
2. Block height exceeds finality activation height
3. Finality provider has sufficient voting power

Upon passing all checks, the finality provider:

1. Requests a finality signature from the EOTS manager
2. Submits the vote transaction to the consumer chain
3. Implements retry logic until either:
- Maximum retry attempts are reached
- Block becomes finalized

#### Batch submission

A batch submission mechanism is needed to deal with cases where:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would maybe note that there is a synchronization mechanism between loops so that we don't submit from fast sync loop and regular one

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the current code, all the submission is done in the submission loop. So there's no race condition. The fast sync loop is for sending the lagging signal. If lagging is detected, the sumission loop will do fast sync. See

case targetBlock := <-fp.laggingTargetChan:
.

Nevertheless, in the spec, we use batch submission other than fast sync and we plan to remove the latter completely.


- recovery from downtime, and
- the consumer chain has rapid block production.

Batch sumission puts multiple new blocks into a batch and
process them in the same loop, after which all the finality votes will be sent
in the same transaction to the consumer chain.

### Generating Finality Votes

Each finality vote contains:

1. EOTS signature for the target block (signed by EOTS manager)
2. Public randomness for the block height
3. Merkle inclusion proof for the public randomness

The consumer chain verifies:

- EOTS signature validity
- Randomness was pre-committed and BTC-timestamped
Loading