Skip to content

chore: bloom filter opt #1811

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sergeytimoshin merged 1 commit into from
Jun 15, 2025
Merged

chore: bloom filter opt #1811

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 11 additions & 7 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion program-libs/bloom-filter/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ pinocchio = ["dep:pinocchio"]

[dependencies]
bitvec = "1.0.1"
fastmurmur3 = "0.2.0"
solana-nostd-keccak = "0.1.3"
num-bigint = { workspace = true }
solana-program-error = { workspace = true, optional = true }
pinocchio = { workspace = true, optional = true }
Expand Down
23 changes: 14 additions & 9 deletions program-libs/bloom-filter/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,15 +68,20 @@ impl<'a> BloomFilter<'a> {
})
}

pub fn probe_index_fast_murmur(value_bytes: &[u8], iteration: usize, capacity: &u64) -> usize {
let iter_bytes = iteration.to_le_bytes();
let base_hash = fastmurmur3::hash(value_bytes);
let mut combined_bytes = [0u8; 24];
combined_bytes[..16].copy_from_slice(&base_hash.to_le_bytes());
combined_bytes[16..].copy_from_slice(&iter_bytes);
pub fn probe_index_keccak(value_bytes: &[u8; 32], iteration: usize, capacity: &u64) -> usize {
let iter_bytes: [u8; 8] = iteration.to_le_bytes();
let mut combined_bytes = [0u8; 40];
combined_bytes[..32].copy_from_slice(value_bytes);
combined_bytes[32..].copy_from_slice(&iter_bytes);

let combined_hash = fastmurmur3::hash(&combined_bytes);
(combined_hash % (*capacity as u128)) as usize
let hash = solana_nostd_keccak::hash(&combined_bytes);

let mut index = 0u64;
for chunk in hash.chunks(8) {
let value = u64::from_le_bytes(chunk.try_into().unwrap());
index = value.wrapping_add(index) % *capacity;
}
index as usize
Comment on lines +79 to +84
Copy link
Contributor

@coderabbitai coderabbitai bot Jun 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Potential usize overflow & extra modulo work

index is accumulated as u64 but returned as usize. On a 32-bit build capacity may legitimately exceed usize::MAX, making the cast truncating and producing out-of-range bit indices (bits[index] will panic). Either:

  1. Store capacity as usize everywhere, or
  2. Keep the function u64 throughout and check/guard before indexing.

Also, taking mod for every 8-byte chunk is unnecessary; taking a single u64 from the hash and modding once is sufficient and faster.

🤖 Prompt for AI Agents 
In program-libs/bloom-filter/src/lib.rs around lines 79 to 84, the code
accumulates an index as u64 and then casts it to usize, which can cause overflow
on 32-bit systems if capacity exceeds usize::MAX, leading to panics when
indexing. To fix this, either change capacity to usize throughout the code to
ensure safe indexing or keep the index as u64 and add checks before indexing to
prevent out-of-range access. Additionally, optimize by removing the modulo
operation inside the loop and instead take a single u64 from the hash and apply
modulo once after accumulation to improve performance.

}

pub fn insert(&mut self, value: &[u8; 32]) -> Result<(), BloomFilterError> {
Expand All @@ -98,7 +103,7 @@ impl<'a> BloomFilter<'a> {

let bits = BitSlice::<u8, Msb0>::from_slice_mut(self.store);
for i in 0..self.num_iters {
let probe_index = Self::probe_index_fast_murmur(value, i, &(self.capacity));
let probe_index = Self::probe_index_keccak(value, i, &(self.capacity));
if bits[probe_index] {
continue;
} else if insert {
Expand Down
1 change: 1 addition & 0 deletions program-libs/hasher/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ solana-program-error = { workspace = true, optional = true }
solana-pubkey = { workspace = true, optional = true }
pinocchio = { workspace = true, optional = true }
borsh = { workspace = true }
solana-nostd-keccak = "0.1.3"

[target.'cfg(not(target_os = "solana"))'.dependencies]
ark-bn254 = { workspace = true }
Expand Down
26 changes: 1 addition & 25 deletions program-libs/hasher/src/keccak.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,31 +14,7 @@ impl Hasher for Keccak {
}

fn hashv(vals: &[&[u8]]) -> Result<Hash, HasherError> {
#[cfg(not(target_os = "solana"))]
{
use sha3::{Digest, Keccak256};

let mut hasher = Keccak256::default();
for val in vals {
hasher.update(val);
}
Ok(hasher.finalize().into())
}
// Call via a system call to perform the calculation
#[cfg(target_os = "solana")]
{
use crate::HASH_BYTES;

let mut hash_result = [0; HASH_BYTES];
unsafe {
crate::syscalls::sol_keccak256(
vals as *const _ as *const u8,
vals.len() as u64,
&mut hash_result as *mut _ as *mut u8,
);
}
Ok(hash_result)
}
Ok(solana_nostd_keccak::hashv(vals))
}

fn zero_bytes() -> ZeroBytes {
Expand Down
Loading