phantom/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-04-29 14:59:39 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions 1

Merge bitcoin/bitcoin#31727: miniscript: convert non-critical asserts to CHECK_NONFATAL
Some checks are pending
CI / test each commit (push) Waiting to run
Details
CI / macOS 14 native, arm64, no depends, sqlite only, gui (push) Waiting to run
Details
CI / macOS 14 native, arm64, fuzz (push) Waiting to run
Details
CI / Windows native, VS 2022 (push) Waiting to run
Details
CI / Windows native, fuzz, VS 2022 (push) Waiting to run
Details
CI / Linux->Windows cross, no tests (push) Waiting to run
Details
CI / Windows, test cross-built (push) Blocked by required conditions
Details
CI / ASan + LSan + UBSan + integer, no depends, USDT (push) Waiting to run
Details

Browse source 
ff0194a7ce miniscript: convert non-critical asserts to CHECK_NONFATAL (Antoine Poinsot)

Pull request description:

  The Miniscript code contains assertions to prevent ending up in an insane state or prevent UB, but also to enforce logical invariants. For the latter it is not necessary to crash the program if they are broken. Raising an exception suffices, especially as this code is often called through the RPC interface which can in turn handle the exception and the user can report it to developers.

  This revives #28678 from Pieter Wuille.

ACKs for top commit:
  hodlinator:
    ACK ff0194a7ce
  TheCharlatan:
    ACK ff0194a7ce
  brunoerg:
    code review ACK ff0194a7ce

Tree-SHA512: 8ed8f7b494e46ecf7cdebe75120cd0ffe543b6bc289bf882dac631fe2ec2cae590d5f7bc2316e52db085791694b136dffbc71c40c1e16886fa53ab00bd8cabd0
This commit is contained in:
merge-script 2025-04-10 14:06:21 -04:00
commit a4fd565191
No known key found for this signature in database
GPG key ID: BA03F4DBE0C63FB4
2 changed files with 54 additions and 51 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

62
src/script/miniscript.cpp
View file

@ -19,20 +19,20 @@ namespace internal {
Type SanitizeType(Type e) { Type SanitizeType(Type e) {
int num_types = (e << "K"_mst) + (e << "V"_mst) + (e << "B"_mst) + (e << "W"_mst); int num_types = (e << "K"_mst) + (e << "V"_mst) + (e << "B"_mst) + (e << "W"_mst);
if (num_types == 0) return ""_mst; // No valid type, don't care about the rest if (num_types == 0) return ""_mst; // No valid type, don't care about the rest
assert(num_types == 1); // K, V, B, W all conflict with each other CHECK_NONFATAL(num_types == 1); // K, V, B, W all conflict with each other
assert(!(e << "z"_mst) || !(e << "o"_mst)); // z conflicts with o CHECK_NONFATAL(!(e << "z"_mst) || !(e << "o"_mst)); // z conflicts with o
assert(!(e << "n"_mst) || !(e << "z"_mst)); // n conflicts with z CHECK_NONFATAL(!(e << "n"_mst) || !(e << "z"_mst)); // n conflicts with z
assert(!(e << "n"_mst) || !(e << "W"_mst)); // n conflicts with W CHECK_NONFATAL(!(e << "n"_mst) || !(e << "W"_mst)); // n conflicts with W
assert(!(e << "V"_mst) || !(e << "d"_mst)); // V conflicts with d CHECK_NONFATAL(!(e << "V"_mst) || !(e << "d"_mst)); // V conflicts with d
assert(!(e << "K"_mst) || (e << "u"_mst)); // K implies u CHECK_NONFATAL(!(e << "K"_mst) || (e << "u"_mst)); // K implies u
assert(!(e << "V"_mst) || !(e << "u"_mst)); // V conflicts with u CHECK_NONFATAL(!(e << "V"_mst) || !(e << "u"_mst)); // V conflicts with u
assert(!(e << "e"_mst) || !(e << "f"_mst)); // e conflicts with f CHECK_NONFATAL(!(e << "e"_mst) || !(e << "f"_mst)); // e conflicts with f
assert(!(e << "e"_mst) || (e << "d"_mst)); // e implies d CHECK_NONFATAL(!(e << "e"_mst) || (e << "d"_mst)); // e implies d
assert(!(e << "V"_mst) || !(e << "e"_mst)); // V conflicts with e CHECK_NONFATAL(!(e << "V"_mst) || !(e << "e"_mst)); // V conflicts with e
assert(!(e << "d"_mst) || !(e << "f"_mst)); // d conflicts with f CHECK_NONFATAL(!(e << "d"_mst) || !(e << "f"_mst)); // d conflicts with f
assert(!(e << "V"_mst) || (e << "f"_mst)); // V implies f CHECK_NONFATAL(!(e << "V"_mst) || (e << "f"_mst)); // V implies f
assert(!(e << "K"_mst) || (e << "s"_mst)); // K implies s CHECK_NONFATAL(!(e << "K"_mst) || (e << "s"_mst)); // K implies s
assert(!(e << "z"_mst) || (e << "m"_mst)); // z implies m CHECK_NONFATAL(!(e << "z"_mst) || (e << "m"_mst)); // z implies m
return e; return e;
} }
@ -40,46 +40,46 @@ Type ComputeType(Fragment fragment, Type x, Type y, Type z, const std::vector<Ty
size_t data_size, size_t n_subs, size_t n_keys, MiniscriptContext ms_ctx) { size_t data_size, size_t n_subs, size_t n_keys, MiniscriptContext ms_ctx) {
// Sanity check on data // Sanity check on data
if (fragment == Fragment::SHA256 || fragment == Fragment::HASH256) { if (fragment == Fragment::SHA256 || fragment == Fragment::HASH256) {
assert(data_size == 32); CHECK_NONFATAL(data_size == 32);
} else if (fragment == Fragment::RIPEMD160 || fragment == Fragment::HASH160) { } else if (fragment == Fragment::RIPEMD160 || fragment == Fragment::HASH160) {
assert(data_size == 20); CHECK_NONFATAL(data_size == 20);
} else { } else {
assert(data_size == 0); CHECK_NONFATAL(data_size == 0);
} }
// Sanity check on k // Sanity check on k
if (fragment == Fragment::OLDER || fragment == Fragment::AFTER) { if (fragment == Fragment::OLDER || fragment == Fragment::AFTER) {
assert(k >= 1 && k < 0x80000000UL); CHECK_NONFATAL(k >= 1 && k < 0x80000000UL);
} else if (fragment == Fragment::MULTI || fragment == Fragment::MULTI_A) { } else if (fragment == Fragment::MULTI || fragment == Fragment::MULTI_A) {
assert(k >= 1 && k <= n_keys); CHECK_NONFATAL(k >= 1 && k <= n_keys);
} else if (fragment == Fragment::THRESH) { } else if (fragment == Fragment::THRESH) {
assert(k >= 1 && k <= n_subs); CHECK_NONFATAL(k >= 1 && k <= n_subs);
} else { } else {
assert(k == 0); CHECK_NONFATAL(k == 0);
} }
// Sanity check on subs // Sanity check on subs
if (fragment == Fragment::AND_V || fragment == Fragment::AND_B || fragment == Fragment::OR_B || if (fragment == Fragment::AND_V || fragment == Fragment::AND_B || fragment == Fragment::OR_B ||
fragment == Fragment::OR_C || fragment == Fragment::OR_I || fragment == Fragment::OR_D) { fragment == Fragment::OR_C || fragment == Fragment::OR_I || fragment == Fragment::OR_D) {
assert(n_subs == 2); CHECK_NONFATAL(n_subs == 2);
} else if (fragment == Fragment::ANDOR) { } else if (fragment == Fragment::ANDOR) {
assert(n_subs == 3); CHECK_NONFATAL(n_subs == 3);
} else if (fragment == Fragment::WRAP_A || fragment == Fragment::WRAP_S || fragment == Fragment::WRAP_C || } else if (fragment == Fragment::WRAP_A || fragment == Fragment::WRAP_S || fragment == Fragment::WRAP_C ||
fragment == Fragment::WRAP_D || fragment == Fragment::WRAP_V || fragment == Fragment::WRAP_J || fragment == Fragment::WRAP_D || fragment == Fragment::WRAP_V || fragment == Fragment::WRAP_J ||
fragment == Fragment::WRAP_N) { fragment == Fragment::WRAP_N) {
assert(n_subs == 1); CHECK_NONFATAL(n_subs == 1);
} else if (fragment != Fragment::THRESH) { } else if (fragment != Fragment::THRESH) {
assert(n_subs == 0); CHECK_NONFATAL(n_subs == 0);
} }
// Sanity check on keys // Sanity check on keys
if (fragment == Fragment::PK_K || fragment == Fragment::PK_H) { if (fragment == Fragment::PK_K || fragment == Fragment::PK_H) {
assert(n_keys == 1); CHECK_NONFATAL(n_keys == 1);
} else if (fragment == Fragment::MULTI) { } else if (fragment == Fragment::MULTI) {
assert(n_keys >= 1 && n_keys <= MAX_PUBKEYS_PER_MULTISIG); CHECK_NONFATAL(n_keys >= 1 && n_keys <= MAX_PUBKEYS_PER_MULTISIG);
assert(!IsTapscript(ms_ctx)); CHECK_NONFATAL(!IsTapscript(ms_ctx));
} else if (fragment == Fragment::MULTI_A) { } else if (fragment == Fragment::MULTI_A) {
assert(n_keys >= 1 && n_keys <= MAX_PUBKEYS_PER_MULTI_A); CHECK_NONFATAL(n_keys >= 1 && n_keys <= MAX_PUBKEYS_PER_MULTI_A);
assert(IsTapscript(ms_ctx)); CHECK_NONFATAL(IsTapscript(ms_ctx));
} else { } else {
assert(n_keys == 0); CHECK_NONFATAL(n_keys == 0);
} }
// Below is the per-fragment logic for computing the expression types. // Below is the per-fragment logic for computing the expression types.

43
src/script/miniscript.h
View file

@ -659,7 +659,8 @@ private:
stack.pop_back(); stack.pop_back();
} }
// The final remaining results element is the root result, return it. // The final remaining results element is the root result, return it.
assert(results.size() == 1); assert(results.size() >= 1);
CHECK_NONFATAL(results.size() == 1);
return std::move(results[0]); return std::move(results[0]);
} }
@ -1225,7 +1226,7 @@ private:
// The dissatisfaction consists of as many empty vectors as there are keys, which is the same as // The dissatisfaction consists of as many empty vectors as there are keys, which is the same as
// satisfying 0 keys. // satisfying 0 keys.
auto& nsat{sats[0]}; auto& nsat{sats[0]};
assert(node.k != 0); CHECK_NONFATAL(node.k != 0);
assert(node.k <= sats.size()); assert(node.k <= sats.size());
return {std::move(nsat), std::move(sats[node.k])}; return {std::move(nsat), std::move(sats[node.k])};
} }
@ -1392,38 +1393,38 @@ private:
// (the actual satisfaction code in ProduceInputHelper does not use GetType) // (the actual satisfaction code in ProduceInputHelper does not use GetType)
// For 'z' nodes, available satisfactions/dissatisfactions must have stack size 0. // For 'z' nodes, available satisfactions/dissatisfactions must have stack size 0.
if (node.GetType() << "z"_mst && ret.nsat.available != Availability::NO) assert(ret.nsat.stack.size() == 0); if (node.GetType() << "z"_mst && ret.nsat.available != Availability::NO) CHECK_NONFATAL(ret.nsat.stack.size() == 0);
if (node.GetType() << "z"_mst && ret.sat.available != Availability::NO) assert(ret.sat.stack.size() == 0); if (node.GetType() << "z"_mst && ret.sat.available != Availability::NO) CHECK_NONFATAL(ret.sat.stack.size() == 0);
// For 'o' nodes, available satisfactions/dissatisfactions must have stack size 1. // For 'o' nodes, available satisfactions/dissatisfactions must have stack size 1.
if (node.GetType() << "o"_mst && ret.nsat.available != Availability::NO) assert(ret.nsat.stack.size() == 1); if (node.GetType() << "o"_mst && ret.nsat.available != Availability::NO) CHECK_NONFATAL(ret.nsat.stack.size() == 1);
if (node.GetType() << "o"_mst && ret.sat.available != Availability::NO) assert(ret.sat.stack.size() == 1); if (node.GetType() << "o"_mst && ret.sat.available != Availability::NO) CHECK_NONFATAL(ret.sat.stack.size() == 1);
// For 'n' nodes, available satisfactions/dissatisfactions must have stack size 1 or larger. For satisfactions, // For 'n' nodes, available satisfactions/dissatisfactions must have stack size 1 or larger. For satisfactions,
// the top element cannot be 0. // the top element cannot be 0.
if (node.GetType() << "n"_mst && ret.sat.available != Availability::NO) assert(ret.sat.stack.size() >= 1); if (node.GetType() << "n"_mst && ret.sat.available != Availability::NO) CHECK_NONFATAL(ret.sat.stack.size() >= 1);
if (node.GetType() << "n"_mst && ret.nsat.available != Availability::NO) assert(ret.nsat.stack.size() >= 1); if (node.GetType() << "n"_mst && ret.nsat.available != Availability::NO) CHECK_NONFATAL(ret.nsat.stack.size() >= 1);
if (node.GetType() << "n"_mst && ret.sat.available != Availability::NO) assert(!ret.sat.stack.back().empty()); if (node.GetType() << "n"_mst && ret.sat.available != Availability::NO) CHECK_NONFATAL(!ret.sat.stack.back().empty());
// For 'd' nodes, a dissatisfaction must exist, and they must not need a signature. If it is non-malleable, // For 'd' nodes, a dissatisfaction must exist, and they must not need a signature. If it is non-malleable,
// it must be canonical. // it must be canonical.
if (node.GetType() << "d"_mst) assert(ret.nsat.available != Availability::NO); if (node.GetType() << "d"_mst) CHECK_NONFATAL(ret.nsat.available != Availability::NO);
if (node.GetType() << "d"_mst) assert(!ret.nsat.has_sig); if (node.GetType() << "d"_mst) CHECK_NONFATAL(!ret.nsat.has_sig);
if (node.GetType() << "d"_mst && !ret.nsat.malleable) assert(!ret.nsat.non_canon); if (node.GetType() << "d"_mst && !ret.nsat.malleable) CHECK_NONFATAL(!ret.nsat.non_canon);
// For 'f'/'s' nodes, dissatisfactions/satisfactions must have a signature. // For 'f'/'s' nodes, dissatisfactions/satisfactions must have a signature.
if (node.GetType() << "f"_mst && ret.nsat.available != Availability::NO) assert(ret.nsat.has_sig); if (node.GetType() << "f"_mst && ret.nsat.available != Availability::NO) CHECK_NONFATAL(ret.nsat.has_sig);
if (node.GetType() << "s"_mst && ret.sat.available != Availability::NO) assert(ret.sat.has_sig); if (node.GetType() << "s"_mst && ret.sat.available != Availability::NO) CHECK_NONFATAL(ret.sat.has_sig);
// For non-malleable 'e' nodes, a non-malleable dissatisfaction must exist. // For non-malleable 'e' nodes, a non-malleable dissatisfaction must exist.
if (node.GetType() << "me"_mst) assert(ret.nsat.available != Availability::NO); if (node.GetType() << "me"_mst) CHECK_NONFATAL(ret.nsat.available != Availability::NO);
if (node.GetType() << "me"_mst) assert(!ret.nsat.malleable); if (node.GetType() << "me"_mst) CHECK_NONFATAL(!ret.nsat.malleable);
// For 'm' nodes, if a satisfaction exists, it must be non-malleable. // For 'm' nodes, if a satisfaction exists, it must be non-malleable.
if (node.GetType() << "m"_mst && ret.sat.available != Availability::NO) assert(!ret.sat.malleable); if (node.GetType() << "m"_mst && ret.sat.available != Availability::NO) CHECK_NONFATAL(!ret.sat.malleable);
// If a non-malleable satisfaction exists, it must be canonical. // If a non-malleable satisfaction exists, it must be canonical.
if (ret.sat.available != Availability::NO && !ret.sat.malleable) assert(!ret.sat.non_canon); if (ret.sat.available != Availability::NO && !ret.sat.malleable) CHECK_NONFATAL(!ret.sat.non_canon);
return ret; return ret;
}; };
@ -1604,7 +1605,8 @@ public:
case Fragment::THRESH: case Fragment::THRESH:
return static_cast<uint32_t>(std::count(subs.begin(), subs.end(), true)) >= node.k; return static_cast<uint32_t>(std::count(subs.begin(), subs.end(), true)) >= node.k;
default: // wrappers default: // wrappers
assert(subs.size() == 1); assert(subs.size() >= 1);
CHECK_NONFATAL(subs.size() == 1);
return subs[0]; return subs[0];
} }
}); });
@ -2157,7 +2159,8 @@ inline NodeRef<Key> Parse(std::span<const char> in, const Ctx& ctx)
} }
// Sanity checks on the produced miniscript // Sanity checks on the produced miniscript
assert(constructed.size() == 1); assert(constructed.size() >= 1);
CHECK_NONFATAL(constructed.size() == 1);
assert(constructed[0]->ScriptSize() == script_size); assert(constructed[0]->ScriptSize() == script_size);
if (in.size() > 0) return {}; if (in.size() > 0) return {};
NodeRef<Key> tl_node = std::move(constructed.front()); NodeRef<Key> tl_node = std::move(constructed.front());