fix(node): add index check to sliver PUT endpoint (#2966)

Author: mlegner

.github/workflows/code.yml

@@ -137,7 +137,6 @@ jobs:
       - name: Run all unit tests and doctests with `cargo test`
         run: cargo test
 
-
   simtests:
     name: Run all simtests
     needs: diff

.github/workflows/update-ws-install.yaml

@@ -40,7 +40,7 @@ jobs:
           SUI_KEYSTORE: "${{ secrets.SUI_KEYSTORE }}"
           WALRUS_CONFIG: "${{ vars.WALRUS_CONFIG }}"
           SUI_NETWORK: mainnet
-          SITE_BUILDER_VERSION: v1  # keep uploading this as single blobs instead of quilts for now (SEW-509)
+          SITE_BUILDER_VERSION: v1 # keep uploading this as single blobs instead of quilts for now (SEW-509)
 
       - name: Create temporary directory
         run: "mkdir -p ${{ env.BUILD_DIR }}"

crates/walrus-core/src/lib.rs

@@ -598,6 +598,11 @@ impl Sliver {
         by_axis::flat_map!(self.as_ref(), |x| x.verify(encoding_config, metadata))
     }
 
+    /// Returns the [`SliverIndex`] of this sliver (primary or secondary).
+    pub fn sliver_index(&self) -> SliverIndex {
+        by_axis::flat_map!(self.as_ref(), |x| x.index)
+    }
+
     /// Returns the [`Sliver<T>`][Sliver] contained within the enum.
     pub fn to_raw<T>(self) -> Result<encoding::SliverData<T>, WrongSliverVariantError>
     where

crates/walrus-service/src/node.rs

@@ -4003,6 +4003,20 @@ impl ServiceState for StorageNodeInner {
         intent: UploadIntent,
     ) -> Result<bool, StoreSliverError> {
         self.check_index(sliver_pair_index)?;
+
+        let n_shards = self.n_shards();
+        let expected_pair_index = match sliver.r#type() {
+            SliverType::Primary => sliver.sliver_index().to_pair_index::<Primary>(n_shards),
+            SliverType::Secondary => sliver.sliver_index().to_pair_index::<Secondary>(n_shards),
+        };
+        ensure!(
+            sliver_pair_index == expected_pair_index,
+            StoreSliverError::SliverIndexMismatch {
+                sliver_pair_index,
+                sliver_index: sliver.sliver_index(),
+            }
+        );
+
         let (metadata_persisted, persisted) = self
             .resolve_metadata_for_sliver(&blob_id, intent.is_pending())
             .await?;
@@ -4424,7 +4438,9 @@ fn map_sliver_error_to_metadata(error: StoreSliverError) -> StoreMetadataError {
         StoreSliverError::UnsupportedEncodingType(kind) => {
             StoreMetadataError::UnsupportedEncodingType(kind)
         }
-        StoreSliverError::SliverOutOfRange(_) | StoreSliverError::InvalidSliver(_) => {
+        StoreSliverError::SliverOutOfRange(_)
+        | StoreSliverError::InvalidSliver(_)
+        | StoreSliverError::SliverIndexMismatch { .. } => {
             StoreMetadataError::Internal(anyhow!("sliver cache flush failed: {error:?}"))
         }
         StoreSliverError::ShardNotAssigned(inner) => StoreMetadataError::Internal(inner.into()),
@@ -4937,6 +4953,77 @@ mod tests {
         assert_eq!(stored_status, StoredOnNodeStatus::Stored);
         Ok(())
     }
+
+    #[tokio::test]
+    async fn store_sliver_rejects_inconsistent_sliver_pair_index() -> TestResult {
+        let (cluster, _) = cluster_at_epoch1_without_blobs(&[&[0, 1, 2, 3]], None).await?;
+        let storage_node = cluster.nodes[0].storage_node.clone();
+        let encoding_config = storage_node.as_ref().inner.encoding_config.as_ref().clone();
+        let encoded = EncodedBlob::new(BLOB, encoding_config);
+        let blob_id = *encoded.blob_id();
+        let pair_0 = encoded.assigned_sliver_pair(SHARD_INDEX);
+        let pair_1 = encoded.assigned_sliver_pair(OTHER_SHARD_INDEX);
+
+        assert!(
+            storage_node
+                .as_ref()
+                .store_metadata(
+                    encoded.metadata.clone().into_unverified(),
+                    UploadIntent::Pending
+                )
+                .await?
+        );
+
+        // Primary sliver from pair 0 has sliver index 0, so it must be stored with pair index 0.
+        // Passing pair_1.index() is inconsistent and must be rejected.
+        let err = storage_node
+            .as_ref()
+            .store_sliver(
+                blob_id,
+                pair_1.index(),
+                Sliver::Primary(pair_0.primary.clone()),
+                UploadIntent::Pending,
+            )
+            .await
+            .expect_err("store_sliver must reject inconsistent sliver pair index");
+        assert!(
+            matches!(err, StoreSliverError::SliverIndexMismatch { .. }),
+            "expected SliverIndexMismatch, got {err:?}"
+        );
+
+        // Secondary sliver from pair 0 has sliver index n_shards-1 (for pair index 0).
+        // Passing pair_1.index() is inconsistent and must be rejected.
+        let err = storage_node
+            .as_ref()
+            .store_sliver(
+                blob_id,
+                pair_1.index(),
+                Sliver::Secondary(pair_0.secondary.clone()),
+                UploadIntent::Pending,
+            )
+            .await
+            .expect_err("store_sliver must reject inconsistent sliver pair index for secondary");
+        assert!(
+            matches!(err, StoreSliverError::SliverIndexMismatch { .. }),
+            "expected SliverIndexMismatch, got {err:?}"
+        );
+
+        // Consistent indices must be accepted.
+        for pair in [pair_0, pair_1] {
+            storage_node
+                .as_ref()
+                .store_sliver(
+                    blob_id,
+                    pair.index(),
+                    Sliver::Primary(pair.primary.clone()),
+                    UploadIntent::Pending,
+                )
+                .await?;
+        }
+
+        Ok(())
+    }
+
     async fn check_sliver_status<A: EncodingAxis>(
         storage_node: &StorageNodeHandle,
         pair_index: SliverPairIndex,

crates/walrus-service/src/node/errors.rs

@@ -16,6 +16,8 @@ use walrus_core::{
     Epoch,
     SUPPORTED_ENCODING_TYPES,
     ShardIndex,
+    SliverIndex,
+    SliverPairIndex,
     encoding::SliverVerificationError,
     inconsistency::InconsistencyVerificationError,
     messages::MessageVerificationError,
@@ -310,6 +312,16 @@ pub enum StoreSliverError {
     #[rest_api_error(reason = "INVALID_SLIVER", status = ApiStatusCode::InvalidArgument)]
     InvalidSliver(#[from] SliverVerificationError),
 
+    /// The sliver's index is inconsistent with the requested sliver pair index.
+    #[error(
+        "sliver index {sliver_index} is inconsistent with sliver pair index {sliver_pair_index}"
+    )]
+    #[rest_api_error(reason = "SLIVER_INDEX_MISMATCH", status = ApiStatusCode::InvalidArgument)]
+    SliverIndexMismatch {
+        sliver_pair_index: SliverPairIndex,
+        sliver_index: SliverIndex,
+    },
+
     #[error(transparent)]
     #[rest_api_error(delegate)]
     ShardNotAssigned(#[from] ShardNotAssigned),

crates/walrus-service/storage_openapi.html

@@ -492,7 +492,7 @@ paths:
               schema:
                 $ref: '#/components/schemas/ApiSuccess_String'
         '400':
-          description: May be returned when (1) The blob has not been registered or has already expired. (2) The index identifying the resource is out-of-range for the system. (3) The metadata for the blob is required but missing. (4) The node refused the sliver because the pending upload cache is full. (5) The provided sliver failed verification against the previously uploaded metadata for that blob ID. (6) The shard associated with the operation is not assigned to this storage node. (7) The sliver was too large to fit in the pending cache for the node.
+          description: May be returned when (1) The blob has not been registered or has already expired. (2) The index identifying the resource is out-of-range for the system. (3) The metadata for the blob is required but missing. (4) The node refused the sliver because the pending upload cache is full. (5) The provided sliver failed verification against the previously uploaded metadata for that blob ID. (6) The shard associated with the operation is not assigned to this storage node. (7) The sliver was too large to fit in the pending cache for the node. (8) The sliver's index is inconsistent with the requested sliver pair index.
           content:
             application/json:
               schema: