Fix double spend issue in reorgs

src/consensus/block_body_validation.py

@@ -39,6 +39,7 @@ log = logging.getLogger(__name__)
 async def validate_block_body(
     constants: ConsensusConstants,
     sub_blocks: Dict[bytes32, SubBlockRecord],
+    sub_height_to_hash: Dict[uint32, bytes32],
     block_store: BlockStore,
     coin_store: CoinStore,
     peak: Optional[SubBlockRecord],
@@ -215,9 +216,18 @@ async def validate_block_body(
 
     # 15. Check if removals exist and were not previously spent. (unspent_db + diff_store + this_block)
     if peak is None or sub_height == 0:
-        fork_h: int = -1
+        fork_sub_h: int = -1
     else:
-        fork_h = find_fork_point_in_chain(sub_blocks, peak, sub_blocks[block.prev_header_hash])
+        fork_sub_h = find_fork_point_in_chain(sub_blocks, peak, sub_blocks[block.prev_header_hash])
+
+    if fork_sub_h == -1:
+        coin_store_reorg_height = -1
+    else:
+        last_sb_in_common = sub_blocks[sub_height_to_hash[uint32(fork_sub_h)]]
+        if last_sb_in_common.is_block:
+            coin_store_reorg_height = last_sb_in_common.height
+        else:
+            coin_store_reorg_height = last_sb_in_common.height - 1
 
     # Get additions and removals since (after) fork_h but not including this block
     additions_since_fork: Dict[bytes32, Tuple[Coin, uint32]] = {}
@@ -228,7 +238,7 @@ async def validate_block_body(
         curr: Optional[FullBlock] = await block_store.get_full_block(block.prev_header_hash)
         assert curr is not None
 
-        while curr.sub_block_height > fork_h:
+        while curr.sub_block_height > fork_sub_h:
             removals_in_curr, additions_in_curr = await curr.tx_removals_and_additions()
             for c_name in removals_in_curr:
                 removals_since_fork.add(c_name)
@@ -259,10 +269,10 @@ async def validate_block_body(
             removal_coin_records[new_unspent.name] = new_unspent
         else:
             unspent = await coin_store.get_coin_record(rem)
-            if unspent is not None and unspent.confirmed_block_index <= fork_h:
+            if unspent is not None and unspent.confirmed_block_index <= coin_store_reorg_height:
                 # Spending something in the current chain, confirmed before fork
                 # (We ignore all coins confirmed after fork)
-                if unspent.spent == 1 and unspent.spent_block_index <= fork_h:
+                if unspent.spent == 1 and unspent.spent_block_index <= coin_store_reorg_height:
                     # Check for coins spent in an ancestor block
                     return Err.DOUBLE_SPEND
                 removal_coin_records[unspent.name] = unspent

src/consensus/blockchain.py

@@ -214,6 +214,7 @@ class Blockchain:
         error_code = await validate_block_body(
             self.constants,
             self.sub_blocks,
+            self.sub_height_to_hash,
             self.block_store,
             self.coin_store,
             self.get_peak(),
@@ -489,6 +490,7 @@ class Blockchain:
         error_code = await validate_block_body(
             self.constants,
             self.sub_blocks,
+            self.sub_height_to_hash,
             self.block_store,
             self.coin_store,
             self.get_peak(),

src/rpc/rpc_server.py

@@ -45,7 +45,6 @@ class RpcServer:
         if self.websocket is None:
             return
         payloads: List[Dict] = await self.rpc_api._state_changed(*args)
-        log.info(f"State changed: {change}")
 
         if change == "add_connection" or change == "close_connection":
             data = await self.get_connections({})

tests/core/consensus/test_blockchain.py

@@ -16,6 +16,7 @@ from src.util.block_tools import get_vdf_info_and_proof
 from src.util.errors import Err
 from src.util.hash import std_hash
 from src.util.ints import uint64, uint8, int512
+from src.util.wallet_tools import WalletTool
 from tests.recursive_replace import recursive_replace
 from tests.setup_nodes import test_constants, bt
 from tests.core.fixtures import empty_blockchain  # noqa: F401
@@ -1503,6 +1504,9 @@ class TestReorgs:
     @pytest.mark.asyncio
     async def test_reorg_from_genesis(self, empty_blockchain):
         b = empty_blockchain
+        WALLET_A = WalletTool()
+        WALLET_A_PUZZLE_HASHES = [WALLET_A.get_new_puzzlehash() for _ in range(5)]
+
         blocks = bt.get_consecutive_blocks(15)
 
         for block in blocks:
@@ -1535,3 +1539,50 @@ class TestReorgs:
         assert result == ReceiveBlockResult.NEW_PEAK
         assert found_orphan
         assert b.get_peak().sub_block_height == 17
+
+    @pytest.mark.asyncio
+    async def test_reorg_transaction(self, empty_blockchain):
+        b = empty_blockchain
+        wallet_a = WalletTool()
+        WALLET_A_PUZZLE_HASHES = [wallet_a.get_new_puzzlehash() for _ in range(5)]
+        coinbase_puzzlehash = WALLET_A_PUZZLE_HASHES[0]
+        receiver_puzzlehash = WALLET_A_PUZZLE_HASHES[1]
+
+        blocks = bt.get_consecutive_blocks(10, farmer_reward_puzzle_hash=coinbase_puzzlehash)
+        blocks = bt.get_consecutive_blocks(
+            2, blocks, farmer_reward_puzzle_hash=coinbase_puzzlehash, guarantee_block=True
+        )
+
+        spend_block = blocks[10]
+        spend_coin = None
+        for coin in list(spend_block.get_included_reward_coins()):
+            if coin.puzzle_hash == coinbase_puzzlehash:
+                spend_coin = coin
+        spend_bundle = wallet_a.generate_signed_transaction(1000, receiver_puzzlehash, spend_coin)
+
+        blocks = bt.get_consecutive_blocks(
+            2,
+            blocks,
+            farmer_reward_puzzle_hash=coinbase_puzzlehash,
+            transaction_data=spend_bundle,
+            guarantee_block=True,
+        )
+
+        blocks_fork = bt.get_consecutive_blocks(
+            1, blocks[:12], farmer_reward_puzzle_hash=coinbase_puzzlehash, seed=b"123", guarantee_block=True
+        )
+        blocks_fork = bt.get_consecutive_blocks(
+            2,
+            blocks_fork,
+            farmer_reward_puzzle_hash=coinbase_puzzlehash,
+            transaction_data=spend_bundle,
+            guarantee_block=True,
+            seed=b"1245",
+        )
+        for block in blocks:
+            result, error_code, _ = await b.receive_block(block)
+            assert error_code is None and result == ReceiveBlockResult.NEW_PEAK
+
+        for block in blocks_fork:
+            result, error_code, _ = await b.receive_block(block)
+            assert error_code is None

tests/core/full_node/test_coin_store.py

@@ -67,7 +67,6 @@ class TestCoinStore:
         should_be_included: Set[Coin] = set()
         last_block_height = -1
         for block in blocks:
-            print(f"Block {block.sub_block_height} {block.is_block()}")
             farmer_coin, pool_coin = block.get_future_reward_coins(last_block_height + 1)
             should_be_included.add(farmer_coin)
             should_be_included.add(pool_coin)
@@ -75,10 +74,6 @@ class TestCoinStore:
                 last_block_height = block.height
                 removals, additions = await block.tx_removals_and_additions()
 
-                print(len(block.get_included_reward_coins()), len(should_be_included_prev))
-                print([c.amount for c in block.get_included_reward_coins()])
-                print([c.amount for c in should_be_included_prev])
-
                 assert block.get_included_reward_coins() == should_be_included_prev
 
                 await coin_store.new_block(block)