From 477a5298569bafe456d468d04b0f8ff790478a9c Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 19 Dec 2017 10:28:15 +1030 Subject: [PATCH] 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 --- lightningd/pay.c | 1 + tests/test_lightningd.py | 39 +++++++++++++++++++++++++++++++++++++++ wire/gen_onion_wire_csv | 4 ++-- 3 files changed, 42 insertions(+), 2 deletions(-) diff --git a/lightningd/pay.c b/lightningd/pay.c index 3d26f5b85..851f259ba 100644 --- a/lightningd/pay.c +++ b/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); diff --git a/tests/test_lightningd.py b/tests/test_lightningd.py index bc50f427b..4b99e3bc4 100644 --- a/tests/test_lightningd.py +++ b/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) diff --git a/wire/gen_onion_wire_csv b/wire/gen_onion_wire_csv index f5d07c9bd..bccb2e647 100644 --- a/wire/gen_onion_wire_csv +++ b/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