JITrebalance plugin improvements

[darosior]

Just a summary to keep a trace of the discussion on IRC :

• Don't log onions
• Don't try again channels that were tried some seconds ago
• Maybe exhaust all possible routes instead of stopping at 5 ?

[darosior]

Wrt to exhausting all possibilities, maybe something like :

diff --git a/jitrebalance/jitrebalance.py b/jitrebalance/jitrebalance.py
index d64a04e..d8519fd 100755
--- a/jitrebalance/jitrebalance.py
+++ b/jitrebalance/jitrebalance.py
@@ -41,17 +41,18 @@ def get_circular_route(scid, chan, amt, peer, exclusions, request):
         riskfactor=1,
         exclude=exclusions,
         cltv=last_cltv,
-    )['route']
-
-    # Append the last hop we computed manually above
-    route += [{
-        'id': plugin.node_id,
-        'channel': scid,
-        'direction': chan['direction'],
-        'msatoshi': amt,
-        'amount_msat': '{}msat'.format(amt),
-        'delay': 9
-    }]
+    ).get('route', None)
+
+    if route is not None:
+        # Append the last hop we computed manually above
+        route += [{
+            'id': plugin.node_id,
+            'channel': scid,
+            'direction': chan['direction'],
+            'msatoshi': amt,
+            'amount_msat': '{}msat'.format(amt),
+            'delay': 9
+        }]

     return route

@@ -65,8 +66,14 @@ def try_rebalance(scid, chan, amt, peer, request):
     ]

     # Try up to 5 times to rebalance that last leg.
-    for i in range(0, 5):
+    while True:
         route = get_circular_route(scid, chan, amt, peer, exclusions, request)
+        # We exhausted all the possibilities, Game Over
+        if route is None:
+            plugin.log("Could not get a route, no remaining one? Exclusions : {}"
+                       .format(exclusions))
+            request.set_result({"result": "continue"})
+            return

         # We're about to initiate a rebalancing, we'd better remember how we can
         # settle it once we see it back here.

Potentially adding a timeout, too

[cdecker]

I like the idea of moving from a fixed number of attempts to a timeout-based system, because that better reflects the tradeoff at hand: more attempts means longer delays for the end-to-end payment, but also increases the chances of not having to retry from the start, while attempts may take very long or no time at all.

Exhaustive search without a timeout is not doable, simply because there are exponentially many paths to try as the network grows, but with timeout it's very much doable :+1:

[darosior]

b'2020-05-27T13:58:42.873Z INFO plugin-jitrebalance.py: Excluding 110x1x0 due to a failed attempt'
b"2020-05-27T13:58:42.888Z INFO plugin-jitrebalance.py: Sending rebalance request using payment_hash=2c56fe5c5e390848b1f9c3e630872de20f3727dc3e4ca24839a18e1257e9d144, route=[{'id': '032cf15d1ad9c4a08d26eab1918f732d8ef8fdc6abb9640bf3db174372c491304e', 'channel': '124x1x0', 'direction': 0, 'msatoshi': 1248835, 'amount_msat': 1248835msat, 'delay': 21, 'style': 'tlv'}, {'id': '035d2b1192dfba134e10e540875d366ebc8bc353d5aa766b80c090b39c3a5d885d', 'channel': '131x1x0', 'direction': 0, 'msatoshi': 1248822, 'amount_msat': 1248822msat, 'delay': 15, 'style': 'tlv'}, {'id': '022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59', 'channel': '110x1x0', 'direction': 0, 'msatoshi': 1248808, 'amount_msat': '1248808msat', 'delay': 9}]"
b'2020-05-27T13:58:43.340Z INFO plugin-jitrebalance.py: Excluding 110x1x0 due to a failed attempt'
b"2020-05-27T13:58:43.354Z INFO plugin-jitrebalance.py: Sending rebalance request using payment_hash=f7c5d52e46e55c7b80f2dda753b1ced0eed3f5de996f01f5ffd0ebe4115abceb, route=[{'id': '032cf15d1ad9c4a08d26eab1918f732d8ef8fdc6abb9640bf3db174372c491304e', 'channel': '124x1x0', 'direction': 0, 'msatoshi': 1248835, 'amount_msat': 1248835msat, 'delay': 21, 'style': 'tlv'}, {'id': '035d2b1192dfba134e10e540875d366ebc8bc353d5aa766b80c090b39c3a5d885d', 'channel': '131x1x0', 'direction': 0, 'msatoshi': 1248822, 'amount_msat': 1248822msat, 'delay': 15, 'style': 'tlv'}, {'id': '022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59', 'channel': '110x1x0', 'direction': 0, 'msatoshi': 1248808, 'amount_msat': '1248808msat', 'delay': 9}]"
b'2020-05-27T13:58:43.790Z INFO plugin-jitrebalance.py: Excluding 110x1x0 due to a failed attempt'
b"2020-05-27T13:58:43.805Z INFO plugin-jitrebalance.py: Sending rebalance request using payment_hash=2b26f2f464d92c20e5db82ff2c9a8eaa8c70647ceb0bab2485a807ecfa6178a9, route=[{'id': '032cf15d1ad9c4a08d26eab1918f732d8ef8fdc6abb9640bf3db174372c491304e', 'channel': '124x1x0', 'direction': 0, 'msatoshi': 1248835, 'amount_msat': 1248835msat, 'delay': 21, 'style': 'tlv'}, {'id': '035d2b1192dfba134e10e540875d366ebc8bc353d5aa766b80c090b39c3a5d885d', 'channel': '131x1x0', 'direction': 0, 'msatoshi': 1248822, 'amount_msat': 1248822msat, 'delay': 15, 'style': 'tlv'}, {'id': '022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59', 'channel': '110x1x0', 'direction': 0, 'msatoshi': 1248808, 'amount_msat': '1248808msat', 'delay': 9}]"
b'2020-05-27T13:58:44.233Z INFO plugin-jitrebalance.py: Excluding 110x1x0 due to a failed attempt'

Hmmmmm ^^

[darosior]

OK it was because the destination, added to the route no matter the exclusions, sent the errors

[darosior]

JITrebalance crashed again (but at least not lightningd!)

2020-05-29T00:01:31.727Z INFO plugin-jitrebalance.py: Got an incoming HTLC htlc={'cltv_expiry': 632349, 'cltv_expiry_relative': 241, 'amount': '200074206msat', 'payment_hash': '1781cbdb4b86aa1fa161e896206e699aa14ed9801cdddca6e406a784f2243266'}
2020-05-29T00:01:31.728Z INFO lightningd: Sending 200157717msat over 5 hops to deliver 200157517msat
2020-05-29T00:01:31.740Z DEBUG 02ad6fb8d693dc1e4569bcedefadf5f72a931ae027dc0f0c544b34c1c6f3b9a02b-channeld-chan#4931: Failed to add 1 remove 0 htlcs
2020-05-29T00:01:31.740Z DEBUG 02ad6fb8d693dc1e4569bcedefadf5f72a931ae027dc0f0c544b34c1c6f3b9a02b-channeld-chan#4931: Adding HTLC 4525 amount=200157717msat cltv=632163 gave CHANNEL_ERR_CHANNEL_CAPACITY_EXCEEDED
2020-05-29T00:01:31.740Z DEBUG 02ad6fb8d693dc1e4569bcedefadf5f72a931ae027dc0f0c544b34c1c6f3b9a02b-gossipd: schanid 614062x2375x0: got update
2020-05-29T00:01:31.740Z DEBUG 02ad6fb8d693dc1e4569bcedefadf5f72a931ae027dc0f0c544b34c1c6f3b9a02b-channeld-chan#4931: REPLY WIRE_CHANNEL_OFFER_HTLC_REPLY with 0 fds
2020-05-29T00:01:31.740Z DEBUG 02ad6fb8d693dc1e4569bcedefadf5f72a931ae027dc0f0c544b34c1c6f3b9a02b-chan#4931: local_routing_failure: 4103 (WIRE_TEMPORARY_CHANNEL_FAILURE)
2020-05-29T00:01:31.945Z DEBUG 0298f6074a454a1f5345cb2a7c6f9fce206cd0bf675d177cdbf0ca7508dd28852f-gossipd: Received channel_update for channel 629201x1816x0/1 now ACTIVE
2020-05-29T00:01:31.946Z DEBUG 0298f6074a454a1f5345cb2a7c6f9fce206cd0bf675d177cdbf0ca7508dd28852f-gossipd: Ignoring spammy update for 629201x1816x0/1 (last 1590709641, now 1590710370)
2020-05-29T00:01:31.947Z DEBUG lightningd: Payment part 0/0 status 2
2020-05-29T00:01:31.947Z DEBUG gossipd: REPLY WIRE_GOSSIP_GETCHANNELS_REPLY with 0 fds
2020-05-29T00:01:31.948Z INFO plugin-jitrebalance.py: Could not compute parameters for the last hop
2020-05-29T00:01:31.948Z INFO plugin-jitrebalance.py: Could not get a route, no remaining one? Exclusions : ['631995x1352x0/1']
2020-05-29T00:01:31.948Z INFO plugin-jitrebalance.py: Killing plugin: Received a JSON-RPC response for non-existent request

EDIT: Ok, this was a simple one. Added a commit to https://github.com/lightningd/plugins/pull/121

[darosior]

New crash with (apparently..) a doubled response :

2020-07-03T02:28:19.021Z INFO plugin-jitrebalance.py: Excluding xxxxx

--snipped--

2020-07-03T02:28:19.051Z INFO plugin-jitrebalance.py: Sending rebalance request using payment_hash=

--snipped--

2020-07-03T02:28:33.909Z INFO plugin-jitrebalance.py: Timed out while trying to rebalance

--snipped--

2020-07-03T02:28:38.134Z INFO plugin-jitrebalance.py: Could not compute parameters for the last hop
2020-07-03T02:28:38.135Z INFO plugin-jitrebalance.py: Could not get a route, no remaining one? Exclusions : ['607582x1309x0/0']
2020-07-03T02:28:38.135Z INFO plugin-jitrebalance.py: Killing plugin: Received a JSON-RPC response for non-existent request

[ca-ruz]

@chrisguida This plugin has been archived.

[chrisguida]

Thanks @ca-ruz !

Closing due to jitrebalance being archived.