miniscript: mark nodes with duplicate keys as insane

As stated on the website, duplicate keys make it hard to reason about
malleability as a single signature may unlock multiple paths.

We use a custom KeyCompare function instead of operator< to be explicit
about the requirement.
This commit is contained in:
Antoine Poinsot
2022-04-14 19:01:26 +02:00
parent 8c0f8bf7bc
commit 7a549c6c59
3 changed files with 150 additions and 69 deletions

View File

@@ -71,6 +71,10 @@ std::unique_ptr<const TestData> g_testdata;
struct KeyConverter {
typedef CPubKey Key;
bool KeyCompare(const Key& a, const Key& b) const {
return a < b;
}
//! Convert a public key to bytes.
std::vector<unsigned char> ToPKBytes(const CPubKey& key) const { return {key.begin(), key.end()}; }
@@ -273,6 +277,19 @@ BOOST_AUTO_TEST_CASE(fixed_tests)
// its subs to all be 'u' (taken from https://github.com/rust-bitcoin/rust-miniscript/discussions/341).
const auto ms_minimalif = miniscript::FromString("thresh(3,c:pk_k(03d30199d74fb5a22d47b6e054e2f378cedacffcb89904a61d75d0dbd407143e65),sc:pk_k(03fff97bd5755eeea420453a14355235d382f6472f8568a18b2f057a1460297556),sc:pk_k(0279be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798),sdv:older(32))", CONVERTER);
BOOST_CHECK(!ms_minimalif);
// A Miniscript with duplicate keys is not sane
const auto ms_dup1 = miniscript::FromString("and_v(v:pk(03d30199d74fb5a22d47b6e054e2f378cedacffcb89904a61d75d0dbd407143e65),pk(03d30199d74fb5a22d47b6e054e2f378cedacffcb89904a61d75d0dbd407143e65))", CONVERTER);
BOOST_CHECK(ms_dup1);
BOOST_CHECK(!ms_dup1->IsSane() && !ms_dup1->CheckDuplicateKey());
// Same with a disjunction, and different key nodes (pk and pkh)
const auto ms_dup2 = miniscript::FromString("or_b(c:pk_k(03d30199d74fb5a22d47b6e054e2f378cedacffcb89904a61d75d0dbd407143e65),ac:pk_h(03d30199d74fb5a22d47b6e054e2f378cedacffcb89904a61d75d0dbd407143e65))", CONVERTER);
BOOST_CHECK(ms_dup2 && !ms_dup2->IsSane() && !ms_dup2->CheckDuplicateKey());
// Same when the duplicates are leaves or a larger tree
const auto ms_dup3 = miniscript::FromString("or_i(and_b(pk(03d30199d74fb5a22d47b6e054e2f378cedacffcb89904a61d75d0dbd407143e65),s:pk(03fff97bd5755eeea420453a14355235d382f6472f8568a18b2f057a1460297556)),and_b(older(1),s:pk(03d30199d74fb5a22d47b6e054e2f378cedacffcb89904a61d75d0dbd407143e65)))", CONVERTER);
BOOST_CHECK(ms_dup3 && !ms_dup3->IsSane() && !ms_dup3->CheckDuplicateKey());
// Same when the duplicates are on different levels in the tree
const auto ms_dup4 = miniscript::FromString("thresh(2,pkh(03d30199d74fb5a22d47b6e054e2f378cedacffcb89904a61d75d0dbd407143e65),s:pk(03fff97bd5755eeea420453a14355235d382f6472f8568a18b2f057a1460297556),a:and_b(dv:older(1),s:pk(03d30199d74fb5a22d47b6e054e2f378cedacffcb89904a61d75d0dbd407143e65)))", CONVERTER);
BOOST_CHECK(ms_dup4 && !ms_dup4->IsSane() && !ms_dup4->CheckDuplicateKey());
// Timelock tests
Test("after(100)", "?", TESTMODE_VALID | TESTMODE_NONMAL); // only heightlock