feat: collision-resistant memory-location hashing via seed + u128 key#515
Draft
datnguyencse wants to merge 1 commit into
Draft
feat: collision-resistant memory-location hashing via seed + u128 key#515datnguyencse wants to merge 1 commit into
datnguyencse wants to merge 1 commit into
Conversation
Contributor
Author
|
benchmark |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a per-execution random seed to mitigate hash collision attacks on memory locations. The MemoryLocationHash type is expanded from 64-bit (u64) to 128-bit (u128) to practically eliminate collisions, and the deterministic hashing function now performs two FxHash passes mixed with the random seed. The seed is propagated through MvMemory, PevmChain, and the VM execution. Feedback on these changes suggests a performance optimization in crates/pevm/src/chain/rise.rs to precompute the fee recipient hashes outside of the transaction loop instead of recalculating them in each iteration.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Comment on lines
+113
to
134
| let recipients = [ | ||
| block_env.beneficiary, | ||
| BASE_FEE_RECIPIENT, | ||
| L1_FEE_RECIPIENT, | ||
| OPERATOR_FEE_RECIPIENT, | ||
| ]; | ||
|
|
||
| // TODO: Estimate more locations based on sender, to, etc. | ||
| // TODO: Benchmark to check whether adding these estimated | ||
| // locations helps or harms the performance. | ||
| let mut estimated_locations = HashMap::with_hasher(BuildIdentityHasher::default()); | ||
|
|
||
| for (index, tx) in txs.iter().enumerate() { | ||
| // Deposit transactions pay no fees so they write nothing to these fee recipients. | ||
| if !tx.is_deposit() { | ||
| estimated_locations | ||
| .entry(beneficiary_location_hash) | ||
| .or_insert_with(|| Vec::with_capacity(txs.len())) | ||
| .push(index); | ||
| estimated_locations | ||
| .entry(*BASE_FEE_RECIPIENT_LOCATION_HASH) | ||
| .or_insert_with(|| Vec::with_capacity(txs.len())) | ||
| .push(index); | ||
| estimated_locations | ||
| .entry(*L1_FEE_RECIPIENT_LOCATION_HASH) | ||
| .or_insert_with(|| Vec::with_capacity(txs.len())) | ||
| .push(index); | ||
| estimated_locations | ||
| .entry(*OPERATOR_FEE_RECIPIENT_LOCATION_HASH) | ||
| .or_insert_with(|| Vec::with_capacity(txs.len())) | ||
| .push(index); | ||
| for address in recipients { | ||
| estimated_locations | ||
| .entry(basic_location_hash(address, seed)) | ||
| .or_insert_with(|| Vec::with_capacity(txs.len())) | ||
| .push(index); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The hashes of the fee recipients (recipients) are constant for the entire block and only depend on the static addresses and the block-level seed. Recalculating them inside the transaction loop results in redundant hashing operations (2 FxHash passes per recipient per transaction). Precomputing these hashes once before the loop improves performance, especially for blocks with a large number of transactions.
let recipients = [
block_env.beneficiary,
BASE_FEE_RECIPIENT,
L1_FEE_RECIPIENT,
OPERATOR_FEE_RECIPIENT,
];
let recipient_hashes = [
basic_location_hash(block_env.beneficiary, seed),
basic_location_hash(BASE_FEE_RECIPIENT, seed),
basic_location_hash(L1_FEE_RECIPIENT, seed),
basic_location_hash(OPERATOR_FEE_RECIPIENT, seed),
];
// TODO: Estimate more locations based on sender, to, etc.
// TODO: Benchmark to check whether adding these estimated
// locations helps or harms the performance.
let mut estimated_locations = HashMap::with_hasher(BuildIdentityHasher::default());
for (index, tx) in txs.iter().enumerate() {
// Deposit transactions pay no fees so they write nothing to these fee recipients.
if !tx.is_deposit() {
for hash in recipient_hashes {
estimated_locations
.entry(hash)
.or_insert_with(|| Vec::with_capacity(txs.len()))
.push(index);
}
}
}
The u64 MemoryLocationHash doubles as both the MvMemory map key and the location's identity. Two distinct locations that collide in the u64 share an entry, so a crafted (FxHash is invertible) or random (birthday ~2^32) collision can make a read return another location's value and validate cleanly — wrong state. Fixed with two independent levers: - Widen MemoryLocationHash to u128 (built from two decorrelated FxHash passes). The map's own 128-bit key comparison becomes a reliable identity check, so random collisions drop to the ~2^-64 birthday bound (effectively never). The IdentityHasher projects the key to its low 64 bits for bucketing, so sharding cost is unchanged. - Mix a per-execution random seed into the hash. Unknown to submitters until the block runs, so collisions cannot be aimed at this block. The seed never escapes into results (state is keyed by real addresses), so output is identical for any seed — verified by the full test suite. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
datnguyencse
force-pushed
the
feat/seeded-u128-location-hash
branch
from
82c26b2 to
5e63d30
Compare
|
✅ Gigagas benchmark for commit 5e63d30 Detail |
|
✅ Mainnet benchmark for commit 5e63d30 This pr: Detail |
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
1 participant
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.
Problem
MemoryLocationHash = u64is used as theMvMemorymap key itself, so it doubles as both the bucket key and the location's identity. Two distinct locations that collide in theu64share one entry:Fix — two independent levers
MemoryLocationHashtou128(built from two decorrelated FxHash passes,seedand!seed, forming the low/high 64 bits). The map's own 128-bit key comparison becomes a reliable identity check, so random collisions drop to the ~2⁻⁶⁴ birthday bound (effectively never).IdentityHasherprojects the key to its low 64 bits for bucketing, so DashMap sharding cost is unchanged.RandomState(no new dependency), stored onMvMemory.The seed never escapes into results (state is keyed by real
Addresses), so output is identical for any seed.Cost
Hashing does two FxHash passes per location instead of one (~2× a cheap op, negligible vs EVM execution); keys widen
u64→u128(+8 bytes per live key); bucketing/sharding unchanged. OneRandomState::new()per block.Verification
cargo clippy --workspace --all-targets— cleancargo test --workspace --release -- --test-threads=1— all green, including the fullethereum/testsstate-test suite, RISE/mainnet/uniswap blocks (these compare pevm output to expected/sequential, proving the change is output-preserving)Benchmarks (
gigagas,mainnet) pending — opening as draft for perf review. An alternative implementation (seed + per-entry tag, keeping theu64key) exists for comparison.🤖 Generated with Claude Code