option_static_remotekey: update to latest draft.

531c8d7d9b 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 <rusty@rustcorp.com.au>
5 years ago · 226e2aee48
10 changed files with 42 additions and 35 deletions
--- 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",
--- 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",
--- 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                            |...
--- 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
 	 *
--- 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
--- 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`]
--- 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
 		 *
--- 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()
--- 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
--- 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 @@