Merge #16841: Replace GetScriptForWitness with GetScriptForDestination

fanquake · fanquake · commit d052f5e6b79b · 2020-08-15T08:54:45.000+08:00
7966aa4 Add variables for repeated scripts (MeshCollider) fec8336 Remove GetScriptForWitness function (MeshCollider) b887060 Replace usage of GetScriptForWitness with GetScriptForDestination (MeshCollider) Pull request description: As per this TODO in the code: > TODO: replace calls to GetScriptForWitness with GetScriptForDestination using the various witness-specific CTxDestination subtypes. The commit "Add additional check for P2SH before adding extra wrapper" also adds an additional check that the scriptPubKey is a P2SH before auto-wrapping the witness script. We shouldn't wrap the witness script if not. Note: #16251 is even better than this check, please review that. ACKs for top commit: instagibbs: ACK bitcoin/bitcoin@7966aa4 jonatack: Code review re-ACK 7966aa4 per `git range-diff b4d0366 ed266f7 7966aa4` achow101: re-ACK 7966aa4 only changes since last is rebase. Tree-SHA512: 3449e0e83bd842acc7c94544a85367da97ac20d859eefc1a618caef0c98204398c266fe8fb9600b78326df5175402e1ae4a132eb766e2c4485e7cda6a2a95c43
diff --git a/src/bitcoin-tx.cpp b/src/bitcoin-tx.cpp
@@ -320,8 +320,8 @@ static void MutateTxAddOutPubKey(CMutableTransaction& tx, const std::string& str
         if (!pubkey.IsCompressed()) {
             throw std::runtime_error("Uncompressed pubkeys are not useable for SegWit outputs");
         }
-        // Call GetScriptForWitness() to build a P2WSH scriptPubKey
-        scriptPubKey = GetScriptForWitness(scriptPubKey);
+        // Build a P2WPKH script
+        scriptPubKey = GetScriptForDestination(WitnessV0KeyHash(pubkey));
     }
     if (bScriptHash) {
         // Get the ID for the script, and then construct a P2SH destination for it.
@@ -390,8 +390,8 @@ static void MutateTxAddOutMultiSig(CMutableTransaction& tx, const std::string& s
                 throw std::runtime_error("Uncompressed pubkeys are not useable for SegWit outputs");
             }
         }
-        // Call GetScriptForWitness() to build a P2WSH scriptPubKey
-        scriptPubKey = GetScriptForWitness(scriptPubKey);
+        // Build a P2WSH with the multisig script
+        scriptPubKey = GetScriptForDestination(WitnessV0ScriptHash(scriptPubKey));
     }
     if (bScriptHash) {
         if (scriptPubKey.size() > MAX_SCRIPT_ELEMENT_SIZE) {
@@ -464,7 +464,7 @@ static void MutateTxAddOutScript(CMutableTransaction& tx, const std::string& str
     }
 
     if (bSegWit) {
-        scriptPubKey = GetScriptForWitness(scriptPubKey);
+        scriptPubKey = GetScriptForDestination(WitnessV0ScriptHash(scriptPubKey));
     }
     if (bScriptHash) {
         if (scriptPubKey.size() > MAX_SCRIPT_ELEMENT_SIZE) {
diff --git a/src/script/standard.cpp b/src/script/standard.cpp
@@ -313,18 +313,6 @@ CScript GetScriptForMultisig(int nRequired, const std::vector<CPubKey>& keys)
     return script;
 }
 
-CScript GetScriptForWitness(const CScript& redeemscript)
-{
-    std::vector<std::vector<unsigned char> > vSolutions;
-    TxoutType typ = Solver(redeemscript, vSolutions);
-    if (typ == TxoutType::PUBKEY) {
-        return GetScriptForDestination(WitnessV0KeyHash(Hash160(vSolutions[0])));
-    } else if (typ == TxoutType::PUBKEYHASH) {
-        return GetScriptForDestination(WitnessV0KeyHash(uint160{vSolutions[0]}));
-    }
-    return GetScriptForDestination(WitnessV0ScriptHash(redeemscript));
-}
-
 bool IsValidDestination(const CTxDestination& dest) {
     return dest.which() != 0;
 }
diff --git a/src/script/standard.h b/src/script/standard.h
@@ -263,14 +263,4 @@ CScript GetScriptForRawPubKey(const CPubKey& pubkey);
 /** Generate a multisig script. */
 CScript GetScriptForMultisig(int nRequired, const std::vector<CPubKey>& keys);
 
-/**
- * Generate a pay-to-witness script for the given redeem script. If the redeem
- * script is P2PK or P2PKH, this returns a P2WPKH script, otherwise it returns a
- * P2WSH script.
- *
- * TODO: replace calls to GetScriptForWitness with GetScriptForDestination using
- * the various witness-specific CTxDestination subtypes.
- */
-CScript GetScriptForWitness(const CScript& redeemscript);
-
 #endif // BITCOIN_SCRIPT_STANDARD_H
diff --git a/src/test/fuzz/script.cpp b/src/test/fuzz/script.cpp
@@ -63,8 +63,6 @@ void test_one_input(const std::vector<uint8_t>& buffer)
     int required_ret;
     (void)ExtractDestinations(script, type_ret, addresses, required_ret);
 
-    (void)GetScriptForWitness(script);
-
     const FlatSigningProvider signing_provider;
     (void)InferDescriptor(script, signing_provider);
 
diff --git a/src/test/script_standard_tests.cpp b/src/test/script_standard_tests.cpp
@@ -349,21 +349,16 @@ BOOST_AUTO_TEST_CASE(script_standard_GetScriptFor_)
     result = GetScriptForMultisig(2, std::vector<CPubKey>(pubkeys, pubkeys + 3));
     BOOST_CHECK(result == expected);
 
-    // GetScriptForWitness
-    CScript witnessScript;
-
-    witnessScript << ToByteVector(pubkeys[0]) << OP_CHECKSIG;
+    // WitnessV0KeyHash
     expected.clear();
     expected << OP_0 << ToByteVector(pubkeys[0].GetID());
-    result = GetScriptForWitness(witnessScript);
+    result = GetScriptForDestination(WitnessV0KeyHash(Hash160(ToByteVector(pubkeys[0]))));
     BOOST_CHECK(result == expected);
-
-    witnessScript.clear();
-    witnessScript << OP_DUP << OP_HASH160 << ToByteVector(pubkeys[0].GetID()) << OP_EQUALVERIFY << OP_CHECKSIG;
-    result = GetScriptForWitness(witnessScript);
+    result = GetScriptForDestination(WitnessV0KeyHash(pubkeys[0].GetID()));
     BOOST_CHECK(result == expected);
 
-    witnessScript.clear();
+    // WitnessV0ScriptHash (multisig)
+    CScript witnessScript;
     witnessScript << OP_1 << ToByteVector(pubkeys[0]) << OP_1 << OP_CHECKMULTISIG;
 
     uint256 scriptHash;
@@ -372,7 +367,7 @@ BOOST_AUTO_TEST_CASE(script_standard_GetScriptFor_)
 
     expected.clear();
     expected << OP_0 << ToByteVector(scriptHash);
-    result = GetScriptForWitness(witnessScript);
+    result = GetScriptForDestination(WitnessV0ScriptHash(witnessScript));
     BOOST_CHECK(result == expected);
 }
 
diff --git a/src/test/sigopcount_tests.cpp b/src/test/sigopcount_tests.cpp
@@ -154,8 +154,7 @@ BOOST_AUTO_TEST_CASE(GetTxSigOpCost)
 
     // P2WPKH witness program
     {
-        CScript p2pk = CScript() << ToByteVector(pubkey) << OP_CHECKSIG;
-        CScript scriptPubKey = GetScriptForWitness(p2pk);
+        CScript scriptPubKey = GetScriptForDestination(WitnessV0KeyHash(pubkey));
         CScript scriptSig = CScript();
         CScriptWitness scriptWitness;
         scriptWitness.stack.push_back(std::vector<unsigned char>(0));
@@ -183,8 +182,7 @@ BOOST_AUTO_TEST_CASE(GetTxSigOpCost)
 
     // P2WPKH nested in P2SH
     {
-        CScript p2pk = CScript() << ToByteVector(pubkey) << OP_CHECKSIG;
-        CScript scriptSig = GetScriptForWitness(p2pk);
+        CScript scriptSig = GetScriptForDestination(WitnessV0KeyHash(pubkey));
         CScript scriptPubKey = GetScriptForDestination(ScriptHash(scriptSig));
         scriptSig = CScript() << ToByteVector(scriptSig);
         CScriptWitness scriptWitness;
@@ -199,7 +197,7 @@ BOOST_AUTO_TEST_CASE(GetTxSigOpCost)
     // P2WSH witness program
     {
         CScript witnessScript = CScript() << 1 << ToByteVector(pubkey) << ToByteVector(pubkey) << 2 << OP_CHECKMULTISIGVERIFY;
-        CScript scriptPubKey = GetScriptForWitness(witnessScript);
+        CScript scriptPubKey = GetScriptForDestination(WitnessV0ScriptHash(witnessScript));
         CScript scriptSig = CScript();
         CScriptWitness scriptWitness;
         scriptWitness.stack.push_back(std::vector<unsigned char>(0));
@@ -215,7 +213,7 @@ BOOST_AUTO_TEST_CASE(GetTxSigOpCost)
     // P2WSH nested in P2SH
     {
         CScript witnessScript = CScript() << 1 << ToByteVector(pubkey) << ToByteVector(pubkey) << 2 << OP_CHECKMULTISIGVERIFY;
-        CScript redeemScript = GetScriptForWitness(witnessScript);
+        CScript redeemScript = GetScriptForDestination(WitnessV0ScriptHash(witnessScript));
         CScript scriptPubKey = GetScriptForDestination(ScriptHash(redeemScript));
         CScript scriptSig = CScript() << ToByteVector(redeemScript);
         CScriptWitness scriptWitness;
diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp
@@ -501,13 +501,19 @@ BOOST_AUTO_TEST_CASE(test_witness)
     BOOST_CHECK(keystore.AddCScript(scriptPubkey1L));
     BOOST_CHECK(keystore.AddCScript(scriptPubkey2L));
     BOOST_CHECK(keystore.AddCScript(scriptMulti));
-    BOOST_CHECK(keystore.AddCScript(GetScriptForWitness(scriptPubkey1)));
-    BOOST_CHECK(keystore.AddCScript(GetScriptForWitness(scriptPubkey2)));
-    BOOST_CHECK(keystore.AddCScript(GetScriptForWitness(scriptPubkey1L)));
-    BOOST_CHECK(keystore.AddCScript(GetScriptForWitness(scriptPubkey2L)));
-    BOOST_CHECK(keystore.AddCScript(GetScriptForWitness(scriptMulti)));
+    CScript destination_script_1, destination_script_2, destination_script_1L, destination_script_2L, destination_script_multi;
+    destination_script_1 = GetScriptForDestination(WitnessV0KeyHash(pubkey1));
+    destination_script_2 = GetScriptForDestination(WitnessV0KeyHash(pubkey2));
+    destination_script_1L = GetScriptForDestination(WitnessV0KeyHash(pubkey1L));
+    destination_script_2L = GetScriptForDestination(WitnessV0KeyHash(pubkey2L));
+    destination_script_multi = GetScriptForDestination(WitnessV0ScriptHash(scriptMulti));
+    BOOST_CHECK(keystore.AddCScript(destination_script_1));
+    BOOST_CHECK(keystore.AddCScript(destination_script_2));
+    BOOST_CHECK(keystore.AddCScript(destination_script_1L));
+    BOOST_CHECK(keystore.AddCScript(destination_script_2L));
+    BOOST_CHECK(keystore.AddCScript(destination_script_multi));
     BOOST_CHECK(keystore2.AddCScript(scriptMulti));
-    BOOST_CHECK(keystore2.AddCScript(GetScriptForWitness(scriptMulti)));
+    BOOST_CHECK(keystore2.AddCScript(destination_script_multi));
     BOOST_CHECK(keystore2.AddKeyPubKey(key3, pubkey3));
 
     CTransactionRef output1, output2;
@@ -539,8 +545,8 @@ BOOST_AUTO_TEST_CASE(test_witness)
     CheckWithFlag(output1, input2, STANDARD_SCRIPT_VERIFY_FLAGS, false);
 
     // Witness pay-to-compressed-pubkey (v0).
-    CreateCreditAndSpend(keystore, GetScriptForWitness(scriptPubkey1), output1, input1);
-    CreateCreditAndSpend(keystore, GetScriptForWitness(scriptPubkey2), output2, input2);
+    CreateCreditAndSpend(keystore, destination_script_1, output1, input1);
+    CreateCreditAndSpend(keystore, destination_script_2, output2, input2);
     CheckWithFlag(output1, input1, 0, true);
     CheckWithFlag(output1, input1, SCRIPT_VERIFY_P2SH, true);
     CheckWithFlag(output1, input1, SCRIPT_VERIFY_WITNESS | SCRIPT_VERIFY_P2SH, true);
@@ -551,9 +557,9 @@ BOOST_AUTO_TEST_CASE(test_witness)
     CheckWithFlag(output1, input2, STANDARD_SCRIPT_VERIFY_FLAGS, false);
 
     // P2SH witness pay-to-compressed-pubkey (v0).
-    CreateCreditAndSpend(keystore, GetScriptForDestination(ScriptHash(GetScriptForWitness(scriptPubkey1))), output1, input1);
-    CreateCreditAndSpend(keystore, GetScriptForDestination(ScriptHash(GetScriptForWitness(scriptPubkey2))), output2, input2);
-    ReplaceRedeemScript(input2.vin[0].scriptSig, GetScriptForWitness(scriptPubkey1));
+    CreateCreditAndSpend(keystore, GetScriptForDestination(ScriptHash(destination_script_1)), output1, input1);
+    CreateCreditAndSpend(keystore, GetScriptForDestination(ScriptHash(destination_script_2)), output2, input2);
+    ReplaceRedeemScript(input2.vin[0].scriptSig, destination_script_1);
     CheckWithFlag(output1, input1, 0, true);
     CheckWithFlag(output1, input1, SCRIPT_VERIFY_P2SH, true);
     CheckWithFlag(output1, input1, SCRIPT_VERIFY_WITNESS | SCRIPT_VERIFY_P2SH, true);
@@ -589,12 +595,12 @@ BOOST_AUTO_TEST_CASE(test_witness)
     CheckWithFlag(output1, input2, STANDARD_SCRIPT_VERIFY_FLAGS, false);
 
     // Signing disabled for witness pay-to-uncompressed-pubkey (v1).
-    CreateCreditAndSpend(keystore, GetScriptForWitness(scriptPubkey1L), output1, input1, false);
-    CreateCreditAndSpend(keystore, GetScriptForWitness(scriptPubkey2L), output2, input2, false);
+    CreateCreditAndSpend(keystore, destination_script_1L, output1, input1, false);
+    CreateCreditAndSpend(keystore, destination_script_2L, output2, input2, false);
 
     // Signing disabled for P2SH witness pay-to-uncompressed-pubkey (v1).
-    CreateCreditAndSpend(keystore, GetScriptForDestination(ScriptHash(GetScriptForWitness(scriptPubkey1L))), output1, input1, false);
-    CreateCreditAndSpend(keystore, GetScriptForDestination(ScriptHash(GetScriptForWitness(scriptPubkey2L))), output2, input2, false);
+    CreateCreditAndSpend(keystore, GetScriptForDestination(ScriptHash(destination_script_1L)), output1, input1, false);
+    CreateCreditAndSpend(keystore, GetScriptForDestination(ScriptHash(destination_script_2L)), output2, input2, false);
 
     // Normal 2-of-2 multisig
     CreateCreditAndSpend(keystore, scriptMulti, output1, input1, false);
@@ -618,10 +624,10 @@ BOOST_AUTO_TEST_CASE(test_witness)
     CheckWithFlag(output1, input1, STANDARD_SCRIPT_VERIFY_FLAGS, true);
 
     // Witness 2-of-2 multisig
-    CreateCreditAndSpend(keystore, GetScriptForWitness(scriptMulti), output1, input1, false);
+    CreateCreditAndSpend(keystore, destination_script_multi, output1, input1, false);
     CheckWithFlag(output1, input1, 0, true);
     CheckWithFlag(output1, input1, SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_WITNESS, false);
-    CreateCreditAndSpend(keystore2, GetScriptForWitness(scriptMulti), output2, input2, false);
+    CreateCreditAndSpend(keystore2, destination_script_multi, output2, input2, false);
     CheckWithFlag(output2, input2, 0, true);
     CheckWithFlag(output2, input2, SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_WITNESS, false);
     BOOST_CHECK(*output1 == *output2);
@@ -630,10 +636,10 @@ BOOST_AUTO_TEST_CASE(test_witness)
     CheckWithFlag(output1, input1, STANDARD_SCRIPT_VERIFY_FLAGS, true);
 
     // P2SH witness 2-of-2 multisig
-    CreateCreditAndSpend(keystore, GetScriptForDestination(ScriptHash(GetScriptForWitness(scriptMulti))), output1, input1, false);
+    CreateCreditAndSpend(keystore, GetScriptForDestination(ScriptHash(destination_script_multi)), output1, input1, false);
     CheckWithFlag(output1, input1, SCRIPT_VERIFY_P2SH, true);
     CheckWithFlag(output1, input1, SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_WITNESS, false);
-    CreateCreditAndSpend(keystore2, GetScriptForDestination(ScriptHash(GetScriptForWitness(scriptMulti))), output2, input2, false);
+    CreateCreditAndSpend(keystore2, GetScriptForDestination(ScriptHash(destination_script_multi)), output2, input2, false);
     CheckWithFlag(output2, input2, SCRIPT_VERIFY_P2SH, true);
     CheckWithFlag(output2, input2, SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_WITNESS, false);
     BOOST_CHECK(*output1 == *output2);
diff --git a/src/test/txvalidationcache_tests.cpp b/src/test/txvalidationcache_tests.cpp
@@ -157,7 +157,7 @@ BOOST_FIXTURE_TEST_CASE(checkinputs_test, TestChain100Setup)
     CScript p2pk_scriptPubKey = CScript() << ToByteVector(coinbaseKey.GetPubKey()) << OP_CHECKSIG;
     CScript p2sh_scriptPubKey = GetScriptForDestination(ScriptHash(p2pk_scriptPubKey));
     CScript p2pkh_scriptPubKey = GetScriptForDestination(PKHash(coinbaseKey.GetPubKey()));
-    CScript p2wpkh_scriptPubKey = GetScriptForWitness(p2pkh_scriptPubKey);
+    CScript p2wpkh_scriptPubKey = GetScriptForDestination(WitnessV0KeyHash(coinbaseKey.GetPubKey()));
 
     FillableSigningProvider keystore;
     BOOST_CHECK(keystore.AddKey(coinbaseKey));

Original file line number	Diff line number	Diff line change
`@@ -320,8 +320,8 @@ static void MutateTxAddOutPubKey(CMutableTransaction& tx, const std::string& str`
`320`	`320`	`if (!pubkey.IsCompressed()) {`
`321`	`321`	`throw std::runtime_error("Uncompressed pubkeys are not useable for SegWit outputs");`
`322`	`322`	`}`
`323`		`- // Call GetScriptForWitness() to build a P2WSH scriptPubKey`
`324`		`- scriptPubKey = GetScriptForWitness(scriptPubKey);`
	`323`	`+ // Build a P2WPKH script`
	`324`	`+ scriptPubKey = GetScriptForDestination(WitnessV0KeyHash(pubkey));`
`325`	`325`	`}`
`326`	`326`	`if (bScriptHash) {`
`327`	`327`	`// Get the ID for the script, and then construct a P2SH destination for it.`
`@@ -390,8 +390,8 @@ static void MutateTxAddOutMultiSig(CMutableTransaction& tx, const std::string& s`
`390`	`390`	`throw std::runtime_error("Uncompressed pubkeys are not useable for SegWit outputs");`
`391`	`391`	`}`
`392`	`392`	`}`
`393`		`- // Call GetScriptForWitness() to build a P2WSH scriptPubKey`
`394`		`- scriptPubKey = GetScriptForWitness(scriptPubKey);`
	`393`	`+ // Build a P2WSH with the multisig script`
	`394`	`+ scriptPubKey = GetScriptForDestination(WitnessV0ScriptHash(scriptPubKey));`
`395`	`395`	`}`
`396`	`396`	`if (bScriptHash) {`
`397`	`397`	`if (scriptPubKey.size() > MAX_SCRIPT_ELEMENT_SIZE) {`
`@@ -464,7 +464,7 @@ static void MutateTxAddOutScript(CMutableTransaction& tx, const std::string& str`
`464`	`464`	`}`
`465`	`465`
`466`	`466`	`if (bSegWit) {`
`467`		`- scriptPubKey = GetScriptForWitness(scriptPubKey);`
	`467`	`+ scriptPubKey = GetScriptForDestination(WitnessV0ScriptHash(scriptPubKey));`
`468`	`468`	`}`
`469`	`469`	`if (bScriptHash) {`
`470`	`470`	`if (scriptPubKey.size() > MAX_SCRIPT_ELEMENT_SIZE) {`