Skip to content

Commit

Permalink
refactor!: Check bounds in list::set::Set
Browse files Browse the repository at this point in the history
BREAKING CHANGE: Make fields of `list::set::Set` private. To construct a
new instance, use function `new`. Also, remove the implementation of
`DeprecatedSnippet` for `list::set::Set`, implement `BasicSnippet`
directly.
  • Loading branch information
jan-ferdinand committed Jan 23, 2025
1 parent bb4245c commit d6ca00a
Show file tree
Hide file tree
Showing 6 changed files with 279 additions and 303 deletions.
20 changes: 10 additions & 10 deletions tasm-lib/benchmarks/tasmlib_list_set_element___digest.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,22 +2,22 @@
{
"name": "tasmlib_list_set_element___digest",
"benchmark_result": {
"clock_cycle_count": 10,
"hash_table_height": 12,
"u32_table_height": 0,
"op_stack_table_height": 11,
"ram_table_height": 5
"clock_cycle_count": 24,
"hash_table_height": 24,
"u32_table_height": 15,
"op_stack_table_height": 21,
"ram_table_height": 6
},
"case": "CommonCase"
},
{
"name": "tasmlib_list_set_element___digest",
"benchmark_result": {
"clock_cycle_count": 10,
"hash_table_height": 12,
"u32_table_height": 0,
"op_stack_table_height": 11,
"ram_table_height": 5
"clock_cycle_count": 24,
"hash_table_height": 24,
"u32_table_height": 18,
"op_stack_table_height": 21,
"ram_table_height": 6
},
"case": "WorstCase"
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,22 +2,22 @@
{
"name": "tasmlib_mmr_calculate_new_peaks_from_leaf_mutation",
"benchmark_result": {
"clock_cycle_count": 2422,
"hash_table_height": 414,
"u32_table_height": 1263,
"op_stack_table_height": 1574,
"ram_table_height": 191
"clock_cycle_count": 2436,
"hash_table_height": 426,
"u32_table_height": 1265,
"op_stack_table_height": 1584,
"ram_table_height": 192
},
"case": "CommonCase"
},
{
"name": "tasmlib_mmr_calculate_new_peaks_from_leaf_mutation",
"benchmark_result": {
"clock_cycle_count": 4636,
"hash_table_height": 600,
"u32_table_height": 2158,
"op_stack_table_height": 3072,
"ram_table_height": 377
"clock_cycle_count": 4650,
"hash_table_height": 612,
"u32_table_height": 2160,
"op_stack_table_height": 3082,
"ram_table_height": 378
},
"case": "WorstCase"
}
Expand Down
1 change: 1 addition & 0 deletions tasm-lib/src/assertion_error_ids.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,3 +48,4 @@ often.
| 360..370 | [`u64::Pow2`](arithmetic/u64/pow2.rs) |
| 370..380 | [`u64::ShiftLeft`](arithmetic/u64/shift_left.rs) |
| 380..390 | [`list::get`](list/get.rs) |
| 390..400 | [`list::set`](list/set.rs) |
57 changes: 37 additions & 20 deletions tasm-lib/src/list/get.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,24 @@ impl Get {

/// # Panics
///
/// Panics if the element has [dynamic length][BFieldCodec::static_length].
/// Panics if the element has [dynamic length][BFieldCodec::static_length], or
/// if the static length is 0.
pub fn new(element_type: DataType) -> Self {
let has_static_len = element_type.static_length().is_some();
assert!(has_static_len, "element should have static length");
Self::assert_element_type_is_supported(&element_type);

Self { element_type }
}

/// # Panics
///
/// Panics if the element has [dynamic length][BFieldCodec::static_length], or
/// if the static length is 0.
pub(crate) fn assert_element_type_is_supported(element_type: &DataType) {
let Some(static_len) = element_type.static_length() else {
panic!("element should have static length");
};
assert_ne!(0, static_len, "element must not be zero-sized");
}
}

impl BasicSnippet for Get {
Expand All @@ -50,13 +61,12 @@ impl BasicSnippet for Get {
}

fn code(&self, library: &mut Library) -> Vec<LabelledInstruction> {
let list_length = library.import(Box::new(Length));
let mul_with_element_size = match self.element_type.stack_size() {
1 => triton_asm!(/* no-op */),
1 => triton_asm!(), // no-op
n => triton_asm!(push {n} mul),
};

let list_length = library.import(Box::new(Length));

triton_asm!(
// BEFORE: _ *list index
// AFTER: _ [element: self.element_type]
Expand Down Expand Up @@ -89,7 +99,7 @@ impl BasicSnippet for Get {
}

#[cfg(test)]
mod tests {
pub(crate) mod tests {
use triton_vm::error::OpStackError::FailedU32Conversion;

use super::*;
Expand All @@ -115,6 +125,23 @@ mod tests {

AccessorInitialState { stack, memory }
}

pub fn random_len_idx_ptr(
bench_case: Option<BenchmarkCase>,
rng: &mut impl Rng,
) -> (usize, usize, BFieldElement) {
let (index, list_length) = match bench_case {
Some(BenchmarkCase::CommonCase) => (16, 32),
Some(BenchmarkCase::WorstCase) => (63, 64),
None => {
let list_length = rng.gen_range(1..=100);
(rng.gen_range(0..list_length), list_length)
}
};
let list_pointer = rng.gen();

(list_length, index, list_pointer)
}
}

impl Accessor for Get {
Expand All @@ -138,18 +165,8 @@ mod tests {
seed: [u8; 32],
bench_case: Option<BenchmarkCase>,
) -> AccessorInitialState {
let mut rng = StdRng::from_seed(seed);
let list_length = match bench_case {
Some(BenchmarkCase::CommonCase) => 1 << 5,
Some(BenchmarkCase::WorstCase) => 1 << 6,
None => rng.gen_range(1..=100),
};
let index = match bench_case {
Some(BenchmarkCase::CommonCase) => list_length / 2,
Some(BenchmarkCase::WorstCase) => list_length - 1,
None => rng.gen_range(0..list_length),
};
let list_pointer = rng.gen();
let (list_length, index, list_pointer) =
Self::random_len_idx_ptr(bench_case, &mut StdRng::from_seed(seed));

self.set_up_initial_state(list_length, index, list_pointer)
}
Expand Down Expand Up @@ -237,7 +254,7 @@ mod benches {
use crate::test_prelude::*;

#[test]
fn get_benchmark() {
fn benchmark() {
ShadowedAccessor::new(Get::new(DataType::Digest)).bench();
}
}
Loading

0 comments on commit d6ca00a

Please sign in to comment.