Block ordering (#142)

* Look at current block additions before removals
* Fix block ordering, and change state machine of wallet node
* Fix infinite recursion
* Fix some serious bugs with wallets/coinbases/reorgs and add testing

Author: mariano54

src/wallet/block_record.py

@@ -19,7 +19,7 @@ class BlockRecord(Streamable):
     prev_header_hash: bytes32
     height: uint32
     weight: uint128
-    additions: List[Coin]
-    removals: List[bytes32]
+    additions: Optional[List[Coin]]  # A block record without additions is not finished
+    removals: Optional[List[bytes32]]  # A block record without removals is not finished
     total_iters: Optional[uint64]
     new_challenge_hash: Optional[bytes32]

src/wallet/wallet.py

@@ -129,15 +129,13 @@ async def get_keys(
 
     async def select_coins(self, amount) -> Optional[Set[Coin]]:
         """ Returns a set of coins that can be used for generating a new transaction. """
+        spendable_am = await self.wallet_state_manager.get_unconfirmed_spendable_for_wallet(
+            self.wallet_info.id
+        )
 
-        if (
-            amount
-            > await self.wallet_state_manager.get_unconfirmed_spendable_for_wallet(
-                self.wallet_info.id
-            )
-        ):
+        if amount > spendable_am:
             self.log.warning(
-                f"Can't select amount higher than our spendable balance {amount}"
+                f"Can't select amount higher than our spendable balance {amount}, spendable {spendable_am}"
             )
             return None
 

src/wallet/wallet_node.py

@@ -240,7 +240,7 @@ async def get_confirmed_balance_for_wallet(self, wallet_id: int) -> uint64:
 
         for record in record_list:
             amount = uint64(amount + record.coin.amount)
-        self.log.info(f"amount is {amount}")
+        self.log.info(f"Confirmed balance amount is {amount}")
         return uint64(amount)
 
     async def get_unconfirmed_balance(self, wallet_id) -> uint64:
@@ -487,6 +487,30 @@ async def receive_block(
         fast sync). If validation succeeds, block is adedd to DB. If it's a new TIP, transactions are
         reorged accordingly.
         """
+        cb_and_fees_additions = []
+        if header_block is not None:
+            coinbase = header_block.header.data.coinbase
+            fees_coin = header_block.header.data.fees_coin
+            if await self.is_addition_relevant(coinbase):
+                cb_and_fees_additions.append(coinbase)
+            if await self.is_addition_relevant(fees_coin):
+                cb_and_fees_additions.append(fees_coin)
+        assert block.additions is not None
+        if len(cb_and_fees_additions) > 0:
+            block = BlockRecord(
+                block.header_hash,
+                block.prev_header_hash,
+                block.height,
+                block.weight,
+                block.additions + cb_and_fees_additions,
+                block.removals,
+                block.total_iters,
+                block.new_challenge_hash,
+            )
+
+        assert block.additions is not None
+        assert block.removals is not None
+
         async with self.lock:
             if block.header_hash in self.block_records:
                 return ReceiveBlockResult.ALREADY_HAVE_BLOCK
@@ -547,15 +571,18 @@ async def receive_block(
                 for path_block in blocks_to_add:
                     self.height_to_hash[path_block.height] = path_block.header_hash
                     await self.wallet_store.add_block_to_path(path_block.header_hash)
-                    if header_block is not None:
-                        coinbase = header_block.header.data.coinbase
-                        fees_coin = header_block.header.data.fees_coin
-                        if await self.is_addition_relevant(coinbase):
-                            await self.coin_added(coinbase, path_block.height, True)
-                        if await self.is_addition_relevant(fees_coin):
-                            await self.coin_added(fees_coin, path_block.height, True)
+                    assert (
+                        path_block.additions is not None
+                        and path_block.removals is not None
+                    )
                     for coin in path_block.additions:
-                        await self.coin_added(coin, path_block.height, False)
+                        is_coinbase = (
+                            True
+                            if bytes32((path_block.height).to_bytes(32, "big"))
+                            == coin.parent_coin_info
+                            else False
+                        )
+                        await self.coin_added(coin, path_block.height, is_coinbase)
                     for coin_name in path_block.removals:
                         await self.coin_removed(coin_name, path_block.height)
                 self.lca = block.header_hash
@@ -810,7 +837,7 @@ def validate_select_proofs(
         self,
         all_proof_hashes: List[Tuple[bytes32, Optional[Tuple[uint64, uint64]]]],
         heights: List[uint32],
-        cached_blocks: Dict[bytes32, Tuple[BlockRecord, HeaderBlock]],
+        cached_blocks: Dict[bytes32, Tuple[BlockRecord, HeaderBlock, Optional[bytes]]],
         potential_header_hashes: Dict[uint32, bytes32],
     ) -> bool:
         """
@@ -825,7 +852,7 @@ def validate_select_proofs(
             prev_height = uint32(height - 1)
             # Get previous header block
             prev_hh = potential_header_hashes[prev_height]
-            _, prev_header_block = cached_blocks[prev_hh]
+            _, prev_header_block, _ = cached_blocks[prev_hh]
 
             # Validate proof hash of previous header block
             if (
@@ -868,7 +895,7 @@ def validate_select_proofs(
 
             # Get header block
             hh = potential_header_hashes[height]
-            _, header_block = cached_blocks[hh]
+            _, header_block, _ = cached_blocks[hh]
 
             # Validate challenge hash is == pospace challenge hash
             if challenge_hash != header_block.proof_of_space.challenge_hash:
@@ -966,21 +993,59 @@ def validate_select_proofs(
         return True
 
     async def get_filter_additions_removals(
-        self, transactions_fitler: bytes
+        self, new_block: BlockRecord, transactions_filter: bytes
     ) -> Tuple[List[bytes32], List[bytes32]]:
         """ Returns a list of our coin ids, and a list of puzzle_hashes that positively match with provided filter. """
-        tx_filter = PyBIP158([b for b in transactions_fitler])
-        my_coin_records: Set[
+        assert new_block.prev_header_hash in self.block_records
+
+        tx_filter = PyBIP158([b for b in transactions_filter])
+
+        # Find fork point
+        fork_h: uint32 = self._find_fork_point_in_chain(
+            self.block_records[self.lca], new_block
+        )
+
+        # Get all unspent coins
+        my_coin_records_lca: Set[
             WalletCoinRecord
-        ] = await self.wallet_store.get_coin_records_by_spent(False)
+        ] = await self.wallet_store.get_coin_records_by_spent(False, uint32(fork_h + 1))
+
+        # Filter coins up to and including fork point
+        unspent_coin_names: Set[bytes32] = set()
+        for coin in my_coin_records_lca:
+            if coin.confirmed_block_index <= fork_h:
+                unspent_coin_names.add(coin.name())
+
+        # Get all blocks after fork point up to but not including this block
+        curr: BlockRecord = self.block_records[new_block.prev_header_hash]
+        reorg_blocks: List[BlockRecord] = []
+        while curr.height > fork_h:
+            reorg_blocks.append(curr)
+            curr = self.block_records[curr.prev_header_hash]
+        reorg_blocks.reverse()
+
+        # For each block, process additions to get all Coins, then process removals to get unspent coins
+        for reorg_block in reorg_blocks:
+            assert (
+                reorg_block.additions is not None and reorg_block.removals is not None
+            )
+            for addition in reorg_block.additions:
+                unspent_coin_names.add(addition.name())
+            for removal in reorg_block.removals:
+                unspent_coin_names.remove(removal)
+
+        if new_block.additions is not None:
+            for addition in new_block.additions:
+                unspent_coin_names.add(addition.name())
+
         my_puzzle_hashes = await self.puzzle_store.get_all_puzzle_hashes()
 
         removals_of_interest: bytes32 = []
         additions_of_interest: bytes32 = []
 
-        for record in my_coin_records:
-            if tx_filter.Match(bytearray(record.name())):
-                removals_of_interest.append(record.name())
+        for coin_name in unspent_coin_names:
+            if tx_filter.Match(bytearray(coin_name)):
+                removals_of_interest.append(coin_name)
 
         for puzzle_hash in my_puzzle_hashes:
             if tx_filter.Match(bytearray(puzzle_hash)):

src/wallet/wallet_state_manager.py

@@ -143,13 +143,20 @@ async def get_coin_record(self, coin_name: bytes32) -> Optional[WalletCoinRecord
             )
         return None
 
-    async def get_coin_records_by_spent(self, spent: bool) -> Set[WalletCoinRecord]:
+    async def get_coin_records_by_spent(
+        self, spent: bool, spend_before_height: Optional[uint32] = None
+    ) -> Set[WalletCoinRecord]:
         """ Returns set of CoinRecords that have not been spent yet. """
         coins = set()
-
-        cursor = await self.db_connection.execute(
-            "SELECT * from coin_record WHERE spent=?", (int(spent),)
-        )
+        if spend_before_height:
+            cursor = await self.db_connection.execute(
+                "SELECT * from coin_record WHERE spent=? OR spent_index>=?",
+                (int(spent), spend_before_height),
+            )
+        else:
+            cursor = await self.db_connection.execute(
+                "SELECT * from coin_record WHERE spent=?", (int(spent),)
+            )
         rows = await cursor.fetchall()
         await cursor.close()
         for row in rows:
@@ -191,9 +198,10 @@ async def get_spendable_for_index(
     ) -> Set[WalletCoinRecord]:
         """ Returns set of CoinRecords that have been confirmed before index height. """
         coins = set()
+        print("index,", index, "walelt id", wallet_id)
 
         cursor_coinbase_coins = await self.db_connection.execute(
-            "SELECT * from coin_record WHERE spent=? and confirmed_index<? and wallet_id=? and coinbase=?",
+            "SELECT * from coin_record WHERE spent=? and confirmed_index<=? and wallet_id=? and coinbase=?",
             (0, int(index), wallet_id, 1),
         )
 

src/wallet/wallet_store.py

@@ -279,16 +279,18 @@ async def test_wallet_make_transaction_hop(self, two_wallet_nodes_five_freeze):
         spend_bundle = await wallet_0.generate_signed_transaction(
             10, await wallet_node_1.main_wallet.get_new_puzzlehash(), 0
         )
+
         await wallet_0.push_transaction(spend_bundle)
 
         await asyncio.sleep(1)
+        # Full node height 11, wallet height 9
         confirmed_balance = await wallet_0.get_confirmed_balance()
         unconfirmed_balance = await wallet_0.get_unconfirmed_balance()
 
         assert confirmed_balance == funds
         assert unconfirmed_balance == funds - 10
 
-        for i in range(0, 3):
+        for i in range(0, 7):
             await full_node_0.farm_new_block(FarmNewBlockProtocol(token_bytes()))
 
         await asyncio.sleep(1)
@@ -300,6 +302,7 @@ async def test_wallet_make_transaction_hop(self, two_wallet_nodes_five_freeze):
             ]
         )
 
+        # Full node height 17, wallet height 15
         confirmed_balance = await wallet_0.get_confirmed_balance()
         unconfirmed_balance = await wallet_0.get_unconfirmed_balance()
         wallet_2_confirmed_balance = await wallet_1.get_confirmed_balance()
@@ -313,7 +316,7 @@ async def test_wallet_make_transaction_hop(self, two_wallet_nodes_five_freeze):
         )
         await wallet_1.push_transaction(spend_bundle)
 
-        for i in range(0, 3):
+        for i in range(0, 7):
             await full_node_0.farm_new_block(FarmNewBlockProtocol(token_bytes()))
 
         await asyncio.sleep(1)