diff --git a/sequencer/src/block/full_payload/ns_table.rs b/sequencer/src/block/full_payload/ns_table.rs index 50ce3489e..d2d2290ef 100644 --- a/sequencer/src/block/full_payload/ns_table.rs +++ b/sequencer/src/block/full_payload/ns_table.rs @@ -19,7 +19,7 @@ use committable::{Commitment, Committable, RawCommitmentBuilder}; use derive_more::Display; use hotshot_types::traits::EncodeBytes; use serde::{de, Deserialize, Deserializer, Serialize, Serializer}; -use std::{ops::Range, sync::Arc}; +use std::{collections::HashSet, ops::Range, sync::Arc}; use thiserror::Error; /// Byte lengths for the different items that could appear in a namespace table. @@ -219,8 +219,8 @@ impl NsTable { /// /// # Checks /// 1. Byte length must hold a whole number of entries. - /// 2. All namespace IDs and offsets must increase monotonically. Offsets - /// must be nonzero. + /// 2. All offsets must increase monotonically. Offsets + /// must be nonzero. Namespace IDs must be unique. /// 3. Header consistent with byte length. (Obsolete after /// .) /// 4. Final offset must equal `payload_byte_len`. (Obsolete after @@ -324,25 +324,24 @@ impl NsTable { return Err(InvalidHeader); } - // Namespace IDs and offsets must increase monotonically. Offsets must - // be nonzero. + // Offsets must increase monotonically. Offsets must + // be nonzero. Namespace IDs must be unique { - let (mut prev_ns_id, mut prev_offset) = (None, 0); + let mut prev_offset = 0; + let mut repeat_ns_ids = HashSet::::new(); for (ns_id, offset) in self.iter().map(|i| { ( self.read_ns_id_unchecked(&i), self.read_ns_offset_unchecked(&i), ) }) { - if let Some(prev_ns_id) = prev_ns_id { - if ns_id <= prev_ns_id { - return Err(NonIncreasingEntries); - } + if !repeat_ns_ids.insert(ns_id) { + return Err(DuplicateNamespaceId); } if offset <= prev_offset { return Err(NonIncreasingEntries); } - (prev_ns_id, prev_offset) = (Some(ns_id), offset); + prev_offset = offset; } } @@ -373,6 +372,7 @@ impl Committable for NsTable { pub enum NsTableValidationError { InvalidByteLen, NonIncreasingEntries, + DuplicateNamespaceId, InvalidHeader, // TODO this variant obsolete after https://github.com/EspressoSystems/espresso-sequencer/issues/1604 InvalidFinalOffset, // TODO this variant obsolete after https://github.com/EspressoSystems/espresso-sequencer/issues/1604 ExpectNonemptyNsTable, diff --git a/sequencer/src/block/full_payload/ns_table/test.rs b/sequencer/src/block/full_payload/ns_table/test.rs index df92d25e1..6f02b74b4 100644 --- a/sequencer/src/block/full_payload/ns_table/test.rs +++ b/sequencer/src/block/full_payload/ns_table/test.rs @@ -131,33 +131,37 @@ fn monotonic_increase() { setup_backtrace(); // Duplicate namespace ID - two_entries_ns_table((5, 5), (5, 6), false); + two_entries_ns_table((5, 5), (5, 6), Some(DuplicateNamespaceId)); // Decreasing namespace ID - two_entries_ns_table((5, 5), (4, 6), false); + two_entries_ns_table((5, 5), (4, 6), None); // Duplicate offset - two_entries_ns_table((5, 5), (6, 5), false); + two_entries_ns_table((5, 5), (6, 5), Some(NonIncreasingEntries)); // Decreasing offset - two_entries_ns_table((5, 5), (6, 4), false); + two_entries_ns_table((5, 5), (6, 4), Some(NonIncreasingEntries)); // Zero namespace ID - two_entries_ns_table((0, 5), (6, 6), true); + two_entries_ns_table((0, 5), (6, 6), None); // Zero offset - two_entries_ns_table((5, 0), (6, 6), false); + two_entries_ns_table((5, 0), (6, 6), Some(NonIncreasingEntries)); // Helper fn: build a 2-entry NsTable, assert failure - fn two_entries_ns_table(entry1: (u32, usize), entry2: (u32, usize), expect_success: bool) { + fn two_entries_ns_table( + entry1: (u32, usize), + entry2: (u32, usize), + expect_err: Option, + ) { let mut ns_table_builder = NsTableBuilder::new(); ns_table_builder.append_entry(NamespaceId::from(entry1.0), entry1.1); ns_table_builder.append_entry(NamespaceId::from(entry2.0), entry2.1); let ns_table = ns_table_builder.into_ns_table(); - if expect_success { - expect_valid(&ns_table); + if let Some(err) = expect_err { + expect_invalid(&ns_table, err); } else { - expect_invalid(&ns_table, NonIncreasingEntries); + expect_valid(&ns_table); } } }