AmountSettingsHandler: enforce the Planner's channel size precondition (#147)

The ChannelCreator Planner asserts
min_channel + min_remaining <= max_channel at construction, where
min_remaining = 2 * min_channel + 20000 sats.  Option validation
only enforced max_channel >= 2 * min_channel, so a pair such as
min-channel=1000000 max-channel=2000000 passed startup and aborted
the plugin on the first channel-creation run (#147).  The old 2x
clamp never prevented the crash: any pair it adjusted still
violated the planner precondition.

AmountSettingsHandler now enforces the precondition directly.  On
conflict it keeps max-channel, which sets typical open size, and
lowers min-channel to the largest fitting value, logging a warning.
A max-channel too low for even the smallest permitted min-channel
is raised.

ChannelCreator::Manager also re-checks both planner preconditions
before constructing the Planner and skips the creation cycle with a
log line instead of aborting.  This covers the sibling assert
min_amount * 2 <= total, which fails when onchain funds change
between the decider's trigger and the creator's run (#137).

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));

Boss/Mod/ChannelCreator/Manager.cpp

@@ -100,6 +100,28 @@ void Manager::start() {
 
 Ev::Io<void>
 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<std::size_t>();
 	auto plan = std::make_shared<std::map<Ln::NodeId, Ln::Amount>>();
 

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**.

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 \

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.
 

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<assert.h>
+#include<cstdint>
+#include<memory>
+#include<string>
+#include<vector>
+
+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<Case>{
+	/* 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<std::unique_ptr<S::Bus>>();
+	auto handlers = std::vector<
+		std::unique_ptr<Boss::Mod::AmountSettingsHandler>
+	>();
+
+	auto code = Ev::lift();
+	for (auto const& c : cases) {
+		buses.push_back(Util::make_unique<S::Bus>());
+		auto& bus = *buses.back();
+		handlers.push_back(
+			Util::make_unique<Boss::Mod::AmountSettingsHandler>(bus)
+		);
+
+		auto captured = std::make_shared<Boss::Msg::AmountSettings>();
+		auto have = std::make_shared<bool>(false);
+		bus.subscribe<Boss::Msg::AmountSettings
+			     >([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);
+	}));
+}