This looks like an ad-hoc deserialization code for PresaltedSipHasher - could we encapsulate that inside the object itself?
diff --git a/src/crypto/siphash.h b/src/crypto/siphash.h
index 2f28473a4f..e1eeeb25c5 100644
--- a/src/crypto/siphash.h
+++ b/src/crypto/siphash.h
@@ -10,10 +10,13 @@
#include <span>
class uint256;
+class PresaltedSipHasher;
/** Shared SipHash internal state v[0..3], initialized from (k0, k1). */
class SipHashState
{
+ friend class PresaltedSipHasher;
+
static constexpr uint64_t C0{0x736f6d6570736575ULL}, C1{0x646f72616e646f6dULL}, C2{0x6c7967656e657261ULL}, C3{0x7465646279746573ULL};
public:
@@ -48,17 +51,32 @@ public:
*
* This class caches the initial SipHash v[0..3] state derived from (k0, k1)
* and implements a specialized hashing path for uint256 values, with or
- * without an extra 32-bit word. The internal state is immutable, so
- * PresaltedSipHasher instances can be reused for multiple hashes with the
- * same key.
+ * without an extra 32-bit word. The call operators leave the cached state
+ * unchanged, so PresaltedSipHasher instances can be reused for multiple hashes
+ * with the same key.
*/
class PresaltedSipHasher
{
- const SipHashState m_state;
+ SipHashState m_state;
public:
+ PresaltedSipHasher() noexcept : PresaltedSipHasher{0, 0} {}
explicit PresaltedSipHasher(uint64_t k0, uint64_t k1) noexcept : m_state{k0, k1} {}
+ template <typename Stream>
+ void Serialize(Stream& s) const
+ {
+ s << (m_state.v[0] ^ SipHashState::C0) << (m_state.v[1] ^ SipHashState::C1);
+ }
+
+ template <typename Stream>
+ void Unserialize(Stream& s)
+ {
+ uint64_t k0, k1;
+ s >> k0 >> k1;
+ m_state = SipHashState{k0, k1};
+ }
+
/** Equivalent to CSipHasher(k0, k1).Write(val).Finalize(). */
uint64_t operator()(const uint256& val) const noexcept;
diff --git a/src/index/txindex.cpp b/src/index/txindex.cpp
index a71e8046f7..404b57a56a 100644
--- a/src/index/txindex.cpp
+++ b/src/index/txindex.cpp
@@ -21,6 +21,7 @@
#include <streams.h>
#include <sync.h>
#include <uint256.h>
+#include <util/check.h>
#include <util/fs.h>
#include <util/log.h>
#include <validation.h>
@@ -63,13 +64,14 @@ public:
TxIndex::DB::DB(size_t n_cache_size, bool f_memory, bool f_wipe) :
BaseIndex::DB(gArgs.GetDataDirNet() / "indexes" / "txindex", n_cache_size, f_memory, f_wipe),
m_hasher{[](CDBWrapper& db) {
- std::pair<uint64_t, uint64_t> siphash_key;
- if (!db.Read("siphash_key", siphash_key)) {
+ PresaltedSipHasher hasher;
+ if (!db.Read(txindex::DB_TXID_HASH_SALT, hasher)) {
+ // The salt only needs to be generated once and persisted.
FastRandomContext rng{};
- siphash_key = {rng.rand64(), rng.rand64()};
- db.Write("siphash_key", siphash_key, /*fSync=*/true);
+ db.Write(txindex::DB_TXID_HASH_SALT, PresaltedSipHasher{rng.rand64(), rng.rand64()}, /*fSync=*/true);
+ Assert(db.Read(txindex::DB_TXID_HASH_SALT, hasher));
}
- return PresaltedSipHasher{siphash_key.first, siphash_key.second};
+ return hasher;
}(*this)}
{}
diff --git a/src/index/txindex_key.h b/src/index/txindex_key.h
index 025638c36a..8228de8075 100644
--- a/src/index/txindex_key.h
+++ b/src/index/txindex_key.h
@@ -15,9 +15,11 @@
#include <cstddef>
#include <cstdint>
#include <ios>
+#include <string>
namespace txindex {
constexpr uint8_t DB_TXINDEX_HASHED{'x'};
+static const std::string DB_TXID_HASH_SALT{"txid_hash_salt"};
//! The location of a transaction: the height of the block that contains it and the
//! transaction's byte offset within that block (after the header).
diff --git a/src/test/hash_tests.cpp b/src/test/hash_tests.cpp
index a5059a8fe8..d51fbecf9f 100644
--- a/src/test/hash_tests.cpp
+++ b/src/test/hash_tests.cpp
@@ -5,6 +5,7 @@
#include <clientversion.h>
#include <crypto/siphash.h>
#include <hash.h>
+#include <streams.h>
#include <test/util/random.h>
#include <test/util/setup_common.h>
#include <util/strencodings.h>
@@ -128,7 +129,14 @@ BOOST_AUTO_TEST_CASE(siphash)
// and the test would be affected by default tx version bumps if not fixed.
tx.version = 1;
ss << TX_WITH_WITNESS(tx);
- BOOST_CHECK_EQUAL(PresaltedSipHasher(1, 2)(ss.GetHash()), 0x79751e980c2a0a35ULL);
+ const uint256 tx_hash{ss.GetHash()};
+ BOOST_CHECK_EQUAL(PresaltedSipHasher(1, 2)(tx_hash), 0x79751e980c2a0a35ULL);
+
+ PresaltedSipHasher roundtrip_hasher;
+ DataStream serialized_hasher;
+ serialized_hasher << PresaltedSipHasher{1, 2};
+ serialized_hasher >> roundtrip_hasher;
+ BOOST_CHECK_EQUAL(roundtrip_hasher(tx_hash), 0x79751e980c2a0a35ULL);
// Check consistency between CSipHasher and PresaltedSipHasher.
FastRandomContext ctx;
diff --git a/src/test/txindex_tests.cpp b/src/test/txindex_tests.cpp
index 7a01325102..c9a65fe7f7 100644
--- a/src/test/txindex_tests.cpp
+++ b/src/test/txindex_tests.cpp
@@ -99,9 +99,8 @@ BOOST_FIXTURE_TEST_CASE(txindex_collision_scan_path, TestChain100Setup)
CDBWrapper& db{TxIndexTest::GetDB(txindex)};
ChainstateManager& chainman{*m_node.chainman};
- std::pair<uint64_t, uint64_t> siphash_key;
- BOOST_REQUIRE(db.Read("siphash_key", siphash_key));
- const PresaltedSipHasher hasher{siphash_key.first, siphash_key.second};
+ PresaltedSipHasher hasher;
+ BOOST_REQUIRE(db.Read(txindex::DB_TXID_HASH_SALT, hasher));
// Resolve a position to its physical on-disk location via the active chain, the
// same way TxIndex::FindTx does.
