Skip to content

Commit a14e221

Browse files
committed
perf(trie): use Arc for proof sharing and add batch validation
- Wrap DecodedMultiProof and DecodedStorageMultiProof in Arc within ProofResult for O(1) cloning when sending batched results to multiple receivers - Add debug assertions in BatchedStorageProof::merge and BatchedAccountProof::merge to validate that all batched jobs share the same Arc for multi_added_removed_keys and missed_leaves_storage_roots (critical invariants for correctness) - Unwrap Arc at extraction sites using try_unwrap for zero-cost when sole owner
1 parent 1ea84bb commit a14e221

File tree

2 files changed

+78
-16
lines changed

2 files changed

+78
-16
lines changed

crates/trie/parallel/src/proof.rs

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -126,15 +126,17 @@ impl ParallelProof {
126126
)))
127127
})?;
128128

129-
// Extract storage proof directly from the result
129+
// Extract storage proof directly from the result.
130+
// The proof is Arc-wrapped for efficient batch sharing, so we unwrap it here.
130131
let storage_proof = match proof_msg.result? {
131132
crate::proof_task::ProofResult::StorageProof { hashed_address: addr, proof } => {
132133
debug_assert_eq!(
133134
addr,
134135
hashed_address,
135136
"storage worker must return same address: expected {hashed_address}, got {addr}"
136137
);
137-
proof
138+
// Efficiently unwrap Arc: returns inner value if sole owner, clones otherwise.
139+
Arc::try_unwrap(proof).unwrap_or_else(|arc| (*arc).clone())
138140
}
139141
crate::proof_task::ProofResult::AccountMultiproof { .. } => {
140142
unreachable!("storage worker only sends StorageProof variant")
@@ -223,8 +225,12 @@ impl ParallelProof {
223225
)
224226
})?;
225227

228+
// The proof is Arc-wrapped for efficient batch sharing, so we unwrap it here.
226229
let (multiproof, stats) = match proof_result_msg.result? {
227-
crate::proof_task::ProofResult::AccountMultiproof { proof, stats } => (proof, stats),
230+
crate::proof_task::ProofResult::AccountMultiproof { proof, stats } => {
231+
// Efficiently unwrap Arc: returns inner value if sole owner, clones otherwise.
232+
(Arc::try_unwrap(proof).unwrap_or_else(|arc| (*arc).clone()), stats)
233+
}
228234
crate::proof_task::ProofResult::StorageProof { .. } => {
229235
unreachable!("account worker only sends AccountMultiproof variant")
230236
}

crates/trie/parallel/src/proof_task.rs

Lines changed: 69 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,22 @@ impl BatchedStorageProof {
116116
}
117117

118118
/// Merges another storage proof job into this batch.
119+
///
120+
/// # Panics (debug builds only)
121+
/// Panics if `input.multi_added_removed_keys` does not point to the same Arc as the batch's.
119122
fn merge(&mut self, input: StorageProofInput, sender: ProofResultContext) {
123+
// Validate that all batched jobs share the same multi_added_removed_keys Arc.
124+
// This is a critical invariant: if jobs have different keys, the merged proof
125+
// would be computed with only the first job's keys, producing incorrect results.
126+
debug_assert!(
127+
match (&self.multi_added_removed_keys, &input.multi_added_removed_keys) {
128+
(Some(a), Some(b)) => Arc::ptr_eq(a, b),
129+
(None, None) => true,
130+
_ => false,
131+
},
132+
"All batched storage proof jobs must share the same multi_added_removed_keys Arc"
133+
);
134+
120135
self.prefix_set.extend_keys(input.prefix_set.iter().copied());
121136
self.target_slots.extend(input.target_slots);
122137
self.with_branch_node_masks |= input.with_branch_node_masks;
@@ -221,7 +236,31 @@ impl BatchedAccountProof {
221236
}
222237

223238
/// Merges another account multiproof job into this batch.
239+
///
240+
/// # Panics (debug builds only)
241+
/// Panics if `input.multi_added_removed_keys` or `input.missed_leaves_storage_roots`
242+
/// do not point to the same Arc as the batch's.
224243
fn merge(&mut self, input: AccountMultiproofInput) {
244+
// Validate that all batched jobs share the same multi_added_removed_keys Arc.
245+
// This is a critical invariant: if jobs have different keys, the merged proof
246+
// would be computed with only the first job's keys, producing incorrect results.
247+
debug_assert!(
248+
match (&self.multi_added_removed_keys, &input.multi_added_removed_keys) {
249+
(Some(a), Some(b)) => Arc::ptr_eq(a, b),
250+
(None, None) => true,
251+
_ => false,
252+
},
253+
"All batched account proof jobs must share the same multi_added_removed_keys Arc"
254+
);
255+
256+
// Validate that all batched jobs share the same missed_leaves_storage_roots cache.
257+
// This is critical because workers may add entries to this cache during proof computation,
258+
// and all receivers expect to see those entries in their shared cache.
259+
debug_assert!(
260+
Arc::ptr_eq(&self.missed_leaves_storage_roots, &input.missed_leaves_storage_roots),
261+
"All batched account proof jobs must share the same missed_leaves_storage_roots Arc"
262+
);
263+
225264
// Merge targets.
226265
self.targets.extend(input.targets);
227266

@@ -780,21 +819,25 @@ impl TrieNodeProvider for ProofTaskTrieNodeProvider {
780819
}
781820
}
782821
/// Result of a proof calculation, which can be either an account multiproof or a storage proof.
822+
///
823+
/// The proof data is wrapped in `Arc` to enable efficient sharing when batching multiple
824+
/// proof requests. This avoids expensive cloning of the underlying proof structures
825+
/// when sending results to multiple receivers.
783826
#[derive(Debug, Clone)]
784827
pub enum ProofResult {
785828
/// Account multiproof with statistics
786829
AccountMultiproof {
787-
/// The account multiproof
788-
proof: DecodedMultiProof,
830+
/// The account multiproof (Arc-wrapped for efficient sharing in batches)
831+
proof: Arc<DecodedMultiProof>,
789832
/// Statistics collected during proof computation
790833
stats: ParallelTrieStats,
791834
},
792835
/// Storage proof for a specific account
793836
StorageProof {
794837
/// The hashed address this storage proof belongs to
795838
hashed_address: B256,
796-
/// The storage multiproof
797-
proof: DecodedStorageMultiProof,
839+
/// The storage multiproof (Arc-wrapped for efficient sharing in batches)
840+
proof: Arc<DecodedStorageMultiProof>,
798841
},
799842
}
800843

@@ -803,11 +846,17 @@ impl ProofResult {
803846
///
804847
/// For account multiproofs, returns the multiproof directly (discarding stats).
805848
/// For storage proofs, wraps the storage proof into a minimal multiproof.
849+
///
850+
/// Note: This method clones the inner proof data. If you need to avoid the clone
851+
/// when you're the sole owner, consider using `Arc::try_unwrap` first.
806852
pub fn into_multiproof(self) -> DecodedMultiProof {
807853
match self {
808-
Self::AccountMultiproof { proof, stats: _ } => proof,
854+
Self::AccountMultiproof { proof, stats: _ } => {
855+
Arc::try_unwrap(proof).unwrap_or_else(|arc| (*arc).clone())
856+
}
809857
Self::StorageProof { hashed_address, proof } => {
810-
DecodedMultiProof::from_storage_proof(hashed_address, proof)
858+
let storage_proof = Arc::try_unwrap(proof).unwrap_or_else(|arc| (*arc).clone());
859+
DecodedMultiProof::from_storage_proof(hashed_address, storage_proof)
811860
}
812861
}
813862
}
@@ -1139,9 +1188,9 @@ where
11391188
let num_senders = senders.len();
11401189
match result {
11411190
Ok(storage_proof) => {
1142-
// Success case: clone the proof for each sender.
1191+
// Success case: wrap proof in Arc for efficient sharing across all senders.
11431192
let proof_result =
1144-
ProofResult::StorageProof { hashed_address, proof: storage_proof };
1193+
ProofResult::StorageProof { hashed_address, proof: Arc::new(storage_proof) };
11451194

11461195
for ProofResultContext { sender, sequence_number, state, start_time } in senders {
11471196
*storage_proofs_processed += 1;
@@ -1562,8 +1611,8 @@ where
15621611
let num_senders = senders.len();
15631612
match result {
15641613
Ok(proof) => {
1565-
// Success case: clone the proof for each sender.
1566-
let proof_result = ProofResult::AccountMultiproof { proof, stats };
1614+
// Success case: wrap proof in Arc for efficient sharing across all senders.
1615+
let proof_result = ProofResult::AccountMultiproof { proof: Arc::new(proof), stats };
15671616

15681617
for ProofResultContext { sender, sequence_number, state, start_time } in senders {
15691618
*account_proofs_processed += 1;
@@ -1764,15 +1813,19 @@ where
17641813

17651814
drop(_guard);
17661815

1767-
// Extract storage proof from the result
1816+
// Extract storage proof from the result.
1817+
// The proof is Arc-wrapped for efficient batch sharing, so we unwrap it
1818+
// here.
17681819
let proof = match proof_msg.result? {
17691820
ProofResult::StorageProof { hashed_address: addr, proof } => {
17701821
debug_assert_eq!(
17711822
addr,
17721823
hashed_address,
17731824
"storage worker must return same address: expected {hashed_address}, got {addr}"
17741825
);
1775-
proof
1826+
// Efficiently unwrap Arc: returns inner value if sole owner, clones
1827+
// otherwise.
1828+
Arc::try_unwrap(proof).unwrap_or_else(|arc| (*arc).clone())
17761829
}
17771830
ProofResult::AccountMultiproof { .. } => {
17781831
unreachable!("storage worker only sends StorageProof variant")
@@ -1835,8 +1888,11 @@ where
18351888
// Consume remaining storage proof receivers for accounts not encountered during trie walk.
18361889
for (hashed_address, receiver) in storage_proof_receivers {
18371890
if let Ok(proof_msg) = receiver.recv() {
1838-
// Extract storage proof from the result
1891+
// Extract storage proof from the result.
1892+
// The proof is Arc-wrapped for efficient batch sharing, so we unwrap it here.
18391893
if let Ok(ProofResult::StorageProof { proof, .. }) = proof_msg.result {
1894+
// Efficiently unwrap Arc: returns inner value if sole owner, clones otherwise.
1895+
let proof = Arc::try_unwrap(proof).unwrap_or_else(|arc| (*arc).clone());
18401896
collected_decoded_storages.insert(hashed_address, proof);
18411897
}
18421898
}

0 commit comments

Comments
 (0)