storage: any provider can process upload

Cargo.lock

@@ -2053,7 +2053,7 @@ dependencies = [
 
 [[package]]
 name = "joystream-node-runtime"
-version = "7.15.0"
+version = "7.16.0"
 dependencies = [
  "frame-benchmarking",
  "frame-executive",

runtime-modules/storage/src/data_directory.rs

@@ -63,9 +63,6 @@ pub trait Trait:
     /// _Data directory_ event type.
     type Event: From<Event<Self>> + Into<<Self as system::Trait>::Event>;
 
-    /// Provides random storage provider id.
-    type StorageProviderHelper: StorageProviderHelper<Self>;
-
     /// Active data object type validator.
     type IsActiveDataObjectType: data_object_type_registry::IsActiveDataObjectType<Self>;
 
@@ -118,9 +115,6 @@ decl_error! {
         /// Provided owner should be equal o the data object owner under given content id
         OwnersAreNotEqual,
 
-        /// No storage provider available to service the request
-        NoProviderAvailable,
-
         /// New voucher limit being set is less than used.
         VoucherLimitLessThanUsed,
 
@@ -138,9 +132,6 @@ pub enum LiaisonJudgement {
 
     /// Content accepted.
     Accepted,
-
-    /// Content rejected.
-    Rejected,
 }
 
 impl Default for LiaisonJudgement {
@@ -447,8 +438,6 @@ decl_module! {
             let new_owner_voucher = owner_voucher.fill_voucher::<T>(upload_voucher_delta)?;
             let new_global_voucher = Self::global_voucher().fill_voucher::<T>(upload_voucher_delta)?;
 
-            let liaison = T::StorageProviderHelper::get_random_storage_provider()?;
-
             //
             // == MUTATION SAFE ==
             //
@@ -459,7 +448,7 @@ decl_module! {
             // Update global voucher
             <GlobalVoucher>::put(new_global_voucher);
 
-            Self::upload_content(liaison, content.clone(), owner.clone());
+            Self::upload_content(content.clone(), owner.clone());
 
             Self::deposit_event(RawEvent::ContentAdded(content, owner));
         }
@@ -665,27 +654,19 @@ decl_module! {
         ) {
             <StorageWorkingGroup<T>>::ensure_worker_signed(origin, &storage_provider_id)?;
 
-            // == MUTATION SAFE ==
-
-            Self::update_content_judgement(&storage_provider_id, content_id, LiaisonJudgement::Accepted)?;
-
-            Self::deposit_event(RawEvent::ContentAccepted(content_id, storage_provider_id));
-        }
-
-        /// Storage provider rejects a content. Requires signed storage provider account and its id.
-        /// The LiaisonJudgement can be updated, but only by the liaison.
-        #[weight = 10_000_000] // TODO: adjust weight
-        pub(crate) fn reject_content(
-            origin,
-            storage_provider_id: StorageProviderId<T>,
-            content_id: T::ContentId
-        ) {
-            <StorageWorkingGroup<T>>::ensure_worker_signed(origin, &storage_provider_id)?;
+            let mut data = Self::get_data_object(&content_id)?;
 
             // == MUTATION SAFE ==
 
-            Self::update_content_judgement(&storage_provider_id, content_id, LiaisonJudgement::Rejected)?;
-            Self::deposit_event(RawEvent::ContentRejected(content_id, storage_provider_id));
+            if data.liaison_judgement == LiaisonJudgement::Pending {
+                // Set the liaison which is updating the judgement
+                data.liaison = storage_provider_id;
+
+                // Set the judgement
+                data.liaison_judgement = LiaisonJudgement::Accepted;
+                <DataByContentId<T>>::insert(content_id, data);
+                Self::deposit_event(RawEvent::ContentAccepted(content_id, storage_provider_id));
+            }
         }
 
         /// Locks / unlocks content uploading
@@ -830,9 +811,8 @@ impl<T: Trait> Module<T> {
         Ok(())
     }
 
-    // Complete content upload, update vouchers
+    // Complete content upload
     fn upload_content(
-        liaison: StorageProviderId<T>,
         multi_content: Vec<ContentParameters<T::ContentId, DataObjectTypeId<T>>>,
         owner: ObjectOwner<T>,
     ) {
@@ -842,7 +822,8 @@ impl<T: Trait> Module<T> {
                 size: content.size,
                 added_at: common::current_block_time::<T>(),
                 owner: owner.clone(),
-                liaison,
+                // It is irrelevant what liaison is set when judgement is pending
+                liaison: StorageProviderId::<T>::default(),
                 liaison_judgement: LiaisonJudgement::Pending,
                 ipfs_content_id: content.ipfs_content_id,
             };
@@ -867,31 +848,6 @@ impl<T: Trait> Module<T> {
         }
         Ok(())
     }
-
-    fn update_content_judgement(
-        storage_provider_id: &StorageProviderId<T>,
-        content_id: T::ContentId,
-        judgement: LiaisonJudgement,
-    ) -> DispatchResult {
-        let mut data = Self::get_data_object(&content_id)?;
-
-        // Make sure the liaison matches
-        ensure!(
-            data.liaison == *storage_provider_id,
-            Error::<T>::LiaisonRequired
-        );
-
-        data.liaison_judgement = judgement;
-        <DataByContentId<T>>::insert(content_id, data);
-
-        Ok(())
-    }
-}
-
-/// Provides random storage provider id. We use it when assign the content to the storage provider.
-pub trait StorageProviderHelper<T: Trait> {
-    /// Provides random storage provider id.
-    fn get_random_storage_provider() -> Result<StorageProviderId<T>, Error<T>>;
 }
 
 /// Content access helper.
@@ -940,8 +896,6 @@ impl<T: Trait> common::storage::StorageSystem<T> for Module<T> {
         let new_owner_voucher = owner_voucher.fill_voucher::<T>(upload_voucher)?;
         let new_global_voucher = Self::global_voucher().fill_voucher::<T>(upload_voucher)?;
 
-        let liaison = T::StorageProviderHelper::get_random_storage_provider()?;
-
         //
         // == MUTATION SAFE ==
         //
@@ -952,7 +906,7 @@ impl<T: Trait> common::storage::StorageSystem<T> for Module<T> {
         // Update global voucher
         <GlobalVoucher>::put(new_global_voucher);
 
-        Self::upload_content(liaison, content, owner);
+        Self::upload_content(content, owner);
         Ok(())
     }
 
@@ -992,7 +946,6 @@ impl<T: Trait> common::storage::StorageSystem<T> for Module<T> {
     ) -> DispatchResult {
         Self::ensure_uploading_is_not_blocked()?;
 
-        T::StorageProviderHelper::get_random_storage_provider()?;
         let owner_voucher = Self::get_voucher(&owner);
 
         // Ensure owner voucher constraints satisfied.

runtime-modules/storage/src/tests/data_directory.rs

@@ -437,13 +437,6 @@ fn accept_and_reject_content_fail_with_invalid_storage_provider() {
             content_id,
         );
         assert_eq!(res, Err(working_group::Error::<Test, crate::StorageWorkingGroupInstance>::WorkerDoesNotExist.into()));
-
-        let res = TestDataDirectory::reject_content(
-            Origin::signed(storage_provider_account_id),
-            storage_provider_id,
-            content_id,
-        );
-        assert_eq!(res, Err(working_group::Error::<Test, crate::StorageWorkingGroupInstance>::WorkerDoesNotExist.into()));
     });
 }
 
@@ -498,57 +491,6 @@ fn accept_content_as_liaison() {
     });
 }
 
-#[test]
-fn reject_content_as_liaison() {
-    with_default_mock_builder(|| {
-        /*
-           Events are not emitted on block 0.
-           So any dispatchable calls made during genesis block formation will have no events emitted.
-           https://substrate.dev/recipes/2-appetizers/4-events.html
-        */
-        run_to_block(1);
-
-        let sender = 1u64;
-        let owner = StorageObjectOwner::Member(1u64);
-
-        let content_parameters = ContentParameters {
-            content_id: 1,
-            type_id: 1234,
-            size: 0,
-            ipfs_content_id: vec![1, 2, 3, 4],
-        };
-
-        let res =
-            TestDataDirectory::add_content(Origin::signed(sender), owner, vec![content_parameters]);
-        assert!(res.is_ok());
-
-        // An appropriate event should have been fired.
-        let (content_id, creator) = match &System::events().last().unwrap().event {
-            MetaEvent::data_directory(data_directory::RawEvent::ContentAdded(content, creator)) => {
-                (content[0].content_id, creator.clone())
-            }
-            _ => (0u64, StorageObjectOwner::Member(0xdeadbeefu64)), // invalid value, unlikely to match
-        };
-        assert_ne!(creator, StorageObjectOwner::Member(0xdeadbeefu64));
-        assert_eq!(creator, StorageObjectOwner::Member(sender));
-
-        let (storage_provider_account_id, storage_provider_id) = hire_storage_provider();
-
-        // Rejecting content should not work with some random origin
-        let res =
-            TestDataDirectory::reject_content(Origin::signed(55), storage_provider_id, content_id);
-        assert!(res.is_err());
-
-        // However, with the liaison as origin it should.
-        let res = TestDataDirectory::reject_content(
-            Origin::signed(storage_provider_account_id),
-            storage_provider_id,
-            content_id,
-        );
-        assert_eq!(res, Ok(()));
-    });
-}
-
 #[test]
 fn set_global_voucher_limits() {
     with_default_mock_builder(|| {

runtime-modules/storage/src/tests/mock.rs

@@ -208,17 +208,10 @@ impl data_object_type_registry::Trait for Test {
 
 impl data_directory::Trait for Test {
     type Event = MetaEvent;
-    type StorageProviderHelper = ();
     type IsActiveDataObjectType = AnyDataObjectTypeIsActive;
     type MemberOriginValidator = ();
 }
 
-impl crate::data_directory::StorageProviderHelper<Test> for () {
-    fn get_random_storage_provider() -> Result<u32, data_directory::Error<Test>> {
-        Ok(1)
-    }
-}
-
 impl common::origin::ActorOriginValidator<Origin, u64, u64> for () {
     fn ensure_actor_origin(origin: Origin, _account_id: u64) -> Result<u64, &'static str> {
         let signed_account_id = system::ensure_signed(origin)?;

runtime/Cargo.toml

@@ -4,7 +4,7 @@ edition = '2018'
 name = 'joystream-node-runtime'
 # Follow convention: https://github.com/Joystream/substrate-runtime-joystream/issues/1
 # {Authoring}.{Spec}.{Impl} of the RuntimeVersion
-version = '7.15.0'
+version = '7.16.0'
 
 [dependencies]
 # Third-party dependencies

runtime/src/integration/mod.rs

@@ -1,6 +1,5 @@
 pub mod content_directory;
 pub mod forum;
 pub mod proposals;
-pub mod storage;
 pub mod transactions;
 pub mod working_group;

runtime/src/integration/storage.rs

@@ -1,37 +0,0 @@
-use frame_support::traits::Randomness;
-use sp_std::vec::Vec;
-
-use crate::{ActorId, Runtime};
-
-/// Provides random storage provider id. We use it when assign the content to the storage provider.
-pub struct StorageProviderHelper;
-
-impl storage::data_directory::StorageProviderHelper<Runtime> for StorageProviderHelper {
-    fn get_random_storage_provider() -> Result<ActorId, storage::data_directory::Error<Runtime>> {
-        let ids = crate::StorageWorkingGroup::get_all_worker_ids();
-
-        // Filter workers that have set value for their storage value
-        let ids: Vec<ActorId> = ids
-            .into_iter()
-            .filter(|id| !crate::StorageWorkingGroup::worker_storage(id).is_empty())
-            .collect();
-
-        if ids.is_empty() {
-            Err(storage::data_directory::Error::<Runtime>::NoProviderAvailable)
-        } else {
-            let index = Self::random_index(ids.len());
-            Ok(ids[index])
-        }
-    }
-}
-
-impl StorageProviderHelper {
-    fn random_index(upper_bound: usize) -> usize {
-        let seed = crate::RandomnessCollectiveFlip::random_seed();
-        let mut rand: u64 = 0;
-        for offset in 0..8 {
-            rand += (seed.as_ref()[offset] as u64) << offset;
-        }
-        (rand as usize) % upper_bound
-    }
-}

runtime/src/lib.rs

@@ -71,7 +71,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
     spec_name: create_runtime_str!("joystream-node"),
     impl_name: create_runtime_str!("joystream-node"),
     authoring_version: 7,
-    spec_version: 15,
+    spec_version: 16,
     impl_version: 0,
     apis: crate::runtime_api::EXPORTED_RUNTIME_API_VERSIONS,
     transaction_version: 1,
@@ -462,7 +462,6 @@ impl storage::data_object_type_registry::Trait for Runtime {
 
 impl storage::data_directory::Trait for Runtime {
     type Event = Event;
-    type StorageProviderHelper = integration::storage::StorageProviderHelper;
     type IsActiveDataObjectType = DataObjectTypeRegistry;
     type MemberOriginValidator = MembershipOriginValidator<Self>;
 }

runtime/src/tests/mod.rs

@@ -4,7 +4,6 @@
 #[macro_use]
 
 mod proposals_integration;
-mod storage_integration;
 use sp_runtime::BuildStorage;
 
 pub(crate) fn initial_test_ext() -> sp_io::TestExternalities {

runtime/src/tests/storage_integration.rs

@@ -1,40 +0,0 @@
-use super::initial_test_ext;
-use crate::integration::storage::StorageProviderHelper;
-use crate::Runtime;
-
-use frame_support::StorageMap;
-use working_group::{Instance2, Worker};
-
-#[test]
-fn storage_provider_helper_succeeds() {
-    initial_test_ext().execute_with(|| {
-		// Bug in random module requires move the initial block number.
-		<system::Module<Runtime>>::set_block_number(1);
-
-		// Error - no workers.
-		let random_provider_result = <StorageProviderHelper
-			as storage::data_directory::StorageProviderHelper<Runtime>>::get_random_storage_provider();
-		assert!(random_provider_result.is_err());
-
-		let worker_id1 = 1;
-		let worker_id2 = 7;
-		let worker_id3 = 19;
-
-		<working_group::WorkerById<Runtime, Instance2>>::insert(worker_id1, Worker::default());
-		<working_group::WorkerById<Runtime, Instance2>>::insert(worker_id2, Worker::default());
-		<working_group::WorkerById<Runtime, Instance2>>::insert(worker_id3, Worker::default());
-
-		// Still error - endpoints not set in worker storage value.
-		let random_provider_result = <StorageProviderHelper as storage::data_directory::StorageProviderHelper<Runtime>>::get_random_storage_provider();
-		assert!(random_provider_result.is_err());
-
-		<working_group::WorkerStorage<Runtime, Instance2>>::insert(worker_id1, b"http://storage1.net/".to_vec());
-		<working_group::WorkerStorage<Runtime, Instance2>>::insert(worker_id2, b"http://storage2.net/".to_vec());
-		<working_group::WorkerStorage<Runtime, Instance2>>::insert(worker_id3, b"http://storage3.net/".to_vec());
-
-		// Should work now.
-		let worker_ids = vec![worker_id1, worker_id2, worker_id3];
-		let random_provider_id = <StorageProviderHelper as storage::data_directory::StorageProviderHelper<Runtime>>::get_random_storage_provider().unwrap();
-		assert!(worker_ids.contains(&random_provider_id));
-	});
-}