channeld: report the fundee's full post-splice balance to hsmd in msat

channeld/channeld.c

@@ -3401,29 +3401,56 @@ relative_splice_balance_fundee(struct peer *peer,
 			       int chan_output_index,
 			       int chan_input_index)
 {
-	/* Relative fundee channel balance */
-	u64 push_value;
-
-	/* We calculcate the `push_value` to send to the
-	 * hsmd, that is the remote amount in the channel
-	 * after the splice. */
-	switch (our_role) {
-	case TX_INITIATOR:
-		/* push_value is the fundee relative value so if we open the channel
-		 * fundee is the remote node. */
-		push_value = peer->splicing->accepter_relative;
-		break;
-	case TX_ACCEPTER:
-		/* push_value is the fundee relative value so if the remote node open the channel
-		 * fundee in this case is the opener. */
-		push_value = peer->splicing->opener_relative;
-		break;
-	default:
-		/* This should never happen. Help us to early catch the tx_role change */
-		abort();
+	/* hsmd_setup_channel's push_value is the fundee's TOTAL balance at
+	 * the start of the new channel era: pre-splice settled balance,
+	 * plus HTLCs pending at splice setup attributable to the fundee,
+	 * plus its funding contribution.  A signer that validates the
+	 * balances reported to it reads this as the fundee's entitlement
+	 * for the new era, so it must cover the fundee's balance in every
+	 * pending-HTLC resolution direction (a fundee-owned HTLC failing
+	 * back raises the fundee output above its settled at-setup
+	 * balance).  A negative total is a genuine over-draw and fails the
+	 * peer; nothing is ever wrapped or clamped. */
+	enum side fundee_side = peer->channel->opener == LOCAL ? REMOTE : LOCAL;
+	bool fundee_is_splice_initiator =
+		(fundee_side == LOCAL) == (our_role == TX_INITIATOR);
+	s64 fundee_contribution = fundee_is_splice_initiator
+		? peer->splicing->opener_relative
+		: peer->splicing->accepter_relative;
+	struct htlc_map_iter it;
+	const struct htlc *htlc;
+
+	/* The fundee's pre-splice settled balance; views agree on owed[]. */
+	struct amount_msat push_value_msat
+		= peer->channel->view[LOCAL].owed[fundee_side];
+
+	/* HTLCs pending at splice setup attributable to the fundee,
+	 * selected by owner side (the same bucketing check_balances uses
+	 * for its pending_htlcs).  Callers run after check_balances, so
+	 * the view and htlc set are final for this round. */
+	for (htlc = htlc_map_first(peer->channel->htlcs, &it);
+	     htlc;
+	     htlc = htlc_map_next(peer->channel->htlcs, &it)) {
+		if (htlc_owner(htlc) != fundee_side)
+			continue;
+		if (!amount_msat_accumulate(&push_value_msat, htlc->amount))
+			peer_failed_warn(peer->pps, &peer->channel_id,
+					 "Unable to add HTLC balance");
 	}
 
-	return amount_msat(push_value);
+	/* opener_relative/accepter_relative are satoshi contributions
+	 * (everywhere else they feed amount_msat_add_sat_s64); a negative
+	 * contribution is a splice-out and subtracts.  A negative total,
+	 * or an add that overflows, is an over-draw the peer is failed
+	 * for rather than reported wrapped. */
+	if (fundee_contribution == INT64_MIN ||
+	    !amount_msat_add_sat_s64(&push_value_msat, push_value_msat,
+				     fundee_contribution))
+		peer_failed_warn(peer->pps, &peer->channel_id,
+				 "splice funding contribution out of range"
+				 " for fundee balance");
+
+	return push_value_msat;
 }
 
 static struct amount_sat calc_balance(struct peer *peer)

channeld/test/Makefile

@@ -21,7 +21,7 @@ channeld/test/run-full_channel:			\
 	common/features.o			\
 	common/htlc_state.o			\
 	common/htlc_trim.o			\
-	common/htlc_tx.o			\
+	common/htlc_tx.o				\
 	common/initial_commit_tx.o		\
 	common/key_derive.o			\
 	common/msg_queue.o			\
@@ -31,6 +31,16 @@ channeld/test/run-full_channel:			\
 	common/setup.o				\
 	common/utils.o
 
+# run-splice_fundee_msat includes channeld.c itself (so we can see statics),
+# like the hsmd tests: do not also link channeld.o.  The sibling channeld
+# objects, hsmd client objects and libcommon.a satisfy the rest.
+channeld/test/run-splice_fundee_msat:			\
+	$(filter-out channeld/channeld.o,$(CHANNELD_OBJS))	\
+	$(HSMD_CLIENT_OBJS)				\
+	$(BITCOIN_OBJS)					\
+	wire/towire.o					\
+	wire/fromwire.o
+
 $(CHANNELD_TEST_OBJS): $(CHANNELD_HEADERS) $(CHANNELD_SRC) channeld/test/Makefile
 
 check-units: $(CHANNELD_TEST_PROGRAMS:%=unittest/%)

channeld/test/run-splice_fundee_msat.c

@@ -0,0 +1,257 @@
+/* Unit test for relative_splice_balance_fundee().
+ *
+ * hsmd_setup_channel's push_value is the fundee's balance at the start
+ * of the new channel era: its pre-splice balance plus its funding
+ * contribution.  The splicing opener_relative/accepter_relative fields
+ * are satoshi amounts selected by SPLICE role, so the function must
+ * (a) pick the contribution of the side that did not open the CHANNEL,
+ * (b) convert sat -> msat, and (c) add the pre-splice balance.  This
+ * pins that contract, byte-exact, through the real code in
+ * channeld.c.
+ *
+ * Following hsmd/test/run-bad-request-close.c, we pull in channeld.c
+ * itself (renaming its main) so the static function is testable.
+ */
+#include "config.h"
+#include <sys/wait.h>
+#include <unistd.h>
+#include <ccan/array_size/array_size.h>
+#include <ccan/err/err.h>
+#include <common/setup.h>
+#include <common/status.h>
+
+int unused_main(int argc, char *argv[]);
+#define main unused_main
+#include "../channeld.c"
+#undef main
+
+/* AUTOGENERATED MOCKS START */
+/* AUTOGENERATED MOCKS END */
+
+static struct peer *make_peer(const tal_t *ctx,
+			      enum side opener,
+			      u64 owed_local_msat, u64 owed_remote_msat,
+			      s64 opener_relative, s64 accepter_relative)
+{
+	struct peer *peer = talz(ctx, struct peer);
+
+	peer->channel = talz(peer, struct channel);
+	peer->channel->opener = opener;
+	peer->channel->view[LOCAL].owed[LOCAL].millisatoshis = owed_local_msat;
+	peer->channel->view[LOCAL].owed[REMOTE].millisatoshis = owed_remote_msat;
+	/* The real splice path always has a live htlc map; the total
+	 * iterates it (the check_balances pending_htlcs bucketing). */
+	peer->channel->htlcs = new_htable(peer->channel, htlc_map);
+
+	peer->splicing = tal(peer, struct splicing);
+	peer->splicing->opener_relative = opener_relative;
+	peer->splicing->accepter_relative = accepter_relative;
+	peer->pps = talz(peer, struct per_peer_state);
+	return peer;
+}
+
+/* Install a pending HTLC owned by `owner`.  Fully-acked states
+ * (SENT_ADD_ACK_REVOCATION -> LOCAL, RCVD_ADD_ACK_REVOCATION ->
+ * REMOTE) are what htlc_owner() buckets by; the enum's SENT/RCVD
+ * prefixes alone do NOT pick the owner. */
+static void add_pending_htlc(struct peer *peer, u64 id,
+			     struct amount_msat amount, enum side owner)
+{
+	struct htlc *htlc = talz(peer->channel, struct htlc);
+
+	htlc->id = id;
+	htlc->amount = amount;
+	htlc->state = owner == LOCAL ? SENT_ADD_ACK_REVOCATION
+				     : RCVD_ADD_ACK_REVOCATION;
+	htlc_map_add(peer->channel->htlcs, htlc);
+}
+
+/* Bits on the wire to hsmd are the raw millisatoshis integer: compare
+ * the raw field, not a pretty-printed string. */
+static u64 push_value_msat(enum side opener, enum tx_role our_role,
+			   u64 owed_local_msat, u64 owed_remote_msat,
+			   s64 opener_relative, s64 accepter_relative)
+{
+	struct peer *peer = make_peer(tmpctx, opener,
+				      owed_local_msat, owed_remote_msat,
+				      opener_relative, accepter_relative);
+	struct amount_msat got;
+
+	got = relative_splice_balance_fundee(peer, our_role, NULL, 0, 0);
+	return got.millisatoshis;
+}
+
+static void test_must_be(enum side opener, enum tx_role our_role,
+			 u64 owed_local_msat, u64 owed_remote_msat,
+			 s64 opener_relative, s64 accepter_relative,
+			 u64 expect_msat, const char *desc)
+{
+	u64 got = push_value_msat(opener, our_role,
+				  owed_local_msat, owed_remote_msat,
+				  opener_relative, accepter_relative);
+	if (got != expect_msat)
+		errx(1, "%s: expected %llu msat, got %llu msat",
+		     desc, (unsigned long long)expect_msat,
+		     (unsigned long long)got);
+}
+
+/* An out-of-range contribution must never return: the real
+ * peer_failed_warn() writes what it can and exits nonzero, so we
+ * fork and insist the child dies.  alarm() guards against a hang. */
+static void test_must_refuse(enum side opener, enum tx_role our_role,
+			     u64 owed_local_msat, u64 owed_remote_msat,
+			     s64 opener_relative, s64 accepter_relative,
+			     const char *desc)
+{
+	pid_t child = fork();
+
+	if (child == 0) {
+		struct peer *peer;
+		alarm(30);
+		peer = make_peer(NULL, opener,
+				 owed_local_msat, owed_remote_msat,
+				 opener_relative, accepter_relative);
+		/* Must not return. */
+		relative_splice_balance_fundee(peer, our_role, NULL, 0, 0);
+		_exit(0);
+	} else {
+		int status;
+
+		if (waitpid(child, &status, 0) != child)
+			errx(1, "%s: waitpid failed", desc);
+		if (WIFEXITED(status) && WEXITSTATUS(status) == 0)
+			errx(1, "%s: returned instead of failing peer", desc);
+		if (!(WIFEXITED(status) || WIFSIGNALED(status)))
+			errx(1, "%s: child neither exited nor signalled", desc);
+	}
+}
+
+int main(int argc, const char *argv[])
+{
+	common_setup(argv[0]);
+
+	/* The contribution comes from the side that did not open the
+	 * channel, whatever the splice roles: distinct contributions and
+	 * distinct pre-splice balances pin both the party and the base.
+	 *
+	 * opener_relative (the splice initiator's contribution) is
+	 * 300 sat = 300000 msat; accepter_relative is 70000 sat =
+	 * 70000000 msat; owed[LOCAL] is 123 msat, owed[REMOTE] 456 msat.
+	 */
+	test_must_be(LOCAL, TX_INITIATOR, 123, 456, 300, 70000,
+		     456 + 70000000, "we open; we initiate; fundee=remote accepter");
+	test_must_be(LOCAL, TX_ACCEPTER, 123, 456, 300, 70000,
+		     456 + 300000, "we open; they initiate; fundee=remote initiator");
+	test_must_be(REMOTE, TX_INITIATOR, 123, 456, 300, 70000,
+		     123 + 300000, "they open; we initiate; fundee=local initiator");
+	test_must_be(REMOTE, TX_ACCEPTER, 123, 456, 300, 70000,
+		     123 + 70000000, "they open; they initiate; fundee=local accepter");
+
+	/* A fundee holding a pre-splice balance must have it reported,
+	 * not just its contribution: 500000 sat routed earlier =
+	 * 500000000 msat, plus a 1000 sat splice-in contribution. */
+	test_must_be(LOCAL, TX_INITIATOR, 0, 500000000, 0, 1000,
+		     500000000 + 1000000, "fundee with balance, splice-in");
+
+	/* A negative contribution is a splice-out: it subtracts from the
+	 * pre-splice balance. */
+	test_must_be(LOCAL, TX_INITIATOR, 0, 500000000, 0, -4000,
+		     500000000 - 4000000, "fundee with balance, splice-out");
+
+	/* HTLCs pending at setup and owned by the fundee are part of the
+	 * fundee's total (a failing-back fundee-owned HTLC raises the
+	 * fundee output above its settled balance); the other side's
+	 * pending HTLCs never count.  Fundee LOCAL: owed 300k sat +
+	 * fundee-owned pending 75k+25k sat + 100k sat contribution = 500k
+	 * sat (other side's 400k sat pending excluded). */
+	{
+		struct peer *peer = make_peer(tmpctx, REMOTE,
+					      300000000, 0, 100000, 0);
+		struct amount_msat got;
+
+		add_pending_htlc(peer, 0, amount_msat(75000000), LOCAL);
+		add_pending_htlc(peer, 1, amount_msat(25000000), LOCAL);
+		add_pending_htlc(peer, 2, amount_msat(400000000), REMOTE);
+		got = relative_splice_balance_fundee(peer, TX_INITIATOR,
+						     NULL, 0, 0);
+		if (got.millisatoshis != 500000000)
+			errx(1, "pending-term LOCAL: expected 500000000 msat,"
+			     " got %llu msat",
+			     (unsigned long long)got.millisatoshis);
+	}
+
+	/* Fundee REMOTE: owed 300k sat + fundee-owned pending 100k sat -
+	 * 50k sat withdrawal = 350k sat (other side's pending excluded). */
+	{
+		struct peer *peer = make_peer(tmpctx, LOCAL,
+					      0, 300000000, 0, -50000);
+		struct amount_msat got;
+
+		add_pending_htlc(peer, 0, amount_msat(100000000), REMOTE);
+		add_pending_htlc(peer, 1, amount_msat(400000000), LOCAL);
+		got = relative_splice_balance_fundee(peer, TX_INITIATOR,
+						     NULL, 0, 0);
+		if (got.millisatoshis != 350000000)
+			errx(1, "pending-term REMOTE: expected 350000000 msat,"
+			     " got %llu msat",
+			     (unsigned long long)got.millisatoshis);
+	}
+
+	/* The sat/msat wrap fingerprint: with zero pre-splice balance a
+	 * nonzero satoshi contribution must be reported as sat*1000 msat,
+	 * never as the raw satoshi number -- that equality is exactly the
+	 * 1000x-under-report bug this test guards against. */
+	{
+		static const s64 amounts[] = {
+			1, 2, 999, 1000, 123456, 1000000, 2100000000000000
+		};
+		for (size_t i = 0; i < ARRAY_SIZE(amounts); i++) {
+			s64 sat = amounts[i];
+			test_must_be(LOCAL, TX_INITIATOR, 0, 0, 7, sat,
+				     (u64)sat * 1000, "fresh fundee, remote accepter");
+			test_must_be(REMOTE, TX_INITIATOR, 0, 0, sat, 7,
+				     (u64)sat * 1000, "fresh fundee, local initiator");
+		}
+	}
+
+	/* Out-of-range contributions must fail the peer, not wrap:
+	 * underflow (splice-out beyond the fundee's balance), INT64_MIN
+	 * (negation would overflow), and overflow past u64 msat. */
+	test_must_refuse(LOCAL, TX_INITIATOR, 0, 1000, 0, -100000,
+			 "splice-out beyond balance");
+	test_must_refuse(LOCAL, TX_INITIATOR, 0, 0, 0, INT64_MIN,
+			 "contribution INT64_MIN");
+	test_must_refuse(REMOTE, TX_INITIATOR, 0, 0, INT64_MAX, 7,
+			 "contribution INT64_MAX");
+	test_must_refuse(REMOTE, TX_INITIATOR, UINT64_MAX, 0, 1000, 7,
+			 "balance + contribution overflow");
+
+	/* A true negative TOTAL fails the peer even when pending HTLCs
+	 * cushion part of the withdrawal: owed 100k sat + fundee-owned
+	 * pending 200k sat - 400k sat withdrawal = -100k sat. */
+	{
+		pid_t child = fork();
+
+		if (child == 0) {
+			struct peer *peer;
+			alarm(30);
+			peer = make_peer(NULL, REMOTE, 100000000, 0, -400000, 0);
+			add_pending_htlc(peer, 0, amount_msat(200000000), LOCAL);
+			/* Must not return. */
+			relative_splice_balance_fundee(peer, TX_INITIATOR,
+						       NULL, 0, 0);
+			_exit(0);
+		} else {
+			int status;
+
+			if (waitpid(child, &status, 0) != child)
+				errx(1, "true-negative total: waitpid failed");
+			if (WIFEXITED(status) && WEXITSTATUS(status) == 0)
+				errx(1, "true-negative total: returned instead"
+				     " of failing peer");
+		}
+	}
+
+	common_shutdown();
+	return 0;
+}