From 1a4084442b2b58deb542ee916527c932c3589af3 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Fri, 17 Aug 2018 14:36:36 +0930 Subject: [PATCH] onchaind: use a point-of-last-resort if we see an unknown transaction. This may have been supplied by the peer if it's nice and supports option_data_loss_protect. Signed-off-by: Rusty Russell --- lightningd/onchain_control.c | 3 +- onchaind/onchain.c | 109 +++++++++++++++++++++++++++--- onchaind/onchain_types.h | 3 + onchaind/onchain_wire.csv | 1 + onchaind/test/run-grind_feerate.c | 2 +- tests/test_connection.py | 14 +++- 6 files changed, 120 insertions(+), 12 deletions(-) diff --git a/lightningd/onchain_control.c b/lightningd/onchain_control.c index 35b5b0812..0c277e759 100644 --- a/lightningd/onchain_control.c +++ b/lightningd/onchain_control.c @@ -467,7 +467,8 @@ enum watch_result onchaind_funding_spent(struct channel *channel, channel->last_htlc_sigs, tal_count(stubs), channel->min_possible_feerate, - channel->max_possible_feerate); + channel->max_possible_feerate, + channel->future_per_commitment_point); subd_send_msg(channel->owner, take(msg)); /* FIXME: Don't queue all at once, use an empty cb... */ diff --git a/onchaind/onchain.c b/onchaind/onchain.c index 6ad04e3b8..20c6fd8c0 100644 --- a/onchaind/onchain.c +++ b/onchaind/onchain.c @@ -2117,6 +2117,94 @@ static void handle_their_unilateral(const struct bitcoin_tx *tx, wait_for_resolved(outs); } +static void handle_unknown_commitment(const struct bitcoin_tx *tx, + u32 tx_blockheight, + u64 commit_num, + const struct bitcoin_txid *txid, + const struct pubkey *possible_remote_per_commitment_point, + const struct basepoints basepoints[NUM_SIDES], + const struct htlc_stub *htlcs, + const bool *tell_if_missing, + struct tracked_output **outs) +{ + struct keyset *ks; + int to_us_output = -1; + u8 *local_script; + + resolved_by_other(outs[0], txid, UNKNOWN_UNILATERAL); + + if (!possible_remote_per_commitment_point) + goto search_done; + + keyset = ks = tal(tx, struct keyset); + if (!derive_keyset(possible_remote_per_commitment_point, + &basepoints[REMOTE], + &basepoints[LOCAL], + ks)) + status_failed(STATUS_FAIL_INTERNAL_ERROR, + "Deriving keyset for possible_remote_per_commitment_point %s", + type_to_string(tmpctx, struct pubkey, + possible_remote_per_commitment_point)); + + local_script = scriptpubkey_p2wpkh(tmpctx, &keyset->other_payment_key); + for (size_t i = 0; i < tal_count(tx->output); i++) { + struct tracked_output *out; + + if (local_script + && scripteq(tx->output[i].script, local_script)) { + /* BOLT #5: + * + * - MAY take no action in regard to the associated + * `to_remote`, which is simply a P2WPKH output to + * the *local node*. + * - Note: `to_remote` is considered *resolved* by the + * commitment transaction itself. + */ + out = new_tracked_output(&outs, txid, tx_blockheight, + UNKNOWN_UNILATERAL, + i, tx->output[i].amount, + OUTPUT_TO_US, NULL, NULL, NULL); + ignore_output(out); + local_script = NULL; + + /* Tell the master that it will want to add + * this UTXO to its outputs */ + wire_sync_write(REQ_FD, towire_onchain_add_utxo( + tmpctx, txid, i, + possible_remote_per_commitment_point, + tx->output[i].amount, + tx_blockheight)); + to_us_output = i; + continue; + } + } + +search_done: + if (to_us_output == -1) { + status_broken("FUNDS LOST. Unknown commitment #%"PRIu64"!", + commit_num); + init_reply("ERROR: FUNDS LOST. Unknown commitment!"); + } else { + status_broken("ERROR: Unknown commitment #%"PRIu64 + ", recovering our funds!", + commit_num); + init_reply("ERROR: Unknown commitment, recovering our funds!"); + } + + /* Tell master to give up on HTLCs immediately. */ + for (size_t i = 0; i < tal_count(htlcs); i++) { + u8 *msg; + + if (!tell_if_missing[i]) + continue; + + msg = towire_onchain_missing_htlc_output(NULL, &htlcs[i]); + wire_sync_write(REQ_FD, take(msg)); + } + + wait_for_resolved(outs); +} + int main(int argc, char *argv[]) { setup_locale(); @@ -2136,6 +2224,7 @@ int main(int argc, char *argv[]) struct htlc_stub *htlcs; bool *tell_if_missing, *tell_immediately; u32 tx_blockheight; + struct pubkey *possible_remote_per_commitment_point; subdaemon_setup(argc, argv); @@ -2166,7 +2255,8 @@ int main(int argc, char *argv[]) &remote_htlc_sigs, &num_htlcs, &min_possible_feerate, - &max_possible_feerate)) { + &max_possible_feerate, + &possible_remote_per_commitment_point)) { master_badmsg(WIRE_ONCHAIN_INIT, msg); } @@ -2284,13 +2374,16 @@ int main(int argc, char *argv[]) tell_if_missing, tell_immediately, outs); - } else - status_failed(STATUS_FAIL_INTERNAL_ERROR, - "Unknown commitment index %"PRIu64 - " for tx %s", - commit_num, - type_to_string(tmpctx, struct bitcoin_tx, - tx)); + } else { + handle_unknown_commitment(tx, tx_blockheight, + commit_num, + &txid, + possible_remote_per_commitment_point, + basepoints, + htlcs, + tell_if_missing, + outs); + } } /* We're done! */ diff --git a/onchaind/onchain_types.h b/onchaind/onchain_types.h index 58f8ba750..fb54728fb 100644 --- a/onchaind/onchain_types.h +++ b/onchaind/onchain_types.h @@ -13,6 +13,9 @@ enum tx_type { /* Their unilateral: spends funding */ THEIR_UNILATERAL, + /* Unknown unilateral (presumably theirs): spends funding */ + UNKNOWN_UNILATERAL, + /* Our unilateral: spends funding */ OUR_UNILATERAL, diff --git a/onchaind/onchain_wire.csv b/onchaind/onchain_wire.csv index 39b6b9c9a..f844eaea5 100644 --- a/onchaind/onchain_wire.csv +++ b/onchaind/onchain_wire.csv @@ -32,6 +32,7 @@ onchain_init,,htlc_signature,num_htlc_sigs*secp256k1_ecdsa_signature onchain_init,,num_htlcs,u64 onchain_init,,min_possible_feerate,u32 onchain_init,,max_possible_feerate,u32 +onchain_init,,possible_remote_per_commit_point,?struct pubkey #include # This is all the HTLCs: one per message diff --git a/onchaind/test/run-grind_feerate.c b/onchaind/test/run-grind_feerate.c index 09e551ac1..49072ffd8 100644 --- a/onchaind/test/run-grind_feerate.c +++ b/onchaind/test/run-grind_feerate.c @@ -37,7 +37,7 @@ bool fromwire_onchain_depth(const void *p UNNEEDED, struct bitcoin_txid *txid UN bool fromwire_onchain_htlc(const void *p UNNEEDED, struct htlc_stub *htlc UNNEEDED, bool *tell_if_missing UNNEEDED, bool *tell_immediately UNNEEDED) { fprintf(stderr, "fromwire_onchain_htlc called!\n"); abort(); } /* Generated stub for fromwire_onchain_init */ -bool fromwire_onchain_init(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, struct shachain *shachain UNNEEDED, u64 *funding_amount_satoshi UNNEEDED, struct pubkey *old_remote_per_commitment_point UNNEEDED, struct pubkey *remote_per_commitment_point UNNEEDED, u32 *local_to_self_delay UNNEEDED, u32 *remote_to_self_delay UNNEEDED, u32 *feerate_per_kw UNNEEDED, u64 *local_dust_limit_satoshi UNNEEDED, struct bitcoin_txid *our_broadcast_txid UNNEEDED, u8 **local_scriptpubkey UNNEEDED, u8 **remote_scriptpubkey UNNEEDED, struct pubkey *ourwallet_pubkey UNNEEDED, enum side *funder UNNEEDED, struct basepoints *local_basepoints UNNEEDED, struct basepoints *remote_basepoints UNNEEDED, struct bitcoin_tx **tx UNNEEDED, u32 *tx_blockheight UNNEEDED, u32 *reasonable_depth UNNEEDED, secp256k1_ecdsa_signature **htlc_signature UNNEEDED, u64 *num_htlcs UNNEEDED, u32 *min_possible_feerate UNNEEDED, u32 *max_possible_feerate UNNEEDED) +bool fromwire_onchain_init(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, struct shachain *shachain UNNEEDED, u64 *funding_amount_satoshi UNNEEDED, struct pubkey *old_remote_per_commitment_point UNNEEDED, struct pubkey *remote_per_commitment_point UNNEEDED, u32 *local_to_self_delay UNNEEDED, u32 *remote_to_self_delay UNNEEDED, u32 *feerate_per_kw UNNEEDED, u64 *local_dust_limit_satoshi UNNEEDED, struct bitcoin_txid *our_broadcast_txid UNNEEDED, u8 **local_scriptpubkey UNNEEDED, u8 **remote_scriptpubkey UNNEEDED, struct pubkey *ourwallet_pubkey UNNEEDED, enum side *funder UNNEEDED, struct basepoints *local_basepoints UNNEEDED, struct basepoints *remote_basepoints UNNEEDED, struct bitcoin_tx **tx UNNEEDED, u32 *tx_blockheight UNNEEDED, u32 *reasonable_depth UNNEEDED, secp256k1_ecdsa_signature **htlc_signature UNNEEDED, u64 *num_htlcs UNNEEDED, u32 *min_possible_feerate UNNEEDED, u32 *max_possible_feerate UNNEEDED, struct pubkey **possible_remote_per_commit_point UNNEEDED) { fprintf(stderr, "fromwire_onchain_init called!\n"); abort(); } /* Generated stub for fromwire_onchain_known_preimage */ bool fromwire_onchain_known_preimage(const void *p UNNEEDED, struct preimage *preimage UNNEEDED) diff --git a/tests/test_connection.py b/tests/test_connection.py index d8b7165a7..22a120e68 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -1141,7 +1141,7 @@ def test_funder_feerate_reconnect(node_factory, bitcoind): @unittest.skipIf(not DEVELOPER, "needs LIGHTNINGD_DEV_LOG_IO") -def test_dataloss_protection(node_factory): +def test_dataloss_protection(node_factory, bitcoind): l1 = node_factory.get_node(may_reconnect=True, log_all_io=True) l2 = node_factory.get_node(may_reconnect=True, log_all_io=True) @@ -1214,4 +1214,14 @@ def test_dataloss_protection(node_factory): l2.daemon.wait_for_log("Cannot broadcast our commitment tx: they have a future one") assert not l2.daemon.is_in_log('sendrawtx exit 0') - # FIXME: l2 should still be able to collect onchain. + closetxid = only_one(bitcoind.rpc.getrawmempool(False)) + + # l2 should still recover something! + bitcoind.generate_block(1) + + l2.daemon.wait_for_log("ERROR: Unknown commitment #2, recovering our funds!") + bitcoind.generate_block(100) + l2.daemon.wait_for_log('WIRE_ONCHAIN_ALL_IRREVOCABLY_RESOLVED') + + # l2 should have it in wallet. + assert (closetxid, "confirmed") in set([(o['txid'], o['status']) for o in l2.rpc.listfunds()['outputs']])