Merge pull request #586 from AdamISZ/walletsyncfix2

Wallet sync imports at least what is in index_cache, reducing number …

joinmarket/blockchaininterface.py

@@ -575,12 +575,37 @@ def sync_addresses(self, wallet):
             return
         log.debug('requesting wallet history')
         wallet_name = self.get_wallet_name(wallet)
+	#TODO It is worth considering making this user configurable:
         addr_req_count = 20
         wallet_addr_list = []
         for mix_depth in range(wallet.max_mix_depth):
             for forchange in [0, 1]:
+		#If we have an index-cache available, we can use it
+		#to decide how much to import (note that this list
+		#*always* starts from index 0 on each branch).
+		#In cases where the Bitcoin Core instance is fresh,
+		#this will allow the entire import+rescan to occur
+		#in 2 steps only.
+		if wallet.index_cache != [[0,0]]*wallet.max_mix_depth:
+		    #Need to request N*addr_req_count where N is least s.t.
+		    #N*addr_req_count > index_cache val. This is so that the batching
+		    #process in the main loop *always* has already imported enough
+		    #addresses to complete.
+		    req_count = int(wallet.index_cache[mix_depth]
+		                    [forchange]/addr_req_count) + 1
+		    req_count *= addr_req_count
+		else:
+		    #If we have *nothing* - no index_cache, and no info
+		    #in Core wallet (imports), we revert to a batching mode
+		    #with a default size.
+		    #In this scenario it could require several restarts *and*
+		    #rescans; perhaps user should set addr_req_count high
+		    #(see above TODO)
+		    req_count = addr_req_count
                 wallet_addr_list += [wallet.get_new_addr(mix_depth, forchange)
-                                     for _ in range(addr_req_count)]
+                                     for _ in range(req_count)]
+		#Indices are reset here so that the next algorithm step starts
+		#from the beginning of each branch
                 wallet.index[mix_depth][forchange] = 0
         # makes more sense to add these in an account called "joinmarket-imported" but its much
         # simpler to add to the same account here
@@ -633,12 +658,26 @@ def sync_addresses(self, wallet):
                             unused_addr_count = 0
                         else:
                             unused_addr_count += 1
-
-                if last_used_addr == '':
-                    wallet.index[mix_depth][forchange] = 0
-                else:
-		    next_avail_idx = max([wallet.addr_cache[last_used_addr][2]+1,
-		                        wallet.index_cache[mix_depth][forchange]])
+		#index setting here depends on whether we broke out of the loop
+		#early; if we did, it means we need to prepare the index
+		#at the level of the last used address or zero so as to not
+		#miss any imports in add_watchonly_addresses.
+		#If we didn't, we need to respect the index_cache to avoid
+		#potential address reuse.
+		if breakloop:
+		    if last_used_addr == '':
+			wallet.index[mix_depth][forchange] = 0
+		    else:
+			wallet.index[mix_depth][forchange] = \
+			    wallet.addr_cache[last_used_addr][2] + 1
+		else:
+		    if last_used_addr == '':
+			next_avail_idx = max([wallet.index_cache[mix_depth]
+			                      [forchange], 0])
+		    else:
+			next_avail_idx = max([wallet.addr_cache[last_used_addr]
+			                      [2]+1, wallet.index_cache[mix_depth]
+			                      [forchange]])
 		    wallet.index[mix_depth][forchange] = next_avail_idx
 
         wallet_addr_list = []

test/test_wallets.py

@@ -118,7 +118,10 @@ def test_wallet_sync(setup_wallets, num_txs, fake_count,
         jm_single().bc_interface.sync_wallet(wallet)
         #avoid infinite loop on failure.
         assert sync_count < 10
-
+    #Wallet should recognize index_cache on sync, so should not need to
+    #run sync process more than twice (twice if cache bump has moved us
+    #past the first round of imports).
+    assert sync_count <= 2
     #validate the wallet index values after sync
     for i, ws in enumerate(wallet_structure):
         assert wallet.index[i][0] == ws #spends into external only
@@ -131,6 +134,56 @@ def test_wallet_sync(setup_wallets, num_txs, fake_count,
     #Now try to do more transactions as sanity check.
     do_tx(wallet, 50000000)
 
+
+@pytest.mark.parametrize(
+    "wallet_structure, wallet_file, password, ic",
+    [
+        #As usual, more test cases are preferable but time
+        #of build test is too long, so only one activated.
+        #([11,3,4,5,6], 'test_import_wallet.json', 'import-pwd',
+        # [(12,3),(100,99),(7, 40), (200, 201), (10,0)]
+        # ),
+        ([1,3,0,2,9], 'test_import_wallet.json', 'import-pwd',
+         [(0,7),(100,99),(0, 0), (200, 201), (21,41)]
+         ),
+    ])
+def test_wallet_sync_from_scratch(setup_wallets, wallet_structure,
+                                  wallet_file, password, ic):
+    """Simulate a scenario in which we use a new bitcoind, thusly:
+    generate a new wallet and simply pretend that it has an existing
+    index_cache. This will force import of all addresses up to
+    the index_cache values.
+    """
+    setup_import(mainnet=False)
+    wallet = make_wallets(1,[wallet_structure],
+                              fixed_seeds=[wallet_file],
+                              test_wallet=True, passwords=[password])[0]['wallet']
+    sync_count = 0
+    jm_single().bc_interface.wallet_synced = False
+    wallet.index_cache = ic
+    while not jm_single().bc_interface.wallet_synced:
+        wallet.index = []
+        for i in range(5):
+            wallet.index.append([0, 0])
+        jm_single().bc_interface.sync_wallet(wallet)
+        sync_count += 1
+        #avoid infinite loop
+        assert sync_count < 10
+        log.debug("Tried " + str(sync_count) + " times")
+    #after #586 we expect to ALWAYS succeed within 2 rounds
+    assert sync_count == 2
+    #for each external branch, the new index may be higher than
+    #the original index_cache if there was a higher used address
+    expected_wallet_index = []
+    for i, val in enumerate(wallet_structure):
+        if val > wallet.index_cache[i][0]:
+            expected_wallet_index.append([val, wallet.index_cache[i][1]])
+        else:
+            expected_wallet_index.append([wallet.index_cache[i][0],
+                                          wallet.index_cache[i][1]])
+    assert wallet.index == expected_wallet_index
+
+
 @pytest.mark.parametrize(
     "pwd, in_privs",
     [
@@ -196,6 +249,7 @@ def test_utxo_selection(setup_wallets, nw, wallet_structures, mean_amt,
     """
     wallets = make_wallets(nw, wallet_structures, mean_amt, sdev_amt)
     for w in wallets.values():
+        jm_single().bc_interface.wallet_synced = False
         jm_single().bc_interface.sync_wallet(w['wallet'])
     for k, w in enumerate(wallets.values()):
         for algo in [select_gradual, select_greedy, select_greediest, None]: