diff --git a/Makefile b/Makefile index d186e8eb5..a61d2b684 100644 --- a/Makefile +++ b/Makefile @@ -24,7 +24,7 @@ CCANDIR := ccan # Where we keep the BOLT RFCs BOLTDIR := ../lightning-rfc/ -DEFAULT_BOLTVERSION := c876dac2b5038f6499154d0a739240b6ff5db70d +DEFAULT_BOLTVERSION := 93909f67f6a48ee3f155a6224c182e612dd5f187 # Can be overridden on cmdline. BOLTVERSION := $(DEFAULT_BOLTVERSION) diff --git a/channeld/channeld.c b/channeld/channeld.c index ddce25310..24c41b9d1 100644 --- a/channeld/channeld.c +++ b/channeld/channeld.c @@ -728,7 +728,8 @@ static void handle_peer_feechange(struct peer *peer, const u8 *msg) * A receiving node: *... * - if the sender is not responsible for paying the Bitcoin fee: - * - MUST fail the channel. + * - MUST send a `warning` and close the connection, or send an + * `error` and fail the channel. */ if (peer->channel->opener != REMOTE) peer_failed_warn(peer->pps, &peer->channel_id, @@ -742,7 +743,8 @@ static void handle_peer_feechange(struct peer *peer, const u8 *msg) * A receiving node: * - if the `update_fee` is too low for timely processing, OR is * unreasonably large: - * - SHOULD fail the channel. + * - MUST send a `warning` and close the connection, or send an + * `error` and fail the channel. */ if (!feerate_same_or_better(peer->channel, feerate, peer->feerate_min, peer->feerate_max)) @@ -757,7 +759,8 @@ static void handle_peer_feechange(struct peer *peer, const u8 *msg) * * - if the sender cannot afford the new fee rate on the receiving * node's current commitment transaction: - * - SHOULD fail the channel, + * - SHOULD send a `warning` and close the connection, or send an + * `error` and fail the channel. * - but MAY delay this check until the `update_fee` is committed. */ if (!channel_update_feerate(peer->channel, feerate)) @@ -1627,7 +1630,8 @@ static void handle_peer_commit_sig(struct peer *peer, const u8 *msg) * - once all pending updates are applied: * - if `signature` is not valid for its local commitment transaction * OR non-compliant with LOW-S-standard rule...: - * - MUST fail the channel. + * - MUST send a `warning` and close the connection, or send an + * `error` and fail the channel. */ if (!check_tx_sig(txs[0], 0, NULL, funding_wscript, &peer->channel->funding_pubkey[REMOTE], &commit_sig)) { @@ -1651,7 +1655,8 @@ static void handle_peer_commit_sig(struct peer *peer, const u8 *msg) *... * - if `num_htlcs` is not equal to the number of HTLC outputs in the * local commitment transaction: - * - MUST fail the channel. + * - MUST send a `warning` and close the connection, or send an + * `error` and fail the channel. */ if (tal_count(htlc_sigs) != tal_count(txs) - 1) peer_failed_warn(peer->pps, &peer->channel_id, @@ -1662,7 +1667,8 @@ static void handle_peer_commit_sig(struct peer *peer, const u8 *msg) * * - if any `htlc_signature` is not valid for the corresponding HTLC * transaction OR non-compliant with LOW-S-standard rule...: - * - MUST fail the channel. + * - MUST send a `warning` and close the connection, or send an + * `error` and fail the channel. */ for (i = 0; i < tal_count(htlc_sigs); i++) { u8 *wscript; @@ -1813,13 +1819,13 @@ static void handle_peer_revoke_and_ack(struct peer *peer, const u8 *msg) * A receiving node: * - if `per_commitment_secret` is not a valid secret key or does not * generate the previous `per_commitment_point`: - * - MUST fail the channel. + * - MUST send an `error` and fail the channel. */ memcpy(&privkey, &old_commit_secret, sizeof(privkey)); if (!pubkey_from_privkey(&privkey, &per_commit_point)) { - peer_failed_warn(peer->pps, &peer->channel_id, - "Bad privkey %s", - type_to_string(msg, struct privkey, &privkey)); + peer_failed_err(peer->pps, &peer->channel_id, + "Bad privkey %s", + type_to_string(msg, struct privkey, &privkey)); } if (!pubkey_eq(&per_commit_point, &peer->old_remote_per_commit)) { peer_failed_err(peer->pps, &peer->channel_id, @@ -1957,7 +1963,8 @@ static void handle_peer_fail_malformed_htlc(struct peer *peer, const u8 *msg) * * - if the `BADONION` bit in `failure_code` is not set for * `update_fail_malformed_htlc`: - * - MUST fail the channel. + * - MUST send a `warning` and close the connection, or send an + * `error` and fail the channel. */ if (!(failure_code & BADONION)) { peer_failed_warn(peer->pps, &peer->channel_id, @@ -2011,6 +2018,7 @@ static void handle_peer_shutdown(struct peer *peer, const u8 *shutdown) * feature, and the receiving node received a non-zero-length * `shutdown_scriptpubkey` in `open_channel` or `accept_channel`, and * that `shutdown_scriptpubkey` is not equal to `scriptpubkey`: + * - MAY send a `warning`. * - MUST fail the connection. */ /* openingd only sets this if feature was negotiated at opening. */ @@ -2018,10 +2026,10 @@ static void handle_peer_shutdown(struct peer *peer, const u8 *shutdown) && !memeq(scriptpubkey, tal_count(scriptpubkey), peer->remote_upfront_shutdown_script, tal_count(peer->remote_upfront_shutdown_script))) - peer_failed_err(peer->pps, &peer->channel_id, - "scriptpubkey %s is not as agreed upfront (%s)", - tal_hex(peer, scriptpubkey), - tal_hex(peer, peer->remote_upfront_shutdown_script)); + peer_failed_warn(peer->pps, &peer->channel_id, + "scriptpubkey %s is not as agreed upfront (%s)", + tal_hex(peer, scriptpubkey), + tal_hex(peer, peer->remote_upfront_shutdown_script)); /* We only accept an wrong_funding if: * 1. It was negotiated. @@ -2528,12 +2536,12 @@ static void check_future_dataloss_fields(struct peer *peer, * commitment transaction: * ... * - if `your_last_per_commitment_secret` does not match the expected values: - * - SHOULD fail the channel. + * - SHOULD send an `error` and fail the channel. * - 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): - * - SHOULD fail the channel. + * - SHOULD send an `error` and fail the channel. */ static void check_current_dataloss_fields(struct peer *peer, u64 next_revocation_number, @@ -2950,10 +2958,10 @@ skip_tlvs: * - if `next_revocation_number` is not equal to 1 greater * than the commitment number of the last `revoke_and_ack` the * receiving node has sent: - * - SHOULD fail the channel. + * - SHOULD send an `error` and fail the channel. * - if it has not sent `revoke_and_ack`, AND * `next_revocation_number` is not equal to 0: - * - SHOULD fail the channel. + * - SHOULD send an `error` and fail the channel. */ if (next_revocation_number == peer->next_index[LOCAL] - 2) { /* Don't try to retransmit revocation index -1! */ @@ -3021,7 +3029,7 @@ skip_tlvs: * - if `next_commitment_number` is not 1 greater than the * commitment number of the last `commitment_signed` message the * receiving node has sent: - * - SHOULD fail the channel. + * - SHOULD send an `error` and fail the channel. */ } else if (next_commitment_number != peer->next_index[REMOTE]) peer_failed_err(peer->pps, diff --git a/channeld/full_channel.c b/channeld/full_channel.c index ee7140e2b..5c7102b24 100644 --- a/channeld/full_channel.c +++ b/channeld/full_channel.c @@ -559,7 +559,8 @@ static enum channel_add_err add_htlc(struct channel *channel, *... * - if sending node sets `cltv_expiry` to greater or equal to * 500000000: - * - SHOULD fail the channel. + * - SHOULD send a `warning` and close the connection, or send an + * `error` and fail the channel. */ if (!blocks_to_abs_locktime(cltv_expiry, &htlc->expiry)) { return CHANNEL_ERR_INVALID_EXPIRY; @@ -584,7 +585,8 @@ static enum channel_add_err add_htlc(struct channel *channel, * A receiving node: * - receiving an `amount_msat` equal to 0, OR less than its own * `htlc_minimum_msat`: - * - SHOULD fail the channel. + * - SHOULD send a `warning` and close the connection, or send an + * `error` and fail the channel. */ if (amount_msat_eq(htlc->amount, AMOUNT_MSAT(0))) { return CHANNEL_ERR_HTLC_BELOW_MINIMUM; @@ -652,7 +654,8 @@ static enum channel_add_err add_htlc(struct channel *channel, * - if a sending node... adds more than receiver * `max_htlc_value_in_flight_msat` worth of offered HTLCs to its * local commitment transaction: - * - SHOULD fail the channel. + * - SHOULD send a `warning` and close the connection, or send an + * `error` and fail the channel. */ /* We don't enforce this for channel_force_htlcs: some might already @@ -670,7 +673,8 @@ static enum channel_add_err add_htlc(struct channel *channel, * - receiving an `amount_msat` that the sending node cannot afford at * the current `feerate_per_kw` (while maintaining its channel * reserve and any `to_local_anchor` and `to_remote_anchor` costs): - * - SHOULD fail the channel. + * - SHOULD send a `warning` and close the connection, or send an + * `error` and fail the channel. */ if (enforce_aggregate_limits) { struct amount_msat remainder; @@ -894,7 +898,8 @@ enum channel_remove_err channel_fulfill_htlc(struct channel *channel, * * - if the `payment_preimage` value in `update_fulfill_htlc` * doesn't SHA256 hash to the corresponding HTLC `payment_hash`: - * - MUST fail the channel. + * - MUST send a `warning` and close the connection, or send an + * `error` and fail the channel. */ if (!sha256_eq(&hash, &htlc->rhash)) return CHANNEL_ERR_BAD_PREIMAGE; @@ -905,7 +910,8 @@ enum channel_remove_err channel_fulfill_htlc(struct channel *channel, * * - if the `id` does not correspond to an HTLC in its current * commitment transaction: - * - MUST fail the channel. + * - MUST send a `warning` and close the connection, or send an + * `error` and fail the channel. */ if (!htlc_has(htlc, HTLC_FLAG(!htlc_owner(htlc), HTLC_F_COMMITTED))) { status_unusual("channel_fulfill_htlc: %"PRIu64" in state %s", @@ -957,7 +963,8 @@ enum channel_remove_err channel_fail_htlc(struct channel *channel, * A receiving node: * - if the `id` does not correspond to an HTLC in its current * commitment transaction: - * - MUST fail the channel. + * - MUST send a `warning` and close the connection, or send an + * `error` and fail the channel. */ if (!htlc_has(htlc, HTLC_FLAG(!htlc_owner(htlc), HTLC_F_COMMITTED))) { status_unusual("channel_fail_htlc: %"PRIu64" in state %s", @@ -1145,7 +1152,8 @@ static int change_htlcs(struct channel *channel, *... * - if the sender cannot afford the new fee rate on the receiving node's * current commitment transaction: - * - SHOULD fail the channel, + * - SHOULD send a `warning` and close the connection, or send an + * `error` and fail the channel. */ u32 approx_max_feerate(const struct channel *channel) { @@ -1257,7 +1265,8 @@ bool can_opener_afford_feerate(const struct channel *channel, u32 feerate_per_kw * * - if the sender cannot afford the new fee rate on the receiving * node's current commitment transaction: - * - SHOULD fail the channel + * - SHOULD send a `warning` and close the connection, or send an + * `error` and fail the channel. */ /* Note: sender == opener */ diff --git a/closingd/closingd.c b/closingd/closingd.c index 41349d80d..e2a8fcbff 100644 --- a/closingd/closingd.c +++ b/closingd/closingd.c @@ -291,7 +291,8 @@ receive_offer(struct per_peer_state *pps, * - if the `signature` is not valid for either variant of closing transaction * specified in [BOLT #3](03-transactions.md#closing-transaction) * OR non-compliant with LOW-S-standard rule...: - * - MUST fail the connection. + * - MUST send a `warning` and close the connection, or send an + * `error` and fail the channel. */ tx = close_tx(tmpctx, chainparams, pps, channel_id, local_wallet_index, diff --git a/common/decode_array.c b/common/decode_array.c index 8093282b7..7296d072a 100644 --- a/common/decode_array.c +++ b/common/decode_array.c @@ -33,10 +33,12 @@ struct short_channel_id *decode_short_ids(const tal_t *ctx, const u8 *encoded) * The receiver: * - if the first byte of `encoded_short_ids` is not a known encoding * type: - * - MAY fail the connection + * - MAY send a `warning`. + * - MAY close the connection. * - if `encoded_short_ids` does not decode into a whole number of * `short_channel_id`: - * - MAY fail the connection + * - MAY send a `warning`. + * - MAY close the connection. */ type = fromwire_u8(&encoded, &max); switch (type) { @@ -75,10 +77,12 @@ bigsize_t *decode_scid_query_flags(const tal_t *ctx, *... * - if the incoming message includes `query_short_channel_ids_tlvs`: * - if `encoding_type` is not a known encoding type: - * - MAY fail the connection + * - MAY send a `warning`. + * - MAY close the connection. * - if `encoded_query_flags` does not decode to exactly one flag per * `short_channel_id`: - * - MAY fail the connection. + * - MAY send a `warning`. + * - MAY close the connection. */ switch (qf->encoding_type) { case ARR_ZLIB: diff --git a/common/features.c b/common/features.c index 360946d74..777afbe3c 100644 --- a/common/features.c +++ b/common/features.c @@ -123,7 +123,8 @@ static const struct dependency feature_deps[] = { * `option_anchor_outputs` | ... | ... | `option_static_remotekey` */ { OPT_ANCHOR_OUTPUTS, OPT_STATIC_REMOTEKEY }, - /* BOLT #9: + /* FIXME: This dep was added later! */ + /* BOLT-master #9: * Name | Description | Context | Dependencies | *... * `option_anchors_zero_fee_htlc_tx` | ... | ... | `option_static_remotekey` diff --git a/common/ping.c b/common/ping.c index 9ff545ec9..4a38ce19d 100644 --- a/common/ping.c +++ b/common/ping.c @@ -16,7 +16,6 @@ bool check_ping_make_pong(const tal_t *ctx, const u8 *ping, u8 **pong) /* BOLT #1: * * A node receiving a `ping` message: - *... * - if `num_pong_bytes` is less than 65532: * - MUST respond by sending a `pong` message, with `byteslen` equal * to `num_pong_bytes`. diff --git a/common/read_peer_msg.c b/common/read_peer_msg.c index fcf403f91..d61abfd53 100644 --- a/common/read_peer_msg.c +++ b/common/read_peer_msg.c @@ -33,17 +33,24 @@ bool is_peer_error(const tal_t *ctx, const u8 *msg, * 0 (i.e. all bytes are 0), in which case it refers to all channels. * ... * The receiving node: - * - upon receiving `error`: - * - MUST fail the channel referred to by the error message, if that - * channel is with the sending node. - * - if no existing channel is referred to by the message: - * - MUST ignore the message. + * - upon receiving `error`: + * - if `channel_id` is all zero: + * - MUST fail all channels with the sending node. + * - otherwise: + * - MUST fail the channel referred to by `channel_id`, if that channel is with the sending node. + * - upon receiving `warning`: + * - SHOULD log the message for later diagnosis. + * - MAY disconnect. + * - MAY reconnect after some delay to retry. + * - MAY attempt `shutdown` if permitted at this point. + * - if no existing channel is referred to by `channel_id`: + * - MUST ignore the message. */ - /* FIXME: The spec changed, so for *errors* all 0 is not special. - * But old gossipd would send these, so we turn them into warnings */ - if (channel_id_is_all(&err_chanid)) - *warning = true; - else if (!channel_id_eq(&err_chanid, channel_id)) + + /* FIXME: We don't actually free all channels at once, they'll + * have to error each in turn. */ + if (!channel_id_is_all(&err_chanid) + && !channel_id_eq(&err_chanid, channel_id)) *desc = tal_free(*desc); return true; diff --git a/common/wireaddr.h b/common/wireaddr.h index acdb2e4fe..783c5ba2d 100644 --- a/common/wireaddr.h +++ b/common/wireaddr.h @@ -26,10 +26,7 @@ struct sockaddr_un; * * * `1`: ipv4; data = `[4:ipv4_addr][2:port]` (length 6) * * `2`: ipv6; data = `[16:ipv6_addr][2:port]` (length 18) - * * `3`: Tor v2 onion service; data = `[10:onion_addr][2:port]` (length 12) - * * version 2 onion service addresses; Encodes an 80-bit, truncated `SHA-1` hash - * of a 1024-bit `RSA` public key for the onion service (a.k.a. Tor - * hidden service). + * * `3`: Deprecated (length 12). Used to contain Tor v2 onion services. * * `4`: Tor v3 onion service; data = `[35:onion_addr][2:port]` (length 37) * * version 3 ([prop224](https://gitweb.torproject.org/torspec.git/tree/proposals/224-rend-spec-ng.txt)) * onion service addresses; Encodes: diff --git a/connectd/connectd.c b/connectd/connectd.c index 0e3781f65..3add35b3f 100644 --- a/connectd/connectd.c +++ b/connectd/connectd.c @@ -364,7 +364,7 @@ struct io_plan *peer_connected(struct io_conn *conn, * - upon receiving unknown _odd_ feature bits that are non-zero: * - MUST ignore the bit. * - upon receiving unknown _even_ feature bits that are non-zero: - * - MUST fail the connection. + * - MUST close the connection. */ unsup = features_unsupported(daemon->our_features, their_features, INIT_FEATURE); diff --git a/connectd/multiplex.c b/connectd/multiplex.c index 71664438f..5e71dbe94 100644 --- a/connectd/multiplex.c +++ b/connectd/multiplex.c @@ -1225,7 +1225,6 @@ void send_manual_ping(struct daemon *daemon, const u8 *msg) /* BOLT #1: * * A node receiving a `ping` message: - *... * - if `num_pong_bytes` is less than 65532: * - MUST respond by sending a `pong` message, with `byteslen` equal * to `num_pong_bytes`. diff --git a/connectd/peer_exchange_initmsg.c b/connectd/peer_exchange_initmsg.c index 3f0df9946..eea73188a 100644 --- a/connectd/peer_exchange_initmsg.c +++ b/connectd/peer_exchange_initmsg.c @@ -75,7 +75,7 @@ static struct io_plan *peer_init_received(struct io_conn *conn, * The receiving node: * ... * - upon receiving `networks` containing no common chains - * - MAY fail the connection. + * - MAY close the connection. */ if (tlvs->networks) { if (!contains_common_chain(tlvs->networks)) { diff --git a/gossipd/queries.c b/gossipd/queries.c index 0ddb96bd2..de7d02fd6 100644 --- a/gossipd/queries.c +++ b/gossipd/queries.c @@ -258,7 +258,8 @@ const u8 *handle_query_short_channel_ids(struct peer *peer, const u8 *msg) * - if the incoming message includes * `query_short_channel_ids_tlvs`: * - if `encoding_type` is not a known encoding type: - * - MAY fail the connection + * - MAY send a `warning`. + * - MAY close the connection. */ flags = decode_scid_query_flags(tmpctx, tlvs->query_flags); if (!flags) { @@ -288,7 +289,8 @@ const u8 *handle_query_short_channel_ids(struct peer *peer, const u8 *msg) * - if it has not sent `reply_short_channel_ids_end` to a * previously received `query_short_channel_ids` from this * sender: - * - MAY fail the connection. + * - MAY send a `warning`. + * - MAY close the connection. */ if (peer->scid_queries || peer->scid_query_nodes) { return towire_warningfmt(peer, NULL, @@ -308,7 +310,8 @@ const u8 *handle_query_short_channel_ids(struct peer *peer, const u8 *msg) *... * - if `encoded_query_flags` does not decode to exactly one flag per * `short_channel_id`: - * - MAY fail the connection. + * - MAY send a `warning`. + * - MAY close the connection. */ if (!flags) { /* Pretend they asked for everything. */ diff --git a/gossipd/routing.c b/gossipd/routing.c index 09d866cf5..abe1d7a49 100644 --- a/gossipd/routing.c +++ b/gossipd/routing.c @@ -886,7 +886,7 @@ u8 *handle_channel_announcement(struct routing_state *rstate, { struct pending_cannouncement *pending; struct bitcoin_blkid chain_hash; - u8 *features, *err; + u8 *features, *warn; secp256k1_ecdsa_signature node_signature_1, node_signature_2; secp256k1_ecdsa_signature bitcoin_signature_1, bitcoin_signature_2; struct chan *chan; @@ -911,7 +911,7 @@ u8 *handle_channel_announcement(struct routing_state *rstate, &pending->node_id_2, &pending->bitcoin_key_1, &pending->bitcoin_key_2)) { - err = towire_warningfmt(rstate, NULL, + warn = towire_warningfmt(rstate, NULL, "Malformed channel_announcement %s", tal_hex(pending, pending->announce)); goto malformed; @@ -991,7 +991,7 @@ u8 *handle_channel_announcement(struct routing_state *rstate, } /* Note that if node_id_1 or node_id_2 are malformed, it's caught here */ - err = check_channel_announcement(rstate, + warn = check_channel_announcement(rstate, &pending->node_id_1, &pending->node_id_2, &pending->bitcoin_key_1, @@ -1001,13 +1001,15 @@ u8 *handle_channel_announcement(struct routing_state *rstate, &bitcoin_signature_1, &bitcoin_signature_2, pending->announce); - if (err) { + if (warn) { /* BOLT #7: * * - if `bitcoin_signature_1`, `bitcoin_signature_2`, * `node_signature_1` OR `node_signature_2` are invalid OR NOT * correct: - * - SHOULD fail the connection. + * - SHOULD send a `warning`. + * - MAY close the connection. + * - MUST ignore the message. */ goto malformed; } @@ -1049,7 +1051,7 @@ u8 *handle_channel_announcement(struct routing_state *rstate, malformed: tal_free(pending); *scid = NULL; - return err; + return warn; ignored: tal_free(pending); @@ -1458,7 +1460,7 @@ u8 *handle_channel_update(struct routing_state *rstate, const u8 *update TAKES, struct bitcoin_blkid chain_hash; u8 direction; struct pending_cannouncement *pending; - u8 *err; + u8 *warn; serialized = tal_dup_talarr(tmpctx, u8, update); if (!fromwire_channel_update(serialized, &signature, @@ -1467,10 +1469,10 @@ u8 *handle_channel_update(struct routing_state *rstate, const u8 *update TAKES, &channel_flags, &expiry, &htlc_minimum, &fee_base_msat, &fee_proportional_millionths)) { - err = towire_warningfmt(rstate, NULL, - "Malformed channel_update %s", - tal_hex(tmpctx, serialized)); - return err; + warn = towire_warningfmt(rstate, NULL, + "Malformed channel_update %s", + tal_hex(tmpctx, serialized)); + return warn; } direction = channel_flags & 0x1; @@ -1523,18 +1525,18 @@ u8 *handle_channel_update(struct routing_state *rstate, const u8 *update TAKES, return NULL; } - err = check_channel_update(rstate, owner, &signature, serialized); - if (err) { + warn = check_channel_update(rstate, owner, &signature, serialized); + if (warn) { /* BOLT #7: * * - if `signature` is not a valid signature, using `node_id` * of the double-SHA256 of the entire message following the * `signature` field (including unknown fields following * `fee_proportional_millionths`): + * - SHOULD send a `warning` and close the connection. * - MUST NOT process the message further. - * - SHOULD fail the connection. */ - return err; + return warn; } routing_add_channel_update(rstate, take(serialized), 0, peer, force); @@ -1712,13 +1714,14 @@ u8 *handle_node_announcement(struct routing_state *rstate, const u8 *node_ann, /* BOLT #7: * * - if `node_id` is NOT a valid compressed public key: - * - SHOULD fail the connection. + * - SHOULD send a `warning`. + * - MAY close the connection. * - MUST NOT process the message further. */ - u8 *err = towire_warningfmt(rstate, NULL, + u8 *warn = towire_warningfmt(rstate, NULL, "Malformed node_announcement %s", tal_hex(tmpctx, node_ann)); - return err; + return warn; } sha256_double(&hash, serialized + 66, tal_count(serialized) - 66); @@ -1731,10 +1734,10 @@ u8 *handle_node_announcement(struct routing_state *rstate, const u8 *node_ann, * message following the `signature` field * (including unknown fields following * `fee_proportional_millionths`): - * - MUST NOT process the message further. - * - SHOULD fail the connection. + * - SHOULD send a `warning` and close the connection. + * - MUST NOT process the message further. */ - u8 *err = towire_warningfmt(rstate, NULL, + u8 *warn = towire_warningfmt(rstate, NULL, "Bad signature for %s hash %s" " on node_announcement %s", type_to_string(tmpctx, @@ -1744,7 +1747,7 @@ u8 *handle_node_announcement(struct routing_state *rstate, const u8 *node_ann, struct sha256_double, &hash), tal_hex(tmpctx, node_ann)); - return err; + return warn; } wireaddrs = fromwire_wireaddr_array(tmpctx, addresses); @@ -1753,13 +1756,14 @@ u8 *handle_node_announcement(struct routing_state *rstate, const u8 *node_ann, * * - if `addrlen` is insufficient to hold the address * descriptors of the known types: - * - SHOULD fail the connection. + * - SHOULD send a `warning`. + * - MAY close the connection. */ - u8 *err = towire_warningfmt(rstate, NULL, + u8 *warn = towire_warningfmt(rstate, NULL, "Malformed wireaddrs %s in %s.", tal_hex(tmpctx, wireaddrs), tal_hex(tmpctx, node_ann)); - return err; + return warn; } /* May still fail, if we don't know the node. */ diff --git a/lightningd/channel_control.c b/lightningd/channel_control.c index a46b47eac..9c73949de 100644 --- a/lightningd/channel_control.c +++ b/lightningd/channel_control.c @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include @@ -289,18 +290,32 @@ static void peer_got_shutdown(struct channel *channel, const u8 *msg) return; } - /* FIXME: Add to spec that we must allow repeated shutdown! */ - tal_free(channel->shutdown_scriptpubkey[REMOTE]); - channel->shutdown_scriptpubkey[REMOTE] = scriptpubkey; - + /* BOLT #2: + * A receiving node: + *... + * - if the `scriptpubkey` is not in one of the above forms: + * - SHOULD send a `warning`. + */ if (!valid_shutdown_scriptpubkey(scriptpubkey, anysegwit, anchors)) { - channel_fail_permanent(channel, - REASON_PROTOCOL, - "Bad shutdown scriptpubkey %s", + u8 *warning = towire_warningfmt(NULL, + &channel->cid, + "Bad shutdown scriptpubkey %s", + tal_hex(tmpctx, scriptpubkey)); + + /* Get connectd to send warning, and then allow reconnect. */ + subd_send_msg(ld->connectd, + take(towire_connectd_peer_final_msg(NULL, + &channel->peer->id, + warning))); + channel_fail_reconnect(channel, "Bad shutdown scriptpubkey %s", tal_hex(tmpctx, scriptpubkey)); return; } + /* FIXME: Add to spec that we must allow repeated shutdown! */ + tal_free(channel->shutdown_scriptpubkey[REMOTE]); + channel->shutdown_scriptpubkey[REMOTE] = scriptpubkey; + /* If we weren't already shutting down, we are now */ if (channel->state != CHANNELD_SHUTTING_DOWN) channel_set_state(channel, diff --git a/lightningd/dual_open_control.c b/lightningd/dual_open_control.c index 6eaecd098..d7620dfce 100644 --- a/lightningd/dual_open_control.c +++ b/lightningd/dual_open_control.c @@ -15,6 +15,7 @@ #include #include #include +#include #include #include #include @@ -1324,13 +1325,21 @@ static void handle_peer_wants_to_close(struct subd *dualopend, * A receiving node: *... * - if the `scriptpubkey` is not in one of the above forms: - * - SHOULD fail the connection. + * - SHOULD send a `warning` */ if (!valid_shutdown_scriptpubkey(scriptpubkey, anysegwit, anchors)) { - channel_fail_permanent(channel, - REASON_PROTOCOL, - "Bad shutdown scriptpubkey %s", - tal_hex(channel, scriptpubkey)); + u8 *warning = towire_warningfmt(NULL, + &channel->cid, + "Bad shutdown scriptpubkey %s", + tal_hex(tmpctx, scriptpubkey)); + + /* Get connectd to send warning, and then allow reconnect. */ + subd_send_msg(ld->connectd, + take(towire_connectd_peer_final_msg(NULL, + &channel->peer->id, + warning))); + channel_fail_reconnect(channel, "Bad shutdown scriptpubkey %s", + tal_hex(tmpctx, scriptpubkey)); return; } diff --git a/lightningd/peer_control.c b/lightningd/peer_control.c index 6bfd09f68..197ea9e41 100644 --- a/lightningd/peer_control.c +++ b/lightningd/peer_control.c @@ -329,24 +329,30 @@ void channel_errmsg(struct channel *channel, * * A sending node: *... - * - when `channel_id` is 0: - * - MUST fail all channels with the receiving node. - * - MUST close the connection. + * - when sending `error`: + * - MUST fail the channel(s) referred to by the error message. + * - MAY set `channel_id` to all zero to indicate all channels. */ /* FIXME: Close if it's an all-channels error sent or rcvd */ /* BOLT #1: * * A sending node: + *... * - when sending `error`: - * - MUST fail the channel referred to by the error message. + * - MUST fail the channel(s) referred to by the error message. + * - MAY set `channel_id` to all zero to indicate all channels. *... * The receiving node: * - upon receiving `error`: - * - MUST fail the channel referred to by the error message, - * if that channel is with the sending node. + * - if `channel_id` is all zero: + * - MUST fail all channels with the sending node. + * - otherwise: + * - MUST fail the channel referred to by `channel_id`, if that channel is with the + * sending node. */ + /* FIXME: We don't close all channels */ /* We should immediately forget the channel if we receive error during * CHANNELD_AWAITING_LOCKIN if we are fundee. */ if (!err_for_them && channel->opener == REMOTE diff --git a/lightningd/peer_htlcs.c b/lightningd/peer_htlcs.c index 7933192b9..0f506c3b9 100644 --- a/lightningd/peer_htlcs.c +++ b/lightningd/peer_htlcs.c @@ -1945,7 +1945,8 @@ static bool channel_added_their_htlc(struct channel *channel, /* BOLT #2: * * - receiving an `amount_msat` equal to 0, OR less than its own `htlc_minimum_msat`: - * - SHOULD fail the channel. + * - SHOULD send a `warning` and close the connection, or send an + * `error` and fail the channel. */ if (amount_msat_eq(added->amount, AMOUNT_MSAT(0)) || amount_msat_less(added->amount, channel->our_config.htlc_minimum)) { @@ -2280,7 +2281,8 @@ void peer_got_revoke(struct channel *channel, const u8 *msg) * * - if the `per_commitment_secret` was not generated by the protocol * in [BOLT #3](03-transactions.md#per-commitment-secret-requirements): - * - MAY fail the channel. + * - MAY send a `warning` and close the connection, or send an + * `error` and fail the channel. */ if (!wallet_shachain_add_hash(ld->wallet, &channel->their_shachain, @@ -2488,6 +2490,7 @@ void htlcs_notify_new_block(struct lightningd *ld, u32 height) * * - if an HTLC which it offered is in either node's current * commitment transaction, AND is past this timeout deadline: + * - SHOULD send an `error` to the receiving peer (if connected). * - MUST fail the channel. */ /* FIXME: use db to look this up in one go (earliest deadline per-peer) */ @@ -2532,6 +2535,7 @@ void htlcs_notify_new_block(struct lightningd *ld, u32 height) *... * - if an HTLC it has fulfilled is in either node's current commitment * transaction, AND is past this fulfillment deadline: + * - SHOULD send an `error` to the offering peer (if connected). * - MUST fail the channel. */ do { diff --git a/openingd/dualopend.c b/openingd/dualopend.c index 0f95e3645..2bf7d6010 100644 --- a/openingd/dualopend.c +++ b/openingd/dualopend.c @@ -1202,8 +1202,7 @@ static u8 *opening_negotiate_msg(const tal_t *ctx, struct state *state) &err, &warning)) { /* BOLT #1: * - * - if no existing channel is referred to by the - * message: + * - if no existing channel is referred to by `channel_id`: * - MUST ignore the message. */ /* In this case, is_peer_error returns true, but sets @@ -1850,13 +1849,14 @@ static u8 *accepter_commits(struct state *state, * The recipient: * - if `signature` is incorrect OR non-compliant with LOW-S-standard * rule...: - * - MUST fail the channel. + * - MUST send a `warning` and close the connection, or send an + * `error` and fail the channel. */ if (!check_tx_sig(local_commit, 0, NULL, wscript, &state->their_funding_pubkey, &remote_sig)) { /* BOLT #1: * - * ### The `error` Message + * ### The `error` and `warning` Messages *... * - when failure was caused by an invalid signature check: * - SHOULD include the raw, hex-encoded transaction in reply @@ -2575,14 +2575,15 @@ static u8 *opener_commits(struct state *state, * The recipient: * - if `signature` is incorrect OR non-compliant with LOW-S-standard * rule...: - * - MUST fail the channel. + * - MUST send a `warning` and close the connection, or send an + * `error` and fail the channel. */ if (!check_tx_sig(local_commit, 0, NULL, wscript, &state->their_funding_pubkey, &remote_sig)) { /* BOLT #1: * - * ### The `error` Message + * ### The `error` and `warning` Messages *... * - when failure was caused by an invalid signature check: * - SHOULD include the raw, hex-encoded transaction in reply @@ -3588,7 +3589,7 @@ static void do_reconnect_dance(struct state *state) /* BOLT #2: * - if it has not sent `revoke_and_ack`, AND * `next_revocation_number` is not equal to 0: - * - SHOULD fail the channel. + * - SHOULD send an `error` and fail the channel. */ /* It's possible that we've opened an outdated copy of the * database, and the peer is very much ahead of us. diff --git a/openingd/openingd.c b/openingd/openingd.c index 04807cd5d..5a83ae1c1 100644 --- a/openingd/openingd.c +++ b/openingd/openingd.c @@ -192,8 +192,7 @@ static u8 *opening_negotiate_msg(const tal_t *ctx, struct state *state, &err, &warning)) { /* BOLT #1: * - * - if no existing channel is referred to by the - * message: + * - if no existing channel is referred to by `channel_id`: * - MUST ignore the message. */ /* In this case, is_peer_error returns true, but sets @@ -1107,7 +1106,8 @@ static u8 *fundee_channel(struct state *state, const u8 *open_channel_msg) * The recipient: * - if `signature` is incorrect OR non-compliant with LOW-S-standard * rule...: - * - MUST fail the channel. + * - MUST send a `warning` and close the connection, or send an + * `error` and fail the channel. */ local_commit = initial_channel_tx(state, &wscript, state->channel, &state->first_per_commitment_point[LOCAL], @@ -1125,7 +1125,7 @@ static u8 *fundee_channel(struct state *state, const u8 *open_channel_msg) &theirsig)) { /* BOLT #1: * - * ### The `error` Message + * ### The `error` and `warning` Messages *... * - when failure was caused by an invalid signature check: * - SHOULD include the raw, hex-encoded transaction in reply diff --git a/tests/test_closing.py b/tests/test_closing.py index 0013cde15..36c510b63 100644 --- a/tests/test_closing.py +++ b/tests/test_closing.py @@ -1,5 +1,4 @@ from fixtures import * # noqa: F401,F403 -from flaky import flaky from pyln.client import RpcError, Millisatoshi from shutil import copyfile from pyln.testing.utils import SLOW_MACHINE @@ -3186,31 +3185,27 @@ def test_shutdown(node_factory): l1.rpc.stop() -@flaky @pytest.mark.developer("needs to set upfront_shutdown_script") def test_option_upfront_shutdown_script(node_factory, bitcoind, executor): - # There's a workaround in channeld, that it treats incoming errors - # before both sides are locked in as warnings; this happens in - # this test, so l1 reports the error as a warning! l1 = node_factory.get_node(start=False, allow_warning=True) # Insist on upfront script we're not going to match. # '0014' + l1.rpc.call('dev-listaddrs', [10])['addresses'][-1]['bech32_redeemscript'] l1.daemon.env["DEV_OPENINGD_UPFRONT_SHUTDOWN_SCRIPT"] = "00143d43d226bcc27019ade52d7a3dc52a7ac1be28b8" l1.start() - l2 = node_factory.get_node() + l2 = node_factory.get_node(allow_warning=True) l1.rpc.connect(l2.info['id'], 'localhost', l2.port) l1.fundchannel(l2, 1000000, False) - # This will block, as l12 will send an error but l2 will retry. + # This will block, as l1 will send a warning but l2 will retry. fut = executor.submit(l1.rpc.close, l2.info['id']) - # l2 will close unilaterally when it dislikes shutdown script. - l1.daemon.wait_for_log(r'scriptpubkey .* is not as agreed upfront \(00143d43d226bcc27019ade52d7a3dc52a7ac1be28b8\)') + # l2 will send a warning when it dislikes shutdown script. + l1.daemon.wait_for_log(r'WARNING.*scriptpubkey .* is not as agreed upfront \(00143d43d226bcc27019ade52d7a3dc52a7ac1be28b8\)') - # Clear channel. - wait_for(lambda: len(bitcoind.rpc.getrawmempool()) != 0) - bitcoind.generate_block(1) + # Close from l2's side and clear channel. + l2.rpc.close(l1.info['id'], unilateraltimeout=1) + bitcoind.generate_block(1, wait_for_mempool=1) fut.result(TIMEOUT) wait_for(lambda: [c['state'] for c in only_one(l1.rpc.listpeers()['peers'])['channels']] == ['ONCHAIN']) wait_for(lambda: [c['state'] for c in only_one(l2.rpc.listpeers()['peers'])['channels']] == ['ONCHAIN']) @@ -3219,14 +3214,12 @@ def test_option_upfront_shutdown_script(node_factory, bitcoind, executor): l1.rpc.connect(l2.info['id'], 'localhost', l2.port) l1.fundchannel(l2, 1000000, False) - l2.rpc.close(l1.info['id']) + l2.rpc.close(l1.info['id'], unilateraltimeout=5) - # l2 will close unilaterally when it dislikes shutdown script. - l1.daemon.wait_for_log(r'scriptpubkey .* is not as agreed upfront \(00143d43d226bcc27019ade52d7a3dc52a7ac1be28b8\)') + # l2 will send warning unilaterally when it dislikes shutdown script. + l1.daemon.wait_for_log(r'WARNING.*scriptpubkey .* is not as agreed upfront \(00143d43d226bcc27019ade52d7a3dc52a7ac1be28b8\)') - # Clear channel. - wait_for(lambda: len(bitcoind.rpc.getrawmempool()) != 0) - bitcoind.generate_block(1) + bitcoind.generate_block(1, wait_for_mempool=1) wait_for(lambda: [c['state'] for c in only_one(l1.rpc.listpeers()['peers'])['channels']] == ['ONCHAIN', 'ONCHAIN']) wait_for(lambda: [c['state'] for c in only_one(l2.rpc.listpeers()['peers'])['channels']] == ['ONCHAIN', 'ONCHAIN']) diff --git a/wire/extracted_peer_02_warning.patch b/wire/extracted_peer_02_warning.patch deleted file mode 100644 index 6dc9e7c5b..000000000 --- a/wire/extracted_peer_02_warning.patch +++ /dev/null @@ -1,13 +0,0 @@ ---- wire/peer_exp_wire.csv 2021-01-14 11:00:27.526336550 +1030 -+++ - 2021-01-21 15:31:37.071118999 +1030 -@@ -10,6 +10,10 @@ - msgdata,error,channel_id,channel_id, - msgdata,error,len,u16, - msgdata,error,data,byte,len -+msgtype,warning,1 -+msgdata,warning,channel_id,channel_id, -+msgdata,warning,len,u16, -+msgdata,warning,data,byte,len - msgtype,ping,18 - msgdata,ping,num_pong_bytes,u16, - msgdata,ping,byteslen,u16,