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

[RFC] Long fork consensus #2227

Closed
wants to merge 11 commits into from
Closed

[RFC] Long fork consensus #2227

wants to merge 11 commits into from

Conversation

es92
Copy link
Contributor

@es92 es92 commented Apr 12, 2019

I wrote up an RFC for adding a long fork consensus rule to Coda

It has a part 1, which would make the protocol secure under usual 51% assumptions and be easy to implement

It also has a part 2, which would function as a fallback for the chain to recover if the 51% assumption is ever violated

Copy link
Member

@nholland94 nholland94 left a comment

Choose a reason for hiding this comment

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

Left a couple of questions, mostly around attempting to make this document more specific. Part 1 makes sense to me so far. Not able to follow Part 2 yet as there are too many TODOs. Would prefer to see Part 2 fleshed out in a convincing way before we fully commit to starting development on this mechanism.

rfcs/0017-long-fork-consensus.md Show resolved Hide resolved

We currently do not support long forks. This means that to join the network you need a recent checkpoint, in other words to have always been online or to trust another party that has always been online.

This adds support for a client to join the network at any point of time, regardless of if its been online recently. It adds this safely under a [50% + epsilon] security assumption. It also includes a mechanism to restore a chain to strength if this assumption was to be temporarily violated.
Copy link
Member

Choose a reason for hiding this comment

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

I'm sure it's just a parameter I've forgotten from the paper, but how is epsilon defined here? Would be nice to just have a short mention here within the RFC.

Copy link
Contributor

Choose a reason for hiding this comment

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

It just means >1/2. "honest majority". so 50% plus the smallest unit of currency.

rfcs/0017-long-fork-consensus.md Outdated Show resolved Hide resolved
Copy link
Contributor

@enolan enolan left a comment

Choose a reason for hiding this comment

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

I'd like to see the TODOs filled in.

rfcs/0017-long-fork-consensus.md Outdated Show resolved Hide resolved
rfcs/0017-long-fork-consensus.md Outdated Show resolved Hide resolved
staged_leger_.vote_stake += get_ledger_of_hash(staged_ledger.vote_hash).stake_of(transaction.public_key)
else
staged_leger_.vote_stake = 0
staged_ledger.vote_hash = transaction.vote_hash
Copy link
Contributor Author

Choose a reason for hiding this comment

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

add check of account.vote_hash to account for duplicate voting


* if the `snarked_ledger` is being updated
* `if new_snarked_ledger.vote_hash == new_protocol_state.last_epoch_locked_checkpoint`
* set `current_protocol_state.min_epoch` to `MAX(current_protocol_state.min_epoch, new_snarked_ledger.voted_stake/staged_ledger.total_stake)`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

update current_protocol_state.min_epochs as well using inflation discount rule

Copy link
Contributor

@enolan enolan left a comment

Choose a reason for hiding this comment

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

aside from typos this seems workable, although by default I don't trust any security design that hasn't seen adversarial review


* Add a new field to `protocol_state` called `min_window`, initally set to `8k`
* Add a new field to `protocol_state` called `current_window_length`, initially set to 0
* `let window_diff = FLOOR(current_protocol_state.global_slot_number/(28)) - FLOOR(previous_protocol_state.global_slot_number/(8k))`
Copy link
Contributor

Choose a reason for hiding this comment

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

should the 28 be 8k?

* `let window_diff = FLOOR(current_protocol_state.global_slot_number/(28)) - FLOOR(previous_protocol_state.global_slot_number/(8k))`
* if `window_diff == 0`
* set `current_protocol_state.current_window_length` to `previous_protocol_state.current_window_length + 1`
* else if `window_iff == 1`
Copy link
Contributor

Choose a reason for hiding this comment

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

s/iff/diff


Add to the chain select(A,B) function:

* If the previous-epoch checkpoint does not match between current and proposed, choose the chain with the greater `min_`. If its a tie choose the already held chain.
Copy link
Contributor

Choose a reason for hiding this comment

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

s/min_/min_window

Copy link
Contributor

Choose a reason for hiding this comment

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

it should use the existing tiebreaker logic in the event of a tie in min_window. using the already held chain will lead to inconsistency when different nodes get the blocks in different orders.

* add to client software a field called `expected_reset_hash`, initially set to `0`
* when deciding whether to accept a protocol state, check that `client.expected_reset_hash = protocol_state.reset_hash`
* when creating a new `protocol_state`, if the `client.expected_reset_hash` does not equal the previous protocol states' `reset_hash`, update the `reset_hash` in the new protocol state to `client.expected_reset_hash`.
* Add to the `protocol_state` update logic that if the `reset_hash` has been updated, reset `min_window` to `1`
Copy link
Contributor

Choose a reason for hiding this comment

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

s/1/8k I think

* set `current_protocol_state.min_window` to `MIN(current_protocol_state.current_window_length, previous_protocol_state.min_window)`
* set `current_protocol_state.current_window_length` to `1`
* else
* set `current_protocol_state.min_window` to `0`

Choose a reason for hiding this comment

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

I think there should be a special case for the proposers who just start proposing off the genesis point. In this case, there might be a lot of empty epochs before the first transition is proposed. We should check for this special case and do not drop the min_window to 0 in this case.

@bkase bkase changed the base branch from master to develop July 30, 2019 19:40
@nholland94
Copy link
Member

Closing as this is no longer relevant with Ouroboros Samasika.

@nholland94 nholland94 closed this Mar 31, 2020
@nholland94 nholland94 deleted the long-fork-consensus-rfc branch March 31, 2020 19:55
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.

None yet

4 participants