From acd6c030b277b13a0465f4f17f1e77d0b2033c98 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 10 Sep 2019 11:48:27 +0930 Subject: [PATCH 01/11] tools/check-bolt: don't get confused by 'BOLT #1' in middle of a comment. Insist it be prefixed with '* '. Signed-off-by: Rusty Russell --- tools/check-bolt.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tools/check-bolt.c b/tools/check-bolt.c index dd1c7ae840db..59bbf5c20a2d 100644 --- a/tools/check-bolt.c +++ b/tools/check-bolt.c @@ -102,9 +102,14 @@ static char *find_bolt_ref(const char *prefix, char **p, size_t *len) size_t preflen; /* BOLT #X: */ - *p = strstr(*p, prefix); + *p = strchr(*p, '*'); if (!*p) return NULL; + *p += 1; + while (cisspace(**p)) + (*p)++; + if (strncmp(*p, prefix, strlen(prefix)) != 0) + continue; *p += strlen(prefix); while (cisspace(**p)) (*p)++; From 9bab298d5a46d8191756bd31d68d61f4726ff983 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 10 Sep 2019 11:49:27 +0930 Subject: [PATCH 02/11] tools/generate-wire.py: accept multiple comma-separated options. Somehow this change got lost, but it's needed for option_static_remotekey, to quote gen_peer_wire_csv: msgtype,channel_reestablish,136 msgdata,channel_reestablish,channel_id,channel_id, msgdata,channel_reestablish,next_commitment_number,u64, msgdata,channel_reestablish,next_revocation_number,u64, msgdata,channel_reestablish,your_last_per_commitment_secret,byte,32,option_data_loss_protect,option_static_remotekey msgdata,channel_reestablish,my_current_per_commitment_point,point,,option_data_loss_protect Signed-off-by: Rusty Russell --- tools/generate-wire.py | 35 +++++++++++++++++++---------------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/tools/generate-wire.py b/tools/generate-wire.py index 8382940f01ad..b1856b95a1ad 100755 --- a/tools/generate-wire.py +++ b/tools/generate-wire.py @@ -577,6 +577,11 @@ def main(options, args=None, output=sys.stdout, lines=None): raise ValueError('Unknown message type {}. {}:{}'.format(tokens[1], ln, line)) type_obj, collapse, optional = master.add_type(tokens[3], tokens[2], tokens[1]) + if collapse: + count = 1 + else: + count = tokens[4] + # if this is an 'extension' field*, we want to add a new 'message' type # in the future, extensions will be handled as TLV's # @@ -585,18 +590,24 @@ def main(options, args=None, output=sys.stdout, lines=None): # differently. for the sake of clarity here, for bolt-wire messages, # we'll refer to 'optional' message fields as 'extensions') # - if bool(tokens[5:]): # is an extension field + if tokens[5:] == []: + msg.add_data_field(tokens[2], type_obj, count, comments=list(comment_set), + optional=optional) + else: # is one or more extension fields if optional: raise ValueError("Extension fields cannot be optional. {}:{}" .format(ln, line)) - extension_name = "{}_{}".format(tokens[1], tokens[5]) orig_msg = msg - msg = master.find_message(extension_name) - if not msg: - msg = copy.deepcopy(orig_msg) - msg.enumname = msg.name - msg.name = extension_name - master.add_extension_msg(msg.name, msg) + for extension in tokens[5:]: + extension_name = "{}_{}".format(tokens[1], extension) + msg = master.find_message(extension_name) + if not msg: + msg = copy.deepcopy(orig_msg) + msg.enumname = msg.name + msg.name = extension_name + master.add_extension_msg(msg.name, msg) + msg.add_data_field(tokens[2], type_obj, count, comments=list(comment_set), optional=optional) + # If this is a print_wire page, add the extension fields to the # original message, so we can print them if present. if options.print_wire: @@ -605,14 +616,6 @@ def main(options, args=None, output=sys.stdout, lines=None): comments=list(comment_set), optional=optional) - if collapse: - count = 1 - else: - count = tokens[4] - - msg.add_data_field(tokens[2], type_obj, count, comments=list(comment_set), - optional=optional) - comment_set = [] elif token_type.startswith('#include'): master.add_include(token_type) From cb854296e0fcd220b4425662ed41ae1ffe6f0c73 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 10 Sep 2019 11:50:27 +0930 Subject: [PATCH 03/11] wire/extracted_peer_experimental_csv: option_static_remotekey Aka. BOLTVERSION=930a9b44076a8f25a8626b31b3d5a55c0888308c from https://github.com/lightningnetwork/lightning-rfc/pull/642 Signed-off-by: Rusty Russell --- wire/extracted_peer_experimental_csv | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/wire/extracted_peer_experimental_csv b/wire/extracted_peer_experimental_csv index eebeb84700a7..62edd5a36e49 100644 --- a/wire/extracted_peer_experimental_csv +++ b/wire/extracted_peer_experimental_csv @@ -1,5 +1,14 @@ --- wire/extracted_peer_wire_csv 2019-08-01 11:33:48.136457293 +0930 +++ - 2019-08-01 11:40:21.313665504 +0930 +@@ -108,7 +108,7 @@ + msgdata,channel_reestablish,channel_id,channel_id, + msgdata,channel_reestablish,next_commitment_number,u64, + msgdata,channel_reestablish,next_revocation_number,u64, +-msgdata,channel_reestablish,your_last_per_commitment_secret,byte,32,option_data_loss_protect ++msgdata,channel_reestablish,your_last_per_commitment_secret,byte,32,option_data_loss_protect,option_static_remotekey + msgdata,channel_reestablish,my_current_per_commitment_point,point,,option_data_loss_protect + msgtype,announcement_signatures,259 + msgdata,announcement_signatures,channel_id,channel_id, @@ -154,6 +168,11 @@ msgdata,query_short_channel_ids,chain_hash,chain_hash, msgdata,query_short_channel_ids,len,u16, From 35c8749d47ace50133e74a9870046dee3a8f2980 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 10 Sep 2019 11:51:27 +0930 Subject: [PATCH 04/11] common/features: if EXPERIMENTAL_FEATURES, advertise `option_static_remotekey` Signed-off-by: Rusty Russell --- common/features.c | 3 ++- common/features.h | 6 ++++-- tests/test_connection.py | 12 +++++++----- 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/common/features.c b/common/features.c index a6241a179313..7cd520c8532d 100644 --- a/common/features.c +++ b/common/features.c @@ -10,7 +10,8 @@ static const u32 our_localfeatures[] = { OPTIONAL_FEATURE(LOCAL_UPFRONT_SHUTDOWN_SCRIPT), OPTIONAL_FEATURE(LOCAL_GOSSIP_QUERIES), #if EXPERIMENTAL_FEATURES - OPTIONAL_FEATURE(LOCAL_GOSSIP_QUERIES_EX) + OPTIONAL_FEATURE(LOCAL_GOSSIP_QUERIES_EX), + OPTIONAL_FEATURE(LOCAL_STATIC_REMOTEKEY), #endif }; diff --git a/common/features.h b/common/features.h index 129a3578ed29..1514e940367f 100644 --- a/common/features.h +++ b/common/features.h @@ -25,7 +25,7 @@ const char **list_supported_features(const tal_t *ctx); bool feature_is_set(const u8 *features, size_t bit); void set_feature_bit(u8 **ptr, u32 bit); -/* BOLT #9: +/* BOLT-930a9b44076a8f25a8626b31b3d5a55c0888308c #9: * * Flags are numbered from the least-significant bit, at bit 0 (i.e. 0x1, * an _even_ bit). They are generally assigned in pairs so that features @@ -36,7 +36,7 @@ void set_feature_bit(u8 **ptr, u32 bit); #define COMPULSORY_FEATURE(x) ((x) & 0xFFFFFFFE) #define OPTIONAL_FEATURE(x) ((x) | 1) -/* BOLT #9: +/* BOLT-930a9b44076a8f25a8626b31b3d5a55c0888308c #9: * * ## Assigned `localfeatures` flags *... @@ -45,11 +45,13 @@ void set_feature_bit(u8 **ptr, u32 bit); * | 3 | `initial_routing_sync` |... * | 4/5 | `option_upfront_shutdown_script` |... * | 6/7 | `gossip_queries` |... + * | 48/49| `option_static_remotekey` |... */ #define LOCAL_DATA_LOSS_PROTECT 0 #define LOCAL_INITIAL_ROUTING_SYNC 2 #define LOCAL_UPFRONT_SHUTDOWN_SCRIPT 4 #define LOCAL_GOSSIP_QUERIES 6 +#define LOCAL_STATIC_REMOTEKEY 48 /* BOLT-927c96daab2338b716708a57cd75c84a2d169e0e #9: * | Bits | Name |... diff --git a/tests/test_connection.py b/tests/test_connection.py index 0b76011b3faf..b7c82ce98fc3 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -1345,7 +1345,7 @@ def test_peerinfo(node_factory, bitcoind): l1, l2 = node_factory.line_graph(2, fundchannel=False, opts={'may_reconnect': True}) lfeatures = 'aa' if EXPERIMENTAL_FEATURES: - lfeatures = '08aa' + lfeatures = '020000000008aa' # Gossiping but no node announcement yet assert l1.rpc.getpeer(l2.info['id'])['connected'] assert len(l1.rpc.getpeer(l2.info['id'])['channels']) == 0 @@ -1596,16 +1596,18 @@ def test_dataloss_protection(node_factory, bitcoind): feerates=(7500, 7500, 7500), allow_broken_log=True) if EXPERIMENTAL_FEATURES: - # lflen == 2, features 1, 3, 5, 7 and 11 (0x08aa). - lf = "0002" + "08aa" + # features 1, 3, 5, 7, 11 and 49 (0x020000000008aa). + lf = "020000000008aa" else: - # lflen == 1, features 1, 3, 5 and 7 (0xaa). - lf = "0001" + "aa" + # features 1, 3, 5 and 7 (0xaa). + lf = "aa" l1.rpc.connect(l2.info['id'], 'localhost', l2.port) # l1 should send out WIRE_INIT (0010) l1.daemon.wait_for_log(r"\[OUT\] 0010" # gflen == 0 "0000" + # lflen + + format(len(lf) // 2, '04x') + lf) l1.fund_channel(l2, 10**6) From 21813465c152212d00835434b377731688738ba2 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 10 Sep 2019 11:52:27 +0930 Subject: [PATCH 05/11] db: store option_static_remotekey for each channel. Signed-off-by: Rusty Russell --- lightningd/channel.c | 4 +++- lightningd/channel.h | 6 +++++- lightningd/opening_control.c | 3 ++- wallet/db.c | 3 +++ wallet/wallet.c | 10 +++++++--- 5 files changed, 20 insertions(+), 6 deletions(-) diff --git a/lightningd/channel.c b/lightningd/channel.c index 98202459e544..34c87779cd46 100644 --- a/lightningd/channel.c +++ b/lightningd/channel.c @@ -177,7 +177,8 @@ struct channel *new_channel(struct peer *peer, u64 dbid, const struct pubkey *future_per_commitment_point, u32 feerate_base, u32 feerate_ppm, - const u8 *remote_upfront_shutdown_script) + const u8 *remote_upfront_shutdown_script, + bool option_static_remotekey) { struct channel *channel = tal(peer->ld, struct channel); @@ -248,6 +249,7 @@ struct channel *new_channel(struct peer *peer, u64 dbid, channel->feerate_ppm = feerate_ppm; channel->remote_upfront_shutdown_script = tal_steal(channel, remote_upfront_shutdown_script); + channel->option_static_remotekey = option_static_remotekey; list_add_tail(&peer->channels, &channel->list); tal_add_destructor(channel, destroy_channel); diff --git a/lightningd/channel.h b/lightningd/channel.h index bb78abff3232..f1dc8f6b7e36 100644 --- a/lightningd/channel.h +++ b/lightningd/channel.h @@ -117,6 +117,9 @@ struct channel { /* If they used option_upfront_shutdown_script. */ const u8 *remote_upfront_shutdown_script; + + /* Was this negotiated with `option_static_remotekey? */ + bool option_static_remotekey; }; struct channel *new_channel(struct peer *peer, u64 dbid, @@ -165,7 +168,8 @@ struct channel *new_channel(struct peer *peer, u64 dbid, u32 feerate_base, u32 feerate_ppm, /* NULL or stolen */ - const u8 *remote_upfront_shutdown_script); + const u8 *remote_upfront_shutdown_script, + bool option_static_remotekey); void delete_channel(struct channel *channel); diff --git a/lightningd/opening_control.c b/lightningd/opening_control.c index ba59b2bac38e..34c3dd3e605c 100644 --- a/lightningd/opening_control.c +++ b/lightningd/opening_control.c @@ -237,7 +237,8 @@ wallet_commit_channel(struct lightningd *ld, NULL, ld->config.fee_base, ld->config.fee_per_satoshi, - remote_upfront_shutdown_script); + remote_upfront_shutdown_script, + false); /* Now we finally put it in the database. */ wallet_channel_insert(ld->wallet, channel); diff --git a/wallet/db.c b/wallet/db.c index dc4b9cd0427e..2e741ab1f341 100644 --- a/wallet/db.c +++ b/wallet/db.c @@ -457,6 +457,9 @@ static struct migration dbmigrations[] = { " WHERE short_channel_id IS NOT NULL;"), NULL }, {SQL("UPDATE payments SET failchannel = REPLACE(failchannel, ':', 'x')" " WHERE failchannel IS NOT NULL;"), NULL }, + /* option_static_remotekey is nailed at creation time. */ + {SQL("ALTER TABLE channels ADD COLUMN option_static_remotekey" + " DEFAULT FALSE;"), NULL }, }; /* Leak tracking. */ diff --git a/wallet/wallet.c b/wallet/wallet.c index d775c9f6135c..855f121f3ccd 100644 --- a/wallet/wallet.c +++ b/wallet/wallet.c @@ -909,7 +909,8 @@ static struct channel *wallet_stmt2channel(struct wallet *w, struct db_stmt *stm future_per_commitment_point, db_column_int(stmt, 42), db_column_int(stmt, 43), - db_column_arr(tmpctx, stmt, 44, u8)); + db_column_arr(tmpctx, stmt, 44, u8), + db_column_int(stmt, 45)); return chan; } @@ -980,6 +981,7 @@ static bool wallet_channels_load_active(struct wallet *w) ", feerate_base" ", feerate_ppm" ", remote_upfront_shutdown_script" + ", option_static_remotekey" " FROM channels WHERE state < ?;")); db_bind_int(stmt, 0, CLOSED); db_query_prepared(stmt); @@ -1241,7 +1243,8 @@ void wallet_channel_save(struct wallet *w, struct channel *chan) " msatoshi_to_us_max=?," " feerate_base=?," " feerate_ppm=?," - " remote_upfront_shutdown_script=?" + " remote_upfront_shutdown_script=?," + " option_static_remotekey=?" " WHERE id=?")); db_bind_u64(stmt, 0, chan->their_shachain.id); if (chan->scid) @@ -1288,7 +1291,8 @@ void wallet_channel_save(struct wallet *w, struct channel *chan) tal_count(chan->remote_upfront_shutdown_script)); else db_bind_null(stmt, 27); - db_bind_u64(stmt, 28, chan->dbid); + db_bind_int(stmt, 28, chan->option_static_remotekey); + db_bind_u64(stmt, 29, chan->dbid); db_exec_prepared_v2(take(stmt)); wallet_channel_config_save(w, &chan->channel_info.their_config); From 291a35613c67410764eea1998e3bf4217f1d641d Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 10 Sep 2019 11:53:27 +0930 Subject: [PATCH 06/11] channeld: set option_static_remotekey when negotiated. Signed-off-by: Rusty Russell --- channeld/channel_wire.csv | 1 + channeld/channeld.c | 5 ++- channeld/full_channel.c | 2 + channeld/full_channel.h | 2 + channeld/test/run-full_channel.c | 4 +- common/initial_channel.c | 2 + common/initial_channel.h | 4 ++ devtools/mkcommit.c | 59 ++++++++++++++++----------- lightningd/channel_control.c | 6 ++- lightningd/onchain_control.c | 3 +- lightningd/opening_control.c | 27 +++++++++++- onchaind/onchain_wire.csv | 1 + onchaind/onchaind.c | 6 ++- onchaind/test/run-grind_feerate-bug.c | 2 +- onchaind/test/run-grind_feerate.c | 2 +- openingd/opening_wire.csv | 1 + openingd/openingd.c | 4 ++ 17 files changed, 99 insertions(+), 32 deletions(-) diff --git a/channeld/channel_wire.csv b/channeld/channel_wire.csv index e6df3b4114af..79242db8317d 100644 --- a/channeld/channel_wire.csv +++ b/channeld/channel_wire.csv @@ -69,6 +69,7 @@ msgdata,channel_init,upfront_shutdown_script,u8,upfront_shutdown_script_len msgdata,channel_init,remote_ann_node_sig,?secp256k1_ecdsa_signature, msgdata,channel_init,remote_ann_bitcoin_sig,?secp256k1_ecdsa_signature, msgdata,channel_init,announce_delay,u32, +msgdata,channel_init,option_static_remotekey,bool, # master->channeld funding hit new depth(funding locked if >= lock depth) msgtype,channel_funding_depth,1002 diff --git a/channeld/channeld.c b/channeld/channeld.c index a9bab9e4bb23..d74e00211282 100644 --- a/channeld/channeld.c +++ b/channeld/channeld.c @@ -2866,6 +2866,7 @@ static void init_channel(struct peer *peer) struct secret last_remote_per_commit_secret; secp256k1_ecdsa_signature *remote_ann_node_sig; secp256k1_ecdsa_signature *remote_ann_bitcoin_sig; + bool option_static_remotekey; assert(!(fcntl(MASTER_FD, F_GETFL) & O_NONBLOCK)); @@ -2924,7 +2925,8 @@ static void init_channel(struct peer *peer) &peer->remote_upfront_shutdown_script, &remote_ann_node_sig, &remote_ann_bitcoin_sig, - &peer->announce_delay)) { + &peer->announce_delay, + &option_static_remotekey)) { master_badmsg(WIRE_CHANNEL_INIT, msg); } /* stdin == requests, 3 == peer, 4 = gossip, 5 = gossip_store, 6 = HSM */ @@ -2980,6 +2982,7 @@ static void init_channel(struct peer *peer) &points[LOCAL], &points[REMOTE], &funding_pubkey[LOCAL], &funding_pubkey[REMOTE], + option_static_remotekey, funder); if (!channel_force_htlcs(peer->channel, htlcs, hstates, diff --git a/channeld/full_channel.c b/channeld/full_channel.c index c6116bc117a6..14238246f37a 100644 --- a/channeld/full_channel.c +++ b/channeld/full_channel.c @@ -46,6 +46,7 @@ struct channel *new_full_channel(const tal_t *ctx, const struct basepoints *remote_basepoints, const struct pubkey *local_funding_pubkey, const struct pubkey *remote_funding_pubkey, + bool option_static_remotekey, enum side funder) { struct channel *channel = new_initial_channel(ctx, @@ -61,6 +62,7 @@ struct channel *new_full_channel(const tal_t *ctx, remote_basepoints, local_funding_pubkey, remote_funding_pubkey, + option_static_remotekey, funder); if (channel) { diff --git a/channeld/full_channel.h b/channeld/full_channel.h index eca838049da7..01d4ce1607c3 100644 --- a/channeld/full_channel.h +++ b/channeld/full_channel.h @@ -24,6 +24,7 @@ * @remote_basepoints: remote basepoints. * @local_fundingkey: local funding key * @remote_fundingkey: remote funding key + * @option_static_remotekey: use `option_static_remotekey`. * @funder: which side initiated it. * * Returns state, or NULL if malformed. @@ -42,6 +43,7 @@ struct channel *new_full_channel(const tal_t *ctx, const struct basepoints *remote_basepoints, const struct pubkey *local_funding_pubkey, const struct pubkey *remote_funding_pubkey, + bool option_static_remotekey, enum side funder); /** diff --git a/channeld/test/run-full_channel.c b/channeld/test/run-full_channel.c index 1241cda46c50..2fdd75ed0fa1 100644 --- a/channeld/test/run-full_channel.c +++ b/channeld/test/run-full_channel.c @@ -473,7 +473,7 @@ int main(void) &localbase, &remotebase, &local_funding_pubkey, &remote_funding_pubkey, - LOCAL); + false, LOCAL); rchannel = new_full_channel(tmpctx, &chainparams->genesis_blockhash, &funding_txid, funding_output_index, 0, @@ -484,7 +484,7 @@ int main(void) &remotebase, &localbase, &remote_funding_pubkey, &local_funding_pubkey, - REMOTE); + false, REMOTE); /* BOLT #3: * diff --git a/common/initial_channel.c b/common/initial_channel.c index c4a5be5c890f..010dc84f518c 100644 --- a/common/initial_channel.c +++ b/common/initial_channel.c @@ -22,6 +22,7 @@ struct channel *new_initial_channel(const tal_t *ctx, const struct basepoints *remote_basepoints, const struct pubkey *local_funding_pubkey, const struct pubkey *remote_funding_pubkey, + bool option_static_remotekey, enum side funder) { struct channel *channel = tal(ctx, struct channel); @@ -65,6 +66,7 @@ struct channel *new_initial_channel(const tal_t *ctx, if (channel->chainparams == NULL) return tal_free(channel); + channel->option_static_remotekey = option_static_remotekey; return channel; } diff --git a/common/initial_channel.h b/common/initial_channel.h index b4a91d3f0969..4db6d72a6cd0 100644 --- a/common/initial_channel.h +++ b/common/initial_channel.h @@ -64,6 +64,9 @@ struct channel { /* Chain params to check against */ const struct chainparams *chainparams; + + /* Is this using option_static_remotekey? */ + bool option_static_remotekey; }; /** @@ -101,6 +104,7 @@ struct channel *new_initial_channel(const tal_t *ctx, const struct basepoints *remote_basepoints, const struct pubkey *local_funding_pubkey, const struct pubkey *remote_funding_pubkey, + bool option_static_remotekey, enum side funder); diff --git a/devtools/mkcommit.c b/devtools/mkcommit.c index d671725f8a3c..acb765403688 100644 --- a/devtools/mkcommit.c +++ b/devtools/mkcommit.c @@ -10,6 +10,7 @@ */ #include #include +#include #include #include #include @@ -20,6 +21,7 @@ #include #include #include +#include #include #include #include @@ -247,6 +249,7 @@ int main(int argc, char *argv[]) const struct htlc **htlcmap; struct privkey local_htlc_privkey, remote_htlc_privkey; struct pubkey local_htlc_pubkey, remote_htlc_pubkey; + bool option_static_remotekey = false; const struct chainparams *chainparams = chainparams_for_network("bitcoin"); setup_locale(); @@ -254,30 +257,36 @@ int main(int argc, char *argv[]) secp256k1_ctx = secp256k1_context_create(SECP256K1_CONTEXT_VERIFY | SECP256K1_CONTEXT_SIGN); - if (argv[1] && streq(argv[1], "-v")) { - verbose = true; - argv++; - argc--; - } + opt_register_noarg("--help|-h", opt_usage_and_exit, + " [...]\n" + "Where are:\n" + " \n" + " \n" + " \n" + "Where are:\n" + " \n" + " \n" + " \n" + " \n" + " \n" + " \n" + "Where s are:\n" + " \n" + " \n" + " \n" + " \n", + "Show this message"); + opt_register_noarg("-v|--verbose", opt_set_bool, &verbose, + "Increase verbosity"); + opt_register_noarg("--option-static-remotekey", opt_set_bool, + &option_static_remotekey, + "Use option_static_remotekey generation rules"); + opt_register_version(); + + opt_parse(&argc, argv, opt_log_stderr_exit); if (argc < 1 + 7 + 3*2 + 6*2) - errx(1, "Usage: mkcommit [-v] [...]\n" - "Where are:\n" - " \n" - " \n" - " \n" - "Where are:\n" - " \n" - " \n" - " \n" - " \n" - " \n" - " \n" - "Where s are:\n" - " \n" - " \n" - " \n" - " \n"); + opt_usage_exit_fail("Too few arguments"); argnum = 1; commitnum = atol(argv[argnum++]); @@ -312,8 +321,11 @@ int main(int argc, char *argv[]) errx(1, "Can't afford local_msat"); printf("## HTLCs\n"); - while (argnum < argc) + while (argnum < argc) { + if (argnum + 4 > argc) + opt_usage_exit_fail("Too few arguments for htlc"); argnum += parse_htlc(argv + argnum, &htlcs, &hstates, &preimages); + } printf("\n"); if (!pubkey_from_privkey(&local.funding_privkey, &funding_localkey) @@ -355,6 +367,7 @@ int main(int argc, char *argv[]) &localconfig, &remoteconfig, &localbase, &remotebase, &funding_localkey, &funding_remotekey, + option_static_remotekey, fee_payer); if (!channel_force_htlcs(channel, htlcs, hstates, NULL, NULL, NULL, NULL, diff --git a/lightningd/channel_control.c b/lightningd/channel_control.c index b05bdc9557c6..d9aa93897633 100644 --- a/lightningd/channel_control.c +++ b/lightningd/channel_control.c @@ -1,6 +1,7 @@ #include #include #include +#include #include #include #include @@ -431,7 +432,10 @@ void peer_start_channeld(struct channel *channel, remote_ann_bitcoin_sig, /* Delay announce by 60 seconds after * seeing block (adjustable if dev) */ - ld->topology->poll_seconds * 2); + ld->topology->poll_seconds * 2, + /* Set at channel open, even if not + * negotiated now! */ + channel->option_static_remotekey); /* We don't expect a response: we are triggered by funding_depth_cb. */ subd_send_msg(channel->owner, take(initmsg)); diff --git a/lightningd/onchain_control.c b/lightningd/onchain_control.c index a7f1df2e2503..8c38eb992e75 100644 --- a/lightningd/onchain_control.c +++ b/lightningd/onchain_control.c @@ -543,7 +543,8 @@ enum watch_result onchaind_funding_spent(struct channel *channel, tal_count(stubs), channel->min_possible_feerate, channel->max_possible_feerate, - channel->future_per_commitment_point); + channel->future_per_commitment_point, + channel->option_static_remotekey); subd_send_msg(channel->owner, take(msg)); /* FIXME: Don't queue all at once, use an empty cb... */ diff --git a/lightningd/opening_control.c b/lightningd/opening_control.c index 34c3dd3e605c..14b1dd9f83c9 100644 --- a/lightningd/opening_control.c +++ b/lightningd/opening_control.c @@ -4,6 +4,7 @@ #include #include #include +#include #include #include #include @@ -168,6 +169,7 @@ wallet_commit_channel(struct lightningd *ld, struct channel *channel; struct amount_msat our_msat; s64 final_key_idx; + bool option_static_remotekey; /* Get a key to use for closing outputs from this tx */ final_key_idx = wallet_get_newindex(ld); @@ -196,6 +198,27 @@ wallet_commit_channel(struct lightningd *ld, /* old_remote_per_commit not valid yet, copy valid one. */ channel_info->old_remote_per_commit = channel_info->remote_per_commit; + /* BOLT-930a9b44076a8f25a8626b31b3d5a55c0888308c #2: + * 1. type: 35 (`funding_signed`) + * 2. data: + * * [`channel_id`:`channel_id`] + * * [`signature`:`signature`] + * + * #### Requirements + * + * Both peers: + * - if `option_static_remotekey` was negotiated: + * - `option_static_remotekey` applies to all commitment + * transactions + * - otherwise: + * - `option_static_remotekey` does not apply to any commitment + * transactions + */ + /* i.e. We set it now for the channel permanently. */ + option_static_remotekey + = local_feature_negotiated(uc->peer->localfeatures, + LOCAL_STATIC_REMOTEKEY); + channel = new_channel(uc->peer, uc->dbid, NULL, /* No shachain yet */ CHANNELD_AWAITING_LOCKIN, @@ -238,7 +261,7 @@ wallet_commit_channel(struct lightningd *ld, ld->config.fee_base, ld->config.fee_per_satoshi, remote_upfront_shutdown_script, - false); + option_static_remotekey); /* Now we finally put it in the database. */ wallet_channel_insert(ld->wallet, channel); @@ -1106,6 +1129,8 @@ void peer_start_openingd(struct peer *peer, feerate_min(peer->ld, NULL), feerate_max(peer->ld, NULL), peer->localfeatures, + local_feature_negotiated(peer->localfeatures, + LOCAL_STATIC_REMOTEKEY), send_msg); subd_send_msg(uc->openingd, take(msg)); } diff --git a/onchaind/onchain_wire.csv b/onchaind/onchain_wire.csv index 08f7d362fb6f..0af89927b5dc 100644 --- a/onchaind/onchain_wire.csv +++ b/onchaind/onchain_wire.csv @@ -35,6 +35,7 @@ msgdata,onchain_init,num_htlcs,u64, msgdata,onchain_init,min_possible_feerate,u32, msgdata,onchain_init,max_possible_feerate,u32, msgdata,onchain_init,possible_remote_per_commit_point,?pubkey, +msgdata,onchain_init,option_static_remotekey,bool, #include # This is all the HTLCs: one per message diff --git a/onchaind/onchaind.c b/onchaind/onchaind.c index 85a88a0ba4a2..e6006556c8ec 100644 --- a/onchaind/onchaind.c +++ b/onchaind/onchaind.c @@ -67,6 +67,9 @@ static u32 reasonable_depth; /* The messages to send at that depth. */ static u8 **missing_htlc_msgs; +/* Does option_static_remotekey apply to this commitment tx? */ +bool option_static_remotekey; + /* 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 */ @@ -2567,7 +2570,8 @@ int main(int argc, char *argv[]) &num_htlcs, &min_possible_feerate, &max_possible_feerate, - &possible_remote_per_commitment_point)) { + &possible_remote_per_commitment_point, + &option_static_remotekey)) { master_badmsg(WIRE_ONCHAIN_INIT, msg); } diff --git a/onchaind/test/run-grind_feerate-bug.c b/onchaind/test/run-grind_feerate-bug.c index ae305148e061..fe0590edae42 100644 --- a/onchaind/test/run-grind_feerate-bug.c +++ b/onchaind/test/run-grind_feerate-bug.c @@ -42,7 +42,7 @@ bool fromwire_onchain_dev_memleak(const void *p UNNEEDED) 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, struct bitcoin_blkid *chain_hash UNNEEDED, struct amount_sat *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, struct amount_sat *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) +bool fromwire_onchain_init(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, struct shachain *shachain UNNEEDED, struct bitcoin_blkid *chain_hash UNNEEDED, struct amount_sat *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, struct amount_sat *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, bool *option_static_remotekey 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/onchaind/test/run-grind_feerate.c b/onchaind/test/run-grind_feerate.c index f575f215073e..13468cf162d6 100644 --- a/onchaind/test/run-grind_feerate.c +++ b/onchaind/test/run-grind_feerate.c @@ -46,7 +46,7 @@ bool fromwire_onchain_dev_memleak(const void *p UNNEEDED) 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, struct bitcoin_blkid *chain_hash UNNEEDED, struct amount_sat *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, struct amount_sat *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) +bool fromwire_onchain_init(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, struct shachain *shachain UNNEEDED, struct bitcoin_blkid *chain_hash UNNEEDED, struct amount_sat *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, struct amount_sat *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, bool *option_static_remotekey 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/openingd/opening_wire.csv b/openingd/opening_wire.csv index 810435118d69..365dfdd4175d 100644 --- a/openingd/opening_wire.csv +++ b/openingd/opening_wire.csv @@ -20,6 +20,7 @@ msgdata,opening_init,min_feerate,u32, msgdata,opening_init,max_feerate,u32, msgdata,opening_init,lfeatures_len,u16, msgdata,opening_init,lfeatures,u8,lfeatures_len +msgdata,opening_init,option_static_remotekey,bool, # Optional msg to send. msgdata,opening_init,len,u16, msgdata,opening_init,msg,u8,len diff --git a/openingd/openingd.c b/openingd/openingd.c index b84eb1e41b40..c9790fe58a17 100644 --- a/openingd/openingd.c +++ b/openingd/openingd.c @@ -105,6 +105,7 @@ struct state { /* Which chain we're on, so we can check/set `chain_hash` fields */ const struct chainparams *chainparams; + bool option_static_remotekey; }; static const u8 *dev_upfront_shutdown_script(const tal_t *ctx) @@ -660,6 +661,7 @@ static bool funder_finalize_channel_setup(struct state *state, &state->their_points, &state->our_funding_pubkey, &state->their_funding_pubkey, + state->option_static_remotekey, /* Funder is local */ LOCAL); /* We were supposed to do enough checks above, but just in case, @@ -1378,6 +1380,7 @@ static u8 *fundee_channel(struct state *state, const u8 *open_channel_msg) &state->our_points, &theirs, &state->our_funding_pubkey, &their_funding_pubkey, + state->option_static_remotekey, REMOTE); /* We don't expect this to fail, but it does do some additional * internal sanity checks. */ @@ -1703,6 +1706,7 @@ int main(int argc, char *argv[]) &state->minimum_depth, &state->min_feerate, &state->max_feerate, &state->localfeatures, + &state->option_static_remotekey, &inner)) master_badmsg(WIRE_OPENING_INIT, msg); From 93f7240801b25139ea674b78f265df20c51869eb Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 10 Sep 2019 11:54:27 +0930 Subject: [PATCH 07/11] common/utxo: make commitment_point optional in close_info. We don't rotate key for option_static_remotekey, so we don't need this point for such channels. Signed-off-by: Rusty Russell --- common/utxo.c | 12 ++++++++++-- common/utxo.h | 3 ++- hsmd/hsmd.c | 2 +- onchaind/onchain_wire.csv | 2 +- wallet/test/run-wallet.c | 27 +++++++++++++++++++++++++-- wallet/wallet.c | 13 +++++++++++-- 6 files changed, 50 insertions(+), 9 deletions(-) diff --git a/common/utxo.c b/common/utxo.c index 41f286a28599..6b5eb7d73b50 100644 --- a/common/utxo.c +++ b/common/utxo.c @@ -20,7 +20,9 @@ void towire_utxo(u8 **pptr, const struct utxo *utxo) if (is_unilateral_close) { towire_u64(pptr, utxo->close_info->channel_id); towire_node_id(pptr, &utxo->close_info->peer_id); - towire_pubkey(pptr, &utxo->close_info->commitment_point); + towire_bool(pptr, utxo->close_info->commitment_point != NULL); + if (utxo->close_info->commitment_point) + towire_pubkey(pptr, utxo->close_info->commitment_point); } } @@ -42,7 +44,13 @@ struct utxo *fromwire_utxo(const tal_t *ctx, const u8 **ptr, size_t *max) utxo->close_info = tal(utxo, struct unilateral_close_info); utxo->close_info->channel_id = fromwire_u64(ptr, max); fromwire_node_id(ptr, max, &utxo->close_info->peer_id); - fromwire_pubkey(ptr, max, &utxo->close_info->commitment_point); + if (fromwire_bool(ptr, max)) { + utxo->close_info->commitment_point = tal(utxo, + struct pubkey); + fromwire_pubkey(ptr, max, + utxo->close_info->commitment_point); + } else + utxo->close_info->commitment_point = NULL; } else { utxo->close_info = NULL; } diff --git a/common/utxo.h b/common/utxo.h index 32ce97414187..1eef4a0828d5 100644 --- a/common/utxo.h +++ b/common/utxo.h @@ -17,7 +17,8 @@ struct ext_key; struct unilateral_close_info { u64 channel_id; struct node_id peer_id; - struct pubkey commitment_point; + /* NULL if this is an option_static_remotekey commitment */ + struct pubkey *commitment_point; }; struct utxo { diff --git a/hsmd/hsmd.c b/hsmd/hsmd.c index da46e088891a..3b7117c097a4 100644 --- a/hsmd/hsmd.c +++ b/hsmd/hsmd.c @@ -1386,7 +1386,7 @@ static void hsm_unilateral_close_privkey(struct privkey *dst, derive_basepoints(&channel_seed, NULL, &basepoints, &secrets, NULL); if (!derive_simple_privkey(&secrets.payment_basepoint_secret, - &basepoints.payment, &info->commitment_point, + &basepoints.payment, info->commitment_point, dst)) { status_failed(STATUS_FAIL_INTERNAL_ERROR, "Deriving unilateral_close_privkey"); diff --git a/onchaind/onchain_wire.csv b/onchaind/onchain_wire.csv index 0af89927b5dc..47ad2576a7fc 100644 --- a/onchaind/onchain_wire.csv +++ b/onchaind/onchain_wire.csv @@ -91,7 +91,7 @@ msgtype,onchain_all_irrevocably_resolved,5011 msgtype,onchain_add_utxo,5012 msgdata,onchain_add_utxo,prev_out_tx,bitcoin_txid, msgdata,onchain_add_utxo,prev_out_index,u32, -msgdata,onchain_add_utxo,per_commit_point,pubkey, +msgdata,onchain_add_utxo,per_commit_point,?pubkey, msgdata,onchain_add_utxo,value,amount_sat, msgdata,onchain_add_utxo,blockheight,u32, msgdata,onchain_add_utxo,len,u16, diff --git a/wallet/test/run-wallet.c b/wallet/test/run-wallet.c index 5d208ffc6ed2..e5af87224a8b 100644 --- a/wallet/test/run-wallet.c +++ b/wallet/test/run-wallet.c @@ -784,7 +784,7 @@ static bool test_wallet_outputs(struct lightningd *ld, const tal_t *ctx) u.close_info = tal(w, struct unilateral_close_info); u.close_info->channel_id = 42; u.close_info->peer_id = id; - u.close_info->commitment_point = pk; + u.close_info->commitment_point = &pk; CHECK_MSG(wallet_add_utxo(w, &u, p2sh_wpkh), "wallet_add_utxo with close_info"); @@ -796,7 +796,7 @@ static bool test_wallet_outputs(struct lightningd *ld, const tal_t *ctx) u = *utxos[1]; CHECK(u.close_info->channel_id == 42 && - pubkey_eq(&u.close_info->commitment_point, &pk) && + pubkey_eq(u.close_info->commitment_point, &pk) && node_id_eq(&u.close_info->peer_id, &id)); /* Now un-reserve them for the tests below */ tal_free(utxos); @@ -826,6 +826,29 @@ static bool test_wallet_outputs(struct lightningd *ld, const tal_t *ctx) output_state_spent), "could not change output state ignoring oldstate"); + /* Attempt to save an UTXO with close_info set, no commitment_point */ + memset(&u.txid, 2, sizeof(u.txid)); + u.amount = AMOUNT_SAT(5); + u.close_info = tal(w, struct unilateral_close_info); + u.close_info->channel_id = 42; + u.close_info->peer_id = id; + u.close_info->commitment_point = NULL; + CHECK_MSG(wallet_add_utxo(w, &u, p2sh_wpkh), + "wallet_add_utxo with close_info no commitment_point"); + + /* Now select it */ + utxos = wallet_select_coins(w, w, AMOUNT_SAT(5), 0, 21, + 0 /* no confirmations required */, + &fee_estimate, &change_satoshis); + CHECK(utxos && tal_count(utxos) == 2); + + u = *utxos[1]; + CHECK(u.close_info->channel_id == 42 && + u.close_info->commitment_point == NULL && + node_id_eq(&u.close_info->peer_id, &id)); + /* Now un-reserve them */ + tal_free(utxos); + db_commit_transaction(w->db); return true; } diff --git a/wallet/wallet.c b/wallet/wallet.c index 855f121f3ccd..508e7e2e5424 100644 --- a/wallet/wallet.c +++ b/wallet/wallet.c @@ -111,7 +111,10 @@ bool wallet_add_utxo(struct wallet *w, struct utxo *utxo, if (utxo->close_info) { db_bind_u64(stmt, 6, utxo->close_info->channel_id); db_bind_node_id(stmt, 7, &utxo->close_info->peer_id); - db_bind_pubkey(stmt, 8, &utxo->close_info->commitment_point); + if (utxo->close_info->commitment_point) + db_bind_pubkey(stmt, 8, utxo->close_info->commitment_point); + else + db_bind_null(stmt, 8); } else { db_bind_null(stmt, 6); db_bind_null(stmt, 7); @@ -155,7 +158,13 @@ static struct utxo *wallet_stmt2output(const tal_t *ctx, struct db_stmt *stmt) utxo->close_info = tal(utxo, struct unilateral_close_info); utxo->close_info->channel_id = db_column_u64(stmt, 6); db_column_node_id(stmt, 7, &utxo->close_info->peer_id); - db_column_pubkey(stmt, 8, &utxo->close_info->commitment_point); + if (!db_column_is_null(stmt, 8)) { + utxo->close_info->commitment_point + = tal(utxo->close_info, struct pubkey); + db_column_pubkey(stmt, 8, + utxo->close_info->commitment_point); + } else + utxo->close_info->commitment_point = NULL; } else { utxo->close_info = NULL; } From ab04b0cdb0bdb9e67510f62e4cd88e29baf28b59 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 10 Sep 2019 11:55:27 +0930 Subject: [PATCH 08/11] 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 14238246f37a..b1302458c7ba 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 010dc84f518c..16547da5852f 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 f174b8770bfb..2592b11ab91e 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 09f8770937af..fcb371d9ab4e 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 3b7117c097a4..17c615cd2bd8 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 e6006556c8ec..41dff8693d6f 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 fe0590edae42..918e35ad0044 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 13468cf162d6..19925fa00df6 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 */ From 2b43bb675dfb646cebf30e92ab96cd5795078aa9 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 10 Sep 2019 12:57:51 +0930 Subject: [PATCH 09/11] channeld: don't exchange my_current_per_commitment_point if option_static_remotekey Signed-off-by: Rusty Russell --- .travis/build.sh | 1 - channeld/channel_wire.csv | 3 +- channeld/channeld.c | 76 ++++++++++++++++++++++++++++++------ lightningd/channel_control.c | 23 ++++++++--- tests/test_connection.py | 18 ++++++--- 5 files changed, 96 insertions(+), 25 deletions(-) diff --git a/.travis/build.sh b/.travis/build.sh index 63f67c42f463..61465f2a027b 100755 --- a/.travis/build.sh +++ b/.travis/build.sh @@ -9,7 +9,6 @@ export EXPERIMENTAL_FEATURES=${EXPERIMENTAL_FEATURES:-0} export SOURCE_CHECK_ONLY=${SOURCE_CHECK_ONLY:-"false"} export COMPAT=${COMPAT:-1} export PATH=$CWD/dependencies/bin:"$HOME"/.local/bin:"$PATH" -export TIMEOUT=180 export PYTEST_PAR=2 export PYTHONPATH=$PWD/contrib/pylightning:$PYTHONPATH # If we're not in developer mode, tests spend a lot of time waiting for gossip! diff --git a/channeld/channel_wire.csv b/channeld/channel_wire.csv index 79242db8317d..51aaba116c1a 100644 --- a/channeld/channel_wire.csv +++ b/channeld/channel_wire.csv @@ -186,7 +186,8 @@ msgdata,channel_dev_memleak_reply,leak,bool, # Peer presented proof it was from the future. msgtype,channel_fail_fallen_behind,1028 -msgdata,channel_fail_fallen_behind,remote_per_commitment_point,pubkey, +# This is NULL if option_static_remotekey. +msgdata,channel_fail_fallen_behind,remote_per_commitment_point,?pubkey, # Handle a channel specific feerate base ppm configuration msgtype,channel_specific_feerates,1029 diff --git a/channeld/channeld.c b/channeld/channeld.c index d74e00211282..a2dcc2ea7a73 100644 --- a/channeld/channeld.c +++ b/channeld/channeld.c @@ -2013,10 +2013,15 @@ static void resend_commitment(struct peer *peer, const struct changed_htlc *last peer->revocations_received); } -/* BOLT #2: +/* BOLT-930a9b44076a8f25a8626b31b3d5a55c0888308c #2: * * A receiving node: - * - if it supports `option_data_loss_protect`, AND the + * - if `option_static_remotekey` applies to the commitment transaction: + * - if `next_revocation_number` is greater than expected above, AND + * `your_last_per_commitment_secret` is correct for that + * `next_revocation_number` minus 1: + *... + * - otherwise, if it supports `option_data_loss_protect`, AND the * `option_data_loss_protect` fields are present: * - if `next_revocation_number` is greater than expected above, * AND `your_last_per_commitment_secret` is correct for that @@ -2025,6 +2030,7 @@ static void resend_commitment(struct peer *peer, const struct changed_htlc *last static void check_future_dataloss_fields(struct peer *peer, u64 next_revocation_number, const struct secret *last_local_per_commit_secret, + /* This is NULL if option_static_remotekey */ const struct pubkey *remote_current_per_commitment_point) { const u8 *msg; @@ -2070,10 +2076,14 @@ static void check_future_dataloss_fields(struct peer *peer, peer_failed(peer->pps, &peer->channel_id, "Awaiting unilateral close"); } -/* BOLT #2: +/* BOLT-930a9b44076a8f25a8626b31b3d5a55c0888308c #2: * * A receiving node: - * - if it supports `option_data_loss_protect`, AND the + * - if `option_static_remotekey` applies to the commitment transaction: + * ... + * - if `your_last_per_commitment_secret` does not match the expected values: + * - SHOULD fail the channel. + * - otherwise, if it supports `option_data_loss_protect`, AND the * `option_data_loss_protect` fields are present: *... * - otherwise (`your_last_per_commitment_secret` or @@ -2084,6 +2094,7 @@ static void check_current_dataloss_fields(struct peer *peer, u64 next_revocation_number, u64 next_commitment_number, const struct secret *last_local_per_commit_secret, + /* NULL if option_static_remotekey */ const struct pubkey *remote_current_per_commitment_point) { struct secret old_commit_secret; @@ -2130,6 +2141,11 @@ static void check_current_dataloss_fields(struct peer *peer, type_to_string(tmpctx, struct secret, &old_commit_secret)); + if (!remote_current_per_commitment_point) { + status_debug("option_static_remotekey: fields are correct"); + return; + } + status_debug("Reestablish, comparing commitments. Remote's next local commitment number" " is %"PRIu64". Our next remote is %"PRIu64" with %"PRIu64 " revocations received", @@ -2207,18 +2223,22 @@ static void peer_reconnect(struct peer *peer, struct pubkey my_current_per_commitment_point, remote_current_per_commitment_point; struct secret last_local_per_commitment_secret; - bool dataloss_protect; + bool dataloss_protect, check_extra_fields; const u8 **premature_msgs = tal_arr(peer, const u8 *, 0); dataloss_protect = local_feature_negotiated(peer->localfeatures, LOCAL_DATA_LOSS_PROTECT); + /* Both these options give us extra fields to check. */ + check_extra_fields + = dataloss_protect || peer->channel->option_static_remotekey; + /* Our current per-commitment point is the commitment point in the last * received signed commitment */ get_per_commitment_point(peer->next_index[LOCAL] - 1, &my_current_per_commitment_point, NULL); - /* BOLT #2: + /* BOLT-930a9b44076a8f25a8626b31b3d5a55c0888308c #2: * * - upon reconnection: * - if a channel is in an error state: @@ -2234,7 +2254,7 @@ static void peer_reconnect(struct peer *peer, * of the next `commitment_signed` it expects to receive. * - MUST set `next_revocation_number` to the commitment number * of the next `revoke_and_ack` message it expects to receive. - * - if it supports `option_data_loss_protect`: + * - if it supports `option_data_loss_protect` or `option_static_remotekey`: * - MUST set `my_current_per_commitment_point` to its commitment * point for the last signed commitment it received from its * channel peer (i.e. the commitment_point corresponding to the @@ -2246,6 +2266,15 @@ static void peer_reconnect(struct peer *peer, * - MUST set `your_last_per_commitment_secret` to the last * `per_commitment_secret` it received */ +#if EXPERIMENTAL_FEATURES + if (peer->channel->option_static_remotekey) { + msg = towire_channel_reestablish_option_static_remotekey + (NULL, &peer->channel_id, + peer->next_index[LOCAL], + peer->revocations_received, + last_remote_per_commit_secret); + } else +#endif /* EXPERIMENTAL_FEATURES */ if (dataloss_protect) { msg = towire_channel_reestablish_option_data_loss_protect (NULL, &peer->channel_id, @@ -2273,6 +2302,21 @@ static void peer_reconnect(struct peer *peer, } while (handle_peer_gossip_or_error(peer->pps, &peer->channel_id, msg) || capture_premature_msg(&premature_msgs, msg)); +#if EXPERIMENTAL_FEATURES + if (peer->channel->option_static_remotekey) { + if (!fromwire_channel_reestablish_option_static_remotekey(msg, + &channel_id, + &next_commitment_number, + &next_revocation_number, + &last_local_per_commitment_secret)) { + peer_failed(peer->pps, + &peer->channel_id, + "bad reestablish static_remotekey msg: %s %s", + wire_type_name(fromwire_peektype(msg)), + tal_hex(msg, msg)); + } + } else +#endif /* EXPERIMENTAL_FEATURES */ if (dataloss_protect) { if (!fromwire_channel_reestablish_option_data_loss_protect(msg, &channel_id, @@ -2361,9 +2405,10 @@ static void peer_reconnect(struct peer *peer, next_revocation_number, peer->next_index[LOCAL]); } else if (next_revocation_number > peer->next_index[LOCAL] - 1) { - if (!dataloss_protect) - /* They don't support option_data_loss_protect, we - * fail it due to unexpected number */ + if (!check_extra_fields) + /* They don't support option_data_loss_protect or + * option_static_remotekey, we fail it due to + * unexpected number */ peer_failed(peer->pps, &peer->channel_id, "bad reestablish revocation_number: %"PRIu64 @@ -2376,6 +2421,7 @@ static void peer_reconnect(struct peer *peer, check_future_dataloss_fields(peer, next_revocation_number, &last_local_per_commitment_secret, + peer->channel->option_static_remotekey ? NULL : &remote_current_per_commitment_point); } else retransmit_revoke_and_ack = false; @@ -2418,12 +2464,14 @@ static void peer_reconnect(struct peer *peer, retransmit_commitment_signed = false; /* After we checked basic sanity, we check dataloss fields if any */ - if (dataloss_protect) + if (check_extra_fields) check_current_dataloss_fields(peer, next_revocation_number, next_commitment_number, &last_local_per_commitment_secret, - &remote_current_per_commitment_point); + peer->channel->option_static_remotekey + ? NULL + : &remote_current_per_commitment_point); /* We have to re-send in the same order we sent originally: * revoke_and_ack (usually) alters our next commitment. */ @@ -2947,6 +2995,10 @@ static void init_channel(struct peer *peer) feerate_per_kw[LOCAL], feerate_per_kw[REMOTE], peer->feerate_min, peer->feerate_max); +#if EXPERIMENTAL_FEATURES + status_debug("option_static_remotekey = %u", option_static_remotekey); +#endif + if(remote_ann_node_sig && remote_ann_bitcoin_sig) { peer->announcement_node_sigs[REMOTE] = *remote_ann_node_sig; peer->announcement_bitcoin_sigs[REMOTE] = *remote_ann_bitcoin_sig; diff --git a/lightningd/channel_control.c b/lightningd/channel_control.c index d9aa93897633..6b85a530af96 100644 --- a/lightningd/channel_control.c +++ b/lightningd/channel_control.c @@ -1,5 +1,6 @@ #include #include +#include #include #include #include @@ -183,17 +184,29 @@ static void peer_got_shutdown(struct channel *channel, const u8 *msg) static void channel_fail_fallen_behind(struct channel *channel, const u8 *msg) { - struct pubkey per_commitment_point; - - if (!fromwire_channel_fail_fallen_behind(msg, &per_commitment_point)) { + if (!fromwire_channel_fail_fallen_behind(channel, msg, + cast_const2(struct pubkey **, + &channel->future_per_commitment_point))) { channel_internal_error(channel, "bad channel_fail_fallen_behind %s", tal_hex(tmpctx, msg)); return; } - channel->future_per_commitment_point - = tal_dup(channel, struct pubkey, &per_commitment_point); + /* per_commitment_point is NULL if option_static_remotekey, but we + * use its presence as a flag so set it any valid key in that case. */ + if (!channel->future_per_commitment_point) { + struct pubkey *any = tal(channel, struct pubkey); + if (!channel->option_static_remotekey) { + channel_internal_error(channel, + "bad channel_fail_fallen_behind %s", + tal_hex(tmpctx, msg)); + return; + } + if (!pubkey_from_node_id(any, &channel->peer->ld->id)) + fatal("Our own id invalid?"); + channel->future_per_commitment_point = any; + } /* Peer sees this, so send a generic msg about unilateral close. */ channel_fail_permanent(channel, "Awaiting unilateral close"); diff --git a/tests/test_connection.py b/tests/test_connection.py index b7c82ce98fc3..042254e3e21a 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -1618,7 +1618,12 @@ def test_dataloss_protection(node_factory, bitcoind): orig_db = open(dbpath, "rb").read() l2.start() - # l1 should have sent WIRE_CHANNEL_REESTABLISH with option_data_loss_protect. + if EXPERIMENTAL_FEATURES: + # No my_current_per_commitment_point with option_static_remotekey + my_current_per_commitment_point_regex = "" + else: + my_current_per_commitment_point_regex = "0[23][0-9a-f]{64}" + # l1 should have sent WIRE_CHANNEL_REESTABLISH with extra fields. l1.daemon.wait_for_log(r"\[OUT\] 0088" # channel_id "[0-9a-f]{64}" @@ -1630,8 +1635,9 @@ def test_dataloss_protection(node_factory, bitcoind): # trigger a fee-update and commit, hence this may not # be zero) "[0-9a-f]{64}" - # my_current_per_commitment_point - "0[23][0-9a-f]{64}") + # my_current_per_commitment_point (maybe) + + my_current_per_commitment_point_regex + "'$") + # After an htlc, we should get different results (two more commits) l1.pay(l2, 200000000) @@ -1642,7 +1648,7 @@ def test_dataloss_protection(node_factory, bitcoind): l2.restart() - # l1 should have sent WIRE_CHANNEL_REESTABLISH with option_data_loss_protect. + # l1 should have sent WIRE_CHANNEL_REESTABLISH with extra fields. l1.daemon.wait_for_log(r"\[OUT\] 0088" # channel_id "[0-9a-f]{64}" @@ -1652,8 +1658,8 @@ def test_dataloss_protection(node_factory, bitcoind): "000000000000000[1-9]" # your_last_per_commitment_secret "[0-9a-f]{64}" - # my_current_per_commitment_point - "0[23][0-9a-f]{64}") + # my_current_per_commitment_point (maybe) + + my_current_per_commitment_point_regex + "'$") # Now, move l2 back in time. l2.stop() From 9911f9e7c09727c46e151fc3bd7f0d48dcad9f6e Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 10 Sep 2019 12:58:08 +0930 Subject: [PATCH 10/11] Makefile: fix race if we run `make update-mocks` in a clean tree. /bin/sh: 1: ccan/ccan/cdump/tools/cdump-enumstr: Text file busy make[1]: *** [common/Makefile:81: common/gen_htlc_state_names.h] Error 2 make[1]: *** Waiting for unfinished jobs.... The fix is to make sure all generated headers are made first, and thus cdump-enumstr. Signed-off-by: Rusty Russell --- Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/Makefile b/Makefile index 1a8431bd4066..85e34f4f1273 100644 --- a/Makefile +++ b/Makefile @@ -430,6 +430,7 @@ clean: find . -name '*gcno' -delete find . -name '*.nccout' -delete +update-mocks: $(ALL_GEN_HEADERS) update-mocks/%: % @MAKE=$(MAKE) tools/update-mocks.sh "$*" From 7529744abc0bbf0dc07040969b1befa47df0250a Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 10 Sep 2019 12:58:12 +0930 Subject: [PATCH 11/11] option_static_remotekey: update to latest draft. https://github.com/lightningnetwork/lightning-rfc/pull/642/commits/531c8d7d9b01ab610b8a73a0deba1b9e9c83e1ed In this one, we always send my_current_per_commitment_point, though it's ignored. And we have our official feature numbers. Signed-off-by: Rusty Russell --- channeld/channeld.c | 31 ++++++++++++++++++---------- common/features.c | 3 ++- common/features.h | 8 +++---- common/keyset.c | 4 ++-- hsmd/hsmd.c | 2 +- lightningd/opening_control.c | 2 +- onchaind/onchaind.c | 2 +- tests/test_connection.py | 19 +++++++---------- tests/test_misc.py | 3 ++- wire/extracted_peer_experimental_csv | 3 ++- 10 files changed, 42 insertions(+), 35 deletions(-) diff --git a/channeld/channeld.c b/channeld/channeld.c index a2dcc2ea7a73..9e801e51a70c 100644 --- a/channeld/channeld.c +++ b/channeld/channeld.c @@ -2013,7 +2013,7 @@ static void resend_commitment(struct peer *peer, const struct changed_htlc *last peer->revocations_received); } -/* BOLT-930a9b44076a8f25a8626b31b3d5a55c0888308c #2: +/* BOLT-531c8d7d9b01ab610b8a73a0deba1b9e9c83e1ed #2: * * A receiving node: * - if `option_static_remotekey` applies to the commitment transaction: @@ -2076,7 +2076,7 @@ static void check_future_dataloss_fields(struct peer *peer, peer_failed(peer->pps, &peer->channel_id, "Awaiting unilateral close"); } -/* BOLT-930a9b44076a8f25a8626b31b3d5a55c0888308c #2: +/* BOLT-531c8d7d9b01ab610b8a73a0deba1b9e9c83e1ed #2: * * A receiving node: * - if `option_static_remotekey` applies to the commitment transaction: @@ -2238,7 +2238,7 @@ static void peer_reconnect(struct peer *peer, get_per_commitment_point(peer->next_index[LOCAL] - 1, &my_current_per_commitment_point, NULL); - /* BOLT-930a9b44076a8f25a8626b31b3d5a55c0888308c #2: + /* BOLT-531c8d7d9b01ab610b8a73a0deba1b9e9c83e1ed #2: * * - upon reconnection: * - if a channel is in an error state: @@ -2254,12 +2254,17 @@ static void peer_reconnect(struct peer *peer, * of the next `commitment_signed` it expects to receive. * - MUST set `next_revocation_number` to the commitment number * of the next `revoke_and_ack` message it expects to receive. - * - if it supports `option_data_loss_protect` or `option_static_remotekey`: - * - MUST set `my_current_per_commitment_point` to its commitment - * point for the last signed commitment it received from its - * channel peer (i.e. the commitment_point corresponding to the - * commitment transaction the sender would use to unilaterally - * close). + * - if `option_static_remotekey` applies to the commitment transaction: + * - MUST set `my_current_per_commitment_point` to a valid point. + * - otherwise, if it supports `option_data_loss_protect`: + * - MUST set `my_current_per_commitment_point` to its commitment + * point for the last signed commitment it received from its + * channel peer (i.e. the commitment_point corresponding to the + * commitment transaction the sender would use to unilaterally + * close). + * - if `option_static_remotekey` applies to the commitment + * transaction, or the sending node supports + * `option_data_loss_protect`: * - if `next_revocation_number` equals 0: * - MUST set `your_last_per_commitment_secret` to all zeroes * - otherwise: @@ -2272,7 +2277,9 @@ static void peer_reconnect(struct peer *peer, (NULL, &peer->channel_id, peer->next_index[LOCAL], peer->revocations_received, - last_remote_per_commit_secret); + last_remote_per_commit_secret, + /* Can send any (valid) point here */ + &peer->remote_per_commit); } else #endif /* EXPERIMENTAL_FEATURES */ if (dataloss_protect) { @@ -2304,11 +2311,13 @@ static void peer_reconnect(struct peer *peer, #if EXPERIMENTAL_FEATURES if (peer->channel->option_static_remotekey) { + struct pubkey ignore; if (!fromwire_channel_reestablish_option_static_remotekey(msg, &channel_id, &next_commitment_number, &next_revocation_number, - &last_local_per_commitment_secret)) { + &last_local_per_commitment_secret, + &ignore)) { peer_failed(peer->pps, &peer->channel_id, "bad reestablish static_remotekey msg: %s %s", diff --git a/common/features.c b/common/features.c index 7cd520c8532d..d9b6c9fc284b 100644 --- a/common/features.c +++ b/common/features.c @@ -161,7 +161,8 @@ static const char *feature_name(const tal_t *ctx, size_t f) "option_upfront_shutdown_script", "option_gossip_queries", "option_var_onion_optin", - "option_gossip_queries_ex" }; + "option_gossip_queries_ex", + "option_static_remotekey" }; assert(f / 2 < ARRAY_SIZE(fnames)); return tal_fmt(ctx, "%s/%s", diff --git a/common/features.h b/common/features.h index 1514e940367f..e76ed0e758b5 100644 --- a/common/features.h +++ b/common/features.h @@ -25,7 +25,7 @@ const char **list_supported_features(const tal_t *ctx); bool feature_is_set(const u8 *features, size_t bit); void set_feature_bit(u8 **ptr, u32 bit); -/* BOLT-930a9b44076a8f25a8626b31b3d5a55c0888308c #9: +/* BOLT-531c8d7d9b01ab610b8a73a0deba1b9e9c83e1ed #9: * * Flags are numbered from the least-significant bit, at bit 0 (i.e. 0x1, * an _even_ bit). They are generally assigned in pairs so that features @@ -36,7 +36,7 @@ void set_feature_bit(u8 **ptr, u32 bit); #define COMPULSORY_FEATURE(x) ((x) & 0xFFFFFFFE) #define OPTIONAL_FEATURE(x) ((x) | 1) -/* BOLT-930a9b44076a8f25a8626b31b3d5a55c0888308c #9: +/* BOLT-531c8d7d9b01ab610b8a73a0deba1b9e9c83e1ed #9: * * ## Assigned `localfeatures` flags *... @@ -45,13 +45,13 @@ void set_feature_bit(u8 **ptr, u32 bit); * | 3 | `initial_routing_sync` |... * | 4/5 | `option_upfront_shutdown_script` |... * | 6/7 | `gossip_queries` |... - * | 48/49| `option_static_remotekey` |... + * | 12/13| `option_static_remotekey` |... */ #define LOCAL_DATA_LOSS_PROTECT 0 #define LOCAL_INITIAL_ROUTING_SYNC 2 #define LOCAL_UPFRONT_SHUTDOWN_SCRIPT 4 #define LOCAL_GOSSIP_QUERIES 6 -#define LOCAL_STATIC_REMOTEKEY 48 +#define LOCAL_STATIC_REMOTEKEY 12 /* BOLT-927c96daab2338b716708a57cd75c84a2d169e0e #9: * | Bits | Name |... diff --git a/common/keyset.c b/common/keyset.c index 2592b11ab91e..559f63d86b31 100644 --- a/common/keyset.c +++ b/common/keyset.c @@ -8,7 +8,7 @@ bool derive_keyset(const struct pubkey *per_commitment_point, bool option_static_remotekey, struct keyset *keyset) { - /* BOLT-930a9b44076a8f25a8626b31b3d5a55c0888308c #3: + /* BOLT-531c8d7d9b01ab610b8a73a0deba1b9e9c83e1ed #3: * * ### `localpubkey`, `local_htlcpubkey`, `remote_htlcpubkey`, `local_delayedpubkey`, and `remote_delayedpubkey` Derivation * @@ -27,7 +27,7 @@ bool derive_keyset(const struct pubkey *per_commitment_point, &keyset->self_payment_key)) return false; - /* BOLT-930a9b44076a8f25a8626b31b3d5a55c0888308c #3: + /* BOLT-531c8d7d9b01ab610b8a73a0deba1b9e9c83e1ed #3: * * ### `remotepubkey` Derivation * diff --git a/hsmd/hsmd.c b/hsmd/hsmd.c index 17c615cd2bd8..3a3b7fd02e8b 100644 --- a/hsmd/hsmd.c +++ b/hsmd/hsmd.c @@ -1385,7 +1385,7 @@ 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); - /* BOLT-930a9b44076a8f25a8626b31b3d5a55c0888308c #3: + /* BOLT-531c8d7d9b01ab610b8a73a0deba1b9e9c83e1ed #3: * * If `option_static_remotekey` is negotiated the `remotepubkey` * is simply the remote node's `payment_basepoint`, otherwise it is diff --git a/lightningd/opening_control.c b/lightningd/opening_control.c index 14b1dd9f83c9..456dd1c49bf1 100644 --- a/lightningd/opening_control.c +++ b/lightningd/opening_control.c @@ -198,7 +198,7 @@ wallet_commit_channel(struct lightningd *ld, /* old_remote_per_commit not valid yet, copy valid one. */ channel_info->old_remote_per_commit = channel_info->remote_per_commit; - /* BOLT-930a9b44076a8f25a8626b31b3d5a55c0888308c #2: + /* BOLT-531c8d7d9b01ab610b8a73a0deba1b9e9c83e1ed #2: * 1. type: 35 (`funding_signed`) * 2. data: * * [`channel_id`:`channel_id`] diff --git a/onchaind/onchaind.c b/onchaind/onchaind.c index 41dff8693d6f..16f723a7c61a 100644 --- a/onchaind/onchaind.c +++ b/onchaind/onchaind.c @@ -2485,7 +2485,7 @@ static void handle_unknown_commitment(const struct bitcoin_tx *tx, local_script = scriptpubkey_p2wpkh(tmpctx, &ks->other_payment_key); } else { - /* BOLT-930a9b44076a8f25a8626b31b3d5a55c0888308c #3: + /* BOLT-531c8d7d9b01ab610b8a73a0deba1b9e9c83e1ed #3: * * ### `remotepubkey` Derivation * diff --git a/tests/test_connection.py b/tests/test_connection.py index 042254e3e21a..982690d2a726 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -1345,7 +1345,7 @@ def test_peerinfo(node_factory, bitcoind): l1, l2 = node_factory.line_graph(2, fundchannel=False, opts={'may_reconnect': True}) lfeatures = 'aa' if EXPERIMENTAL_FEATURES: - lfeatures = '020000000008aa' + lfeatures = '28aa' # Gossiping but no node announcement yet assert l1.rpc.getpeer(l2.info['id'])['connected'] assert len(l1.rpc.getpeer(l2.info['id'])['channels']) == 0 @@ -1596,8 +1596,8 @@ def test_dataloss_protection(node_factory, bitcoind): feerates=(7500, 7500, 7500), allow_broken_log=True) if EXPERIMENTAL_FEATURES: - # features 1, 3, 5, 7, 11 and 49 (0x020000000008aa). - lf = "020000000008aa" + # features 1, 3, 5, 7, 11 and 13 (0x28aa). + lf = "28aa" else: # features 1, 3, 5 and 7 (0xaa). lf = "aa" @@ -1618,11 +1618,6 @@ def test_dataloss_protection(node_factory, bitcoind): orig_db = open(dbpath, "rb").read() l2.start() - if EXPERIMENTAL_FEATURES: - # No my_current_per_commitment_point with option_static_remotekey - my_current_per_commitment_point_regex = "" - else: - my_current_per_commitment_point_regex = "0[23][0-9a-f]{64}" # l1 should have sent WIRE_CHANNEL_REESTABLISH with extra fields. l1.daemon.wait_for_log(r"\[OUT\] 0088" # channel_id @@ -1635,8 +1630,8 @@ def test_dataloss_protection(node_factory, bitcoind): # trigger a fee-update and commit, hence this may not # be zero) "[0-9a-f]{64}" - # my_current_per_commitment_point (maybe) - + my_current_per_commitment_point_regex + "'$") + # my_current_per_commitment_point + "0[23][0-9a-f]{64}'$") # After an htlc, we should get different results (two more commits) l1.pay(l2, 200000000) @@ -1658,8 +1653,8 @@ def test_dataloss_protection(node_factory, bitcoind): "000000000000000[1-9]" # your_last_per_commitment_secret "[0-9a-f]{64}" - # my_current_per_commitment_point (maybe) - + my_current_per_commitment_point_regex + "'$") + # my_current_per_commitment_point + "0[23][0-9a-f]{64}'$") # Now, move l2 back in time. l2.stop() diff --git a/tests/test_misc.py b/tests/test_misc.py index e53c67a58c60..db943d181c33 100644 --- a/tests/test_misc.py +++ b/tests/test_misc.py @@ -1445,5 +1445,6 @@ def test_list_features_only(node_factory): 'option_upfront_shutdown_script/odd', 'option_gossip_queries/odd'] if EXPERIMENTAL_FEATURES: - expected.append('option_gossip_queries_ex/odd') + expected += ['option_gossip_queries_ex/odd', + 'option_static_remotekey/odd'] assert features == expected diff --git a/wire/extracted_peer_experimental_csv b/wire/extracted_peer_experimental_csv index 62edd5a36e49..51863997f73f 100644 --- a/wire/extracted_peer_experimental_csv +++ b/wire/extracted_peer_experimental_csv @@ -6,7 +6,8 @@ msgdata,channel_reestablish,next_revocation_number,u64, -msgdata,channel_reestablish,your_last_per_commitment_secret,byte,32,option_data_loss_protect +msgdata,channel_reestablish,your_last_per_commitment_secret,byte,32,option_data_loss_protect,option_static_remotekey - msgdata,channel_reestablish,my_current_per_commitment_point,point,,option_data_loss_protect +-msgdata,channel_reestablish,my_current_per_commitment_point,point,,option_data_loss_protect ++msgdata,channel_reestablish,my_current_per_commitment_point,point,,option_data_loss_protect,option_static_remotekey msgtype,announcement_signatures,259 msgdata,announcement_signatures,channel_id,channel_id, @@ -154,6 +168,11 @@