From 1cf33eefe2626ab3bcdfb200b6700ff634ce8826 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 30 Aug 2017 10:51:48 +0930 Subject: [PATCH] lightningd: handle case where channeld fails locally-generated HTLC. jl777 reported a crash when we try to pay past reserve. Fix that (and a whole class of related bugs) and add tests. In test_lightning.py I had to make non-async path for sendpay() non-threaded to get the exception passed through for testing. Closes: #236 Signed-off-by: Rusty Russell --- channeld/full_channel.c | 5 +++++ lightningd/peer_htlcs.c | 9 ++++++++- tests/test_lightningd.py | 36 +++++++++++++++++++++++++++++++----- 3 files changed, 44 insertions(+), 6 deletions(-) diff --git a/channeld/full_channel.c b/channeld/full_channel.c index 677f074d4..c8716262a 100644 --- a/channeld/full_channel.c +++ b/channeld/full_channel.c @@ -442,6 +442,11 @@ static enum channel_add_err add_htlc(struct channel *channel, * sender's reserve. */ if (enforce_aggregate_limits && balance_msat - fee_msat < (s64)channel_reserve_msat(channel, sender)) { + status_trace("balance = %"PRIu64 + ", fee is %"PRIu64 + ", reserve is %"PRIu64, + balance_msat, fee_msat, + channel_reserve_msat(channel, sender)); e = CHANNEL_ERR_CHANNEL_CAPACITY_EXCEEDED; goto out; } diff --git a/lightningd/peer_htlcs.c b/lightningd/peer_htlcs.c index 28f1a2f27..f793251cf 100644 --- a/lightningd/peer_htlcs.c +++ b/lightningd/peer_htlcs.c @@ -413,7 +413,14 @@ static bool rcvd_htlc_reply(struct subd *subd, const u8 *msg, const int *fds, } if (failure_code) { - local_fail_htlc(hout->in, failure_code); + if (!hout->in) { + char *localfail = tal_fmt(msg, "%s: %.*s", + onion_type_name(failure_code), + (int)tal_len(failurestr), + (const char *)failurestr); + payment_failed(hout->key.peer->ld, hout, localfail); + } else + local_fail_htlc(hout->in, failure_code); return true; } diff --git a/tests/test_lightningd.py b/tests/test_lightningd.py index 53414ef3f..9f0928d86 100644 --- a/tests/test_lightningd.py +++ b/tests/test_lightningd.py @@ -208,10 +208,6 @@ class LightningDTests(BaseLightningDTests): } lsrc.rpc.sendpay(to_json([routestep]), rhash, async=False) - t = threading.Thread(target=call_pay) - t.daemon = True - t.start() - def wait_pay(): # Up to 10 seconds for payment to succeed. start_time = time.time() @@ -221,9 +217,12 @@ class LightningDTests(BaseLightningDTests): time.sleep(0.1) if async: + t = threading.Thread(target=call_pay) + t.daemon = True + t.start() return self.executor.submit(wait_pay) else: - return wait_pay() + call_pay() def test_connect(self): l1,l2 = self.connect() @@ -329,6 +328,33 @@ class LightningDTests(BaseLightningDTests): l1.rpc.sendpay(to_json([routestep]), rhash) assert l2.rpc.listinvoice('testpayment3')[0]['complete'] == True + def test_sendpay_cant_afford(self): + l1,l2 = self.connect() + + # Note, this is in SATOSHI, rest are in MILLISATOSHI! + self.fund_channel(l1, l2, 10**6) + + # Can't pay more than channel capacity. + self.assertRaises(ValueError, self.pay, l1, l2, 10**9 + 1) + + # This is the fee, which needs to be taken into account for l1. + available = 10**9 - 13440 + # Reserve is 1%. + reserve = 10**7 + + # Can't pay past reserve. + self.assertRaises(ValueError, self.pay, l1, l2, available) + self.assertRaises(ValueError, self.pay, l1, l2, available - reserve + 1) + + # Can pay up to reserve (1%) + self.pay(l1, l2, available - reserve) + + # And now it can't pay back, due to its own reserve. + self.assertRaises(ValueError, self.pay, l2, l1, available - reserve) + + # But this should work. + self.pay(l2, l1, available - reserve*2) + def test_closing(self): l1,l2 = self.connect()