Cleanup rebalance sendinvoiceless (#33)

* doc: cleanup and key-value for optional parameters

* rebalance: cleanup and refinements

* sendinvoiceless: cleanup and refinements

* summary: reflects upcoming changes of pylightning to_approx_str

* rebalance: check peer connection on local channels

rebalance/README.md

@@ -18,6 +18,13 @@ rebalance channels like this:
 lightning-cli rebalance outgoing_scid incoming_scid [msatoshi] [maxfeepercent] [retry_for] [exemptfee]
 ```
 
+If you want to skip/default certain optional parameters but use others, you can
+use always the `lightning-cli -k` (key=value) syntax like this:
+
+```bash
+lightning-cli rebalance -k outgoing_scid=1514942x51x0 incoming_scid=1515133x10x0 maxfeepercent=1
+```
+
 ### Parameters
 
 - The `outgoing_scid` is the short_channel_id of the sending channel,
@@ -25,7 +32,7 @@ lightning-cli rebalance outgoing_scid incoming_scid [msatoshi] [maxfeepercent] [
 - OPTIONAL: The `msatoshi` parameter sets the amount in milli-satoshis to be
   transferred. If the parameter is left out, the plugin will calucate an amount
   that will balance the channels 50%/50%. The parameter can also be given in
-   other denominations by appending i.e. '10000sat' or '0.01btc'.
+  other denominations by appending i.e. '1000000sat', '0.01btc' or '10mbtc'.
 - OPTIONAL: `maxfeepercent` is a perecentage limit of the money to be paid in
   fees and defaults to 0.5.
 - OPTIONAL: `retry_for` defines the number of seconds the plugin will retry to

rebalance/rebalance.py

@@ -9,17 +9,28 @@ plugin = Plugin()
 def setup_routing_fees(plugin, route, msatoshi):
     delay = int(plugin.get_option('cltv-final'))
     for r in reversed(route):
-        r['msatoshi'] = r['amount_msat'] = msatoshi
+        r['msatoshi'] = msatoshi.millisatoshis
+        r['amount_msat'] = msatoshi
         r['delay'] = delay
         channels = plugin.rpc.listchannels(r['channel'])
-        for ch in channels.get('channels'):
+        ch = next(c for c in channels.get('channels') if c['destination'] == r['id'])
-            if ch['destination'] == r['id']:
         fee = Millisatoshi(ch['base_fee_millisatoshi'])
-                fee += msatoshi * ch['fee_per_millionth'] // 1000000
+        fee += msatoshi * ch['fee_per_millionth'] // 10**6
         msatoshi += fee
         delay += ch['delay']
 
 
+def get_channel(plugin, payload, peer_id, scid, check_state: bool=False):
+    peer = plugin.rpc.listpeers(peer_id).get('peers')[0]
+    channel = next(c for c in peer['channels'] if 'short_channel_id' in c and c['short_channel_id'] == scid)
+    if check_state:
+        if channel['state'] != "CHANNELD_NORMAL":
+            raise RpcError('rebalance', payload, {'message': 'Channel %s not in state CHANNELD_NORMAL, but: %s' % (scid, channel['state']) })
+        if not peer['connected']:
+            raise RpcError('rebalance', payload, {'message': 'Channel %s peer is not connected.' % scid})
+    return channel
+
+
 def amounts_from_scid(plugin, scid):
     channels = plugin.rpc.listfunds().get('channels')
     channel = next(c for c in channels if 'short_channel_id' in c and c['short_channel_id'] == scid)
@@ -27,6 +38,7 @@ def amounts_from_scid(plugin, scid):
     total_msat = Millisatoshi(channel['amount_msat'])
     return our_msat, total_msat
 
+
 def peer_from_scid(plugin, short_channel_id, my_node_id, payload):
     channels = plugin.rpc.listchannels(short_channel_id).get('channels')
     for ch in channels:
@@ -49,7 +61,7 @@ def find_worst_channel(route):
     return worst
 
 
-def rebalance_fail(plugin, label, payload, success_msg, error=None):
+def cleanup(plugin, label, payload, success_msg, error=None):
     try:
         plugin.rpc.delinvoice(label, 'unpaid')
     except RpcError as e:
@@ -79,7 +91,6 @@ def rebalance_fail(plugin, label, payload, success_msg, error=None):
 # return min(vo, vi)
 #
 # ... and cover edge cases with exceeding in/out capacity or negative values.
-# TODO: their_reserve_msat our_reserve_msat spendable_msat
 def calc_optimal_amount(out_ours, out_total, in_ours, in_total, payload):
     out_ours, out_total = int(out_ours), int(out_total)
     in_ours, in_total = int(in_ours), int(in_total)
@@ -109,12 +120,15 @@ def calc_optimal_amount(out_ours, out_total, in_ours, in_total, payload):
 
 @plugin.method("rebalance")
 def rebalance(plugin, outgoing_scid, incoming_scid, msatoshi: Millisatoshi=None,
-              maxfeepercent="0.5", retry_for="60", exemptfee: Millisatoshi=Millisatoshi(5000)):
+              maxfeepercent: float=0.5, retry_for: int=60, exemptfee: Millisatoshi=Millisatoshi(5000)):
     """Rebalancing channel liquidity with circular payments.
 
     This tool helps to move some msatoshis between your channels.
-
     """
+    msatoshi = Millisatoshi(msatoshi)
+    maxfeepercent = float(maxfeepercent)
+    retry_for = int(retry_for)
+    exemptfee = Millisatoshi(exemptfee)
     payload = {
         "outgoing_scid": outgoing_scid,
         "incoming_scid": incoming_scid,
@@ -126,52 +140,64 @@ def rebalance(plugin, outgoing_scid, incoming_scid, msatoshi: Millisatoshi=None,
     my_node_id = plugin.rpc.getinfo().get('id')
     outgoing_node_id = peer_from_scid(plugin, outgoing_scid, my_node_id, payload)
     incoming_node_id = peer_from_scid(plugin, incoming_scid, my_node_id, payload)
+    get_channel(plugin, payload, outgoing_node_id, outgoing_scid, True)
+    get_channel(plugin, payload, incoming_node_id, incoming_scid, True)
+    out_ours, out_total = amounts_from_scid(plugin, outgoing_scid)
+    in_ours, in_total = amounts_from_scid(plugin, incoming_scid)
     plugin.log("Outgoing node: %s, channel: %s" % (outgoing_node_id, outgoing_scid))
     plugin.log("Incoming node: %s, channel: %s" % (incoming_node_id, incoming_scid))
 
     # If amount was not given, calculate a suitable 50/50 rebalance amount
     if msatoshi is None:
-        out_ours, out_total = amounts_from_scid(plugin, outgoing_scid)
-        in_ours, in_total = amounts_from_scid(plugin, incoming_scid)
         msatoshi = calc_optimal_amount(out_ours, out_total, in_ours, in_total, payload)
         plugin.log("Estimating optimal amount %s" % msatoshi)
 
+    # Check requested amounts are selected channels
+    if msatoshi > out_ours or msatoshi > in_total - in_ours:
+        raise RpcError("rebalance", payload, {'message': 'Channel capacities too low'})
+
     route_out = {'id': outgoing_node_id, 'channel': outgoing_scid}
     route_in = {'id': my_node_id, 'channel': incoming_scid}
     start_ts = int(time.time())
     label = "Rebalance-" + str(uuid.uuid4())
     description = "%s to %s" % (outgoing_scid, incoming_scid)
-    invoice = plugin.rpc.invoice(msatoshi, label, description, int(retry_for) + 60)
+    invoice = plugin.rpc.invoice(msatoshi, label, description, retry_for + 60)
     payment_hash = invoice['payment_hash']
     plugin.log("Invoice payment_hash: %s" % payment_hash)
     success_msg = ""
     try:
         excludes = []
+        # excude all own channels to prevent unwanted shortcuts [out,mid,in]
         mychannels = plugin.rpc.listchannels(source=my_node_id)['channels']
         for channel in mychannels:
             excludes += [channel['short_channel_id'] + '/0', channel['short_channel_id'] + '/1']
-        while int(time.time()) - start_ts < int(retry_for):
+
-            r = plugin.rpc.getroute(incoming_node_id, msatoshi, riskfactor=1, cltv=9, fromid=outgoing_node_id,
+        while int(time.time()) - start_ts < retry_for:
-                                    exclude=excludes)
+            r = plugin.rpc.getroute(incoming_node_id, msatoshi, riskfactor=1, cltv=9, fromid=outgoing_node_id, exclude=excludes)
             route_mid = r['route']
             route = [route_out] + route_mid + [route_in]
             setup_routing_fees(plugin, route, msatoshi)
-            fees = route[0]['msatoshi'] - route[-1]['msatoshi']
+            fees = route[0]['amount_msat'] - msatoshi
+
+            # check fee and exclude worst channel the next time
             # NOTE: the int(msat) casts are just a workaround for outdated pylightning versions
-            if fees > exemptfee and int(fees) > int(msatoshi) * float(maxfeepercent) / 100:
+            if fees > exemptfee and int(fees) > int(msatoshi) * maxfeepercent / 100:
                 worst_channel_id = find_worst_channel(route)
                 if worst_channel_id is None:
                     raise RpcError("rebalance", payload, {'message': 'Insufficient fee'})
                 excludes += [worst_channel_id + '/0', worst_channel_id + '/1']
                 continue
-            try:
+
+            success_msg = "%d msat sent over %d hops to rebalance %d msat" % (msatoshi + fees, len(route), msatoshi)
             plugin.log("Sending %s over %d hops to rebalance %s" % (msatoshi + fees, len(route), msatoshi))
             for r in route:
-                    plugin.log("Node: %s, channel: %13s, %s" % (r['id'], r['channel'], r['msatoshi']))
+                plugin.log("    - %s  %14s  %s" % (r['id'], r['channel'], r['amount_msat']))
-                success_msg = "%d msat sent over %d hops to rebalance %d msat" % (msatoshi + fees, len(route), msatoshi)
+
+            try:
                 plugin.rpc.sendpay(route, payment_hash)
-                plugin.rpc.waitsendpay(payment_hash, int(retry_for) + start_ts - int(time.time()))
+                plugin.rpc.waitsendpay(payment_hash, retry_for + start_ts - int(time.time()))
                 return success_msg
+
             except RpcError as e:
                 plugin.log("RpcError: " + str(e))
                 erring_channel = e.error.get('data', {}).get('erring_channel')
@@ -182,10 +208,11 @@ def rebalance(plugin, outgoing_scid, incoming_scid, msatoshi: Millisatoshi=None,
                 erring_direction = e.error.get('data', {}).get('erring_direction')
                 if erring_channel is not None and erring_direction is not None:
                     excludes.append(erring_channel + '/' + str(erring_direction))
+
     except Exception as e:
         plugin.log("Exception: " + str(e))
-        return rebalance_fail(plugin, label, payload, success_msg, e)
+        return cleanup(plugin, label, payload, success_msg, e)
-    return rebalance_fail(plugin, label, payload, success_msg)
+    return cleanup(plugin, label, payload, success_msg)
 
 
 @plugin.init()

sendinvoiceless/README.md

@@ -18,12 +18,24 @@ Once the plugin is active you can send payment by running:
 lightning-cli sendinvoiceless nodeid msatoshi [maxfeepercent] [retry_for] [exemptfee]
 ```
 
-The `nodeid` is the identifier of the receiving node. The `maxfeepercent` limits
+If you want to skip/default certain optional parameters but use others, you can
-the money paid in fees and defaults to 0.5. The `maxfeepercent` is a percentage
+use always the `lightning-cli -k` (key=value) syntax like this:
-of the amount that is to be paid. The `exemptfee` option can be used for tiny
+
-payments which would be dominated by the fee leveraged by forwarding nodes.
+```bash
-Setting exemptfee allows the maxfeepercent check to be skipped on fees that are
+lightning-cli sendinvoiceless -k nodeid=022368... msatoshi=1000 retry_for=600
-smaller than exemptfee (default: 5000 millisatoshi).
+```
+
+### Parameters
+
+- The `nodeid` is the identifier of the receiving node.
+- The `msatoshi` parameter defines the millisatoshi amount to send.
+  Can be denominated in other units, i.e.: `1000000sat`, `0.01btc` or `10mbtc`.
+- The `maxfeepercent` limits the money paid in fees and defaults to 0.5.
+  The `maxfeepercent` is a percentage of the amount that is to be paid.
+- The `exemptfee` option can be used for tiny payments which would be dominated
+  by the fee leveraged by forwarding nodes. Setting exemptfee allows the
+  maxfeepercent check to be skipped on fees that are smaller than exemptfee
+  (default: 5000 millisatoshi).
 
 The command will keep finding routes and retrying the payment until it succeeds,
 or the given `retry_for` seconds pass. retry_for defaults to 60 seconds and can
@@ -42,8 +54,10 @@ lightning-cli receivedinvoiceless [min_amount]
 ```
 
 This will return an array of detected payments using this method. The plugin
-will filter the results by the optional `min_amount` parameter. Default: 10sat.
+will filter the results by the optional `min_amount` parameter (default: 10sat).
-The results will contain the `amount_msat` and `timestamp` of the payments.
+This will suppress unexpected results caused by route fee fuzzing and changed
+past channel fees. The results will contain the `amount_msat` and `timestamp`
+of the payments.
 
 NOTE: The plugin currently does not use a database, so it can only assume fees
 have not changed in the past. It will also apply default fees for already

sendinvoiceless/sendinvoiceless.py

@@ -9,13 +9,13 @@ plugin = Plugin()
 def setup_routing_fees(plugin, route, msatoshi, payload):
     delay = int(plugin.get_option('cltv-final'))
     for r in reversed(route):
-        r['msatoshi'] = r['amount_msat'] = msatoshi
+        r['msatoshi'] = msatoshi.millisatoshis
+        r['amount_msat'] = msatoshi
         r['delay'] = delay
         channels = plugin.rpc.listchannels(r['channel'])
-        for ch in channels.get('channels'):
+        ch = next(c for c in channels.get('channels') if c['destination'] == r['id'])
-            if ch['destination'] == r['id']:
         fee = Millisatoshi(ch['base_fee_millisatoshi'])
-                fee += msatoshi * ch['fee_per_millionth'] // 1000000
+        fee += msatoshi * ch['fee_per_millionth'] // 10**6
         if ch['source'] == payload['nodeid']:
             fee += payload['msatoshi']
         msatoshi += fee
@@ -36,7 +36,7 @@ def find_worst_channel(route, nodeid):
     return worst
 
 
-def sendinvoiceless_fail(plugin, label, payload, success_msg, error=None):
+def cleanup(plugin, label, payload, success_msg, error=None):
     try:
         plugin.rpc.delinvoice(label, 'unpaid')
     except RpcError as e:
@@ -49,13 +49,15 @@ def sendinvoiceless_fail(plugin, label, payload, success_msg, error=None):
 
 
 @plugin.method("sendinvoiceless")
-def sendinvoiceless(plugin, nodeid, msatoshi: Millisatoshi, maxfeepercent="0.5",
+def sendinvoiceless(plugin, nodeid, msatoshi: Millisatoshi, maxfeepercent: float=0.5,
-                    retry_for=60, exemptfee: Millisatoshi=Millisatoshi(5000)):
+                    retry_for: int=60, exemptfee: Millisatoshi=Millisatoshi(5000)):
-    """Invoiceless payment with circular routes.
+    """Send invoiceless payments with circular routes.
-
     This tool sends some msatoshis without needing to have an invoice from the receiving node.
-
     """
+    msatoshi = Millisatoshi(msatoshi)
+    maxfeepercent = float(maxfeepercent)
+    retry_for = int(retry_for)
+    exemptfee = Millisatoshi(exemptfee)
     payload = {
         "nodeid": nodeid,
         "msatoshi": msatoshi,
@@ -67,46 +69,50 @@ def sendinvoiceless(plugin, nodeid, msatoshi: Millisatoshi, maxfeepercent="0.5",
     label = "InvoicelessChange-" + str(uuid.uuid4())
     description = "Sending %s to %s" % (msatoshi, nodeid)
     change = Millisatoshi(1000)
-    invoice = plugin.rpc.invoice(change, label, description, int(retry_for) + 60)
+    invoice = plugin.rpc.invoice(change, label, description, retry_for + 60)
     payment_hash = invoice['payment_hash']
     plugin.log("Invoice payment_hash: %s" % payment_hash)
     success_msg = ""
     try:
         excludes = []
         start_ts = int(time.time())
-        while int(time.time()) - start_ts < int(retry_for):
+        while int(time.time()) - start_ts < retry_for:
             forth = plugin.rpc.getroute(nodeid, msatoshi + change, riskfactor=10, exclude=excludes)
             back = plugin.rpc.getroute(myid, change, riskfactor=10, fromid=nodeid, exclude=excludes)
             route = forth['route'] + back['route']
             setup_routing_fees(plugin, route, change, payload)
-            fees = route[0]['msatoshi'] - route[-1]['msatoshi'] - msatoshi
+            fees = route[0]['amount_msat'] - route[-1]['amount_msat'] - msatoshi
-            # Next line would be correct, but must be fixed to work around #2601 - cleanup when merged
+
-            # if fees > exemptfee and fees > msatoshi * float(maxfeepercent) / 100:
+            # check fee and exclude worst channel the next time
-            if fees > exemptfee and int(fees) > int(msatoshi) * float(maxfeepercent) / 100:
+            # NOTE: the int(msat) casts are just a workaround for outdated pylightning versions
+            if fees > exemptfee and int(fees) > int(msatoshi) * maxfeepercent / 100:
                 worst_channel = find_worst_channel(route, nodeid)
                 if worst_channel is None:
                     raise RpcError("sendinvoiceless", payload, {'message': 'Insufficient fee'})
                 excludes.append(worst_channel)
                 continue
-            try:
+
-                plugin.log("Sending %s over %d hops to deliver %s and bring back %s" %
-                           (route[0]['msatoshi'], len(route), msatoshi, change))
-                for r in route:
-                    plugin.log("Node: %s, channel: %13s, %s" % (r['id'], r['channel'], r['msatoshi']))
             success_msg = "%d msat delivered with %d msat fee over %d hops" % (msatoshi, fees, len(route))
+            plugin.log("Sending %s over %d hops to send %s and return %s" % (route[0]['msatoshi'], len(route), msatoshi, change))
+            for r in route:
+                plugin.log("    - %s  %14s  %s" % (r['id'], r['channel'], r['amount_msat']))
+
+            try:
                 plugin.rpc.sendpay(route, payment_hash)
-                plugin.rpc.waitsendpay(payment_hash, int(retry_for) + start_ts - int(time.time()))
+                plugin.rpc.waitsendpay(payment_hash, retry_for + start_ts - int(time.time()))
                 return success_msg
+
             except RpcError as e:
                 plugin.log("RpcError: " + str(e))
                 erring_channel = e.error.get('data', {}).get('erring_channel')
                 erring_direction = e.error.get('data', {}).get('erring_direction')
                 if erring_channel is not None and erring_direction is not None:
                     excludes.append(erring_channel + '/' + str(erring_direction))
+
     except Exception as e:
         plugin.log("Exception: " + str(e))
-        return sendinvoiceless_fail(plugin, label, payload, success_msg, e)
+        return cleanup(plugin, label, payload, success_msg, e)
-    return sendinvoiceless_fail(plugin, label, payload, success_msg)
+    return cleanup(plugin, label, payload, success_msg)
 
 
 @plugin.method("receivedinvoiceless")

summary/README.md

@@ -4,6 +4,11 @@ This plugin is a little hack to show a summary of your node, including
 fiat amounts.  If you have pylightning 0.0.7.1 or above, you get nice linegraphs,
 otherwise normal ASCII.
 
+## Installation
+
+For general plugin installation instructions see the repos main
+[README.md](https://github.com/lightningd/plugins/blob/master/README.md#Installation)
+
 ## Options:
 
 * --summary-currency: Currency ticker to look up on bitaverage (default: `USD`)

summary/summary.py

@@ -48,29 +48,43 @@ def to_fiatstr(msat: Millisatoshi):
     return "{}{:.2f}".format(plugin.currency_prefix,
                               int(msat) / 10**11 * plugin.fiat_per_btc)
 
-# this is included here for backwards compatibility to old pylightning version
+
-def msat_to_short_str(msat, digits: int = 3):
+# This is part of pylightning, but its just merged,
-    """
+# so old releases wont have it yet.
-    Returns the shortmost string using common units representation.
+def msat_to_approx_str(msat, digits: int = 3):
+    """Returns the shortmost string using common units representation.
+
     Rounds to significant `digits`. Default: 3
     """
-    # first round everything down 3 effective digits
     round_to_n = lambda x, n: round(x, -int(floor(log10(x))) + (n - 1))
-    amount_eff = round_to_n(msat, digits)
+    result = None
 
+    # we try to increase digits to check if we did loose out on precision
+    # without gaining a shorter string, since this is a rarely used UI
+    # function, performance is not an issue. Adds at least one iteration.
+    while True:
+        # first round everything down to effective digits
+        amount_rounded = round_to_n(msat.millisatoshis, digits)
         # try different units and take shortest resulting normalized string
-    amounts = [
+        amounts_str = [
-        "%gbtc"  % (amount_eff / 1000 / 10**8),
+            "%gbtc" % (amount_rounded / 1000 / 10**8),
-        "%gmbtc" % (amount_eff / 1000 / 10**5),
+            "%gsat" % (amount_rounded / 1000),
-        "%gµbtc" % (amount_eff / 1000 / 10**2),
+            "%gmsat" % (amount_rounded),
-        "%gsat"  % (amount_eff / 1000),
-        "%gmsat"  % (amount_eff),
         ]
-    return min(amounts, key=len)
+        test_result = min(amounts_str, key=len)
+
+        # check result and do another run if necessary
+        if test_result == result:
+            return result
+        elif not result or len(test_result) <= len(result):
+            digits = digits + 1
+            result = test_result
+        else:
+            return result
 
 # appends an output table header that explains fields and capacity
 def append_header(table, max_msat):
-    short_str = msat_to_short_str(max_msat)
+    short_str = msat_to_approx_str(Millisatoshi(max_msat))
     table.append("%c%-13sOUT/OURS %c IN/THEIRS%12s%c SCID           FLAG ALIAS"
             % (draw.left, short_str, draw.mid, short_str, draw.right))