diff --git a/Boss/Mod/AmountSettingsHandler.cpp b/Boss/Mod/AmountSettingsHandler.cpp index 23107fa..f289129 100644 --- a/Boss/Mod/AmountSettingsHandler.cpp +++ b/Boss/Mod/AmountSettingsHandler.cpp @@ -24,8 +24,6 @@ auto const default_reserve = Ln::Amount::sat( 30000); /* The absolute lowest min_channel setting. */ auto const min_min_channel = Ln::Amount::sat( 500000); -/* How much larger the max_channel should be over the min_channel. */ -auto const max_channel_factor = double(2.0); /* How much larger the channel-creation trigger should be over * the min_channel. */ auto const trigger_factor = double(2.0); @@ -33,6 +31,15 @@ auto const trigger_factor = double(2.0); * the amount to leave after creation. */ auto const additional_remaining = Ln::Amount::sat(20000); +/* The ChannelCreator Planner requires + * min_channel + min_remaining <= max_channel + * where min_remaining = trigger_factor * min_channel + * + additional_remaining. This is the lowest max_channel that + * satisfies it at the lowest allowed min_channel; below this no + * valid min_channel exists. */ +auto const min_usable_max_channel = + (1.0 + trigger_factor) * min_min_channel + additional_remaining; + Ln::Amount parse_sats(Jsmn::Object value) { auto is = std::istringstream(std::string(value)); auto sats = std::uint64_t(); @@ -153,19 +160,19 @@ private: min_min_channel.to_sat() ); } - if (settings->max_channel < ( max_channel_factor - * settings->min_channel - )) { - settings->max_channel = ( max_channel_factor - * settings->min_channel - ); - act += Boss::log( bus, Info + if (settings->max_channel < min_usable_max_channel) { + act += Boss::log( bus, Warn , "AmountSettingsHandler: " - "--clboss-max-channel too " - "low, forced to %u." + "--clboss-max-channel %u too " + "low for any allowed " + "--clboss-min-channel, " + "forced to %u." , (unsigned int) settings->max_channel.to_sat() + , (unsigned int) + min_usable_max_channel.to_sat() ); + settings->max_channel = min_usable_max_channel; } /* Compute the rest. */ @@ -176,6 +183,50 @@ private: + additional_remaining ; + /* The ChannelCreator Planner asserts + * min_channel + min_remaining <= max_channel + * at construction, so a violating config would + * abort on the first channel-creation run. + * max_channel is the knob that sets typical + * channel size: keep it, and lower min_channel + * to the largest value that fits. */ + if ( settings->min_channel + settings->min_remaining + > settings->max_channel + ) { + auto lowered = Ln::Amount::sat( + ( settings->max_channel + - additional_remaining + ).to_sat() + / (std::uint64_t)(1.0 + trigger_factor) + ); + act += Boss::log( bus, Warn + , "AmountSettingsHandler: " + "--clboss-min-channel %u " + "and --clboss-max-channel %u " + "conflict (max must be at " + "least %u), " + "--clboss-min-channel " + "forced to %u." + , (unsigned int) + settings->min_channel.to_sat() + , (unsigned int) + settings->max_channel.to_sat() + , (unsigned int) + ( settings->min_channel + + settings->min_remaining + ).to_sat() + , (unsigned int) + lowered.to_sat() + ); + settings->min_channel = lowered; + settings->min_amount = trigger_factor + * settings->min_channel + ; + settings->min_remaining = settings->min_amount + + additional_remaining + ; + } + /* Grab the settings and send it. */ auto msg = std::move(settings); return act + bus.raise(std::move(*msg)); diff --git a/Boss/Mod/ChannelCreator/Manager.cpp b/Boss/Mod/ChannelCreator/Manager.cpp index 27ccd13..81cd61b 100644 --- a/Boss/Mod/ChannelCreator/Manager.cpp +++ b/Boss/Mod/ChannelCreator/Manager.cpp @@ -100,6 +100,28 @@ void Manager::start() { Ev::Io Manager::on_request_channel_creation(Ln::Amount amt) { + /* The Planner asserts both of these at construction; check + * here and skip the cycle instead of aborting. The first + * can fail if onchain funds changed between the decider's + * trigger and now; the second is enforced at option + * validation, so failing it here is a bug. */ + if (amt < min_amount * 2.0) + return Boss::log( bus, Warn + , "ChannelCreator: Onchain amount %s " + "below twice the minimum channel size " + "%s, not creating channels." + , std::string(amt).c_str() + , std::string(min_amount).c_str() + ); + if (min_amount + min_remaining > max_amount) + return Boss::log( bus, Error + , "ChannelCreator: Channel size limits " + "(min %s, max %s) violate the planner " + "precondition, not creating channels." + , std::string(min_amount).c_str() + , std::string(max_amount).c_str() + ); + auto num_chans = std::make_shared(); auto plan = std::make_shared>(); diff --git a/CHANGELOG.md b/CHANGELOG.md index bab0b6b..b4d90bd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,18 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ## [Unreleased] +### Fixed + +- Channel size options that violate the channel-creation planner's + sizing requirement (`max-channel >= 3 * min-channel + 20000` + satoshis) no longer crash CLBOSS on the first creation run after + startup (#147). The maximum is kept, since it sets typical open + size, and the minimum is lowered to the largest fitting value, + with a warning logged. The creator also now skips a creation + cycle with a log line, instead of aborting, if onchain funds + drop below twice the minimum channel size between the decider's + trigger and planning (#137). + ### Changed - **BREAKING**: CLBOSS now requires **Core Lightning v26.06 or later**. diff --git a/Makefile.am b/Makefile.am index 3528a8d..cc1a774 100644 --- a/Makefile.am +++ b/Makefile.am @@ -643,6 +643,7 @@ TESTS = \ tests/boltz/test_match_lockscript \ tests/boss/channelcandidateinvestigator/test_gumshoe \ tests/boss/channelcandidateinvestigator/test_secretary \ + tests/boss/test_amountsettingshandler \ tests/boss/test_askrene_layer \ tests/boss/test_availablerpccommandsannouncer \ tests/boss/test_channel_create_destroy_monitor_missing_old_state \ diff --git a/README.md b/README.md index 67199de..07e4643 100644 --- a/README.md +++ b/README.md @@ -569,6 +569,12 @@ The defaults are: * Minimum: 500000sats = 5mBTC * Maximum: 16777215sats = 167.77215mBTC +The channel-creation planner requires +`max-channel >= 3 * min-channel + 20000`. +If the configured pair violates this, CLBOSS keeps the +maximum and lowers the minimum to the largest value that +fits, logging a warning. + Specify the value in satoshis without adding any unit suffix, e.g. diff --git a/tests/boss/test_amountsettingshandler.cpp b/tests/boss/test_amountsettingshandler.cpp new file mode 100644 index 0000000..bad6b28 --- /dev/null +++ b/tests/boss/test_amountsettingshandler.cpp @@ -0,0 +1,122 @@ +#undef NDEBUG + +#include"Boss/Mod/AmountSettingsHandler.hpp" +#include"Boss/Msg/AmountSettings.hpp" +#include"Boss/Msg/EndOfOptions.hpp" +#include"Boss/Msg/Option.hpp" +#include"Ev/Io.hpp" +#include"Ev/start.hpp" +#include"Jsmn/Object.hpp" +#include"Ln/Amount.hpp" +#include"S/Bus.hpp" +#include"Util/make_unique.hpp" + +#include +#include +#include +#include +#include + +namespace { + +struct Case { + /* Option values as delivered by lightningd; nullptr = unset. */ + char const* min_channel; + char const* max_channel; + /* Expected settings after validation. */ + std::uint64_t expect_min; + std::uint64_t expect_max; +}; + +auto const cases = std::vector{ + /* Defaults pass through untouched. */ + { nullptr , nullptr , 500000, 16777215}, + /* A pair satisfying max >= 3 * min + 20k passes through. */ + {"1000000", "3020000", 1000000, 3020000}, + /* min below the absolute floor is raised. */ + { "400000", nullptr , 500000, 16777215}, + /* Conflicting pair: max kept, min lowered to the largest + * value satisfying min_channel + min_remaining <= max_channel. */ + {"1000000", "2000000", 660000, 2000000}, + /* Non-divisible conflict: truncation keeps the invariant. */ + {"1000000", "3000000", 993333, 3000000}, + /* max too low for any allowed min: max raised, min floored. */ + {"2000000", "1000000", 500000, 1520000}, +}; + +Boss::Msg::Option make_option(char const* name, char const* value) { + auto json = "\"" + std::string(value) + "\""; + return Boss::Msg::Option{ + name, + Jsmn::Object::parse_json(json.c_str()), + nullptr + }; +} + +} + +int main() { + auto buses = std::vector>(); + auto handlers = std::vector< + std::unique_ptr + >(); + + auto code = Ev::lift(); + for (auto const& c : cases) { + buses.push_back(Util::make_unique()); + auto& bus = *buses.back(); + handlers.push_back( + Util::make_unique(bus) + ); + + auto captured = std::make_shared(); + auto have = std::make_shared(false); + bus.subscribe([captured, have](Boss::Msg::AmountSettings const& m) { + *captured = m; + *have = true; + return Ev::lift(); + }); + + code += Ev::lift().then([&bus, c]() { + if (!c.min_channel) + return Ev::lift(); + return bus.raise(make_option( "clboss-min-channel" + , c.min_channel + )); + }).then([&bus, c]() { + if (!c.max_channel) + return Ev::lift(); + return bus.raise(make_option( "clboss-max-channel" + , c.max_channel + )); + }).then([&bus]() { + return bus.raise(Boss::Msg::EndOfOptions{}); + }).then([captured, have, c]() { + assert(*have); + assert( captured->min_channel + == Ln::Amount::sat(c.expect_min) + ); + assert( captured->max_channel + == Ln::Amount::sat(c.expect_max) + ); + /* min_remaining derivation. */ + assert( captured->min_remaining + == 2.0 * captured->min_channel + + Ln::Amount::sat(20000) + ); + /* Whatever was configured, the published + * settings must satisfy the Planner + * precondition. */ + assert( captured->min_channel + + captured->min_remaining + <= captured->max_channel + ); + return Ev::lift(); + }); + } + + return Ev::start(std::move(code).then([]() { + return Ev::lift(0); + })); +}