From 62f785b15f1cf073f564db3c7385892666047709 Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Thu, 28 Aug 2025 17:29:03 +1000 Subject: [PATCH 01/11] Optimise FixedVector Decode --- Cargo.toml | 6 ++++ benches/decode.rs | 84 +++++++++++++++++++++++++++++++++++++++++++++ src/fixed_vector.rs | 24 ++++++++----- 3 files changed, 105 insertions(+), 9 deletions(-) create mode 100644 benches/decode.rs diff --git a/Cargo.toml b/Cargo.toml index f37d1e5..985bd4d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -22,5 +22,11 @@ arbitrary = { version = "1.0", features = ["derive"], optional = true } itertools = "0.14.0" [dev-dependencies] +criterion = "0.7.0" +ethereum_ssz_derive = "0.9.0" serde_json = "1.0.0" tree_hash_derive = "0.10.0" + +[[bench]] +harness = false +name = "decode" diff --git a/benches/decode.rs b/benches/decode.rs new file mode 100644 index 0000000..35c6fbe --- /dev/null +++ b/benches/decode.rs @@ -0,0 +1,84 @@ +use criterion::{Criterion, criterion_group, criterion_main}; +use ssz::{Decode, DecodeError, Encode}; +use ssz_types::FixedVector; +use std::hint::black_box; +use typenum::{U131072, Unsigned}; + +#[derive(Clone, Debug, Default, PartialEq, Eq, ssz_derive::Encode)] +#[ssz(struct_behaviour = "transparent")] +pub struct ByteVector(FixedVector); + +impl ssz::Decode for ByteVector { + fn is_ssz_fixed_len() -> bool { + true + } + + fn from_ssz_bytes(bytes: &[u8]) -> Result { + FixedVector::new(bytes.to_vec()) + .map(Self) + .map_err(|e| DecodeError::BytesInvalid(format!("{e:?}"))) + } + + fn ssz_fixed_len() -> usize { + as ssz::Decode>::ssz_fixed_len() + } +} + +#[derive(Clone, Debug, Default, PartialEq, Eq, ssz_derive::Encode)] +#[ssz(struct_behaviour = "transparent")] +pub struct FastU8(pub u8); + +impl ssz::Decode for FastU8 { + #[inline(always)] + fn is_ssz_fixed_len() -> bool { + true + } + + #[inline(always)] + fn from_ssz_bytes(bytes: &[u8]) -> Result { + if bytes.len() == 1 { + Ok(FastU8(bytes[0])) + } else { + Err(DecodeError::BytesInvalid("invalid".to_string())) + } + } + + #[inline(always)] + fn ssz_fixed_len() -> usize { + 1 + } +} + +fn benchmark_fixed_vector_decode(c: &mut Criterion) { + let mut group = c.benchmark_group("decode_fixed_vector"); + + let fixed_vector = FixedVector::::new(vec![255u8; 131072]).unwrap(); + let fixed_vector_bytes = fixed_vector.as_ssz_bytes(); + + group.bench_function("u8_128k", |b| { + b.iter(|| { + let vector = FixedVector::::from_ssz_bytes(&fixed_vector_bytes).unwrap(); + black_box(vector); + }); + }); + + group.bench_function("fast_u8_128k", |b| { + b.iter(|| { + let vector = + FixedVector::::from_ssz_bytes(&fixed_vector_bytes).unwrap(); + black_box(vector); + }); + }); + + group.bench_function("byte_u8_128k", |b| { + b.iter(|| { + let vector = ByteVector::::from_ssz_bytes(&fixed_vector_bytes).unwrap(); + black_box(vector); + }); + }); + + group.finish(); +} + +criterion_group!(benches, benchmark_fixed_vector_decode); +criterion_main!(benches); diff --git a/src/fixed_vector.rs b/src/fixed_vector.rs index 7d5d391..0499058 100644 --- a/src/fixed_vector.rs +++ b/src/fixed_vector.rs @@ -1,5 +1,5 @@ -use crate::tree_hash::vec_tree_hash_root; use crate::Error; +use crate::tree_hash::vec_tree_hash_root; use serde::Deserialize; use serde_derive::Serialize; use std::marker::PhantomData; @@ -312,13 +312,19 @@ where ))); } - let vec = bytes.chunks(T::ssz_fixed_len()).try_fold( - Vec::with_capacity(num_items), - |mut vec, chunk| { - vec.push(T::from_ssz_bytes(chunk)?); - Ok(vec) - }, - )?; + if bytes.len() != num_items * T::ssz_fixed_len() { + return Err(ssz::DecodeError::BytesInvalid(format!( + "FixedVector of {} items has {} bytes", + num_items, + bytes.len() + ))); + } + + let mut vec = Vec::with_capacity(num_items); + for chunk in bytes.chunks_exact(T::ssz_fixed_len()) { + vec.push(T::from_ssz_bytes(chunk)?); + } + Self::new(vec).map_err(|e| { ssz::DecodeError::BytesInvalid(format!( "Wrong number of FixedVector elements: {:?}", @@ -381,7 +387,7 @@ mod test { use super::*; use ssz::*; use std::collections::HashSet; - use tree_hash::{merkle_root, TreeHash}; + use tree_hash::{TreeHash, merkle_root}; use tree_hash_derive::TreeHash; use typenum::*; From b729e99e9b7056f4452889864d11d5de45432e61 Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Thu, 4 Sep 2025 10:44:50 +1000 Subject: [PATCH 02/11] Add encoding benchmarks --- Cargo.toml | 6 ++- benches/{decode.rs => encode_decode.rs} | 51 ++++++++----------------- src/fixed_vector.rs | 4 +- 3 files changed, 23 insertions(+), 38 deletions(-) rename benches/{decode.rs => encode_decode.rs} (55%) diff --git a/Cargo.toml b/Cargo.toml index 985bd4d..20d2298 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -29,4 +29,8 @@ tree_hash_derive = "0.10.0" [[bench]] harness = false -name = "decode" +name = "encode_decode" + +[patch.crates-io] +ethereum_ssz = { path = "../ethereum_ssz/ssz" } +ethereum_ssz_derive = { path = "../ethereum_ssz/ssz_derive" } diff --git a/benches/decode.rs b/benches/encode_decode.rs similarity index 55% rename from benches/decode.rs rename to benches/encode_decode.rs index 35c6fbe..4f0a2c8 100644 --- a/benches/decode.rs +++ b/benches/encode_decode.rs @@ -1,8 +1,8 @@ -use criterion::{Criterion, criterion_group, criterion_main}; +use criterion::{criterion_group, criterion_main, Criterion}; use ssz::{Decode, DecodeError, Encode}; use ssz_types::FixedVector; use std::hint::black_box; -use typenum::{U131072, Unsigned}; +use typenum::{Unsigned, U131072, U16384}; #[derive(Clone, Debug, Default, PartialEq, Eq, ssz_derive::Encode)] #[ssz(struct_behaviour = "transparent")] @@ -24,58 +24,39 @@ impl ssz::Decode for ByteVector { } } -#[derive(Clone, Debug, Default, PartialEq, Eq, ssz_derive::Encode)] -#[ssz(struct_behaviour = "transparent")] -pub struct FastU8(pub u8); - -impl ssz::Decode for FastU8 { - #[inline(always)] - fn is_ssz_fixed_len() -> bool { - true - } - - #[inline(always)] - fn from_ssz_bytes(bytes: &[u8]) -> Result { - if bytes.len() == 1 { - Ok(FastU8(bytes[0])) - } else { - Err(DecodeError::BytesInvalid("invalid".to_string())) - } - } - - #[inline(always)] - fn ssz_fixed_len() -> usize { - 1 - } -} - fn benchmark_fixed_vector_decode(c: &mut Criterion) { let mut group = c.benchmark_group("decode_fixed_vector"); - let fixed_vector = FixedVector::::new(vec![255u8; 131072]).unwrap(); - let fixed_vector_bytes = fixed_vector.as_ssz_bytes(); + let fixed_vector_u8 = FixedVector::::new(vec![255u8; 131072]).unwrap(); + let fixed_vector_u64 = FixedVector::::new(vec![255u64; 16384]).unwrap(); + let fixed_vector_bytes = fixed_vector_u8.as_ssz_bytes(); - group.bench_function("u8_128k", |b| { + group.bench_function("decode_u8_128k", |b| { b.iter(|| { let vector = FixedVector::::from_ssz_bytes(&fixed_vector_bytes).unwrap(); black_box(vector); }); }); - group.bench_function("fast_u8_128k", |b| { + group.bench_function("decode_byte_u8_128k", |b| { b.iter(|| { - let vector = - FixedVector::::from_ssz_bytes(&fixed_vector_bytes).unwrap(); + let vector = ByteVector::::from_ssz_bytes(&fixed_vector_bytes).unwrap(); black_box(vector); }); }); - group.bench_function("byte_u8_128k", |b| { + group.bench_function("decode_u64_16k", |b| { b.iter(|| { - let vector = ByteVector::::from_ssz_bytes(&fixed_vector_bytes).unwrap(); + let vector = FixedVector::::from_ssz_bytes(&fixed_vector_bytes).unwrap(); black_box(vector); }); }); + group.bench_function("encode_u64_16k", |b| { + b.iter(|| { + let bytes = fixed_vector_u64.as_ssz_bytes(); + black_box(bytes); + }); + }); group.finish(); } diff --git a/src/fixed_vector.rs b/src/fixed_vector.rs index 0499058..88f2e5a 100644 --- a/src/fixed_vector.rs +++ b/src/fixed_vector.rs @@ -1,5 +1,5 @@ -use crate::Error; use crate::tree_hash::vec_tree_hash_root; +use crate::Error; use serde::Deserialize; use serde_derive::Serialize; use std::marker::PhantomData; @@ -387,7 +387,7 @@ mod test { use super::*; use ssz::*; use std::collections::HashSet; - use tree_hash::{TreeHash, merkle_root}; + use tree_hash::{merkle_root, TreeHash}; use tree_hash_derive::TreeHash; use typenum::*; From 2f52ad075c0a65caee618e0486191fec0336aed6 Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Thu, 4 Sep 2025 10:53:07 +1000 Subject: [PATCH 03/11] u8 encoding benchmark --- benches/encode_decode.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/benches/encode_decode.rs b/benches/encode_decode.rs index 4f0a2c8..e8264ec 100644 --- a/benches/encode_decode.rs +++ b/benches/encode_decode.rs @@ -38,6 +38,13 @@ fn benchmark_fixed_vector_decode(c: &mut Criterion) { }); }); + group.bench_function("encode_u8_128k", |b| { + b.iter(|| { + let bytes = fixed_vector_u8.as_ssz_bytes(); + black_box(bytes); + }); + }); + group.bench_function("decode_byte_u8_128k", |b| { b.iter(|| { let vector = ByteVector::::from_ssz_bytes(&fixed_vector_bytes).unwrap(); From 56fd2f81e85174386f451d2f2980b85a5e05137c Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Thu, 4 Sep 2025 16:33:48 +1000 Subject: [PATCH 04/11] Update benchmarks to include encoding, optimise VariableList --- benches/encode_decode.rs | 78 +++++++++++++++++++++++++++++++--------- src/fixed_vector.rs | 27 ++++++++++++++ src/lib.rs | 2 +- src/variable_list.rs | 37 +++++++++++++++---- 4 files changed, 120 insertions(+), 24 deletions(-) diff --git a/benches/encode_decode.rs b/benches/encode_decode.rs index e8264ec..4981b6a 100644 --- a/benches/encode_decode.rs +++ b/benches/encode_decode.rs @@ -1,8 +1,9 @@ use criterion::{criterion_group, criterion_main, Criterion}; use ssz::{Decode, DecodeError, Encode}; -use ssz_types::FixedVector; +use ssz_types::{FixedVector, VariableList}; use std::hint::black_box; -use typenum::{Unsigned, U131072, U16384}; +use std::time::Duration; +use typenum::{Unsigned, U1048576, U131072}; #[derive(Clone, Debug, Default, PartialEq, Eq, ssz_derive::Encode)] #[ssz(struct_behaviour = "transparent")] @@ -24,43 +25,88 @@ impl ssz::Decode for ByteVector { } } -fn benchmark_fixed_vector_decode(c: &mut Criterion) { - let mut group = c.benchmark_group("decode_fixed_vector"); +fn benchmark_fixed_vector(c: &mut Criterion) { + let mut group = c.benchmark_group("fixed_vector"); - let fixed_vector_u8 = FixedVector::::new(vec![255u8; 131072]).unwrap(); - let fixed_vector_u64 = FixedVector::::new(vec![255u64; 16384]).unwrap(); + let fixed_vector_u8 = FixedVector::::new(vec![255u8; 1048576]).unwrap(); + let fixed_vector_u64 = FixedVector::::new(vec![255u64; 131072]).unwrap(); let fixed_vector_bytes = fixed_vector_u8.as_ssz_bytes(); - group.bench_function("decode_u8_128k", |b| { + group.warm_up_time(Duration::from_secs(15)); + group.measurement_time(Duration::from_secs(10)); + + group.bench_function("decode_byte_u8_1m", |b| { b.iter(|| { - let vector = FixedVector::::from_ssz_bytes(&fixed_vector_bytes).unwrap(); + let vector = ByteVector::::from_ssz_bytes(&fixed_vector_bytes).unwrap(); black_box(vector); }); }); - group.bench_function("encode_u8_128k", |b| { + group.bench_function("decode_u8_1m", |b| { + b.iter(|| { + let vector = FixedVector::::from_ssz_bytes(&fixed_vector_bytes).unwrap(); + black_box(vector); + }); + }); + + group.bench_function("encode_u8_1m", |b| { b.iter(|| { let bytes = fixed_vector_u8.as_ssz_bytes(); black_box(bytes); }); }); - group.bench_function("decode_byte_u8_128k", |b| { + group.bench_function("decode_u64_128k", |b| { + b.iter(|| { + let vector = FixedVector::::from_ssz_bytes(&fixed_vector_bytes).unwrap(); + black_box(vector); + }); + }); + group.bench_function("encode_u64_128k", |b| { + b.iter(|| { + let bytes = fixed_vector_u64.as_ssz_bytes(); + black_box(bytes); + }); + }); + + group.finish(); +} + +fn benchmark_variable_list(c: &mut Criterion) { + let mut group = c.benchmark_group("variable_list"); + + let variable_list_u8 = VariableList::::new(vec![255u8; 1048576]).unwrap(); + let variable_list_u64 = VariableList::::new(vec![255u64; 131072]).unwrap(); + let variable_list_bytes = variable_list_u8.as_ssz_bytes(); + + group.warm_up_time(Duration::from_secs(15)); + group.measurement_time(Duration::from_secs(10)); + + group.bench_function("decode_u8_1m", |b| { b.iter(|| { - let vector = ByteVector::::from_ssz_bytes(&fixed_vector_bytes).unwrap(); + let vector = + VariableList::::from_ssz_bytes(&variable_list_bytes).unwrap(); black_box(vector); }); }); - group.bench_function("decode_u64_16k", |b| { + group.bench_function("encode_u8_1m", |b| { b.iter(|| { - let vector = FixedVector::::from_ssz_bytes(&fixed_vector_bytes).unwrap(); + let bytes = variable_list_u8.as_ssz_bytes(); + black_box(bytes); + }); + }); + + group.bench_function("decode_u64_128k", |b| { + b.iter(|| { + let vector = + VariableList::::from_ssz_bytes(&variable_list_bytes).unwrap(); black_box(vector); }); }); - group.bench_function("encode_u64_16k", |b| { + group.bench_function("encode_u64_128k", |b| { b.iter(|| { - let bytes = fixed_vector_u64.as_ssz_bytes(); + let bytes = variable_list_u64.as_ssz_bytes(); black_box(bytes); }); }); @@ -68,5 +114,5 @@ fn benchmark_fixed_vector_decode(c: &mut Criterion) { group.finish(); } -criterion_group!(benches, benchmark_fixed_vector_decode); +criterion_group!(benches, benchmark_fixed_vector, benchmark_variable_list); criterion_main!(benches); diff --git a/src/fixed_vector.rs b/src/fixed_vector.rs index 88f2e5a..6e52f7a 100644 --- a/src/fixed_vector.rs +++ b/src/fixed_vector.rs @@ -3,6 +3,7 @@ use crate::Error; use serde::Deserialize; use serde_derive::Serialize; use std::marker::PhantomData; +use std::mem; use std::ops::{Deref, DerefMut, Index, IndexMut}; use std::slice::SliceIndex; use tree_hash::Hash256; @@ -275,6 +276,13 @@ impl ssz::TryFromIter for FixedVector { } } +#[inline(always)] +pub fn from_ssz_bytes_u8_only( + bytes: &[u8], +) -> Result, ssz::DecodeError> { + Ok(FixedVector::new(bytes.to_vec()).unwrap()) +} + impl ssz::Decode for FixedVector where T: ssz::Decode, @@ -299,6 +307,25 @@ where len: 0, expected: 1, }) + } else if mem::size_of::() == 1 && mem::align_of::() == 1 { + if bytes.len() != fixed_len { + return Err(ssz::DecodeError::BytesInvalid(format!( + "FixedVector of {} items has {} items", + fixed_len, + bytes.len(), + ))); + } + + // Safety: We've verified T is u8, so Vec is Vec + // and bytes.to_vec() produces Vec + let vec_u8 = bytes.to_vec(); + let vec_t = unsafe { std::mem::transmute::, Vec>(vec_u8) }; + Self::new(vec_t).map_err(|e| { + ssz::DecodeError::BytesInvalid(format!( + "Wrong number of FixedVector elements: {:?}", + e + )) + }) } else if T::is_ssz_fixed_len() { let num_items = bytes .len() diff --git a/src/lib.rs b/src/lib.rs index 66223cb..3f886a0 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -38,7 +38,7 @@ //! ``` #[macro_use] -mod fixed_vector; +pub mod fixed_vector; pub mod serde_utils; mod tree_hash; mod variable_list; diff --git a/src/variable_list.rs b/src/variable_list.rs index 02a9ea9..e85d2aa 100644 --- a/src/variable_list.rs +++ b/src/variable_list.rs @@ -285,6 +285,26 @@ where return Ok(Self::default()); } + if std::mem::size_of::() == 1 && std::mem::align_of::() == 1 { + if bytes.len() > max_len { + return Err(ssz::DecodeError::BytesInvalid(format!( + "VariableList of {} items exceeds maximum of {}", + bytes.len(), + max_len + ))); + } + + // Safety: We've verified T has the same memory layout as u8, so Vec *is* Vec. + let vec_u8 = bytes.to_vec(); + let vec_t = unsafe { std::mem::transmute::, Vec>(vec_u8) }; + return Self::new(vec_t).map_err(|e| { + ssz::DecodeError::BytesInvalid(format!( + "Wrong number of VariableList elements: {:?}", + e + )) + }); + } + if T::is_ssz_fixed_len() { let num_items = bytes .len() @@ -298,13 +318,16 @@ where ))); } - bytes.chunks(T::ssz_fixed_len()).try_fold( - Vec::with_capacity(num_items), - |mut vec, chunk| { - vec.push(T::from_ssz_bytes(chunk)?); - Ok(vec) - }, - ) + let mut vec = Vec::with_capacity(num_items); + for chunk in bytes.chunks_exact(T::ssz_fixed_len()) { + vec.push(T::from_ssz_bytes(chunk)?); + } + Self::new(vec).map_err(|e| { + ssz::DecodeError::BytesInvalid(format!( + "Wrong number of VariableList elements: {:?}", + e + )) + }) } else { ssz::decode_list_of_variable_length_items(bytes, Some(max_len)) }? From 31aee182a186421adb5d7a9061fa78d75a59c0da Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Thu, 4 Sep 2025 16:35:36 +1000 Subject: [PATCH 05/11] Remove path patch --- Cargo.toml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 20d2298..cea5a7b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -31,6 +31,7 @@ tree_hash_derive = "0.10.0" harness = false name = "encode_decode" +# FIXME: remove before release [patch.crates-io] -ethereum_ssz = { path = "../ethereum_ssz/ssz" } -ethereum_ssz_derive = { path = "../ethereum_ssz/ssz_derive" } +ethereum_ssz = { git = "https://github.com/sigp/ethereum_ssz", branch = "main" } +ethereum_ssz_derive = { git = "https://github.com/sigp/ethereum_ssz", branch = "main" } From b042216ff30e5202f46e59c0c2c27c33dfccf081 Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Thu, 4 Sep 2025 16:44:13 +1000 Subject: [PATCH 06/11] Remove unnecessary pub --- src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index 3f886a0..66223cb 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -38,7 +38,7 @@ //! ``` #[macro_use] -pub mod fixed_vector; +mod fixed_vector; pub mod serde_utils; mod tree_hash; mod variable_list; From 37e9308b9c15a2800dacdf45d3554694ee386ae5 Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Mon, 8 Sep 2025 10:26:56 +1000 Subject: [PATCH 07/11] Fix trailing bytes bug and add more tests --- src/fixed_vector.rs | 19 +++++++++++++++++-- src/lib.rs | 4 ++-- src/variable_list.rs | 29 +++++++++++++++++++++++++---- 3 files changed, 44 insertions(+), 8 deletions(-) diff --git a/src/fixed_vector.rs b/src/fixed_vector.rs index 6e52f7a..4ad1251 100644 --- a/src/fixed_vector.rs +++ b/src/fixed_vector.rs @@ -335,11 +335,12 @@ where if num_items != fixed_len { return Err(ssz::DecodeError::BytesInvalid(format!( "FixedVector of {} items has {} items", - num_items, fixed_len + fixed_len, num_items ))); } - if bytes.len() != num_items * T::ssz_fixed_len() { + // Check that we have a whole number of items and that it is safe to use chunks_exact + if bytes.len() % T::ssz_fixed_len() != 0 { return Err(ssz::DecodeError::BytesInvalid(format!( "FixedVector of {} items has {} bytes", num_items, @@ -506,6 +507,20 @@ mod test { ssz_round_trip::>(vec![0; 8].try_into().unwrap()); } + // Test byte decoding (we have a specialised code path with unsafe code that NEEDS coverage). + #[test] + fn ssz_round_trip_u8_len_1024() { + ssz_round_trip::>(vec![42; 1024].try_into().unwrap()); + ssz_round_trip::>(vec![0; 1024].try_into().unwrap()); + } + + // Decoding an input with invalid trailing bytes MUST fail. + #[test] + fn ssz_bytes_u64_trailing() { + let bytes = [1, 0, 0, 0, 2, 0, 0, 0, 1]; + FixedVector::::from_ssz_bytes(&bytes).unwrap_err(); + } + #[test] fn tree_hash_u8() { let fixed: FixedVector = FixedVector::try_from(vec![]).unwrap(); diff --git a/src/lib.rs b/src/lib.rs index 66223cb..fdff006 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -24,8 +24,8 @@ //! } //! //! let mut example = Example { -//! bit_vector: Bitfield::new(), -//! bit_list: Bitfield::with_capacity(4).unwrap(), +//! bit_vector: BitVector::new(), +//! bit_list: BitList::with_capacity(4).unwrap(), //! variable_list: VariableList::try_from(vec![0, 1]).unwrap(), //! fixed_vector: FixedVector::try_from(vec![2, 3, 4, 5, 6, 7, 8, 9]).unwrap(), //! }; diff --git a/src/variable_list.rs b/src/variable_list.rs index e85d2aa..69ffd49 100644 --- a/src/variable_list.rs +++ b/src/variable_list.rs @@ -318,6 +318,15 @@ where ))); } + // Check that we have a whole number of items and that it is safe to use chunks_exact + if bytes.len() % T::ssz_fixed_len() != 0 { + return Err(ssz::DecodeError::BytesInvalid(format!( + "VariableList of {} items has {} bytes", + num_items, + bytes.len() + ))); + } + let mut vec = Vec::with_capacity(num_items); for chunk in bytes.chunks_exact(T::ssz_fixed_len()) { vec.push(T::from_ssz_bytes(chunk)?); @@ -451,7 +460,7 @@ mod test { assert_eq!( as Encode>::ssz_fixed_len(), 4); } - fn round_trip(item: T) { + fn ssz_round_trip(item: T) { let encoded = &item.as_ssz_bytes(); assert_eq!(item.ssz_bytes_len(), encoded.len()); assert_eq!(T::from_ssz_bytes(encoded), Ok(item)); @@ -459,9 +468,15 @@ mod test { #[test] fn u16_len_8() { - round_trip::>(vec![42; 8].try_into().unwrap()); - round_trip::>(vec![0; 8].try_into().unwrap()); - round_trip::>(vec![].try_into().unwrap()); + ssz_round_trip::>(vec![42; 8].try_into().unwrap()); + ssz_round_trip::>(vec![0; 8].try_into().unwrap()); + ssz_round_trip::>(vec![].try_into().unwrap()); + } + + #[test] + fn ssz_round_trip_u8_len_1024() { + ssz_round_trip::>(vec![42; 1024].try_into().unwrap()); + ssz_round_trip::>(vec![0; 1024].try_into().unwrap()); } #[test] @@ -472,6 +487,12 @@ mod test { assert_eq!(VariableList::from_ssz_bytes(&[]).unwrap(), empty_list); } + #[test] + fn ssz_bytes_u64_trailing() { + let bytes = [1, 0, 0, 0, 2, 0]; + VariableList::::from_ssz_bytes(&bytes).unwrap_err(); + } + fn root_with_length(bytes: &[u8], len: usize) -> Hash256 { let root = merkle_root(bytes, 0); tree_hash::mix_in_length(&root, len) From 2eecf7a505d3ec30a7c8871080436bd4d7f9fba0 Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Mon, 8 Sep 2025 12:33:29 +1000 Subject: [PATCH 08/11] Remove junk --- src/fixed_vector.rs | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/fixed_vector.rs b/src/fixed_vector.rs index 4ad1251..d1bb7ab 100644 --- a/src/fixed_vector.rs +++ b/src/fixed_vector.rs @@ -276,13 +276,6 @@ impl ssz::TryFromIter for FixedVector { } } -#[inline(always)] -pub fn from_ssz_bytes_u8_only( - bytes: &[u8], -) -> Result, ssz::DecodeError> { - Ok(FixedVector::new(bytes.to_vec()).unwrap()) -} - impl ssz::Decode for FixedVector where T: ssz::Decode, From 6bca3893d05daa08fbd1ce4921b23633fcf81487 Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Mon, 8 Sep 2025 12:53:02 +1000 Subject: [PATCH 09/11] More tests for oversize FixedVector --- src/fixed_vector.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/fixed_vector.rs b/src/fixed_vector.rs index c31e13d..ba65e8f 100644 --- a/src/fixed_vector.rs +++ b/src/fixed_vector.rs @@ -510,6 +510,16 @@ mod test { ssz_round_trip::>(vec![0; 1024].try_into().unwrap()); } + #[test] + fn ssz_u8_len_1024_too_long() { + FixedVector::::new(vec![42; 1025]).unwrap_err(); + } + + #[test] + fn ssz_u64_len_1024_too_long() { + FixedVector::::new(vec![42; 1025]).unwrap_err(); + } + // Decoding an input with invalid trailing bytes MUST fail. #[test] fn ssz_bytes_u64_trailing() { From 7f2505b36209729ec9056b61300980c4571fa5b0 Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Mon, 8 Sep 2025 13:04:35 +1000 Subject: [PATCH 10/11] Remove temp ByteVector from benches --- benches/encode_decode.rs | 31 ++----------------------------- 1 file changed, 2 insertions(+), 29 deletions(-) diff --git a/benches/encode_decode.rs b/benches/encode_decode.rs index 4981b6a..6371478 100644 --- a/benches/encode_decode.rs +++ b/benches/encode_decode.rs @@ -1,29 +1,9 @@ use criterion::{criterion_group, criterion_main, Criterion}; -use ssz::{Decode, DecodeError, Encode}; +use ssz::{Decode, Encode}; use ssz_types::{FixedVector, VariableList}; use std::hint::black_box; use std::time::Duration; -use typenum::{Unsigned, U1048576, U131072}; - -#[derive(Clone, Debug, Default, PartialEq, Eq, ssz_derive::Encode)] -#[ssz(struct_behaviour = "transparent")] -pub struct ByteVector(FixedVector); - -impl ssz::Decode for ByteVector { - fn is_ssz_fixed_len() -> bool { - true - } - - fn from_ssz_bytes(bytes: &[u8]) -> Result { - FixedVector::new(bytes.to_vec()) - .map(Self) - .map_err(|e| DecodeError::BytesInvalid(format!("{e:?}"))) - } - - fn ssz_fixed_len() -> usize { - as ssz::Decode>::ssz_fixed_len() - } -} +use typenum::{U1048576, U131072}; fn benchmark_fixed_vector(c: &mut Criterion) { let mut group = c.benchmark_group("fixed_vector"); @@ -35,13 +15,6 @@ fn benchmark_fixed_vector(c: &mut Criterion) { group.warm_up_time(Duration::from_secs(15)); group.measurement_time(Duration::from_secs(10)); - group.bench_function("decode_byte_u8_1m", |b| { - b.iter(|| { - let vector = ByteVector::::from_ssz_bytes(&fixed_vector_bytes).unwrap(); - black_box(vector); - }); - }); - group.bench_function("decode_u8_1m", |b| { b.iter(|| { let vector = FixedVector::::from_ssz_bytes(&fixed_vector_bytes).unwrap(); From da45efae3d19968d0d8dcd18bb544a9e25fbd28d Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Mon, 8 Sep 2025 13:19:04 +1000 Subject: [PATCH 11/11] Fix tests --- src/fixed_vector.rs | 15 ++++++++++++--- src/variable_list.rs | 33 +++++++++++++++++++++++++++++++-- 2 files changed, 43 insertions(+), 5 deletions(-) diff --git a/src/fixed_vector.rs b/src/fixed_vector.rs index ba65e8f..4594541 100644 --- a/src/fixed_vector.rs +++ b/src/fixed_vector.rs @@ -512,19 +512,28 @@ mod test { #[test] fn ssz_u8_len_1024_too_long() { - FixedVector::::new(vec![42; 1025]).unwrap_err(); + assert_eq!( + FixedVector::::from_ssz_bytes(&vec![42; 1025]).unwrap_err(), + ssz::DecodeError::BytesInvalid("FixedVector of 1024 items has 1025 items".into()) + ); } #[test] fn ssz_u64_len_1024_too_long() { - FixedVector::::new(vec![42; 1025]).unwrap_err(); + assert_eq!( + FixedVector::::from_ssz_bytes(&vec![42; 8 * 1025]).unwrap_err(), + ssz::DecodeError::BytesInvalid("FixedVector of 1024 items has 1025 items".into()) + ); } // Decoding an input with invalid trailing bytes MUST fail. #[test] fn ssz_bytes_u64_trailing() { let bytes = [1, 0, 0, 0, 2, 0, 0, 0, 1]; - FixedVector::::from_ssz_bytes(&bytes).unwrap_err(); + assert_eq!( + FixedVector::::from_ssz_bytes(&bytes).unwrap_err(), + ssz::DecodeError::BytesInvalid("FixedVector of 2 items has 9 bytes".into()) + ); } #[test] diff --git a/src/variable_list.rs b/src/variable_list.rs index fa2a126..6c5241e 100644 --- a/src/variable_list.rs +++ b/src/variable_list.rs @@ -482,6 +482,26 @@ mod test { ssz_round_trip::>(vec![0; 1024].try_into().unwrap()); } + #[test] + fn ssz_u8_len_1024_too_long() { + assert_eq!( + VariableList::::from_ssz_bytes(&vec![42; 1025]).unwrap_err(), + ssz::DecodeError::BytesInvalid( + "VariableList of 1025 items exceeds maximum of 1024".into() + ) + ); + } + + #[test] + fn ssz_u64_len_1024_too_long() { + assert_eq!( + VariableList::::from_ssz_bytes(&vec![42; 8 * 1025]).unwrap_err(), + ssz::DecodeError::BytesInvalid( + "VariableList of 1025 items exceeds maximum of 1024".into() + ) + ); + } + #[test] fn ssz_empty_list() { let empty_list = VariableList::::default(); @@ -491,9 +511,18 @@ mod test { } #[test] - fn ssz_bytes_u64_trailing() { + fn ssz_bytes_u32_trailing() { let bytes = [1, 0, 0, 0, 2, 0]; - VariableList::::from_ssz_bytes(&bytes).unwrap_err(); + assert_eq!( + VariableList::::from_ssz_bytes(&bytes).unwrap_err(), + ssz::DecodeError::BytesInvalid("VariableList of 1 items has 6 bytes".into()) + ); + + let bytes = [1, 0, 0, 0, 2, 0, 0, 0, 3]; + assert_eq!( + VariableList::::from_ssz_bytes(&bytes).unwrap_err(), + ssz::DecodeError::BytesInvalid("VariableList of 2 items has 9 bytes".into()) + ); } fn root_with_length(bytes: &[u8], len: usize) -> Hash256 {