test(miniscript): Prove avoidance of stack overflow
What the developer wrote
Authored by Hodlinator
test(miniscript): Prove avoidance of stack overflow
Can be tested through emptying the function body of ~Node() or replacing Clone() implementation with naive version:
```C++
Node<Key> Clone() const
{
std::vector<Node> new_subs;
new_subs.reserve(subs.size());
for (const Node& child : subs) {
new_subs.push_back(child.Clone());
}
return Node{internal::NoDupCheck{}, m_script_ctx, fragment, std::move(new_subs), keys, data, k};
}
```
Co-authored-by: Lőrinc <pap.lorinc@gmail.com>
What changed, and why it matters
This commit only adds a new automated test to Bitcoin Core. The test builds an extremely deep Miniscript node tree and then exercises its destructor, clone, and move-assignment to confirm they do not overflow the call stack. It is a defensive regression test, not a fix for any currently broken or exploitable behavior.
No action required. This is a test-only addition. Reviewers may verify that the test passes in CI and that the existing ~Node(), Clone(), and move-assignment implementations are indeed non-recursive or tail-call bounded.
Security signals we found
Defensive regression test for stack overflow in deep Miniscript trees
Tests destructor, clone, and move-assignment stack safety
No production code changes
Evidence from the diff
The diff adds BOOST_AUTO_TEST_CASE(node_deep_destruct) in src/test/miniscript_tests.cpp. It constructs a chain of 200,000 WRAP_S fragments, producing a tree depth of 200,000, then calls Clone() and move-assignment. The intent is to guard against stack-unsafe recursive implementations of ~Node(), Clone(), and operator=(Node&&). The commit message explicitly frames this as proving avoidance of stack overflow and notes how to reproduce a failure by intentionally replacing the implementations with naive recursive versions.
Changed components
src/test/miniscript_tests.cppInspect captured patch +21 / −0
diff --git a/src/test/miniscript_tests.cpp b/src/test/miniscript_tests.cpp
index 0cd5b62f..757f89b6 100644
--- a/src/test/miniscript_tests.cpp
+++ b/src/test/miniscript_tests.cpp
@@ -727,4 +727,25 @@ BOOST_AUTO_TEST_CASE(fixed_tests)
g_testdata.reset();
}
+// Confirm that ~Node(), Node::Clone() and operator=(Node&&) are stack-safe.
+BOOST_AUTO_TEST_CASE(node_deep_destruct)
+{
+ using miniscript::internal::NoDupCheck;
+ using miniscript::Fragment;
+ using NodeU32 = miniscript::Node<uint32_t>;
+
+ constexpr auto ctx{miniscript::MiniscriptContext::P2WSH};
+
+ 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))};
+ }
+ BOOST_CHECK_EQUAL(root.ScriptSize(), 200'001);
+
+ auto clone{root.Clone()};
+ BOOST_CHECK_EQUAL(clone.ScriptSize(), root.ScriptSize());
+
+ clone = std::move(root);
+}
+
BOOST_AUTO_TEST_SUITE_END()
Why this scored 12/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.