Browse Source

Merge pull request #2152 from iorveth/redundant_storage_logic

Storage: remove known_content_ids structure & related logic
Mokhtar Naamani 4 years ago
parent
commit
d72c3b7da1

+ 6 - 11
node/src/chain_spec/content_config.rs

@@ -73,7 +73,7 @@ struct EncodedContentData {
     /// hex encoded GlobalQuota
     global_quota: String,
     /// hex encoded UploadingBlocked flag
-    uploading_blocked: String
+    uploading_blocked: String,
 }
 
 fn parse_content_data(data_file: &Path) -> EncodedContentData {
@@ -110,8 +110,9 @@ impl EncodedContentData {
                 Decode::decode(&mut encoded_global_quota.as_slice()).unwrap()
             },
             uploading_blocked: {
-                let encoded_uploading_blocked = hex::decode(&self.uploading_blocked[2..].as_bytes())
-                .expect("failed to parse data_object hex string");
+                let encoded_uploading_blocked =
+                    hex::decode(&self.uploading_blocked[2..].as_bytes())
+                        .expect("failed to parse data_object hex string");
 
                 Decode::decode(&mut encoded_uploading_blocked.as_slice()).unwrap()
             },
@@ -123,12 +124,11 @@ impl EncodedContentData {
 pub fn empty_data_directory_config() -> DataDirectoryConfig {
     DataDirectoryConfig {
         data_object_by_content_id: vec![],
-        known_content_ids: vec![],
         quotas: vec![],
         quota_size_limit_upper_bound: 20000,
         quota_objects_limit_upper_bound: 200,
         global_quota: Quota::new(2000000, 2000),
-        uploading_blocked: false
+        uploading_blocked: false,
     }
 }
 
@@ -149,14 +149,9 @@ pub fn data_directory_config_from_json(data_file: &Path) -> DataDirectoryConfig
             .iter()
             .map(|object| (object.storage_object_owner.clone(), object.quota))
             .collect(),
-        known_content_ids: content
-            .data_objects
-            .into_iter()
-            .map(|object| object.content_id)
-            .collect(),
         quota_size_limit_upper_bound: content.quota_size_limit_upper_bound,
         quota_objects_limit_upper_bound: content.quota_objects_limit_upper_bound,
         global_quota: content.global_quota,
-        uploading_blocked: content.uploading_blocked
+        uploading_blocked: content.uploading_blocked,
     }
 }

+ 0 - 51
runtime-modules/storage/src/data_directory.rs

@@ -62,8 +62,6 @@ pub trait Trait:
     /// Validates member id and origin combination.
     type MemberOriginValidator: ActorOriginValidator<Self::Origin, MemberId<Self>, Self::AccountId>;
 
-    type MaxObjectsPerInjection: Get<u32>;
-
     /// Default content quota for all actors.
     type DefaultQuota: Get<Quota>;
 }
@@ -244,8 +242,6 @@ pub type DataObjectsMap<T> = BTreeMap<ContentId<T>, DataObject<T>>;
 
 decl_storage! {
     trait Store for Module<T: Trait> as DataDirectory {
-        /// List of ids known to the system.
-        pub KnownContentIds get(fn known_content_ids) config(): Vec<ContentId<T>> = Vec::new();
 
         /// Maps data objects by their content id.
         pub DataObjectByContentId get(fn data_object_by_content_id) config():
@@ -325,9 +321,6 @@ decl_module! {
         /// Predefined errors.
         type Error = Error<T>;
 
-        /// Maximum objects allowed per inject_data_objects() transaction
-        const MaxObjectsPerInjection: u32 = T::MaxObjectsPerInjection::get();
-
         /// Adds the content to the system. The created DataObject
         /// awaits liaison to accept or reject it.
         #[weight = 10_000_000] // TODO: adjust weight
@@ -433,8 +426,6 @@ decl_module! {
 
             Self::update_content_judgement(&storage_provider_id, content_id, LiaisonJudgement::Accepted)?;
 
-            <KnownContentIds<T>>::mutate(|ids| ids.push(content_id));
-
             Self::deposit_event(RawEvent::ContentAccepted(content_id, storage_provider_id));
         }
 
@@ -464,48 +455,6 @@ decl_module! {
             <UploadingBlocked>::put(is_blocked);
             Self::deposit_event(RawEvent::ContentUploadingStatusUpdated(is_blocked));
         }
-
-        // Sudo methods
-
-        /// Removes the content id from the list of known content ids. Requires root privileges.
-        #[weight = 10_000_000] // TODO: adjust weight
-        fn remove_known_content_id(origin, content_id: T::ContentId) {
-            ensure_root(origin)?;
-
-            // == MUTATION SAFE ==
-
-            let upd_content_ids: Vec<T::ContentId> = Self::known_content_ids()
-                .into_iter()
-                .filter(|&id| id != content_id)
-                .collect();
-            <KnownContentIds<T>>::put(upd_content_ids);
-        }
-
-        /// Injects a set of data objects and their corresponding content id into the directory.
-        /// The operation is "silent" - no events will be emitted as objects are added.
-        /// The number of objects that can be added per call is limited to prevent the dispatch
-        /// from causing the block production to fail if it takes too much time to process.
-        /// Existing data objects will be overwritten.
-        #[weight = 10_000_000] // TODO: adjust weight
-        pub(crate) fn inject_data_objects(origin, objects: DataObjectsMap<T>) {
-            ensure_root(origin)?;
-
-            // Must provide something to inject
-            ensure!(objects.len() <= T::MaxObjectsPerInjection::get() as usize, Error::<T>::DataObjectsInjectionExceededLimit);
-
-            for (id, object) in objects.into_iter() {
-                // append to known content ids
-                // duplicates will be removed at the end
-                <KnownContentIds<T>>::mutate(|ids| ids.push(id));
-                <DataObjectByContentId<T>>::insert(id, object);
-            }
-
-            // remove duplicate ids
-            <KnownContentIds<T>>::mutate(|ids| {
-                ids.sort();
-                ids.dedup();
-            });
-        }
     }
 }
 

+ 0 - 137
runtime-modules/storage/src/tests/data_directory.rs

@@ -2,7 +2,6 @@
 
 use common::storage::StorageObjectOwner;
 use frame_support::dispatch::DispatchError;
-use sp_std::collections::btree_map::BTreeMap;
 use system::RawOrigin;
 
 use super::mock::*;
@@ -205,139 +204,3 @@ fn reject_content_as_liaison() {
         assert_eq!(res, Ok(()));
     });
 }
-
-#[test]
-fn data_object_injection_works() {
-    with_default_mock_builder(|| {
-        // No objects in directory before injection
-        assert_eq!(TestDataDirectory::known_content_ids(), vec![]);
-
-        // new objects to inject into the directory
-        let mut objects = BTreeMap::new();
-
-        let object = data_directory::DataObjectInternal {
-            type_id: 1,
-            size: 1234,
-            added_at: data_directory::BlockAndTime {
-                block: 10,
-                time: 1024,
-            },
-            owner: StorageObjectOwner::Member(1),
-            liaison: TEST_MOCK_LIAISON_STORAGE_PROVIDER_ID,
-            liaison_judgement: data_directory::LiaisonJudgement::Pending,
-            ipfs_content_id: vec![],
-        };
-
-        let content_id_1 = 1;
-        objects.insert(content_id_1, object.clone());
-
-        let content_id_2 = 2;
-        objects.insert(content_id_2, object.clone());
-
-        let res = TestDataDirectory::inject_data_objects(RawOrigin::Root.into(), objects);
-        assert!(res.is_ok());
-
-        assert_eq!(
-            TestDataDirectory::known_content_ids(),
-            vec![content_id_1, content_id_2]
-        );
-
-        assert_eq!(
-            TestDataDirectory::data_object_by_content_id(content_id_1),
-            Some(object.clone())
-        );
-
-        assert_eq!(
-            TestDataDirectory::data_object_by_content_id(content_id_2),
-            Some(object)
-        );
-    });
-}
-
-#[test]
-fn data_object_injection_overwrites_and_removes_duplicate_ids() {
-    with_default_mock_builder(|| {
-        let sender = 1u64;
-        let owner = StorageObjectOwner::Member(1u64);
-        let content_id_1 = 1;
-        let content_id_2 = 2;
-
-        let content_parameters_first = ContentParameters {
-            content_id: content_id_1,
-            type_id: 1,
-            size: 10,
-            ipfs_content_id: vec![8, 8, 8, 8],
-        };
-
-        let content_parameters_second = ContentParameters {
-            content_id: content_id_2,
-            type_id: 2,
-            size: 20,
-            ipfs_content_id: vec![9, 9, 9, 9],
-        };
-
-        // Start with some existing objects in directory which will be
-        // overwritten
-        let res = TestDataDirectory::add_content(
-            Origin::signed(sender),
-            owner.clone(),
-            vec![content_parameters_first],
-        );
-        assert!(res.is_ok());
-        let res = TestDataDirectory::add_content(
-            Origin::signed(sender),
-            owner,
-            vec![content_parameters_second],
-        );
-        assert!(res.is_ok());
-
-        let mut objects = BTreeMap::new();
-
-        let object1 = data_directory::DataObjectInternal {
-            type_id: 1,
-            size: 6666,
-            added_at: data_directory::BlockAndTime {
-                block: 10,
-                time: 1000,
-            },
-            owner: StorageObjectOwner::Member(5),
-            liaison: TEST_MOCK_LIAISON_STORAGE_PROVIDER_ID,
-            liaison_judgement: data_directory::LiaisonJudgement::Pending,
-            ipfs_content_id: vec![5, 6, 7],
-        };
-
-        let object2 = data_directory::DataObjectInternal {
-            type_id: 1,
-            size: 7777,
-            added_at: data_directory::BlockAndTime {
-                block: 20,
-                time: 2000,
-            },
-            owner: StorageObjectOwner::Member(6),
-            liaison: TEST_MOCK_LIAISON_STORAGE_PROVIDER_ID,
-            liaison_judgement: data_directory::LiaisonJudgement::Pending,
-            ipfs_content_id: vec![5, 6, 7],
-        };
-
-        objects.insert(content_id_1, object1.clone());
-        objects.insert(content_id_2, object2.clone());
-
-        let res = TestDataDirectory::inject_data_objects(RawOrigin::Root.into(), objects);
-        assert!(res.is_ok());
-
-        assert_eq!(
-            TestDataDirectory::known_content_ids(),
-            vec![content_id_1, content_id_2]
-        );
-
-        assert_eq!(
-            TestDataDirectory::data_object_by_content_id(content_id_1),
-            Some(object1.clone())
-        );
-
-        assert_eq!(
-            TestDataDirectory::data_object_by_content_id(content_id_2),
-            Some(object2)
-        );
-    });
-}

+ 0 - 3
runtime-modules/storage/src/tests/mock.rs

@@ -96,7 +96,6 @@ parameter_types! {
     pub const MaximumBlockLength: u32 = 2 * 1024;
     pub const AvailableBlockRatio: Perbill = Perbill::one();
     pub const MinimumPeriod: u64 = 5;
-    pub const MaxObjectsPerInjection: u32 = 5;
     pub const DefaultQuota: Quota = Quota::new(5000, 50);
 }
 
@@ -180,7 +179,6 @@ impl data_directory::Trait for Test {
     type StorageProviderHelper = ();
     type IsActiveDataObjectType = AnyDataObjectTypeIsActive;
     type MemberOriginValidator = ();
-    type MaxObjectsPerInjection = MaxObjectsPerInjection;
     type DefaultQuota = DefaultQuota;
 }
 
@@ -293,7 +291,6 @@ impl ExtBuilder {
             quota_objects_limit_upper_bound: self.quota_objects_limit_upper_bound,
             global_quota: self.global_quota,
             data_object_by_content_id: vec![],
-            known_content_ids: vec![],
             quotas: vec![],
             uploading_blocked: false,
         }

+ 0 - 2
runtime/src/lib.rs

@@ -453,7 +453,6 @@ impl memo::Trait for Runtime {
 }
 
 parameter_types! {
-    pub const MaxObjectsPerInjection: u32 = 100;
     pub const DefaultQuota: Quota = Quota::new(5000, 50);
 }
 
@@ -466,7 +465,6 @@ impl storage::data_directory::Trait for Runtime {
     type StorageProviderHelper = integration::storage::StorageProviderHelper;
     type IsActiveDataObjectType = DataObjectTypeRegistry;
     type MemberOriginValidator = MembershipOriginValidator<Self>;
-    type MaxObjectsPerInjection = MaxObjectsPerInjection;
     type DefaultQuota = DefaultQuota;
 }