From aea77653d3b6c851207540332d2f3435ac2efb8f Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 27 Sep 2017 06:32:47 +0930 Subject: [PATCH] onchaind: update bolt #5, and implement failure of timed-out onchain HTLCs. We re-use the value for reasonable_depth given by the master, and we tell it when our timeout transactions reach that depth. Signed-off-by: Rusty Russell --- lightningd/peer_control.c | 23 +++++++++++++ onchaind/onchain.c | 58 +++++++++++++++++++++++--------- onchaind/onchain_wire.csv | 6 +++- tests/test_lightningd.py | 69 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 140 insertions(+), 16 deletions(-) diff --git a/lightningd/peer_control.c b/lightningd/peer_control.c index 993bc9d71..fd1796ac7 100644 --- a/lightningd/peer_control.c +++ b/lightningd/peer_control.c @@ -1282,6 +1282,26 @@ static int handle_missing_htlc_output(struct peer *peer, const u8 *msg) return 0; } +static int handle_onchain_htlc_timeout(struct peer *peer, const u8 *msg) +{ + struct htlc_stub htlc; + + if (!fromwire_onchain_htlc_timeout(msg, NULL, &htlc)) { + log_broken(peer->log, "Invalid onchain_htlc_timeout"); + return -1; + } + + /* BOLT #5: + * + * If the HTLC output has *timed out* and not been *resolved*, the node + * MUST *resolve* the output and MUST fail the corresponding incoming + * HTLC (if any) once the resolving transaction has reached reasonable + * depth. + */ + onchain_failed_our_htlc(peer, &htlc, "timed out"); + return 0; +} + static int onchain_msg(struct subd *sd, const u8 *msg, const int *fds) { enum onchain_wire_type t = fromwire_peektype(msg); @@ -1302,6 +1322,9 @@ static int onchain_msg(struct subd *sd, const u8 *msg, const int *fds) case WIRE_ONCHAIN_MISSING_HTLC_OUTPUT: return handle_missing_htlc_output(sd->peer, msg); + case WIRE_ONCHAIN_HTLC_TIMEOUT: + return handle_onchain_htlc_timeout(sd->peer, msg); + /* We send these, not receive them */ case WIRE_ONCHAIN_INIT: case WIRE_ONCHAIN_SPENT: diff --git a/onchaind/onchain.c b/onchaind/onchain.c index 52917aa40..2e0bd85a5 100644 --- a/onchaind/onchain.c +++ b/onchaind/onchain.c @@ -51,8 +51,8 @@ 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; +/* When to tell master about HTLCs which are missing/timed out */ +static u32 reasonable_depth; /* The messages to send at that depth. */ static u8 **missing_htlc_msgs; @@ -691,7 +691,6 @@ 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; } @@ -749,6 +748,36 @@ static void output_spent(struct tracked_output ***outs, unwatch_tx(tx); } +static void update_resolution_depth(struct tracked_output *out, u32 depth) +{ + status_trace("%s/%s->%s depth %u", + tx_type_name(out->tx_type), + output_type_name(out->output_type), + tx_type_name(out->resolved->tx_type), + depth); + + /* BOLT #5: + * + * If the HTLC output has *timed out* and not been *resolved*, + * the node MUST *resolve* the output and MUST fail the + * corresponding incoming HTLC (if any) once the resolving + * transaction has reached reasonable depth. */ + if (out->resolved->tx_type == OUR_HTLC_TIMEOUT_TX + || out->resolved->tx_type == OUR_HTLC_TIMEOUT_TO_US) { + if (out->resolved->depth < reasonable_depth + && depth >= reasonable_depth) { + u8 *msg; + status_trace("%s/%s reached reasonable depth %u", + tx_type_name(out->tx_type), + output_type_name(out->output_type), + depth); + msg = towire_onchain_htlc_timeout(out, out->htlc); + wire_sync_write(REQ_FD, take(msg)); + } + } + out->resolved->depth = depth; +} + static void tx_new_depth(struct tracked_output **outs, const struct sha256_double *txid, u32 depth) { @@ -756,7 +785,7 @@ static void tx_new_depth(struct tracked_output **outs, /* Special handling for commitment tx reaching depth */ if (structeq(&outs[0]->resolved->txid, txid) - && depth >= htlc_missing_depth + && depth >= reasonable_depth && missing_htlc_msgs) { status_trace("Sending %zu missing htlc messages", tal_count(missing_htlc_msgs)); @@ -770,12 +799,7 @@ static void tx_new_depth(struct tracked_output **outs, /* Is this tx resolving an output? */ if (outs[i]->resolved) { if (structeq(&outs[i]->resolved->txid, txid)) { - status_trace("%s/%s->%s depth %u", - tx_type_name(outs[i]->tx_type), - output_type_name(outs[i]->output_type), - tx_type_name(outs[i]->resolved->tx_type), - depth); - outs[i]->resolved->depth = depth; + update_resolution_depth(outs[i], depth); } continue; } @@ -969,7 +993,9 @@ static void resolve_our_htlc_ourcommit(struct tracked_output *out) * ... * * If the HTLC output has *timed out* and not been *resolved*, the - * node MUST *resolve* the output. If the transaction is the node's + * node MUST *resolve* the output and MUST fail the corresponding + * incoming HTLC (if any) once the resolving transaction has reached + * reasonable depth. If the transaction is the node's * own commitment transaction, it MUST *resolve* the output by * spending it using the HTLC-timeout transaction, and the * HTLC-timeout transaction output MUST be *resolved* as described in @@ -1016,9 +1042,11 @@ static void resolve_our_htlc_theircommit(struct tracked_output *out) * ... * * If the HTLC output has *timed out* and not been *resolved*, the - * node MUST *resolve* the output. If the transaction is the node's - * own commitment transaction, .... Otherwise it MUST resolve the - * output by spending it to a convenient address. + * node MUST *resolve* the output and MUST fail the corresponding + * incoming HTLC (if any) once the resolving transaction has reached + * reasonable depth. If the transaction is the node's own commitment + * transaction, .... Otherwise it MUST resolve the output by spending + * it to a convenient address. */ tx = tx_to_us(out, out, 0, out->htlc->cltv_expiry, NULL, 0, out->wscript, @@ -1884,7 +1912,7 @@ int main(int argc, char *argv[]) &remote_delayed_payment_basepoint, tx, &tx_blockheight, - &htlc_missing_depth, + &reasonable_depth, &remote_htlc_sigs, &num_htlcs)) { master_badmsg(WIRE_ONCHAIN_INIT, msg); diff --git a/onchaind/onchain_wire.csv b/onchaind/onchain_wire.csv index a38a7e3df..1b1de8763 100644 --- a/onchaind/onchain_wire.csv +++ b/onchaind/onchain_wire.csv @@ -26,7 +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,,reasonable_depth,u32 onchain_init,,num_htlc_sigs,u16 onchain_init,,htlc_signature,num_htlc_sigs*secp256k1_ecdsa_signature onchain_init,,num_htlcs,u64 @@ -74,3 +74,7 @@ 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 + +# onchaind->master: this HTLC has timed out (after reasonable_depth) +onchain_htlc_timeout,10 +onchain_htlc_timeout,,htlc,struct htlc_stub diff --git a/tests/test_lightningd.py b/tests/test_lightningd.py index 1bf1c0355..61095a697 100644 --- a/tests/test_lightningd.py +++ b/tests/test_lightningd.py @@ -513,6 +513,75 @@ class LightningDTests(BaseLightningDTests): # Payment failed, BTW assert l2.rpc.listinvoice('onchain_dust_out')[0]['complete'] == False + def test_onchain_timeout(self): + """Onchain handling of outgoing failed htlcs""" + # HTLC 1->2, 1 fails just 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) + + rhash = l2.rpc.invoice(10**8, 'onchain_timeout')['rhash'] + # We underpay, so it fails. + routestep = { + 'msatoshi' : 10**8 - 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') + + # Wait for timeout. + l1.daemon.wait_for_log('Propose handling OUR_UNILATERAL/DELAYED_OUTPUT_TO_US by OUR_DELAYED_RETURN_TO_WALLET .* in 6 blocks') + bitcoind.rpc.generate(6) + + # (l1 will also collect its to-self payment.) + l1.daemon.wait_for_log('sendrawtx exit 0') + l1.daemon.wait_for_log('sendrawtx exit 0') + + # 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: timed out') + + # 91 later, l2 is done. + bitcoind.rpc.generate(91) + 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_timeout')[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']