highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2026-01-21 07:39:08 +01:00
Code Issues Packages Projects Releases Wiki Activity

Merge bitcoin/bitcoin#28651: Make miniscript GetWitnessSize accurate for tapscript

Browse Source 
b22810887b miniscript: make GetWitnessSize accurate for tapscript (Pieter Wuille)
8be9851408 test: add tests for miniscript GetWitnessSize (Pieter Wuille)
7ed2b2d430 test: remove mutable global contexts in miniscript fuzzer/test (Pieter Wuille)

Pull request description:

  So far, the same algorithm is used to compute an (upper bound on) the maximum witness size for both P2WSH and P2TR miniscript. That's unfortunate, because it means fee estimations for P2TR miniscript will miss out on the generic savings brought by P2TR witnesses (smaller signatures and public keys, specifically).

  Fix this by making the algorithm use script context specification calculations, and add tests for it. Also included is a cleanup for the tests to avoid mutable globals, as I found it hard to reason about what exactly was being tested.

ACKs for top commit:
  achow101:
    ACK b22810887b
  darosior:
    ACK b22810887b

Tree-SHA512: e4bda7376628f3e91cfc74917cefc554ca16eb5f2a0e1adddc33eb8717c4aaa071e56a40f85a2041ae74ec445a7bd0129bba48994c203e0e6e4d25af65954d9e
This commit is contained in:
Andrew Chow
2023-10-17 18:22:29 -04:00
parent 9c30f5ef9d b22810887b
commit c2d4e40e45
3 changed files with 124 additions and 101 deletions
Download Patch File Download Diff File Expand all files Collapse all files

61
src/test/fuzz/miniscript.cpp
View File

@@ -70,6 +70,7 @@ struct TestData {
sig.push_back(1); // SIGHASH_ALL
dummy_sigs.insert({pubkey, {sig, i & 1}});
assert(privkey.SignSchnorr(MESSAGE_HASH, schnorr_sig, nullptr, EMPTY_AUX));
schnorr_sig.push_back(1); // Maximally-sized signature has sighash byte
schnorr_sigs.emplace(XOnlyPubKey{pubkey}, std::make_pair(std::move(schnorr_sig), i & 1));
std::vector<unsigned char> hash;
@@ -113,7 +114,9 @@ struct TestData {
struct ParserContext {
typedef CPubKey Key;
MsCtx script_ctx{MsCtx::P2WSH};
const MsCtx script_ctx;
constexpr ParserContext(MsCtx ctx) noexcept : script_ctx(ctx) {}
bool KeyCompare(const Key& a, const Key& b) const {
return a < b;
@@ -178,11 +181,13 @@ struct ParserContext {
MsCtx MsContext() const {
return script_ctx;
}
} PARSER_CTX;
};
//! Context that implements naive conversion from/to script only, for roundtrip testing.
struct ScriptParserContext {
MsCtx script_ctx{MsCtx::P2WSH};
const MsCtx script_ctx;
constexpr ScriptParserContext(MsCtx ctx) noexcept : script_ctx(ctx) {}
//! For Script roundtrip we never need the key from a key hash.
struct Key {
@@ -228,10 +233,13 @@ struct ScriptParserContext {
MsCtx MsContext() const {
return script_ctx;
}
} SCRIPT_PARSER_CONTEXT;
};
//! Context to produce a satisfaction for a Miniscript node using the pre-computed data.
struct SatisfierContext: ParserContext {
struct SatisfierContext : ParserContext {
constexpr SatisfierContext(MsCtx ctx) noexcept : ParserContext(ctx) {}
// Timelock challenges satisfaction. Make the value (deterministically) vary to explore different
// paths.
bool CheckAfter(uint32_t value) const { return value % 2; }
@@ -267,12 +275,10 @@ struct SatisfierContext: ParserContext {
miniscript::Availability SatHASH160(const std::vector<unsigned char>& hash, std::vector<unsigned char>& preimage) const {
return LookupHash(hash, preimage, TEST_DATA.hash160_preimages);
}
} SATISFIER_CTX;
};
//! Context to check a satisfaction against the pre-computed data.
struct CheckerContext: BaseSignatureChecker {
TestData *test_data;
const struct CheckerContext: BaseSignatureChecker {
// Signature checker methods. Checks the right dummy signature is used.
bool CheckECDSASignature(const std::vector<unsigned char>& sig, const std::vector<unsigned char>& vchPubKey,
const CScript& scriptCode, SigVersion sigversion) const override
@@ -294,7 +300,7 @@ struct CheckerContext: BaseSignatureChecker {
} CHECKER_CTX;
//! Context to check for duplicates when instancing a Node.
struct KeyComparator {
const struct KeyComparator {
bool KeyCompare(const CPubKey& a, const CPubKey& b) const {
return a < b;
}
@@ -1027,15 +1033,15 @@ void TestNode(const MsCtx script_ctx, const NodeRef& node, FuzzedDataProvider& p
if (!node) return;
// Check that it roundtrips to text representation
PARSER_CTX.script_ctx = script_ctx;
std::optional<std::string> str{node->ToString(PARSER_CTX)};
const ParserContext parser_ctx{script_ctx};
std::optional<std::string> str{node->ToString(parser_ctx)};
assert(str);
auto parsed = miniscript::FromString(*str, PARSER_CTX);
auto parsed = miniscript::FromString(*str, parser_ctx);
assert(parsed);
assert(*parsed == *node);
// Check consistency between script size estimation and real size.
auto script = node->ToScript(PARSER_CTX);
auto script = node->ToScript(parser_ctx);
assert(node->ScriptSize() == script.size());
// Check consistency of "x" property with the script (type K is excluded, because it can end
@@ -1049,12 +1055,12 @@ void TestNode(const MsCtx script_ctx, const NodeRef& node, FuzzedDataProvider& p
if (!node->IsValidTopLevel()) return;
// Check roundtrip to script
auto decoded = miniscript::FromScript(script, PARSER_CTX);
auto decoded = miniscript::FromScript(script, parser_ctx);
assert(decoded);
// Note we can't use *decoded == *node because the miniscript representation may differ, so we check that:
// - The script corresponding to that decoded form matches exactly
// - The type matches exactly
assert(decoded->ToScript(PARSER_CTX) == script);
assert(decoded->ToScript(parser_ctx) == script);
assert(decoded->GetType() == node->GetType());
// Optionally pad the script or the witness in order to increase the sensitivity of the tests of
@@ -1091,7 +1097,7 @@ void TestNode(const MsCtx script_ctx, const NodeRef& node, FuzzedDataProvider& p
}
}
SATISFIER_CTX.script_ctx = script_ctx;
const SatisfierContext satisfier_ctx{script_ctx};
// Get the ScriptPubKey for this script, filling spend data if it's Taproot.
TaprootBuilder builder;
@@ -1099,11 +1105,11 @@ void TestNode(const MsCtx script_ctx, const NodeRef& node, FuzzedDataProvider& p
// Run malleable satisfaction algorithm.
std::vector<std::vector<unsigned char>> stack_mal;
const bool mal_success = node->Satisfy(SATISFIER_CTX, stack_mal, false) == miniscript::Availability::YES;
const bool mal_success = node->Satisfy(satisfier_ctx, stack_mal, false) == miniscript::Availability::YES;
// Run non-malleable satisfaction algorithm.
std::vector<std::vector<unsigned char>> stack_nonmal;
const bool nonmal_success = node->Satisfy(SATISFIER_CTX, stack_nonmal, true) == miniscript::Availability::YES;
const bool nonmal_success = node->Satisfy(satisfier_ctx, stack_nonmal, true) == miniscript::Availability::YES;
if (nonmal_success) {
// Non-malleable satisfactions are bounded by the satisfaction size plus:
@@ -1114,6 +1120,9 @@ void TestNode(const MsCtx script_ctx, const NodeRef& node, FuzzedDataProvider& p
// If a non-malleable satisfaction exists, the malleable one must also exist, and be identical to it.
assert(mal_success);
assert(stack_nonmal == stack_mal);
// Compute witness size (excluding script push, control block, and witness count encoding).
const size_t wit_size = GetSerializeSize(stack_nonmal, PROTOCOL_VERSION) - GetSizeOfCompactSize(stack_nonmal.size());
assert(wit_size <= *node->GetWitnessSize());
// Test non-malleable satisfaction.
witness_nonmal.stack.insert(witness_nonmal.stack.end(), std::make_move_iterator(stack_nonmal.begin()), std::make_move_iterator(stack_nonmal.end()));
@@ -1229,13 +1238,13 @@ FUZZ_TARGET(miniscript_string, .init = FuzzInit)
if (buffer.empty()) return;
FuzzedDataProvider provider(buffer.data(), buffer.size());
auto str = provider.ConsumeBytesAsString(provider.remaining_bytes() - 1);
PARSER_CTX.script_ctx = (MsCtx)provider.ConsumeBool();
auto parsed = miniscript::FromString(str, PARSER_CTX);
const ParserContext parser_ctx{(MsCtx)provider.ConsumeBool()};
auto parsed = miniscript::FromString(str, parser_ctx);
if (!parsed) return;
const auto str2 = parsed->ToString(PARSER_CTX);
const auto str2 = parsed->ToString(parser_ctx);
assert(str2);
auto parsed2 = miniscript::FromString(*str2, PARSER_CTX);
auto parsed2 = miniscript::FromString(*str2, parser_ctx);
assert(parsed2);
assert(*parsed == *parsed2);
}
@@ -1247,9 +1256,9 @@ FUZZ_TARGET(miniscript_script)
const std::optional<CScript> script = ConsumeDeserializable<CScript>(fuzzed_data_provider);
if (!script) return;
SCRIPT_PARSER_CONTEXT.script_ctx = (MsCtx)fuzzed_data_provider.ConsumeBool();
const auto ms = miniscript::FromScript(*script, SCRIPT_PARSER_CONTEXT);
const ScriptParserContext script_parser_ctx{(MsCtx)fuzzed_data_provider.ConsumeBool()};
const auto ms = miniscript::FromScript(*script, script_parser_ctx);
if (!ms) return;
assert(ms->ToScript(SCRIPT_PARSER_CONTEXT) == *script);
assert(ms->ToScript(script_parser_ctx) == *script);
}