Dynamic plugins are not passed lightningd config options?

[wtogami]

~/.lightning/conf contains:

plugin=/PATH/TO/peerswap-plugin
peerswap-elementsd-rpchost=http://127.0.0.1
peerswap-elementsd-rpcport=8049
peerswap-elementsd-rpcuser=peerswap
peerswap-elementsd-rpcpassword=REDACTED
peerswap-elementsd-rpcwallet=swap

Conf options are passed as expected only during initial startup of lightningd.

lightning-cli plugin stop peerswap-plugin
lightning-cli plugin start /PATH/TO/peerswap-plugin

You would expect the plugin to behave the same way but it is not passed the lightningd config options when started as a dynamic plugin.

My Position:

• Some have wondered why is this a big deal. "Just restart lightningd." they may be thinking. When you have 1,000+ channels it is unnecessarily disruptive to restart the entire node when you could restart just that one plugin.
• A plugin depending on configuration options passed from the lightningd config file I expect to behave identically whether started during init or later with start. Anything different from this expectation is terribly confusing and difficult to support. (New evidence today: I witnessed a well-meaning sysadmin trying to "help" a failed plugin by using start manually upon it then wondering why it was wildly misbehaving.)
• I am heartened that Rusty understood and supports this expected behavior. It just takes some re-engineering to achieve this.
• If we don't have this behavior by v0.12 then I will be forced to switch entirely away from lightningd config passed options to putting everything into the plugin's own config file. In the case of PeerSwap this isn't a big deal because the LND peerswapd already has its own config file so it would be simple to switch. It would be a shame though if we don't fix this because several other CLN plugins rely upon config options and I guarantee you those users had been confused when they don't behave the same upon a re-start.

[vincenzopalazzo]

Sorry! I miss understanding the issue!

The question is on the title of the issue and not in the body!

I never check this but it should be receiving all the information, but we have no change between lightning 0.10.2 and 0.11, are we?

What wrapper are you using? it is written in go right?

[wtogami]

https://github.com/elementsproject/glightning PeerSwap uses this fork of glightning as the Golang plugin wrapper. It has a bunch of commits on top of niftynei's May 2020 work.

You mean dynamic plugins written in other languages are able to get config options from lightningd conf?

[vincenzopalazzo]

You mean dynamic plugins written in other languages are able to get config options from lightningd conf?

I'm just asking because I don't understand why 0.10.2 works and the 0.11, no?

from the docs (https://lightning.readthedocs.io/lightning-plugin.7.html) I'm not able to see a way to pass the plugin conf, but I need to check also the code!

[wtogami]

Oops. I meant I tested it with only v0.10.2 where conf options are passed to plugins only during lightningd startup, not during a dynamic load afterwards. I didn't test 0.11 but I had guessed it's the same there. In any case this is unexpected for it to behave differently and it might be the cause of confusion. Can we change it so plugins are always told conf options?

[SimonVrouwe]

@wtogami

You would expect the plugin to behave the same way but it is not passed the lightningd config options when started as a dynamic plugin.

Yes this is currently the case, also in latest version. When lightningd starts with a (plugin registered) option given, listconfigs will show the option's value as parsed from the command line (or config). But after a plugin stop/start, listconfigs will show option with a default value, this is because killing the plugin also destroys its (parsed) value.

Could this change in a future version of lightningd?

It sounds reasonable, but I am curious what use case is there (other then for plugin development) to need to restart a plugin that registered command line options?

Anyway I hacked something together, with some assumptions:

• any option "name" can only be registered once and is only parsed during lightningd's startup
• when plugin dies, the option (name, type, value) and absolute path of associated plugin is stored
• when same plugin (re)starts it reuses the stored option if its name, type and absolute path match
• plugins cannot register the same option name (at lightningd's startup nor after)

[wtogami]

It sounds reasonable, but I am curious what use case is there (other then for plugin development) to need to restart a plugin that registered command line options?

• Plugin upgrades or reload plugin because it has its own configuration file (like thousands of ACL policies that don't belong in lightningd's config file.)
• Restarts really shouldn't require restarting lightningd

Anyway I hacked something together with some assumptions:

Why these restrictions? It sounds a desire to protect lightningd from plugins being passed sensitive info? If so that's futile because of the wide access plugins have during runtime.

It sounds like these restrictions would lead to further difficult to understand behavioral surprises where a lightning conf registered plugin would behave differently from a non-config dynamic plugin. ~~Further a config plugin upgrade where a new config option was added would also behave differently.~~(edit: I don't care about this part.)

[wtogami]

The entire reason for this ticket was to ask for lightning config plugins to behave exactly the same as plugins started later. Passing any option a plugin asks for I would think is the best way to avoid behavioral surprises when you expect it to behave the same either way.

[SimonVrouwe]

Why these restrictions?

To keep it is simple as possible and (belief it or not) to prevent behavioral surprises :)

For example pluginA registers the option name pluginConfig and start lightningd --pluginConfig=/path/to/plugin/config Then if we stop pluginA and start (dynamic) pluginB which registers the same name pluginConfig, well that's confusing.

Sharing options between plugins IMHO is also asking for trouble. So an option "name" should be unique and be associated with the plugin (identified via absolute path) that registered it.

The entire reason for this ticket was to ask for lightning config plugins to behave exactly the same as plugins started later.

Command line (config) parsing is only done at lightningd's startup, it checks argument types (string, bool or int), checks config compatibility and also generates the --help messages from option's "description" and "default" fields, but it gets these from the plugin!

Note also that plugins don't need to use lightningd's option passthrough, they can always do their own configuration file parsing. Such (internal) config will not show up in listconfigs, but a plugin can register its own method (with a unique name).

Plugin upgrades or reload plugin because it has its own configuration file

Good example of above? Such a plugin could maybe use option passthrough only for configuration file's path, but for that the plugin needs to start when lightningd starts.

[SimonVrouwe]

Well I should have read manual more carefully myself.

The start command takes a path as the first parameter and will load the plugin available from this path. Any additional parameters are passed to the plugin.

You pass options to a dynamic plugin by adding it as parameter to plugin start command, like: lightning-cli -k plugin subcommand=start plugin=/path/to/peerswap-plugin peerswap-db-path=path peerswap-policy-path=another

Would this solve your issue?

[wtogami]

Would this solve your issue?

No, that is terribly inconvenient. If that is all we had in lightningd then I'd opt to instead move all plugin options into its own config file as that would be less burdensome for users to get expected behavior.

~~Perhaps the draft changes you outlined above is a sufficient improvement in addition to passing overrides during plugin start.~~

[SimonVrouwe]

If that is all we had in lightningd then I'd opt to instead move all plugin options into its own config file

Good point, I think you are right. AFAIK plugins internal configs are not really relevant to lightningd (itself), let's keeps the core clean an simple.

Perhaps the draft changes you outlined above is a sufficient improvement

I came a long way, but the fact that parameters to the plugin start override, complicates it further. If the plugin is stopped again, should we then remember overridden options or the original (parsed at ld's startup) options?

Whatever we choose effectively becomes the (new) default, i.e. what we get when we call plugin start without a parameter. Changing defaults seems unexpected.

[SimonVrouwe]

Just to sum up my thoughts (and lessons):

• lightningd parses plugin options (given at startup or as extra parameter to plugin start) and passes them via init, but only keeps a copy to list them in listconfigs
• don't expect lightningd to remember plugin options after the plugin dies, as currently reflected in listconfigs result

Plugins can always make a copy (on disk) of options it received at startup (check the "startup" field "init" parameter) and reuse them with a plugin start. But from user perspective this seems odd, as these won't show up in listconfigs. Better pass them explicitly as parameters to plugin start, or (if that's too much typing) create an alias parameter, e.g. --conf=last or such.

There where some issues I came across during hacking (w.r.t. unique options names etc.) for which I will create a PR. And perhaps adding examples to documentation.

[rustyrussell]

This is problematic :( We parse the options straight into the plugin; if it goes away, we lose the memory of those options.

If the plugin comes back and no longer accepts those same options, it would now fail; there's no clean way to clear options short of restarting. Maybe that's an OK failure mode?

[wtogami]

Requiring -k of options to plugin start is too inconvenient. I expect the options parsed by lightningd to be passed plugins during init and again the same during a plugin start dynamic restart. Anything short of this is unexpected and confusing to the user.

If the plugin comes back and no longer accepts those same options, it would now fail

I am not concerned about this case. If the plugin's expected options change then it is fine to require restarting lightningd.

[SimonVrouwe]

This is problematic

It's not lightningd's problem, it provides option passthrough, nothing more

the fact that parameters to the plugin start override, complicates it further. If the plugin is stopped again, should we then remember overridden options or the original (parsed at ld's startup) options? Whatever we choose effectively becomes the (new) default, i.e. what we get when we call plugin start without a parameter. Changing defaults seems unexpected.

What I mean here, when a user executes a command without arguments it expects (fixed) default behavior. The idea of remembering options between runs seems flawed to me, perhaps a stupid example: when we call ls -l in a shell we also don't expect subsequent ls to have remembered the -l.

[rustyrussell]

... but commands that have a config file definitely remember those options!

It's not impossible, it just requires some re-engineering. It means properly recalling options even after the plugin has gone.

[SimonVrouwe]

The entire reason for this ticket was to ask for lightning config plugins to behave exactly the same as plugins started later.

I expect the options parsed by lightningd to be passed plugins during init and again the same during a plugin start dynamic restart. Anything short of this is unexpected and confusing to the user.

@wtogami If you really insist on this special behavior, you can engineer it in the plugin. Below an example how to do it with a plugin based on pyln-client

#!/usr/bin/env python3
from pyln.client import Plugin
import json

plugin = Plugin()


@plugin.init()
def init(options, configuration, plugin):
    # Save options parsed during lightningd's startup
    if configuration['startup']:
        with open('pluginxyz_startup.conf', 'w') as f:
            json.dump(plugin.options, f)
    else:
        with open('pluginxyz_startup.conf', 'r') as f:
            startup_conf = json.load(f)

        plugin.log('reusing startup options {}'.format(startup_conf))
        for k, v in startup_conf.items():
            plugin.options[k] = v


plugin.add_option('option_one', 'default_one', 'description')
plugin.add_option('option_two', 42, 'description', opt_type='int')
plugin.run()

This works with current version, only reused values will not appear in the listconfigs (it will show default values). Maybe that can be fixed, as proposed in #5343

[wtogami]

My Position:

• Some have wondered why is this a big deal. "Just restart lightningd." they may be thinking. When you have 1,000+ channels it is unnecessarily disruptive to restart the entire node when you could restart just that one plugin.
• A plugin depending on configuration options passed from the lightningd config file I expect to behave identically whether started during init or later with start. Anything different from this expectation is terribly confusing and difficult to support. (New evidence today: I witnessed a well-meaning sysadmin trying to "help" a failed plugin by using start manually upon it then wondering why it was wildly misbehaving.)
• I am heartened that Rusty understood and supports this expected behavior. It just takes some re-engineering to achieve this.
• If we don't have this behavior by v0.12 then I will be forced to switch entirely away from lightningd config passed options to putting everything into the plugin's own config file. In the case of PeerSwap this isn't a big deal because the LND peerswapd already has its own config file so it would be simple to switch. It would be a shame though if we don't fix this because several other CLN plugins rely upon config options and I guarantee you those users had been confused when they don't behave the same upon a re-start.

[zerofeerouting]

I agree with Warren here.

If I were to stop and restart a plugin that has options configured, I'd expect it to start with these options. Deviating from this behaviour could have grave consequences for the node operator (depending on the plugin's functionality and the options).

[rustyrussell]

This isn't going to get fixed for 0.12, sorry.

I recommend you manually parse $HOME/.lightning/config and $HOME/.lightning//config if they exist, and parse any lines starting with peerswap-.

Use those results as the defaults before overriding with any the init method hands you, so you will properly overwrite if they specify parameters manually.

Then document the limitation for 0.12: that peerswap parameters on restart will only be found if they're directly in one of those two config files, not in any included files. That will be fixed in 0.13.

[wtogami]

I'm glad that you agree with the intent. Thank you!