Skip to content

Commit

Permalink
bkpr: fixup htlc penalty test with anchors
Browse files Browse the repository at this point in the history
Test flake where the balance for lightning-2 went negative

```
>       assert account_balance(l2, channel_id) == 0

tests/test_closing.py:1314:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
tests/utils.py:183: in account_balance
    m_sum -= Millisatoshi(m['debit_msat'])
contrib/pyln-client/pyln/client/lightning.py:193: in __sub__
    return Millisatoshi(int(self) - int(other))
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = -10000msat, v = -10000
```

Led me to look into this test. lightning-2 should go negative since we
roll back the amounts it's received by going to a prior database state.

Rather than trying to do the right thing with obviously broken node
records, instead we just stop trying to account for them correctly
(impossible).

I also noticed that the anchor tests were failing the utxo output
matchup, which we should be asserting on it. The HTLC RBF that our
anchor code creates was causing an issue by creating another wallet
deposit utxo under the HTLC output. We now optionally add this utxo
in the case that anchors are turned on.

Changelog-None: Fix test flake
  • Loading branch information
niftynei authored and endothermicdev committed Feb 7, 2025
1 parent 473aefd commit fc08340
Showing 1 changed file with 6 additions and 20 deletions.
26 changes: 6 additions & 20 deletions tests/test_closing.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
account_balance, first_channel_id, closing_fee, TEST_NETWORK,
scriptpubkey_addr, calc_lease_fee,
check_utxos_channel, check_coin_moves,
check_balance_snaps, mine_funding_to_announce, check_inspect_channel,
mine_funding_to_announce, check_inspect_channel,
first_scid
)

Expand Down Expand Up @@ -1311,7 +1311,7 @@ def test_penalty_htlc_tx_fulfill(node_factory, bitcoind, chainparams, anchors):
l2.daemon.wait_for_log('{}.*: onchaind complete, forgetting peer'.format(l3.info['id']))

assert account_balance(l3, channel_id) == 0
assert account_balance(l2, channel_id) == 0
# we can't check the account balance on l2 because it goes negative

expected_2 = {
'A': [('cid1', ['channel_open', 'opener'], ['channel_close'], 'B')],
Expand All @@ -1332,25 +1332,11 @@ def test_penalty_htlc_tx_fulfill(node_factory, bitcoind, chainparams, anchors):
expected_3['B'].append(('external', ['anchor'], None, None))
expected_2['B'].append(('wallet', ['anchor', 'ignored'], None, None))
expected_3['B'].append(('wallet', ['anchor', 'ignored'], None, None))
# We RBF spend the HTLC tx, which creates a new deposit
expected_2['C'].append(('wallet', ['deposit'], None, None))

# FIXME: Why does this fail?
if not anchors:
tags = check_utxos_channel(l2, [channel_id], expected_2, filter_channel=channel_id)
check_utxos_channel(l3, [channel_id], expected_3, tags, filter_channel=channel_id)

if not chainparams['elements']:
# Also check snapshots
expected_bals_2 = [
{'blockheight': 101, 'accounts': [
{'balance_msat': '0msat', 'account_id': 'wallet'}
]}
] + [
{'blockheight': 108, 'accounts': [
{'balance_msat': '995073000msat', 'account_id': 'wallet'},
{'balance_msat': '500000000msat', 'account_id': first_channel_id(l1, l2)},
{'balance_msat': '499994999msat', 'account_id': channel_id}]}
] * 2 # duplicated; we stop and restart l2 twice (both at block 108)
check_balance_snaps(l2, expected_bals_2)
tags = check_utxos_channel(l2, [channel_id], expected_2, filter_channel=channel_id)
check_utxos_channel(l3, [channel_id], expected_3, tags, filter_channel=channel_id)


@unittest.skipIf(os.getenv('TEST_DB_PROVIDER', 'sqlite3') != 'sqlite3', "Makes use of the sqlite3 db")
Expand Down

0 comments on commit fc08340

Please sign in to comment.