[plugin] Plugin that depicts the off- and onchain funds in a nicer overview #2139

renepickhardt · 2018-12-05T16:14:30Z

the plugins directory doesn't seem to work yet. I had to call the plugin explicitly with
lightningd --plugin=/Users/rpickhardt/hacken/lightning/plugins/funds.py not sure though weather this plugin should be a standard plugin anyway.

you can test the plugin with:
lightning-cli funds and the output should be similar to this:
{"total_sat": 200000, "onchain_sat": 100000, "offchain_sat": 100000}
if you call the plugin with the m argument the ammounts are depicted in millibitoins (other units are available via help) eg:

lightning-cli funds m
{"total_mBTC": 2, "onchain_mBTC": 1, "offchain_mBTC": 1}

I will probably use this plugin as a tutorial for next Mondays youtube video, so that people can learn how easy it is to create plugins in c-lightning (chapeau for @cdecker )

I have prepared a couple more plugins (for example for channel balance overview, an invoice server for donations, maybe even the spontaneous payments one) but I would like to have feedback on this "quicky" first instead of pushing several plugins.

Once the easy ones are there I will transfer my autopilot code to the plugin system.

I have also some feedback where the current system could be improved to ease up the life of developers. Should I do this as a pull request?

Also is there a better way to talk to the RPC interface (for example directly via stdout) so that the plugin does not need to know where the rpc-interface is located?

renepickhardt · 2018-12-05T16:16:51Z

plugins/funds.py

+from lightning.lightning import LightningRpc
+from os.path import expanduser
+
+path = expanduser("~/.lightning/lightning-rpc")


I was talking explicitly about this line. My feeling is that I should be able to send RPC calls via json directly to the lightningdaemon. However I don't know the format of the json and it is not documented yet.

#2131 will contain a configuration field in the configure call that'll explicitly tell you were to connect 😉

#2131 didn't? I have a patch which adds a 'rpc_filename' to the init msg however, will push (I needed this too, of course)!

So I will be able to find this in def json_init() in the options argument?

How can I look at these variables? Since stdout is sent to lightningd I can't print variables to inspect them which is annoying

Wait, where did that patch end up?

Oh, I stashed it... I'll reapply it

cdecker · 2018-12-05T18:11:46Z

plugins/funds.py

+
+
+def json_get_funds(request, unit="s"):
+    if unit.lower() == "bitcoin" or unit.lower() == "btc":


How about creating a dict that maps the long form to the short form? If you do the reverse step, you can also create the inverse map with a simple dict comprehension :-)

Yeah dict will be more python style

cdecker · 2018-12-05T18:21:35Z

plugins/funds.py

+m, milli, btc to depict milliBitcoin
+B, bitcoin, btc to depict Bitcoins
+
+When not using Satshis (default) the comma values are rounded off."""


Typo: Satshis -> Satoshis

No millisatoshi option?

conscott · 2018-12-06T11:40:21Z

plugins/funds.py

+    outputs = funds["outputs"]
+    onchain_value = 0
+    for output in outputs:
+        onchain_value += int(output["value"])


Nothing wrong with this code, but fyi, you can also use list comprehensions to shorten this down a bit to something like:

onchain_value = sum([int(output['value']) for output in funds['outputs']]) offchain_value = sum([int(channel["channel_sat"]) for channel in funds['channels']])

yeah I will do that since I have to touch the code once more anyway (:

conscott · 2018-12-06T12:07:26Z

Cool! There should be lots of nice simple plugins that can be added, and it's nice to have a reference for a 'standard' plugin

Would also be nice to add unconfirmed balance later if #2104 is implemented, but I understand this may just be a sample.

As a general question to @cdecker and @rustyrussell .... Is there a general idea or documentation in mind for some 'standard' plugins to be released with c-lightning?

Something like closedchannels (close all channels) command in lnd seems better implemented as a simple plugin command in c-lightning rather than base rpc, just based on how cmd life-cycles are implemented. Just didn't know if there was any guidance on what would or would not be considered for inclusion into a standard set.

Sorry to hijack the comments @renepickhardt - Looking forward to seeing the autopilot revived :)

renepickhardt · 2018-12-06T16:39:40Z

I rebased the master and have made the requested changes and also merged #2141 in this branch as this plugin relies on the fact that the deamon is giving us the location of the RPC interface in the init message. I thought this is the best way to do so otherwise we would have to touch this plugin again once #2141 is merged

rustyrussell · 2018-12-07T00:24:59Z

Cool! There should be lots of nice simple plugins that can be added, and it's nice to have a reference for a 'standard' plugin

Would also be nice to add unconfirmed balance later if #2104 is implemented, but I understand this may just be a sample.

As a general question to @cdecker and @rustyrussell .... Is there a general idea or documentation in mind for some 'standard' plugins to be released with c-lightning?

Something like closedchannels (close all channels) command in lnd seems better implemented as a simple plugin command in c-lightning rather than base rpc, just based on how cmd life-cycles are implemented. Just didn't know if there was any guidance on what would or would not be considered for inclusion into a standard set.

Sorry to hijack the comments @renepickhardt - Looking forward to seeing the autopilot revived :)

Well, pay is moving to a "standard" plugin, since there is a lot of smarts we want to eventually add there. The initial version I have is fairly primitive, though. That will be installed by default; the rest will be in contrib/ until/if they graduate by common consensus, I think.

renepickhardt · 2018-12-07T17:56:39Z

@cdecker @rustyrussell I have force pushed the code once more to have the plugin in the required location and I have also taken back my early merge of #2141 . I guess this plugin should now be ready to be merged (: On the way I have created a more complex (in the sense of more third party technologies that have been used) plugin over at #2152 .

Once these two are merged I guess I will be able to transfer the code of #1888 to the plugin architecture. Should the autopilot be placed in /plugins or in /contrib/plugins/autopilot ? My understanding is that the autopilot is a rather standard / close feature (in particular it should help users who are less technical so activating it as a sperate plugin seems unncessary burden)

renepickhardt · 2019-01-02T10:44:18Z

I have yet updated the plugin to the new python bindings which makes use of the decorators so that there is no more boilerplate code left in the plugin code.

If everything is fine now and the plugin can be merged I would love to create the educational video about the plugin system soon (:

cdecker

Some minor comments, and improvements.

As discussed on IRC, I'm considering to add a new repository that serves as a nursery for these tiny plugins, and only bundle a few very select plugins into the c-lightning distribution by default.

contrib/plugins/funds/funds.py

cdecker · 2019-01-02T11:11:24Z

contrib/plugins/funds/funds.py

+    plugin.log("Funds Plugin successfully initialezed")
+
+
+plugin.add_option('funds', 'funds',


This option doesn't really seem to do anything. From the description I take it that it's supposed to be the @plugin.method call above.

What you could do instead is provide the default unit here, so that on startup we can chose whether we'd like to display in satoshis, bits, mBTC or BTC by default.

Another option that would make sense is the number of decimal places to show.

Maybe I did not understand the add_option() call correctly. I had the feeling this was similar to the entry in the get_manifest that I had to be used previously. I guess I got that wrong. Will take a closer look at the implementation tomorrow. default display is satoshis btw

add_option will add command line options to lightningd when starting the daemon and the option values will be passed to the plugin on init. So this declaration would result in a command line option funds with default value funds for the lightningd. So this would be perfectly fine:

lightningd --funds=bla

But then you don't extract and store the value of that option, so it's not being used at all 😉

Damn it I already knew that. But with the frequent api changes I oversaw that. Well certainly fix that (:

also I am just happy that I made this mistake. I will try to change the code so that I will add an argument ---fundsunit=s to lightningd which would set the standard value for the command line tool. this is also good for educational purpose to see the difference and both methods

contrib/plugins/funds/funds.py

renepickhardt · 2019-01-03T00:17:27Z

Will fix those changes. I have uploaded the current version (before I saw your feedback) and with regard to the conversation in the irc to https://github.com/renepickhardt/c-lightning-plugin-collection (where I would also publish the donation plugin and the autopilot)
(though I would recommend to put the autopilot to the default plugin dir of plugins which are loaded automatically)

cdecker · 2019-01-03T09:16:32Z

(though I would recommend to put the autopilot to the default plugin dir of plugins which are loaded automatically)

Absolutely, my plan is to externally curate a list somewhere and pull plugins in as they grow in popularity. Maybe some sort of plugin manager might be in order. But I'll definitely bundle things like the autopilot at least during release.

…dard unit via an argument of lightningd

renepickhardt · 2019-01-03T13:49:06Z

I have changed the requested changes (besides replacing // with round) and extended the API of lightningd to be able to set a default unit.

As a side effect I have opened #2217 which still seems like a pitty to me.

conscott · 2019-01-15T14:09:35Z

The following has just occurred in playing with another balance plugin:

listfunds output will show channels in ONCHAIN state (closed) in the channels list (I guess until the channel is forgotten) while ALSO showing the closing tx transaction in the outputs list.

For this reason, I think the plugin will 'count' a closed channel into the "offchain" (channel) balance, as well as the "onchain" balance at the same time. The correct behavior would be to not count a closed channel as part of the channel balance.

If calculating from listpeers you can know the channel state and can avoid this condition, but it also brings up the question...

Should listfunds even list closed channels, given that the closing txid is already listed in the outputs field? @cdecker

If it's still desired to list it, perhaps it makes sense to add the channel state so people have to the context to know its closed / closing.

conscott · 2019-01-17T06:57:08Z

btw - working on a 'balance of every possible thing' plugin here if anyone reading is interested.

https://github.com/conscott/c-lightning-plugins/blob/master/balance.py

cdecker · 2019-05-10T10:24:26Z

Since we now have quite a few plugins that will address this I think I'll close this 😉

renepickhardt requested a review from cdecker as a code owner December 5, 2018 16:14

renepickhardt commented Dec 5, 2018

View reviewed changes

cdecker reviewed Dec 5, 2018

View reviewed changes

conscott reviewed Dec 6, 2018

View reviewed changes

renepickhardt force-pushed the funds-plugin branch from 1d915c1 to 2f19975 Compare December 6, 2018 16:36

renepickhardt mentioned this pull request Dec 6, 2018

plugin: Added .params.configuration to init call #2141

Merged

renepickhardt force-pushed the funds-plugin branch from 2f19975 to d14baa4 Compare December 7, 2018 17:40

renepickhardt mentioned this pull request Dec 7, 2018

[plugin] 0.0.1 of donation plugin and invoice query service #2152

Closed

renepickhardt changed the title ~~Plugin that depicts the off- and onchain funds in a nicer overview~~ [plugin] Plugin that depicts the off- and onchain funds in a nicer overview Dec 7, 2018

cdecker added the plugin label Dec 9, 2018

renepickhardt mentioned this pull request Dec 11, 2018

incompatible with plugins shesek/spark-wallet#43

Closed

renepickhardt added 2 commits January 2, 2019 11:34

removed conflicts with PR 2141 and moved plugin to contrib folder

110c00b

updated to new plugin module that ships with c-lightning

681c40d

renepickhardt force-pushed the funds-plugin branch from d14baa4 to 681c40d Compare January 2, 2019 10:39

cdecker requested changes Jan 2, 2019

View reviewed changes

renepickhardt mentioned this pull request Jan 3, 2019

python plugin lib removes line breaks from doc string for help function #2216

Closed

included feedback from @cdecker and included a way to change the stan…

43cb5b0

…dard unit via an argument of lightningd

cdecker closed this May 10, 2019



		def json_get_funds(request, unit="s"):
		if unit.lower() == "bitcoin" or unit.lower() == "btc":

		plugin.log("Funds Plugin successfully initialezed")


		plugin.add_option('funds', 'funds',

[plugin] Plugin that depicts the off- and onchain funds in a nicer overview #2139

[plugin] Plugin that depicts the off- and onchain funds in a nicer overview #2139

Uh oh!

Conversation

renepickhardt commented Dec 5, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

conscott commented Dec 6, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

renepickhardt commented Dec 6, 2018

Uh oh!

rustyrussell commented Dec 7, 2018

Uh oh!

renepickhardt commented Dec 7, 2018

Uh oh!

renepickhardt commented Jan 2, 2019

Uh oh!

cdecker left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

renepickhardt Jan 3, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

renepickhardt commented Jan 3, 2019

Uh oh!

cdecker commented Jan 3, 2019

Uh oh!

renepickhardt commented Jan 3, 2019

Uh oh!

conscott commented Jan 15, 2019

Uh oh!

conscott commented Jan 17, 2019

Uh oh!

cdecker commented May 10, 2019

Uh oh!

Reviewers

Assignees

conscott commented Dec 6, 2018 •

edited

Loading

renepickhardt Jan 3, 2019 •

edited

Loading