From ee3480e56bde9127f2587f762d47c0837959def7 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 10 Sep 2019 11:55:27 +0930 Subject: [PATCH] derive_keyset: don't rotate key for remote iff option_static_remotekey. The largest change is inside hsmd: it hands a null per-commitment key to the wallet to tell it to spend the to_remote output. It can also now resolve unknown commitments, even if it doesn't have a possible_remote_per_commitment_point from the peer. Signed-off-by: Rusty Russell --- channeld/full_channel.c | 1 + common/initial_channel.c | 3 +- common/keyset.c | 22 +++-- common/keyset.h | 1 + hsmd/hsmd.c | 17 +++- onchaind/onchaind.c | 116 +++++++++++++++++--------- onchaind/test/run-grind_feerate-bug.c | 1 + onchaind/test/run-grind_feerate.c | 1 + 8 files changed, 114 insertions(+), 48 deletions(-) diff --git a/channeld/full_channel.c b/channeld/full_channel.c index 14238246f..b1302458c 100644 --- a/channeld/full_channel.c +++ b/channeld/full_channel.c @@ -273,6 +273,7 @@ struct bitcoin_tx **channel_txs(const tal_t *ctx, if (!derive_keyset(per_commitment_point, &channel->basepoints[side], &channel->basepoints[!side], + channel->option_static_remotekey, &keyset)) return NULL; diff --git a/common/initial_channel.c b/common/initial_channel.c index 010dc84f5..16547da58 100644 --- a/common/initial_channel.c +++ b/common/initial_channel.c @@ -86,7 +86,8 @@ struct bitcoin_tx *initial_channel_tx(const tal_t *ctx, if (!derive_keyset(per_commitment_point, &channel->basepoints[side], &channel->basepoints[!side], - &keyset)){ + channel->option_static_remotekey, + &keyset)) { *err_reason = "Cannot derive keyset"; return NULL; } diff --git a/common/keyset.c b/common/keyset.c index f174b8770..2592b11ab 100644 --- a/common/keyset.c +++ b/common/keyset.c @@ -5,18 +5,18 @@ bool derive_keyset(const struct pubkey *per_commitment_point, const struct basepoints *self, const struct basepoints *other, + bool option_static_remotekey, struct keyset *keyset) { - /* BOLT #3: + /* BOLT-930a9b44076a8f25a8626b31b3d5a55c0888308c #3: * - * ### `localpubkey`, `remotepubkey`, `local_htlcpubkey`, `remote_htlcpubkey`, `local_delayedpubkey`, and `remote_delayedpubkey` Derivation + * ### `localpubkey`, `local_htlcpubkey`, `remote_htlcpubkey`, `local_delayedpubkey`, and `remote_delayedpubkey` Derivation * * These pubkeys are simply generated by addition from their base points: * * pubkey = basepoint + SHA256(per_commitment_point || basepoint) * G * * The `localpubkey` uses the local node's `payment_basepoint`; - * the `remotepubkey` uses the remote node's `payment_basepoint`; * the `local_htlcpubkey` uses the local node's `htlc_basepoint`; * the `remote_htlcpubkey` uses the remote node's `htlc_basepoint`; * the `local_delayedpubkey` uses the local node's `delayed_payment_basepoint`; @@ -27,9 +27,19 @@ bool derive_keyset(const struct pubkey *per_commitment_point, &keyset->self_payment_key)) return false; - if (!derive_simple_key(&other->payment, - per_commitment_point, - &keyset->other_payment_key)) + /* BOLT-930a9b44076a8f25a8626b31b3d5a55c0888308c #3: + * + * ### `remotepubkey` Derivation + * + * If `option_static_remotekey` is negotiated the `remotepubkey` + * is simply the remote node's `payment_basepoint`, otherwise it is + * calculated as above using the remote node's `payment_basepoint`. + */ + if (option_static_remotekey) + keyset->other_payment_key = other->payment; + else if (!derive_simple_key(&other->payment, + per_commitment_point, + &keyset->other_payment_key)) return false; if (!derive_simple_key(&self->htlc, diff --git a/common/keyset.h b/common/keyset.h index 09f877093..fcb371d9a 100644 --- a/common/keyset.h +++ b/common/keyset.h @@ -18,5 +18,6 @@ struct keyset { bool derive_keyset(const struct pubkey *per_commitment_point, const struct basepoints *self, const struct basepoints *other, + bool option_static_remotekey, struct keyset *keyset); #endif /* LIGHTNING_COMMON_KEYSET_H */ diff --git a/hsmd/hsmd.c b/hsmd/hsmd.c index 3b7117c09..17c615cd2 100644 --- a/hsmd/hsmd.c +++ b/hsmd/hsmd.c @@ -1385,9 +1385,20 @@ static void hsm_unilateral_close_privkey(struct privkey *dst, get_channel_seed(&info->peer_id, info->channel_id, &channel_seed); derive_basepoints(&channel_seed, NULL, &basepoints, &secrets, NULL); - if (!derive_simple_privkey(&secrets.payment_basepoint_secret, - &basepoints.payment, info->commitment_point, - dst)) { + /* BOLT-930a9b44076a8f25a8626b31b3d5a55c0888308c #3: + * + * If `option_static_remotekey` is negotiated the `remotepubkey` + * is simply the remote node's `payment_basepoint`, otherwise it is + * calculated as above using the remote node's `payment_basepoint`. + */ + /* In our UTXO representation, this is indicated by a NULL + * commitment_point. */ + if (!info->commitment_point) + dst->secret = secrets.payment_basepoint_secret; + else if (!derive_simple_privkey(&secrets.payment_basepoint_secret, + &basepoints.payment, + info->commitment_point, + dst)) { status_failed(STATUS_FAIL_INTERNAL_ERROR, "Deriving unilateral_close_privkey"); } diff --git a/onchaind/onchaind.c b/onchaind/onchaind.c index e6006556c..41dff8693 100644 --- a/onchaind/onchaind.c +++ b/onchaind/onchaind.c @@ -1726,6 +1726,7 @@ static void handle_our_unilateral(const struct bitcoin_tx *tx, if (!derive_keyset(&local_per_commitment_point, &basepoints[LOCAL], &basepoints[REMOTE], + option_static_remotekey, ks)) status_failed(STATUS_FAIL_INTERNAL_ERROR, "Deriving keyset for %"PRIu64, commit_num); @@ -1969,6 +1970,31 @@ static void steal_htlc(struct tracked_output *out) propose_resolution(out, tx, 0, tx_type); } +/* Tell wallet that we have discovered a UTXO from a to-remote output, + * which it can spend with a little additional info we give here. */ +static void tell_wallet_to_remote(const struct bitcoin_tx *tx, + unsigned int outnum, + const struct bitcoin_txid *txid, + u32 tx_blockheight, + const u8 *scriptpubkey, + const struct pubkey *per_commit_point, + bool option_static_remotekey) +{ + struct amount_sat amt = bitcoin_tx_output_get_amount(tx, outnum); + + /* A NULL per_commit_point is how we indicate the pubkey doesn't need + * changing. */ + if (option_static_remotekey) + per_commit_point = NULL; + + wire_sync_write(REQ_FD, + take(towire_onchain_add_utxo(NULL, txid, outnum, + per_commit_point, + amt, + tx_blockheight, + scriptpubkey))); +} + /* BOLT #5: * * If any node tries to cheat by broadcasting an outdated commitment @@ -2045,6 +2071,7 @@ static void handle_their_cheat(const struct bitcoin_tx *tx, if (!derive_keyset(remote_per_commitment_point, &basepoints[REMOTE], &basepoints[LOCAL], + option_static_remotekey, ks)) status_failed(STATUS_FAIL_INTERNAL_ERROR, "Deriving keyset for %"PRIu64, commit_num); @@ -2056,7 +2083,8 @@ static void handle_their_cheat(const struct bitcoin_tx *tx, " self_payment_key: %s" " other_payment_key: %s" " self_htlc_key: %s" - " other_htlc_key: %s", + " other_htlc_key: %s" + " (option_static_remotekey = %i)", commit_num, type_to_string(tmpctx, struct pubkey, &keyset->self_revocation_key), @@ -2069,7 +2097,8 @@ static void handle_their_cheat(const struct bitcoin_tx *tx, type_to_string(tmpctx, struct pubkey, &keyset->self_htlc_key), type_to_string(tmpctx, struct pubkey, - &keyset->other_htlc_key)); + &keyset->other_htlc_key), + option_static_remotekey); remote_wscript = to_self_wscript(tmpctx, to_self_delay[REMOTE], keyset); @@ -2119,14 +2148,11 @@ static void handle_their_cheat(const struct bitcoin_tx *tx, OUTPUT_TO_US, NULL, NULL, NULL); ignore_output(out); - /* 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, - remote_per_commitment_point, - amt, - tx_blockheight, - script[LOCAL])); + tell_wallet_to_remote(tx, i, txid, + tx_blockheight, + script[LOCAL], + remote_per_commitment_point, + option_static_remotekey); script[LOCAL] = NULL; continue; } @@ -2266,6 +2292,7 @@ static void handle_their_unilateral(const struct bitcoin_tx *tx, if (!derive_keyset(remote_per_commitment_point, &basepoints[REMOTE], &basepoints[LOCAL], + option_static_remotekey, ks)) status_failed(STATUS_FAIL_INTERNAL_ERROR, "Deriving keyset for %"PRIu64, commit_num); @@ -2339,14 +2366,11 @@ static void handle_their_unilateral(const struct bitcoin_tx *tx, OUTPUT_TO_US, NULL, NULL, NULL); ignore_output(out); - /* 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, - remote_per_commitment_point, - amt, - tx_blockheight, - script[LOCAL])); + tell_wallet_to_remote(tx, i, txid, + tx_blockheight, + script[LOCAL], + remote_per_commitment_point, + option_static_remotekey); script[LOCAL] = NULL; continue; } @@ -2433,7 +2457,6 @@ static void handle_unknown_commitment(const struct bitcoin_tx *tx, const bool *tell_if_missing, struct tracked_output **outs) { - struct keyset *ks; int to_us_output = -1; u8 *local_script; @@ -2441,20 +2464,40 @@ static void handle_unknown_commitment(const struct bitcoin_tx *tx, resolved_by_other(outs[0], txid, UNKNOWN_UNILATERAL); - if (!possible_remote_per_commitment_point) + /* If they don't give us a per-commitment point and we rotate keys, + * we're out of luck. */ + if (!possible_remote_per_commitment_point + && !option_static_remotekey) 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)); + if (!option_static_remotekey) { + struct keyset *ks = tal(tmpctx, struct keyset); + if (!derive_keyset(possible_remote_per_commitment_point, + &basepoints[REMOTE], + &basepoints[LOCAL], + option_static_remotekey, + 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, + &ks->other_payment_key); + } else { + /* BOLT-930a9b44076a8f25a8626b31b3d5a55c0888308c #3: + * + * ### `remotepubkey` Derivation + * + * If `option_static_remotekey` is negotiated the + * `remotepubkey` is simply the remote node's + * `payment_basepoint`, otherwise it is calculated as above + * using the remote node's `payment_basepoint`. + */ + local_script = scriptpubkey_p2wpkh(tmpctx, + &basepoints[LOCAL].payment); + } - local_script = scriptpubkey_p2wpkh(tmpctx, &keyset->other_payment_key); for (size_t i = 0; i < tx->wtx->num_outputs; i++) { struct tracked_output *out; const u8 *oscript = bitcoin_tx_output_get_script(tmpctx, tx, i); @@ -2477,14 +2520,11 @@ static void handle_unknown_commitment(const struct bitcoin_tx *tx, OUTPUT_TO_US, NULL, NULL, NULL); ignore_output(out); - /* 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, - amt, - tx_blockheight, - local_script)); + tell_wallet_to_remote(tx, i, txid, + tx_blockheight, + local_script, + possible_remote_per_commitment_point, + option_static_remotekey); local_script = NULL; to_us_output = i; } diff --git a/onchaind/test/run-grind_feerate-bug.c b/onchaind/test/run-grind_feerate-bug.c index fe0590eda..918e35ad0 100644 --- a/onchaind/test/run-grind_feerate-bug.c +++ b/onchaind/test/run-grind_feerate-bug.c @@ -21,6 +21,7 @@ void daemon_shutdown(void) bool derive_keyset(const struct pubkey *per_commitment_point UNNEEDED, const struct basepoints *self UNNEEDED, const struct basepoints *other UNNEEDED, + bool option_static_remotekey UNNEEDED, struct keyset *keyset UNNEEDED) { fprintf(stderr, "derive_keyset called!\n"); abort(); } /* Generated stub for dump_memleak */ diff --git a/onchaind/test/run-grind_feerate.c b/onchaind/test/run-grind_feerate.c index 13468cf16..19925fa00 100644 --- a/onchaind/test/run-grind_feerate.c +++ b/onchaind/test/run-grind_feerate.c @@ -22,6 +22,7 @@ void daemon_shutdown(void) bool derive_keyset(const struct pubkey *per_commitment_point UNNEEDED, const struct basepoints *self UNNEEDED, const struct basepoints *other UNNEEDED, + bool option_static_remotekey UNNEEDED, struct keyset *keyset UNNEEDED) { fprintf(stderr, "derive_keyset called!\n"); abort(); } /* Generated stub for dump_memleak */