From 9075ee32fcc695d061c772c66ca840ca94474349 Mon Sep 17 00:00:00 2001 From: Ken Sedgwick Date: Wed, 12 Aug 2026 13:25:31 -0700 Subject: [PATCH 1/2] Boltz/Detail/ClaimTxHandler.cpp: Scope claim db update to the claimed swap. After broadcasting a reverse-swap claim transaction, the handler recorded the claim with an UPDATE that had no WHERE clause, marking every row in BoltzServiceFactory_rsub lockedUp=1. Any other in-flight swap then took the "Already broadcasted claim tx." early exit and never broadcast its own claim; its off-chain payment stayed unsettled until the timeout/refund path unwound it. Add the same apiAccess + swapId predicates used by the neighboring SELECTs and DELETE, and a regression test that seeds two swaps, claims one, and asserts the sibling row is untouched. The test fails before this change and passes after. Reported by Vincenzo Palazzo (Bitcoin Security Council / Sentinel). Fixes #325. --- Boltz/Detail/ClaimTxHandler.cpp | 4 + Makefile.am | 1 + .../test_claimtxhandler_scoped_update.cpp | 262 ++++++++++++++++++ 3 files changed, 267 insertions(+) create mode 100644 tests/boltz/test_claimtxhandler_scoped_update.cpp diff --git a/Boltz/Detail/ClaimTxHandler.cpp b/Boltz/Detail/ClaimTxHandler.cpp index cee447f..71f4b69 100644 --- a/Boltz/Detail/ClaimTxHandler.cpp +++ b/Boltz/Detail/ClaimTxHandler.cpp @@ -231,12 +231,16 @@ Ev::Io ClaimTxHandler::core_run() { , lockupOut = :lockupOut , lockupConfirmedHeight = :blockheight , lockupClaimFees = :lockupClaimFees + WHERE apiAccess = :apiAccess + AND swapId = :swapId ; )QRY") .bind(":lockup_txid", std::string(lockup_txid)) .bind(":lockupOut", lockupOut) .bind(":blockheight", blockheight) .bind(":lockupClaimFees", lockupClaimFees.to_sat()) + .bind(":apiAccess", api_endpoint) + .bind(":swapId", swapId) .execute(); tx.commit(); diff --git a/Makefile.am b/Makefile.am index f2cc462..b958e63 100644 --- a/Makefile.am +++ b/Makefile.am @@ -595,6 +595,7 @@ TESTS = \ tests/bitcoin/test_serial \ tests/bitcoin/test_sighash \ tests/bitcoin/test_tx \ + tests/boltz/test_claimtxhandler_scoped_update \ tests/boltz/test_match_lockscript \ tests/boss/channelcandidateinvestigator/test_gumshoe \ tests/boss/channelcandidateinvestigator/test_secretary \ diff --git a/tests/boltz/test_claimtxhandler_scoped_update.cpp b/tests/boltz/test_claimtxhandler_scoped_update.cpp new file mode 100644 index 0000000..4fcc148 --- /dev/null +++ b/tests/boltz/test_claimtxhandler_scoped_update.cpp @@ -0,0 +1,262 @@ +#undef NDEBUG +#include"Boltz/Detail/ClaimTxHandler.hpp" +#include"Boltz/EnvIF.hpp" +#include"Bitcoin/Tx.hpp" +#include"Bitcoin/TxId.hpp" +#include"Ev/Io.hpp" +#include"Ev/start.hpp" +#include"Ln/Amount.hpp" +#include"Ln/Preimage.hpp" +#include"Secp256k1/PrivKey.hpp" +#include"Secp256k1/PubKey.hpp" +#include"Secp256k1/Signature.hpp" +#include"Secp256k1/SignerIF.hpp" +#include"Sha256/Hash.hpp" +#include"Sha256/fun.hpp" +#include"Sqlite3.hpp" +#include"Util/Str.hpp" +#include +#include +#include +#include +#include + +namespace { + +/* Mock environment: fixed feerate, broadcast always succeeds + * and records that it was called. + */ +class MockEnv : public Boltz::EnvIF { +private: + bool& broadcast_called; + +public: + explicit + MockEnv(bool& broadcast_called_) + : broadcast_called(broadcast_called_) { } + + Ev::Io get_feerate() override { + return Ev::lift(std::uint32_t(1000)); + } + Ev::Io broadcast_tx(Bitcoin::Tx) override { + broadcast_called = true; + return Ev::lift(true); + } + Ev::Io logd(std::string) override { return Ev::lift(); } + Ev::Io loge(std::string) override { return Ev::lift(); } +}; + +/* Mock signer backed by a fixed privkey. Nothing in the claim + * path verifies the signature, so signing with an arbitrary key + * is fine. + */ +class MockSigner : public Secp256k1::SignerIF { +private: + Secp256k1::PrivKey privkey; + +public: + MockSigner() + : privkey(Secp256k1::PrivKey(std::string( + "0101010101010101010101010101010101010101010101010101010101010101" + ))) + { } + + Secp256k1::PubKey get_pubkey_tweak(Secp256k1::PrivKey const&) override { + return Secp256k1::PubKey(privkey); + } + Secp256k1::Signature get_signature_tweak( Secp256k1::PrivKey const& + , Sha256::Hash const& m + ) override { + return Secp256k1::Signature::create(privkey, m); + } + Sha256::Hash get_privkey_salted_hash(std::uint8_t salt[32]) override { + return Sha256::fun(salt, 32); + } +}; + +/* Table schema, copied from Boltz/ServiceFactory.cpp. */ +char const schema[] = R"QRY( +CREATE TABLE "BoltzServiceFactory_rsub" + ( id INTEGER PRIMARY KEY + , apiAccess TEXT NOT NULL + , tweak TEXT NOT NULL + , preimage TEXT NOT NULL + , destinationAddress TEXT NOT NULL + , swapId TEXT NOT NULL + , redeemScript TEXT NOT NULL + , timeoutBlockheight INTEGER NOT NULL + , onchainAmount INTEGER NOT NULL + , lockedUp INTEGER NOT NULL + , lockupTxid TEXT NULL + , lockupOut INTEGER NULL + , lockupConfirmedHeight INTEGER NULL + , lockupClaimFees INTEGER NULL + , comment TEXT + ); +)QRY"; + +} + +int main() { + auto db = Sqlite3::Db(":memory:"); + + auto const api = std::string("testapi"); + /* The claim path only hashes the redeem script and copies + * it into the claim witness; its content does not matter. + */ + auto const redeemScript = std::string("51"); + auto const tweak = std::string( + "0202020202020202020202020202020202020202020202020202020202020202" + ); + auto const preimage = std::string( + "0303030303030303030303030303030303030303030303030303030303030303" + ); + auto const addr = std::string( + "bc1qg430dgu75qvphu8nn3kvcp3yg2km2tavpc8lqs" + ); + auto const timeout = std::uint32_t(1000); + auto const amount = std::uint64_t(100000); + auto const blockheight = std::uint32_t(100); + + /* Lockup tx paying P2WSH of the redeem script. */ + auto script_bytes = Util::Str::hexread(redeemScript); + auto script_hash = Sha256::fun(&script_bytes[0], script_bytes.size()); + auto scriptPubKey = std::vector(34); + scriptPubKey[0] = 0x00; + scriptPubKey[1] = 0x20; + script_hash.to_buffer(&scriptPubKey[2]); + + auto lockup_tx = Bitcoin::Tx(); + lockup_tx.inputs.resize(1); + lockup_tx.inputs[0].prevTxid = Bitcoin::TxId(std::string( + "0404040404040404040404040404040404040404040404040404040404040404" + )); + lockup_tx.inputs[0].prevOut = 0; + lockup_tx.outputs.resize(1); + lockup_tx.outputs[0].scriptPubKey = scriptPubKey; + lockup_tx.outputs[0].amount = Ln::Amount::sat(amount); + + auto broadcast_called = bool(false); + auto env = MockEnv(broadcast_called); + auto signer = MockSigner(); + + auto insert_swap = [&](std::string swapId) { + return db.transact().then([&, swapId](Sqlite3::Tx tx) { + tx.query(R"QRY( + INSERT INTO "BoltzServiceFactory_rsub" + ( apiAccess + , tweak + , preimage + , destinationAddress + , swapId + , redeemScript + , timeoutBlockheight + , onchainAmount + , lockedUp + , comment + ) + VALUES + ( :apiAccess + , :tweak + , :preimage + , :destinationAddress + , :swapId + , :redeemScript + , :timeoutBlockheight + , :onchainAmount + , 0 + , '' + ); + )QRY") + .bind(":apiAccess", api) + .bind(":tweak", tweak) + .bind(":preimage", preimage) + .bind(":destinationAddress", addr) + .bind(":swapId", swapId) + .bind(":redeemScript", redeemScript) + .bind(":timeoutBlockheight", timeout) + .bind(":onchainAmount", amount) + .execute(); + tx.commit(); + return Ev::lift(); + }); + }; + + auto code = Ev::lift().then([&]() { + return db.transact(); + }).then([&](Sqlite3::Tx tx) { + tx.query_execute(schema); + tx.commit(); + /* Two swaps in flight at once. */ + return insert_swap("swapA"); + }).then([&]() { + return insert_swap("swapB"); + }).then([&]() { + /* Claim the first swap. */ + auto handler = Boltz::Detail::ClaimTxHandler::create( + signer, db, env, api, "swapA", blockheight, lockup_tx + ); + return handler->run(); + }).then([&]() { + return db.transact(); + }).then([&](Sqlite3::Tx tx) { + /* The handler must have reached the broadcast, + * otherwise the assertions below are vacuous. + */ + assert(broadcast_called); + + /* Sanity: the claimed swap was stamped. */ + auto stamped = tx.query(R"QRY( + SELECT lockedUp, lockupTxid + FROM "BoltzServiceFactory_rsub" + WHERE apiAccess = :apiAccess + AND swapId = 'swapA' + ; + )QRY") + .bind(":apiAccess", api) + .execute(); + auto found = false; + for (auto& r : stamped) { + found = true; + assert(r.get(0)); + assert(!r.get(1).empty()); + } + assert(found); + + /* Regression: the sibling swap must be untouched. */ + auto sibling = tx.query(R"QRY( + SELECT lockedUp + FROM "BoltzServiceFactory_rsub" + WHERE apiAccess = :apiAccess + AND swapId = 'swapB' + ; + )QRY") + .bind(":apiAccess", api) + .execute(); + found = false; + for (auto& r : sibling) { + found = true; + assert(!r.get(0)); + } + assert(found); + + /* Exactly one row may be stamped overall. */ + auto count = tx.query(R"QRY( + SELECT COUNT(*) FROM "BoltzServiceFactory_rsub" + WHERE lockedUp <> 0 + OR lockupTxid IS NOT NULL + ; + )QRY").execute(); + found = false; + for (auto& r : count) { + found = true; + assert(r.get(0) == 1); + } + assert(found); + + tx.commit(); + return Ev::lift(0); + }); + + return Ev::start(code); +} From 99c0592612bdcd9d4634433f5ccfb408a3128f2c Mon Sep 17 00:00:00 2001 From: Ken Sedgwick Date: Wed, 12 Aug 2026 14:55:09 -0700 Subject: [PATCH 2/2] tests/boltz/test_claimtxhandler_scoped_update.cpp: Also cover the apiAccess predicate. Both inserted rows used the same apiAccess, so the test would not catch a claim UPDATE scoped by swapId alone. Add a third row sharing swapA's swapId under a different apiAccess; the single-stamped-row count assertion now fails if either predicate is dropped. Verified: a swapId-only variant fails the count assertion. Prompted by review on #326. --- .../boltz/test_claimtxhandler_scoped_update.cpp | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/tests/boltz/test_claimtxhandler_scoped_update.cpp b/tests/boltz/test_claimtxhandler_scoped_update.cpp index 4fcc148..dc1e318 100644 --- a/tests/boltz/test_claimtxhandler_scoped_update.cpp +++ b/tests/boltz/test_claimtxhandler_scoped_update.cpp @@ -140,8 +140,8 @@ int main() { auto env = MockEnv(broadcast_called); auto signer = MockSigner(); - auto insert_swap = [&](std::string swapId) { - return db.transact().then([&, swapId](Sqlite3::Tx tx) { + auto insert_swap = [&](std::string apiAccess, std::string swapId) { + return db.transact().then([&, apiAccess, swapId](Sqlite3::Tx tx) { tx.query(R"QRY( INSERT INTO "BoltzServiceFactory_rsub" ( apiAccess @@ -168,7 +168,7 @@ int main() { , '' ); )QRY") - .bind(":apiAccess", api) + .bind(":apiAccess", apiAccess) .bind(":tweak", tweak) .bind(":preimage", preimage) .bind(":destinationAddress", addr) @@ -188,9 +188,15 @@ int main() { tx.query_execute(schema); tx.commit(); /* Two swaps in flight at once. */ - return insert_swap("swapA"); + return insert_swap(api, "swapA"); }).then([&]() { - return insert_swap("swapB"); + return insert_swap(api, "swapB"); + }).then([&]() { + /* A third swap with the same swapId under a different + * apiAccess, so the test fails if the apiAccess + * predicate is dropped from the update. + */ + return insert_swap("other-api", "swapA"); }).then([&]() { /* Claim the first swap. */ auto handler = Boltz::Detail::ClaimTxHandler::create(