diff --git a/Boss/Mod/AmountSettingsHandler.cpp b/Boss/Mod/AmountSettingsHandler.cpp index c7eaae9..23107fa 100644 --- a/Boss/Mod/AmountSettingsHandler.cpp +++ b/Boss/Mod/AmountSettingsHandler.cpp @@ -78,7 +78,26 @@ private: bus.subscribe([this](Msg::Option o) { - assert(settings); + /* Msg::Option was originally a startup-only signal + * (delivered once per registered option between + * Manifestation and EndOfOptions), and EndOfOptions + * moves `settings` away on line below. Now that + * SetConfigHandler can re-raise Msg::Option at + * runtime to deliver setconfig updates, this handler + * may receive events for unrelated names long after + * `settings` has been moved -- e.g. a runtime + * `setconfig` on any dynamic option triggers a + * Msg::Option that hits every subscriber. + * + * Drop those silently: none of the options this + * handler owns is registered dynamic, so by the time + * we see a post-EndOfOptions event it cannot + * legitimately apply. This also covers a latent + * assertion crash that pre-existed dynamic options + * (any post-EOO Msg::Option for any name would have + * tripped the old assert(settings) here). */ + if (!settings) + return Ev::lift(); auto const& name = o.name; if (name == "clboss-min-onchain") { settings->reserve = parse_sats(o.value); diff --git a/Boss/Mod/Manifester.cpp b/Boss/Mod/Manifester.cpp index 1a3c53d..088d333 100644 --- a/Boss/Mod/Manifester.cpp +++ b/Boss/Mod/Manifester.cpp @@ -90,6 +90,9 @@ void Manifester::start() { .field( "description" , o.description ) + .field( "dynamic" + , o.dynamic + ) .end_object() ); } diff --git a/Boss/Mod/SetConfigHandler.cpp b/Boss/Mod/SetConfigHandler.cpp new file mode 100644 index 0000000..5cc5ec6 --- /dev/null +++ b/Boss/Mod/SetConfigHandler.cpp @@ -0,0 +1,100 @@ +#include"Boss/Mod/SetConfigHandler.hpp" +#include"Boss/Msg/CommandFail.hpp" +#include"Boss/Msg/CommandRequest.hpp" +#include"Boss/Msg/CommandResponse.hpp" +#include"Boss/Msg/ManifestOption.hpp" +#include"Boss/Msg/Option.hpp" +#include"Boss/log.hpp" +#include"Ev/Io.hpp" +#include"Jsmn/Object.hpp" +#include"Json/Out.hpp" +#include"S/Bus.hpp" + +namespace { + +/* JSON-RPC error code for malformed parameters, matching the + * JSONRPC2 invalid-params constant used elsewhere in clboss. */ +constexpr int RPC_INVALID_PARAMS = -32602; + +} + +namespace Boss { namespace Mod { + +void SetConfigHandler::start() { + bus.subscribe([this](Boss::Msg::ManifestOption const& o) { + options[o.name] = o.dynamic; + return Ev::lift(); + }); + bus.subscribe([this](Boss::Msg::CommandRequest const& m) { + if (m.command != "setconfig") + return Ev::lift(); + + auto id = m.id; + auto const& params = m.params; + + /* Extract `config` (required string). */ + if (!params.is_object() || !params.has("config") + || !params["config"].is_string()) { + return bus.raise(Boss::Msg::CommandFail{ + id, RPC_INVALID_PARAMS, + "setconfig: missing or non-string 'config' " + "parameter", + Json::Out::empty_object() + }); + } + auto name = std::string(params["config"]); + + /* Verify the option is one we registered, and is + * declared dynamic. Lightningd should never forward + * setconfig for a non-dynamic option (libplugin would + * refuse it on the receiving side too), but defending + * here keeps the error surface clear and prevents a + * surprise Msg::Option re-raise for an option whose + * handler may not expect runtime updates. */ + auto it = options.find(name); + if (it == options.end()) { + return bus.raise(Boss::Msg::CommandFail{ + id, RPC_INVALID_PARAMS, + "setconfig: unknown option '" + name + "'", + Json::Out::empty_object() + }); + } + if (!it->second) { + return bus.raise(Boss::Msg::CommandFail{ + id, RPC_INVALID_PARAMS, + "setconfig: option '" + name + + "' is not dynamic", + Json::Out::empty_object() + }); + } + + /* Forward the value as-is. Lightningd encodes the new + * value as a JSON string (see plugin_set_dynamic_opt in + * cln/lightningd/plugin.c), so handlers will see a + * Jsmn::Object with is_string() == true here even for + * numeric option types. The contract documented on + * SetConfigHandler covers this. + * + * Note: bus.raise(Msg::Option) broadcasts to every + * Msg::Option subscriber, not just the one that owns + * this option. Subscribers must filter by name and + * no-op on non-matches -- see the doc comment on + * Boss::Msg::Option for the full contract. */ + auto value = params.has("val") + ? params["val"] + : Jsmn::Object(); + return Boss::log( bus, Debug + , "SetConfigHandler: dispatching setconfig " + "'%s'" + , name.c_str() + ) + + bus.raise(Boss::Msg::Option{name, std::move(value)}) + + bus.raise(Boss::Msg::CommandResponse{ + id, Json::Out::empty_object() + }); + }); +} + +}} diff --git a/Boss/Mod/SetConfigHandler.hpp b/Boss/Mod/SetConfigHandler.hpp new file mode 100644 index 0000000..51def94 --- /dev/null +++ b/Boss/Mod/SetConfigHandler.hpp @@ -0,0 +1,53 @@ +#ifndef BOSS_MOD_SETCONFIGHANDLER_HPP +#define BOSS_MOD_SETCONFIGHANDLER_HPP + +#include +#include + +namespace S { class Bus; } + +namespace Boss { namespace Mod { + +/** class Boss::Mod::SetConfigHandler + * + * @brief Dispatches `setconfig` JSON-RPC calls from lightningd + * for options that were registered with `dynamic = true` on their + * Msg::ManifestOption. + * + * Lightningd routes `setconfig ` to the plugin that + * owns the option, as a JSON-RPC method call. We turn that into + * a fresh Msg::Option on the bus, so existing option handlers + * re-apply the new value without a plugin restart. + * + * Contract for module authors who mark an option `dynamic = true`: + * at startup lightningd delivers Int / Bool / Flag option values + * as JSON primitives, but at setconfig time lightningd encodes the + * value as a JSON string. Any module that opts in to dynamic + * updates MUST tolerate both shapes in its Msg::Option handler -- + * inspect `o.value.is_string()` and parse from the string form + * when appropriate. + */ +class SetConfigHandler { +private: + S::Bus& bus; + /* Name -> dynamic flag, populated from Msg::ManifestOption + * events during the Manifestation phase. Non-dynamic + * options are recorded too so we can return a clearer error + * than "unknown option" if lightningd ever forwards a + * setconfig for a non-dynamic name (which it should not). */ + std::map options; + + void start(); + +public: + SetConfigHandler() =delete; + SetConfigHandler(SetConfigHandler&&) =delete; + SetConfigHandler(SetConfigHandler const&) =delete; + + explicit + SetConfigHandler(S::Bus& bus_) : bus(bus_) { start(); } +}; + +}} + +#endif /* !defined(BOSS_MOD_SETCONFIGHANDLER_HPP) */ diff --git a/Boss/Mod/all.cpp b/Boss/Mod/all.cpp index 9674265..5cb777c 100644 --- a/Boss/Mod/all.cpp +++ b/Boss/Mod/all.cpp @@ -66,6 +66,7 @@ #include"Boss/Mod/RpcWrapper.hpp" #include"Boss/Mod/SelfUptimeMonitor.hpp" #include"Boss/Mod/SendpayResultMonitor.hpp" +#include"Boss/Mod/SetConfigHandler.hpp" #include"Boss/Mod/StatusCommand.hpp" #include"Boss/Mod/SwapManager.hpp" #include"Boss/Mod/SwapReporter.hpp" @@ -119,6 +120,7 @@ std::shared_ptr all( std::ostream& cout /* Startup. */ all->install(bus); all->install(bus, threadpool, std::move(open_rpc_socket)); + all->install(bus); /* General settings. */ all->install(bus); diff --git a/Boss/Msg/ManifestOption.hpp b/Boss/Msg/ManifestOption.hpp index 222e8e4..4883c58 100644 --- a/Boss/Msg/ManifestOption.hpp +++ b/Boss/Msg/ManifestOption.hpp @@ -17,6 +17,14 @@ struct ManifestOption { OptionType type; Json::Out default_value; std::string description; + /* If true, lightningd will accept `setconfig ` at + * runtime for this option and forward the new value to clboss + * via the `setconfig` JSON-RPC method. SetConfigHandler turns + * that into a fresh Msg::Option on the bus, so existing option + * handlers re-apply the new value without a plugin restart. + * Default false preserves the original startup-only contract. + */ + bool dynamic = false; }; }} diff --git a/Boss/Msg/Option.hpp b/Boss/Msg/Option.hpp index 8b4d946..6ab22b9 100644 --- a/Boss/Msg/Option.hpp +++ b/Boss/Msg/Option.hpp @@ -8,8 +8,21 @@ namespace Boss { namespace Msg { /** struct Boss::Msg::Option * - * @brief emitted during `init` handling, providing the value - * of an option that we registered. + * @brief providing the value of an option we registered. + * + * Emitted during `init` handling (one Msg::Option per option + * lightningd actually carried in the init request), AND re- + * emitted by Boss::Mod::SetConfigHandler at runtime when + * lightningd forwards a `setconfig` JSON-RPC call for an option + * we registered as `dynamic = true`. + * + * Subscribers MUST filter by `name` and no-op for names they do + * not own (the bus broadcasts to all Msg::Option subscribers, so + * a dynamic option update for module A will be delivered to + * module B as well). Subscribers MUST also tolerate post-init + * arrival -- any local invariants that were valid only "between + * Manifestation and EndOfOptions" must be re-checked rather than + * asserted. */ struct Option { std::string name; diff --git a/Makefile.am b/Makefile.am index 292a793..8b5c05a 100644 --- a/Makefile.am +++ b/Makefile.am @@ -258,6 +258,8 @@ libclboss_la_SOURCES = \ Boss/Mod/SelfUptimeMonitor.hpp \ Boss/Mod/SendpayResultMonitor.cpp \ Boss/Mod/SendpayResultMonitor.hpp \ + Boss/Mod/SetConfigHandler.cpp \ + Boss/Mod/SetConfigHandler.hpp \ Boss/Mod/StatusCommand.cpp \ Boss/Mod/StatusCommand.hpp \ Boss/Mod/SwapManager.cpp \