Skip to content

refactor(crypto): move inline merkle-backend tests to tests/ #615

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

Open
diegokingston wants to merge 1 commit into
base: main
Choose a base branch
from
Open

refactor(crypto): move inline merkle-backend tests to tests/ #615

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
135 changes: 0 additions & 135 deletions crypto/crypto/src/merkle_tree/backends/field_element.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,138 +70,3 @@ where
P::hash(left, right)
}
}

#[cfg(test)]
mod tests {
use alloc::vec::Vec;
use math::field::{element::FieldElement, goldilocks::GoldilocksField};
use sha3::{Keccak256, Keccak512, Sha3_256, Sha3_512};

use crate::merkle_tree::{backends::field_element::FieldElementBackend, merkle::MerkleTree};

type F = GoldilocksField;
type FE = FieldElement<F>;

#[test]
fn hash_data_field_element_backend_works_with_keccak_256() {
let values: Vec<FE> = (1..6).map(FE::from).collect();
let merkle_tree =
MerkleTree::<FieldElementBackend<F, Keccak256, 32>>::build(&values).unwrap();
let proof = merkle_tree.get_proof_by_pos(0).unwrap();
assert!(proof.verify::<FieldElementBackend<F, Keccak256, 32>>(
&merkle_tree.root,
0,
&values[0]
));
}

#[test]
fn hash_data_field_element_backend_works_with_sha3_256() {
let values: Vec<FE> = (1..6).map(FE::from).collect();
let merkle_tree =
MerkleTree::<FieldElementBackend<F, Sha3_256, 32>>::build(&values).unwrap();
let proof = merkle_tree.get_proof_by_pos(0).unwrap();
assert!(proof.verify::<FieldElementBackend<F, Sha3_256, 32>>(
&merkle_tree.root,
0,
&values[0]
));
}

#[test]
fn hash_data_field_element_backend_works_with_keccak_512() {
let values: Vec<FE> = (1..6).map(FE::from).collect();
let merkle_tree =
MerkleTree::<FieldElementBackend<F, Keccak512, 64>>::build(&values).unwrap();
let proof = merkle_tree.get_proof_by_pos(0).unwrap();
assert!(proof.verify::<FieldElementBackend<F, Keccak512, 64>>(
&merkle_tree.root,
0,
&values[0]
));
}

#[test]
fn hash_data_field_element_backend_works_with_sha3_512() {
let values: Vec<FE> = (1..6).map(FE::from).collect();
let merkle_tree =
MerkleTree::<FieldElementBackend<F, Sha3_512, 64>>::build(&values).unwrap();
let proof = merkle_tree.get_proof_by_pos(0).unwrap();
assert!(proof.verify::<FieldElementBackend<F, Sha3_512, 64>>(
&merkle_tree.root,
0,
&values[0]
));
}

/// Tests batch proof with a real cryptographic hash (Keccak256).
/// This verifies that proof ordering works correctly with non-commutative hashes.
#[test]
fn batch_proof_with_keccak256_verifies_sparse_leaves() {
let values: Vec<FE> = (1..=16).map(FE::from).collect();
let merkle_tree =
MerkleTree::<FieldElementBackend<F, Keccak256, 32>>::build(&values).unwrap();

// Test with sparse leaves across different subtrees
let pos_list: Vec<usize> = vec![1, 8, 9, 15];
let batch_proof = merkle_tree.get_batch_proof(&pos_list).unwrap();
let leaves_to_verify: Vec<FE> = pos_list.iter().map(|&i| values[i]).collect();

assert!(
batch_proof.verify::<FieldElementBackend<F, Keccak256, 32>>(
&merkle_tree.root,
&pos_list,
&leaves_to_verify,
16
),
"Batch proof verification failed with Keccak256"
);
}

/// Tests batch proof with adjacent leaves using real hash.
#[test]
fn batch_proof_with_keccak256_verifies_adjacent_leaves() {
let values: Vec<FE> = (1..=8).map(FE::from).collect();
let merkle_tree =
MerkleTree::<FieldElementBackend<F, Keccak256, 32>>::build(&values).unwrap();

// Adjacent leaves (siblings)
let pos_list: Vec<usize> = vec![0, 1];
let batch_proof = merkle_tree.get_batch_proof(&pos_list).unwrap();
let leaves_to_verify: Vec<FE> = pos_list.iter().map(|&i| values[i]).collect();

assert!(
batch_proof.verify::<FieldElementBackend<F, Keccak256, 32>>(
&merkle_tree.root,
&pos_list,
&leaves_to_verify,
8
),
"Batch proof verification failed for adjacent leaves"
);
}

/// Tests that batch proof fails with wrong values using real hash.
#[test]
fn batch_proof_with_keccak256_fails_with_wrong_values() {
let values: Vec<FE> = (1..=8).map(FE::from).collect();
let merkle_tree =
MerkleTree::<FieldElementBackend<F, Keccak256, 32>>::build(&values).unwrap();

let pos_list: Vec<usize> = vec![0, 1];
let batch_proof = merkle_tree.get_batch_proof(&pos_list).unwrap();

// Use wrong values
let wrong_values: Vec<FE> = vec![FE::from(999), FE::from(998)];

assert!(
!batch_proof.verify::<FieldElementBackend<F, Keccak256, 32>>(
&merkle_tree.root,
&pos_list,
&wrong_values,
8
),
"Batch proof should fail with wrong values"
);
}
}
124 changes: 0 additions & 124 deletions crypto/crypto/src/merkle_tree/backends/field_element_vector.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,127 +144,3 @@ where
P::hash(left, right)
}
}

#[cfg(test)]
mod tests {
use math::field::{element::FieldElement, goldilocks::GoldilocksField};
use sha2::Sha512;
use sha3::{Keccak256, Keccak512, Sha3_256, Sha3_512};

use crate::merkle_tree::{
backends::field_element_vector::FieldElementVectorBackend, merkle::MerkleTree,
};

type F = GoldilocksField;
type FE = FieldElement<F>;

#[test]
fn hash_data_field_element_backend_works_with_sha3_256() {
let values = [
vec![FE::from(2u64), FE::from(11u64)],
vec![FE::from(3u64), FE::from(14u64)],
vec![FE::from(4u64), FE::from(7u64)],
vec![FE::from(5u64), FE::from(3u64)],
vec![FE::from(6u64), FE::from(5u64)],
vec![FE::from(7u64), FE::from(16u64)],
vec![FE::from(8u64), FE::from(19u64)],
vec![FE::from(9u64), FE::from(21u64)],
];
let merkle_tree =
MerkleTree::<FieldElementVectorBackend<F, Sha3_256, 32>>::build(&values).unwrap();
let proof = merkle_tree.get_proof_by_pos(0).unwrap();
assert!(proof.verify::<FieldElementVectorBackend<F, Sha3_256, 32>>(
&merkle_tree.root,
0,
&values[0]
));
}

#[test]
fn hash_data_field_element_backend_works_with_keccak256() {
let values = [
vec![FE::from(2u64), FE::from(11u64)],
vec![FE::from(3u64), FE::from(14u64)],
vec![FE::from(4u64), FE::from(7u64)],
vec![FE::from(5u64), FE::from(3u64)],
vec![FE::from(6u64), FE::from(5u64)],
vec![FE::from(7u64), FE::from(16u64)],
vec![FE::from(8u64), FE::from(19u64)],
vec![FE::from(9u64), FE::from(21u64)],
];
let merkle_tree =
MerkleTree::<FieldElementVectorBackend<F, Keccak256, 32>>::build(&values).unwrap();
let proof = merkle_tree.get_proof_by_pos(0).unwrap();
assert!(proof.verify::<FieldElementVectorBackend<F, Keccak256, 32>>(
&merkle_tree.root,
0,
&values[0]
));
}

#[test]
fn hash_data_field_element_backend_works_with_sha3_512() {
let values = [
vec![FE::from(2u64), FE::from(11u64)],
vec![FE::from(3u64), FE::from(14u64)],
vec![FE::from(4u64), FE::from(7u64)],
vec![FE::from(5u64), FE::from(3u64)],
vec![FE::from(6u64), FE::from(5u64)],
vec![FE::from(7u64), FE::from(16u64)],
vec![FE::from(8u64), FE::from(19u64)],
vec![FE::from(9u64), FE::from(21u64)],
];
let merkle_tree =
MerkleTree::<FieldElementVectorBackend<F, Sha3_512, 64>>::build(&values).unwrap();
let proof = merkle_tree.get_proof_by_pos(0).unwrap();
assert!(proof.verify::<FieldElementVectorBackend<F, Sha3_512, 64>>(
&merkle_tree.root,
0,
&values[0]
));
}

#[test]
fn hash_data_field_element_backend_works_with_keccak512() {
let values = [
vec![FE::from(2u64), FE::from(11u64)],
vec![FE::from(3u64), FE::from(14u64)],
vec![FE::from(4u64), FE::from(7u64)],
vec![FE::from(5u64), FE::from(3u64)],
vec![FE::from(6u64), FE::from(5u64)],
vec![FE::from(7u64), FE::from(16u64)],
vec![FE::from(8u64), FE::from(19u64)],
vec![FE::from(9u64), FE::from(21u64)],
];
let merkle_tree =
MerkleTree::<FieldElementVectorBackend<F, Keccak512, 64>>::build(&values).unwrap();
let proof = merkle_tree.get_proof_by_pos(0).unwrap();
assert!(proof.verify::<FieldElementVectorBackend<F, Keccak512, 64>>(
&merkle_tree.root,
0,
&values[0]
));
}

#[test]
fn hash_data_field_element_backend_works_with_sha2_512() {
let values = [
vec![FE::from(2u64), FE::from(11u64)],
vec![FE::from(3u64), FE::from(14u64)],
vec![FE::from(4u64), FE::from(7u64)],
vec![FE::from(5u64), FE::from(3u64)],
vec![FE::from(6u64), FE::from(5u64)],
vec![FE::from(7u64), FE::from(16u64)],
vec![FE::from(8u64), FE::from(19u64)],
vec![FE::from(9u64), FE::from(21u64)],
];
let merkle_tree =
MerkleTree::<FieldElementVectorBackend<F, Sha512, 64>>::build(&values).unwrap();
let proof = merkle_tree.get_proof_by_pos(0).unwrap();
assert!(proof.verify::<FieldElementVectorBackend<F, Sha512, 64>>(
&merkle_tree.root,
0,
&values[0]
));
}
}
38 changes: 1 addition & 37 deletions crypto/crypto/src/merkle_tree/merkle.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ pub struct MerkleTree<B: IsMerkleTreeBackend> {
nodes: Vec<B::Node>,
#[cfg(feature = "disk-spill")]
#[cfg_attr(feature = "serde", serde(skip))]
mmap_backing: Option<MmapNodeBacking>,
pub(crate) mmap_backing: Option<MmapNodeBacking>,
Copy link
Copy Markdown

@claude claude Bot May 22, 2026

Choose a reason for hiding this comment

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

Medium — unnecessary visibility widening

mmap_backing is promoted to pub(crate) solely so the relocated test can write restored.mmap_backing.is_none(). This leaks a disk-spill implementation detail to every crate-internal caller, not just test code.

Preferred alternatives (in order of preference):

  1. Add a #[cfg(test)] accessor on MerkleTree so the visibility stays private outside tests:
Suggested change
pub(crate) mmap_backing: Option<MmapNodeBacking>,
mmap_backing: Option<MmapNodeBacking>, 
#[cfg(test)]
impl<B: IsMerkleTreeBackend> MerkleTree<B> {
    pub(crate) fn has_mmap_backing(&self) -> bool {
        self.mmap_backing.is_some()
    }
}
  1. Test a weaker but equivalent invariant that doesn't require field access, e.g. check that restored.nodes is non-empty instead.

}

// `mmap_backing` is `#[serde(skip)]` and `spill_nodes_to_disk` empties `nodes`,
Expand Down Expand Up @@ -352,39 +352,3 @@ where
Ok(())
}
}

#[cfg(all(test, feature = "serde", feature = "disk-spill"))]
mod disk_spill_serde_tests {
use super::*;
use crate::merkle_tree::backends::field_element::FieldElementBackend;
use math::field::{element::FieldElement, goldilocks::GoldilocksField};
use sha3::Keccak256;

type F = GoldilocksField;
type FE = FieldElement<F>;
type Backend = FieldElementBackend<F, Keccak256, 32>;

/// Serializing a spilled MerkleTree must produce identical bytes to
/// serializing the same tree before spilling, and round-trip back to an
/// equal tree.
#[test]
fn test_serialize_spilled_merkle_tree_matches_unspilled() {
let values: Vec<FE> = (1..17).map(FE::from).collect();
let unspilled = MerkleTree::<Backend>::build(&values).expect("build merkle tree");
let unspilled_bytes = bincode::serialize(&unspilled).expect("serialize unspilled");

let mut spilled = MerkleTree::<Backend>::build(&values).expect("build merkle tree");
spilled.spill_nodes_to_disk().expect("spill_nodes_to_disk");
let spilled_bytes = bincode::serialize(&spilled).expect("serialize spilled");

assert_eq!(
spilled_bytes, unspilled_bytes,
"spilled and unspilled trees must serialize to identical bytes"
);

let restored: MerkleTree<Backend> =
bincode::deserialize(&spilled_bytes).expect("deserialize spilled bytes");
assert!(restored.mmap_backing.is_none());
assert_eq!(restored.root, unspilled.root);
}
}
Loading
Loading