commit fda6fb652110fb26b22e15bf21d54dd4b153bab7
parent f47519bdf3d6551dfe3287824b24446fc91450d9
Author: SomberNight <somber.night@protonmail.com>
Date: Thu, 21 Mar 2019 17:34:42 +0100
lnhtlc: handle fails asymmetrically
Diffstat:
2 files changed, 103 insertions(+), 85 deletions(-)
diff --git a/electrum/lnhtlc.py b/electrum/lnhtlc.py
@@ -23,8 +23,7 @@ class HTLCManager:
# "side who offered htlc" -> action -> htlc_id -> whose ctx -> ctn
log[sub]['locked_in'] = {int(x): coerceHtlcOwner2IntMap(y) for x, y in log[sub]['locked_in'].items()}
log[sub]['settles'] = {int(x): coerceHtlcOwner2IntMap(y) for x, y in log[sub]['settles'].items()}
- # FIXME "fails" should be handled like "settles"
- log[sub]['fails'] = {int(x): y for x, y in log[sub]['fails'].items()}
+ log[sub]['fails'] = {int(x): coerceHtlcOwner2IntMap(y) for x, y in log[sub]['fails'].items()}
self.log = log
def to_save(self):
@@ -67,9 +66,10 @@ class HTLCManager:
def send_rev(self) -> None:
self.ctn[LOCAL] += 1
- for htlc_id, ctns in self.log[LOCAL]['settles'].items():
- if ctns[REMOTE] is None:
- ctns[REMOTE] = self.ctn[REMOTE] + 1
+ for log_action in ('settles', 'fails'):
+ for htlc_id, ctns in self.log[LOCAL][log_action].items():
+ if ctns[REMOTE] is None:
+ ctns[REMOTE] = self.ctn[REMOTE] + 1
def recv_rev(self) -> None:
self.ctn[REMOTE] += 1
@@ -77,9 +77,10 @@ class HTLCManager:
if ctns[LOCAL] is None:
assert ctns[REMOTE] == self.ctn[REMOTE]
ctns[LOCAL] = self.ctn[LOCAL] + 1
- for htlc_id, ctns in self.log[REMOTE]['settles'].items():
- if ctns[LOCAL] is None:
- ctns[LOCAL] = self.ctn[LOCAL] + 1
+ for log_action in ('settles', 'fails'):
+ for htlc_id, ctns in self.log[REMOTE][log_action].items():
+ if ctns[LOCAL] is None:
+ ctns[LOCAL] = self.ctn[LOCAL] + 1
def htlcs_by_direction(self, subject: HTLCOwner, direction: Direction,
ctn: int = None) -> Sequence[UpdateAddHtlc]:
@@ -105,9 +106,10 @@ class HTLCManager:
include = htlc_height <= ctn
if include:
settles = self.log[party]['settles']
- if htlc_id not in settles or settles[htlc_id][subject] is None or settles[htlc_id][subject] > ctn:
- fails = self.log[party]['fails']
- if htlc_id not in fails or fails[htlc_id] > ctn:
+ fails = self.log[party]['fails']
+ not_settled = htlc_id not in settles or settles[htlc_id][subject] is None or settles[htlc_id][subject] > ctn
+ not_failed = htlc_id not in fails or fails[htlc_id][subject] is None or fails[htlc_id][subject] > ctn
+ if not_settled and not_failed:
l.append(self.log[party]['adds'][htlc_id])
return l
@@ -179,7 +181,7 @@ class HTLCManager:
if ctns[LOCAL] == ctn]
def send_fail(self, htlc_id: int) -> None:
- self.log[REMOTE]['fails'][htlc_id] = self.ctn[REMOTE] + 1
+ self.log[REMOTE]['fails'][htlc_id] = {LOCAL: None, REMOTE: self.ctn[REMOTE] + 1}
def recv_fail(self, htlc_id: int) -> None:
- self.log[LOCAL]['fails'][htlc_id] = self.ctn[LOCAL] + 1
+ self.log[LOCAL]['fails'][htlc_id] = {LOCAL: self.ctn[LOCAL] + 1, REMOTE: None}
diff --git a/electrum/tests/test_lnhtlc.py b/electrum/tests/test_lnhtlc.py
@@ -9,7 +9,7 @@ class H(NamedTuple):
htlc_id : int
class TestHTLCManager(unittest.TestCase):
- def test_race(self):
+ def test_adding_htlcs_race(self):
A = HTLCManager()
B = HTLCManager()
ah0, bh0 = H('A', 0), H('B', 0)
@@ -41,81 +41,97 @@ class TestHTLCManager(unittest.TestCase):
self.assertEqual(B.current_htlcs(LOCAL), [(RECEIVED, ah0), (SENT, bh0)][::-1])
self.assertEqual(A.current_htlcs(LOCAL), [(RECEIVED, bh0), (SENT, ah0)][::-1])
- def test_no_race(self):
- A = HTLCManager()
- B = HTLCManager()
- B.recv_htlc(A.send_htlc(H('A', 0)))
- self.assertEqual(len(B.pending_htlcs(REMOTE)), 0)
- self.assertEqual(len(A.pending_htlcs(REMOTE)), 1)
- self.assertEqual(len(B.pending_htlcs(LOCAL)), 1)
- self.assertEqual(len(A.pending_htlcs(LOCAL)), 0)
- A.send_ctx()
- B.recv_ctx()
- B.send_rev()
- A.recv_rev()
- B.send_ctx()
- A.recv_ctx()
- A.send_rev()
- B.recv_rev()
- self.assertEqual(len(A.current_htlcs(LOCAL)), 1)
- self.assertEqual(len(B.current_htlcs(LOCAL)), 1)
- B.send_settle(0)
- A.recv_settle(0)
- self.assertEqual(A.htlcs_by_direction(REMOTE, RECEIVED), [H('A', 0)])
- self.assertNotEqual(A.current_htlcs(LOCAL), [])
- self.assertNotEqual(B.current_htlcs(REMOTE), [])
+ def test_single_htlc_full_lifecycle(self):
+ def htlc_lifecycle(htlc_success: bool):
+ A = HTLCManager()
+ B = HTLCManager()
+ B.recv_htlc(A.send_htlc(H('A', 0)))
+ self.assertEqual(len(B.pending_htlcs(REMOTE)), 0)
+ self.assertEqual(len(A.pending_htlcs(REMOTE)), 1)
+ self.assertEqual(len(B.pending_htlcs(LOCAL)), 1)
+ self.assertEqual(len(A.pending_htlcs(LOCAL)), 0)
+ A.send_ctx()
+ B.recv_ctx()
+ B.send_rev()
+ A.recv_rev()
+ B.send_ctx()
+ A.recv_ctx()
+ A.send_rev()
+ B.recv_rev()
+ self.assertEqual(len(A.current_htlcs(LOCAL)), 1)
+ self.assertEqual(len(B.current_htlcs(LOCAL)), 1)
+ if htlc_success:
+ B.send_settle(0)
+ A.recv_settle(0)
+ else:
+ B.send_fail(0)
+ A.recv_fail(0)
+ self.assertEqual(A.htlcs_by_direction(REMOTE, RECEIVED), [H('A', 0)])
+ self.assertNotEqual(A.current_htlcs(LOCAL), [])
+ self.assertNotEqual(B.current_htlcs(REMOTE), [])
- self.assertEqual(A.pending_htlcs(LOCAL), [])
- self.assertNotEqual(A.pending_htlcs(REMOTE), [])
- self.assertEqual(A.pending_htlcs(REMOTE), A.current_htlcs(REMOTE))
+ self.assertEqual(A.pending_htlcs(LOCAL), [])
+ self.assertNotEqual(A.pending_htlcs(REMOTE), [])
+ self.assertEqual(A.pending_htlcs(REMOTE), A.current_htlcs(REMOTE))
- self.assertEqual(B.pending_htlcs(REMOTE), [])
- B.send_ctx()
- A.recv_ctx()
- A.send_rev() # here pending_htlcs(REMOTE) should become empty
- self.assertEqual(A.pending_htlcs(REMOTE), [])
+ self.assertEqual(B.pending_htlcs(REMOTE), [])
+ B.send_ctx()
+ A.recv_ctx()
+ A.send_rev() # here pending_htlcs(REMOTE) should become empty
+ self.assertEqual(A.pending_htlcs(REMOTE), [])
- B.recv_rev()
- A.send_ctx()
- B.recv_ctx()
- B.send_rev()
- A.recv_rev()
- self.assertEqual(B.current_htlcs(LOCAL), [])
- self.assertEqual(A.current_htlcs(LOCAL), [])
- self.assertEqual(A.current_htlcs(REMOTE), [])
- self.assertEqual(B.current_htlcs(REMOTE), [])
- self.assertEqual(len(A.all_settled_htlcs_ever(LOCAL)), 1)
- self.assertEqual(len(A.sent_in_ctn(2)), 1)
- self.assertEqual(len(B.received_in_ctn(2)), 1)
+ B.recv_rev()
+ A.send_ctx()
+ B.recv_ctx()
+ B.send_rev()
+ A.recv_rev()
+ self.assertEqual(B.current_htlcs(LOCAL), [])
+ self.assertEqual(A.current_htlcs(LOCAL), [])
+ self.assertEqual(A.current_htlcs(REMOTE), [])
+ self.assertEqual(B.current_htlcs(REMOTE), [])
+ self.assertEqual(len(A.all_settled_htlcs_ever(LOCAL)), int(htlc_success))
+ self.assertEqual(len(A.sent_in_ctn(2)), int(htlc_success))
+ self.assertEqual(len(B.received_in_ctn(2)), int(htlc_success))
- A.recv_htlc(B.send_htlc(H('B', 0)))
- self.assertEqual(A.pending_htlcs(REMOTE), [])
- self.assertNotEqual(A.pending_htlcs(LOCAL), [])
- self.assertNotEqual(B.pending_htlcs(REMOTE), [])
- self.assertEqual(B.pending_htlcs(LOCAL), [])
+ A.recv_htlc(B.send_htlc(H('B', 0)))
+ self.assertEqual(A.pending_htlcs(REMOTE), [])
+ self.assertNotEqual(A.pending_htlcs(LOCAL), [])
+ self.assertNotEqual(B.pending_htlcs(REMOTE), [])
+ self.assertEqual(B.pending_htlcs(LOCAL), [])
- B.send_ctx()
- A.recv_ctx()
- A.send_rev()
- B.recv_rev()
+ B.send_ctx()
+ A.recv_ctx()
+ A.send_rev()
+ B.recv_rev()
- self.assertNotEqual(A.pending_htlcs(REMOTE), A.current_htlcs(REMOTE))
- self.assertEqual(A.pending_htlcs(LOCAL), A.current_htlcs(LOCAL))
- self.assertEqual(B.pending_htlcs(REMOTE), B.current_htlcs(REMOTE))
- self.assertNotEqual(B.pending_htlcs(LOCAL), B.pending_htlcs(REMOTE))
+ self.assertNotEqual(A.pending_htlcs(REMOTE), A.current_htlcs(REMOTE))
+ self.assertEqual(A.pending_htlcs(LOCAL), A.current_htlcs(LOCAL))
+ self.assertEqual(B.pending_htlcs(REMOTE), B.current_htlcs(REMOTE))
+ self.assertNotEqual(B.pending_htlcs(LOCAL), B.pending_htlcs(REMOTE))
- def test_settle_while_owing_commitment(self):
- A = HTLCManager()
- B = HTLCManager()
- B.recv_htlc(A.send_htlc(H('A', 0)))
- A.send_ctx()
- B.recv_ctx()
- B.send_rev()
- A.recv_rev()
- B.send_settle(0)
- A.recv_settle(0)
- self.assertEqual(B.pending_htlcs(REMOTE), [])
- B.send_ctx()
- A.recv_ctx()
- A.send_rev()
- B.recv_rev()
+ htlc_lifecycle(htlc_success=True)
+ htlc_lifecycle(htlc_success=False)
+
+ def test_remove_htlc_while_owing_commitment(self):
+ def htlc_lifecycle(htlc_success: bool):
+ A = HTLCManager()
+ B = HTLCManager()
+ B.recv_htlc(A.send_htlc(H('A', 0)))
+ A.send_ctx()
+ B.recv_ctx()
+ B.send_rev()
+ A.recv_rev()
+ if htlc_success:
+ B.send_settle(0)
+ A.recv_settle(0)
+ else:
+ B.send_fail(0)
+ A.recv_fail(0)
+ self.assertEqual(B.pending_htlcs(REMOTE), [])
+ B.send_ctx()
+ A.recv_ctx()
+ A.send_rev()
+ B.recv_rev()
+
+ htlc_lifecycle(htlc_success=True)
+ htlc_lifecycle(htlc_success=False)
