channeld: allow transient negative balance. #3286
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Travis randomly picked up an error in test_feerate_stress: **BROKEN** 0266e4598d1d3c415f572a8488830b60f7e744ed9235eb0b1ba93283b315c03518-channeld-chan#1: Cannot add htlc #0 10000msat to LOCAL (version a2541b9-modded) This is because it hit an unlikely corner case involving applying multiple HTLCs (similar to the previous c96cee9). In this case, the test sends a 500,000,000 "balancing" setup payment L1->L2. It waits for L2 to get the preimage (which is the when pay() helper returns), but crucially, it starts spamming with HTLCs before that HTLC is completely removed. From L2's point of view, the setup HTLC is in state RCVD_REMOVE_REVOCATION; gone from L1's commitment tx, but still waiting for the commitment_signed from L1 to remove it from L2's. Note that each side keeps a local and remove view of both sides' current balances: at this point, L2's view is REMOTE: "500,000,000 to L1, 499,900,000 to L2", LOCAL: "500,000,000 to L1, 0 to L2". L2 sends a 10,000 msat HTLC to L1: legal, since L1 will allow it, then the commitment_signed. L1 sends the revoke-and-ack for this, *then* belatedly follows with the commitment_signed which both completes the removal of the setup HTLC and adds the new one. But L2 processes the HTLCs in hashtable (i.e. random) order: so if it tries to apply its own HTLC first, it freaks out because it doesn't have funds in its local view. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> Changelog-Fixed: Unlikely corner case is simultanous HTLCs near balance limits fixed.
|
ACK 90a9bd2 |
2 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Travis randomly picked up an error in test_feerate_stress:
BROKEN 0266e4598d1d3c415f572a8488830b60f7e744ed9235eb0b1ba93283b315c03518-channeld-chan#1: Cannot add htlc #0 10000msat to LOCAL (version a2541b9-modded)
This is because it hit an unlikely corner case involving applying multiple HTLCs
(similar to the previous c96cee9).
In this case, the test sends a 500,000,000 "balancing" setup payment L1->L2.
It waits for L2 to get the preimage (which is the when pay() helper returns),
but crucially, it starts spamming with HTLCs before that HTLC is completely
removed.
From L2's point of view, the setup HTLC is in state RCVD_REMOVE_REVOCATION;
gone from L1's commitment tx, but still waiting for the commitment_signed
from L1 to remove it from L2's.
Note that each side keeps a local and remove view of both sides' current
balances: at this point, L2's view is REMOTE: "500,000,000 to L1, 499,900,000
to L2", LOCAL: "500,000,000 to L1, 0 to L2".
L2 sends a 10,000 msat HTLC to L1: legal, since L1 will allow it,
then the commitment_signed. L1 sends the revoke-and-ack for this,
then belatedly follows with the commitment_signed which both completes the
removal of the setup HTLC and adds the new one.
But L2 processes the HTLCs in hashtable (i.e. random) order: so if it
tries to apply its own HTLC first, it freaks out because it doesn't have
funds in its local view.
Signed-off-by: Rusty Russell rusty@rustcorp.com.au
Changelog-Fixed: Unlikely corner case is simultanous HTLCs near balance limits fixed.