diff --git a/Boss/Mod/AskreneUpdates.cpp b/Boss/Mod/AskreneUpdates.cpp index 9c16f8c..39d0ea8 100644 --- a/Boss/Mod/AskreneUpdates.cpp +++ b/Boss/Mod/AskreneUpdates.cpp @@ -205,6 +205,8 @@ private: } else if (o.value.is_string()) { secs = std::stoll(std::string(o.value)); } else { + o.reject( std::string(name) + + ": unsupported value type"); return Boss::log( bus, Warn , "AskreneUpdates: %s: " "unsupported value type; " @@ -213,18 +215,22 @@ private: ); } } catch (std::exception const& e) { + o.reject( std::string(name) + + ": not a valid number"); return Boss::log( bus, Warn , "AskreneUpdates: %s: parse error " "'%s'; keeping %" PRIu64 "." , name, e.what(), target ); } - if (secs <= 0) + if (secs <= 0) { + o.reject(std::string(name) + ": must be > 0"); return Boss::log( bus, Warn , "AskreneUpdates: %s: must be > 0; " "keeping %" PRIu64 "." , name, target ); + } target = std::uint64_t(secs); return Boss::log( bus, Info , "AskreneUpdates: %s = %" PRIu64 " seconds." diff --git a/Boss/Mod/FundsMover/Main.cpp b/Boss/Mod/FundsMover/Main.cpp index 4ce41fc..afe10f2 100644 --- a/Boss/Mod/FundsMover/Main.cpp +++ b/Boss/Mod/FundsMover/Main.cpp @@ -177,6 +177,9 @@ private: } else if (o.value.is_string()) { secs = std::stoll(std::string(o.value)); } else { + o.reject("clboss-classic-layer-age-" + "secs: unsupported value " + "type"); return Boss::log( bus, Warn , "FundsMover: clboss-" "classic-layer-age-secs: " @@ -186,6 +189,8 @@ private: ); } } catch (std::exception const& e) { + o.reject("clboss-classic-layer-age-secs: " + "not a valid number"); return Boss::log( bus, Warn , "FundsMover: clboss-classic-" "layer-age-secs: parse error " @@ -195,6 +200,8 @@ private: ); } if (secs <= 0) { + o.reject("clboss-classic-layer-age-secs: " + "must be > 0"); return Boss::log( bus, Warn , "FundsMover: clboss-classic-" "layer-age-secs: must be > 0; " @@ -230,6 +237,8 @@ private: } else if (o.value.is_string()) { ppm = std::stoll(std::string(o.value)); } else { + o.reject("clboss-min-rebalance-ppm: " + "unsupported value type"); return Boss::log( bus, Warn , "FundsMover: clboss-min-" "rebalance-ppm: unsupported " @@ -239,6 +248,8 @@ private: ); } } catch (std::exception const& e) { + o.reject("clboss-min-rebalance-ppm: " + "not a valid number"); return Boss::log( bus, Warn , "FundsMover: clboss-min-rebalance-" "ppm: parse error '%s'; keeping %" @@ -248,6 +259,8 @@ private: ); } if (ppm < 0) { + o.reject("clboss-min-rebalance-ppm: " + "must be >= 0"); return Boss::log( bus, Warn , "FundsMover: clboss-min-rebalance-" "ppm: must be >= 0; keeping %" @@ -277,6 +290,9 @@ private: } else if (o.value.is_string()) { ppm = std::stoll(std::string(o.value)); } else { + o.reject("clboss-min-rebalance-prob-" + "ppm: unsupported value " + "type"); return Boss::log( bus, Warn , "FundsMover: clboss-min-" "rebalance-prob-ppm: " @@ -286,6 +302,8 @@ private: ); } } catch (std::exception const& e) { + o.reject("clboss-min-rebalance-prob-ppm: " + "not a valid number"); return Boss::log( bus, Warn , "FundsMover: clboss-min-rebalance-" "prob-ppm: parse error '%s'; " @@ -295,6 +313,8 @@ private: ); } if (ppm < 0) { + o.reject("clboss-min-rebalance-prob-ppm: " + "must be >= 0"); return Boss::log( bus, Warn , "FundsMover: clboss-min-rebalance-" "prob-ppm: must be >= 0; keeping %" diff --git a/Boss/Mod/RebalanceModeManager.cpp b/Boss/Mod/RebalanceModeManager.cpp index 7bf3ead..3fce165 100644 --- a/Boss/Mod/RebalanceModeManager.cpp +++ b/Boss/Mod/RebalanceModeManager.cpp @@ -58,7 +58,15 @@ private: return Ev::lift(); auto s = std::string(o.value); auto m = RebalanceMode(); - if (!rebalance_mode_from_string(s, m)) + if (!rebalance_mode_from_string(s, m)) { + /* No double quotes in the message: + * lightningd forwards plugin setconfig + * errors as the raw JSON-escaped token + * (plugin_setconfig_done uses the wire + * bytes verbatim), so embedded quotes + * reach the user doubled-escaped. */ + o.reject( option_name + ": unrecognized " + "value '" + s + "'"); return Boss::log( bus, Error , "RebalanceModeManager: " "ignoring unrecognized " @@ -68,6 +76,7 @@ private: , s.c_str() , rebalance_mode_to_string(mode) ); + } if (m == mode) return Ev::lift(); mode = m; diff --git a/Boss/Mod/SetConfigHandler.cpp b/Boss/Mod/SetConfigHandler.cpp index 5cc5ec6..b69da8d 100644 --- a/Boss/Mod/SetConfigHandler.cpp +++ b/Boss/Mod/SetConfigHandler.cpp @@ -9,6 +9,7 @@ #include"Jsmn/Object.hpp" #include"Json/Out.hpp" #include"S/Bus.hpp" +#include namespace { @@ -85,15 +86,36 @@ void SetConfigHandler::start() { auto value = params.has("val") ? params["val"] : Jsmn::Object(); + /* Rejection back-channel: bus.raise() completes only + * after every subscriber has run, so the owner's verdict + * is in reject_reason by the time we acknowledge. + * Failing the command matters beyond cosmetics: + * lightningd persists the new value (configvar_save) + * only on a success response, so a blanket ack would + * record values clboss never applied -- and a + * non-numeric value persisted for an int-typed option + * even fails lightningd's own option parse on the next + * start. */ + auto reject_reason = std::make_shared(); 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{ + + bus.raise(Boss::Msg::Option{ + name, std::move(value), reject_reason + }) + + Ev::lift().then([this, id, reject_reason]() { + if (!reject_reason->empty()) + return bus.raise(Boss::Msg::CommandFail{ + id, RPC_INVALID_PARAMS, + "setconfig: " + *reject_reason, + Json::Out::empty_object() + }); + return bus.raise(Boss::Msg::CommandResponse{ id, Json::Out::empty_object() - }); + }); + }); }); } diff --git a/Boss/Mod/SetConfigHandler.hpp b/Boss/Mod/SetConfigHandler.hpp index 51def94..840b97b 100644 --- a/Boss/Mod/SetConfigHandler.hpp +++ b/Boss/Mod/SetConfigHandler.hpp @@ -26,6 +26,14 @@ namespace Boss { namespace Mod { * updates MUST tolerate both shapes in its Msg::Option handler -- * inspect `o.value.is_string()` and parse from the string form * when appropriate. + * + * A handler that REJECTS a dynamic value keeps its current setting + * and MUST also report the rejection via Msg::Option::reject(), so + * the setconfig command fails and lightningd does not persist the + * value. A silently-acked rejected value lands in config.setconfig + * and diverges from the running configuration; a non-numeric value + * persisted for an int-typed option even fails lightningd's own + * option parse on the next start. */ class SetConfigHandler { private: diff --git a/Boss/Msg/Option.hpp b/Boss/Msg/Option.hpp index 6ab22b9..a9bbef1 100644 --- a/Boss/Msg/Option.hpp +++ b/Boss/Msg/Option.hpp @@ -2,6 +2,7 @@ #define BOSS_MSG_OPTION_HPP #include"Jsmn/Object.hpp" +#include #include namespace Boss { namespace Msg { @@ -27,6 +28,29 @@ namespace Boss { namespace Msg { struct Option { std::string name; Jsmn::Object value; + /* Rejection back-channel, non-null only on the setconfig path + * (Boss::Mod::SetConfigHandler allocates it; init-time Option + * raises leave it null). The subscriber that owns `name` and + * rejects `value` reports the reason via reject(); + * SetConfigHandler then fails the setconfig command. Failing + * matters beyond cosmetics: lightningd persists a setconfig + * value (configvar_save) only on a success response, so + * acking a rejected value records it in config.setconfig and + * listconfigs forever -- and a non-numeric value persisted + * for an int-typed option even fails lightningd's own option + * parse on the NEXT start. Owners that accept the value + * leave it untouched. + */ + std::shared_ptr reject_reason; + + /* For the owning subscriber: report rejection of a dynamic + * update. No-op at init time (null reject_reason), where + * quietly keeping the default is the correct behaviour. + */ + void reject(std::string reason) const { + if (reject_reason) + *reject_reason = std::move(reason); + } }; }}