diff options
author | Andrew Chow <achow101-github@achow101.com> | 2022-08-11 13:32:08 -0400 |
---|---|---|
committer | Andrew Chow <achow101-github@achow101.com> | 2022-08-11 13:41:47 -0400 |
commit | e078ee9d9d0a7505c38cf22ee4cc757bf31b62c0 (patch) | |
tree | 57dac8f97e35cf1159d794732d109bed9c5ff38e /src | |
parent | 29c195cf6a2c9d6a3fee6cbe889480875efdd8e8 (diff) | |
parent | b16f93caddcd3254eaf3dc43e09adf2142a9c40a (diff) | |
download | bitcoin-e078ee9d9d0a7505c38cf22ee4cc757bf31b62c0.tar.xz |
Merge bitcoin/bitcoin#25664: refactor: Redefine `IsSolvable()` using descriptors
b16f93caddcd3254eaf3dc43e09adf2142a9c40a script/sign: remove needless IsSolvable() utility (Antoine Poinsot)
c232ef20c0fd2e3b55355e52684091cad3af5247 outputtype: remove redundant check for uncompressed keys in AddAndGetDestinationForScript (Antoine Poinsot)
Pull request description:
Now that we have descriptors there is no need to try to sign for a scriptPubKey using dummy signatures, and using a mocked verification of this witness against the interpreter, just to make sure we know how to spend such a Script. Just try to infer a solvable descriptor: any scriptPubKey that we can sign for can be inferred as such.
This came up in #24149 but i think it's worth it on its own.
ACKs for top commit:
instagibbs:
ACK https://github.com/bitcoin/bitcoin/pull/25664/commits/b16f93caddcd3254eaf3dc43e09adf2142a9c40a
achow101:
re-ACK b16f93caddcd3254eaf3dc43e09adf2142a9c40a
furszy:
ACK b16f93ca, only change is the `IsSolvable` helper function removal.
Tree-SHA512: 137068157ce90210b710b1bf9ac3c400e2ff5af1112f892094b69875ea473d6a899f52adb51e5030cb907dee517602059cd1661107808558efa5de842ba12b41
Diffstat (limited to 'src')
-rw-r--r-- | src/outputtype.cpp | 2 | ||||
-rw-r--r-- | src/script/sign.cpp | 19 | ||||
-rw-r--r-- | src/script/sign.h | 6 | ||||
-rw-r--r-- | src/test/descriptor_tests.cpp | 3 | ||||
-rw-r--r-- | src/test/fuzz/key.cpp | 2 | ||||
-rw-r--r-- | src/test/fuzz/script.cpp | 1 | ||||
-rw-r--r-- | src/wallet/rpc/addresses.cpp | 2 | ||||
-rw-r--r-- | src/wallet/scriptpubkeyman.cpp | 3 |
8 files changed, 4 insertions, 34 deletions
diff --git a/src/outputtype.cpp b/src/outputtype.cpp index 19366295e6..5be9942d7d 100644 --- a/src/outputtype.cpp +++ b/src/outputtype.cpp @@ -91,8 +91,6 @@ CTxDestination AddAndGetDestinationForScript(FillableSigningProvider& keystore, case OutputType::BECH32: { CTxDestination witdest = WitnessV0ScriptHash(script); CScript witprog = GetScriptForDestination(witdest); - // Check if the resulting program is solvable (i.e. doesn't use an uncompressed key) - if (!IsSolvable(keystore, witprog)) return ScriptHash(script); // Add the redeemscript, so that P2WSH and P2SH-P2WSH outputs are recognized as ours. keystore.AddCScript(witprog); if (type == OutputType::BECH32) { diff --git a/src/script/sign.cpp b/src/script/sign.cpp index 79cf918c9b..4014ebadbc 100644 --- a/src/script/sign.cpp +++ b/src/script/sign.cpp @@ -632,25 +632,6 @@ public: const BaseSignatureCreator& DUMMY_SIGNATURE_CREATOR = DummySignatureCreator(32, 32); const BaseSignatureCreator& DUMMY_MAXIMUM_SIGNATURE_CREATOR = DummySignatureCreator(33, 32); -bool IsSolvable(const SigningProvider& provider, const CScript& script) -{ - // This check is to make sure that the script we created can actually be solved for and signed by us - // if we were to have the private keys. This is just to make sure that the script is valid and that, - // if found in a transaction, we would still accept and relay that transaction. In particular, - // it will reject witness outputs that require signing with an uncompressed public key. - SignatureData sigs; - // Make sure that STANDARD_SCRIPT_VERIFY_FLAGS includes SCRIPT_VERIFY_WITNESS_PUBKEYTYPE, the most - // important property this function is designed to test for. - static_assert(STANDARD_SCRIPT_VERIFY_FLAGS & SCRIPT_VERIFY_WITNESS_PUBKEYTYPE, "IsSolvable requires standard script flags to include WITNESS_PUBKEYTYPE"); - if (ProduceSignature(provider, DUMMY_SIGNATURE_CREATOR, script, sigs)) { - // VerifyScript check is just defensive, and should never fail. - bool verified = VerifyScript(sigs.scriptSig, script, &sigs.scriptWitness, STANDARD_SCRIPT_VERIFY_FLAGS, DUMMY_CHECKER); - assert(verified); - return true; - } - return false; -} - bool IsSegWitOutput(const SigningProvider& provider, const CScript& script) { int version; diff --git a/src/script/sign.h b/src/script/sign.h index 5e58272154..958d673b9f 100644 --- a/src/script/sign.h +++ b/src/script/sign.h @@ -97,12 +97,6 @@ bool SignSignature(const SigningProvider &provider, const CTransaction& txFrom, SignatureData DataFromTransaction(const CMutableTransaction& tx, unsigned int nIn, const CTxOut& txout); void UpdateInput(CTxIn& input, const SignatureData& data); -/* Check whether we know how to sign for an output like this, assuming we - * have all private keys. While this function does not need private keys, the passed - * provider is used to look up public keys and redeemscripts by hash. - * Solvability is unrelated to whether we consider this output to be ours. */ -bool IsSolvable(const SigningProvider& provider, const CScript& script); - /** Check whether a scriptPubKey is known to be segwit. */ bool IsSegWitOutput(const SigningProvider& provider, const CScript& script); diff --git a/src/test/descriptor_tests.cpp b/src/test/descriptor_tests.cpp index 8a17472cda..1eb4b373b4 100644 --- a/src/test/descriptor_tests.cpp +++ b/src/test/descriptor_tests.cpp @@ -302,7 +302,6 @@ void DoCheck(const std::string& prv, const std::string& pub, const std::string& // For each of the produced scripts, verify solvability, and when possible, try to sign a transaction spending it. for (size_t n = 0; n < spks.size(); ++n) { BOOST_CHECK_EQUAL(ref[n], HexStr(spks[n])); - BOOST_CHECK_EQUAL(IsSolvable(Merge(key_provider, script_provider), spks[n]), (flags & UNSOLVABLE) == 0); if (flags & SIGNABLE) { CMutableTransaction spend; @@ -324,7 +323,7 @@ void DoCheck(const std::string& prv, const std::string& pub, const std::string& BOOST_CHECK(inferred->Expand(0, provider_inferred, spks_inferred, provider_inferred)); BOOST_CHECK_EQUAL(spks_inferred.size(), 1U); BOOST_CHECK(spks_inferred[0] == spks[n]); - BOOST_CHECK_EQUAL(IsSolvable(provider_inferred, spks_inferred[0]), !(flags & UNSOLVABLE)); + BOOST_CHECK_EQUAL(InferDescriptor(spks_inferred[0], provider_inferred)->IsSolvable(), !(flags & UNSOLVABLE)); BOOST_CHECK(GetKeyOriginData(provider_inferred, flags) == GetKeyOriginData(script_provider, flags)); } diff --git a/src/test/fuzz/key.cpp b/src/test/fuzz/key.cpp index 6d2d2e2bc5..a76901e473 100644 --- a/src/test/fuzz/key.cpp +++ b/src/test/fuzz/key.cpp @@ -138,8 +138,6 @@ FUZZ_TARGET_INIT(key, initialize_key) assert(tx_multisig_script.size() == 37); FillableSigningProvider fillable_signing_provider; - assert(IsSolvable(fillable_signing_provider, tx_pubkey_script)); - assert(IsSolvable(fillable_signing_provider, tx_multisig_script)); assert(!IsSegWitOutput(fillable_signing_provider, tx_pubkey_script)); assert(!IsSegWitOutput(fillable_signing_provider, tx_multisig_script)); assert(fillable_signing_provider.GetKeys().size() == 0); diff --git a/src/test/fuzz/script.cpp b/src/test/fuzz/script.cpp index 69a4b782aa..00d7b7e29a 100644 --- a/src/test/fuzz/script.cpp +++ b/src/test/fuzz/script.cpp @@ -89,7 +89,6 @@ FUZZ_TARGET_INIT(script, initialize_script) const FlatSigningProvider signing_provider; (void)InferDescriptor(script, signing_provider); (void)IsSegWitOutput(signing_provider, script); - (void)IsSolvable(signing_provider, script); (void)RecursiveDynamicUsage(script); diff --git a/src/wallet/rpc/addresses.cpp b/src/wallet/rpc/addresses.cpp index 148343a8b0..903a569cb9 100644 --- a/src/wallet/rpc/addresses.cpp +++ b/src/wallet/rpc/addresses.cpp @@ -578,7 +578,7 @@ RPCHelpMan getaddressinfo() if (provider) { auto inferred = InferDescriptor(scriptPubKey, *provider); - bool solvable = inferred->IsSolvable() || IsSolvable(*provider, scriptPubKey); + bool solvable = inferred->IsSolvable(); ret.pushKV("solvable", solvable); if (solvable) { ret.pushKV("desc", inferred->ToString()); diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index e2cf081259..7ff017775e 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -1456,7 +1456,8 @@ void LegacyScriptPubKeyMan::LearnRelatedScripts(const CPubKey& key, OutputType t CTxDestination witdest = WitnessV0KeyHash(key.GetID()); CScript witprog = GetScriptForDestination(witdest); // Make sure the resulting program is solvable. - assert(IsSolvable(*this, witprog)); + const auto desc = InferDescriptor(witprog, *this); + assert(desc && desc->IsSolvable()); AddCScript(witprog); } } |