From faa5425629d35708326b255570c51139aef0c8c4 Mon Sep 17 00:00:00 2001 From: MacroFake Date: Fri, 10 Jun 2022 10:39:44 +0200 Subject: [PATCH 1/2] Add HashWriter without ser-type and ser-version The moved parts can be reviewed with "--color-moved=dimmed-zebra". --- src/hash.h | 30 +++++++++++++++++++++--------- 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/src/hash.h b/src/hash.h index 0ccef2105f8aa..0f0c5d4b2cc13 100644 --- a/src/hash.h +++ b/src/hash.h @@ -96,20 +96,12 @@ inline uint160 Hash160(const T1& in1) } /** A writer stream (for serialization) that computes a 256-bit hash. */ -class CHashWriter +class HashWriter { private: CSHA256 ctx; - const int nType; - const int nVersion; public: - - CHashWriter(int nTypeIn, int nVersionIn) : nType(nTypeIn), nVersion(nVersionIn) {} - - int GetType() const { return nType; } - int GetVersion() const { return nVersion; } - void write(Span src) { ctx.Write(UCharCast(src.data()), src.size()); @@ -144,6 +136,26 @@ class CHashWriter return ReadLE64(result.begin()); } + template + HashWriter& operator<<(const T& obj) + { + ::Serialize(*this, obj); + return *this; + } +}; + +class CHashWriter : public HashWriter +{ +private: + const int nType; + const int nVersion; + +public: + CHashWriter(int nTypeIn, int nVersionIn) : nType(nTypeIn), nVersion(nVersionIn) {} + + int GetType() const { return nType; } + int GetVersion() const { return nVersion; } + template CHashWriter& operator<<(const T& obj) { // Serialize to this stream From faf9accd662974a69390213fee1b5c6237846b42 Mon Sep 17 00:00:00 2001 From: MacroFake Date: Fri, 10 Jun 2022 12:02:12 +0200 Subject: [PATCH 2/2] Use HashWriter where possible --- src/chainparams.cpp | 2 +- src/crypto/muhash.cpp | 4 ++-- src/hash.cpp | 4 ++-- src/hash.h | 6 +++--- src/kernel/coinstats.cpp | 8 ++++---- src/node/blockstorage.cpp | 2 +- src/pubkey.cpp | 6 +++--- src/script/interpreter.cpp | 32 ++++++++++++++++---------------- src/script/interpreter.h | 6 +++--- src/script/sign.cpp | 2 +- src/script/standard.cpp | 8 ++++---- src/test/script_tests.cpp | 2 +- src/util/message.cpp | 2 +- src/wallet/dump.cpp | 4 ++-- 14 files changed, 44 insertions(+), 44 deletions(-) diff --git a/src/chainparams.cpp b/src/chainparams.cpp index 7a7c72ea2570f..dd7b93234d1c6 100644 --- a/src/chainparams.cpp +++ b/src/chainparams.cpp @@ -352,7 +352,7 @@ class SigNetParams : public CChainParams { consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].min_activation_height = 0; // No activation delay // message start is defined as the first 4 bytes of the sha256d of the block script - CHashWriter h(SER_DISK, 0); + HashWriter h{}; h << consensus.signet_challenge; uint256 hash = h.GetHash(); memcpy(pchMessageStart, hash.begin(), 4); diff --git a/src/crypto/muhash.cpp b/src/crypto/muhash.cpp index 57ed357645f68..7d14b7938e358 100644 --- a/src/crypto/muhash.cpp +++ b/src/crypto/muhash.cpp @@ -298,7 +298,7 @@ void Num3072::ToBytes(unsigned char (&out)[BYTE_SIZE]) { Num3072 MuHash3072::ToNum3072(Span in) { unsigned char tmp[Num3072::BYTE_SIZE]; - uint256 hashed_in = (CHashWriter(SER_DISK, 0) << in).GetSHA256(); + uint256 hashed_in{(HashWriter{} << in).GetSHA256()}; ChaCha20(hashed_in.data(), hashed_in.size()).Keystream(tmp, Num3072::BYTE_SIZE); Num3072 out{tmp}; @@ -318,7 +318,7 @@ void MuHash3072::Finalize(uint256& out) noexcept unsigned char data[Num3072::BYTE_SIZE]; m_numerator.ToBytes(data); - out = (CHashWriter(SER_DISK, 0) << data).GetSHA256(); + out = (HashWriter{} << data).GetSHA256(); } MuHash3072& MuHash3072::operator*=(const MuHash3072& mul) noexcept diff --git a/src/hash.cpp b/src/hash.cpp index f58b29e3ba428..111b707964770 100644 --- a/src/hash.cpp +++ b/src/hash.cpp @@ -86,9 +86,9 @@ uint256 SHA256Uint256(const uint256& input) return result; } -CHashWriter TaggedHash(const std::string& tag) +HashWriter TaggedHash(const std::string& tag) { - CHashWriter writer(SER_GETHASH, 0); + HashWriter writer{}; uint256 taghash; CSHA256().Write((const unsigned char*)tag.data(), tag.size()).Finalize(taghash.begin()); writer << taghash << taghash; diff --git a/src/hash.h b/src/hash.h index 0f0c5d4b2cc13..b1ff3acc7df85 100644 --- a/src/hash.h +++ b/src/hash.h @@ -215,12 +215,12 @@ unsigned int MurmurHash3(unsigned int nHashSeed, Span vData void BIP32Hash(const ChainCode &chainCode, unsigned int nChild, unsigned char header, const unsigned char data[32], unsigned char output[64]); -/** Return a CHashWriter primed for tagged hashes (as specified in BIP 340). +/** Return a HashWriter primed for tagged hashes (as specified in BIP 340). * * The returned object will have SHA256(tag) written to it twice (= 64 bytes). * A tagged hash can be computed by feeding the message into this object, and - * then calling CHashWriter::GetSHA256(). + * then calling HashWriter::GetSHA256(). */ -CHashWriter TaggedHash(const std::string& tag); +HashWriter TaggedHash(const std::string& tag); #endif // BITCOIN_HASH_H diff --git a/src/kernel/coinstats.cpp b/src/kernel/coinstats.cpp index 8d948bfc1db80..06a4b8c974198 100644 --- a/src/kernel/coinstats.cpp +++ b/src/kernel/coinstats.cpp @@ -68,7 +68,7 @@ CDataStream TxOutSer(const COutPoint& outpoint, const Coin& coin) { //! It is also possible, though very unlikely, that a change in this //! construction could cause a previously invalid (and potentially malicious) //! UTXO snapshot to be considered valid. -static void ApplyHash(CHashWriter& ss, const uint256& hash, const std::map& outputs) +static void ApplyHash(HashWriter& ss, const uint256& hash, const std::map& outputs) { for (auto it = outputs.begin(); it != outputs.end(); ++it) { if (it == outputs.begin()) { @@ -159,7 +159,7 @@ std::optional ComputeUTXOStats(CoinStatsHashType hash_type, CCoinsV bool success = [&]() -> bool { switch (hash_type) { case(CoinStatsHashType::HASH_SERIALIZED): { - CHashWriter ss(SER_GETHASH, PROTOCOL_VERSION); + HashWriter ss{}; return ComputeUTXOStats(view, stats, ss, interruption_point); } case(CoinStatsHashType::MUHASH): { @@ -180,7 +180,7 @@ std::optional ComputeUTXOStats(CoinStatsHashType hash_type, CCoinsV } // The legacy hash serializes the hashBlock -static void PrepareHash(CHashWriter& ss, const CCoinsStats& stats) +static void PrepareHash(HashWriter& ss, const CCoinsStats& stats) { ss << stats.hashBlock; } @@ -188,7 +188,7 @@ static void PrepareHash(CHashWriter& ss, const CCoinsStats& stats) static void PrepareHash(MuHash3072& muhash, CCoinsStats& stats) {} static void PrepareHash(std::nullptr_t, CCoinsStats& stats) {} -static void FinalizeHash(CHashWriter& ss, CCoinsStats& stats) +static void FinalizeHash(HashWriter& ss, CCoinsStats& stats) { stats.hashSerialized = ss.GetHash(); } diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp index 103f4f0d7f8f2..bac05f6be269c 100644 --- a/src/node/blockstorage.cpp +++ b/src/node/blockstorage.cpp @@ -471,7 +471,7 @@ static bool UndoWriteToDisk(const CBlockUndo& blockundo, FlatFilePos& pos, const fileout << blockundo; // calculate & write checksum - CHashWriter hasher(SER_GETHASH, PROTOCOL_VERSION); + HashWriter hasher{}; hasher << hashBlock; hasher << blockundo; fileout << hasher.GetHash(); diff --git a/src/pubkey.cpp b/src/pubkey.cpp index 324f681a0a307..a4a1be6388a3d 100644 --- a/src/pubkey.cpp +++ b/src/pubkey.cpp @@ -211,16 +211,16 @@ bool XOnlyPubKey::VerifySchnorr(const uint256& msg, Span si return secp256k1_schnorrsig_verify(secp256k1_context_verify, sigbytes.data(), msg.begin(), 32, &pubkey); } -static const CHashWriter HASHER_TAPTWEAK = TaggedHash("TapTweak"); +static const HashWriter HASHER_TAPTWEAK{TaggedHash("TapTweak")}; uint256 XOnlyPubKey::ComputeTapTweakHash(const uint256* merkle_root) const { if (merkle_root == nullptr) { // We have no scripts. The actual tweak does not matter, but follow BIP341 here to // allow for reproducible tweaking. - return (CHashWriter(HASHER_TAPTWEAK) << m_keydata).GetSHA256(); + return (HashWriter{HASHER_TAPTWEAK} << m_keydata).GetSHA256(); } else { - return (CHashWriter(HASHER_TAPTWEAK) << m_keydata << *merkle_root).GetSHA256(); + return (HashWriter{HASHER_TAPTWEAK} << m_keydata << *merkle_root).GetSHA256(); } } diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp index 9f56301377b31..38bb11aad43a5 100644 --- a/src/script/interpreter.cpp +++ b/src/script/interpreter.cpp @@ -1342,7 +1342,7 @@ class CTransactionSignatureSerializer template uint256 GetPrevoutsSHA256(const T& txTo) { - CHashWriter ss(SER_GETHASH, 0); + HashWriter ss{}; for (const auto& txin : txTo.vin) { ss << txin.prevout; } @@ -1353,7 +1353,7 @@ uint256 GetPrevoutsSHA256(const T& txTo) template uint256 GetSequencesSHA256(const T& txTo) { - CHashWriter ss(SER_GETHASH, 0); + HashWriter ss{}; for (const auto& txin : txTo.vin) { ss << txin.nSequence; } @@ -1364,7 +1364,7 @@ uint256 GetSequencesSHA256(const T& txTo) template uint256 GetOutputsSHA256(const T& txTo) { - CHashWriter ss(SER_GETHASH, 0); + HashWriter ss{}; for (const auto& txout : txTo.vout) { ss << txout; } @@ -1374,7 +1374,7 @@ uint256 GetOutputsSHA256(const T& txTo) /** Compute the (single) SHA256 of the concatenation of all amounts spent by a tx. */ uint256 GetSpentAmountsSHA256(const std::vector& outputs_spent) { - CHashWriter ss(SER_GETHASH, 0); + HashWriter ss{}; for (const auto& txout : outputs_spent) { ss << txout.nValue; } @@ -1384,7 +1384,7 @@ uint256 GetSpentAmountsSHA256(const std::vector& outputs_spent) /** Compute the (single) SHA256 of the concatenation of all scriptPubKeys spent by a tx. */ uint256 GetSpentScriptsSHA256(const std::vector& outputs_spent) { - CHashWriter ss(SER_GETHASH, 0); + HashWriter ss{}; for (const auto& txout : outputs_spent) { ss << txout.scriptPubKey; } @@ -1458,9 +1458,9 @@ template void PrecomputedTransactionData::Init(const CMutableTransaction& txTo, template PrecomputedTransactionData::PrecomputedTransactionData(const CTransaction& txTo); template PrecomputedTransactionData::PrecomputedTransactionData(const CMutableTransaction& txTo); -const CHashWriter HASHER_TAPSIGHASH = TaggedHash("TapSighash"); -const CHashWriter HASHER_TAPLEAF = TaggedHash("TapLeaf"); -const CHashWriter HASHER_TAPBRANCH = TaggedHash("TapBranch"); +const HashWriter HASHER_TAPSIGHASH{TaggedHash("TapSighash")}; +const HashWriter HASHER_TAPLEAF{TaggedHash("TapLeaf")}; +const HashWriter HASHER_TAPBRANCH{TaggedHash("TapBranch")}; static bool HandleMissingData(MissingDataBehavior mdb) { @@ -1499,7 +1499,7 @@ bool SignatureHashSchnorr(uint256& hash_out, ScriptExecutionData& execdata, cons return HandleMissingData(mdb); } - CHashWriter ss = HASHER_TAPSIGHASH; + HashWriter ss{HASHER_TAPSIGHASH}; // Epoch static constexpr uint8_t EPOCH = 0; @@ -1544,7 +1544,7 @@ bool SignatureHashSchnorr(uint256& hash_out, ScriptExecutionData& execdata, cons if (output_type == SIGHASH_SINGLE) { if (in_pos >= tx_to.vout.size()) return false; if (!execdata.m_output_hash) { - CHashWriter sha_single_output(SER_GETHASH, 0); + HashWriter sha_single_output{}; sha_single_output << tx_to.vout[in_pos]; execdata.m_output_hash = sha_single_output.GetSHA256(); } @@ -1587,12 +1587,12 @@ uint256 SignatureHash(const CScript& scriptCode, const T& txTo, unsigned int nIn if ((nHashType & 0x1f) != SIGHASH_SINGLE && (nHashType & 0x1f) != SIGHASH_NONE) { hashOutputs = cacheready ? cache->hashOutputs : SHA256Uint256(GetOutputsSHA256(txTo)); } else if ((nHashType & 0x1f) == SIGHASH_SINGLE && nIn < txTo.vout.size()) { - CHashWriter ss(SER_GETHASH, 0); + HashWriter ss{}; ss << txTo.vout[nIn]; hashOutputs = ss.GetHash(); } - CHashWriter ss(SER_GETHASH, 0); + HashWriter ss{}; // Version ss << txTo.nVersion; // Input prevouts/nSequence (none/all, depending on flags) @@ -1627,7 +1627,7 @@ uint256 SignatureHash(const CScript& scriptCode, const T& txTo, unsigned int nIn CTransactionSignatureSerializer txTmp(txTo, scriptCode, nIn, nHashType); // Serialize and hash - CHashWriter ss(SER_GETHASH, 0); + HashWriter ss{}; ss << txTmp << nHashType; return ss.GetHash(); } @@ -1827,7 +1827,7 @@ static bool ExecuteWitnessScript(const Span& stack_span, const CS uint256 ComputeTapleafHash(uint8_t leaf_version, const CScript& script) { - return (CHashWriter(HASHER_TAPLEAF) << leaf_version << script).GetSHA256(); + return (HashWriter{HASHER_TAPLEAF} << leaf_version << script).GetSHA256(); } uint256 ComputeTaprootMerkleRoot(Span control, const uint256& tapleaf_hash) @@ -1839,7 +1839,7 @@ uint256 ComputeTaprootMerkleRoot(Span control, const uint25 const int path_len = (control.size() - TAPROOT_CONTROL_BASE_SIZE) / TAPROOT_CONTROL_NODE_SIZE; uint256 k = tapleaf_hash; for (int i = 0; i < path_len; ++i) { - CHashWriter ss_branch{HASHER_TAPBRANCH}; + HashWriter ss_branch{HASHER_TAPBRANCH}; Span node{Span{control}.subspan(TAPROOT_CONTROL_BASE_SIZE + TAPROOT_CONTROL_NODE_SIZE * i, TAPROOT_CONTROL_NODE_SIZE)}; if (std::lexicographical_compare(k.begin(), k.end(), node.begin(), node.end())) { ss_branch << k << node; @@ -1902,7 +1902,7 @@ static bool VerifyWitnessProgram(const CScriptWitness& witness, int witversion, if (stack.size() >= 2 && !stack.back().empty() && stack.back()[0] == ANNEX_TAG) { // Drop annex (this is non-standard; see IsWitnessStandard) const valtype& annex = SpanPopBack(stack); - execdata.m_annex_hash = (CHashWriter(SER_GETHASH, 0) << annex).GetSHA256(); + execdata.m_annex_hash = (HashWriter{} << annex).GetSHA256(); execdata.m_annex_present = true; } else { execdata.m_annex_present = false; diff --git a/src/script/interpreter.h b/src/script/interpreter.h index adf454aa1538a..f91578d6845b7 100644 --- a/src/script/interpreter.h +++ b/src/script/interpreter.h @@ -233,9 +233,9 @@ static constexpr size_t TAPROOT_CONTROL_NODE_SIZE = 32; static constexpr size_t TAPROOT_CONTROL_MAX_NODE_COUNT = 128; static constexpr size_t TAPROOT_CONTROL_MAX_SIZE = TAPROOT_CONTROL_BASE_SIZE + TAPROOT_CONTROL_NODE_SIZE * TAPROOT_CONTROL_MAX_NODE_COUNT; -extern const CHashWriter HASHER_TAPSIGHASH; //!< Hasher with tag "TapSighash" pre-fed to it. -extern const CHashWriter HASHER_TAPLEAF; //!< Hasher with tag "TapLeaf" pre-fed to it. -extern const CHashWriter HASHER_TAPBRANCH; //!< Hasher with tag "TapBranch" pre-fed to it. +extern const HashWriter HASHER_TAPSIGHASH; //!< Hasher with tag "TapSighash" pre-fed to it. +extern const HashWriter HASHER_TAPLEAF; //!< Hasher with tag "TapLeaf" pre-fed to it. +extern const HashWriter HASHER_TAPBRANCH; //!< Hasher with tag "TapBranch" pre-fed to it. template uint256 SignatureHash(const CScript& scriptCode, const T& txTo, unsigned int nIn, int nHashType, const CAmount& amount, SigVersion sigversion, const PrecomputedTransactionData* cache = nullptr); diff --git a/src/script/sign.cpp b/src/script/sign.cpp index a3681d26ccb2d..3b8071d9d1c0d 100644 --- a/src/script/sign.cpp +++ b/src/script/sign.cpp @@ -165,7 +165,7 @@ static bool SignTaprootScript(const SigningProvider& provider, const BaseSignatu if (leaf_version != TAPROOT_LEAF_TAPSCRIPT) return false; SigVersion sigversion = SigVersion::TAPSCRIPT; - uint256 leaf_hash = (CHashWriter(HASHER_TAPLEAF) << uint8_t(leaf_version) << script).GetSHA256(); + uint256 leaf_hash = (HashWriter{HASHER_TAPLEAF} << uint8_t(leaf_version) << script).GetSHA256(); // OP_CHECKSIG if (script.size() == 34 && script[33] == OP_CHECKSIG && script[0] == 0x20) { diff --git a/src/script/standard.cpp b/src/script/standard.cpp index 5d8089148549b..b3f6a1b669895 100644 --- a/src/script/standard.cpp +++ b/src/script/standard.cpp @@ -375,9 +375,9 @@ bool IsValidDestination(const CTxDestination& dest) { } /* Lexicographically sort a and b's hash, and compute parent hash. */ if (a.hash < b.hash) { - ret.hash = (CHashWriter(HASHER_TAPBRANCH) << a.hash << b.hash).GetSHA256(); + ret.hash = (HashWriter{HASHER_TAPBRANCH} << a.hash << b.hash).GetSHA256(); } else { - ret.hash = (CHashWriter(HASHER_TAPBRANCH) << b.hash << a.hash).GetSHA256(); + ret.hash = (HashWriter{HASHER_TAPBRANCH} << b.hash << a.hash).GetSHA256(); } return ret; } @@ -452,7 +452,7 @@ TaprootBuilder& TaprootBuilder::Add(int depth, const CScript& script, int leaf_v if (!IsValid()) return *this; /* Construct NodeInfo object with leaf hash and (if track is true) also leaf information. */ NodeInfo node; - node.hash = (CHashWriter{HASHER_TAPLEAF} << uint8_t(leaf_version) << script).GetSHA256(); + node.hash = (HashWriter{HASHER_TAPLEAF} << uint8_t(leaf_version) << script).GetSHA256(); if (track) node.leaves.emplace_back(LeafInfo{script, leaf_version, {}}); /* Insert into the branch. */ Insert(std::move(node), depth); @@ -610,7 +610,7 @@ std::optional>> InferTaprootTree(const node.done = true; stack.pop_back(); } else if (node.sub[0]->done && !node.sub[1]->done && !node.sub[1]->explored && !node.sub[1]->hash.IsNull() && - (CHashWriter{HASHER_TAPBRANCH} << node.sub[1]->hash << node.sub[1]->hash).GetSHA256() == node.hash) { + (HashWriter{HASHER_TAPBRANCH} << node.sub[1]->hash << node.sub[1]->hash).GetSHA256() == node.hash) { // Whenever there are nodes with two identical subtrees under it, we run into a problem: // the control blocks for the leaves underneath those will be identical as well, and thus // they will all be matched to the same path in the tree. The result is that at the location diff --git a/src/test/script_tests.cpp b/src/test/script_tests.cpp index 05bb89ab55441..fbcf1f14efc77 100644 --- a/src/test/script_tests.cpp +++ b/src/test/script_tests.cpp @@ -1813,7 +1813,7 @@ BOOST_AUTO_TEST_CASE(bip341_keypath_test_vectors) BOOST_CHECK_EQUAL(HexStr(sighash), input["intermediary"]["sigHash"].get_str()); // To verify the sigmsg, hash the expected sigmsg, and compare it with the (expected) sighash. - BOOST_CHECK_EQUAL(HexStr((CHashWriter(HASHER_TAPSIGHASH) << Span{ParseHex(input["intermediary"]["sigMsg"].get_str())}).GetSHA256()), input["intermediary"]["sigHash"].get_str()); + BOOST_CHECK_EQUAL(HexStr((HashWriter{HASHER_TAPSIGHASH} << Span{ParseHex(input["intermediary"]["sigMsg"].get_str())}).GetSHA256()), input["intermediary"]["sigHash"].get_str()); } } diff --git a/src/util/message.cpp b/src/util/message.cpp index d395c4b0bccd1..028251a5a80ae 100644 --- a/src/util/message.cpp +++ b/src/util/message.cpp @@ -74,7 +74,7 @@ bool MessageSign( uint256 MessageHash(const std::string& message) { - CHashWriter hasher(SER_GETHASH, 0); + HashWriter hasher{}; hasher << MESSAGE_MAGIC << message; return hasher.GetHash(); diff --git a/src/wallet/dump.cpp b/src/wallet/dump.cpp index d80c3e25b03aa..f7fee443d0743 100644 --- a/src/wallet/dump.cpp +++ b/src/wallet/dump.cpp @@ -41,7 +41,7 @@ bool DumpWallet(const ArgsManager& args, CWallet& wallet, bilingual_str& error) return false; } - CHashWriter hasher(0, 0); + HashWriter hasher{}; WalletDatabase& db = wallet.GetDatabase(); std::unique_ptr batch = db.MakeBatch(); @@ -132,7 +132,7 @@ bool CreateFromDump(const ArgsManager& args, const std::string& name, const fs:: std::ifstream dump_file{dump_path}; // Compute the checksum - CHashWriter hasher(0, 0); + HashWriter hasher{}; uint256 checksum; // Check the magic and version