From 57c6313bbc9c97042d241d9d3879a24013b6199f Mon Sep 17 00:00:00 2001 From: Ken Sedgwick Date: Tue, 21 Jul 2026 13:27:53 -0700 Subject: [PATCH] lightningd: store raw failure message so waitsendpay always has raw_message If a payment's HTLC failure completes before waitsendpay is called, wait_payment() rebuilds the error from the database, which did not persist the raw BOLT4 failure message: a 2019 FIXME in that path set fail->msg = NULL, silently dropping raw_message from the error data. This is the cause of the test_error_returns_blockheight CI flake - the test loses the race occasionally, calls waitsendpay after the failure has landed, and KeyErrors on the missing raw_message. Add a failmsg column to the payments table (with a downgrade drop), persist fail->msg when recording the failure, and read it back in wait_payment. Local and self-payment failures store NULL as before, since no onion failure message exists for them; failed payments recorded before this migration also return NULL, matching the old behavior. The flaky test now also calls waitsendpay a second time, which deterministically takes the database-replay path, so the regression is covered without any timing dependence. Changelog-Fixed: JSON-RPC: `waitsendpay` error data now includes `raw_message` even when the payment already failed before the command was called. Fixes: #9341 --- lightningd/pay.c | 12 +++++++----- tests/test_pay.py | 9 +++++++++ wallet/migrations.c | 5 +++++ wallet/wallet.c | 14 ++++++++++++-- wallet/wallet.h | 6 ++++-- 5 files changed, 37 insertions(+), 9 deletions(-) diff --git a/lightningd/pay.c b/lightningd/pay.c index 1abf7585a7e1..28bfe75c04c0 100644 --- a/lightningd/pay.c +++ b/lightningd/pay.c @@ -652,7 +652,8 @@ void payment_failed(struct lightningd *ld, fail ? fail->erring_channel : NULL, NULL, failstr, - fail ? fail->channel_dir : 0); + fail ? fail->channel_dir : 0, + fail ? fail->msg : NULL); tell_waiters_failed(ld, payment_hash, payment, pay_errcode, failonion, fail, failstr); @@ -675,6 +676,7 @@ static struct command_result *wait_payment(struct lightningd *ld, struct short_channel_id *failchannel; u8 *failupdate; char *faildetail; + u8 *failmsg; struct routing_failure *fail; int faildirection; enum jsonrpc_errcode rpcerrorcode; @@ -715,7 +717,8 @@ static struct command_result *wait_payment(struct lightningd *ld, &failchannel, &failupdate, &faildetail, - &faildirection); + &faildirection, + &failmsg); /* Old DB might not save failure information */ if (!failonionreply && !failnode) { return command_fail(cmd, PAY_UNSPECIFIED_ERROR, @@ -745,8 +748,7 @@ static struct command_result *wait_payment(struct lightningd *ld, fail->erring_channel = NULL; } - /* FIXME: We don't store this! */ - fail->msg = NULL; + fail->msg = tal_dup_talarr(fail, u8, failmsg); /* Peers which fail directly can hit this! */ if (failcode & BADONION) @@ -1530,7 +1532,7 @@ static struct command_result *self_payment(struct lightningd *ld, fail->failcode, fail->erring_node, NULL, NULL, err, - 0); + 0, NULL); /* We do this even though there really can't be any waiters, * since we didn't block. */ tell_waiters_failed(ld, rhash, payment, PAY_DESTINATION_PERM_FAIL, diff --git a/tests/test_pay.py b/tests/test_pay.py index 0f357547ae76..de044997e9eb 100644 --- a/tests/test_pay.py +++ b/tests/test_pay.py @@ -2861,6 +2861,15 @@ def test_error_returns_blockheight(node_factory, bitcoind): assert (err.value.error['data']['raw_message'] == '400f{:016x}{:08x}'.format(100, bitcoind.rpc.getblockcount())) + # A second waitsendpay replays the failure from the database (the + # path a waitsendpay racing the failure takes): raw_message must + # survive that round trip too. + with pytest.raises(RpcError, match=r"INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS.*'erring_index': 1") as err: + l1.rpc.waitsendpay('00' * 32, TIMEOUT) + + assert (err.value.error['data']['raw_message'] + == '400f{:016x}{:08x}'.format(100, bitcoind.rpc.getblockcount())) + @unittest.skipIf(TEST_NETWORK != 'regtest', "Invoice is network specific") def test_pay_no_secret(node_factory, bitcoind): diff --git a/wallet/migrations.c b/wallet/migrations.c index 1f5d3b51c0f2..0100210d071b 100644 --- a/wallet/migrations.c +++ b/wallet/migrations.c @@ -1085,6 +1085,11 @@ static const struct db_migration dbmigrations[] = { {SQL("ALTER TABLE offers ADD COLUMN force_paths INTEGER DEFAULT 0;"), NULL, SQL("ALTER TABLE offers DROP COLUMN force_paths"), NULL}, /* ^v26.04 */ + + /* Raw BOLT4 failure message, so waitsendpay can report it even + * after the failure was recorded (issue #9341). */ + {SQL("ALTER TABLE payments ADD failmsg BLOB;"), NULL, + SQL("ALTER TABLE payments DROP COLUMN failmsg"), NULL}, }; const struct db_migration *get_db_migrations(size_t *num) diff --git a/wallet/wallet.c b/wallet/wallet.c index 91eb5079ff6b..d838b0f77384 100644 --- a/wallet/wallet.c +++ b/wallet/wallet.c @@ -4351,7 +4351,8 @@ void wallet_payment_get_failinfo(const tal_t *ctx, struct short_channel_id **failchannel, u8 **failupdate, char **faildetail, - int *faildirection) + int *faildirection, + u8 **failmsg) { struct db_stmt *stmt; bool resb; @@ -4361,6 +4362,7 @@ void wallet_payment_get_failinfo(const tal_t *ctx, ", failindex, failcode" ", failnode, failscid" ", failupdate, faildetail, faildirection" + ", failmsg" " FROM payments" " WHERE payment_hash=? AND partid=? AND groupid=?;")); db_bind_sha256(stmt, payment_hash); @@ -4395,6 +4397,10 @@ void wallet_payment_get_failinfo(const tal_t *ctx, *faildetail = db_col_strdup(ctx, stmt, "faildetail"); else *faildetail = NULL; + if (db_col_is_null(stmt, "failmsg")) + *failmsg = NULL; + else + *failmsg = db_col_arr(ctx, stmt, "failmsg", u8); tal_free(stmt); } @@ -4410,7 +4416,8 @@ void wallet_payment_set_failinfo(struct wallet *wallet, const struct short_channel_id *failchannel, const u8 *failupdate /*tal_arr*/, const char *faildetail, - int faildirection) + int faildirection, + const u8 *failmsg /*tal_arr*/) { struct db_stmt *stmt; @@ -4424,6 +4431,7 @@ void wallet_payment_set_failinfo(struct wallet *wallet, " , faildirection=?" " , failupdate=?" " , faildetail=?" + " , failmsg=?" " WHERE payment_hash=?" " AND partid=?;")); if (failonionreply) @@ -4454,6 +4462,8 @@ void wallet_payment_set_failinfo(struct wallet *wallet, else db_bind_null(stmt); + db_bind_talarr(stmt, failmsg); + db_bind_sha256(stmt, payment_hash); db_bind_u64(stmt, partid); diff --git a/wallet/wallet.h b/wallet/wallet.h index 504e49aad91a..6766fec866fc 100644 --- a/wallet/wallet.h +++ b/wallet/wallet.h @@ -1059,7 +1059,8 @@ void wallet_payment_get_failinfo(const tal_t *ctx, struct short_channel_id **failchannel, u8 **failupdate, char **faildetail, - int *faildirection); + int *faildirection, + u8 **failmsg); /** * wallet_payment_set_failinfo - Set failure information for a given * `payment_hash`. @@ -1075,7 +1076,8 @@ void wallet_payment_set_failinfo(struct wallet *wallet, const struct short_channel_id *failchannel, const u8 *failupdate, const char *faildetail, - int faildirection); + int faildirection, + const u8 *failmsg); /** * payments_first: get first payment, optionally filtering by status