From 5666ba551f06a5d21d3537cfd978759559405c05 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 27 Sep 2017 06:32:47 +0930 Subject: [PATCH] onchaind: fail htlcs which are not included in commitment tx. As per update https://github.com/lightningnetwork/lightning-rfc/commit/149cf020d6d589b6a30431c7aaee5103569b7dd5 Fixes: #249 Signed-off-by: Rusty Russell --- lightningd/peer_control.c | 63 ++++++++++++++++++++++++++- lightningd/peer_htlcs.c | 47 +++++++++++++++++++++ lightningd/peer_htlcs.h | 5 +++ onchaind/onchain.c | 89 ++++++++++++++++++++++++++++++++++++--- onchaind/onchain_wire.csv | 7 +++ tests/test_lightningd.py | 66 +++++++++++++++++++++++++++++ 6 files changed, 270 insertions(+), 7 deletions(-) diff --git a/lightningd/peer_control.c b/lightningd/peer_control.c index d8bb655fd..993bc9d71 100644 --- a/lightningd/peer_control.c +++ b/lightningd/peer_control.c @@ -1261,6 +1261,27 @@ static int handle_extracted_preimage(struct peer *peer, const u8 *msg) return 0; } +static int handle_missing_htlc_output(struct peer *peer, const u8 *msg) +{ + struct htlc_stub htlc; + + if (!fromwire_onchain_missing_htlc_output(msg, NULL, &htlc)) { + log_broken(peer->log, "Invalid missing_htlc_output"); + return -1; + } + + /* BOLT #5: + * + * For any committed HTLC which does not have an output in this + * commitment transaction, the node MUST fail the corresponding + * incoming HTLC (if any) once the commitment transaction has reached + * reasonable depth, and MAY fail it sooner if no valid commitment + * transaction contains an output corresponding to the HTLC. + */ + onchain_failed_our_htlc(peer, &htlc, "missing in commitment tx"); + return 0; +} + static int onchain_msg(struct subd *sd, const u8 *msg, const int *fds) { enum onchain_wire_type t = fromwire_peektype(msg); @@ -1278,6 +1299,9 @@ static int onchain_msg(struct subd *sd, const u8 *msg, const int *fds) case WIRE_ONCHAIN_EXTRACTED_PREIMAGE: return handle_extracted_preimage(sd->peer, msg); + case WIRE_ONCHAIN_MISSING_HTLC_OUTPUT: + return handle_missing_htlc_output(sd->peer, msg); + /* We send these, not receive them */ case WIRE_ONCHAIN_INIT: case WIRE_ONCHAIN_SPENT: @@ -1300,6 +1324,37 @@ static u8 *p2wpkh_for_keyidx(const tal_t *ctx, struct lightningd *ld, u64 keyidx return scriptpubkey_p2wpkh(ctx, &shutdownkey); } +/* If we want to know if this HTLC is missing, return depth. */ +static bool tell_if_missing(const struct peer *peer, size_t n, + bool *tell_immediate) +{ + struct htlc_out *hout; + + /* Keep valgrind happy. */ + *tell_immediate = false; + + /* Is it a current HTLC? */ + hout = find_htlc_out_by_ripemd(peer, &peer->htlcs[n].ripemd); + if (!hout) + return false; + + /* BOLT #5: + * + * For any committed HTLC which does not have an output in this + * commitment transaction, the node MUST fail the corresponding + * incoming HTLC (if any) once the commitment transaction has reached + * reasonable depth, and MAY fail it sooner if no valid commitment + * transaction contains an output corresponding to the HTLC. + */ + if (hout->hstate >= RCVD_ADD_REVOCATION + && hout->hstate < SENT_REMOVE_REVOCATION) + *tell_immediate = true; + + log_debug(peer->log, "We want to know if htlc %"PRIu64" is missing (%s)", + hout->key.id, *tell_immediate ? "immediate" : "later"); + return true; +} + /* With a reorg, this can get called multiple times; each time we'll kill * onchaind (like any other owner), and restart */ static enum watch_result funding_spent(struct peer *peer, @@ -1374,17 +1429,21 @@ static enum watch_result funding_spent(struct peer *peer, &peer->channel_info->theirbase.delayed_payment, tx, block->height, + /* FIXME: config for 'reasonable depth' */ + 3, peer->last_htlc_sigs, tal_count(peer->htlcs)); subd_send_msg(peer->owner, take(msg)); /* FIXME: Don't queue all at once, use an empty cb... */ for (size_t i = 0; i < tal_count(peer->htlcs); i++) { - msg = towire_onchain_htlc(peer, peer->htlcs+i); + bool tell_immediate; + bool tell = tell_if_missing(peer, i, &tell_immediate); + msg = towire_onchain_htlc(peer, peer->htlcs+i, + tell, tell_immediate); subd_send_msg(peer->owner, take(msg)); } - /* FIXME: Send any known HTLC preimage for live HTLCs! */ watch_tx_and_outputs(peer, tx); /* We keep watching until peer finally deleted, for reorgs. */ diff --git a/lightningd/peer_htlcs.c b/lightningd/peer_htlcs.c index 84c004a5c..1a97a54ef 100644 --- a/lightningd/peer_htlcs.c +++ b/lightningd/peer_htlcs.c @@ -791,6 +791,53 @@ static bool peer_failed_our_htlc(struct peer *peer, return true; } +/* FIXME: Crazy slow! */ +struct htlc_out *find_htlc_out_by_ripemd(const struct peer *peer, + const struct ripemd160 *ripemd) +{ + struct htlc_out_map_iter outi; + struct htlc_out *hout; + + for (hout = htlc_out_map_first(&peer->ld->htlcs_out, &outi); + hout; + hout = htlc_out_map_next(&peer->ld->htlcs_out, &outi)) { + struct ripemd160 hash; + + if (hout->key.peer != peer) + continue; + + ripemd160(&hash, + &hout->payment_hash, sizeof(hout->payment_hash)); + if (structeq(&hash, ripemd)) + return hout; + } + return NULL; +} + +void onchain_failed_our_htlc(const struct peer *peer, + const struct htlc_stub *htlc, + const char *why) +{ + struct htlc_out *hout = find_htlc_out_by_ripemd(peer, &htlc->ripemd); + + /* Don't fail twice! */ + if (hout->failuremsg) + return; + + hout->failuremsg = make_failmsg(hout, hout->msatoshi, + WIRE_PERMANENT_CHANNEL_FAILURE, + NULL); + + if (!hout->in) { + char *localfail = tal_fmt(peer, "%s: %s", + onion_type_name(WIRE_PERMANENT_CHANNEL_FAILURE), + why); + payment_failed(hout->key.peer->ld, hout, localfail); + tal_free(localfail); + } else + local_fail_htlc(hout->in, WIRE_PERMANENT_CHANNEL_FAILURE); +} + static void remove_htlc_in(struct peer *peer, struct htlc_in *hin) { htlc_in_check(hin, __func__); diff --git a/lightningd/peer_htlcs.h b/lightningd/peer_htlcs.h index 389a4d9be..33f976a32 100644 --- a/lightningd/peer_htlcs.h +++ b/lightningd/peer_htlcs.h @@ -41,5 +41,10 @@ enum onion_type send_htlc_out(struct peer *out, u64 amount, u32 cltv, struct pay_command *pc, struct htlc_out **houtp); +struct htlc_out *find_htlc_out_by_ripemd(const struct peer *peer, + const struct ripemd160 *ripemd160); +void onchain_failed_our_htlc(const struct peer *peer, + const struct htlc_stub *htlc, + const char *why); void onchain_fulfilled_htlc(struct peer *peer, const struct preimage *preimage); #endif /* LIGHTNING_LIGHTNINGD_PEER_HTLCS_H */ diff --git a/onchaind/onchain.c b/onchaind/onchain.c index 757a3d846..52917aa40 100644 --- a/onchaind/onchain.c +++ b/onchaind/onchain.c @@ -51,6 +51,12 @@ static struct privkey *revocation_privkey; /* one value is useful for a few witness scripts */ static const u8 ONE = 0x1; +/* When to tell master about HTLCs which aren't in commitment tx */ +static u32 htlc_missing_depth; + +/* The messages to send at that depth. */ +static u8 **missing_htlc_msgs; + /* If we broadcast a tx, or need a delay to resolve the output. */ struct proposed_resolution { /* This can be NULL if our proposal is to simply ignore it after depth */ @@ -685,6 +691,7 @@ static void output_spent(struct tracked_output ***outs, || out->resolved->tx_type == OUR_HTLC_TIMEOUT_TX) resolve_htlc_tx(outs, i, tx, &txid, tx_blockheight); + /* FIXME: Fail timed out htlc after reasonable depth. */ return; } @@ -747,6 +754,18 @@ static void tx_new_depth(struct tracked_output **outs, { size_t i; + /* Special handling for commitment tx reaching depth */ + if (structeq(&outs[0]->resolved->txid, txid) + && depth >= htlc_missing_depth + && missing_htlc_msgs) { + status_trace("Sending %zu missing htlc messages", + tal_count(missing_htlc_msgs)); + for (i = 0; i < tal_count(missing_htlc_msgs); i++) + wire_sync_write(REQ_FD, missing_htlc_msgs[i]); + /* Don't do it again. */ + missing_htlc_msgs = tal_free(missing_htlc_msgs); + } + for (i = 0; i < tal_count(outs); i++) { /* Is this tx resolving an output? */ if (outs[i]->resolved) { @@ -1049,6 +1068,35 @@ static int match_htlc_output(const struct bitcoin_tx *tx, return -1; } +/* Tell master about any we didn't use, if it wants to know. */ +static void note_missing_htlcs(u8 **htlc_scripts, + const struct htlc_stub *htlcs, + const bool *tell_if_missing, + const bool *tell_immediately) +{ + for (size_t i = 0; i < tal_count(htlcs); i++) { + u8 *msg; + + /* Used. */ + if (!htlc_scripts[i]) + continue; + + /* Doesn't care. */ + if (!tell_if_missing[i]) + continue; + + msg = towire_onchain_missing_htlc_output(missing_htlc_msgs, + &htlcs[i]); + if (tell_immediately[i]) + wire_sync_write(REQ_FD, take(msg)); + else { + size_t n = tal_count(missing_htlc_msgs); + tal_resize(&missing_htlc_msgs, n+1); + missing_htlc_msgs[n] = msg; + } + } +} + static void handle_our_unilateral(const struct bitcoin_tx *tx, u32 tx_blockheight, const struct sha256_double *txid, @@ -1060,6 +1108,8 @@ static void handle_our_unilateral(const struct bitcoin_tx *tx, const struct pubkey *local_delayed_payment_basepoint, u64 commit_num, const struct htlc_stub *htlcs, + const bool *tell_if_missing, + const bool *tell_immediately, const secp256k1_ecdsa_signature *remote_htlc_sigs, struct tracked_output **outs) { @@ -1258,12 +1308,16 @@ static void handle_our_unilateral(const struct bitcoin_tx *tx, * Output Handling: Their Offers" below. */ resolve_their_htlc(out); } + /* Each of these consumes one HTLC signature */ remote_htlc_sigs++; /* We've matched this HTLC, can't do again. */ htlc_scripts[j] = NULL; + } + note_missing_htlcs(htlc_scripts, htlcs, + tell_if_missing, tell_immediately); wait_for_resolved(outs); tal_free(tmpctx); } @@ -1336,6 +1390,8 @@ static void handle_their_cheat(const struct bitcoin_tx *tx, const struct pubkey *remote_delayed_payment_basepoint, u64 commit_num, const struct htlc_stub *htlcs, + const bool *tell_if_missing, + const bool *tell_immediately, struct tracked_output **outs) { const tal_t *tmpctx = tal_tmpctx(NULL); @@ -1564,6 +1620,8 @@ static void handle_their_cheat(const struct bitcoin_tx *tx, htlc_scripts[j] = NULL; } + note_missing_htlcs(htlc_scripts, htlcs, + tell_if_missing, tell_immediately); wait_for_resolved(outs); tal_free(tmpctx); } @@ -1580,6 +1638,8 @@ static void handle_their_unilateral(const struct bitcoin_tx *tx, const struct pubkey *remote_delayed_payment_basepoint, u64 commit_num, const struct htlc_stub *htlcs, + const bool *tell_if_missing, + const bool *tell_immediately, struct tracked_output **outs) { const tal_t *tmpctx = tal_tmpctx(NULL); @@ -1759,6 +1819,8 @@ static void handle_their_unilateral(const struct bitcoin_tx *tx, htlc_scripts[j] = NULL; } + note_missing_htlcs(htlc_scripts, htlcs, + tell_if_missing, tell_immediately); wait_for_resolved(outs); tal_free(tmpctx); } @@ -1783,6 +1845,7 @@ int main(int argc, char *argv[]) u64 funding_amount_satoshi, num_htlcs; u8 *scriptpubkey[NUM_SIDES]; struct htlc_stub *htlcs; + bool *tell_if_missing, *tell_immediately; u32 tx_blockheight; if (argc == 2 && streq(argv[1], "--version")) { @@ -1798,6 +1861,8 @@ int main(int argc, char *argv[]) | SECP256K1_CONTEXT_SIGN); status_setup_sync(REQ_FD); + missing_htlc_msgs = tal_arr(ctx, u8 *, 0); + msg = wire_sync_read(ctx, REQ_FD); tx = tal(ctx, struct bitcoin_tx); if (!fromwire_onchain_init(ctx, msg, NULL, @@ -1819,6 +1884,7 @@ int main(int argc, char *argv[]) &remote_delayed_payment_basepoint, tx, &tx_blockheight, + &htlc_missing_depth, &remote_htlc_sigs, &num_htlcs)) { master_badmsg(WIRE_ONCHAIN_INIT, msg); @@ -1828,13 +1894,17 @@ int main(int argc, char *argv[]) /* FIXME: Filter as we go, don't load them all into mem! */ htlcs = tal_arr(ctx, struct htlc_stub, num_htlcs); - if (!htlcs) + tell_if_missing = tal_arr(ctx, bool, num_htlcs); + tell_immediately = tal_arr(ctx, bool, num_htlcs); + if (!htlcs || !tell_if_missing || !tell_immediately) status_failed(STATUS_FAIL_INTERNAL_ERROR, "Can't allocate %"PRIu64" htlcs", num_htlcs); for (u64 i = 0; i < num_htlcs; i++) { msg = wire_sync_read(ctx, REQ_FD); - if (!fromwire_onchain_htlc(msg, NULL, &htlcs[i])) + if (!fromwire_onchain_htlc(msg, NULL, &htlcs[i], + &tell_if_missing[i], + &tell_immediately[i])) master_badmsg(WIRE_ONCHAIN_HTLC, msg); } @@ -1892,6 +1962,7 @@ int main(int argc, char *argv[]) &basepoints.delayed_payment, commit_num, htlcs, + tell_if_missing, tell_immediately, remote_htlc_sigs, outs); /* BOLT #5: @@ -1913,7 +1984,9 @@ int main(int argc, char *argv[]) &remote_payment_basepoint, &remote_delayed_payment_basepoint, commit_num, - htlcs, outs); + htlcs, + tell_if_missing, tell_immediately, + outs); /* BOLT #5: * * Note that there can be more than one valid, @@ -1933,7 +2006,10 @@ int main(int argc, char *argv[]) &remote_payment_basepoint, &remote_delayed_payment_basepoint, commit_num, - htlcs, outs); + htlcs, + tell_if_missing, + tell_immediately, + outs); } else if (commit_num == revocations_received(&shachain) + 1) { status_trace("Their unilateral tx, new commit point"); handle_their_unilateral(tx, tx_blockheight, @@ -1944,7 +2020,10 @@ int main(int argc, char *argv[]) &remote_payment_basepoint, &remote_delayed_payment_basepoint, commit_num, - htlcs, outs); + htlcs, + tell_if_missing, + tell_immediately, + outs); } else status_failed(STATUS_FAIL_INTERNAL_ERROR, "Unknown commitment index %"PRIu64 diff --git a/onchaind/onchain_wire.csv b/onchaind/onchain_wire.csv index 33c141821..a38a7e3df 100644 --- a/onchaind/onchain_wire.csv +++ b/onchaind/onchain_wire.csv @@ -26,6 +26,7 @@ onchain_init,,remote_payment_basepoint,struct pubkey onchain_init,,remote_delayed_payment_basepoint,struct pubkey onchain_init,,tx,struct bitcoin_tx onchain_init,,tx_blockheight,u32 +onchain_init,,htlc_missing_depth,u32 onchain_init,,num_htlc_sigs,u16 onchain_init,,htlc_signature,num_htlc_sigs*secp256k1_ecdsa_signature onchain_init,,num_htlcs,u64 @@ -34,6 +35,9 @@ onchain_init,,num_htlcs,u64 # This is all the HTLCs: one per message onchain_htlc,2 onchain_htlc,,htlc,struct htlc_stub +# If it's not in the commitment tx, tell us (immediately or htlc_missing_depth) +onchain_htlc,,tell_if_missing,bool +onchain_htlc,,tell_immediately,bool # This sets what the state is, depending on tx. onchain_init_reply,101 @@ -67,3 +71,6 @@ onchain_known_preimage,,preimage,struct preimage onchain_extracted_preimage,8 onchain_extracted_preimage,,preimage,struct preimage +# onchaind->master: this HTLC was missing from commit tx. +onchain_missing_htlc_output,9 +onchain_missing_htlc_output,,htlc,struct htlc_stub diff --git a/tests/test_lightningd.py b/tests/test_lightningd.py index 833ee2db1..1bf1c0355 100644 --- a/tests/test_lightningd.py +++ b/tests/test_lightningd.py @@ -447,6 +447,72 @@ class LightningDTests(BaseLightningDTests): bitcoind.rpc.generate(6) l2.daemon.wait_for_log('onchaind complete, forgetting peer') + def test_onchain_dust_out(self): + """Onchain handling of outgoing dust htlcs (they should fail)""" + # HTLC 1->2, 1 fails after it's irrevocably committed + disconnects = ['@WIRE_REVOKE_AND_ACK', 'permfail'] + l1 = self.node_factory.get_node(disconnect=disconnects) + l2 = self.node_factory.get_node() + + l1.rpc.connect('localhost', l2.info['port'], l2.info['id']) + self.fund_channel(l1, l2, 10**6) + + # Must be dust! + rhash = l2.rpc.invoice(1, 'onchain_dust_out')['rhash'] + routestep = { + 'msatoshi' : 1, + 'id' : l2.info['id'], + 'delay' : 5, + 'channel': '1:1:1' + } + + q = queue.Queue() + + def try_pay(): + try: + l1.rpc.sendpay(to_json([routestep]), rhash, async=False) + q.put(None) + except Exception as err: + q.put(err) + + t = threading.Thread(target=try_pay) + t.daemon = True + t.start() + + # l1 will drop to chain. + l1.daemon.wait_for_log('permfail') + l1.daemon.wait_for_log('sendrawtx exit 0') + l1.bitcoin.rpc.generate(1) + l1.daemon.wait_for_log('-> ONCHAIND_OUR_UNILATERAL') + l2.daemon.wait_for_log('-> ONCHAIND_THEIR_UNILATERAL') + + # We use 3 blocks for "reasonable depth" + bitcoind.rpc.generate(3) + + # It should fail. + err = q.get(timeout = 5) + assert type(err) is ValueError + t.join(timeout=1) + assert not t.isAlive() + + l1.daemon.wait_for_log('WIRE_PERMANENT_CHANNEL_FAILURE: missing in commitment tx') + + # 6 later, l1 should collect its to-self payment. + bitcoind.rpc.generate(6) + l1.daemon.wait_for_log('Broadcasting OUR_DELAYED_RETURN_TO_WALLET .* to resolve OUR_UNILATERAL/DELAYED_OUTPUT_TO_US') + l1.daemon.wait_for_log('sendrawtx exit 0') + + # 94 later, l2 is done. + bitcoind.rpc.generate(94) + l2.daemon.wait_for_log('onchaind complete, forgetting peer') + + # Now, 100 blocks and l1 should be done. + bitcoind.rpc.generate(6) + l1.daemon.wait_for_log('onchaind complete, forgetting peer') + + # Payment failed, BTW + assert l2.rpc.listinvoice('onchain_dust_out')[0]['complete'] == False + def test_onchain_middleman(self): # HTLC 1->2->3, 1->2 goes down after 2 gets preimage from 3. disconnects = ['-WIRE_UPDATE_FULFILL_HTLC', 'permfail']