From dc4e0a400f4ed9930f5055cefa4ecfa2a286d6f3 Mon Sep 17 00:00:00 2001 From: Dusty Daemon Date: Thu, 5 Oct 2023 16:50:43 -0400 Subject: [PATCH] Splice: Better balance checking * Regression test added for Issue #6572 (issuecomment-1730808863) w/stuck HTLC * `check_balance` adjusted to calculate pending HTLCs explicitly * Test confirmed to fail prior to PR #6713 ChangeLog-Fixed: Issue splicing with pending / stuck HTLCs fixed. --- channeld/channeld.c | 52 +++++++++++++++++++++++++++++++++++++----- tests/test_splicing.py | 41 +++++++++++++++++++++++++++++++++ 2 files changed, 87 insertions(+), 6 deletions(-) diff --git a/channeld/channeld.c b/channeld/channeld.c index e98603858..3a71b1acb 100644 --- a/channeld/channeld.c +++ b/channeld/channeld.c @@ -2939,13 +2939,35 @@ static struct amount_sat check_balances(struct peer *peer, funding_amount_res, min_multiplied; struct amount_msat funding_amount, initiator_fee, accepter_fee; - struct amount_msat in[NUM_TX_ROLES], out[NUM_TX_ROLES]; + struct amount_msat in[NUM_TX_ROLES], out[NUM_TX_ROLES], + pending_htlcs[NUM_TX_ROLES]; + struct htlc_map_iter it; + const struct htlc *htlc; bool opener = our_role == TX_INITIATOR; u8 *msg; + /* The channel funds less any pending htlcs */ in[TX_INITIATOR] = peer->channel->view->owed[opener ? LOCAL : REMOTE]; in[TX_ACCEPTER] = peer->channel->view->owed[opener ? REMOTE : LOCAL]; + /* pending_htlcs holds the value of all pending htlcs for each side */ + pending_htlcs[TX_INITIATOR] = AMOUNT_MSAT(0); + pending_htlcs[TX_ACCEPTER] = AMOUNT_MSAT(0); + for (htlc = htlc_map_first(peer->channel->htlcs, &it); + htlc; + htlc = htlc_map_next(peer->channel->htlcs, &it)) { + struct amount_msat *itr; + + if (htlc_owner(htlc) == opener ? LOCAL : REMOTE) + itr = &pending_htlcs[TX_INITIATOR]; + else + itr = &pending_htlcs[TX_ACCEPTER]; + + if (!amount_msat_add(itr, *itr, htlc->amount)) + peer_failed_warn(peer->pps, &peer->channel_id, + "Unable to add HTLC balance"); + } + for (size_t i = 0; i < psbt->num_inputs; i++) if (i != chan_input_index) add_amount_to_side(peer, in, @@ -2963,12 +2985,22 @@ static struct amount_sat check_balances(struct peer *peer, psbt_output_get_amount(psbt, i), &psbt->outputs[i].unknowns); - /* Calculate total channel output amount */ + /* Calculate original channel output amount */ if (!amount_msat_add(&funding_amount, peer->channel->view->owed[LOCAL], peer->channel->view->owed[REMOTE])) peer_failed_warn(peer->pps, &peer->channel_id, "Unable to calculate starting channel amount"); + if (!amount_msat_add(&funding_amount, + funding_amount, + pending_htlcs[TX_INITIATOR])) + peer_failed_warn(peer->pps, &peer->channel_id, + "Unable to calculate starting channel amount"); + if (!amount_msat_add(&funding_amount, + funding_amount, + pending_htlcs[TX_ACCEPTER])) + peer_failed_warn(peer->pps, &peer->channel_id, + "Unable to calculate starting channel amount"); /* Tasks: * Add up total funding_amount @@ -3011,9 +3043,13 @@ static struct amount_sat check_balances(struct peer *peer, peer_failed_warn(peer->pps, &peer->channel_id, "Initiator funding is less than commited" " amount. Initiator contributing %s but they" - " committed to %s.", + " committed to %s. Pending offered HTLC" + " balance of %s is not available for this" + " operation.", fmt_amount_msat(tmpctx, in[TX_INITIATOR]), - fmt_amount_msat(tmpctx, out[TX_INITIATOR])); + fmt_amount_msat(tmpctx, out[TX_INITIATOR]), + fmt_amount_msat(tmpctx, + pending_htlcs[TX_INITIATOR])); } if (!amount_msat_sub(&initiator_fee, in[TX_INITIATOR], out[TX_INITIATOR])) @@ -3029,9 +3065,13 @@ static struct amount_sat check_balances(struct peer *peer, peer_failed_warn(peer->pps, &peer->channel_id, "Accepter funding is less than commited" " amount. Accepter contributing %s but they" - " committed to %s.", + " committed to %s. Pending offered HTLC" + " balance of %s is not available for this" + " operation.", fmt_amount_msat(tmpctx, in[TX_INITIATOR]), - fmt_amount_msat(tmpctx, out[TX_INITIATOR])); + fmt_amount_msat(tmpctx, out[TX_INITIATOR]), + fmt_amount_msat(tmpctx, + pending_htlcs[TX_INITIATOR])); } if (!amount_msat_sub(&accepter_fee, in[TX_ACCEPTER], out[TX_ACCEPTER])) diff --git a/tests/test_splicing.py b/tests/test_splicing.py index b2a085a00..2506637c6 100644 --- a/tests/test_splicing.py +++ b/tests/test_splicing.py @@ -276,3 +276,44 @@ def test_commit_crash_splice(node_factory, bitcoind): # Check that the splice doesn't generate a unilateral close transaction time.sleep(5) assert l1.db_query("SELECT count(*) as c FROM channeltxs;")[0]['c'] == 0 + + +@pytest.mark.openchannel('v1') +@pytest.mark.openchannel('v2') +@unittest.skipIf(TEST_NETWORK != 'regtest', 'elementsd doesnt yet support PSBT features we need') +def test_splice_stuck_htlc(node_factory, bitcoind, executor): + l1, l2, l3 = node_factory.line_graph(3, wait_for_announce=True, opts={'experimental-splicing': None}) + + l3.rpc.dev_ignore_htlcs(id=l2.info['id'], ignore=True) + + inv = l3.rpc.invoice(10000000, '1', 'no_1') + executor.submit(l1.rpc.pay, inv['bolt11']) + l3.daemon.wait_for_log('their htlc 0 dev_ignore_htlcs') + + # Now we should have a stuck invoice between l1 -> l2 + + chan_id = l1.get_channel_id(l2) + + # add extra sats to pay fee + funds_result = l1.rpc.fundpsbt("109000sat", "slow", 166, excess_as_change=True) + + result = l1.rpc.splice_init(chan_id, 100000, funds_result['psbt']) + result = l1.rpc.splice_update(chan_id, result['psbt']) + result = l1.rpc.signpsbt(result['psbt']) + result = l1.rpc.splice_signed(chan_id, result['signed_psbt']) + + l2.daemon.wait_for_log(r'CHANNELD_NORMAL to CHANNELD_AWAITING_SPLICE') + l1.daemon.wait_for_log(r'CHANNELD_NORMAL to CHANNELD_AWAITING_SPLICE') + + mempool = bitcoind.rpc.getrawmempool(True) + assert len(list(mempool.keys())) == 1 + assert result['txid'] in list(mempool.keys()) + + bitcoind.generate_block(6, wait_for_mempool=1) + + l2.daemon.wait_for_log(r'CHANNELD_AWAITING_SPLICE to CHANNELD_NORMAL') + l1.daemon.wait_for_log(r'CHANNELD_AWAITING_SPLICE to CHANNELD_NORMAL') + + # Check that the splice doesn't generate a unilateral close transaction + time.sleep(5) + assert l1.db_query("SELECT count(*) as c FROM channeltxs;")[0]['c'] == 0