Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cln-plugin: Required ConfigOption with no default #5274

Closed
justinmoon opened this issue May 19, 2022 · 4 comments
Closed

cln-plugin: Required ConfigOption with no default #5274

justinmoon opened this issue May 19, 2022 · 4 comments

Comments

@justinmoon
Copy link
Contributor

justinmoon commented May 19, 2022

I need to write a c-lightning plugin that takes a command line argument to a config file. This config is required -- the plugin cannot work without it -- and at this point there isn't a default location for it.

Right now I just set a default value to "default-do-not-use" or something like that and panic if that's still the value after initialization. But it might be nice if the library supported this kind of required argument without a default.

(Or maybe I just need to figure out a default location for my plugin config!)

@vincenzopalazzo
Copy link
Collaborator

This is exactly how the grpc plugin works, if the option is missed, you can disabled the plugin

let bind_port = match plugin.option("grpc-port") {
Some(options::Value::Integer(-1)) => {
log::info!("`grpc-port` option is not configured, exiting.");
plugin
.disable("`grpc-port` option is not configured.")
.await?;
return Ok(());
}
Some(options::Value::Integer(i)) => i,
None => return Err(anyhow!("Missing 'grpc-port' option")),
Some(o) => return Err(anyhow!("grpc-port is not a valid integer: {:?}", o)),
};
let plugin = plugin.start().await?;
let bind_addr: SocketAddr = format!("0.0.0.0:{}", bind_port).parse().unwrap();
tokio::spawn(async move {
if let Err(e) = run_interface(bind_addr, state).await {
warn!("Error running the grpc interface: {}", e);
}
});

But this can be an idea to have not a library level but a lightningd level, that can avoid running the plugin if there is not enough info to run it! This make also the plugin lifecycle easier!

@ZmnSCPxj
Copy link
Collaborator

ZmnSCPxj commented May 19, 2022

But this can be an idea to have not a library level but a lightningd level, that can avoid running the plugin if there is not enough info to run it! This make also the plugin lifecycle easier!

If we want plugins to be self-contained, at the minimum lightningd has to run getmanifest on it so that the plugin can tell it "yeah if this option does not exist just exit me immediately mkay".

Now, the general directive is that all your heavy initialization should occur on the init command, so the initial getmanifest should really just return an effectively hardcoded constant, and then if your plugin specifies "if this options does not exist just terminate me early" then the lightningd can simply close the pipe to the plugin and let it terminate. You still need to manage some lifecycle, but at least the plugin can be outright unloaded from memory and its resources freed, not blocked waiting on an open pipe that will never send it data.

@vincenzopalazzo
Copy link
Collaborator

Oh you have right @ZmnSCPxj I think the actual solution is the more convenient one

@justinmoon
Copy link
Contributor Author

Closed by #5369

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants