diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index eefdca61522f..bf3258453b5f 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -28,8 +28,6 @@ jobs: if: | github.event.action != 'edited' || contains(github.event.pull_request.body, 'Changelog') - env: - BOLTDIR: bolts strategy: fail-fast: true steps: @@ -92,8 +90,6 @@ jobs: TEST_NETWORK: ${{ matrix.TEST_NETWORK }} run: | bash -x .github/scripts/setup.sh - # We're going to check BOLT quotes, so get the latest version - git clone https://github.com/lightning/bolts.git ../${BOLTDIR} - name: Configure run: ./configure --enable-debugbuild --enable-rust - name: Check source @@ -199,6 +195,8 @@ jobs: runs-on: ubuntu-24.04 needs: - compile + env: + BOLTDIR: bolts strategy: matrix: CFG: [compile-gcc] @@ -224,6 +222,8 @@ jobs: TEST_NETWORK: ${{ matrix.TEST_NETWORK }} run: | bash -x .github/scripts/setup.sh + # We're going to check BOLT quotes, so get the latest version + git clone https://github.com/lightning/bolts.git ../${BOLTDIR} - name: Download build uses: actions/download-artifact@v4 @@ -732,7 +732,7 @@ jobs: - name: Test env: - PYTEST_OPTS: ${{ env.PYTEST_OPTS_BASE }} --test-group-random-seed=42 + PYTEST_OPTS: ${{ env.PYTEST_OPTS_BASE }} --test-group-random-seed=42 run: | sg wireshark "uv run eatmydata pytest tests/ -n $(($(nproc) + 1)) ${PYTEST_OPTS} --test-group=${{ matrix.GROUP }} --test-group-count=${{ matrix.GROUP_COUNT }}" - name: Upload test results diff --git a/.gitignore b/.gitignore index ffff9f5622e9..b66a5ca7c34d 100644 --- a/.gitignore +++ b/.gitignore @@ -31,7 +31,7 @@ coverage docs/python ccan/config.h __pycache__ -config.vars +config.vars* monkeytype.sqlite3 # Ignore some generated binaries diff --git a/Makefile b/Makefile index 0e8fcfdf21e9..77132e6bcd8f 100644 --- a/Makefile +++ b/Makefile @@ -33,7 +33,7 @@ CCANDIR := ccan # Where we keep the BOLT RFCs BOLTDIR := ../bolts/ -DEFAULT_BOLTVERSION := 68881992b97f20aca29edf7a4d673b8e6a70379a +DEFAULT_BOLTVERSION := a3772650d8ebc06acf457fcadf97968ebfc4dfff # Can be overridden on cmdline. BOLTVERSION := $(DEFAULT_BOLTVERSION) diff --git a/channeld/channeld.c b/channeld/channeld.c index b84fd0f98dbd..af434119c6e4 100644 --- a/channeld/channeld.c +++ b/channeld/channeld.c @@ -5936,19 +5936,32 @@ static void peer_reconnect(struct peer *peer, "next_funding_txid not recognized."); } + /* "none of those channel_reestablish messages contain + * my_current_funding_locked or next_funding for a splice transaction" */ + bool is_splice_active = local_next_funding + || peer->splice_state->locked_ready[LOCAL] + || remote_next_funding + || (recv_tlvs + && recv_tlvs->my_current_funding_locked + && !bitcoin_txid_eq( + &recv_tlvs->my_current_funding_locked->my_current_funding_locked_txid, + &peer->channel->funding.txid)); + /* BOLT #2: * * - if `next_commitment_number` is 1 in both the - * `channel_reestablish` it sent and received: + * `channel_reestablish` it sent and received, and none of those + * `channel_reestablish` messages contain `my_current_funding_locked` or + * `next_funding` for a splice transaction: * - MUST retransmit `channel_ready`. * - otherwise: - * - MUST NOT retransmit `channel_ready`, but MAY send - * `channel_ready` with a different `short_channel_id` - * `alias` field. + * - MUST NOT retransmit `channel_ready`, but MAY send `channel_ready` with + * a different `short_channel_id` `alias` field. */ if (peer->channel_ready[LOCAL] && peer->next_index[LOCAL] == 1 - && next_commitment_number == 1) { + && next_commitment_number == 1 + && !is_splice_active) { struct tlv_channel_ready_tlvs *tlvs = tlv_channel_ready_tlvs_new(tmpctx); tlvs->short_channel_id = &peer->local_alias; @@ -6072,9 +6085,9 @@ static void peer_reconnect(struct peer *peer, /* BOLT #2: * * - otherwise: - * - if `next_commitment_number` is not 1 greater than the - * commitment number of the last `commitment_signed` message the - * receiving node has sent: + * - if `next_commitment_number` is not equal to the + * commitment number of the next `commitment_signed` that the + * receiving node would send: * - SHOULD send an `error` and fail the channel. */ } else if (next_commitment_number != peer->next_index[REMOTE]) diff --git a/common/bech32_util.c b/common/bech32_util.c index bae0f1e814dc..b7533d12e948 100644 --- a/common/bech32_util.c +++ b/common/bech32_util.c @@ -68,7 +68,7 @@ bool from_bech32_charset(const tal_t *ctx, u5 *u5data; const char *sep; bool upper = false, lower = false; - size_t datalen; + size_t datalen, nbits, trailing; sep = memchr(bech32, '1', bech32_len); if (!sep) @@ -94,6 +94,18 @@ bool from_bech32_charset(const tal_t *ctx, if (upper && lower) goto fail; + /* Padding: converting N 5-bit groups to bytes leaves (N*5 % 8) trailing + * bits. These must be zero and fewer than 5 (otherwise a full 5-bit + * group is wasted as padding, which is invalid). */ + nbits = datalen * 5; + trailing = nbits % 8; + if (trailing >= 5) + goto fail; + for (size_t i = nbits - trailing; i < nbits; i++) { + if (get_u5_bit(u5data, i)) + goto fail; + } + *data = tal_arr(ctx, u8, 0); bech32_pull_bits(data, u5data, tal_bytelen(u5data) * 5); tal_free(u5data); diff --git a/common/bolt11.c b/common/bolt11.c index 7698f497b615..2390fe37be88 100644 --- a/common/bolt11.c +++ b/common/bolt11.c @@ -185,7 +185,7 @@ static const char *decode_p(struct bolt11 *b11, /* BOLT #11: * * A reader... - * - MUST fail the payment if any mandatory field (`p`, `h`, `s`, `n`) + * - MUST fail the payment if any field with fixed `data_length` (`p`, `h`, `s`, `n`) * does not have the correct length (52, 52, 52, 53). */ return pull_expected_length(b11, hu5, data, field_len, 52, 'p', @@ -239,7 +239,7 @@ static const char *decode_h(struct bolt11 *b11, /* BOLT #11: * * A reader... - * - MUST fail the payment if any mandatory field (`p`, `h`, `s`, `n`) + * - MUST fail the payment if any field with fixed `data_length` (`p`, `h`, `s`, `n`) * does not have the correct length (52, 52, 52, 53). */ err = pull_expected_length(b11, hu5, data, field_len, 52, 'h', have_h, &hash); @@ -324,7 +324,7 @@ static const char *decode_n(struct bolt11 *b11, /* BOLT #11: * * A reader... - * - MUST fail the payment if any mandatory field (`p`, `h`, `s`, `n`) + * - MUST fail the payment if any field with fixed `data_length` (`p`, `h`, `s`, `n`) * does not have the correct length (52, 52, 52, 53). */ err = pull_expected_length(b11, hu5, data, field_len, 53, 'n', have_n, &b11->receiver_id.k); @@ -360,7 +360,7 @@ static const char *decode_s(struct bolt11 *b11, /* BOLT #11: * * A reader... - * - MUST fail the payment if any mandatory field (`p`, `h`, `s`, `n`) + * - MUST fail the payment if any field with fixed `data_length` (`p`, `h`, `s`, `n`) * does not have the correct length (52, 52, 52, 53). */ err = pull_expected_length(b11, hu5, data, field_len, 52, 's', have_s, &secret); @@ -876,7 +876,7 @@ struct bolt11 *bolt11_decode_nosig(const tal_t *ctx, const char *str, * * 1. `timestamp`: seconds-since-1970 (35 bits, big-endian) * 1. zero or more tagged parts - * 1. `signature`: Bitcoin-style signature of above (520 bits) + * 1. `signature`: compact ECDSA/secp256k1 signature of the above (520 bits: 64-byte R||S + 1-byte recovery id) */ err = pull_uint(&hu5, &data, &data_len, &b11->timestamp, 35, false); if (err) @@ -999,13 +999,12 @@ struct bolt11 *bolt11_decode(const tal_t *ctx, const char *str, /* BOLT #11: * - * A writer...MUST set `signature` to a valid 512-bit - * secp256k1 signature of the SHA2 256-bit hash of the - * human-readable part, represented as UTF-8 bytes, - * concatenated with the data part (excluding the signature) - * with 0 bits appended to pad the data to the next byte - * boundary, with a trailing byte containing the recovery ID - * (0, 1, 2, or 3). + * A writer...MUST set `signature` to a valid compact ECDSA signature over + * secp256k1 of the SHA-256 hash of: the human-readable + * part (as UTF-8 bytes) concatenated with the data + * part (excluding the signature), with 0 bits appended to pad to a byte + * boundary. The signature is encoded as 64 bytes (R || S), followed by a + * trailing 1-byte recovery id in {0,1,2,3}. */ data_len = tal_count(sigdata); err = pull_bits(NULL, &sigdata, &data_len, sig_and_recid, 520, false); @@ -1031,7 +1030,7 @@ struct bolt11 *bolt11_decode(const tal_t *ctx, const char *str, * A reader... MUST check that the `signature` is valid (see * the `n` tagged field specified below). ... A reader... * MUST use the `n` field to validate the signature instead of - * performing signature recovery. + * performing public-key recovery. */ if (!have_n) { struct pubkey k; @@ -1040,7 +1039,7 @@ struct bolt11 *bolt11_decode(const tal_t *ctx, const char *str, &sig, (const u8 *)&hash)) return decode_fail(b11, fail, - "signature recovery failed"); + "public-key recovery failed"); node_id_from_pubkey(&b11->receiver_id, &k); } else { struct pubkey k; @@ -1314,7 +1313,7 @@ char *bolt11_encode_(const tal_t *ctx, * * 1. `timestamp`: seconds-since-1970 (35 bits, big-endian) * 1. zero or more tagged parts - * 1. `signature`: Bitcoin-style signature of above (520 bits) + * 1. `signature`: compact ECDSA/secp256k1 signature of the above (520 bits: 64-byte R||S + 1-byte recovery id) */ push_varlen_uint(&data, b11->timestamp, 35); diff --git a/common/bolt12.c b/common/bolt12.c index 4621e62f6960..f0b7ab0252e8 100644 --- a/common/bolt12.c +++ b/common/bolt12.c @@ -10,7 +10,8 @@ #include #include -/* If chains is NULL, max_num_chains is ignored */ +/* If chains is NULL, max_num_chains is ignored. + * If must_be_chain is NULL, only structural validity is checked. */ bool bolt12_chains_match(const struct bitcoin_blkid *chains, size_t max_num_chains, const struct chainparams *must_be_chain) @@ -28,9 +29,16 @@ bool bolt12_chains_match(const struct bitcoin_blkid *chains, * - if the node does not accept bitcoin invoices: * - MUST NOT respond to the offer * - otherwise: (`offer_chains` is set): - * - if the node does not accept invoices for any of the `chains`: + * - if the node does not accept invoices for at least one of the `chains`: * - MUST NOT respond to the offer */ + if (chains && max_num_chains == 0) + return false; + + /* No specific chain required: structurally valid. */ + if (!must_be_chain) + return true; + if (!chains) { max_num_chains = 1; chains = &chainparams_for_network("bitcoin")->genesis_blockhash; @@ -183,6 +191,18 @@ struct tlv_offer *offer_decode(const tal_t *ctx, return NULL; } + /* BOLT #12: + * - otherwise: (`offer_chains` is set): + * - if the node does not accept invoices for at least one of the `chains`: + * - MUST NOT respond to the offer + */ + for (size_t i = 0; i < tal_count(offer->fields); i++) { + if (offer->fields[i].numtype == 2 && offer->fields[i].length == 0) { + *fail = tal_strdup(ctx, "offer_chains must have at least one entry"); + return tal_free(offer); + } + } + *fail = check_features_and_chain(ctx, our_features, must_be_chain, offer->offer_features, @@ -222,6 +242,16 @@ struct tlv_offer *offer_decode(const tal_t *ctx, return tal_free(offer); } + /* BOLT #12: + * + * - if `offer_amount` is set and is not greater than zero: + * - MUST NOT respond to the offer. + */ + if (offer->offer_amount && *offer->offer_amount == 0) { + *fail = tal_strdup(ctx, "offer_amount must be > 0"); + return tal_free(offer); + } + /* BOLT #12: * * - if `offer_currency` is set and `offer_amount` is not set: diff --git a/common/features.h b/common/features.h index ddc1eeb27ea4..d078f420a380 100644 --- a/common/features.h +++ b/common/features.h @@ -108,21 +108,20 @@ struct feature_set *feature_set_dup(const tal_t *ctx, * | 8/9 | `var_onion_optin` |... ASSUMED ... * | 10/11 | `gossip_queries_ex` |... IN ... * | 12/13 | `option_static_remotekey` |... ASSUMED ... - * | 14/15 | `payment_secret` |... IN9 ... + * | 14/15 | `payment_secret` |... ASSUMED ... * | 16/17 | `basic_mpp` |... IN9 ... * | 18/19 | `option_support_large_channel` |... IN ... - * | 22/23 | `option_anchors` |... IN ... - * | 24/25 | `option_route_blinding` |...IN9 ... + * | 22/23 | `option_anchors` |... INT ... + * | 24/25 | `option_route_blinding` |... IN9 ... * | 26/27 | `option_shutdown_anysegwit` |... IN ... * | 28/29 | `option_dual_fund` |... IN ... * | 34/35 | `option_quiesce` |... IN ... * | 38/39 | `option_onion_messages` |... IN ... * | 42/43 | `option_provide_storage` |... IN ... - * | 44/45 | `option_channel_type` |... IN ... - * | 46/47 | `option_scid_alias` | ... IN ... - * | 48/49 | `option_payment_metadata` |... 9 ... - * | 50/51 | `option_zeroconf` | ... IN ... - * | 60/61 | `option_simple_close` |... IN ... + * | 44/45 | `option_channel_type` |... ASSUMED ... + * | 46/47 | `option_scid_alias` |... INT ... + * | 48/49 | `option_payment_metadata` |... 9 ... + * | 50/51 | `option_zeroconf` |... INT ... * | 62/63 | `option_splice` |... IN ... */ #define OPT_DATA_LOSS_PROTECT 0 diff --git a/common/interactivetx.c b/common/interactivetx.c index 87473aeeebcb..7a3bde39ec9e 100644 --- a/common/interactivetx.c +++ b/common/interactivetx.c @@ -591,9 +591,9 @@ char *process_interactivetx_updates(const tal_t *ctx, * BOLT #2: * The receiving node: ... * - MUST fail the negotiation if:... - * - the `prevtx` and `prevtx_vout` are - * identical to a previously added (and not - * removed) input's + * - `prevtx` and `prevtx_vout` are + * identical to a previously added (and not + * removed) input */ bitcoin_txid(tx, &outpoint.txid); if (psbt_has_input(ictx->current_psbt, &outpoint)) diff --git a/common/sphinx.c b/common/sphinx.c index 66074296ffdf..8457c8664553 100644 --- a/common/sphinx.c +++ b/common/sphinx.c @@ -742,9 +742,8 @@ struct onionreply *create_onionreply(const tal_t *ctx, /* BOLT #4: * - * The node generating the error message (_erring node_) builds a return - * packet consisting of - * the following fields: + * The node generating the error message builds a _return packet_ consisting + * of the following fields: * * 1. data: * * [`32*byte`:`hmac`] @@ -790,8 +789,6 @@ struct onionreply *wrap_onionreply(const tal_t *ctx, * The erring node then generates a new key, using the key type `ammag`. * This key is then used to generate a pseudo-random stream, which is * in turn applied to the packet using `XOR`. - * - * The obfuscation step is repeated by every hop along the return path. */ subkey_from_hmac("ammag", shared_secret, &key); result->contents = tal_dup_talarr(result, u8, reply->contents); diff --git a/common/test/run-bolt11.c b/common/test/run-bolt11.c index fe2b3e2cb5b9..c493d7313fe7 100644 --- a/common/test/run-bolt11.c +++ b/common/test/run-bolt11.c @@ -671,7 +671,7 @@ int main(int argc, char *argv[]) * > lnbc2500u1pvjluezpp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqdq5xysxxatsyp3k7enxv4jsxqzpusp5zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zygs9qrsgqwgt7mcn5yqw3yx0w94pswkpq6j9uh6xfqqqtsk4tnarugeektd4hg5975x9am52rz4qskukxdmjemg92vvqz8nvmsye63r5ykel43pgz7zq0g2 */ assert(!bolt11_decode(tmpctx, "lnbc2500u1pvjluezpp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqdq5xysxxatsyp3k7enxv4jsxqzpusp5zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zygs9qrsgqwgt7mcn5yqw3yx0w94pswkpq6j9uh6xfqqqtsk4tnarugeektd4hg5975x9am52rz4qskukxdmjemg92vvqz8nvmsye63r5ykel43pgz7zq0g2", NULL, NULL, NULL, &fail)); - assert(streq(fail, "signature recovery failed")); + assert(streq(fail, "public-key recovery failed")); /* BOLT #11: * > ### String is too short. diff --git a/common/test/run-bolt12-encode-test.c b/common/test/run-bolt12-encode-test.c index 911ce003c53b..e19897ae5e46 100644 --- a/common/test/run-bolt12-encode-test.c +++ b/common/test/run-bolt12-encode-test.c @@ -429,15 +429,25 @@ int main(int argc, char *argv[]) /* BOLT #12: * - if `offer_amount` is set and `offer_description` is not set: * - MUST NOT respond to the offer. + * - if `offer_amount` is set and is not greater than zero: + * - MUST NOT respond to the offer. * - if `offer_currency` is set and `offer_amount` is not set: * - MUST NOT respond to the offer. * - if neither `offer_issuer_id` nor `offer_paths` are set: * - MUST NOT respond to the offer. */ + offer->offer_amount = tal(offer, u64); + *offer->offer_amount = 10000; + offer->offer_description = NULL; - print_invalid_offer(offer, "Missing offer_description and offer_amount"); + print_invalid_offer(offer, "Missing offer_description"); offer->offer_description = tal_utf8(tmpctx, "Test vectors"); + offer->offer_amount = tal(offer, u64); + *offer->offer_amount = 0; + print_invalid_offer(offer, "offer_amount must be > 0"); + offer->offer_amount = NULL; + offer->offer_currency = tal_utf8(offer, "USD"); print_invalid_offer(offer, "Missing offer_amount with offer_currency"); offer->offer_currency = NULL; diff --git a/common/test/run-bolt12-format-string-test.c b/common/test/run-bolt12-format-string-test.c index d517a60cf11d..d861b3aaaa01 100644 --- a/common/test/run-bolt12-format-string-test.c +++ b/common/test/run-bolt12-format-string-test.c @@ -128,6 +128,7 @@ int main(int argc, char *argv[]) * - SHOULD omit `offer_chains`, implying that bitcoin is only chain. * - if a specific minimum `offer_amount` is required for successful payment: * - MUST set `offer_amount` to the amount expected (per item). + * - MUST set `offer_amount` greater than zero. * - if the currency for `offer_amount` is that of all entries in `chains`: * - MUST specify `offer_amount` in multiples of the minimum lightning-payable unit * (e.g. milli-satoshis for bitcoin). diff --git a/devtools/bolt12-cli.c b/devtools/bolt12-cli.c index a72d57a1d71d..2564981825e1 100644 --- a/devtools/bolt12-cli.c +++ b/devtools/bolt12-cli.c @@ -95,6 +95,7 @@ static bool print_offer_amount(const struct bitcoin_blkid *chains, /* BOLT #12: * - if a specific minimum `offer_amount` is required for successful payment: * - MUST set `offer_amount` to the amount expected (per item). + * - MUST set `offer_amount` greater than zero. * - if the currency for `offer_amount` is that of all entries in `chains`: * - MUST specify `offer_amount` in multiples of the minimum lightning-payable unit * (e.g. milli-satoshis for bitcoin). diff --git a/devtools/check-bolt.c b/devtools/check-bolt.c index 95addc5915b9..04fc519efed7 100644 --- a/devtools/check-bolt.c +++ b/devtools/check-bolt.c @@ -206,10 +206,31 @@ static void fail_mismatch(const char *filename, const char *match = NULL; int matchlen; + if (verbose) { + fprintf(stderr, "%s:%u: BOLT mismatch in %s\n", filename, line, bolt->prefix); + + /* Show what we're trying to match */ + fprintf(stderr, "Looking for %zu parts:\n", tal_count(strings)); + for (size_t i = 0; i < tal_count(strings); i++) { + fprintf(stderr, " [%zu]: '%s'\n", i, strings[i]); + } + } + /* Figure out which substring didn't match, and how much to cut it */ - for (size_t i = 0; i < tal_count(strings); i++) { - if (strstr(bolt->contents, strings[i])) - continue; + bool all_parts_found = true; + for (size_t i = 0; i < tal_count(strings); i++) { + if (strstr(bolt->contents, strings[i])){ + if (verbose) { + fprintf(stderr, " ✓ [%zu] FOUND: '%s'\n", i, strings[i]); + } + + continue; + } + + all_parts_found = false; + if (verbose) { + fprintf(stderr, " ✗ [%zu] NOT FOUND: '%s'\n", i, strings[i]); + } /* OK, it doesn't match, truncate it until it does */ matchlen = strlen(strings[i]); @@ -220,17 +241,39 @@ static void fail_mismatch(const char *filename, break; matchlen--; } - break; + if (!verbose) { + break; + } + + if (match) { + fprintf(stderr, " Closest match (%d chars): '%.*s'...\n", + matchlen, matchlen, match); + fprintf(stderr, " Full context: '%.50s'\n", match); + } else { + fprintf(stderr, " No partial match found\n"); + } } - fprintf(stderr, "%s:%u:", filename, line); - if (match) { - fprintf(stderr, "Closest match: %.*s...[%.20s]\n", - matchlen, match, match + matchlen); + if (!verbose) { + fprintf(stderr, "%s:%u:", filename, line); + if (match) { + fprintf(stderr, "Closest match: %.*s...[%.20s]\n", + matchlen, match, match + matchlen); + } else { + fprintf(stderr, "Parts match, but not in this order\n"); + } } else { - fprintf(stderr, "Parts match, but not in this order\n"); + if (all_parts_found) { + fprintf(stderr, "All parts found individually, but not in sequence!\n"); + fprintf(stderr, "Check the order or spacing in your comment.\n"); + } + + /* Show some context from the source file */ + fprintf(stderr, "\nSource context (showing %zu chars):\n", len > 100 ? 100 : len); + fprintf(stderr, "'%.*s%s'\n", + (int)(len > 100 ? 100 : len), pos, len > 100 ? "..." : ""); } - exit(1); + exit(1); } static bool find_strings(const char *bolttext, char **strings, size_t nstrings) diff --git a/gossipd/gossmap_manage.c b/gossipd/gossmap_manage.c index eee55a15353a..131bebf32223 100644 --- a/gossipd/gossmap_manage.c +++ b/gossipd/gossmap_manage.c @@ -1438,12 +1438,12 @@ void gossmap_manage_channel_spent(struct gossmap_manage *gm, /* BOLT #7: * - once its funding output has been spent OR reorganized out: - * - SHOULD forget a channel after a 12-block delay. + * - SHOULD forget a channel after a 72-block delay. */ - cd.deadline = blockheight + 12; + cd.deadline = blockheight + 72; cd.scid = scid; - /* Remember locally so we can kill it in 12 blocks */ + /* Remember locally so we can kill it in 72 blocks */ status_trace("channel %s closing soon due" " to the funding outpoint being spent", fmt_short_channel_id(tmpctx, scid)); diff --git a/hsmd/libhsmd.c b/hsmd/libhsmd.c index f8d35a044ca6..69d30c9885fb 100644 --- a/hsmd/libhsmd.c +++ b/hsmd/libhsmd.c @@ -1016,13 +1016,12 @@ static u8 *handle_sign_invoice(struct hsmd_client *c, const u8 *msg_in) /* BOLT #11: * - * A writer... MUST set `signature` to a valid 512-bit - * secp256k1 signature of the SHA2 256-bit hash of the - * human-readable part, represented as UTF-8 bytes, - * concatenated with the data part (excluding the signature) - * with 0 bits appended to pad the data to the next byte - * boundary, with a trailing byte containing the recovery ID - * (0, 1, 2, or 3). + * A writer...MUST set `signature` to a valid compact ECDSA signature over + * secp256k1 of the SHA-256 hash of: the human-readable + * part (as UTF-8 bytes) concatenated with the data + * part (excluding the signature), with 0 bits appended to pad to a byte + * boundary. The signature is encoded as 64 bytes (R || S), followed by a + * trailing 1-byte recovery id in {0,1,2,3}. */ /* FIXME: Check invoice! */ diff --git a/lightningd/channel_gossip.c b/lightningd/channel_gossip.c index 5daa79b68080..1d78bb3b3a42 100644 --- a/lightningd/channel_gossip.c +++ b/lightningd/channel_gossip.c @@ -678,10 +678,10 @@ static void stash_remote_announce_sigs(struct channel *channel, * A node: * - If the `open_channel` message has the `announce_channel` bit set AND a * `shutdown` message has not been sent: - * - After `channel_ready` has been sent and received AND the funding - * transaction has enough confirmations to ensure that it won't be - * reorganized: - * - MUST send `announcement_signatures` for the funding transaction. + * - After `channel_ready` has been sent and received AND the funding + * transaction has enough confirmations to ensure that it won't be + * reorganized: + * - MUST send `announcement_signatures` for the funding transaction.... * - Otherwise: * - MUST NOT send the `announcement_signatures` message. */ diff --git a/lightningd/opening_control.c b/lightningd/opening_control.c index d7998197d3d4..b0cf27b806f1 100644 --- a/lightningd/opening_control.c +++ b/lightningd/opening_control.c @@ -161,7 +161,7 @@ wallet_commit_channel(struct lightningd *ld, * * Both peers: * ... - * - MUST use that `channel_type` for all commitment transactions. + * - MUST use the negotiated `channel_type` for all commitment transactions. */ /* i.e. We set it now for the channel permanently. */ if (channel_type_has(type, OPT_STATIC_REMOTEKEY)) diff --git a/openingd/dualopend.c b/openingd/dualopend.c index 733b0c2828bf..b33885a0ad2a 100644 --- a/openingd/dualopend.c +++ b/openingd/dualopend.c @@ -1833,9 +1833,8 @@ static bool run_tx_interactive(struct state *state, * BOLT #2: * The receiving node: ... * - MUST fail the negotiation if:... - * - the `prevtx` and `prevtx_vout` are - * identical to a previously added (and not - * removed) input's + * - `prevtx` and `prevtx_vout` are identical to a previously + * added (and not removed) input */ bitcoin_txid(tx, &outpoint.txid); if (psbt_has_input(psbt, &outpoint)) { @@ -2432,9 +2431,8 @@ static void accepter_start(struct state *state, const u8 *oc2_msg) /* BOLT #2: * The receiving node MUST fail the channel if: *... - * - It supports `channel_type` and `channel_type` was set: - * - if `type` is not suitable. - * - if `type` includes `option_zeroconf` and it does not trust the sender to open an unconfirmed channel. + * - the `channel_type` is not suitable. + * - the `channel_type` includes `option_zeroconf` and it does not trust the sender to open an unconfirmed channel. */ if (!open_tlv->channel_type) { negotiation_failed(state, @@ -2684,7 +2682,7 @@ static void accepter_start(struct state *state, const u8 *oc2_msg) } /* BOLT #2: - * - if `option_channel_type` was negotiated: + * The sender:... * - MUST set `channel_type` to the `channel_type` from `open_channel` */ a_tlv->channel_type = state->channel_type->features; @@ -3145,9 +3143,9 @@ static void opener_start(struct state *state, u8 *msg) } /* BOLT #2: - * - if `channel_type` is set, and `channel_type` was set in - * `open_channel`, and they are not equal types: - * - MUST fail the channel. + * The receiving node: + * - MUST fail the negotiation if:... + * - `channel_type` is not set. */ if (!a_tlv->channel_type) { negotiation_failed(state, @@ -3977,11 +3975,12 @@ static void do_reconnect_dance(struct state *state) * * - if it has sent `commitment_signed` for an * interactive transaction construction but it has - * not received `tx_signatures`: + * not received `tx_signatures`: + * - MUST include the `next_funding` TLV. * - MUST set `next_funding_txid` to the txid of that * interactive transaction. - * - otherwise: - * - MUST NOT set `next_funding_txid`. + * - if it has not received `commitment_signed` for this `next_funding_txid`: + * - MUST set the `commitment_signed` bit in `retransmit_flags`. */ tlvs = tlv_channel_reestablish_tlvs_new(tmpctx); if (!tx_state->remote_funding_sigs_rcvd) { @@ -4046,18 +4045,22 @@ static void do_reconnect_dance(struct state *state) /* BOLT #2: * A receiving node: - * - if `next_funding_txid` is set: - * - if `next_funding_txid` matches the latest interactive funding transaction: - * - if it has not received `tx_signatures` for that funding transaction: - * - MUST retransmit its `commitment_signed` for that funding transaction. - * - if it has already received `commitment_signed` and it should sign first, - * as specified in the [`tx_signatures` requirements](#the-tx_signatures-message): - * - MUST send its `tx_signatures` for that funding transaction. - * - if it has already received `tx_signatures` for that funding transaction: - * - MUST send its `tx_signatures` for that funding transaction. - * - otherwise: - * - MUST send `tx_abort` to let the sending node know that they can forget - * this funding transaction. + * - if the `next_funding` TLV is set: + * - if `next_funding_txid` matches the latest interactive funding transaction: + * - if it has not received `tx_signatures` for that funding transaction: + * - if the `commitment_signed` bit is set in `retransmit_flags`: + * - MUST retransmit its `commitment_signed` for that funding transaction. + * - if it has already received `commitment_signed` and it should sign first, + * as specified in the [`tx_signatures` requirements](#the-tx_signatures-message): + * - MUST send its `tx_signatures` for that funding transaction. + * - if it has already received `tx_signatures` for that funding transaction: + * - MUST send its `tx_signatures` for that funding transaction. + * - if it also sets `next_funding` in its own `channel_reestablish`, but the + * values don't match: + * - MUST send an `error` and fail the channel. + * - otherwise: + * - MUST send `tx_abort` to let the sending node know that they can forget + * this funding transaction. */ if (tlvs->next_funding) { /* Does this match ours? */ diff --git a/openingd/openingd.c b/openingd/openingd.c index 7a86218b339d..aae17559f1bd 100644 --- a/openingd/openingd.c +++ b/openingd/openingd.c @@ -332,12 +332,12 @@ static u8 *funder_channel_start(struct state *state, u8 channel_flags, = state->upfront_shutdown_script[LOCAL]; /* BOLT #2: - * - if it includes `channel_type`: - * - MUST set it to a defined type representing the type it wants. - * - MUST use the smallest bitmap possible to represent the channel - * type. - * - SHOULD NOT set it to a type containing a feature which was not - * negotiated. + * - MUST set `channel_type`: + * - MUST set it to a defined type representing the type it wants. + * - MUST use the smallest bitmap possible to represent the channel + * type. + * - SHOULD NOT set it to a type containing a feature which was not + * negotiated. */ open_tlvs->channel_type = state->channel_type->features; @@ -410,9 +410,9 @@ static u8 *funder_channel_start(struct state *state, u8 channel_flags, their_mindepth); /* BOLT #2: - * - if `channel_type` is set, and `channel_type` was set in - * `open_channel`, and they are not equal types: - * - MUST fail the channel. + * The receiving node MUST:... + * - if the message doesn't include a `channel_type`: + * - fail the channel. */ if (!accept_tlvs->channel_type) { negotiation_failed(state, @@ -877,9 +877,8 @@ static u8 *fundee_channel(struct state *state, const u8 *open_channel_msg) /* BOLT #2: * The receiving node MUST fail the channel if: *... - * - It supports `channel_type` and `channel_type` was set: - * - if `type` is not suitable. - * - if `type` includes `option_zeroconf` and it does not trust the sender to open an unconfirmed channel. + * - the `channel_type` is not suitable. + * - the `channel_type` includes `option_zeroconf` and it does not trust the sender to open an unconfirmed channel. */ /* option_channel_type is compulsory. */ if (!open_tlvs->channel_type) { @@ -1044,7 +1043,7 @@ static u8 *fundee_channel(struct state *state, const u8 *open_channel_msg) /* BOLT #2: * The receiving node MUST fail the channel if: *... - * - if `type` includes `option_zeroconf` and it does not trust the + * - the `channel_type` includes `option_zeroconf` and it does not trust the * sender to open an unconfirmed channel. */ if (channel_type_has(state->channel_type, OPT_ZEROCONF) && @@ -1071,7 +1070,7 @@ static u8 *fundee_channel(struct state *state, const u8 *open_channel_msg) accept_tlvs->upfront_shutdown_script = state->upfront_shutdown_script[LOCAL]; /* BOLT #2: - * - if `option_channel_type` was negotiated: + * The sender:... * - MUST set `channel_type` to the `channel_type` from `open_channel` */ accept_tlvs->channel_type = state->channel_type->features; diff --git a/tests/test_connection.py b/tests/test_connection.py index 2aef856860ff..7b5b5c60f571 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -4065,7 +4065,7 @@ def test_multichan(node_factory, executor, bitcoind): l2.rpc.close(l3.info['id']) l2.rpc.close(scid23b) - bitcoind.generate_block(13, wait_for_mempool=1) + bitcoind.generate_block(73, wait_for_mempool=1) sync_blockheight(bitcoind, [l1, l2, l3]) # Gossip works as expected. @@ -4116,7 +4116,7 @@ def test_multichan(node_factory, executor, bitcoind): "id": 2, "created_index": 3, "updated_index": 27, - "expiry": 135, + "expiry": 195, "direction": "out", "amount_msat": Millisatoshi(100001001), "payment_hash": inv3['payment_hash'], @@ -4125,7 +4125,7 @@ def test_multichan(node_factory, executor, bitcoind): "id": 3, "created_index": 4, "updated_index": 36, - "expiry": 135, + "expiry": 195, "direction": "out", "amount_msat": Millisatoshi(100001001), "payment_hash": inv4['payment_hash'], diff --git a/tests/test_db.py b/tests/test_db.py index b186ee35eb71..98b3446762b5 100644 --- a/tests/test_db.py +++ b/tests/test_db.py @@ -96,7 +96,7 @@ def test_block_backfill(node_factory, bitcoind, chainparams): # Now close the channel and make sure `l3` cleans up correctly: txid = only_one(l1.rpc.close(l2.info['id'])['txids']) - bitcoind.generate_block(13, wait_for_mempool=txid) + bitcoind.generate_block(73, wait_for_mempool=txid) wait_for(lambda: len(l3.rpc.listchannels()['channels']) == 0) diff --git a/tests/test_gossip.py b/tests/test_gossip.py index 688604da7774..e0ecf548d4de 100644 --- a/tests/test_gossip.py +++ b/tests/test_gossip.py @@ -581,8 +581,8 @@ def non_public(node): l1.rpc.dev_fail(l2.info['id']) # We need to wait for the unilateral close to hit the mempool, - # and 12 blocks for nodes to actually forget it. - bitcoind.generate_block(13, wait_for_mempool=1) + # and 72 blocks for nodes to actually forget it. + bitcoind.generate_block(73, wait_for_mempool=1) wait_for(lambda: active(l1) == [scid23, scid23]) wait_for(lambda: active(l2) == [scid23, scid23]) @@ -1445,7 +1445,7 @@ def test_gossip_notices_close(node_factory, bitcoind): txid = only_one(l2.rpc.close(l3.info['id'])['txids']) wait_for(lambda: l2.rpc.listpeerchannels(l3.info['id'])['channels'][0]['state'] == 'CLOSINGD_COMPLETE') - bitcoind.generate_block(13, txid) + bitcoind.generate_block(73, txid) wait_for(lambda: l1.rpc.listchannels()['channels'] == []) wait_for(lambda: l1.rpc.listnodes()['nodes'] == []) @@ -1683,7 +1683,7 @@ def test_gossip_store_compact_while_extending(node_factory, bitcoind, executor): scid23 = only_one(l2.rpc.listpeerchannels(l3.info['id'])['channels'])['short_channel_id'] l2.rpc.close(l3.info['id']) - bitcoind.generate_block(13, wait_for_mempool=1) + bitcoind.generate_block(73, wait_for_mempool=1) wait_for(lambda: l1.rpc.listchannels(scid23) == {'channels': []}) l1.rpc.setchannel(l2.info['id'], 41, 1004) @@ -2008,7 +2008,7 @@ def test_topology_leak(node_factory, bitcoind): # Close and wait for gossip to catchup. txid = only_one(l2.rpc.close(l3.info['id'])['txids']) - bitcoind.generate_block(13, txid) + bitcoind.generate_block(73, txid) wait_for(lambda: len(l1.rpc.listchannels()['channels']) == 2) @@ -2035,7 +2035,7 @@ def test_parms_listforwards(node_factory): assert len(forwards_dep) == 0 -def test_close_12_block_delay(node_factory, bitcoind): +def test_close_72_block_delay(node_factory, bitcoind): l1, l2, l3, l4 = node_factory.line_graph(4, wait_for_announce=True) # Close l1-l2 @@ -2052,16 +2052,16 @@ def test_close_12_block_delay(node_factory, bitcoind): # BOLT #7: # - once its funding output has been spent OR reorganized out: - # - SHOULD forget a channel after a 12-block delay. + # - SHOULD forget a channel after a 72-block delay. - # That implies 12 blocks *after* spending, i.e. 13 blocks deep! + # That implies 72 blocks *after* spending, i.e. 73 blocks deep! - # 12 blocks deep, l4 still sees it - bitcoind.generate_block(10) + # 72 blocks deep, l4 still sees it + bitcoind.generate_block(70) sync_blockheight(bitcoind, [l4]) assert len(l4.rpc.listchannels(source=l1.info['id'])['channels']) == 1 - # 13 blocks deep does it. + # 72 blocks deep does it. bitcoind.generate_block(1) wait_for(lambda: l4.rpc.listchannels(source=l1.info['id'])['channels'] == []) @@ -2490,7 +2490,7 @@ def test_gossmap_lost_node(node_factory, bitcoind): scid23 = only_one(l2.rpc.listpeerchannels(l3.info['id'])['channels'])['short_channel_id'] l2.rpc.close(l3.info['id']) - bitcoind.generate_block(13, wait_for_mempool=1) + bitcoind.generate_block(73, wait_for_mempool=1) # Order of nodes is not stable. sync_blockheight(bitcoind, [l1]) @@ -2522,6 +2522,6 @@ def test_gossip_dying_when_compact(node_factory, bitcoind): wait_for(lambda: len(l1.rpc.listchannels()["channels"]) == 4) l1.rpc.call("dev-compact-gossip-store") - # Now actually close it (12 deep) - bitcoind.generate_block(11) + # Now actually close it (72 deep) + bitcoind.generate_block(71) wait_for(lambda: len(l1.rpc.listchannels()["channels"]) == 2) diff --git a/tests/test_invoices.py b/tests/test_invoices.py index 4b67c9aa79c3..161fbd08931e 100644 --- a/tests/test_invoices.py +++ b/tests/test_invoices.py @@ -362,7 +362,7 @@ def test_invoice_routeboost_private(node_factory, bitcoind): # It will use an explicit exposeprivatechannels even if it thinks its a dead-end l0.rpc.close(l1.info['id']) l0.wait_for_channel_onchain(l1.info['id']) - bitcoind.generate_block(13) + bitcoind.generate_block(73) wait_for(lambda: l2.rpc.listchannels(scid_dummy)['channels'] == []) inv = l2.rpc.invoice(amount_msat=123456, label="inv7", description="?", exposeprivatechannels=scid) diff --git a/tests/test_pay.py b/tests/test_pay.py index ec3b8d490748..63babba3c742 100644 --- a/tests/test_pay.py +++ b/tests/test_pay.py @@ -5897,7 +5897,7 @@ def test_offer_paths(node_factory, bitcoind): # Make scid path invalid by closing it close = l1.rpc.close(paths[0]['first_scid']) - bitcoind.generate_block(13, wait_for_mempool=only_one(close['txids'])) + bitcoind.generate_block(73, wait_for_mempool=only_one(close['txids'])) wait_for(lambda: l5.rpc.listchannels(paths[0]['first_scid']) == {'channels': []}) # Now connect l5->l4, and it will be able to reach l3 via that, and join blinded path. diff --git a/tests/test_plugin.py b/tests/test_plugin.py index 3f4f11cbe90f..78dc00fa3301 100644 --- a/tests/test_plugin.py +++ b/tests/test_plugin.py @@ -4320,7 +4320,7 @@ def test_sql(node_factory, bitcoind): # This has to wait for the hold_invoice plugin to let go! open(os.path.join(l3.daemon.lightning_dir, TEST_NETWORK, "unhold"), "w").close() txid = only_one(l1.rpc.close(l2.info['id'])['txids']) - bitcoind.generate_block(13, wait_for_mempool=txid) + bitcoind.generate_block(73, wait_for_mempool=txid) wait_for(lambda: len(l3.rpc.listchannels(source=l1.info['id'])['channels']) == 0) assert len(l3.rpc.sql("SELECT * FROM channels WHERE source = X'{}';".format(l1.info['id']))['rows']) == 0 l3.daemon.wait_for_log("Deleting channel: {}".format(scid)) diff --git a/tests/test_splicing.py b/tests/test_splicing.py index 458f432f8630..6a644b6d618d 100644 --- a/tests/test_splicing.py +++ b/tests/test_splicing.py @@ -229,7 +229,7 @@ def test_splice_gossip(node_factory, bitcoind): bitcoind.generate_block(5, wait_for_mempool=result['txid']) - # l3 will see channel dying, but still consider it OK for 12 blocks. + # l3 will see channel dying, but still consider it OK for 72 blocks. l3.daemon.wait_for_log(f'gossipd: channel {pre_splice_scid} closing soon due to the funding outpoint being spent') assert len(l3.rpc.listchannels(short_channel_id=pre_splice_scid)['channels']) == 2 assert len(l3.rpc.listchannels(source=l1.info['id'])['channels']) == 1 @@ -246,7 +246,7 @@ def test_splice_gossip(node_factory, bitcoind): wait_for(lambda: len(l3.rpc.listchannels(short_channel_id=post_splice_scid)['channels']) == 2) assert len(l3.rpc.listchannels(short_channel_id=pre_splice_scid)['channels']) == 2 - bitcoind.generate_block(7) + bitcoind.generate_block(67) # The old channel should fall off l3's perspective wait_for(lambda: l3.rpc.listchannels(short_channel_id=pre_splice_scid)['channels'] == []) @@ -416,6 +416,8 @@ def test_commit_crash_splice(node_factory, bitcoind): l1.daemon.wait_for_log(r"Splice initiator: we commit") + # Snapshot log position before restart so we only search post-restart logs below. + pre_restart_logpos = l1.daemon.logsearch_start l1.restart() # The splicing inflight should have been left pending in the DB @@ -426,6 +428,11 @@ def test_commit_crash_splice(node_factory, bitcoind): l1.daemon.wait_for_log(r'Splice resume check with local_next_funding: sent, remote_next_funding: received, inflights: 1') l1.daemon.wait_for_log(r'Splice negotation, will not send commit, not recv commit, send signature, recv signature as initiator') + # channel_ready MUST NOT be retransmitted when either channel_reestablish + # message contains next_funding or my_current_funding_locked + assert not l1.daemon.is_in_log(r'Retransmitting channel_ready for channel', + start=pre_restart_logpos) + assert l1.db_query("SELECT count(*) as c FROM channel_funding_inflights;")[0]['c'] == 1 l2.daemon.wait_for_log(r'CHANNELD_NORMAL to CHANNELD_AWAITING_SPLICE') diff --git a/tests/test_wallet.py b/tests/test_wallet.py index 5be686d11ef4..dd3778af15d9 100644 --- a/tests/test_wallet.py +++ b/tests/test_wallet.py @@ -2548,11 +2548,11 @@ def test_unspend_during_reorg(node_factory, bitcoind): blockheight, txindex, _ = scid.split('x') # Use mainnet settings for rescan. - l3 = node_factory.get_node(options={'rescan': 15}) + l3 = node_factory.get_node(options={'rescan': 75}) l3.connect(l2) mine_funding_to_announce(bitcoind, [l1, l2, l3]) - bitcoind.generate_block(20) + bitcoind.generate_block(90) sync_blockheight(bitcoind, [l3]) wait_for(lambda: len(l3.rpc.listchannels()['channels']) == 2) @@ -2562,10 +2562,10 @@ def test_unspend_during_reorg(node_factory, bitcoind): # Now, l3 sees the close, marks channel dying. l1.rpc.close(l2.info['id']) spentheight = bitcoind.rpc.getblockcount() + 1 - bitcoind.generate_block(14, wait_for_mempool=1) + bitcoind.generate_block(74, wait_for_mempool=1) wait_for(lambda: len(l3.rpc.listchannels()['channels']) == 2) - # In one fell swoop it goes through dying, to dead (12 blocks) + # In one fell swoop it goes through dying, to dead (72 blocks) l3.daemon.wait_for_log(f"Adding block {spentheight}") l3.daemon.wait_for_log(f"gossipd: channel {scid} closing soon due to the funding outpoint being spent") l3.daemon.wait_for_log(f"gossipd: Deleting channel {scid} due to the funding outpoint being spent") @@ -2576,7 +2576,7 @@ def test_unspend_during_reorg(node_factory, bitcoind): # Restart, see replay. l3.stop() # This is enough to take channel from dying to dead. - bitcoind.generate_block(10) + bitcoind.generate_block(70) l3.start() # Channel should still be dead.