pytest: fix flake in tests/test_bookkeeper.py::test_bookkeeping_missed_chans_leases
What the developer wrote
Authored by Rusty Russell
pytest: fix flake in tests/test_bookkeeper.py::test_bookkeeping_missed_chans_leases
```
FAILED tests/test_bookkeeper.py::test_bookkeeping_missed_chans_leases - AssertionError: assert [{'tag': 'channel_open', 'credit_msat': 506268000, 'debit_msat': 0}, {'tag': 'lease_fee', 'credit_msat': 0, 'debit_msat': 6268000}, {'tag': 'invoice', 'credit_msat': 0, 'debit_msat': 11000000}, {'tag': 'onchain_fee', 'credit_msat': 1314000, 'debit_msat': 0}] == [{'tag': 'channel_open', 'credit_msat': 506268000, 'debit_msat': 0}, {'tag': 'lease_fee', 'credit_msat': 0, 'debit_msat': 6268000}, {'tag': 'onchain_fee', 'credit_msat': 1314000, 'debit_msat': 0}, {'tag': 'invoice', 'credit_msat': 0, 'debit_msat': 11000000}]
At index 2 diff: {'tag': 'invoice', 'credit_msat': 0, 'debit_msat': 11000000} != {'tag': 'onchain_fee', 'credit_msat': 1314000, 'debit_msat': 0}
Full diff:
[
{
'credit_msat': 506268000,
'debit_msat': 0,
'tag': 'channel_open',
},
{
'credit_msat': 0,
'debit_msat': 6268000,
'tag': 'lease_fee',
},
{
+ 'credit_msat': 0,
+ 'debit_msat': 11000000,
+ 'tag': 'invoice',
+ },
+ {
'credit_msat': 1314000,
'debit_msat': 0,
'tag': 'onchain_fee',
},
- {
- 'credit_msat': 0,
- 'debit_msat': 11000000,
- 'tag': 'invoice',
- },
]
```
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
What changed, and why it matters
This commit fixes a flaky test in the bookkeeper accounting module. The test sometimes failed because two events (an invoice payment and an on-chain fee) had timestamps so close together that their order could vary. The fix adds a short delay and waits for the payment to fully settle before checking the accounting records. There is no security issue here.
No security action needed. This is a test reliability fix; review and merge as normal.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The change modifies tests/test_bookkeeper.py::test_bookkeeping_missed_chans_leases. It adds a 2-second sleep before paying an invoice to ensure the subsequent invoice event has a timestamp sufficiently separated from the onchain_fee event, because bookkeeper sorts events by timestamp. It also replaces a daemon log wait with a wait_for() that ensures all HTLCs are fully settled before the bookkeeper is enabled and the node restarted. This eliminates a non-deterministic ordering in the expected account event list.
Changed components
tests/test_bookkeeper.pyInspect captured patch +7 / −1
diff --git a/tests/test_bookkeeper.py b/tests/test_bookkeeper.py
index 5860ad92..c6a4f06f 100644
--- a/tests/test_bookkeeper.py
+++ b/tests/test_bookkeeper.py
@@ -318,8 +318,14 @@ def test_bookkeeping_missed_chans_leases(node_factory, bitcoind):
l1.wait_local_channel_active(scid)
channel_id = first_channel_id(l1, l2)
+ # Sigh. bookkeeper sorts events by timestamp. If the invoice event happens
+ # too close, it can change the order, so sleep here.
+ time.sleep(2)
+
+ # Send l2 funds via the channel
l1.pay(l2, invoice_msat)
- l1.daemon.wait_for_log(r'coin movement:.*\'invoice\'')
+ # Make sure they're completely settled, so accounting correct.
+ wait_for(lambda: only_one(l1.rpc.listpeerchannels()['channels'])['htlcs'] == [])
# Now turn the bookkeeper on and restart
l1.stop()
Why this scored 15/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.