feat(consensus): Change

Co-authored-by: Jan Segre <jan@hathor.network>

hathor/consensus/block_consensus.py

@@ -194,11 +194,20 @@ def update_voided_info(self, block: Block) -> None:
                 # 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 = min(blk.hash for blk in heads)
+                    if block.hash < min_hash:
+                        winner = True
+
+                if winner:
+                    # Add voided_by to all heads.
+                    self.add_voided_by_to_multiple_chains(block, heads, 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`,
@@ -216,9 +225,8 @@ def update_voided_info(self, block: Block) -> None:
                 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)
+                    best_block_tip = min(best_block_tips)
+                    storage.update_best_block_tips_cache([best_block_tip])
 
     def union_voided_by_from_parents(self, block: Block) -> set[bytes]:
         """Return the union of the voided_by of block's parents.

hathor/transaction/storage/transaction_storage.py

@@ -624,6 +624,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:
+            best_tip_blocks = [min(best_tip_blocks)]
+
         if timestamp is None:
             self._best_block_tips_cache = best_tip_blocks[:]
         return best_tip_blocks

tests/p2p/test_split_brain.py

@@ -6,12 +6,29 @@
 from hathor.manager import HathorManager
 from hathor.simulator import FakeConnection
 from hathor.simulator.utils import add_new_block
+from hathor.transaction import Block
 from hathor.util import not_none
 from hathor.wallet import HDWallet
 from tests import unittest
 from tests.utils import add_blocks_unlock_reward, add_new_double_spending, add_new_transactions
 
 
+def select_best_block(b1: Block, b2: Block) -> Block:
+    """This function returns the best block according to score and using hash as tiebreaker."""
+    meta1 = b1.get_metadata()
+    meta2 = b2.get_metadata()
+    if meta1.score == meta2.score:
+        if b1.hash < b2.hash:
+            return b1
+        else:
+            return b2
+    else:
+        if meta1.score > meta2.score:
+            return b1
+        else:
+            return b2
+
+
 class BaseHathorSyncMethodsTestCase(unittest.TestCase):
     __test__ = False
 
@@ -145,14 +162,9 @@ def test_split_brain_only_blocks_different_height(self) -> None:
         self.assertEqual(block_tip1, not_none(manager1.tx_storage.indexes).height.get_tip())
         self.assertEqual(block_tip1, not_none(manager2.tx_storage.indexes).height.get_tip())
 
-    # XXX We must decide what to do when different chains have the same score
-    # For now we are voiding everyone until the first common block
     def test_split_brain_only_blocks_same_height(self) -> None:
         manager1 = self.create_peer(self.network, unlock_wallet=True)
-        # manager1.avg_time_between_blocks = 3  # FIXME: This property is not defined. Fix this test.
-
         manager2 = self.create_peer(self.network, unlock_wallet=True)
-        # manager2.avg_time_between_blocks = 3  # FIXME: This property is not defined. Fix this test.
 
         for _ in range(10):
             add_new_block(manager1, advance_clock=1)
@@ -161,13 +173,14 @@ def test_split_brain_only_blocks_same_height(self) -> None:
             unlock_reward_blocks2 = add_blocks_unlock_reward(manager2)
             self.clock.advance(10)
 
-        block_tips1 = unlock_reward_blocks1[-1].hash
-        block_tips2 = unlock_reward_blocks2[-1].hash
+        block_tip1 = unlock_reward_blocks1[-1]
+        block_tip2 = unlock_reward_blocks2[-1]
+        best_block = select_best_block(block_tip1, block_tip2)
 
         self.assertEqual(len(manager1.tx_storage.get_best_block_tips()), 1)
-        self.assertCountEqual(manager1.tx_storage.get_best_block_tips(), {block_tips1})
+        self.assertCountEqual(manager1.tx_storage.get_best_block_tips(), {block_tip1.hash})
         self.assertEqual(len(manager2.tx_storage.get_best_block_tips()), 1)
-        self.assertCountEqual(manager2.tx_storage.get_best_block_tips(), {block_tips2})
+        self.assertCountEqual(manager2.tx_storage.get_best_block_tips(), {block_tip2.hash})
 
         # Save winners for manager1 and manager2
         winners1 = set()
@@ -202,10 +215,13 @@ def test_split_brain_only_blocks_same_height(self) -> None:
         self.assertConsensusValid(manager1)
         self.assertConsensusValid(manager2)
 
-        # self.assertEqual(len(manager1.tx_storage.get_best_block_tips()), 2)
-        # self.assertCountEqual(manager1.tx_storage.get_best_block_tips(), {block_tips1, block_tips2})
-        # self.assertEqual(len(manager2.tx_storage.get_best_block_tips()), 2)
-        # self.assertCountEqual(manager2.tx_storage.get_best_block_tips(), {block_tips1, block_tips2})
+        # XXX: there must always be a single winner, some methods still return containers (set/list/...) because
+        #      multiple winners were supported in the past, but those will eventually be refactored
+        # import pudb; pu.db
+        self.assertEqual(len(manager1.tx_storage.get_best_block_tips()), 1)
+        self.assertCountEqual(manager1.tx_storage.get_best_block_tips(), {best_block.hash})
+        self.assertEqual(len(manager2.tx_storage.get_best_block_tips()), 1)
+        self.assertCountEqual(manager2.tx_storage.get_best_block_tips(), {best_block.hash})
 
         winners1_after = set()
         for tx1 in manager1.tx_storage.get_all_transactions():
@@ -219,10 +235,10 @@ def test_split_brain_only_blocks_same_height(self) -> None:
             if not tx2_meta.voided_by:
                 winners2_after.add(tx2.hash)
 
-        # Both chains have the same height and score
-        # so they will void all blocks and keep only the genesis (the common block and txs)
-        self.assertEqual(len(winners1_after), 3)
-        self.assertEqual(len(winners2_after), 3)
+        # Both chains have the same height and score, which is of the winner block,
+        expected_count = not_none(best_block.get_metadata().height) + 3  # genesis vertices are included
+        self.assertEqual(len(winners1_after), expected_count)
+        self.assertEqual(len(winners2_after), expected_count)
 
         new_block = add_new_block(manager1, advance_clock=1)
         self.clock.advance(20)
@@ -257,7 +273,7 @@ def test_split_brain_only_blocks_same_height(self) -> None:
         winners1.add(new_block.hash)
         winners2.add(new_block.hash)
 
-        if new_block.get_block_parent().hash == block_tips1:
+        if new_block.get_block_parent().hash == block_tip1.hash:
             winners = winners1
         else:
             winners = winners2

tests/tx/test_blockchain.py

@@ -238,21 +238,28 @@ def test_multiple_forks(self):
         sidechain.append(fork_block2)
 
         # Now, both chains have the same score.
-        for block in blocks:
+        if blocks[-1].hash < sidechain[-1].hash:
+            winning_chain = blocks
+            losing_chain = sidechain
+        else:
+            winning_chain = sidechain
+            losing_chain = blocks
+
+        for block in winning_chain:
             meta = block.get_metadata(force_reload=True)
-            self.assertEqual(meta.voided_by, {block.hash})
+            self.assertIsNone(meta.voided_by)
 
-        for block in sidechain:
+        for block in losing_chain:
             meta = block.get_metadata(force_reload=True)
             self.assertEqual(meta.voided_by, {block.hash})
 
         for tx in txs1:
             meta = tx.get_metadata(force_reload=True)
             self.assertEqual(meta.first_block, block_before_fork.hash)
 
-        for tx in txs2:
-            meta = tx.get_metadata(force_reload=True)
-            self.assertIsNone(meta.first_block)
+        # for tx in txs2:
+        #     meta = tx.get_metadata(force_reload=True)
+        #     self.assertIsNone(meta.first_block)
 
         # Mine 1 block, starting another fork.
         # This block belongs to case (vi).