Add timeouts for tx watchers for both broadcast and

confirmation as per TIMEOUT section in config.
Mark wallet as synced after sync_unspent in Core
interface (to match Electrum interface, and wait for
this update before modifying orders).
To correspond, allow fast sync in test setup in several tests.
Disallow sync_unspent in syncing wallet full mode if sync_addresses fails.
This prevents the wallet erroneously being marked as synced in full mode.
This change happened as a result of the new electrum wallet sync; because
this is asynchronous, the call to sync_unspent in the client protocol does
not block, thus must be flagged when complete, this is done with
bci.wallet_synced set to True at end of sync_unspent. But this leads
to the wallet as always being marked synced using full sync_mode if we
call both functions unconditionally.
These wallet sync conditions have become unwieldy; a refactor is in order.
Some small extra documentation in docs/TODO.md.

README.md

@@ -73,4 +73,4 @@ The "server" is just a daemon service that can be run as a separate process (see
 
 ### TESTING
 
-Instructions for developers for testing [here](docs/TESTING.md).
+Instructions for developers for testing [here](docs/TESTING.md). If you want to help improve the project, please have a read of [this todo list](docs/TODO.md).

docs/TODO.md

@@ -23,6 +23,7 @@ like `commitmentlist`.
 ### Architecture
 
 * Probably move all user data to ~ ; see [comment](https://github.com/JoinMarket-Org/joinmarket-clientserver/issues/62#issuecomment-318890399).
+* Make use of the schedule design to fold together sendpayment and tumbler (they share a lot of cli options anyway).
 * Investigate what refactoring of the daemon protocol is necessary so it is possible to run protocol instances concurrently.
 * Moving elements shared into joinmarketbase - in particular, an object representing offers like `JMOffer`, which
 could have serialization routines for passing between client and daemon.
@@ -32,7 +33,7 @@ concern (there is already no bitcoin security concern even without it).
 ### Blockchain
 
 * Investigate adding SPV mode inherited from work on Bitcoin Core
-* Re-work the existing electrum code so it works reliably and with some decent performance (easier short term goal).
+* ~~Re-work the existing electrum code so it works reliably and with some decent performance (easier short term goal).~~ (Done)
 
 ### Joinmarket protocol
 

jmclient/jmclient/blockchaininterface.py

@@ -47,8 +47,13 @@ class BlockchainInterface(object):
         pass
 
     def sync_wallet(self, wallet, restart_cb=None):
+        """Default behaviour is for Core and similar interfaces.
+	If address sync fails, flagged with wallet_synced value;
+	do not attempt to sync_unspent in that case.
+	"""
         self.sync_addresses(wallet, restart_cb)
-        self.sync_unspent(wallet)
+        if self.wallet_synced:
+            self.sync_unspent(wallet)
 
     @staticmethod
     def get_wallet_name(wallet):
@@ -113,20 +118,38 @@ class BlockchainInterface(object):
         self.tx_watcher_loops[loopkey] = [loop, False, False, False]
         #Hardcoded polling interval, but in any case it can be very short.
         loop.start(5.0)
-        #TODO Hardcoded very long timeout interval
-        reactor.callLater(7200, self.tx_timeout, txd, loopkey, timeoutfun)
+        #Give up on un-broadcast transactions and broadcast but not confirmed
+        #transactions as per settings in the config.
+        reactor.callLater(float(jm_single().config.get("TIMEOUT",
+                    "unconfirm_timeout_sec")), self.tx_network_timeout, loopkey)
+        confirm_timeout_sec = int(jm_single().config.get(
+            "TIMEOUT", "confirm_timeout_hours")) * 3600
+        reactor.callLater(confirm_timeout_sec, self.tx_timeout, txd, loopkey, timeoutfun)
+
+    def tx_network_timeout(self, loopkey):
+        """If unconfirm has not been called by the time this
+	is triggered, we abandon monitoring, assuming the tx has
+	not been broadcast.
+	"""
+        if not self.tx_watcher_loops[loopkey][1]:
+            log.info("Abandoning monitoring of un-broadcast tx for: " + str(loopkey))
+            if self.tx_watcher_loops[loopkey][0].running:
+                self.tx_watcher_loops[loopkey][0].stop()
 
     def tx_timeout(self, txd, loopkey, timeoutfun):
-        #TODO: 'loopkey' is an address not a txid for Makers, handle that.
-        if not timeoutfun:
-            return
-        if not txid in self.tx_watcher_loops:
+        """Assuming we are watching for an already-broadcast
+	transaction, give up once this triggers if confirmation has not occurred.
+	"""
+        if not loopkey in self.tx_watcher_loops:
+            #Occurs if the tx has already confirmed before this
             return
-        if not self.tx_watcher_loops[loopkey][1]:
-            #Not confirmed after 2 hours; give up
+        if not self.tx_watcher_loops[loopkey][2]:
+            #Not confirmed after prescribed timeout in hours; give up
             log.info("Timed out waiting for confirmation of: " + str(loopkey))
-            self.tx_watcher_loops[loopkey][0].stop()
-            timeoutfun(txd, loopkey)
+            if self.tx_watcher_loops[loopkey][0].running:
+                self.tx_watcher_loops[loopkey][0].stop()
+            if timeoutfun:
+                timeoutfun(txd, loopkey)
 
     @abc.abstractmethod
     def outputs_watcher(self, wallet_name, notifyaddr, tx_output_set,
@@ -624,6 +647,7 @@ class BitcoinCoreInterface(BlockchainInterface):
             }
         et = time.time()
         log.debug('bitcoind sync_unspent took ' + str((et - st)) + 'sec')
+        self.wallet_synced = True
 
     def get_deser_from_gettransaction(self, rpcretval):
         """Get full transaction deserialization from a call

jmclient/test/test_tx_creation.py

@@ -36,7 +36,7 @@ def test_create_p2sh_output_tx(setup_tx_creation, nw, wallet_structures,
                                mean_amt, sdev_amt, amount, pubs, k):
     wallets = make_wallets(nw, wallet_structures, mean_amt, sdev_amt)
     for w in wallets.values():
-        sync_wallet(w['wallet'])
+        sync_wallet(w['wallet'], fast=True)
     for k, w in enumerate(wallets.values()):
         wallet = w['wallet']
         ins_full = wallet.select_utxos(0, amount)
@@ -89,7 +89,7 @@ def test_all_same_priv(setup_tx_creation):
     #make another utxo on the same address
     addrinwallet = wallet.get_addr(0,0,0)
     jm_single().bc_interface.grab_coins(addrinwallet, 1)
-    sync_wallet(wallet)
+    sync_wallet(wallet, fast=True)
     insfull = wallet.select_utxos(0, 110000000)
     outs = [{"address": addr, "value": 1000000}]
     ins = insfull.keys()
@@ -106,7 +106,7 @@ def test_verify_tx_input(setup_tx_creation, signall, mktxlist):
     priv = "aa"*32 + "01"
     addr = bitcoin.privkey_to_address(priv, magicbyte=get_p2pk_vbyte())
     wallet = make_wallets(1, [[2,0,0,0,0]], 1)[0]['wallet']
-    sync_wallet(wallet)
+    sync_wallet(wallet, fast=True)
     insfull = wallet.select_utxos(0, 110000000)
     print(insfull)    
     if not mktxlist:
@@ -159,7 +159,7 @@ def test_absurd_fees(setup_tx_creation):
     jm_single().bc_interface.absurd_fees = True
     #pay into it
     wallet = make_wallets(1, [[2, 0, 0, 0, 1]], 3)[0]['wallet']
-    sync_wallet(wallet)
+    sync_wallet(wallet, fast=True)
     amount = 350000000
     ins_full = wallet.select_utxos(0, amount)
     with pytest.raises(ValueError) as e_info:
@@ -170,7 +170,7 @@ def test_create_sighash_txs(setup_tx_creation):
     for sighash in [bitcoin.SIGHASH_ANYONECANPAY + bitcoin.SIGHASH_SINGLE,
                     bitcoin.SIGHASH_NONE, bitcoin.SIGHASH_SINGLE]:
         wallet = make_wallets(1, [[2, 0, 0, 0, 1]], 3)[0]['wallet']
-        sync_wallet(wallet)
+        sync_wallet(wallet, fast=True)
         amount = 350000000
         ins_full = wallet.select_utxos(0, amount)
         print "using hashcode: " + str(sighash)
@@ -199,7 +199,7 @@ def test_spend_p2sh_utxos(setup_tx_creation):
     msig_addr = bitcoin.scriptaddr(script, magicbyte=196)
     #pay into it
     wallet = make_wallets(1, [[2, 0, 0, 0, 1]], 3)[0]['wallet']
-    sync_wallet(wallet)
+    sync_wallet(wallet, fast=True)
     amount = 350000000
     ins_full = wallet.select_utxos(0, amount)
     txid = make_sign_and_push(ins_full, wallet, amount, output_addr=msig_addr)

jmclient/test/test_wallets.py

@@ -54,7 +54,7 @@ def test_query_utxo_set(setup_wallets):
     wallet = create_wallet_for_sync("wallet4utxo.json", "4utxo",
                                     [2, 3, 0, 0, 0],
                                     ["wallet4utxo.json", "4utxo", [2, 3]])
-    sync_wallet(wallet)
+    sync_wallet(wallet, fast=True)
     txid = do_tx(wallet, 90000000)
     txid2 = do_tx(wallet, 20000000)
     print("Got txs: ", txid, txid2)

test/test_segwit.py

@@ -128,7 +128,7 @@ def test_spend_p2sh_p2wpkh_multi(setup_segwit, wallet_structure, in_amt, amount,
     other_ins is a list of input indices (where to place the funding non-sw utxos)
     """
     wallet = make_wallets(1, wallet_structure, in_amt, walletclass=Wallet)[0]['wallet']
-    jm_single().bc_interface.sync_wallet(wallet)
+    jm_single().bc_interface.sync_wallet(wallet, fast=True)
     other_ins = {}
     ctr = 0
     for k, v in wallet.unspent.iteritems():

test/ygrunner.py

@@ -64,7 +64,7 @@ def test_start_ygs(setup_ygrunner, num_ygs, wallet_structures, mean_amt,
     wallet = wallets[num_ygs]['wallet']
     print("Seed : " + wallets[num_ygs]['seed'])
     #useful to see the utxos on screen sometimes
-    sync_wallet(wallet)
+    sync_wallet(wallet, fast=True)
     print(wallet.unspent)
     txfee = 1000
     cjfee_a = 4200
@@ -75,7 +75,7 @@ def test_start_ygs(setup_ygrunner, num_ygs, wallet_structures, mean_amt,
     for i in range(num_ygs):
         
         cfg = [txfee, cjfee_a, cjfee_r, ordertype, minsize]
-        sync_wallet(wallets[i]["wallet"])
+        sync_wallet(wallets[i]["wallet"], fast=True)
         yg = ygclass(wallets[i]["wallet"], cfg)
         if malicious:
             yg.set_maliciousness(malicious)