Merge 914c0ca into merged_master (Bitcoin PR bitcoin/bitcoin#22399)

This PR adds a fuzz test ensuring that all the different destination types
round-trip to strings (an important test given that Satoshi managed to mess
this up, having two different address types encode the same way..)

Anyway we fail this test, since both CNoDestination and NullData encode as
empty strings, and then this "decodes" as CNoDestination (and returns an
error, but the fuzztest doesn't check that). To make the fuzzer pass, I
changed NullData to encode and decode as "null".

I think this actually adds some functionality, letting you use "null" as
an "address" for sendtoaddress and createrawtransaction, thus burning the
coins and fixing #1011...but this was not my intent and we probably want
to think a bit more carefully before deliberately supporting this.

Author: apoelstra

src/key_io.cpp

@@ -127,11 +127,14 @@ class DestinationEncoder
     }
 
     std::string operator()(const CNoDestination& no) const { return {}; }
-    std::string operator()(const NullData& null) const { return {}; }
+    std::string operator()(const NullData& null) const { return "null"; }
 };
 
 CTxDestination DecodeDestination(const std::string& str, const CChainParams& params, const bool for_parent, std::string& error_str)
 {
+    // ELEMENTS: special case nulldata as "null"
+    if (str == "null") return NullData{};
+
     std::vector<unsigned char> data;
     size_t pk_size = CPubKey::COMPRESSED_SIZE;
     uint160 hash;

src/test/fuzz/integer.cpp

@@ -16,8 +16,6 @@
 #include <pow.h>
 #include <protocol.h>
 #include <pubkey.h>
-#include <rpc/util.h>
-#include <script/signingprovider.h>
 #include <script/standard.h>
 #include <serialize.h>
 #include <streams.h>
@@ -158,20 +156,6 @@ FUZZ_TARGET_INIT(integer, initialize_integer)
 
     const CKeyID key_id{u160};
     const CScriptID script_id{u160};
-    // CTxDestination = CNoDestination ∪ PKHash ∪ ScriptHash ∪ WitnessV0ScriptHash ∪ WitnessV0KeyHash ∪ WitnessUnknown
-    const PKHash pk_hash{u160};
-    const ScriptHash script_hash{u160};
-    const WitnessV0KeyHash witness_v0_key_hash{u160};
-    const WitnessV0ScriptHash witness_v0_script_hash{u256};
-    const std::vector<CTxDestination> destinations{pk_hash, script_hash, witness_v0_key_hash, witness_v0_script_hash};
-    const SigningProvider store;
-    for (const CTxDestination& destination : destinations) {
-        (void)DescribeAddress(destination);
-        (void)EncodeDestination(destination);
-        (void)GetKeyForDestination(store, destination);
-        (void)GetScriptForDestination(destination);
-        (void)IsValidDestination(destination);
-    }
 
     {
         CDataStream stream(SER_NETWORK, INIT_PROTO_VERSION);

src/test/fuzz/key_io.cpp

@@ -4,9 +4,6 @@
 
 #include <chainparams.h>
 #include <key_io.h>
-#include <rpc/util.h>
-#include <script/signingprovider.h>
-#include <script/standard.h>
 #include <test/fuzz/fuzz.h>
 
 #include <cassert>
@@ -39,12 +36,4 @@ FUZZ_TARGET_INIT(key_io, initialize_key_io)
     if (ext_pub_key.pubkey.size() == CPubKey::COMPRESSED_SIZE) {
         assert(ext_pub_key == DecodeExtPubKey(EncodeExtPubKey(ext_pub_key)));
     }
-
-    const CTxDestination tx_destination = DecodeDestination(random_string);
-    (void)DescribeAddress(tx_destination);
-    (void)GetKeyForDestination(/* store */ {}, tx_destination);
-    (void)GetScriptForDestination(tx_destination);
-    (void)IsValidDestination(tx_destination);
-
-    (void)IsValidDestinationString(random_string);
 }

src/test/fuzz/script.cpp

@@ -6,8 +6,10 @@
 #include <compressor.h>
 #include <core_io.h>
 #include <core_memusage.h>
+#include <key_io.h>
 #include <policy/policy.h>
 #include <pubkey.h>
+#include <rpc/util.h>
 #include <script/descriptor.h>
 #include <script/interpreter.h>
 #include <script/script.h>
@@ -184,26 +186,26 @@ FUZZ_TARGET_INIT(script, initialize_script)
     }
 
     {
-        WitnessUnknown witness_unknown_1{};
-        witness_unknown_1.version = fuzzed_data_provider.ConsumeIntegral<uint32_t>();
-        const std::vector<uint8_t> witness_unknown_program_1 = fuzzed_data_provider.ConsumeBytes<uint8_t>(40);
-        witness_unknown_1.length = witness_unknown_program_1.size();
-        std::copy(witness_unknown_program_1.begin(), witness_unknown_program_1.end(), witness_unknown_1.program);
-
-        WitnessUnknown witness_unknown_2{};
-        witness_unknown_2.version = fuzzed_data_provider.ConsumeIntegral<uint32_t>();
-        const std::vector<uint8_t> witness_unknown_program_2 = fuzzed_data_provider.ConsumeBytes<uint8_t>(40);
-        witness_unknown_2.length = witness_unknown_program_2.size();
-        std::copy(witness_unknown_program_2.begin(), witness_unknown_program_2.end(), witness_unknown_2.program);
-
-        (void)(witness_unknown_1 == witness_unknown_2);
-        (void)(witness_unknown_1 < witness_unknown_2);
-    }
+        const CTxDestination tx_destination_1{
+            fuzzed_data_provider.ConsumeBool() ?
+                DecodeDestination(fuzzed_data_provider.ConsumeRandomLengthString()) :
+                ConsumeTxDestination(fuzzed_data_provider)};
+        const CTxDestination tx_destination_2{ConsumeTxDestination(fuzzed_data_provider)};
+        const std::string encoded_dest{EncodeDestination(tx_destination_1)};
+        const UniValue json_dest{DescribeAddress(tx_destination_1)};
+        Assert(tx_destination_1 == DecodeDestination(encoded_dest));
+        (void)GetKeyForDestination(/* store */ {}, tx_destination_1);
+        const CScript dest{GetScriptForDestination(tx_destination_1)};
+        const bool valid{IsValidDestination(tx_destination_1)};
+        Assert(dest.empty() != valid);
+
+        Assert(valid == IsValidDestinationString(encoded_dest));
 
-    {
-        const CTxDestination tx_destination_1 = ConsumeTxDestination(fuzzed_data_provider);
-        const CTxDestination tx_destination_2 = ConsumeTxDestination(fuzzed_data_provider);
-        (void)(tx_destination_1 == tx_destination_2);
         (void)(tx_destination_1 < tx_destination_2);
+        if (tx_destination_1 == tx_destination_2) {
+            Assert(encoded_dest == EncodeDestination(tx_destination_2));
+            Assert(json_dest.write() == DescribeAddress(tx_destination_2).write());
+            Assert(dest == GetScriptForDestination(tx_destination_2));
+        }
     }
 }

src/test/fuzz/util.cpp

@@ -2,6 +2,7 @@
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
+#include <pubkey.h>
 #include <test/fuzz/util.h>
 #include <test/util/script.h>
 #include <util/rbf.h>
@@ -307,3 +308,44 @@ uint32_t ConsumeSequence(FuzzedDataProvider& fuzzed_data_provider) noexcept
                }) :
                fuzzed_data_provider.ConsumeIntegral<uint32_t>();
 }
+
+CTxDestination ConsumeTxDestination(FuzzedDataProvider& fuzzed_data_provider) noexcept
+{
+    CTxDestination tx_destination;
+    const size_t call_size{CallOneOf(
+        fuzzed_data_provider,
+        [&] {
+            tx_destination = CNoDestination{};
+        },
+        [&] {
+            tx_destination = PKHash{ConsumeUInt160(fuzzed_data_provider)};
+        },
+        [&] {
+            tx_destination = ScriptHash{ConsumeUInt160(fuzzed_data_provider)};
+        },
+        [&] {
+            tx_destination = WitnessV0ScriptHash{ConsumeUInt256(fuzzed_data_provider)};
+        },
+        [&] {
+            tx_destination = WitnessV0KeyHash{ConsumeUInt160(fuzzed_data_provider)};
+        },
+        [&] {
+            tx_destination = WitnessV1Taproot{XOnlyPubKey{ConsumeUInt256(fuzzed_data_provider)}};
+        },
+        [&] {
+            WitnessUnknown witness_unknown{};
+            witness_unknown.version = fuzzed_data_provider.ConsumeIntegralInRange(2, 16);
+            std::vector<uint8_t> witness_unknown_program_1{fuzzed_data_provider.ConsumeBytes<uint8_t>(40)};
+            if (witness_unknown_program_1.size() < 2) {
+                witness_unknown_program_1 = {0, 0};
+            }
+            witness_unknown.length = witness_unknown_program_1.size();
+            std::copy(witness_unknown_program_1.begin(), witness_unknown_program_1.end(), witness_unknown.program);
+            tx_destination = witness_unknown;
+        },
+        [&] {
+            tx_destination = NullData{};
+        })};
+    Assert(call_size == std::variant_size_v<CTxDestination>);
+    return tx_destination;
+}

src/test/fuzz/util.h

@@ -37,14 +37,15 @@
 #include <vector>
 
 template <typename... Callables>
-void CallOneOf(FuzzedDataProvider& fuzzed_data_provider, Callables... callables)
+size_t CallOneOf(FuzzedDataProvider& fuzzed_data_provider, Callables... callables)
 {
     constexpr size_t call_size{sizeof...(callables)};
     static_assert(call_size >= 1);
     const size_t call_index{fuzzed_data_provider.ConsumeIntegralInRange<size_t>(0, call_size - 1)};
 
     size_t i{0};
     ((i++ == call_index ? callables() : void()), ...);
+    return call_size;
 }
 
 template <typename Collection>
@@ -179,36 +180,7 @@ template <typename WeakEnumType, size_t size>
     return CTxMemPoolEntry{MakeTransactionRef(tx), fee, time, entry_height, spends_coinbase, sig_op_cost, {}, setPeginsSpent};
 }
 
-[[nodiscard]] inline CTxDestination ConsumeTxDestination(FuzzedDataProvider& fuzzed_data_provider) noexcept
-{
-    CTxDestination tx_destination;
-    CallOneOf(
-        fuzzed_data_provider,
-        [&] {
-            tx_destination = CNoDestination{};
-        },
-        [&] {
-            tx_destination = PKHash{ConsumeUInt160(fuzzed_data_provider)};
-        },
-        [&] {
-            tx_destination = ScriptHash{ConsumeUInt160(fuzzed_data_provider)};
-        },
-        [&] {
-            tx_destination = WitnessV0ScriptHash{ConsumeUInt256(fuzzed_data_provider)};
-        },
-        [&] {
-            tx_destination = WitnessV0KeyHash{ConsumeUInt160(fuzzed_data_provider)};
-        },
-        [&] {
-            WitnessUnknown witness_unknown{};
-            witness_unknown.version = fuzzed_data_provider.ConsumeIntegral<uint32_t>();
-            const std::vector<uint8_t> witness_unknown_program_1 = fuzzed_data_provider.ConsumeBytes<uint8_t>(40);
-            witness_unknown.length = witness_unknown_program_1.size();
-            std::copy(witness_unknown_program_1.begin(), witness_unknown_program_1.end(), witness_unknown.program);
-            tx_destination = witness_unknown;
-        });
-    return tx_destination;
-}
+[[nodiscard]] CTxDestination ConsumeTxDestination(FuzzedDataProvider& fuzzed_data_provider) noexcept;
 
 template <typename T>
 [[nodiscard]] bool MultiplicationOverflow(const T i, const T j) noexcept