XRebalancePartMonitor: parse first_hop/return_hop as scidds
Some checks are pending
Code Base Sanity Check / tests (push) Waiting to run
Code Base Sanity Check / coverage (push) Waiting to run
Code Base Sanity Check / build-clang (push) Waiting to run

Found live via a probe subscriber: the plugin reports first_hop and
return_hop as SCIDDs ("305293x6x2/1"), but the monitor fed them to
Ln::Scid, which throws BacktraceException<std::invalid_argument> --
and the handler's catch(std::runtime_error) does not cover
logic_error, so every completed part died silently between the amount
parse and the attribution: no Error line, no Warn, no accounting.
The unit test's hand-written payloads used plain scids and kept
passing.

Strip the direction suffix before the Ln::Scid parse (the mapper keys
on the channel alone), widen the catch to std::exception so a future
payload surprise logs instead of vanishing, and reshape the test
payloads to the live-captured form, direction suffixes included.
This commit is contained in:
Ken Sedgwick 2026-07-22 21:16:04 -07:00
parent e46ba619f6
commit 813c8977bd
No known key found for this signature in database
GPG key ID: DBD2AF0849D711A9
2 changed files with 26 additions and 12 deletions

View file

@ -36,6 +36,14 @@ void XRebalancePartMonitor::start() {
auto return_scid = Ln::Scid();
auto amount = Ln::Amount();
auto fee = Ln::Amount();
/* first_hop / return_hop are scidds ("845x1x0/1"); the
* mapper keys on the channel alone, so drop the
* direction suffix. */
auto scid_of_scidd = [](std::string const& s) {
auto slash = s.find('/');
return Ln::Scid( slash == std::string::npos
? s : s.substr(0, slash));
};
try {
/* Custom notifications arrive with the sender's
* payload AS params (lightningd relays it verbatim,
@ -55,10 +63,10 @@ void XRebalancePartMonitor::start() {
if (std::string(payload["status"]) != "complete")
return Ev::lift();
first_scid = Ln::Scid(std::string(
first_scid = scid_of_scidd(std::string(
payload["first_hop"]
));
return_scid = Ln::Scid(std::string(
return_scid = scid_of_scidd(std::string(
payload["return_hop"]
));
amount = Ln::Amount::object(
@ -67,7 +75,11 @@ void XRebalancePartMonitor::start() {
fee = Ln::Amount::object(
payload["fee_msat"]
);
} catch (std::runtime_error const& err) {
/* std::exception, not std::runtime_error: Ln::Scid
* throws invalid_argument (a logic_error), and a
* narrower catch let exactly that escape unlogged --
* the silent-attribution-loss bug. */
} catch (std::exception const& err) {
return Boss::log( bus, Error
, "XRebalancePartMonitor: unexpected "
"xrebalance_part payload: %s: %s"

View file

@ -39,18 +39,20 @@ auto const listpeers_result = R"JSON(
}
)JSON";
/* The plugin's Part::json shape: plain-number msat fields, real scids
* on first_hop / return_hop, label appended by the notifier. Custom
* notifications deliver this AS params (lightningd relays the sender's
* payload verbatim; no topic-key nesting, that is a built-in-topic
* convention). */
/* The plugin's Part::json shape, as captured live off a probe
* subscriber (2026-07-23, lab0-a): plain-number msat fields, real
* SCIDDs (direction-suffixed) on first_hop / return_hop, label
* appended by the notifier. Custom notifications deliver this AS
* params (lightningd relays the sender's payload verbatim; cln-plugin
* 0.7.0 also nests a copy under the topic key, which consumers should
* ignore). */
auto const complete_part = R"JSON(
{
"part_index": 2,
"payment_hash": "f5a6a059a25d1e329d9b094aeeec8c2191ca037d3f5b0662e21ae850debe8ea2",
"status": "complete",
"first_hop": "1000x1x1",
"return_hop": "1000x1x0",
"first_hop": "1000x1x1/1",
"return_hop": "1000x1x0/0",
"planned_msat": 10000000,
"delivered_msat": 10000000,
"sent_msat": 10005958,
@ -68,8 +70,8 @@ auto const failed_part = R"JSON(
"part_index": 1,
"payment_hash": "9d9b094aeeec8c2191ca037d3f5b0662e21ae850debe8ea2f5a6a059a25d1e32",
"status": "failed",
"first_hop": "1000x1x1",
"return_hop": "1000x1x0",
"first_hop": "1000x1x1/1",
"return_hop": "1000x1x0/0",
"planned_msat": 10000000,
"delivered_msat": 0,
"sent_msat": 10005958,