wallet: warn against accidental unsafe older() import

BIP 379 allows height and time locks that have no consensus meaning in BIP 68 / BIP 112.
This is used by some protocols like Lightning to encode extra data, but is unsafe when
used unintentionally. E.g. older(65536) is equivalent to older(1).

This commit emits a warning when importing such a descriptor.

It introduces a helper ForEachNode to traverse all miniscript nodes.
This commit is contained in:
Sjors Provoost
2025-12-02 12:24:22 +01:00
parent 592157b759
commit 76c092ff80
7 changed files with 138 additions and 1 deletions

View File

@@ -791,6 +791,8 @@ protected:
const std::vector<std::unique_ptr<PubkeyProvider>> m_pubkey_args;
//! The string name of the descriptor function.
const std::string m_name;
//! Warnings (not including subdescriptors).
std::vector<std::string> m_warnings;
//! The sub-descriptor arguments (empty for everything but SH and WSH).
//! In doc/descriptors.m this is referred to as SCRIPT expressions sh(SCRIPT)
@@ -990,6 +992,16 @@ public:
}
virtual std::unique_ptr<DescriptorImpl> Clone() const = 0;
// NOLINTNEXTLINE(misc-no-recursion)
std::vector<std::string> Warnings() const override {
std::vector<std::string> all = m_warnings;
for (const auto& sub : m_subdescriptor_args) {
auto sub_w = sub->Warnings();
all.insert(all.end(), sub_w.begin(), sub_w.end());
}
return all;
}
};
/** A parsed addr(A) descriptor. */
@@ -1537,7 +1549,24 @@ protected:
public:
MiniscriptDescriptor(std::vector<std::unique_ptr<PubkeyProvider>> providers, miniscript::NodeRef<uint32_t> node)
: DescriptorImpl(std::move(providers), "?"), m_node(std::move(node)) {}
: DescriptorImpl(std::move(providers), "?"), m_node(std::move(node))
{
// Traverse miniscript tree for unsafe use of older()
miniscript::ForEachNode(*m_node, [&](const miniscript::Node<uint32_t>& node) {
if (node.fragment == miniscript::Fragment::OLDER) {
const uint32_t raw = node.k;
const uint32_t value_part = raw & ~CTxIn::SEQUENCE_LOCKTIME_TYPE_FLAG;
if (value_part > CTxIn::SEQUENCE_LOCKTIME_MASK) {
const bool is_time_based = (raw & CTxIn::SEQUENCE_LOCKTIME_TYPE_FLAG) != 0;
if (is_time_based) {
m_warnings.push_back(strprintf("time-based relative locktime: older(%u) > (65535 * 512) seconds is unsafe", raw));
} else {
m_warnings.push_back(strprintf("height-based relative locktime: older(%u) > 65535 blocks is unsafe", raw));
}
}
}
});
}
bool ToStringHelper(const SigningProvider* arg, std::string& out, const StringType type,
const DescriptorCache* cache = nullptr) const override

View File

@@ -165,6 +165,9 @@ struct Descriptor {
* @param[out] ext_pubs Any extended public keys
*/
virtual void GetPubKeys(std::set<CPubKey>& pubkeys, std::set<CExtPubKey>& ext_pubs) const = 0;
/** Semantic/safety warnings (includes subdescriptors). */
virtual std::vector<std::string> Warnings() const = 0;
};
/** Parse a `descriptor` string. Included private keys are put in `out`.

View File

@@ -7,8 +7,10 @@
#include <algorithm>
#include <compare>
#include <concepts>
#include <cstdint>
#include <cstdlib>
#include <functional>
#include <iterator>
#include <memory>
#include <optional>
@@ -195,6 +197,21 @@ template<typename Key> using NodeRef = std::unique_ptr<const Node<Key>>;
template<typename Key, typename... Args>
NodeRef<Key> MakeNodeRef(Args&&... args) { return std::make_unique<const Node<Key>>(std::forward<Args>(args)...); }
//! Unordered traversal of a miniscript node tree.
template <typename Key, std::invocable<const Node<Key>&> Fn>
void ForEachNode(const Node<Key>& root, Fn&& fn)
{
std::vector<std::reference_wrapper<const Node<Key>>> stack{root};
while (!stack.empty()) {
const Node<Key>& node = stack.back();
std::invoke(fn, node);
stack.pop_back();
for (const auto& sub : node.subs) {
stack.emplace_back(*sub);
}
}
}
//! The different node types in miniscript.
enum class Fragment {
JUST_0, //!< OP_0