Libfc: add keccak-256 cross-impl smoke tests + bench#357
Open
heifner wants to merge 2 commits into
Open
Conversation
Two Keccak-256 implementations sit on the consensus path: fc::sha3 (hand-rolled) is reached by the WASM sha3() host intrinsic, and ethash::keccak256 (via fc::crypto::keccak256) is reached by EM signature recovery in em::public_key::recover. Both must produce bit-identical output for any node validating EM-signed transactions alongside contracts that hash via sysio::keccak() to remain consistent. Add to libraries/libfc/test/crypto/test_hash_functions.cpp: - keccak256_class_golden: pins fc::crypto::keccak256 to standard vectors so a wiring regression in the ethash wrapper surfaces independently of the library. - ethash_fc_sha3_cross_impl_agreement: runs both impls over the empty input, short strings, the ERC-20 Transfer event topic, the Keccak-256 sponge rate boundary (135/136/137 bytes), a multi-block input, and a 100 KiB deterministic buffer; asserts byte equality. - Add the Transfer event topic to the existing keccak256 fc::sha3 vector list so the same constant is anchored against both impls. Add libraries/libfc/benchmark/keccak_bench.cpp (EXCLUDE_FROM_ALL): measures ethash::keccak256 and fc::sha3 (Keccak and NIST padding, one-shot and chunked-at-10-KiB) at 32 B / 4 KiB / 1 MiB / 33 MiB (the default max wasm linear memory, 528 * 64 KiB pages). The chunked variant mirrors the WASM sha3() host intrinsic's hashing_checktime_block_size loop, so the overhead of the inter-block checktime is directly visible.
Apply pre-PR review feedback. No behavioral change to the asserted invariants. test_hash_functions.cpp: - ethash_fc_sha3_cross_impl_agreement now uses the fc::sha3::hash(string, bool) overload, eliminating the implicit size_t -> uint32_t narrowing on msg.size() in the test body. - Rename the per-case typedef from test_kv to test_keccak256_class to match the existing convention (test_sha3, test_keccak256). - Add a doc comment to the agreement case noting it is one leg of a three-way pin (the two golden-vector cases anchor each impl against published constants; this case asserts they agree across additional inputs). The three are complementary, not redundant. keccak_bench.cpp: - Add comment explaining why two bytes of the digest is a sufficient optimizer barrier (Keccak's output depends on every input byte). - Drop redundant static_cast<uint8_t> in the sink. - Comment make_buffer's seed reuse so a future reader does not assume cross-size independence. - Assert that runs is odd in time_it so times[runs / 2] returns the exact median rather than the upper of the two middle values. - Format the size column with fmt_size (e.g. "33 MiB" instead of "34603008") for readable output. - Spell out the chunking comment's reference as sysio::chain::config::hashing_checktime_block_size so a future config change can grep here. - Run the cross-impl smoke per scenario rather than only at the largest size, and return non-zero on the first mismatch.
huangminghuang
approved these changes
May 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Wire has two Keccak-256 implementations sitting on the consensus path:
fc::sha3(hand-rolled) is reached by the WASMsha3()host intrinsic; every contract call tosysio::keccak()runs through it.ethash::keccak256(via thefc::crypto::keccak256wrapper) is reached by EM signature recovery inem::public_key::recover; every EM-signed transaction's verification runs through it.Both must produce bit-identical output or chains carrying EM-signed traffic alongside contracts that hash via
sysio::keccak()will desynchronize. Until now this invariant was implicit -- thesysio.authexcontract tests happen to exercise both paths against the same digest, so a divergence would have failed the link verification, but nothing pinned the invariant explicitly.This PR locks the invariant in a libfc-level test that runs on every CI build, and adds a microbenchmark so future performance work has a baseline.
What's added
Tests (
libraries/libfc/test/crypto/test_hash_functions.cpp):keccak256_class_golden-- pinsfc::crypto::keccak256to standard published vectors (empty input,"abc", the ERC-20Transfer(address,address,uint256)event topic). Catches wiring bugs in the ethash wrapper independent of ethash itself.ethash_fc_sha3_cross_impl_agreement-- runs both impls over the empty input, short strings, the ERC-20 Transfer topic, the Keccak-256 sponge rate boundary (135 / 136 / 137 bytes), a multi-block input, and a 100 KiB deterministic buffer; asserts byte equality.keccak256case so the same recognizable constant is anchored againstfc::sha3too.The three cases form a three-way pin: each impl is independently anchored against published vectors, and they are cross-checked against each other across additional inputs.
Benchmark (
libraries/libfc/benchmark/keccak_bench.cpp,EXCLUDE_FROM_ALL):ethash::keccak256andfc::sha3(Keccak and NIST padding, one-shot and chunked-at-10-KiB) at 32 B / 4 KiB / 1 MiB / 33 MiB (the default max wasm linear memory, 528 x 64 KiB pages).sha3()host intrinsic'shashing_checktime_block_sizeloop, so the overhead of the inter-block checktime is directly visible.Build and run:
Why now
Pre-launch. The cleanest moment to lock cross-impl invariants is before any contract has hashed anything on a live chain. A future refactor of either implementation (vcpkg bump of ethash, perf work on
fc::sha3, eventual consolidation onto a single library) now has a CI guardrail.