Port release 26.06.8 to master

closingd/closingd.c

@@ -215,10 +215,12 @@ static void send_offer(struct per_peer_state *pps,
 	peer_write(pps, take(msg));
 }
 
-static void tell_master_their_offer(const struct bitcoin_signature *their_sig,
+/* Returns false if master says we must not agree to this offer. */
+static bool tell_master_their_offer(const struct bitcoin_signature *their_sig,
 				    const struct bitcoin_tx *tx,
 				    struct bitcoin_txid *tx_id)
 {
+	bool acceptable;
 	u8 *msg = towire_closingd_received_signature(NULL, their_sig, tx);
 	if (!wire_sync_write(REQ_FD, take(msg)))
 		status_failed(STATUS_FAIL_MASTER_IO,
@@ -227,9 +229,11 @@ static void tell_master_their_offer(const struct bitcoin_signature *their_sig,
 
 	/* Wait for master to ack, to make sure it's in db. */
 	msg = wire_sync_read(NULL, REQ_FD);
-	if (!fromwire_closingd_received_signature_reply(msg, tx_id))
+	if (!fromwire_closingd_received_signature_reply(msg, tx_id,
+							&acceptable))
 		master_badmsg(WIRE_CLOSINGD_RECEIVED_SIGNATURE_REPLY, msg);
 	tal_free(msg);
+	return acceptable;
 }
 
 /* Returns fee they offered. */
@@ -384,7 +388,17 @@ receive_offer(struct per_peer_state *pps,
 	/* Master sorts out what is best offer, we just tell it any above min */
 	if (amount_sat_greater_eq(received_fee, min_fee_to_accept)) {
 		status_debug("...offer is reasonable");
-		tell_master_their_offer(&their_sig, tx, closing_txid);
+		/* Our own closing_signed for this round has usually gone
+		 * out by now (the opener sends first), so if their fee
+		 * matched ours they hold both signatures and can broadcast
+		 * the close whatever we do here.  Refusing only keeps us
+		 * from recording the close as agreed.  lightningd checks
+		 * the fee against the same bounds we negotiate within, so
+		 * this is not expected to fire. */
+		if (!tell_master_their_offer(&their_sig, tx, closing_txid))
+			peer_failed_warn(pps, channel_id,
+					 "Closing fee %s is outside our fee limits",
+					 fmt_amount_sat(tmpctx, received_fee));
 	}
 
 	return received_fee;

closingd/closingd_wire.csv

@@ -44,6 +44,8 @@ msgdata,closingd_received_signature,tx,bitcoin_tx,
 
 msgtype,closingd_received_signature_reply,2102
 msgdata,closingd_received_signature_reply,closing_txid,bitcoin_txid,
+# Whether we may agree to this offer at all.
+msgdata,closingd_received_signature_reply,acceptable,bool,
 
 # Negotiations complete, we're exiting.
 msgtype,closingd_complete,2004

lightningd/closing_control.c

@@ -218,12 +218,48 @@ static u32 calc_max_close_feerate(struct lightningd *ld,
 	return max_feerate;
 }
 
+/* The fee closingd negotiated is what it took off our output (we only
+ * bound the fee when we are the opener, and the opener pays it).  The
+ * transaction itself pays more than that whenever the outputs' msat
+ * remainders were rounded away or an output was trimmed as dust: neither
+ * is a fee we chose, so neither counts against our maximum. */
+static bool negotiated_close_fee(const struct channel *channel,
+				 const struct bitcoin_tx *tx,
+				 struct amount_sat *fee)
+{
+	struct amount_sat ours = amount_msat_to_sat_round_down(channel->our_msat);
+	struct amount_sat out_amt;
+
+	for (size_t i = 0; i < tx->wtx->num_outputs; i++) {
+		const struct wally_tx_output *out = &tx->wtx->outputs[i];
+		const u8 *script = tal_dup_arr(tmpctx, u8,
+					       out->script, out->script_len, 0);
+		if (!scripteq(script, channel->shutdown_scriptpubkey[LOCAL]))
+			continue;
+		out_amt = bitcoin_tx_output_get_amount_sat(tx, i);
+		if (!amount_sat_sub(fee, ours, out_amt)) {
+			/* closingd built the tx from this same balance, so
+			 * this cannot underflow; count the whole fee if it
+			 * does. */
+			log_broken(channel->log,
+				   "Closing tx output %zu pays us %s,"
+				   " more than our balance %s",
+				   i, fmt_amount_sat(tmpctx, out_amt),
+				   fmt_amount_sat(tmpctx, ours));
+			return false;
+		}
+		return true;
+	}
+	/* Our output was trimmed: the fee is everything. */
+	return false;
+}
+
 /* Assess whether a proposed closing fee is acceptable. */
 static bool closing_fee_is_acceptable(struct lightningd *ld,
 				      struct channel *channel,
 				      const struct bitcoin_tx *tx)
 {
-	struct amount_sat fee, last_fee;
+	struct amount_sat fee, last_fee, negotiated;
 	u64 weight;
 
 	/* Calculate actual fee (adds in eliminated outputs) */
@@ -244,12 +280,21 @@ static bool closing_fee_is_acceptable(struct lightningd *ld,
 		  fmt_amount_sat(tmpctx, last_fee),
 		  weight);
 
+	if (ld->dev_reject_closing_fee) {
+		log_debug(channel->log, "... dev-reject-closing-fee");
+		return false;
+	}
+
 	if (!channel->ignore_fee_limits && !ld->config.ignore_fee_limits) {
 		struct amount_sat min_fee, max_fee;
 		u32 min_feerate, max_feerate;
 
 		/* If we don't have a feerate estimate, this gives feerate_floor */
 		min_feerate = feerate_min(ld, NULL);
+		/* A feerange given to `close` is what closingd negotiated
+		 * within; its minimum is our floor too. */
+		if (channel->closing_feerate_range)
+			min_feerate = channel->closing_feerate_range[0];
 		max_feerate = calc_max_close_feerate(ld, channel);
 
 		min_fee = amount_tx_fee(min_feerate, weight);
@@ -261,9 +306,14 @@ static bool closing_fee_is_acceptable(struct lightningd *ld,
 			return false;
 		}
 		max_fee = amount_tx_fee(max_feerate, weight);
-		if (channel->opener == LOCAL && amount_sat_less(max_fee, fee)) {
-			log_debug(channel->log, "... That's above our max %s"
-				  " for weight %"PRIu64" at feerate %u",
+		if (!negotiated_close_fee(channel, tx, &negotiated))
+			negotiated = fee;
+		if (channel->opener == LOCAL
+		    && amount_sat_less(max_fee, negotiated)) {
+			log_debug(channel->log, "... Negotiated fee %s is above"
+				  " our max %s for weight %"PRIu64
+				  " at feerate %u",
+				  fmt_amount_sat(tmpctx, negotiated),
 				  fmt_amount_sat(tmpctx, max_fee),
 				  weight, max_feerate);
 			return false;
@@ -284,6 +334,7 @@ static void peer_received_closing_signature(struct channel *channel,
 	struct bitcoin_txid tx_id;
 	struct lightningd *ld = channel->peer->ld;
 	u8 *funding_wscript;
+	bool acceptable;
 
 	if (!fromwire_closingd_received_signature(msg, msg, &sig, &tx)) {
 		channel_internal_error(channel,
@@ -312,17 +363,23 @@ static void peer_received_closing_signature(struct channel *channel,
 		return;
 	}
 
-	if (closing_fee_is_acceptable(ld, channel, tx)) {
+	acceptable = closing_fee_is_acceptable(ld, channel, tx);
+	if (acceptable) {
 		channel_set_last_tx(channel, tx, &sig);
 		wallet_channel_save(ld->wallet, channel);
-	}
-
-
-	// Send back the txid so we can update the billboard on selection.
+	} else
+		log_unusual(channel->log,
+			    "Rejecting peer's closing fee offer:"
+			    " closingd must not agree to it");
+
+	/* Send back the txid so closingd can update the billboard, and
+	 * whether it may agree to this offer at all.  Without the verdict
+	 * a rejected offer would still complete the close, and last_tx,
+	 * still the commitment, would be broadcast as the mutual close. */
 	bitcoin_txid(channel->last_tx, &tx_id);
-	/* OK, you can continue now. */
 	subd_send_msg(channel->owner,
-		      take(towire_closingd_received_signature_reply(channel, &tx_id)));
+		      take(towire_closingd_received_signature_reply(channel, &tx_id,
+								    acceptable)));
 }
 
 static void peer_closing_complete(struct channel *channel, const u8 *msg)

lightningd/invoice.c

@@ -1153,6 +1153,17 @@ static struct command_result *json_invoice(struct command *cmd,
 		return command_fail(cmd, JSONRPC2_INVALID_PARAMS,
 				    "dev-routes requires --developer");
 
+	/* Two hard limits on how far in the future an invoice can expire:
+	 * push_varlen_field() can only encode up to 60 bits (larger values
+	 * abort the daemon in bolt11_encode()), and the invoice expiration
+	 * timer overflows its u64 nanosecond-based grain count far below
+	 * that, leaving the expiry check looping forever.  2^32 seconds
+	 * (~136 years) keeps a wide margin under both. */
+	if (*expiry >= (u64)1 << 32)
+		return command_fail(cmd, JSONRPC2_INVALID_PARAMS,
+				    "expiry must be below 2^32 seconds"
+				    " (~136 years)");
+
 	if (strlen(info->label->s) > inv_max_label_len) {
 		return command_fail(cmd, JSONRPC2_INVALID_PARAMS,
 				    "Label '%s' over %zu bytes", info->label->s, inv_max_label_len);
@@ -1687,6 +1698,14 @@ static struct command_result *json_createinvoice(struct command *cmd,
 				  NULL, chainparams, &hash, &sig, &have_n,
 				  &fail);
 	if (b11) {
+		/* Same bound as the invoice RPC: past 60 bits
+		 * bolt11_encode() below aborts, and the expiry timer
+		 * overflows its u64 nanosecond grain far below that. */
+		if (b11->expiry >= (u64)1 << 32)
+			return command_fail(cmd, JSONRPC2_INVALID_PARAMS,
+					    "expiry must be below 2^32 seconds"
+					    " (~136 years)");
+
 		/* This adds the signature */
 		char *b11enc = bolt11_encode(cmd, b11, have_n,
 					     hsm_sign_b11, cmd->ld);

lightningd/lightningd.c

@@ -131,6 +131,7 @@ static struct lightningd *new_lightningd(const tal_t *ctx)
 	ld->dev_throttle_gossip = false;
 	ld->dev_suppress_gossip = false;
 	ld->dev_fast_reconnect = false;
+	ld->dev_reject_closing_fee = false;
 	ld->dev_force_privkey = NULL;
 	ld->dev_force_bip32_seed = NULL;
 	ld->dev_force_channel_secrets = NULL;

lightningd/lightningd.h

@@ -322,6 +322,9 @@ struct lightningd {
 	/* Speedup reconnect delay, for testing. */
 	bool dev_fast_reconnect;
 
+	/* Reject every closing fee the peer offers. */
+	bool dev_reject_closing_fee;
+
 	/* This is the forced private key for the node. */
 	struct privkey *dev_force_privkey;
 

lightningd/options.c

@@ -801,6 +801,10 @@ static void dev_register_opts(struct lightningd *ld)
 		     opt_set_bool,
 		     &ld->dev_fast_reconnect,
 		     "Make max default reconnect delay 3 (not 300) seconds");
+	clnopt_noarg("--dev-reject-closing-fee", OPT_DEV,
+		     opt_set_bool,
+		     &ld->dev_reject_closing_fee,
+		     "Reject every closing fee the peer offers, as if outside our limits");
 
 	clnopt_noarg("--dev-fail-on-subdaemon-fail", OPT_DEV,
 		     opt_set_bool,

tests/test_closing.py

@@ -4448,6 +4448,105 @@ def test_closing_minfee(node_factory, bitcoind):
     bitcoind.generate_block(1, wait_for_mempool=txid)
 
 
+@unittest.skipIf(TEST_NETWORK != 'regtest', 'elementsd anchors not supportd')
+def test_closing_fee_rounding_at_ceiling(node_factory, bitcoind):
+    """A close pinned at the fee ceiling stays a mutual close.
+
+    With every estimate at the floor, the opener's closing fee range is
+    a single value.  Each output is rounded down to whole satoshis, so
+    the msat remainders end up in the fee and the transaction pays one
+    satoshi more than the agreed fee.  lightningd must still accept it
+    rather than fall back to broadcasting the commitment.
+    """
+    l1, l2 = node_factory.line_graph(2, opts={'feerates': (253, 253, 253, 253)})
+    chan = only_one(l1.rpc.listpeerchannels(l2.info['id'])['channels'])
+    funding = int(Millisatoshi(chan['total_msat']).to_satoshi())
+
+    # Leave remainders which sum to exactly 1000msat: l1 keeps ...999msat,
+    # l2 gets ...001msat.  Rounding both down costs one satoshi of fee.
+    l1.pay(l2, 100000001)
+    wait_for(lambda: only_one(l1.rpc.listpeerchannels()['channels'])['htlcs'] == [])
+
+    fee = closing_fee(253, 2)
+    res = l1.rpc.close(l2.info['id'])
+    assert res['type'] == 'mutual'
+    tx = bitcoind.rpc.decoderawtransaction(only_one(res['txs']))
+
+    # A closing transaction, not the commitment.
+    assert len(tx['vout']) == 2
+    assert tx['locktime'] == 0
+
+    # The agreed fee plus the rounded-off remainders.
+    paid = funding - sum(int(round(o['value'] * 10**8)) for o in tx['vout'])
+    assert paid == fee + 1
+    billboard = only_one(l1.rpc.listpeerchannels(l2.info['id'])['channels'])['status']
+    assert billboard == ['CLOSINGD_SIGEXCHANGE:We agreed on a closing fee of {} satoshi for tx:{}'.format(fee, tx['txid'])]
+
+    bitcoind.generate_block(1, wait_for_mempool=tx['txid'])
+    wait_for(lambda: 'ONCHAIN:Tracking mutual close transaction' in only_one(l1.rpc.listpeerchannels(l2.info['id'])['channels'])['status'])
+    assert tx['txid'] in [o['txid'] for o in l1.rpc.listfunds()['outputs']]
+    assert tx['txid'] in [o['txid'] for o in l2.rpc.listfunds()['outputs']]
+
+
+@unittest.skipIf(TEST_NETWORK != 'regtest', 'elementsd anchors not supportd')
+def test_closing_feerange_below_estimates(node_factory, bitcoind):
+    """A close with a feerange below the estimate floor stays a mutual close.
+
+    closingd negotiates within the feerange given to `close`, but
+    lightningd checked the agreed fee against the floor derived from its
+    fee estimates.  With estimates above the range, the agreed fee was
+    rejected as too low and the commitment was broadcast instead.
+    """
+    l1, l2 = node_factory.line_graph(2, opts={'feerates': (253, 253, 253, 253),
+                                              'may_reconnect': True})
+    l1.pay(l2, 100000000)
+    wait_for(lambda: only_one(l1.rpc.listpeerchannels()['channels'])['htlcs'] == [])
+
+    # l1's estimate floor becomes 1000perkw, well above the range.
+    l1.force_feerates(2000)
+    l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
+
+    fee = closing_fee(253, 2)
+    res = l1.rpc.close(l2.info['id'], feerange=['253perkw', '253perkw'])
+    assert res['type'] == 'mutual'
+    tx = bitcoind.rpc.decoderawtransaction(only_one(res['txs']))
+
+    # A closing transaction at the agreed fee, not the commitment.
+    assert len(tx['vout']) == 2
+    assert tx['locktime'] == 0
+    billboard = only_one(l1.rpc.listpeerchannels(l2.info['id'])['channels'])['status']
+    assert 'CLOSINGD_SIGEXCHANGE:We agreed on a closing fee of {} satoshi for tx:{}'.format(fee, tx['txid']) in billboard
+
+    bitcoind.generate_block(1, wait_for_mempool=tx['txid'])
+    wait_for(lambda: 'ONCHAIN:Tracking mutual close transaction' in only_one(l1.rpc.listpeerchannels(l2.info['id'])['channels'])['status'])
+    assert tx['txid'] in [o['txid'] for o in l1.rpc.listfunds()['outputs']]
+    assert tx['txid'] in [o['txid'] for o in l2.rpc.listfunds()['outputs']]
+
+
+def test_closing_rejected_fee_fails_negotiation(node_factory, bitcoind):
+    """A closing fee lightningd rejects ends the negotiation.
+
+    closingd only learns the txid from lightningd's reply, never the
+    verdict, so it agrees to a rejected offer and lightningd broadcasts
+    the commitment as if it were the mutual close.  The known reasons for
+    a rejection are fixed, so --dev-reject-closing-fee forces one.
+    """
+    l1, l2 = node_factory.line_graph(2, opts=[{}, {'dev-reject-closing-fee': None}])
+    l1.pay(l2, 100000000)
+    wait_for(lambda: only_one(l1.rpc.listpeerchannels()['channels'])['htlcs'] == [])
+
+    # l2 refuses l1's offer, so nobody completes the negotiation and l1
+    # closes unilaterally when its timeout expires.
+    res = l1.rpc.close(l2.info['id'], unilateraltimeout=10)
+    assert res['type'] == 'unilateral'
+    assert not l2.daemon.is_in_log('We agreed on a closing fee')
+    l2.daemon.wait_for_log('outside our fee limits')
+
+    # The only transaction on the wire is that unilateral close.
+    txid = only_one(res['txids'])
+    wait_for(lambda: bitcoind.rpc.getrawmempool() == [txid])
+
+
 @unittest.skipIf(TEST_NETWORK != 'regtest', 'elementsd anchors not supportd')
 def test_peer_anchor_push(node_factory, bitcoind, executor, chainparams):
     """Test that we use anchor on peer's commit to CPFP tx"""