diff --git a/channeld/channeld.c b/channeld/channeld.c index 74781a32cf43..bd5b3adb7d28 100644 --- a/channeld/channeld.c +++ b/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) diff --git a/channeld/test/Makefile b/channeld/test/Makefile index f683ec31b26f..c2284df13c8d 100644 --- a/channeld/test/Makefile +++ b/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/%) diff --git a/channeld/test/run-splice_fundee_msat.c b/channeld/test/run-splice_fundee_msat.c new file mode 100644 index 000000000000..d0d7ebc88d85 --- /dev/null +++ b/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 +#include +#include +#include +#include +#include + +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; +} diff --git a/tests/test_splicing.py b/tests/test_splicing.py index a2799247c5cc..d5bfbdfe2a0c 100644 --- a/tests/test_splicing.py +++ b/tests/test_splicing.py @@ -991,3 +991,51 @@ def test_splice_candidate_spent_before_lock(node_factory, bitcoind): chan = only_one(l1.rpc.listpeerchannels()['channels']) assert chan['state'] == 'ONCHAIN' assert chan['funding_txid'] == splice_txid + + +@pytest.mark.openchannel('v1') +@pytest.mark.openchannel('v2') +@unittest.skipIf(TEST_NETWORK != 'regtest', 'elementsd doesnt yet support PSBT features we need') +def test_splice_fundee_with_balance(node_factory, bitcoind): + """Splice a channel whose fundee already holds a balance. + + The hsmd setup_channel push_value must report the fundee's full + post-splice balance (prior balance + splice contribution), not just + its funding contribution. Other splice tests do exercise funded + fundees, but none can observe this value: stock hsmd signs + unconditionally and no test asserts what hsmd was told. + """ + l1, l2 = node_factory.line_graph(2, fundamount=1000000, wait_for_announce=True) + + # Give the fundee a pre-splice balance: 500k sat routed to l2. + inv = l2.rpc.invoice(500000000, 'fundee-balance', 'fundee balance') + l1.rpc.xpay(inv['bolt11']) + wait_for(lambda: only_one(l2.rpc.listpeerchannels()['channels'])['spendable_msat'] > 450000000) + pre = only_one(l2.rpc.listpeerchannels()['channels'])['spendable_msat'] + + chan_id = l1.get_channel_id(l2) + + # The funder splices in on top of the fundee's balance. + funds_result = l1.rpc.fundpsbt("111722sat", 0, 0, excess_as_change=True) + result = l1.rpc.splice_init(chan_id, 100000, funds_result['psbt']) + result = l1.rpc.splice_update(chan_id, result['psbt']) + assert result['commitments_secured'] is False + result = l1.rpc.splice_update(chan_id, result['psbt']) + assert result['commitments_secured'] is True + result = l1.rpc.signpsbt(result['psbt']) + result = l1.rpc.splice_signed(chan_id, result['signed_psbt']) + + l1.daemon.wait_for_log(r'CHANNELD_NORMAL to CHANNELD_AWAITING_SPLICE') + l2.daemon.wait_for_log(r'CHANNELD_NORMAL to CHANNELD_AWAITING_SPLICE') + bitcoind.generate_block(6, wait_for_mempool=1) + l1.daemon.wait_for_log(r'CHANNELD_AWAITING_SPLICE to CHANNELD_NORMAL') + l2.daemon.wait_for_log(r'CHANNELD_AWAITING_SPLICE to CHANNELD_NORMAL') + + # The fundee keeps its balance across the splice (allow for the + # larger channel reserve on the spliced capacity). + wait_for(lambda: only_one(l2.rpc.listpeerchannels()['channels'])['spendable_msat'] >= pre - 5000000) + + # And the channel still routes: pay the fundee another invoice. + inv = l2.rpc.invoice(100000000, 'fundee-balance-2', 'fundee balance 2') + l1.rpc.xpay(inv['bolt11']) + wait_for(lambda: only_one(l2.rpc.listinvoices('fundee-balance-2')['invoices'])['status'] == 'paid')