chore(cluster): limits for vnodes in cluster

bucket/ddc_bucket/admin.rs

@@ -58,7 +58,7 @@ impl DdcBucket {
 
         let mut node = self.nodes.get(node_key)?;
         // allow node ownership transfer only if the current owner is the admin
-        node.only_provider(admin).map_err(|_| NodeOwnerIsNotSuperAdmin)?;
+        node.only_provider(admin).map_err(|_| NodeProviderIsNotSuperAdmin)?;
 
         node.provider_id = new_owner;
         self.nodes.update(node_key, &node)?;

bucket/ddc_bucket/cluster/messages.rs

@@ -148,8 +148,11 @@ impl DdcBucket {
             self.nodes.update(new_node_key, &new_node)?;
         }
 
-        self.topology
-            .replace_node(cluster_id, new_node_key, v_nodes)?;
+        self.topology.replace_node(
+            cluster_id, 
+            new_node_key, 
+            v_nodes
+        )?;
 
         Self::env().emit_event(ClusterNodeReplaced {
             cluster_id,

bucket/ddc_bucket/tests/test_admin.rs

@@ -190,7 +190,7 @@ fn admin_transfer_node_ownership_err_if_provider_is_not_admin() {
             new_node_key,
             new_owner_id
         ),
-        Err(NodeOwnerIsNotSuperAdmin)
+        Err(NodeProviderIsNotSuperAdmin)
     );
 }
 

bucket/ddc_bucket/tests/test_cluster.rs

@@ -341,6 +341,56 @@ fn cluster_add_node_err_if_not_cluster_manager() {
 }
 
 
+#[ink::test]
+fn cluster_add_node_err_if_no_v_nodes() {
+    let mut ctx = setup_cluster();
+
+    let new_node_key = AccountId::from([0xc4, 0xcd, 0xaa, 0xfa, 0xf1, 0x30, 0x7d, 0x23, 0xf4, 0x99, 0x84, 0x71, 0xdf, 0x78, 0x59, 0xce, 0x06, 0x3d, 0xce, 0x78, 0x59, 0xc4, 0x3a, 0xe8, 0xef, 0x12, 0x0a, 0xbc, 0x43, 0xc4, 0x84, 0x31]);
+    set_caller_value(ctx.provider_id0, 10);
+    ctx.contract.node_create(
+        new_node_key,
+        NodeParams::from("new_node"),
+        1000000000,
+        1
+    )?;
+
+    set_caller_value(ctx.manager_id, 10);
+    assert_eq!(
+        ctx.contract.cluster_add_node(
+            ctx.cluster_id, 
+            new_node_key,
+            vec![], 
+        ),
+        Err(AtLeastOneVNodeHasToBeAssigned(ctx.cluster_id, new_node_key))
+    );
+}
+
+
+#[ink::test]
+fn cluster_add_node_err_if_v_nodes_exceeds_limit() {
+    let mut ctx = setup_cluster();
+
+    let new_node_key = AccountId::from([0xc4, 0xcd, 0xaa, 0xfa, 0xf1, 0x30, 0x7d, 0x23, 0xf4, 0x99, 0x84, 0x71, 0xdf, 0x78, 0x59, 0xce, 0x06, 0x3d, 0xce, 0x78, 0x59, 0xc4, 0x3a, 0xe8, 0xef, 0x12, 0x0a, 0xbc, 0x43, 0xc4, 0x84, 0x31]);
+    set_caller_value(ctx.provider_id0, 10);
+    ctx.contract.node_create(
+        new_node_key,
+        NodeParams::from("new_node"),
+        1000000000,
+        1
+    )?;
+
+    set_caller_value(ctx.manager_id, 10);
+    assert_eq!(
+        ctx.contract.cluster_add_node(
+            ctx.cluster_id, 
+            new_node_key,
+            vec![100; 1801], 
+        ),
+        Err(VNodesSizeExceedsLimit)
+    );
+}
+
+
 #[ink::test]
 fn cluster_add_node_ok() {
     let mut ctx = setup_cluster();
@@ -951,6 +1001,68 @@ fn cluster_replace_node_err_if_node_does_not_exist() {
 }
 
 
+#[ink::test]
+fn cluster_replace_node_err_if_no_v_nodes() {
+    let mut ctx = setup_cluster();
+
+    assert_eq!(
+        ctx.contract.cluster_replace_node(
+            ctx.cluster_id, 
+            vec![], 
+            ctx.node_key2
+        ),
+        Err(AtLeastOneVNodeHasToBeAssigned(ctx.cluster_id, ctx.node_key2))
+    );
+}
+
+
+#[ink::test]
+fn cluster_replace_node_err_if_v_nodes_exceeds_limit() {
+    let mut ctx = setup_cluster();
+
+    let new_node_key = AccountId::from([0xc4, 0xcd, 0xaa, 0xfa, 0xf1, 0x30, 0x7d, 0x23, 0xf4, 0x99, 0x84, 0x71, 0xdf, 0x78, 0x59, 0xce, 0x06, 0x3d, 0xce, 0x78, 0x59, 0xc4, 0x3a, 0xe8, 0xef, 0x12, 0x0a, 0xbc, 0x43, 0xc4, 0x84, 0x31]);
+    set_caller_value(ctx.provider_id0, 10);
+    ctx.contract.node_create(
+        new_node_key,
+        NodeParams::from("new_node"),
+        1000000000,
+        1
+    )?;
+
+    set_caller_value(ctx.manager_id, 10);
+    ctx.contract.cluster_add_node(
+        ctx.cluster_id,
+        new_node_key, 
+        vec![100],
+    )?;
+
+    let v_nodes: Vec<VNodeToken> = vec![100; 1801];
+    assert_eq!(
+        ctx.contract.cluster_replace_node(
+            ctx.cluster_id, 
+            v_nodes, 
+            new_node_key
+        ),
+        Err(VNodesSizeExceedsLimit)
+    );
+}
+
+
+#[ink::test]
+fn cluster_replace_node_err_if_old_node_stays_without_v_nodes() {
+    let mut ctx = setup_cluster();
+
+    assert_eq!(
+        ctx.contract.cluster_replace_node(
+            ctx.cluster_id, 
+            vec![1, 2, 3], 
+            ctx.node_key2
+        ),
+        Err(AtLeastOneVNodeHasToBeAssigned(ctx.cluster_id, ctx.node_key0))
+    );
+}
+
+
 #[ink::test]
 fn cluster_replace_node_ok() {
     let mut ctx = setup_cluster();

bucket/ddc_bucket/topology/store.rs

@@ -10,6 +10,10 @@ use crate::ddc_bucket::{Error::*, Result};
 pub type VNodeToken = u64;
 pub type ClusterVNode = (ClusterId, VNodeToken);
 
+// https://use.ink/datastructures/storage-layout#packed-vs-non-packed-layout
+// There is a buffer with only limited capacity (around 16KB in the default configuration) available.
+pub const MAX_V_NODE_IN_VECTOR: usize = 1800;
+
 #[derive(SpreadAllocate, SpreadLayout, Default)]
 #[cfg_attr(feature = "std", derive(ink_storage::traits::StorageLayout, Debug))]
 pub struct TopologyStore {
@@ -32,7 +36,7 @@ impl TopologyStore {
     }
 
     pub fn get_node_by_v_node(&self, cluster_id: ClusterId, v_node: VNodeToken) -> Result<NodeKey> {
-        self.nodes_map.get((cluster_id, v_node)).ok_or(VNodeInNotAssignedToNode(cluster_id, v_node))
+        self.nodes_map.get((cluster_id, v_node)).ok_or(VNodeIsNotAssignedToNode(cluster_id, v_node))
     }
 
     pub fn v_node_has_node(&self, cluster_id: ClusterId, v_node: VNodeToken) -> bool {
@@ -59,8 +63,20 @@ impl TopologyStore {
         v_nodes: Vec<VNodeToken>,
     ) -> Result<()> {
 
+        if v_nodes.is_empty() {
+            return Err(AtLeastOneVNodeHasToBeAssigned(cluster_id, node_key));
+        }
+
+        if v_nodes.len() > MAX_V_NODE_IN_VECTOR {
+            return Err(VNodesSizeExceedsLimit);
+        }
+
         let mut cluster_v_nodes = self.get_v_nodes_by_cluster(cluster_id);
 
+        if cluster_v_nodes.len() + v_nodes.len() > MAX_V_NODE_IN_VECTOR {
+            return Err(VNodesSizeExceedsLimit);
+        }
+
         for v_node in &v_nodes {
 
             // vnode that is being added should not exist in the cluster topology
@@ -97,7 +113,7 @@ impl TopologyStore {
 
             // vnode that is being removed should exist in the cluster topology
             if !self.v_node_has_node(cluster_id, *v_node) {
-                return Err(VNodeInNotAssignedToNode(cluster_id, *v_node));
+                return Err(VNodeIsNotAssignedToNode(cluster_id, *v_node));
             }
 
             // vnode that is being removed should be unusigned from the physical node
@@ -126,6 +142,14 @@ impl TopologyStore {
         v_nodes_to_reasign: Vec<VNodeToken>,
     ) -> Result<()> {
 
+        if v_nodes_to_reasign.is_empty() {
+            return Err(AtLeastOneVNodeHasToBeAssigned(cluster_id, new_node_key));
+        }
+
+        if v_nodes_to_reasign.len() > MAX_V_NODE_IN_VECTOR {
+            return Err(VNodesSizeExceedsLimit);
+        }
+
         let cluster_v_nodes = self.get_v_nodes_by_cluster(cluster_id);
 
         for v_node in &v_nodes_to_reasign {
@@ -145,6 +169,11 @@ impl TopologyStore {
             if let Some(pos) = old_node_v_nodes.iter().position(|x| *x == *v_node) {
                 old_node_v_nodes.remove(pos);
             };
+
+            if old_node_v_nodes.is_empty() {
+                return Err(AtLeastOneVNodeHasToBeAssigned(cluster_id, old_node_key));
+            }
+
             self.v_nodes_map.insert(old_node_key, &old_node_v_nodes);
 
             // vnode that is being reasigned should be assined to the new physical node
@@ -153,6 +182,11 @@ impl TopologyStore {
 
         // vnodes that are being reasigned should be among other vnodes assigned to the new physical node
         let mut new_node_v_nodes = self.get_v_nodes_by_node(new_node_key);
+
+        if new_node_v_nodes.len() + v_nodes_to_reasign.len() > MAX_V_NODE_IN_VECTOR {
+            return Err(VNodesSizeExceedsLimit);
+        }
+
         new_node_v_nodes.extend(v_nodes_to_reasign);
         self.v_nodes_map.insert(new_node_key, &new_node_v_nodes);
 

bucket/lib.rs

@@ -443,6 +443,8 @@ pub mod ddc_bucket {
         /// * `OnlyTrustedClusterManager` error if the caller is not a trusted cluster manager.
         /// * `NodeDoesNotExist` error if the adding Storage node does not exist.
         /// * `NodeIsAddedToCluster(ClusterId)` error if the adding Storage node is already added to this or another cluster.
+        /// * `AtLeastOneVNodeHasToBeAssigned(ClusterId, NodeKey)` error if there is a Storage node without any virtual nodes in the cluster.
+        /// * `VNodesSizeExceedsLimit` error if virtual nodes length exceeds storage capacity.
         #[ink(message, payable)]
         pub fn cluster_add_node(
             &mut self,
@@ -513,13 +515,15 @@ pub mod ddc_bucket {
         /// * `NodeDoesNotExist` error if the new Storage node does not exist.
         /// * `NodeIsNotAddedToCluster(ClusterId)` error if the new Storage node is not added to this cluster.
         /// * `NodeIsAddedToCluster(ClusterId)` error if the new Storage node is in another cluster.
-        /// * `VNodeInNotAssignedToNode(ClusterId, VNodeToken)` error if the there is some virtual node that is being reasigned, but this virtual node is not assigned to any physical node.
+        /// * `VNodeIsNotAssignedToNode(ClusterId, VNodeToken)` error if the there is some virtual node that is being reasigned, but this virtual node is not assigned to any physical node.
         /// * `VNodeIsAlreadyAssignedToNode(NodeKey)` - error if there is some virtual node that is already assigned to other physical node within the same cluster.
+        /// * `AtLeastOneVNodeHasToBeAssigned(ClusterId, NodeKey)` error if there is a Storage node without any virtual nodes in the cluster.
+        /// * `VNodesSizeExceedsLimit` error if virtual nodes length exceeds storage capacity.
         #[ink(message)]
         pub fn cluster_replace_node(
             &mut self,
             cluster_id: ClusterId,
-            v_nodes: Vec<u64>,
+            v_nodes: Vec<VNodeToken>,
             new_node_key: NodeKey,
         ) -> Result<()> {
             self.message_cluster_replace_node(
@@ -1550,7 +1554,7 @@ pub mod ddc_bucket {
         ///
         /// * `OnlySuperAdmin` error if the caller is not the Super-admin.
         /// * `NodeDoesNotExist` error if the Storage node does not exist.
-        /// * `NodeOwnerIsNotSuperAdmin` error if the owner of the targeting node is not the Super-admin.
+        /// * `NodeProviderIsNotSuperAdmin` error if the owner of the targeting node is not the Super-admin.
         #[ink(message)]
         pub fn admin_transfer_node_ownership(
             &mut self, 
@@ -1682,14 +1686,16 @@ pub mod ddc_bucket {
         ClusterIsNotEmpty,
         TopologyIsNotCreated(ClusterId),
         TopologyAlreadyExists,
+        VNodesSizeExceedsLimit,
         NodeIsNotAddedToCluster(ClusterId),
         NodeIsAddedToCluster(ClusterId),
         CdnNodeIsNotAddedToCluster(ClusterId),
         CdnNodeIsAddedToCluster(ClusterId),
         VNodeDoesNotExistsInCluster(ClusterId),
-        VNodeInNotAssignedToNode(ClusterId, VNodeToken),
+        VNodeIsNotAssignedToNode(ClusterId, VNodeToken),
         VNodeIsAlreadyAssignedToNode(NodeKey),
-        NodeOwnerIsNotSuperAdmin,
+        AtLeastOneVNodeHasToBeAssigned(ClusterId, NodeKey),
+        NodeProviderIsNotSuperAdmin,
         CdnNodeOwnerIsNotSuperAdmin,
         BucketDoesNotExist,
         BondingPeriodNotFinished,