From 226e2aee483c919a9211639a8518b6a41ed40763 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 10 Sep 2019 12:58:12 +0930 Subject: [PATCH] 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 a2dcc2ea7..9e801e51a 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 7cd520c85..d9b6c9fc2 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 1514e9403..e76ed0e75 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 2592b11ab..559f63d86 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 17c615cd2..3a3b7fd02 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 14b1dd9f8..456dd1c49 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 41dff8693..16f723a7c 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 042254e3e..982690d2a 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 e53c67a58..db943d181 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 62edd5a36..51863997f 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 @@