pay: make sure we don't think payment in progress if it immediately fails.

If send_htlc_out() fails, it doesn't initialize pc->out; that can
make us think it's still in progress.

Reported-by: Jonas Nick
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

lightningd/pay.c

@@ -252,6 +252,7 @@ static void send_payment(struct command *cmd,
 	pc->ids = tal_steal(pc, ids);
 	pc->msatoshi = route[n_hops-1].amount;
 	pc->path_secrets = tal_steal(pc, path_secrets);
+	pc->out = NULL;
 
 	log_info(cmd->ld->log, "Sending %u over %zu hops to deliver %"PRIu64,
 		 route[0].amount, n_hops, pc->msatoshi);

tests/test_lightningd.py

@@ -2442,5 +2442,44 @@ class LightningDTests(BaseLightningDTests):
         l1.daemon.wait_for_log('onchaind complete, forgetting peer')
         l2.daemon.wait_for_log('onchaind complete, forgetting peer')
 
+    @unittest.skipIf(not DEVELOPER, "needs DEVELOPER=1")
+    def test_pay_disconnect(self):
+        """If the remote node has disconnected, we fail payment, but can try again when it reconnects"""
+        l1,l2 = self.connect()
+
+        chanid = self.fund_channel(l1, l2, 10**6)
+
+        # Wait for route propagation.
+        bitcoind.generate_block(5)
+        l1.daemon.wait_for_logs(['Received channel_update for channel {}\(0\)'
+                                 .format(chanid),
+                                'Received channel_update for channel {}\(1\)'
+                                 .format(chanid)])
+
+        inv = l2.rpc.invoice(123000, 'test_pay_disconnect', 'description')['bolt11']
+
+        # Make l2 upset by asking for crazy fee.
+        l1.rpc.dev_setfees('150000')
+
+        # Wait for l1 notice
+        l1.daemon.wait_for_log('STATUS_FAIL_PEER_BAD')
+
+        # Can't pay while its offline.
+        self.assertRaises(ValueError, l1.rpc.pay, inv)
+        l1.daemon.wait_for_log('Failing: first peer not ready: WIRE_TEMPORARY_CHANNEL_FAILURE')
+
+        # Should fail due to temporary channel fail
+        self.assertRaises(ValueError, l1.rpc.pay, inv)
+        l1.daemon.wait_for_log('Failing: first peer not ready: WIRE_TEMPORARY_CHANNEL_FAILURE')
+        assert not l1.daemon.is_in_log('... still in progress')
+
+        # After it sees block, someone should close channel.
+        bitcoind.generate_block(1)
+        l1.daemon.wait_for_log('ONCHAIND_.*_UNILATERAL')
+
+        self.assertRaises(ValueError, l1.rpc.pay, inv)
+        # Could fail with almost anything, but should fail with WIRE_PERMANENT_CHANNEL_FAILURE or unknown route.  FIXME
+        #l1.daemon.wait_for_log('Could not find a route')
+
 if __name__ == '__main__':
     unittest.main(verbosity=2)

wire/gen_onion_wire_csv

@@ -30,8 +30,6 @@ incorrect_cltv_expiry,6,channel_update,len
 expiry_too_soon,UPDATE|14
 expiry_too_soon,0,len,2
 expiry_too_soon,2,channel_update,len
-expiry_too_far,21
-channel_disabled,UPDATE|20
 unknown_payment_hash,PERM|15
 incorrect_payment_amount,PERM|16
 final_expiry_too_soon,17
@@ -39,3 +37,5 @@ final_incorrect_cltv_expiry,18
 final_incorrect_cltv_expiry,0,cltv_expiry,4
 final_incorrect_htlc_amount,19
 final_incorrect_htlc_amount,0,incoming_htlc_amt,4
+channel_disabled,UPDATE|20
+expiry_too_far,21