Skip to content

Commit

Permalink
chore: avoid cloning and use .map/.entry in Rust code (#181)
Browse files Browse the repository at this point in the history
  • Loading branch information
@kevinji
kevinji committed Sep 5, 2023
1 parent b6985d0 commit 9f86b67
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 74 deletions.
41 changes: 22 additions & 19 deletions rust/src/api.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -582,26 +582,29 @@ mod tests {

#[cfg(feature = "std")]
fn load_batch(files: &[&str]) -> Result<(ics23::CommitmentProof, Vec<RefData>)> {
let mut entries = Vec::new();
let mut data = Vec::new();

for &file in files {
let (proof, datum) = load_file(file)?;
data.push(datum);
match proof.proof {
Some(ics23::commitment_proof::Proof::Nonexist(non)) => {
entries.push(ics23::BatchEntry {
proof: Some(ics23::batch_entry::Proof::Nonexist(non)),
})
}
Some(ics23::commitment_proof::Proof::Exist(ex)) => {
entries.push(ics23::BatchEntry {
proof: Some(ics23::batch_entry::Proof::Exist(ex)),
})
let (data, entries) = files
.iter()
.map(|file| {
let (proof, datum) = load_file(file)?;
match proof.proof {
Some(ics23::commitment_proof::Proof::Nonexist(non)) => Ok((
datum,
ics23::BatchEntry {
proof: Some(ics23::batch_entry::Proof::Nonexist(non)),
},
)),
Some(ics23::commitment_proof::Proof::Exist(ex)) => Ok((
datum,
ics23::BatchEntry {
proof: Some(ics23::batch_entry::Proof::Exist(ex)),
},
)),
_ => bail!("unknown proof type to batch"),
}
_ => bail!("unknown proof type to batch"),
}
}
})
.collect::<Result<Vec<_>>>()?
.into_iter()
.unzip();

let batch = ics23::CommitmentProof {
proof: Some(ics23::commitment_proof::Proof::Batch(ics23::BatchProof {
Expand Down
91 changes: 43 additions & 48 deletions rust/src/compress.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,42 +30,43 @@ pub fn decompress(proof: &ics23::CommitmentProof) -> Result<ics23::CommitmentPro
}

pub fn compress_batch(proof: &ics23::BatchProof) -> Result<ics23::CommitmentProof> {
let mut centries = Vec::new();
let mut lookup = Vec::new();
let mut registry = HashMap::new();

for entry in &proof.entries {
let centry = match &entry.proof {
let centries = proof
.entries
.iter()
.map(|entry| match &entry.proof {
Some(ics23::batch_entry::Proof::Exist(ex)) => {
let exist = compress_exist(ex, &mut lookup, &mut registry)?;
ics23::CompressedBatchEntry {
Ok(ics23::CompressedBatchEntry {
proof: Some(ics23::compressed_batch_entry::Proof::Exist(exist)),
}
})
}
Some(ics23::batch_entry::Proof::Nonexist(non)) => {
let left = non
.left
.clone()
.map(|l| compress_exist(&l, &mut lookup, &mut registry))
.as_ref()
.map(|l| compress_exist(l, &mut lookup, &mut registry))
.transpose()?;
let right = non
.right
.clone()
.map(|r| compress_exist(&r, &mut lookup, &mut registry))
.as_ref()
.map(|r| compress_exist(r, &mut lookup, &mut registry))
.transpose()?;
let nonexist = ics23::CompressedNonExistenceProof {
key: non.key.clone(),
left,
right,
};
ics23::CompressedBatchEntry {
Ok(ics23::CompressedBatchEntry {
proof: Some(ics23::compressed_batch_entry::Proof::Nonexist(nonexist)),
}
})
}
None => ics23::CompressedBatchEntry { proof: None },
};
centries.push(centry);
}
None => Ok(ics23::CompressedBatchEntry { proof: None }),
})
.collect::<Result<_>>()?;

Ok(ics23::CommitmentProof {
proof: Some(ics23::commitment_proof::Proof::Compressed(
ics23::CompressedBatchProof {
Expand All @@ -85,19 +86,15 @@ pub fn compress_exist(
.path
.iter()
.map(|x| {
let mut buf = Vec::new();
x.encode(&mut buf)
.map_err(|e: prost::EncodeError| anyhow::anyhow!(e))?;

if let Some(&idx) = registry.get(buf.as_slice()) {
return Ok(idx);
};
let idx = lookup.len() as i32;
lookup.push(x.to_owned());
registry.insert(buf, idx);
let buf = x.encode_to_vec();
let idx = *registry.entry(buf).or_insert_with(|| {
let idx = lookup.len() as i32;
lookup.push(x.to_owned());
idx
});
Ok(idx)
})
.collect::<Result<Vec<i32>>>()?;
.collect::<Result<_>>()?;

Ok(ics23::CompressedExistenceProof {
key: exist.key.clone(),
Expand All @@ -112,30 +109,28 @@ pub fn decompress_batch(proof: &ics23::CompressedBatchProof) -> Result<ics23::Co
let entries = proof
.entries
.iter()
.map(|cent| -> Result<ics23::BatchEntry> {
match &cent.proof {
Some(ics23::compressed_batch_entry::Proof::Exist(ex)) => {
let exist = decompress_exist(ex, lookup);
Ok(ics23::BatchEntry {
proof: Some(ics23::batch_entry::Proof::Exist(exist)),
})
}
Some(ics23::compressed_batch_entry::Proof::Nonexist(non)) => {
let left = non.left.clone().map(|l| decompress_exist(&l, lookup));
let right = non.right.clone().map(|r| decompress_exist(&r, lookup));
let nonexist = ics23::NonExistenceProof {
key: non.key.clone(),
left,
right,
};
Ok(ics23::BatchEntry {
proof: Some(ics23::batch_entry::Proof::Nonexist(nonexist)),
})
}
None => Ok(ics23::BatchEntry { proof: None }),
.map(|cent| match &cent.proof {
Some(ics23::compressed_batch_entry::Proof::Exist(ex)) => {
let exist = decompress_exist(ex, lookup);
Ok(ics23::BatchEntry {
proof: Some(ics23::batch_entry::Proof::Exist(exist)),
})
}
Some(ics23::compressed_batch_entry::Proof::Nonexist(non)) => {
let left = non.left.as_ref().map(|l| decompress_exist(l, lookup));
let right = non.right.as_ref().map(|r| decompress_exist(r, lookup));
let nonexist = ics23::NonExistenceProof {
key: non.key.clone(),
left,
right,
};
Ok(ics23::BatchEntry {
proof: Some(ics23::batch_entry::Proof::Nonexist(nonexist)),
})
}
None => Ok(ics23::BatchEntry { proof: None }),
})
.collect::<Result<Vec<_>>>()?;
.collect::<Result<_>>()?;

Ok(ics23::CommitmentProof {
proof: Some(ics23::commitment_proof::Proof::Batch(ics23::BatchProof {
Expand Down
19 changes: 12 additions & 7 deletions rust/src/verify.rs
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// we want to name functions verify_* to match ics23
#![allow(clippy::module_name_repetitions)]

use alloc::borrow::Cow;
use alloc::vec::Vec;

use anyhow::{bail, ensure};
Expand Down Expand Up @@ -39,7 +40,11 @@ pub fn verify_non_existence<H: HostFunctionsProvider>(
let key_for_comparison = |key: &[u8]| -> Vec<u8> {
match spec.prehash_key_before_comparison {
true => do_hash::<H>(
spec.leaf_spec.clone().unwrap_or_default().prehash_key(),
spec.leaf_spec
.as_ref()
.map(Cow::Borrowed)
.unwrap_or_default()
.prehash_key(),
key,
),
false => key.to_vec(),
Expand Down Expand Up @@ -236,8 +241,8 @@ fn ensure_left_neighbor(
left: &[ics23::InnerOp],
right: &[ics23::InnerOp],
) -> Result<()> {
let mut mut_left = Vec::from(left);
let mut mut_right = Vec::from(right);
let mut mut_left = left.to_vec();
let mut mut_right = right.to_vec();

let mut top_left = mut_left.pop().unwrap();
let mut top_right = mut_right.pop().unwrap();
Expand All @@ -252,7 +257,8 @@ fn ensure_left_neighbor(
}

ensure_right_most(spec, &mut_left)?;
ensure_left_most(spec, &mut_right)
ensure_left_most(spec, &mut_right)?;
Ok(())
}

fn is_left_step(
Expand Down Expand Up @@ -616,8 +622,7 @@ mod tests {
},
),
]
.iter()
.cloned()
.into_iter()
.collect();

for (name, tc) in cases {
Expand Down Expand Up @@ -758,7 +763,7 @@ mod tests {

for (i, case) in cases.iter().enumerate() {
ensure_inner(&case.op, case.spec)?;
let inner = &case.spec.inner_spec.as_ref().unwrap();
let inner = case.spec.inner_spec.as_ref().unwrap();
assert_eq!(
case.is_left,
left_branches_are_empty(inner, &case.op)?,
Expand Down

0 comments on commit 9f86b67

Please sign in to comment.