fix: allow nonincreasing namespace IDs (#1669)

Closes #<ISSUE_NUMBER>
<!-- These comments should help create a useful PR message, please
delete any remaining comments before opening the PR. -->
<!-- If there is no issue number make sure to describe clearly *why*
this PR is necessary. -->
<!-- Mention open questions, remaining TODOs, if any -->

### This PR:
<!-- Describe what this PR adds to this repo and why -->
<!-- E.g. -->
<!-- * Implements feature 1 -->
<!-- * Fixes bug 3 -->

### This PR does not:
<!-- Describe what is out of scope for this PR, if applicable. Leave
this section blank if it's not applicable -->
<!-- This section helps avoid the reviewer having to needlessly point
out missing parts -->
<!-- * Implement feature 3 because that feature is blocked by Issue 4
-->
<!-- * Implement xyz because that is tracked in issue #123. -->
<!-- * Address xzy for which I opened issue #456 -->

### Key places to review:
<!-- Describe key places for reviewers to pay close attention to -->
<!-- * file.rs, `add_integers` function -->
<!-- Or directly comment on those files/lines to make it easier for the
reviewers -->

<!-- ### How to test this PR:  -->
<!-- Optional, uncomment the above line if this is relevant to your PR
-->
<!-- If your PR is fully tested through CI there is no need to add this
section -->
<!-- * E.g. `just test` -->

<!-- ### Things tested -->
<!-- Anything that was manually tested (that is not tested in CI). -->
<!-- E.g. building/running of docker containers. Changes to docker demo,
... -->
<!-- Especially mention anything untested, with reasoning and link an
issue to resolve this. -->

<!-- Complete the following items before creating this PR -->
<!-- [ ] Issue linked or PR description mentions why this change is
necessary. -->
<!-- [ ] PR description is clear enough for reviewers. -->
<!-- [ ] Documentation for changes (additions) has been updated (added).
-->
<!-- [ ] If this is a draft it is marked as "draft".  -->

<!-- To make changes to this template edit
https://github.com/EspressoSystems/.github/blob/main/PULL_REQUEST_TEMPLATE.md
-->

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
     ///    <https://github.com/EspressoSystems/espresso-sequencer/issues/1604>.)
     /// 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::<NamespaceId>::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,

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<NsTableValidationError>,
+    ) {
         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);
         }
     }
 }