From da51b5e4d216a2eff0e93d31c93434f3eac02f60 Mon Sep 17 00:00:00 2001 From: Hodlinator <172445034+hodlinator@users.noreply.github.com> Date: Mon, 2 Feb 2026 11:45:01 +0100 Subject: [PATCH 1/6] refactor(miniscript): Move keys to avoid copy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As done in other ctors. Co-authored-by: Lőrinc --- src/script/miniscript.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/script/miniscript.h b/src/script/miniscript.h index 1b7e84f471d0..a65900c78402 100644 --- a/src/script/miniscript.h +++ b/src/script/miniscript.h @@ -607,7 +607,7 @@ class Node // This is kept private as no valid fragment has all of these arguments. // Only used by Clone() Node(internal::NoDupCheck, MiniscriptContext script_ctx, enum Fragment nt, std::vector sub, std::vector key, std::vector arg, uint32_t val) - : fragment(nt), k(val), keys(key), data(std::move(arg)), subs(std::move(sub)), m_script_ctx{script_ctx}, ops(CalcOps()), ss(CalcStackSize()), ws(CalcWitnessSize()), typ(CalcType()), scriptlen(CalcScriptLen()) {} + : fragment(nt), k(val), keys(std::move(key)), data(std::move(arg)), subs(std::move(sub)), m_script_ctx{script_ctx}, ops(CalcOps()), ss(CalcStackSize()), ws(CalcWitnessSize()), typ(CalcType()), scriptlen(CalcScriptLen()) {} //! Compute the length of the script for this miniscript (including children). size_t CalcScriptLen() const From fd7c494c6bfa01a5582d60d234725c564507e9fa Mon Sep 17 00:00:00 2001 From: Hodlinator <172445034+hodlinator@users.noreply.github.com> Date: Thu, 5 Feb 2026 13:00:38 +0100 Subject: [PATCH 2/6] doc(miniscript): Explain why we operate on vectors Explains the reason behind 198bbaee4959119a63b4038cd0dbb519f4daf6f0 where we had earlier switched from operating on unique_ptr to plain Node. --- src/script/miniscript.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/script/miniscript.h b/src/script/miniscript.h index a65900c78402..4f0cf654351c 100644 --- a/src/script/miniscript.h +++ b/src/script/miniscript.h @@ -552,6 +552,9 @@ class Node // Destroy the subexpressions iteratively after moving out their // subexpressions to avoid a stack-overflow due to recursive calls to // the subs' destructors. + // We move vectors in order to only update array-pointers inside them + // rather than moving individual Node instances which would involve + // moving/copying each Node field. std::vector> queue; queue.push_back(std::move(subs)); do { From 5af5e87646ec436bd4c86a62f70c376edb309ea9 Mon Sep 17 00:00:00 2001 From: Hodlinator <172445034+hodlinator@users.noreply.github.com> Date: Wed, 4 Feb 2026 22:28:25 +0100 Subject: [PATCH 3/6] test(miniscript): Make tested script valid Also give more appropriate name to test. Co-authored-by: Antoine Poinsot --- src/test/miniscript_tests.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/test/miniscript_tests.cpp b/src/test/miniscript_tests.cpp index 757f89b63bc3..2bb96144b0a8 100644 --- a/src/test/miniscript_tests.cpp +++ b/src/test/miniscript_tests.cpp @@ -728,18 +728,18 @@ BOOST_AUTO_TEST_CASE(fixed_tests) } // Confirm that ~Node(), Node::Clone() and operator=(Node&&) are stack-safe. -BOOST_AUTO_TEST_CASE(node_deep_destruct) +BOOST_AUTO_TEST_CASE(node_stress_stack) { using miniscript::internal::NoDupCheck; using miniscript::Fragment; using NodeU32 = miniscript::Node; - constexpr auto ctx{miniscript::MiniscriptContext::P2WSH}; - + constexpr auto ctx{miniscript::MiniscriptContext::TAPSCRIPT}; NodeU32 root{NoDupCheck{}, ctx, Fragment::JUST_1}; for (uint32_t i{0}; i < 200'000; ++i) { - root = NodeU32{NoDupCheck{}, ctx, Fragment::WRAP_S, Vector(std::move(root))}; + root = NodeU32{NoDupCheck{}, ctx, Fragment::WRAP_N, Vector(std::move(root))}; } + BOOST_CHECK(root.IsValid()); BOOST_CHECK_EQUAL(root.ScriptSize(), 200'001); auto clone{root.Clone()}; From 39e3295c71035b960bc9c5d0eeeaed3e06e9d1a6 Mon Sep 17 00:00:00 2001 From: Hodlinator <172445034+hodlinator@users.noreply.github.com> Date: Thu, 5 Feb 2026 12:02:15 +0100 Subject: [PATCH 4/6] test(miniscript): Check for depth rather than script size MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI failure due to default ~Node() implementation has been confirmed on Windows native, MacOS native, 32-bit ARM, ASan+LSan+UBSan+integer, i686, TSan, MSan. (Not on Alpine as that runs without CI_LIMIT_STACK_SIZE). Co-authored-by: Antoine Poinsot Co-authored-by: Lőrinc --- src/test/miniscript_tests.cpp | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/src/test/miniscript_tests.cpp b/src/test/miniscript_tests.cpp index 2bb96144b0a8..b1cfad94fed9 100644 --- a/src/test/miniscript_tests.cpp +++ b/src/test/miniscript_tests.cpp @@ -734,18 +734,33 @@ BOOST_AUTO_TEST_CASE(node_stress_stack) using miniscript::Fragment; using NodeU32 = miniscript::Node; + const auto compute_depth{[] (const NodeU32& node) -> size_t { + size_t depth{0}; + for (const auto* n{&node}; !n->Subs().empty(); n = &n->Subs().front()) { + ++depth; + } + return depth; + }}; + constexpr auto ctx{miniscript::MiniscriptContext::TAPSCRIPT}; NodeU32 root{NoDupCheck{}, ctx, Fragment::JUST_1}; - for (uint32_t i{0}; i < 200'000; ++i) { + // Some CI jobs run with CI_LIMIT_STACK_SIZE which reduces the stack size + // via ulimit to 512 kbytes. When tested with ~Node()=default (stack-unsafe) + // implementations the test has been shown to fail for the below depth. + // The test may pass locally despite stack-unsafe implementations unless the + // stack is reduced in a similar way or the depth is temporarily increased. + constexpr size_t depth{200'000}; + for (size_t i{0}; i < depth; ++i) { root = NodeU32{NoDupCheck{}, ctx, Fragment::WRAP_N, Vector(std::move(root))}; } BOOST_CHECK(root.IsValid()); - BOOST_CHECK_EQUAL(root.ScriptSize(), 200'001); + BOOST_CHECK_EQUAL(compute_depth(root), depth); auto clone{root.Clone()}; - BOOST_CHECK_EQUAL(clone.ScriptSize(), root.ScriptSize()); + BOOST_CHECK_EQUAL(compute_depth(clone), depth); clone = std::move(root); + BOOST_CHECK_EQUAL(compute_depth(clone), depth); } BOOST_AUTO_TEST_SUITE_END() From 6b64b181d5a599e330067c8aa98719e9525b28b4 Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Wed, 18 Mar 2026 03:53:08 +0100 Subject: [PATCH 5/6] kernel: Add API function for getting a tx's nLockTime --- src/kernel/bitcoinkernel.cpp | 5 +++++ src/kernel/bitcoinkernel.h | 9 +++++++++ src/kernel/bitcoinkernel_wrapper.h | 5 +++++ src/test/kernel/test_kernel.cpp | 1 + 4 files changed, 20 insertions(+) diff --git a/src/kernel/bitcoinkernel.cpp b/src/kernel/bitcoinkernel.cpp index 09b4853fa91e..b0ce1721a752 100644 --- a/src/kernel/bitcoinkernel.cpp +++ b/src/kernel/bitcoinkernel.cpp @@ -533,6 +533,11 @@ const btck_TransactionInput* btck_transaction_get_input_at(const btck_Transactio return btck_TransactionInput::ref(&btck_Transaction::get(transaction)->vin[input_index]); } +uint32_t btck_transaction_get_locktime(const btck_Transaction* transaction) +{ + return btck_Transaction::get(transaction)->nLockTime; +} + const btck_Txid* btck_transaction_get_txid(const btck_Transaction* transaction) { return btck_Txid::ref(&btck_Transaction::get(transaction)->GetHash()); diff --git a/src/kernel/bitcoinkernel.h b/src/kernel/bitcoinkernel.h index 57c88c1279c8..1513ae3f7cd0 100644 --- a/src/kernel/bitcoinkernel.h +++ b/src/kernel/bitcoinkernel.h @@ -576,6 +576,15 @@ BITCOINKERNEL_API const btck_TransactionInput* BITCOINKERNEL_WARN_UNUSED_RESULT BITCOINKERNEL_API size_t BITCOINKERNEL_WARN_UNUSED_RESULT btck_transaction_count_inputs( const btck_Transaction* transaction) BITCOINKERNEL_ARG_NONNULL(1); +/** + * @brief Get a transaction's nLockTime value. + * + * @param[in] transaction Non-null. + * @return The nLockTime value. + */ +BITCOINKERNEL_API uint32_t BITCOINKERNEL_WARN_UNUSED_RESULT btck_transaction_get_locktime( + const btck_Transaction* transaction) BITCOINKERNEL_ARG_NONNULL(1); + /** * @brief Get the txid of a transaction. The returned txid is not owned and * depends on the lifetime of the transaction. diff --git a/src/kernel/bitcoinkernel_wrapper.h b/src/kernel/bitcoinkernel_wrapper.h index 064d0dd14b9d..83ec5d02c715 100644 --- a/src/kernel/bitcoinkernel_wrapper.h +++ b/src/kernel/bitcoinkernel_wrapper.h @@ -596,6 +596,11 @@ class TransactionApi return TransactionInputView{btck_transaction_get_input_at(impl(), index)}; } + uint32_t GetLocktime() const + { + return btck_transaction_get_locktime(impl()); + } + TxidView Txid() const { return TxidView{btck_transaction_get_txid(impl())}; diff --git a/src/test/kernel/test_kernel.cpp b/src/test/kernel/test_kernel.cpp index 4a97f5ba467b..ab5d694fa494 100644 --- a/src/test/kernel/test_kernel.cpp +++ b/src/test/kernel/test_kernel.cpp @@ -401,6 +401,7 @@ BOOST_AUTO_TEST_CASE(btck_transaction_tests) BOOST_CHECK_EQUAL(tx.CountOutputs(), 2); BOOST_CHECK_EQUAL(tx.CountInputs(), 1); + BOOST_CHECK_EQUAL(tx.GetLocktime(), 510826); auto broken_tx_data{std::span{tx_data.begin(), tx_data.begin() + 10}}; BOOST_CHECK_THROW(Transaction{broken_tx_data}, std::runtime_error); auto output{tx.GetOutput(tx.CountOutputs() - 1)}; From 9f28120a5bff605690b9cc6ac8da255bdafffe48 Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Wed, 18 Mar 2026 04:15:30 +0100 Subject: [PATCH 6/6] kernel: Add API function for getting a tx input's nSequence --- src/kernel/bitcoinkernel.cpp | 5 +++++ src/kernel/bitcoinkernel.h | 9 +++++++++ src/kernel/bitcoinkernel_wrapper.h | 5 +++++ src/test/kernel/test_kernel.cpp | 2 ++ 4 files changed, 21 insertions(+) diff --git a/src/kernel/bitcoinkernel.cpp b/src/kernel/bitcoinkernel.cpp index b0ce1721a752..4216bf921ff9 100644 --- a/src/kernel/bitcoinkernel.cpp +++ b/src/kernel/bitcoinkernel.cpp @@ -697,6 +697,11 @@ const btck_TransactionOutPoint* btck_transaction_input_get_out_point(const btck_ return btck_TransactionOutPoint::ref(&btck_TransactionInput::get(input).prevout); } +uint32_t btck_transaction_input_get_sequence(const btck_TransactionInput* input) +{ + return btck_TransactionInput::get(input).nSequence; +} + void btck_transaction_input_destroy(btck_TransactionInput* input) { delete input; diff --git a/src/kernel/bitcoinkernel.h b/src/kernel/bitcoinkernel.h index 1513ae3f7cd0..fe5bc3f91b54 100644 --- a/src/kernel/bitcoinkernel.h +++ b/src/kernel/bitcoinkernel.h @@ -1514,6 +1514,15 @@ BITCOINKERNEL_API btck_TransactionInput* BITCOINKERNEL_WARN_UNUSED_RESULT btck_t BITCOINKERNEL_API const btck_TransactionOutPoint* BITCOINKERNEL_WARN_UNUSED_RESULT btck_transaction_input_get_out_point( const btck_TransactionInput* transaction_input) BITCOINKERNEL_ARG_NONNULL(1); +/** + * @brief Get a transaction input's nSequence value. + * + * @param[in] transaction_input Non-null. + * @return The nSequence value. + */ +BITCOINKERNEL_API uint32_t BITCOINKERNEL_WARN_UNUSED_RESULT btck_transaction_input_get_sequence( + const btck_TransactionInput* transaction_input) BITCOINKERNEL_ARG_NONNULL(1); + /** * Destroy the transaction input. */ diff --git a/src/kernel/bitcoinkernel_wrapper.h b/src/kernel/bitcoinkernel_wrapper.h index 83ec5d02c715..fd2348578751 100644 --- a/src/kernel/bitcoinkernel_wrapper.h +++ b/src/kernel/bitcoinkernel_wrapper.h @@ -551,6 +551,11 @@ class TransactionInputApi { return OutPointView{btck_transaction_input_get_out_point(impl())}; } + + uint32_t GetSequence() const + { + return btck_transaction_input_get_sequence(impl()); + } }; class TransactionInputView : public View, public TransactionInputApi diff --git a/src/test/kernel/test_kernel.cpp b/src/test/kernel/test_kernel.cpp index ab5d694fa494..d4a8647b662a 100644 --- a/src/test/kernel/test_kernel.cpp +++ b/src/test/kernel/test_kernel.cpp @@ -404,6 +404,8 @@ BOOST_AUTO_TEST_CASE(btck_transaction_tests) BOOST_CHECK_EQUAL(tx.GetLocktime(), 510826); auto broken_tx_data{std::span{tx_data.begin(), tx_data.begin() + 10}}; BOOST_CHECK_THROW(Transaction{broken_tx_data}, std::runtime_error); + auto input{tx.GetInput(0)}; + BOOST_CHECK_EQUAL(input.GetSequence(), 0xfffffffe); auto output{tx.GetOutput(tx.CountOutputs() - 1)}; BOOST_CHECK_EQUAL(output.Amount(), 42130042); auto script_pubkey{output.GetScriptPubkey()};