-
Notifications
You must be signed in to change notification settings - Fork 2
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
Add fork choice rule #58
Conversation
def chain_density(chain: Chain, slot: Slot) -> int: | ||
return count(block for block in chain.blocks if block.slot < slot) | ||
|
||
def maxvalid_bg(local_chain: Chain, forks: List[Chain], k: int, s: int) -> Chain: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we add a note for what k
and s
are?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎸
if m <= k: | ||
# Classic longest chain rule with parameter k | ||
if cmax.length() < chain.length(): | ||
cmax = chain | ||
else: | ||
# The chain is forking too much, we need to pay a bit more attention | ||
# In particular, select the chain that is the densest after the fork | ||
forking_slot = Slot( | ||
cmax.blocks[lowest_common_ancestor].slot.absolute_slot + s | ||
) | ||
cmax_density = chain_density(cmax, forking_slot) | ||
candidate_density = chain_density(chain, forking_slot) | ||
if cmax_density < candidate_density: | ||
cmax = chain |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thinking out loud: If the second forking rule is so great, why do we need the first one? performance? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess 's' can't be to small to avoid tails in the distribution of slot leaders, so you would need to wait a bit every time to observe the distribution after a fork. Also, if you were to propose a block after a slot with duplicate leaders you can't use a rule that needs information about future slots
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! 🚀 ⛓️
Add the fork choice rule as described in 'Ouroboros Genesis: Composable Proof-of-Stake Blockchains with Dynamic Availability'
Tests still missing