ChannelCreator: prefer spliceable peers within the no-record tier

The track-record pass orders open proposals keepers, no-record,
underperformers.  The no-record tier carries no earnings evidence,
so order it by a capability prior: nodes whose node_announcement
advertises splicing (BOLT 9 option_splice, bits 62/63) come first,
since their channels can later be resized without a close and
reopen.  Keepers and underperformers keep their earnings-based
order, and nothing moves across a tier boundary.

New Ln::feature_bit tests a bit in a BOLT 9 hex bitfield (bit 0 is
the least-significant bit of the last byte), with a unit test.
Spliceability is looked up per candidate via listnodes; an RPC
failure or absent features field counts as not spliceable, so the
lookup cannot block channel creation.  The Track records report
marks spliceable no-record nodes with an (S) suffix.

Boss/Mod/ChannelCreator/Manager.cpp

@@ -11,11 +11,13 @@
 #include"Boss/concurrent.hpp"
 #include"Boss/log.hpp"
 #include"Ev/Io.hpp"
+#include"Ev/map.hpp"
 #include"Ev/memoize.hpp"
 #include"Ev/yield.hpp"
 #include"Jsmn/Object.hpp"
 #include"Json/Out.hpp"
 #include"Ln/Amount.hpp"
+#include"Ln/FeatureBit.hpp"
 #include"Net/IPAddrOrOnion.hpp"
 #include"Net/IPBinnerBySubnet.hpp"
 #include"S/Bus.hpp"
@@ -35,6 +37,12 @@ bool plan_is_empty(std::map<Ln::NodeId, Ln::Amount> const& plan) {
 	});
 }
 
+void append_entry(std::string& s, std::string const& entry) {
+	if (!s.empty())
+		s += ", ";
+	s += entry;
+}
+
 Ev::Io<void> report_proposals( S::Bus& bus, char const* prefix
 			     , std::vector< std::pair<Ln::NodeId, Ln::NodeId>
 					  > const& proposals
@@ -301,6 +309,41 @@ Manager::get_peers() {
 		return Ev::lift(std::move(rv));
 	});
 }
+Ev::Io<std::set<Ln::NodeId>>
+Manager::get_spliceable_nodes(std::vector<Ln::NodeId> nodes) {
+	assert(rpc);
+	auto lookup = [this](Ln::NodeId n) {
+		return rpc->command("listnodes"
+				   , Json::Out()
+					.start_object()
+						.field("id", std::string(n))
+					.end_object()
+				   ).then([n](Jsmn::Object res) {
+			auto spliceable = false;
+			try {
+				auto ns = res["nodes"];
+				if (ns.length() != 0 && ns[0].has("features")) {
+					auto f = std::string(ns[0]["features"]);
+					/* BOLT #9 `option_splice`.  */
+					spliceable = Ln::feature_bit(f, 62)
+						  || Ln::feature_bit(f, 63)
+						   ;
+				}
+			} catch (...) { /* Treat as not spliceable.  */ }
+			return Ev::lift(std::make_pair(n, spliceable));
+		}).catching<RpcError>([n](RpcError const&) {
+			return Ev::lift(std::make_pair(n, false));
+		});
+	};
+	return Ev::map( std::move(lookup), std::move(nodes)
+		      ).then([](std::vector<std::pair<Ln::NodeId, bool>> flags) {
+		auto rv = std::set<Ln::NodeId>();
+		for (auto const& f : flags)
+			if (f.second)
+				rv.insert(f.first);
+		return Ev::lift(std::move(rv));
+	});
+}
 Ev::Io<std::vector<std::pair<Ln::NodeId, Ln::NodeId>>>
 Manager::reprioritize(std::vector<std::pair<Ln::NodeId, Ln::NodeId>> proposals_v) {
 	auto proposals = std::make_shared<std::vector<std::pair<Ln::NodeId, Ln::NodeId>>>
@@ -338,22 +381,16 @@ Manager::prioritize_by_track_record(std::vector<std::pair<Ln::NodeId, Ln::NodeId
 	}).then([ this
 		, proposals
 		](Msg::ResponsePeerTrackRecord resp) {
-		auto keepers = Proposals();
-		auto no_records = Proposals();
-		auto underperformers = Proposals();
+		auto keepers = std::make_shared<Proposals>();
+		auto no_records = std::make_shared<Proposals>();
+		auto underperformers = std::make_shared<Proposals>();
 
 		/* Per-tier report text; nodes within a tier keep their
-		 * relative order from the earlier stages.  */
-		auto keepers_s = std::string();
-		auto no_records_s = std::string();
-		auto underperformers_s = std::string();
-		auto append = []( std::string& s
-				, std::string const& entry
-				) {
-			if (!s.empty())
-				s += ", ";
-			s += entry;
-		};
+		 * relative order from the earlier stages.  The no-record
+		 * text is built later, after the splice preference has
+		 * settled that tier's order.  */
+		auto keepers_s = std::make_shared<std::string>();
+		auto underperformers_s = std::make_shared<std::string>();
 
 		for (auto const& p : *proposals) {
 			auto rec = Msg::TrackRecord{
@@ -376,46 +413,83 @@ Manager::prioritize_by_track_record(std::vector<std::pair<Ln::NodeId, Ln::NodeId
 
 			switch (rec.verdict) {
 			case Msg::TrackRecordVerdict::Keeper:
-				keepers.push_back(p);
-				append(keepers_s, os.str());
+				keepers->push_back(p);
+				append_entry(*keepers_s, os.str());
 				break;
 			case Msg::TrackRecordVerdict::NoRecord:
-				no_records.push_back(p);
-				append(no_records_s, os.str());
+				no_records->push_back(p);
 				break;
 			case Msg::TrackRecordVerdict::Underperformer:
-				underperformers.push_back(p);
-				append(underperformers_s, os.str());
+				underperformers->push_back(p);
+				append_entry(*underperformers_s, os.str());
 				break;
 			}
 		}
 
-		auto report = std::string();
-		if (!keepers_s.empty())
-			report += "keepers: " + keepers_s + "; ";
-		if (!no_records_s.empty())
-			report += "no record: " + no_records_s + "; ";
-		if (!underperformers_s.empty())
-			report += "underperformers: "
-				+ underperformers_s + "; "
-				;
-		/* Trim the trailing "; ".  */
-		report.erase(report.size() - 2);
+		/* The no-record tier carries no earnings evidence, so
+		 * order it by a capability prior: nodes announcing
+		 * splicing support first, since their channels can be
+		 * resized later without a close+reopen.  Keepers and
+		 * underperformers are left alone; earnings evidence
+		 * outranks the prior, and this must not move anyone
+		 * across a tier boundary.  */
+		auto no_record_nodes = std::vector<Ln::NodeId>();
+		for (auto const& p : *no_records)
+			no_record_nodes.push_back(p.first);
 
-		*proposals = std::move(keepers);
-		proposals->insert( proposals->end()
-				 , no_records.begin(), no_records.end()
-				 );
-		proposals->insert( proposals->end()
-				 , underperformers.begin()
-				 , underperformers.end()
-				 );
+		return get_spliceable_nodes( std::move(no_record_nodes)
+					   ).then([ this
+						  , proposals
+						  , keepers
+						  , no_records
+						  , underperformers
+						  , keepers_s
+						  , underperformers_s
+						  ](std::set<Ln::NodeId> spliceable) {
+			std::stable_partition( no_records->begin()
+					     , no_records->end()
+					     , [&spliceable]( std::pair< Ln::NodeId
+								       , Ln::NodeId
+								       > const& p) {
+				return spliceable.count(p.first) != 0;
+			});
 
-		return Boss::log( bus, Info
-				, "ChannelCreator: Track records: %s"
-				, report.c_str()
-				).then([proposals]() {
-			return Ev::lift(std::move(*proposals));
+			auto no_records_s = std::string();
+			for (auto const& p : *no_records) {
+				auto os = std::ostringstream();
+				os << p.first;
+				if (spliceable.count(p.first) != 0)
+					os << "(S)";
+				append_entry(no_records_s, os.str());
+			}
+
+			auto report = std::string();
+			if (!keepers_s->empty())
+				report += "keepers: " + *keepers_s + "; ";
+			if (!no_records_s.empty())
+				report += "no record: " + no_records_s + "; ";
+			if (!underperformers_s->empty())
+				report += "underperformers: "
+					+ *underperformers_s + "; "
+					;
+			/* Trim the trailing "; ".  */
+			report.erase(report.size() - 2);
+
+			*proposals = std::move(*keepers);
+			proposals->insert( proposals->end()
+					 , no_records->begin(), no_records->end()
+					 );
+			proposals->insert( proposals->end()
+					 , underperformers->begin()
+					 , underperformers->end()
+					 );
+
+			return Boss::log( bus, Info
+					, "ChannelCreator: Track records: %s"
+					, report.c_str()
+					).then([proposals]() {
+				return Ev::lift(std::move(*proposals));
+			});
 		});
 	});
 }

Boss/Mod/ChannelCreator/Manager.hpp

@@ -9,6 +9,7 @@
 #include"Boss/Msg/ResponsePeerTrackRecord.hpp"
 #include"Ln/NodeId.hpp"
 #include<memory>
+#include<set>
 #include<utility>
 #include<vector>
 
@@ -64,6 +65,9 @@ private:
 	/* Partition proposals by earnings track record and log it.  */
 	Ev::Io<std::vector<std::pair<Ln::NodeId, Ln::NodeId>>>
 	prioritize_by_track_record(std::vector<std::pair<Ln::NodeId, Ln::NodeId>>);
+	/* Which of the given nodes announce splicing support.  */
+	Ev::Io<std::set<Ln::NodeId>>
+	get_spliceable_nodes(std::vector<Ln::NodeId>);
 
 public:
 	Manager() =delete;

Ln/FeatureBit.cpp

@@ -0,0 +1,22 @@
+#include"Ln/FeatureBit.hpp"
+#include"Util/Str.hpp"
+#include<cstdint>
+#include<vector>
+
+namespace Ln {
+
+bool feature_bit(std::string const& features_hex, unsigned int bit) {
+	auto bytes = std::vector<std::uint8_t>();
+	try {
+		bytes = Util::Str::hexread(features_hex);
+	} catch (Util::Str::HexParseFailure const&) {
+		return false;
+	}
+	auto byte_from_end = std::size_t(bit / 8);
+	if (byte_from_end >= bytes.size())
+		return false;
+	auto b = bytes[bytes.size() - 1 - byte_from_end];
+	return ((b >> (bit % 8)) & 1) != 0;
+}
+
+}

Ln/FeatureBit.hpp

@@ -0,0 +1,21 @@
+#ifndef LN_FEATUREBIT_HPP
+#define LN_FEATUREBIT_HPP
+
+#include<string>
+
+namespace Ln {
+
+/** Ln::feature_bit
+ *
+ * @brief determine whether the given feature bit is set in a
+ * hex-encoded feature bitfield (BOLT #9 encoding: bit 0 is the
+ * least-significant bit of the last byte).
+ *
+ * @desc returns false for malformed hex, or for bitfields too
+ * short to contain the bit.
+ */
+bool feature_bit(std::string const& features_hex, unsigned int bit);
+
+}
+
+#endif /* !defined(LN_FEATUREBIT_HPP) */

Makefile.am

@@ -488,6 +488,8 @@ libclboss_la_SOURCES = \
 	Ln/Amount.hpp \
 	Ln/CommandId.cpp \
 	Ln/CommandId.hpp \
+	Ln/FeatureBit.cpp \
+	Ln/FeatureBit.hpp \
 	Ln/HtlcAccepted.cpp \
 	Ln/HtlcAccepted.hpp \
 	Ln/NodeId.cpp \
@@ -699,6 +701,7 @@ TESTS = \
 	tests/json/test_out_simple \
 	tests/ln/test_amount \
 	tests/ln/test_commandid \
+	tests/ln/test_featurebit \
 	tests/ln/test_htlcaccepted \
 	tests/ln/test_nodeid \
 	tests/ln/test_onionerror \

tests/ln/test_featurebit.cpp

@@ -0,0 +1,39 @@
+#undef NDEBUG
+#include"Ln/FeatureBit.hpp"
+#include<assert.h>
+
+int main() {
+	using Ln::feature_bit;
+
+	/* Bit 0 is the least-significant bit of the last byte.  */
+	assert( feature_bit("01", 0));
+	assert(!feature_bit("01", 1));
+	assert( feature_bit("02", 1));
+	assert(!feature_bit("02", 0));
+
+	/* Bits beyond the field are unset.  */
+	assert(!feature_bit("01", 8));
+	assert(!feature_bit("", 0));
+
+	/* Multi-byte: bit 8 is the least-significant bit of the
+	 * second-to-last byte.  */
+	assert( feature_bit("0100", 8));
+	assert(!feature_bit("0100", 0));
+
+	/* option_splice bits 62/63 need an 8-byte field; bit 62 is
+	 * 0x40 of the leading byte, bit 63 is 0x80.  */
+	assert( feature_bit("4000000000000000", 62));
+	assert(!feature_bit("4000000000000000", 63));
+	assert( feature_bit("8000000000000000", 63));
+	assert(!feature_bit("8000000000000000", 62));
+
+	/* Longer fields keep bit positions anchored at the tail.  */
+	assert( feature_bit("00004000000000000000", 62));
+	assert( feature_bit("888a4000000000000000", 62));
+
+	/* Malformed hex is just "no features".  */
+	assert(!feature_bit("xyz", 0));
+	assert(!feature_bit("0", 0));
+
+	return 0;
+}