coins: use SipHash-1-3-UJ for CCoinsMap keys #35215

pull l0rinc wants to merge 8 commits into bitcoin:master from l0rinc:l0rinc/siphash-jumbo changing 16 files +1168 −162
  1. l0rinc commented at 2:02 PM on May 5, 2026: contributor

    Problem: The in-memory UTXO cache hashes COutPoint keys containing a 32-byte txid and a 32-bit output index. SipHash-2-4 processes the txid as four independent 64-bit blocks, so its optimized 32-byte and 36-byte paths both take 14 SipRounds. This also matters for hash-prefix index work such as #35531: once a persisted key format chooses a hash function, changing it later requires reindexing.

    Fix: Add SipHasher13UJ, a custom block-oriented variant combining Pieter Wuille's jumbo-block suggestion with SipHash-1-3, the reduced-round variant discussed in the SipHash analysis. It provides inline Hash overloads for the fixed-width inputs used here. Use a dedicated SaltedCoinsCacheHasher for CCoinsMap and CoinsViewOverlay's temporary earlier-txid set, while other outpoint tables remain on SipHash-2-4. The salted hash values vary between restarts and are never persisted or sent over the network.

    Design: SipHasher13UJ accepts normal 64-bit blocks and 256-bit jumbo blocks. For hash-table use, cryptographic hash outputs must make up all but a small bounded number of retained jumbo blocks. The construction mixes all four limbs around one SipRound, omits byte-oriented padding, and uses an "unpadded" finalizer distinct from standard SipHash-1-3. The fixed-width paths take four rounds for one uint256 jumbo block and five when followed by one normal block. For outpoints, the 32-bit output index is zero-extended into a normal 64-bit block.

    Retained CCoinsMap entries identify real transaction outputs, so their keys contain computed txids. Missing-input validation may probe arbitrary claimed prevouts, but FetchCoin() immediately erases their temporary entries when the backend lookup fails, so non-hash keys cannot accumulate. The assumeutxo loader assumes snapshot txids are valid while loading and verifies the complete snapshot's content hash before activation. Every entry in the temporary earlier-txid set is a computed transaction hash, and the set is bounded by the block's transaction count.

    This construction is limited to local hash tables and is not a general-purpose or protocol SipHash replacement. Pieter discussed the construction with SipHash co-author Jean-Philippe Aumasson, whose preliminary analysis did not find an easier collision construction and supported SipHash-1-3 for this hash-table use.

    <img width="2100" height="860" alt="siphash_compare_updated" src="https://github.com/user-attachments/assets/cefec6f8-5ec0-450a-a0a2-f946de9ef36d" />

    Structure: Shared vectors first cover the existing generic and fixed SipHash-2-4 paths in C++, the generic path in Python, and their randomized equivalence in the fuzzer. A behavior-neutral refactor then moves the round, compression, and finalization logic into inline SipHashState methods; assembly inspection shows that the fixed-width paths retain their instruction counts, while the generic byte loop retains its prior code generation through a local state copy. Three Pieter-authored commits add the generic UJ specification, fixed-width implementation, and shared correctness coverage. Benchmarks follow that coverage, then separate commits change CCoinsMap's hasher and reuse it for the temporary earlier-txid set.

    Tests: The shared JSON supplies the same byte sequences to the generic C++ and Python SipHash-2-4 implementations, with applicable fixed-width paths checked against the same expected output. The SipHash-2-4 rows include the 64 official vectors for inputs from 0 to 63 bytes and cases that vary input chunking. The UJ outputs were generated by an independent implementation and are checked using normal blocks, equivalent zero-extended jumbo blocks, and applicable fixed-width Hash overloads. The integer fuzzer extends these comparisons to arbitrary values and mixed normal/jumbo block encodings.

    Counting the dbcache buckets indicates the new outpoint hasher retains the uniform bucket distribution expected by CCoinsMap: <img width="1200" height="750" alt="ccoinsmap-collisions" src="https://github.com/user-attachments/assets/eeedec81-acdc-4adf-a9c8-bfce089700da" />

    Benchmarks: Fixed-width microbenchmarks compare SipHash-2-4 with SipHash-1-3-UJ for 32-byte hashes and inputs containing a 32-byte hash plus a 32-bit index. Reported aarch64 measurements and an independent x86_64 run show the outpoint path is about 2x faster.

    <details><summary>Benchmark runner</summary>

    for COMPILER in gcc clang; do \
      if [ "$COMPILER" = gcc ]; then CC=gcc; CXX=g++; else CC=clang; CXX=clang++; fi; \
      cmake -B "build-bench-$COMPILER" -DCMAKE_BUILD_TYPE=Release -DBUILD_BENCH=ON -DBUILD_TESTS=OFF -DBUILD_GUI=OFF -DENABLE_WALLET=OFF -DCMAKE_C_COMPILER="$CC" -DCMAKE_CXX_COMPILER="$CXX" >/dev/null 2>&1 && \
      cmake --build "build-bench-$COMPILER" --target bench_bitcoin -j"$(nproc)" >/dev/null 2>&1 && \
      echo "" && echo "$(date -I) | SipHash fixed-width microbench | $("$CXX" --version | head -1) | $(hostname) | $(uname -m) | $(lscpu | awk -F: '/Model name/{print $2; exit}' | xargs) | $(nproc) cores | $(free -h | awk '/^Mem:/{print $2}') RAM" && \
      "build-bench-$COMPILER/bin/bench_bitcoin" -filter='SipHash.*32b|SipHash.*36b' -min-time=10000; \
    done
    

    </details>

    A two-run GCC -reindex-chainstate comparison of the same CCoinsMap hot path through height 957,759 with -dbcache=2000 on a Ryzen 7 3700X/SSD reduced mean wall time from 11,278 s to 10,759 s, a ~5% validation speedup.

  2. DrahtBot added the label UTXO Db and Indexes on May 5, 2026
  3. DrahtBot commented at 2:02 PM on May 5, 2026: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    <!--006a51241073e994b41acfe9ec718e94-->

    Code Coverage & Benchmarks

    For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/35215.

    <!--021abf342d371248e50ceaed478a90ca-->

    Reviews

    See the guideline and AI policy for information on the review process.

    Type Reviewers
    ACK sipa, andrewtoth, optout21, achow101
    Concept ACK sedited, theStack

    If your review is incorrectly listed, please copy-paste <code>&lt;!--meta-tag:bot-skip--&gt;</code> into the comment that the bot should ignore.

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #35195 (coins: cache UTXO outpoint hash codes by l0rinc)
    • #34844 (util: Add util::NotNull<SmartPtrType> by maflcko)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

    <!--5faf32d7da4f0f540f40219e4f7537a3-->

  4. l0rinc force-pushed on May 5, 2026
  5. DrahtBot added the label CI failed on May 5, 2026
  6. DrahtBot commented at 7:27 PM on May 5, 2026: contributor

    <!--85328a0da195eb286784d51f73fa0af9-->

    🚧 At least one of the CI tasks failed. <sub>Task 32 bit ARM: https://github.com/bitcoin/bitcoin/actions/runs/25393628377/job/74474475011</sub> <sub>LLM reason (✨ experimental): CI failed due to a C++ build error: hash_tests.cpp couldn’t compile because uint256’s consteval hex parsing wasn’t a constant expression.</sub>

    <details><summary>Hints</summary>

    Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

    • Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

    • A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.

    • An intermittent issue.

    Leave a comment here, if you need help tracking down a confusing failure.

    </details>

  7. l0rinc force-pushed on May 5, 2026
  8. DrahtBot removed the label CI failed on May 5, 2026
  9. veorq commented at 5:20 AM on May 6, 2026: contributor

    FTR I confirm my statements quoted by OP

    Pieter also ran the jumboblock idea by Jean-Philippe Aumasson, one of the SipHash authors; based on a preliminary analysis, Aumasson did not think this made collisions easier to construct. Aumasson also said SipHash-1-3 is fine for this hashmap use case and offered to comment on or review the PR.

  10. ViniciusCestarii commented at 7:57 PM on May 7, 2026: contributor

    Nice. Ran the microbench and it reproduces ~2x on x86_64 as well.

    archlinux | i7-13650HX | 20 cores | 32GB RAM

    gcc 15.2.1: 31.57 ns -> 16.45 ns (1.92x) clang 22.1.3: 32.39 ns -> 16.73 ns (1.94x)

    <details> <summary>Full bench tables</summary>

    2026-05-07 | SipHash 36-byte microbench | gcc (GCC) 15.2.1 20260209 | g++ (GCC) 15.2.1 20260209 | archlinux | x86_64 | 13th Gen Intel(R) Core(TM) i7-13650HX | 20 cores | 31Gi RAM

    ns/op op/s err% ins/op cyc/op IPC bra/op miss% total benchmark
    16.45 60,793,565.44 0.1% 105.00 45.97 2.284 0.00 29.9% 11.00 SipHash13Jumbo_36b
    31.57 31,678,908.80 0.2% 254.00 88.23 2.879 3.00 0.0% 10.98 SipHash24_36b

    2026-05-07 | SipHash 36-byte microbench | clang version 22.1.3 | clang version 22.1.3 | archlinux | x86_64 | 13th Gen Intel(R) Core(TM) i7-13650HX | 20 cores | 31Gi RAM

    ns/op op/s err% ins/op cyc/op IPC bra/op miss% total benchmark
    16.73 59,756,190.42 0.1% 106.00 46.77 2.267 0.00 1.6% 11.01 SipHash13Jumbo_36b
    32.39 30,871,809.86 0.1% 246.00 90.51 2.718 3.00 0.0% 10.99 SipHash24_36b

    </details>

  11. optout21 commented at 11:45 AM on May 29, 2026: contributor

    I'd share an additional data point here: pushing the idea further, I've explored a bit using a simpler, non-SipHash XOR-only hasher. My conclusion is that that's not a viable solution, I'm only sharing it here as an additional data point (as a kind of lower bound on the hashing time). If pre-change (PresaltedSipHasher) hashing time is 100%, this PR (PresaltedSipHasher13Jumbo) is 50%, XOR-only (XorHasher6x8_2_36b) is 22% (lower means faster). The XOR-only hasher only XOR's together a salt, the four 8-byte parts of the TXID, and the vout (6 parts, 5 XOR's). While the solution may be usable, there are uncertainties stemming from the fact that it's not a secure hash, while the additional performance gain is rather small. The proposal of this PR is sound, and has a better reward/risk ratio. The code delta and a bit more details are available here: https://github.com/optout21/bitcoin/pull/12

  12. sedited commented at 11:59 AM on May 29, 2026: contributor

    Concept ACK

  13. optout21 commented at 12:03 PM on May 29, 2026: contributor

    Concept ACK

    What I would highlight:

    • TXID is a hash already (making the requirements on the hasher less strict)
    • the presented performance improvement
    • the presented bucket analysis
  14. l0rinc force-pushed on Jun 16, 2026
  15. l0rinc commented at 10:12 PM on June 16, 2026: contributor

    Added a simpler 32 byte PresaltedSipHasher13Jumbo hasher for #35531 (+ rebase and code simplification + PR description adjustments)

  16. in src/crypto/siphash.h:18 in 4d00740e29 outdated
      13 |  #include <span>
      14 | +#include <uint256.h>
      15 | +
      16 | +namespace siphash_detail {
      17 | +
      18 | +ALWAYS_INLINE void SipRound(uint64_t& v0, uint64_t& v1, uint64_t& v2, uint64_t& v3)
    


    optout21 commented at 8:26 AM on June 17, 2026:

    4d00740 crypto: inline jumboblock SipHash:

    I haven't seen documented the rationale for switching from preprocessor macro to inline method. Is it just code style or performance? I'm just curious, not opposing.

    Also, "Move siphash_detail::SipRound" in the commit description is not entirely correct, as pre-commit SipRound didn't exist, only SIPROUND. Could you correct it?


    l0rinc commented at 8:41 AM on June 17, 2026:

    The last commit inlines this after the benchmarks to prove that inlining speeds it up - which wasn't the case for previous SipHash implementations. Switching from macros is mostly a modernization attempt now that C++ has proper alternatives.

  17. optout21 commented at 8:27 AM on June 17, 2026: contributor

    Concept ACK (4d00740e2921c09a717bcf1964b94780a64757bb)

    Re-reviewed, including the new 32-byte version of Jumbo. Verified benchmarks locally.

  18. DrahtBot requested review from sedited on Jun 17, 2026
  19. andrewtoth approved
  20. andrewtoth commented at 11:23 PM on June 18, 2026: contributor

    ACK 4d00740e2921c09a717bcf1964b94780a64757bb

    Verified the speedups. I'm not an expert on hashing, but it makes sense to me that if we're hashing a 32-byte cryptographic hash we can do less work since an attacker can't control most of the input either. The attestation of the author of siphash gives me confidence in this change as well.

    variant SipHash-2-4 (ns/op) SipHash13Jumbo (ns/op) speedup
    32-byte 17.46 7.87 2.22x
    36-byte 17.14 8.75 1.96x
  21. DrahtBot requested review from optout21 on Jun 19, 2026
  22. optout21 commented at 7:07 AM on June 22, 2026: contributor

    I've performed 'bit-flip' tests on the new and old SipHasher. The results are good, as expected, virtually identical.

    property SipHasher SipHasher13Jumbo
    mean output-bit flip fraction per input-bit flip (ideal 0.5): 0.5000 0.5000
    bit-independence mean (deviation from 0.5): 0.0040 0.0040
    bit-independence max (deviation from 0.5): 0.0191 0.0193
  23. sedited removed review request from optout21 on Jun 24, 2026
  24. sedited requested review from theStack on Jun 24, 2026
  25. theStack commented at 9:45 PM on June 28, 2026: contributor

    Concept ACK

  26. DrahtBot requested review from optout21 on Jun 28, 2026
  27. l0rinc renamed this:
    coins: use jumboblock SipHash-1-3 for hashing CCoinsMap keys
    coins: use SipHash-1-3-UJ for outpoint hashing
    on Jul 16, 2026
  28. l0rinc force-pushed on Jul 16, 2026
  29. l0rinc commented at 6:25 AM on July 16, 2026: contributor

    Rebased and substantially reworked the PR around Pieter's 202607_siphash13uj branch.

    Compared with the previous fixed-width-only push, this stack first covers and deduplicates SipHash-2-4 so Pieter's generic SipHasher13UJ and optimized fixed paths can be added with smaller diffs. The forced-inline helpers preserve the existing instruction counts.

    The rewritten construction uses Pieter's normal-block-compatible jumboblock mixing and distinct "unpadded" finalizer, so its process-local outputs differ from the previous push. The generic construction, fixed specialization, and shared coverage remain separate Pieter-authored commits, while adoption is isolated in the final commit.

    One JSON file now holds the deterministic SipHash-2-4 and SipHash-1-3-UJ cases, preserving all 64 unique inputs from Pieter's tests. The same data checks the generic and fixed C++ SipHash-2-4 paths and the Python implementation, while applicable UJ rows compare both implementations against one expected output. Randomized generic/fixed and mixed block-encoding comparisons moved to the fuzzer, and the two benchmark commits were combined.

    Fresh Ryzen 7 3700X measurements reproduce the previous speedup:

    • GCC: 2.13× for 32 bytes and 1.84× for 36 bytes
    • Clang: 2.24× for 32 bytes and 1.92× for 36 bytes

    <details> <summary>GCC and Clang benchmark output</summary>

    2026-07-16 | SipHash fixed-width microbench | g++ (Ubuntu 15.2.0-16ubuntu1) 15.2.0 | ssd-ryzen | x86_64 | AMD Ryzen 7 3700X 8-Core Processor | 16 cores | 62Gi RAM

    ns/op op/s err% ins/op cyc/op IPC bra/op miss% total benchmark
    9.51 105,123,046.52 0.0% 85.00 41.09 2.069 0.00 99.9% 11.00 SipHash13UJ_32b
    11.10 90,128,097.48 0.0% 104.00 47.95 2.169 0.00 99.9% 11.01 SipHash13UJ_36b
    20.21 49,480,079.93 0.0% 241.00 87.17 2.765 3.00 0.0% 11.00 SipHash24_32b
    20.38 49,077,937.23 0.1% 248.00 87.95 2.820 3.00 0.0% 10.99 SipHash24_36b

    2026-07-16 | SipHash fixed-width microbench | Ubuntu clang version 21.1.8 (6ubuntu1) | ssd-ryzen | x86_64 | AMD Ryzen 7 3700X 8-Core Processor | 16 cores | 62Gi RAM

    ns/op op/s err% ins/op cyc/op IPC bra/op miss% total benchmark
    9.41 106,276,873.66 0.4% 84.00 40.28 2.086 0.00 99.9% 10.96 SipHash13UJ_32b
    11.10 90,104,574.39 0.1% 106.00 47.67 2.224 0.00 99.9% 10.98 SipHash13UJ_36b
    21.11 47,381,064.81 0.1% 237.00 90.11 2.630 3.00 0.0% 10.98 SipHash24_32b
    21.35 46,833,237.21 0.1% 246.00 91.20 2.697 3.00 0.0% 10.99 SipHash24_36b

    </details>

  30. DrahtBot added the label CI failed on Jul 16, 2026
  31. DrahtBot commented at 7:55 AM on July 16, 2026: contributor

    <!--85328a0da195eb286784d51f73fa0af9-->

    🚧 At least one of the CI tasks failed. <sub>Task riscv32 bare metal, static libbitcoin_consensus: https://github.com/bitcoin/bitcoin/actions/runs/29476532704/job/87550625070</sub> <sub>LLM reason (✨ experimental): CI failed because the RISC-V toolchain submodule checkout (binutils/newlib) failed due to repeated HTTP 502 “remote end hung up unexpectedly” when fetching from sourceware.org (exit code 128).</sub>

    <details><summary>Hints</summary>

    Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

    • Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

    • A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.

    • An intermittent issue.

    Leave a comment here, if you need help tracking down a confusing failure.

    </details>

  32. in test/functional/test_framework/crypto/siphash.py:75 in 138f373492
      67 | @@ -63,3 +68,23 @@ def siphash(k0, k1, data):
      68 |  def siphash256(k0, k1, num):
      69 |      assert type(num) is int
      70 |      return siphash(k0, k1, num.to_bytes(32, 'little'))
      71 | +
      72 | +
      73 | +class TestFrameworkSipHash(unittest.TestCase):
      74 | +    def test_vectors(self):
      75 | +        with (Path(__file__).resolve().parents[4] / "src/test/data/siphash.json").open(encoding="utf8") as vectors_file:
    


    maflcko commented at 9:31 AM on July 16, 2026:

    nit: Please don't specify the encoding for normal files. See fae612424b3e70acd6011a4459518174463b3424

    Also, this fails CI:

    FileNotFoundError: [Errno 2] No such file or directory: 'D:\a\bitcoin\bitcoin\build\src\test\data\siphash.json'

    The reason is that Windows creates a copy, and Linux creates a link of __file__, so resolve() will lead to different results


    l0rinc commented at 3:54 PM on July 16, 2026:

    Thanks, fixed! Instead of working around the OS differences, added a CMake copy for the new siphash.json - also removed the explicit encoding.

  33. optout21 commented at 1:03 PM on July 16, 2026: contributor

    Concept ACK (138f37349222a0978451f92af130a0129a3c7bcd)

    Recent changes: Nice reorganization, improved code reuse without sacrificing performance. I will do my best to perform a full review.

  34. l0rinc force-pushed on Jul 16, 2026
  35. sipa commented at 4:54 PM on July 16, 2026: member

    I think the first commit's JSON tests can be unified a bit more.

    What about:

    {
        "key": ["0706050403020100", "0f0e0d0c0b0a0908"],
        "input": ["0000000000000000000000000000000000000000000000000000000000000000", "0000000000000000", "0000000000000000"],
        "expected": {"siphash24": "8990d3e4299496f4", "siphash13uj": "abcdef0123456789"}
      },
    

    The input is the full input to the hash function, as blocks of byte arrays in hex; no separate "hash" or "b", or "extras". It's interpreted as:

    • For siphash24: all input blocks are fed one by one using CSipHasher::Write(std::span<const unsigned char> data) (or , alternatively, all concatenated)
      • Presalted (uint256): only invoked when the input consists of exactly one 32-byte block (or, alternatively, when the concatenation is 32 bytes)
      • Presalted (uint256, uint32_t): only invoked when the input consists of exactly one 32-byte blocks followed by a 4-byte one (or, alternatively, when the concatenation is 36 bytes)
    • For siphash13uj: assert that all blocks are exactly 32 or 8 bytes, feed then one by use using Write and WriteJumbo.
      • Presalted (uint256): only invoked when the input consists of one 32-byte block
      • Presalted (uint256, uint64_t): only invoked when the input consists of one 32-byte blocks followed by a 8-byte block
  36. DrahtBot removed the label CI failed on Jul 16, 2026
  37. l0rinc renamed this:
    coins: use SipHash-1-3-UJ for outpoint hashing
    coins: use SipHash-1-3-UJ for CCoinsMap keys
    on Jul 16, 2026
  38. l0rinc force-pushed on Jul 16, 2026
  39. l0rinc commented at 11:17 PM on July 16, 2026: contributor

    Reworked the stack based on the latest review. The main changes since the previous push are:

    • Applied Pieter's JSON simplification: every vector now uses key, ordered raw-byte input blocks, and expected outputs. The C++ and Python SipHash-2-4 implementations consume the same bytes, while compatible 8- and 32-byte block sequences also carry UJ outputs and exercise the applicable fixed-width paths.
    • Merged PresaltedSipHasher13UJ into SipHasher13UJ as const inline Hash overloads, so the generic and fixed-width APIs share the same initialized or accumulated state. The internal block helpers are now consistently named Process24 and Process13UJ.
    • Extended the tests to cover Hash after earlier writes, while retaining deterministic normal/zero-extended-jumbo checks and randomized mixed block encodings in the fuzzer. The documentation now permits a small bounded number of retained non-hash jumbo blocks and covers the assumeutxo loader's validity assumption and later content-hash verification.
    • Narrowed the production change from every SaltedOutpointHasher user to a dedicated SaltedCoinsCacheHasher used only by CCoinsMap. Other outpoint tables remain on SipHash-2-4 because they may retain peer-supplied prevouts, while failed CCoinsMap lookups erase temporary non-hash entries immediately.

    Fresh benchmarks on the rewritten implementation confirm performance was retained: UJ is 2.13x/1.83x faster for the 32-byte/outpoint cases with GCC and 2.33x/1.92x with Clang.

    <details> <summary>GCC and Clang benchmark output</summary>

    for COMPILER in gcc clang; do \
      if [ "$COMPILER" = gcc ]; then CC=gcc; CXX=g++; else CC=clang; CXX=clang++; fi; \
      cmake -B "build-bench-$COMPILER" -DCMAKE_BUILD_TYPE=Release -DBUILD_BENCH=ON -DBUILD_TESTS=OFF -DBUILD_GUI=OFF -DENABLE_WALLET=OFF -DCMAKE_C_COMPILER="$CC" -DCMAKE_CXX_COMPILER="$CXX" >/dev/null 2>&1 && \
      cmake --build "build-bench-$COMPILER" --target bench_bitcoin -j"$(nproc)" >/dev/null 2>&1 && \
      echo "" && echo "$(date -I) | SipHash fixed-width microbench | $("$CXX" --version | head -1) | $(hostname) | $(uname -m) | $(lscpu | awk -F: '/Model name/{print $2; exit}' | xargs) | $(nproc) cores | $(free -h | awk '/^Mem:/{print $2}') RAM" && \
      "build-bench-$COMPILER/bin/bench_bitcoin" -filter='SipHash.*32b|SipHash.*36b' -min-time=10000; \
    done
    
    2026-07-17 | SipHash fixed-width microbench | g++ (Ubuntu 15.2.0-16ubuntu1) 15.2.0 | ssd-ryzen | x86_64 | AMD Ryzen 7 3700X 8-Core Processor | 16 cores | 62Gi RAM
    
    |               ns/op |                op/s |    err% |          ins/op |          cyc/op |    IPC |         bra/op |   miss% |     total | benchmark
    |--------------------:|--------------------:|--------:|----------------:|----------------:|-------:|---------------:|--------:|----------:|:----------
    |                9.51 |      105,114,321.80 |    0.0% |           85.00 |           41.09 |  2.069 |           0.00 |   99.9% |     10.99 | `SipHash13UJ_32b`
    |               11.10 |       90,098,656.80 |    0.0% |          104.00 |           47.95 |  2.169 |           0.00 |   99.9% |     11.00 | `SipHash13UJ_36b`
    |               20.21 |       49,481,249.12 |    0.0% |          241.00 |           87.18 |  2.764 |           3.00 |    0.0% |     11.00 | `SipHash24_32b`
    |               20.34 |       49,152,337.32 |    0.1% |          248.00 |           87.85 |  2.823 |           3.00 |    0.0% |     10.99 | `SipHash24_36b`
    
    2026-07-17 | SipHash fixed-width microbench | Ubuntu clang version 21.1.8 (6ubuntu1) | ssd-ryzen | x86_64 | AMD Ryzen 7 3700X 8-Core Processor | 16 cores | 62Gi RAM
    
    |               ns/op |                op/s |    err% |          ins/op |          cyc/op |    IPC |         bra/op |   miss% |     total | benchmark
    |--------------------:|--------------------:|--------:|----------------:|----------------:|-------:|---------------:|--------:|----------:|:----------
    |                9.06 |      110,367,013.27 |    0.0% |           84.00 |           38.95 |  2.157 |           0.00 |   99.9% |     10.98 | `SipHash13UJ_32b`
    |               11.09 |       90,207,517.46 |    0.0% |          106.00 |           47.67 |  2.224 |           0.00 |   99.9% |     11.00 | `SipHash13UJ_36b`
    |               21.07 |       47,451,789.51 |    0.1% |          237.00 |           90.11 |  2.630 |           3.00 |    0.0% |     10.99 | `SipHash24_32b`
    |               21.24 |       47,089,262.47 |    0.1% |          246.00 |           91.16 |  2.699 |           3.00 |    0.0% |     10.99 | `SipHash24_36b`
    

    </details>

  40. in src/coins.cpp:387 in 7c64877a36


    andrewtoth commented at 11:32 PM on July 16, 2026:

    Let's also overload SaltedCoinsCacheHasher::operator()(const Txid& id) and use it here.

            std::unordered_set<Txid, SaltedCoinsCacheHasher> earlier_txids;
    

    andrewtoth commented at 11:42 PM on July 16, 2026:

    Or just update SaltedOutpointHasher, SaltedWtxidHasher, and SaltedTxidHasher in util/hasher to all use SipHasher13UJ.


    l0rinc commented at 11:45 PM on July 16, 2026:

    Done, thanks


    l0rinc commented at 11:49 PM on July 16, 2026:

    Or just update SaltedOutpointHasher, SaltedWtxidHasher, and SaltedTxidHasher in util/hasher to all use SipHasher13UJ.

    We have to check each instance, since user-provided unvalidated prevouts can theoretically contain any value and can cause a collision now if the jumbo siphash ends up easily reversible. So for now I decided to only do it for dbcache directly - let me know if you're sure it's safe for the rest as well.


    andrewtoth commented at 10:32 PM on July 18, 2026:

    Right - that would be unsafe in a lot of places where we aren't computing the hash used as the jumbo block ourselves.

    One other place where we would get a benefit is in the CTxMemPool::indexed_transaction_set. The txid and wtxid indexes are both using the computed hashes there.

  41. DrahtBot added the label CI failed on Jul 16, 2026
  42. in src/coins.h:227 in a734681b15
     218 | @@ -219,6 +219,40 @@ struct CCoinsCacheEntry
     219 |      }
     220 |  };
     221 |  
     222 | +class SaltedCoinsCacheHasher
     223 | +{
     224 | +    const SipHasher13UJ m_hasher;
     225 | +
     226 | +public:
     227 | +    explicit SaltedCoinsCacheHasher(bool deterministic = false);
    


    sipa commented at 1:38 PM on July 17, 2026:

    This should be non-explicit. With it, clang fails to compile apparently.


    maflcko commented at 3:24 PM on July 17, 2026:

    I think there are two GCC bugs:

    • GCC only prints a warning (not an error) when the explicit ctor is called implicitly: https://godbolt.org/z/GdcnoWEfa, and apparently those warnings are disabled in stdlib headers?
    • As a result, in GCC's stdlib headers such implicit call to an explicit ctor is not detected by GCC itself, only clang.

    I'd say the fix would be for GCC stdlib to replace _Tp _M_obj; by _Tp _M_obj{};


    l0rinc commented at 7:17 PM on July 17, 2026:

    Blah, I already fell into this once, thanks.

  43. in src/crypto/siphash.h:30 in b352077807
      26 | +    v0 += v3; v3 = std::rotl(v3, 21); v3 ^= v0;
      27 | +    v2 += v1; v1 = std::rotl(v1, 17); v1 ^= v2;
      28 | +    v2 = std::rotl(v2, 32);
      29 | +}
      30 | +
      31 | +ALWAYS_INLINE void Process24(uint64_t& v0, uint64_t& v1, uint64_t& v2, uint64_t& v3, uint64_t data)
    


    sipa commented at 1:45 PM on July 17, 2026:

    Nit: Since the number of finalization rounds doesn't matter here, maybe call it Process2?


    l0rinc commented at 5:52 PM on July 17, 2026:

    Thanks, the helpers are now Compress1/Compress2 and the finalizers Finalize3U/Finalize4, following the later compression-naming suggestion.

  44. in src/crypto/siphash.h:66 in 1fa5d1bd6b
      61 | +    v3 ^= d0; v0 ^= d1; v1 ^= d2; v2 ^= d3;
      62 | +    SipRound(v0, v1, v2, v3);
      63 | +    v0 ^= d0; v1 ^= d1; v2 ^= d2; v3 ^= d3;
      64 | +}
      65 | +
      66 | +ALWAYS_INLINE uint64_t Finalize13UJ(uint64_t v0, uint64_t v1, uint64_t v2, uint64_t v3)
    


    sipa commented at 1:49 PM on July 17, 2026:

    Nit: this could be Finalize3Unpadded then (since it's independent of the per-block round number, it could be reused for a hypothetical SipHash-1-3-U without jumboblocks or so; not that I expect that to happen).

  45. in src/crypto/siphash.h:51 in 1fa5d1bd6b
      47 | @@ -46,6 +48,30 @@ ALWAYS_INLINE uint64_t Finalize24(uint64_t v0, uint64_t v1, uint64_t v2, uint64_
      48 |      return v0 ^ v1 ^ v2 ^ v3;
      49 |  }
      50 |  
      51 | +ALWAYS_INLINE void Process13UJ(uint64_t& v0, uint64_t& v1, uint64_t& v2, uint64_t& v3, uint64_t data)
    


    sipa commented at 1:49 PM on July 17, 2026:

    Nit: this could be Process1 then (and e.g. be reusable by a SipHash-1-3 implementation).

    (also the Jumbo version below)

  46. in src/test/data/siphash.json:2 in 1a51c54977 outdated
       1 | @@ -2,67 +2,67 @@
       2 |    {
    


    sipa commented at 2:27 PM on July 17, 2026:

    I've regenerated the test vectors in the JSON format, and had it also add the 64 official SipHash-2-4 test vectors: https://gist.github.com/sipa/621047e8fa7f9340bc152c72fd966222


    l0rinc commented at 7:16 PM on July 17, 2026:

    Thanks, added your version with minor changes: removed a few exact duplicates and retained a few 32+4-byte cases from the previous set, since otherwise the fixed PresaltedSipHasher(uint256, uint32_t) path would no longer be covered.

    <details><summary>Regenerated vectors must keep exercising every fixed-width path</summary>

    diff --git a/src/test/hash_tests.cpp b/src/test/hash_tests.cpp
    --- a/src/test/hash_tests.cpp	(revision d688876bcee1973b5355eec80396ae780a12df08)
    +++ b/src/test/hash_tests.cpp	(date 1784317394252)
    @@ -172,14 +172,17 @@
     
     BOOST_AUTO_TEST_CASE(siphash_test_vectors)
     {
    +    int fixed24_32b{0}, fixed24_36b{0}, fixed13uj_32b{0}, fixed13uj_40b{0};
         for (UniValue tests{read_json(json_tests::siphash)}; auto& test : tests.getValues()) {
             const uint64_t k0{FromHex64(test["key"][0])}, k1{FromHex64(test["key"][1])};
             auto& input{test["input"]};
             const uint64_t expected24{FromHex64(test["expected"]["siphash24"])};
             BOOST_CHECK_EQUAL(CalculateSipHash24(input, k0, k1), expected24);
             if (input.size() == 1 && HasByteLength(input[0], uint256::size())) {
    +            ++fixed24_32b;
                 BOOST_CHECK_EQUAL(PresaltedSipHasher(k0, k1)(FromHex256(input[0])), expected24);
             } else if (input.size() == 2 && HasByteLength(input[0], uint256::size()) && HasByteLength(input[1], sizeof(uint32_t))) {
    +            ++fixed24_36b;
                 const auto extra{ParseHex(input[1].get_str())};
                 BOOST_CHECK_EQUAL(PresaltedSipHasher(k0, k1)(FromHex256(input[0]), ReadLE32(extra.data())), expected24);
             }
    @@ -189,13 +192,19 @@
                 BOOST_CHECK_EQUAL(CalculateSipHash13UJ(input, k0, k1, /*normal_as_jumbo=*/true), expected13uj);
                 const SipHasher13UJ fixed_hasher{k0, k1};
                 if (input.size() == 1 && HasByteLength(input[0], uint256::size())) {
    +                ++fixed13uj_32b;
                     BOOST_CHECK_EQUAL(fixed_hasher.Hash(FromHex256(input[0])), expected13uj);
                 } else if (input.size() == 2 && HasByteLength(input[0], uint256::size()) && HasByteLength(input[1], sizeof(uint64_t))) {
    +                ++fixed13uj_40b;
                     const auto extra{ParseHex(input[1].get_str())};
                     BOOST_CHECK_EQUAL(fixed_hasher.Hash(FromHex256(input[0]), ReadLE64(extra.data())), expected13uj);
                 }
             }
         }
    +    BOOST_CHECK(fixed24_32b > 0);
    +    BOOST_CHECK(fixed24_36b > 0);
    +    BOOST_CHECK(fixed13uj_32b > 0);
    +    BOOST_CHECK(fixed13uj_40b > 0);
     }
     
     BOOST_AUTO_TEST_SUITE_END()
    

    fails for the posted json with:

    test/hash_tests.cpp:206: error: in "hash_tests/siphash_test_vectors": check fixed24_36b > 0 has failed

    </details>

  47. sipa commented at 2:29 PM on July 17, 2026: member

    Preliminary code review ACK

  48. in src/crypto/siphash.cpp:96 in a734681b15
     169 | +    uint64_t v0{m_state.v[0]}, v1{m_state.v[1]}, v2{m_state.v[2]}, v3{m_state.v[3]};
     170 | +    Process24(v0, v1, v2, v3, val.GetUint64(0));
     171 | +    Process24(v0, v1, v2, v3, val.GetUint64(1));
     172 | +    Process24(v0, v1, v2, v3, val.GetUint64(2));
     173 | +    Process24(v0, v1, v2, v3, val.GetUint64(3));
     174 | +    return Finalize24(v0, v1, v2, v3, (uint64_t{36} << 56) | extra);
    


    sipa commented at 3:18 PM on July 17, 2026:

    Ultranit. If the padding were moved to a Process24 (or Process2) call, this could read:

        uint64_t v0{m_state.v[0]}, v1{m_state.v[1]}, v2{m_state.v[2]}, v3{m_state.v[3]};
        Process2(v0, v1, v2, v3, val.GetUint64(0));
        Process2(v0, v1, v2, v3, val.GetUint64(1));
        Process2(v0, v1, v2, v3, val.GetUint64(2));
        Process2(v0, v1, v2, v3, val.GetUint64(3));
        Process2(v0, v1, v2, v3, (uint64_t{36} << 56) | extra);
        return Finalize4(v0, v1, v2, v3);
    

    Also, The SipHash paper calls this "compressions", so maybe we can use the name Compress instead of Process?


    l0rinc commented at 7:11 PM on July 17, 2026:

    Good idea, that makes the variants more symmetric, thanks. Adjusted the wording in the commit messages as well to state compression.

  49. maflcko removed the label CI failed on Jul 17, 2026
  50. DrahtBot added the label CI failed on Jul 17, 2026
  51. DrahtBot commented at 3:28 PM on July 17, 2026: contributor

    <!--85328a0da195eb286784d51f73fa0af9-->

    🚧 At least one of the CI tasks failed. <sub>Task fuzzer,address,undefined,integer: https://github.com/bitcoin/bitcoin/actions/runs/29542891905/job/87923199917</sub> <sub>LLM reason (✨ experimental): CI failed due to a C++ build error: coins.cpp fails to compile because SaltedCoinsCacheHasher’s explicit constructor/type doesn’t satisfy std::unordered_set hashing requirements (triggered under -Werror).</sub>

    <details><summary>Hints</summary>

    Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

    • Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

    • A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.

    • An intermittent issue.

    Leave a comment here, if you need help tracking down a confusing failure.

    </details>

  52. l0rinc force-pushed on Jul 17, 2026
  53. l0rinc commented at 8:17 PM on July 17, 2026: contributor

    Thanks for the quick review - rebased and applied the suggestions. The helpers are now Compress1 (overloaded for normal and jumbo blocks) and Compress2, and standard padding is compressed before Finalize4. Finalize3U retains U because it applies the unpadded domain separator. No finalizer needs J because finalization is independent of the preceding block types.

    I incorporated your regenerated vectors, including the 64 official SipHash-2-4 vectors for inputs from 0 to 63 bytes - but removed a few exact duplicate key/input rows and retained the eleven 32+4-byte cases, so PresaltedSipHasher(uint256, uint32_t) remains covered.

    I also fixed the SaltedCoinsCacheHasher constructor, removed SaltedOutpointHasher's now-unused deterministic mode and updated related code comments.

  54. DrahtBot removed the label CI failed on Jul 17, 2026
  55. sipa commented at 12:23 AM on July 18, 2026: member

    One idea I had that I think makes things even cleaner, is to move all state-modification logic into SipHashState functions, and then implement the exposed classes in terms of that: https://github.com/sipa/bitcoin/commit/wip_siphashstate_all. It doesn't seem to affect benchmarks for me.

    Entirely optional, feel free to cherrypick, integrate, or ignore completely.

  56. test: add shared SipHash vectors
    Lock SipHash-2-4 behavior into shared vectors before refactoring its round and finalization code.
    Store inputs as ordered hex byte blocks so `CSipHasher` and the independent Python implementation hash the same byte sequence, with applicable `PresaltedSipHasher` overloads checked against the same vectors.
    Add the 64 official SipHash-2-4 vectors alongside block-partition and empty-block cases for the generic path.
    Move randomized generic/fixed comparisons to the integer fuzzer.
    
    SipHash-1-3-UJ coverage can add expected outputs for compatible 8- and 32-byte block sequences.
    The Python test reads a build-tree copy so functional-test staging behaves consistently when files are symlinked or copied.
    
    Co-authored-by: Pieter Wuille <pieter@wuille.net>
    af50ba8500
  57. l0rinc force-pushed on Jul 18, 2026
  58. l0rinc commented at 4:37 AM on July 18, 2026: contributor

    Thanks for all the review, rebased and integrated @sipa's builder suggestion across the existing crypto commits: round, compression, and finalization now live in the class (the new uint64_t{32} << 56 spelling directly says 32-byte input and matches the 36-byte path). Changed SipRound() to private and kept a local state copy in the byte writer because mutating the member state directly was measurably slower. Also tightened the related state and hasher documentation. Remeasured performance and asm - same as before.

  59. DrahtBot added the label CI failed on Jul 18, 2026
  60. DrahtBot commented at 5:29 AM on July 18, 2026: contributor

    <!--85328a0da195eb286784d51f73fa0af9-->

    🚧 At least one of the CI tasks failed. <sub>Task tidy: https://github.com/bitcoin/bitcoin/actions/runs/29630591521/job/88043238101</sub> <sub>LLM reason (✨ experimental): CI failed because the dependency check detected unexpected linker/object symbol dependencies (siphash.cpp.o → check.cpp.o via assertion_fail(std::source_location, ...)), causing 03_test_script.sh to exit with status 1.</sub>

    <details><summary>Hints</summary>

    Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

    • Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

    • A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.

    • An intermittent issue.

    Leave a comment here, if you need help tracking down a confusing failure.

    </details>

  61. refactor: simplify adding SipHash-1-3-UJ
    Move the SipHash round, compression, and finalization operations shared by `CSipHasher` and `PresaltedSipHasher` into inline `SipHashState` methods.
    This centralizes state mutation, preserves the existing byte-path code generation, and keeps the security-sensitive follow-up focused on its changed block compression, round counts, and finalizer.
    
    Co-authored-by: Pieter Wuille <pieter@wuille.net>
    25bfca06d6
  62. crypto: add generic SipHash-1-3-UJ
    Add `SipHasher13UJ`, a block-oriented implementation accepting arbitrarily mixed 64-bit normal blocks and 256-bit jumbo blocks.
    For hash-table use, cryptographic hash outputs must make up all but a small bounded number of retained jumbo blocks.
    
    The generic interface serves as the executable specification for the fixed-width overloads added next.
    
    Co-authored-by: Lőrinc <pap.lorinc@gmail.com>
    Co-authored-by: Jean-Philippe Aumasson <jeanphilippe.aumasson@gmail.com>
    c2d7931b5c
  63. crypto: add fixed-width SipHash-1-3-UJ
    Add const `SipHasher13UJ::Hash` overloads for one `uint256` jumbo block, optionally followed by one 64-bit normal block.
    Reuse the generic hasher's initialized state and define these short paths in the header because the new hasher benefits from inlining.
    
    Co-authored-by: Lőrinc <pap.lorinc@gmail.com>
    a0ccd4ad17
  64. test: add SipHash-1-3-UJ coverage
    Add SipHash-1-3-UJ outputs to the shared vectors for sequences of 8- and 32-byte blocks.
    Check generic writes and applicable fixed-width `Hash` overloads against those outputs, and fuzz their equivalence including mixed normal/jumbo encodings.
    
    The outputs were generated by an independent implementation that Claude Opus 4.8 produced using only the `SipHasher13UJ` class comment as its prompt.
    
    Co-authored-by: Lőrinc <pap.lorinc@gmail.com>
    3aea85411f
  65. bench: add fixed-width SipHash benchmarks
    Benchmark 32-byte hashes and inputs consisting of a 32-byte hash plus a 32-bit index with SipHash-2-4 and SipHash-1-3-UJ.
    The UJ benchmark zero-extends the index into one 64-bit normal block.
    Keep all four measurements together after shared correctness coverage and before changing `CCoinsMap`'s hasher.
    
    Co-authored-by: Pieter Wuille <pieter@wuille.net>
    7ff55cc650
  66. coins: use SipHash-1-3-UJ for `CCoinsMap`
    Use the fixed-width `SipHasher13UJ::Hash` path for `CCoinsMap`, while keeping other `SaltedOutpointHasher` users on SipHash-2-4.
    The salted outputs are process-local and must not be persisted, serialized, or compared across processes.
    
    Retained cache entries identify real transaction outputs and therefore contain computed txids.
    Missing-input validation may probe arbitrary claimed prevouts, but `FetchCoin()` erases each temporary entry immediately when the backend lookup fails, so non-hash keys cannot accumulate.
    The assumeutxo loader assumes snapshot txids are valid while loading and verifies the complete snapshot content hash before activation.
    
    Co-authored-by: Pieter Wuille <pieter@wuille.net>
    2beab94896
  67. coins: reuse cache hasher for txid set
    Use `SaltedCoinsCacheHasher` for the temporary set of earlier txids in `CoinsViewOverlay`, and in existing overlay tests to exercise the new `Txid` overload.
    Every entry is a computed transaction hash, and the set is limited to a few thousand elements per block, satisfying the SipHash-1-3-UJ jumbo-input requirements.
    
    Co-authored-by: Andrew Toth <andrewstoth@gmail.com>
    3bfdcbd7ee
  68. l0rinc force-pushed on Jul 18, 2026
  69. DrahtBot removed the label CI failed on Jul 18, 2026
  70. sipa commented at 1:52 PM on July 18, 2026: member

    ACK 3bfdcbd7ee494c613deac0840ca6997fcc078c08 (to the extent the code/ideas aren't my own)

  71. DrahtBot requested review from andrewtoth on Jul 18, 2026
  72. in src/crypto/siphash.h:170 in 3bfdcbd7ee
     165 | +    /** Construct a SipHash-1-3-UJ calculator initialized with 128-bit key (k0, k1). */
     166 | +    SipHasher13UJ(uint64_t k0, uint64_t k1) noexcept : m_state{k0, k1} {}
     167 | +    /** Hash a normal 64-bit value. */
     168 | +    SipHasher13UJ& Write(uint64_t data) noexcept;
     169 | +    /** Hash a 256-bit value as a jumbo block. For hash-table use, non-hash inputs must remain few and bounded. */
     170 | +    SipHasher13UJ& WriteJumbo(const uint256& hash) noexcept;
    


    andrewtoth commented at 11:39 PM on July 18, 2026:

    nit: All Compress* and Write* methods that return a reference should be annotated with LIFETIMEBOUND.

        SipHasher13UJ& WriteJumbo(const uint256& hash) noexcept LIFETIMEBOUND;
    
  73. in src/test/hash_tests.cpp:195 in 3bfdcbd7ee
     207 | +            BOOST_CHECK_EQUAL(CalculateSipHash13UJ(input, k0, k1, /*normal_as_jumbo=*/false), expected13uj);
     208 | +            BOOST_CHECK_EQUAL(CalculateSipHash13UJ(input, k0, k1, /*normal_as_jumbo=*/true), expected13uj);
     209 | +            const SipHasher13UJ fixed_hasher{k0, k1};
     210 | +            if (hash_only) {
     211 | +                BOOST_CHECK_EQUAL(fixed_hasher.Hash(FromHex256(input[0])), expected13uj);
     212 | +            } else if (hash_extra && HasByteLength(input[1], sizeof(uint64_t))) {
    


    andrewtoth commented at 11:43 PM on July 18, 2026:

    nit: if any of the four if branches in this test are never hit, the test will still silently succeed. We could define 4 bools initialized to false, and then set them to true inside the branches, then check that all 4 are true at the end of the test.


    l0rinc commented at 2:40 AM on July 19, 2026:

    Yes, that's what I did in #35215 (review), but didn't want to commit it since code coverage should already reveal that.

  74. in src/coins.h:236 in 3bfdcbd7ee
     231 | + * complete snapshot's content hash before activation.
     232 | + *
     233 | + * Hash values are process-local and must not be persisted, serialized, or compared across
     234 | + * processes.
     235 | + *
     236 | + * Having the hash noexcept lets libstdc++ recalculate it during rehash instead of storing it in
    


    andrewtoth commented at 11:44 PM on July 18, 2026:

    Removing the noexcept was explored in #35195. Would this be a good opportunity to remove it here?


    sipa commented at 12:13 AM on July 19, 2026:

    I think we should investigate again after this PR. With faster hashing, it's possible the optimum shifted again to the non-caching version?


    l0rinc commented at 2:39 AM on July 19, 2026:

    Yeah, that's why I opened #35195 to investigate, but want to leave it unchanged here. I have investigated caching it ourselves as a custom CoinsCacheHasher (instead of libstdc++), but it seems dangerous when used across maps.

  75. andrewtoth approved
  76. andrewtoth commented at 11:44 PM on July 18, 2026: contributor

    ACK 3bfdcbd7ee494c613deac0840ca6997fcc078c08

    Comments are non-blocking.

  77. in src/test/hash_tests.cpp:29 in af50ba8500
      26 | +    CSipHasher hasher{k0, k1};
      27 | +    for (auto& block : input.getValues()) {
      28 | +        hasher.Write(ParseHex(block.get_str()));
      29 | +    }
      30 | +    const uint64_t result{hasher.Finalize()};
      31 | +    BOOST_CHECK_EQUAL(hasher.Finalize(), result);
    


    optout21 commented at 12:03 PM on July 20, 2026:

    af50ba8 test: add shared SipHash vectors:

    This line looks unnecessary, what's its purpose? To test the idempotency of Finalize()? Maybe the intent could be clarified.


    l0rinc commented at 5:01 PM on July 20, 2026:

    Yes, it's idempotency, it's testing the claim:

    /** Compute the 64-bit SipHash-1-3-UJ of the data written so far. The object remains untouched. */

  78. in src/crypto/siphash.cpp:59 in 25bfca06d6
     100 | +    return m_state.Copy()
     101 | +                  .Compress2(val.GetUint64(0))
     102 | +                  .Compress2(val.GetUint64(1))
     103 | +                  .Compress2(val.GetUint64(2))
     104 | +                  .Compress2(val.GetUint64(3))
     105 | +                  .Compress2(uint64_t{32} << 56)
    


    optout21 commented at 12:31 PM on July 20, 2026:

    25bfca0 refactor: simplify adding SipHash-1-3-UJ:

    Here uint64_t{4}) << 59 has been changed to uint64_t{32} << 56. The compiler (presumably) will change both the 2^61, but what's the rationale for this change or this representation? Is using 56 more optimal as its a multiple of 8? If this is a concern, why not use an expanded constant 0x2000000000000000?


    sipa commented at 12:59 PM on July 20, 2026:

    The reason is just to match the SipHash specification, which says that the padding value consists of the last 0-7 bytes of input data (0 bytes here), and the last (7th) byte set to (length mod 256). Length is 32 here.


    optout21 commented at 1:07 PM on July 20, 2026:

    Thanks for the clarification! Later I also found the uint64_t{36} << 56 constant for the 36-byte case, so these two are also in sync.


    l0rinc commented at 5:00 PM on July 20, 2026:

    Yes, this is what I meant to explain in:

    the new uint64_t{32} << 56 spelling directly says 32-byte input and matches the 36-byte path

  79. optout21 commented at 12:56 PM on July 20, 2026: contributor

    ACK 3bfdcbd7ee494c613deac0840ca6997fcc078c08

    Reviewed the changes, LGTM, left some minor comments. I've performed a few preliminary reviews earlier, and followed the evolution of the PR. Verified unit tests (C++ and Python) locally, verified micro-benchmark results.

  80. achow101 commented at 10:28 PM on July 21, 2026: member

    light ACK 3bfdcbd7ee494c613deac0840ca6997fcc078c08

  81. achow101 merged this on Jul 21, 2026
  82. achow101 closed this on Jul 21, 2026

  83. l0rinc deleted the branch on Jul 21, 2026
  84. l0rinc commented at 10:47 PM on July 21, 2026: contributor

    Thanks for the reviews, this is another 5% validation speedup in the upcoming v32 <3


github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-07-27 19:51 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me