Skip to content

Commit

Permalink
wire: update to latest version of the spec.
Browse files Browse the repository at this point in the history
The main change here is that the previously-optional open/accept
fields and reestablish fields are now compulsory (everyone was
including them anyway).  In fact, the open/accept is a TLV
because it was actually the same format.

For more details, see lightning-rfc/f068dd0d8dfa5ae75feedd99f269e23be4777381

Changelog-Removed: protocol: support for optioned form of reestablish messages now compulsory.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
  • Loading branch information
rustyrussell authored and cdecker committed Jun 23, 2020
1 parent a66415a commit 93d04d0
Show file tree
Hide file tree
Showing 18 changed files with 123 additions and 164 deletions.
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ CCANDIR := ccan

# Where we keep the BOLT RFCs
BOLTDIR := ../lightning-rfc/
BOLTVERSION := 4107c69e315b4f33d5b00459bef919bcfaa64bf2
BOLTVERSION := 9e8e29af9b9a922eb114b2c716205d0772946e56

-include config.vars

Expand Down
2 changes: 1 addition & 1 deletion bitcoin/script.c
Original file line number Diff line number Diff line change
Expand Up @@ -662,7 +662,7 @@ u8 *bitcoin_wscript_htlc_receive(const tal_t *ctx,
* ## HTLC-Timeout and HTLC-Success Transactions
*
*...
* * `txin[0]` witness stack: `0 <remotehtlcsig> <localhtlcsig> <payment_preimage>` for HTLC-success, `0 <remotehtlcsig> <localhtlcsig> 0` for HTLC-timeout
* * `txin[0]` witness stack: `0 <remotehtlcsig> <localhtlcsig> <payment_preimage>` for HTLC-success, `0 <remotehtlcsig> <localhtlcsig> <>` for HTLC-timeout
*/
u8 **bitcoin_witness_htlc_timeout_tx(const tal_t *ctx,
const struct bitcoin_signature *localhtlcsig,
Expand Down
70 changes: 17 additions & 53 deletions channeld/channeld.c
Original file line number Diff line number Diff line change
Expand Up @@ -2204,8 +2204,7 @@ static void resend_commitment(struct peer *peer, const struct changed_htlc *last
* `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:
* - otherwise, if it supports `option_data_loss_protect`:
* - if `next_revocation_number` is greater than expected above,
* AND `your_last_per_commitment_secret` is correct for that
* `next_revocation_number` minus 1:
Expand Down Expand Up @@ -2266,8 +2265,7 @@ static void check_future_dataloss_fields(struct peer *peer,
* ...
* - 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, if it supports `option_data_loss_protect`:
*...
* - otherwise (`your_last_per_commitment_secret` or
* `my_current_per_commitment_point` do not match the expected values):
Expand Down Expand Up @@ -2440,41 +2438,33 @@ static void peer_reconnect(struct peer *peer,
* of the next `revoke_and_ack` message it expects to receive.
* - 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`:
* - otherwise:
* - 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:
* - MUST set `your_last_per_commitment_secret` to the last
* `per_commitment_secret` it received
* - if `next_revocation_number` equals 0:
* - MUST set `your_last_per_commitment_secret` to all zeroes
* - otherwise:
* - MUST set `your_last_per_commitment_secret` to the last
* `per_commitment_secret` it received
*/
if (peer->channel->option_static_remotekey) {
msg = towire_channel_reestablish_option_static_remotekey
msg = towire_channel_reestablish
(NULL, &peer->channel_id,
peer->next_index[LOCAL],
peer->revocations_received,
last_remote_per_commit_secret,
/* Can send any (valid) point here */
&peer->remote_per_commit);
} else if (dataloss_protect) {
msg = towire_channel_reestablish_option_data_loss_protect
} else {
msg = towire_channel_reestablish
(NULL, &peer->channel_id,
peer->next_index[LOCAL],
peer->revocations_received,
last_remote_per_commit_secret,
&my_current_per_commitment_point);
} else {
msg = towire_channel_reestablish
(NULL, &peer->channel_id,
peer->next_index[LOCAL],
peer->revocations_received);
}

sync_crypto_write(peer->pps, take(msg));
Expand All @@ -2494,43 +2484,17 @@ static void peer_reconnect(struct peer *peer,
msg) ||
capture_premature_msg(&premature_msgs, msg));

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,
&ignore)) {
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 if (dataloss_protect) {
if (!fromwire_channel_reestablish_option_data_loss_protect(msg,
if (!fromwire_channel_reestablish(msg,
&channel_id,
&next_commitment_number,
&next_revocation_number,
&last_local_per_commitment_secret,
&remote_current_per_commitment_point)) {
peer_failed(peer->pps,
&peer->channel_id,
"bad reestablish dataloss msg: %s %s",
wire_type_name(fromwire_peektype(msg)),
tal_hex(msg, msg));
}
} else {
if (!fromwire_channel_reestablish(msg, &channel_id,
&next_commitment_number,
&next_revocation_number)) {
peer_failed(peer->pps,
&peer->channel_id,
"bad reestablish msg: %s %s",
wire_type_name(fromwire_peektype(msg)),
tal_hex(msg, msg));
}
peer_failed(peer->pps,
&peer->channel_id,
"bad reestablish msg: %s %s",
wire_type_name(fromwire_peektype(msg)),
tal_hex(msg, msg));
}

status_debug("Got reestablish commit=%"PRIu64" revoke=%"PRIu64,
Expand Down
9 changes: 5 additions & 4 deletions closingd/closingd.c
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@ static void do_reconnect(struct per_peer_state *pps,
struct channel_id their_channel_id;
u64 next_local_commitment_number, next_remote_revocation_number;
struct pubkey my_current_per_commitment_point, next_commitment_point;
struct secret their_secret;

my_current_per_commitment_point = get_per_commitment_point(next_index[LOCAL]-1);

Expand All @@ -175,9 +176,7 @@ static void do_reconnect(struct per_peer_state *pps,
* of the next `revoke_and_ack` message it expects to receive.
*/

/* We're always allowed to send extra fields, so we send dataloss_protect
* even if we didn't negotiate it */
msg = towire_channel_reestablish_option_data_loss_protect(NULL, channel_id,
msg = towire_channel_reestablish(NULL, channel_id,
next_index[LOCAL],
revocations_received,
last_remote_per_commit_secret,
Expand All @@ -199,7 +198,9 @@ static void do_reconnect(struct per_peer_state *pps,

if (!fromwire_channel_reestablish(channel_reestablish, &their_channel_id,
&next_local_commitment_number,
&next_remote_revocation_number)) {
&next_remote_revocation_number,
&their_secret,
&next_commitment_point)) {
peer_failed(pps, channel_id,
"bad reestablish msg: %s %s",
wire_type_name(fromwire_peektype(channel_reestablish)),
Expand Down
6 changes: 3 additions & 3 deletions common/bigsize.c
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ size_t bigsize_get(const u8 *p, size_t max, bigsize_t *val)
}
*val = ((u64)p[1] << 8) + p[2];
if (*val < 0xfd) {
SUPERVERBOSE("decoded varint is not canonical");
SUPERVERBOSE("decoded bigsize is not canonical");
return 0;
}
return 3;
Expand All @@ -76,7 +76,7 @@ size_t bigsize_get(const u8 *p, size_t max, bigsize_t *val)
*val = ((u64)p[1] << 24) + ((u64)p[2] << 16)
+ ((u64)p[3] << 8) + p[4];
if ((*val >> 16) == 0) {
SUPERVERBOSE("decoded varint is not canonical");
SUPERVERBOSE("decoded bigsize is not canonical");
return 0;
}
return 5;
Expand All @@ -90,7 +90,7 @@ size_t bigsize_get(const u8 *p, size_t max, bigsize_t *val)
+ ((u64)p[5] << 24) + ((u64)p[6] << 16)
+ ((u64)p[7] << 8) + p[8];
if ((*val >> 32) == 0) {
SUPERVERBOSE("decoded varint is not canonical");
SUPERVERBOSE("decoded bigsize is not canonical");
return 0;
}
return 9;
Expand Down
2 changes: 1 addition & 1 deletion common/decode_array.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ struct short_channel_id *decode_short_ids(const tal_t *ctx, const u8 *encoded);

/* BOLT #7:
*
* `encoded_query_flags` is an array of bitfields, one varint per bitfield,
* `encoded_query_flags` is an array of bitfields, one bigsize per bitfield,
* one bitfield for each `short_channel_id`. Bits have the following meaning:
*
* | Bit Position | Meaning |
Expand Down
3 changes: 1 addition & 2 deletions common/features.c
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,8 @@ static const struct feature_style feature_styles[] = {
.copy_style = { [INIT_FEATURE] = FEATURE_REPRESENT,
[NODE_ANNOUNCE_FEATURE] = FEATURE_REPRESENT,
[BOLT11_FEATURE] = FEATURE_REPRESENT } },
/* FIXME: Spec is wrong, and Eclair doesn't include in channel_announce! */
/* BOLT #9:
* | 18/19 | `option_support_large_channel` |... INC+ ...
* | 18/19 | `option_support_large_channel` |... IN ...
*/
{ OPT_LARGE_CHANNELS,
.copy_style = { [INIT_FEATURE] = FEATURE_REPRESENT,
Expand Down
2 changes: 1 addition & 1 deletion common/features.h
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ u8 *featurebits_or(const tal_t *ctx, const u8 *f1 TAKES, const u8 *f2 TAKES);
*
* | 14/15 | `payment_secret` |... IN9 ...
* | 16/17 | `basic_mpp` |... IN9 ...
* | 18/19 | `option_support_large_channel` |... INC+ ...
* | 18/19 | `option_support_large_channel` |... IN ...
*/
#define OPT_PAYMENT_SECRET 14
#define OPT_BASIC_MPP 16
Expand Down
6 changes: 3 additions & 3 deletions common/test/run-bigsize.c
Original file line number Diff line number Diff line change
Expand Up @@ -171,19 +171,19 @@ void towire_u8_array(u8 **pptr UNNEEDED, const u8 *arr UNNEEDED, size_t num UNNE
* "name": "two byte not canonical",
* "value": 0,
* "bytes": "fd00fc",
* "exp_error": "decoded varint is not canonical"
* "exp_error": "decoded bigsize is not canonical"
* },
* {
* "name": "four byte not canonical",
* "value": 0,
* "bytes": "fe0000ffff",
* "exp_error": "decoded varint is not canonical"
* "exp_error": "decoded bigsize is not canonical"
* },
* {
* "name": "eight byte not canonical",
* "value": 0,
* "bytes": "ff00000000ffffffff",
* "exp_error": "decoded varint is not canonical"
* "exp_error": "decoded bigsize is not canonical"
* },
* {
* "name": "two byte short read",
Expand Down
32 changes: 19 additions & 13 deletions gossipd/queries.c
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ bool query_short_channel_ids(struct daemon *daemon,
/* BOLT #7:
* - MAY include an optional `query_flags`. If so:
* - MUST set `encoding_type`, as for `encoded_short_ids`.
* - Each query flag is a minimally-encoded varint.
* - Each query flag is a minimally-encoded bigsize.
* - MUST encode one query flag per `short_channel_id`.
*/
if (query_flags)
Expand Down Expand Up @@ -353,18 +353,15 @@ static void reply_channel_range(struct peer *peer,
{
/* BOLT #7:
*
* - For each `reply_channel_range`:
* - MUST respond with one or more `reply_channel_range`:
* - MUST set with `chain_hash` equal to that of `query_channel_range`,
* - MUST encode a `short_channel_id` for every open channel it
* knows in blocks `first_blocknum` to `first_blocknum` plus
* `number_of_blocks` minus one.
* - MUST limit `number_of_blocks` to the maximum number of blocks
* whose results could fit in `encoded_short_ids`
* - if does not maintain up-to-date channel information for
* `chain_hash`:
* - MUST set `complete` to 0.
* - MUST set `full_information` to 0.
* - otherwise:
* - SHOULD set `complete` to 1.
* - SHOULD set `full_information` to 1.
*/
struct tlv_reply_channel_range_tlvs *tlvs
= tlv_reply_channel_range_tlvs_new(tmpctx);
Expand Down Expand Up @@ -449,7 +446,7 @@ static bool queue_channel_ranges(struct peer *peer,
* * [`chain_hash`:`chain_hash`]
* * [`u32`:`first_blocknum`]
* * [`u32`:`number_of_blocks`]
* * [`byte`:`complete`]
* * [`byte`:`full_information`]
* * [`u16`:`len`]
* * [`len*byte`:`encoded_short_ids`]
*/
Expand Down Expand Up @@ -696,9 +693,18 @@ const u8 *handle_reply_channel_range(struct peer *peer, const u8 *msg)
*
* The receiver of `query_channel_range`:
*...
* - MUST respond with one or more `reply_channel_range` whose
* combined range cover the requested `first_blocknum` to
* `first_blocknum` plus `number_of_blocks` minus one.
* - the first `reply_channel_range` message:
* - MUST set `first_blocknum` less than or equal to the
* `first_blocknum` in `query_channel_range`
* - MUST set `first_blocknum` plus `number_of_blocks` greater than
* `first_blocknum` in `query_channel_range`.
* - successive `reply_channel_range` message:
* - MUST set `first_blocknum` to the previous `first_blocknum`
* plus `number_of_blocks`.
* - the final `reply_channel_range` message:
* - MUST have `first_blocknum` plus `number_of_blocks` equal or
* greater than the `query_channel_range` `first_blocknum` plus
* `number_of_blocks`.
*/
/* ie. They can be outside range we asked, but they must overlap! */
if (first_blocknum + number_of_blocks <= peer->range_first_blocknum
Expand Down Expand Up @@ -988,9 +994,9 @@ void maybe_send_query_responses(struct peer *peer)
* `reply_short_channel_ids_end`.
* - if does not maintain up-to-date channel information for
* `chain_hash`:
* - MUST set `complete` to 0.
* - MUST set `full_information` to 0.
* - otherwise:
* - SHOULD set `complete` to 1.
* - SHOULD set `full_information` to 1.
*/
/* FIXME: We consider ourselves to have complete knowledge. */
u8 *end = towire_reply_short_channel_ids_end(peer,
Expand Down
2 changes: 1 addition & 1 deletion gossipd/queries.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ void maybe_send_query_responses(struct peer *peer);

/* BOLT #7:
*
* `query_option_flags` is a bitfield represented as a minimally-encoded varint.
* `query_option_flags` is a bitfield represented as a minimally-encoded bigsize.
* Bits have the following meaning:
*
* | Bit Position | Meaning |
Expand Down
2 changes: 1 addition & 1 deletion onchaind/onchaind.c
Original file line number Diff line number Diff line change
Expand Up @@ -2270,7 +2270,7 @@ static void handle_our_unilateral(const struct tx_parts *tx,
* only be valid after that duration has passed) and
* witness:
*
* <local_delayedsig> 0
* <local_delayedsig> <>
*/
to_us = tx_to_us(out, delayed_payment_to_us, out,
to_self_delay[LOCAL], 0, NULL, 0,
Expand Down
Loading

0 comments on commit 93d04d0

Please sign in to comment.