pytest: fix coinmoves flake, where routing credit/debit can appear in either order.
What the developer wrote
Authored by Rusty Russell
pytest: fix coinmoves flake, where routing credit/debit can appear in either order.
It's done as HTLCs finalize, but we can close the incoming HTLC as soon as we get the
preimage, so that entire thing could finish before the outgoing HTLC.
```
> check_channel_moves(l1, expected_channel1)
tests/test_coinmoves.py:307:
...
E Full diff:
E [
E {
E 'account_id': '58d371ab100e0ea847a11c9550add273ef8531bc12bb51b0e30c8f833506a772',
E 'created_index': 1,
E 'credit_msat': 0,
E 'debit_msat': 1000000,
E 'fees_msat': 0,
E 'group_id': 1318196858430961660,
E 'part_id': 1,
E 'payment_hash': '8da829ab29715106a4e767facc0b58776ae5bfc11c4e9dcda3063013e1ef8ef0',
E 'primary_tag': 'invoice',
E },
E {
E 'account_id': '0b872506f67b363803cd85cf9ff6807ebc1dc8a4521aa191386b4c5366d490d7',
E 'created_index': 2,
E 'credit_msat': 100000,
E 'debit_msat': 0,
E 'fees_msat': 0,
E 'primary_tag': 'pushed',
E },
E {
E + 'account_id': '0b872506f67b363803cd85cf9ff6807ebc1dc8a4521aa191386b4c5366d490d7',
E + 'created_index': 3,
E + 'credit_msat': 10000100001,
E + 'debit_msat': 0,
E + 'fees_msat': 100001,
E + 'payment_hash': '0ebfa5387de5fd12c15089833b0193fb6007e9f494ec24d479e327a96ac8e8c0',
E + 'primary_tag': 'routed',
E + },
E + {
E 'account_id': '58d371ab100e0ea847a11c9550add273ef8531bc12bb51b0e30c8f833506a772',
E - 'created_index': 3,
E ? ^
E + 'created_index': 4,
E ? ^
E 'credit_msat': 0,
E 'debit_msat': 10000000000,
E 'fees_msat': 100001,
E 'payment_hash': '0ebfa5387de5fd12c15089833b0193fb6007e9f494ec24d479e327a96ac8e8c0',
E 'primary_tag': 'routed',
E },
E - {
E - 'account_id': '0b872506f67b363803cd85cf9ff6807ebc1dc8a4521aa191386b4c5366d490d7',
E - 'created_index': 4,
E - 'credit_msat': 10000100001,
E - 'debit_msat': 0,
E - 'fees_msat': 100001,
E - 'payment_hash': '0ebfa5387de5fd12c15089833b0193fb6007e9f494ec24d479e327a96ac8e8c0',
E - 'primary_tag': 'routed',
E - },
E ]
```
What changed, and why it matters
This commit fixes a flaky test, not a security bug. The test was checking the order of accounting records for routed payments, but the two records can legitimately appear in either order depending on which payment step finishes first. The patch makes the test accept either order. There is no product vulnerability or user impact.
No security action needed. This is a test-only reliability fix.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The change is in tests/test_coinmoves.py only. It updates test_coinmoves to handle non-deterministic ordering of channelmoves entries for a routed payment. The incoming debit and outgoing credit are recorded when each HTLC fully resolves, and the incoming HTLC can resolve before the outgoing one, so the order is not guaranteed. The patch waits for at least one new move, inspects whether the next move is a debit or credit, and appends the expected pair in the matching order. No production code is modified.
Changed components
tests/test_coinmoves.pyInspect captured patch +29 / −12
diff --git a/tests/test_coinmoves.py b/tests/test_coinmoves.py
index 927347d0..b3679fbb 100644
--- a/tests/test_coinmoves.py
+++ b/tests/test_coinmoves.py
@@ -286,18 +286,35 @@ def test_coinmoves(node_factory, bitcoind):
l3.rpc.xpay(inv['bolt11'], '10000000sat')
# Make sure it's fully settled.
wait_for(lambda: only_one(l3.rpc.listpeerchannels(l1.info['id'])['channels'])['htlcs'] == [])
- expected_channel1 += [{'account_id': fundchannel['channel_id'],
- 'credit_msat': 0,
- 'debit_msat': 10000000000,
- 'fees_msat': 100001,
- 'payment_hash': inv['payment_hash'],
- 'primary_tag': 'routed'},
- {'account_id': l3fundchannel['channel_id'],
- 'credit_msat': 10000100001,
- 'debit_msat': 0,
- 'fees_msat': 100001,
- 'payment_hash': inv['payment_hash'],
- 'primary_tag': 'routed'}]
+ # These can actually go in either order, since we record them when HTLC is *fully*
+ # resolved.
+ wait_for(lambda: len(l1.rpc.listchannelmoves()['channelmoves']) > len(expected_channel1))
+ if l1.rpc.listchannelmoves()['channelmoves'][len(expected_channel1)]['credit_msat'] == 0:
+ expected_channel1 += [{'account_id': fundchannel['channel_id'],
+ 'credit_msat': 0,
+ 'debit_msat': 10000000000,
+ 'fees_msat': 100001,
+ 'payment_hash': inv['payment_hash'],
+ 'primary_tag': 'routed'},
+ {'account_id': l3fundchannel['channel_id'],
+ 'credit_msat': 10000100001,
+ 'debit_msat': 0,
+ 'fees_msat': 100001,
+ 'payment_hash': inv['payment_hash'],
+ 'primary_tag': 'routed'}]
+ else:
+ expected_channel1 += [{'account_id': l3fundchannel['channel_id'],
+ 'credit_msat': 10000100001,
+ 'debit_msat': 0,
+ 'fees_msat': 100001,
+ 'payment_hash': inv['payment_hash'],
+ 'primary_tag': 'routed'},
+ {'account_id': fundchannel['channel_id'],
+ 'credit_msat': 0,
+ 'debit_msat': 10000000000,
+ 'fees_msat': 100001,
+ 'payment_hash': inv['payment_hash'],
+ 'primary_tag': 'routed'}]
expected_channel2 += [{'account_id': fundchannel['channel_id'],
'credit_msat': 10000000000,
'debit_msat': 0,
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.