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

feat(consensus): eliminate block ties #673

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
70 changes: 23 additions & 47 deletions hathor/consensus/block_consensus.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
# limitations under the License.

from itertools import chain
from typing import TYPE_CHECKING, Any, Iterable, Optional, cast
from typing import TYPE_CHECKING, Any, Iterable, Optional

from structlog import get_logger

Expand Down Expand Up @@ -161,16 +161,9 @@ def update_voided_info(self, block: Block) -> None:
self.mark_as_voided(block, skip_remove_first_block_markers=True)

# Get the score of the best chains.
heads = [cast(Block, storage.get_transaction(h)) for h in storage.get_best_block_tips()]
best_score: int | None = None
for head in heads:
head_meta = head.get_metadata(force_reload=True)
if best_score is None:
best_score = head_meta.score
else:
# All heads must have the same score.
assert best_score == head_meta.score
assert best_score is not None
head = storage.get_best_block()
head_meta = head.get_metadata(force_reload=True)
best_score = head_meta.score

# Calculate the score.
# We cannot calculate score before getting the heads.
Expand All @@ -185,20 +178,27 @@ def update_voided_info(self, block: Block) -> None:
# Either eveyone has the same score or there is a winner.

valid_heads = []
for head in heads:
meta = head.get_metadata()
if not meta.voided_by:
valid_heads.append(head)
if not head_meta.voided_by:
valid_heads.append(head)

# We must have at most one valid head.
# Either we have a single best chain or all chains have already been voided.
assert len(valid_heads) <= 1, 'We must never have more than one valid head'

# Add voided_by to all heads.
common_block = self._find_first_parent_in_best_chain(block)
self.add_voided_by_to_multiple_chains(block, heads, common_block)

winner = False
if score > best_score:
winner = True
else:
min_hash: bytes = head.hash
if block.hash < min_hash:
winner = True

if winner:
# Add voided_by to all heads.
self.add_voided_by_to_multiple_chains(block, [head], common_block)

# We have a new winner candidate.
self.update_score_and_mark_as_the_best_chain_if_possible(block)
# As `update_score_and_mark_as_the_best_chain_if_possible` may affect `voided_by`,
Expand All @@ -211,14 +211,8 @@ def update_voided_info(self, block: Block) -> None:
storage.indexes.height.update_new_chain(height, block)
storage.update_best_block_tips_cache([block.hash])
# It is only a re-org if common_block not in heads
if common_block not in heads:
if common_block != head:
self.context.mark_as_reorg(common_block)
else:
best_block_tips = [blk.hash for blk in heads]
best_block_tips.append(block.hash)
storage.update_best_block_tips_cache(best_block_tips)
if not meta.voided_by:
self.context.mark_as_reorg(common_block)

def union_voided_by_from_parents(self, block: Block) -> set[bytes]:
"""Return the union of the voided_by of block's parents.
Expand Down Expand Up @@ -286,31 +280,13 @@ def update_score_and_mark_as_the_best_chain_if_possible(self, block: Block) -> N
self.update_score_and_mark_as_the_best_chain(block)
self.remove_voided_by_from_chain(block)

best_score: int
if self.update_voided_by_from_parents(block):
storage = block.storage
heads = [cast(Block, storage.get_transaction(h)) for h in storage.get_best_block_tips()]
best_score = 0
best_heads: list[Block]
for head in heads:
head_meta = head.get_metadata(force_reload=True)
if head_meta.score < best_score:
continue

if head_meta.score > best_score:
best_heads = [head]
best_score = head_meta.score
else:
assert best_score == head_meta.score
best_heads.append(head)
assert isinstance(best_score, int) and best_score > 0

assert len(best_heads) > 0
first_block = self._find_first_parent_in_best_chain(best_heads[0])
self.add_voided_by_to_multiple_chains(best_heads[0], [block], first_block)
if len(best_heads) == 1:
assert best_heads[0].hash != block.hash
self.update_score_and_mark_as_the_best_chain_if_possible(best_heads[0])
head = storage.get_best_block()
first_block = self._find_first_parent_in_best_chain(head)
self.add_voided_by_to_multiple_chains(head, [block], first_block)
assert head.hash != block.hash
self.update_score_and_mark_as_the_best_chain_if_possible(head)

def update_score_and_mark_as_the_best_chain(self, block: Block) -> None:
""" Update score and mark the chain as the best chain.
Expand Down
5 changes: 5 additions & 0 deletions hathor/transaction/storage/transaction_storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -618,6 +618,11 @@ def get_best_block_tips(self, timestamp: Optional[float] = None, *, skip_cache:
elif meta.score > best_score:
best_score = meta.score
best_tip_blocks = [block_hash]

# XXX: if there's more than one we filter it so it's the smallest hash
if len(best_tip_blocks) > 1:
Copy link
Member Author

Choose a reason for hiding this comment

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

Should we allow the caller to choose whether they want to get just one or all of them?

Copy link
Member

Choose a reason for hiding this comment

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

I think that could make sense as a new method, like get_all_block_tips, instead of get_best_block_tips. But currently that method wouldn't be used anywhere else, and when removing the tips indexes, it would be more convenient if we didn't have to maintain that method.

Copy link
Member Author

Choose a reason for hiding this comment

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

Should we create an issue to refactor this method and simply store the current best block in the storage?

Copy link
Member

Choose a reason for hiding this comment

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

#540 sort of does this by using the height-index, but because it's not using this PR as a base it does this incorrectly and hides other tips, after this PR that change would be correct

best_tip_blocks = [min(best_tip_blocks)]

if timestamp is None:
self._best_block_tips_cache = best_tip_blocks[:]
return best_tip_blocks
Expand Down
Loading
Loading