Skip to content

Commit

Permalink
feat!: Refactor ownership pallet to transfer ownership of spaces, pos…
Browse files Browse the repository at this point in the history
…ts and domains (#255)

* Rename pallet-space-ownership to pallet-ownership

* Provide interfaces for changing spaces and domains ownership

* Introduce PostsProvider

- Refactor other providers traits

* Remove redundant code from domains pallet

* Implement transfer ownership logic in ownership pallet

* Add migration to the ownership pallet

* Add migration to the runtime

* Update spec_version to 41

* Update transaction_version to 9

* chore: Add tests for ownership pallet (#256)

* Add utility function to ownership for benchmarking

* Update benchmarks in ownership pallet

* Add migration from old to new ownership pallet

* Update spec_version to 42

* Add missing license headers

---------

Co-authored-by: Oleh Mell <[email protected]>
Co-authored-by: Alex Siman <[email protected]>
  • Loading branch information
3 people authored Mar 27, 2024
1 parent da9c0b8 commit ed4da8c
Show file tree
Hide file tree
Showing 35 changed files with 1,439 additions and 525 deletions.
92 changes: 51 additions & 41 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 4 additions & 2 deletions integration-tests/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,14 @@ std = [
'pallet-timestamp/std',
'frame-support/std',
'frame-system/std',
'pallet-domains/std',
'pallet-permissions/std',
'pallet-posts/std',
'pallet-profiles/std',
'pallet-reactions/std',
'pallet-roles/std',
'pallet-space-follows/std',
'pallet-space-ownership/std',
'pallet-ownership/std',
'pallet-spaces/std',
'subsocial-support/std',
]
Expand All @@ -47,13 +48,14 @@ sp-runtime = { git = 'https://github.com/paritytech/substrate', branch = 'polkad
sp-std = { git = 'https://github.com/paritytech/substrate', branch = 'polkadot-v0.9.40', default-features = false }

[dev-dependencies]
pallet-domains = { path = '../pallets/domains', default-features = false }
pallet-permissions = { path = '../pallets/permissions', default-features = false }
pallet-posts = { path = '../pallets/posts', default-features = false }
pallet-profiles = { path = '../pallets/profiles', default-features = false }
pallet-reactions = { path = '../pallets/reactions', default-features = false }
pallet-roles = { path = '../pallets/roles', default-features = false }
pallet-space-follows = { path = '../pallets/space-follows', default-features = false }
pallet-space-ownership = { path = '../pallets/space-ownership', default-features = false }
pallet-ownership = { path = '../pallets/space-ownership', default-features = false }
pallet-spaces = { path = '../pallets/spaces', default-features = false }
subsocial-support = { path = '../pallets/support', default-features = false }

Expand Down
36 changes: 33 additions & 3 deletions integration-tests/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,9 @@ frame_support::construct_runtime!(
Reactions: pallet_reactions,
Roles: pallet_roles,
SpaceFollows: pallet_space_follows,
SpaceOwnership: pallet_space_ownership,
SpaceOwnership: pallet_ownership,
Spaces: pallet_spaces,
Domains: pallet_domains,
}
);

Expand Down Expand Up @@ -132,7 +133,7 @@ impl pallet_posts::Config for TestRuntime {
impl pallet_profiles::Config for TestRuntime {
type RuntimeEvent = RuntimeEvent;
type SpacePermissionsProvider = Spaces;
type SpacesInterface = Spaces;
type SpacesProvider = Spaces;
type WeightInfo = ();
}

Expand Down Expand Up @@ -160,10 +161,15 @@ impl pallet_space_follows::Config for TestRuntime {
type WeightInfo = pallet_space_follows::weights::SubstrateWeight<TestRuntime>;
}

impl pallet_space_ownership::Config for TestRuntime {
impl pallet_ownership::Config for TestRuntime {
type RuntimeEvent = RuntimeEvent;
type ProfileManager = Profiles;
type SpacesProvider = Spaces;
type SpacePermissionsProvider = Spaces;
type CreatorStakingProvider = ();
type DomainsProvider = Domains;
type PostsProvider = Posts;
type Currency = Balances;
type WeightInfo = ();
}

Expand All @@ -177,6 +183,30 @@ impl pallet_spaces::Config for TestRuntime {
type WeightInfo = ();
}

parameter_types! {
pub const RegistrationPeriodLimit: BlockNumber = 100;
pub const BaseDomainDeposit: u64 = 10;
pub const OuterValueByteDeposit: u64 = 1;
pub const InitialPaymentBeneficiary: AccountId = ACCOUNT1;
pub InitialPricesConfig: pallet_domains::types::PricesConfigVec<TestRuntime> = vec![(1, 100)];
}

impl pallet_domains::Config for TestRuntime {
type RuntimeEvent = RuntimeEvent;
type Currency = Balances;
type MinDomainLength = ConstU32<1>;
type MaxDomainLength = ConstU32<64>;
type MaxDomainsPerAccount = ConstU32<10>;
type DomainsInsertLimit = ConstU32<10>;
type RegistrationPeriodLimit = RegistrationPeriodLimit;
type MaxOuterValueLength = ConstU32<64>;
type BaseDomainDeposit = BaseDomainDeposit;
type OuterValueByteDeposit = OuterValueByteDeposit;
type InitialPaymentBeneficiary = InitialPaymentBeneficiary;
type InitialPricesConfig = InitialPricesConfig;
type WeightInfo = ();
}

pub(crate) type AccountId = u64;
pub(crate) type BlockNumber = u64;

Expand Down
16 changes: 8 additions & 8 deletions integration-tests/src/tests/space_ownership.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
use frame_support::{assert_ok, assert_noop};
use sp_runtime::traits::Zero;

use pallet_space_ownership::Error as SpaceOwnershipError;
use pallet_ownership::{OwnableEntity, Error as SpaceOwnershipError};
use pallet_spaces::Error as SpacesError;

use crate::mock::*;
Expand All @@ -20,7 +20,7 @@ fn transfer_space_ownership_should_work() {
assert_ok!(_transfer_default_space_ownership()); // Transfer SpaceId 1 owned by ACCOUNT1 to ACCOUNT2

assert_eq!(
SpaceOwnership::pending_space_owner(SPACE1).unwrap(),
SpaceOwnership::pending_ownership_transfer(OwnableEntity::Space(SPACE1)).unwrap(),
ACCOUNT2
);
});
Expand Down Expand Up @@ -70,7 +70,7 @@ fn accept_pending_ownership_should_work() {
assert_eq!(space.owner, ACCOUNT2);

// Check that pending storage is cleared:
assert!(SpaceOwnership::pending_space_owner(SPACE1).is_none());
assert!(SpaceOwnership::pending_ownership_transfer(OwnableEntity::Space(SPACE1)).is_none());

assert!(Balances::reserved_balance(ACCOUNT1).is_zero());

Expand All @@ -96,7 +96,7 @@ fn accept_pending_ownership_should_fail_when_no_pending_transfer_for_space() {
ExtBuilder::build_with_space().execute_with(|| {
assert_noop!(
_accept_default_pending_ownership(),
SpaceOwnershipError::<TestRuntime>::NoPendingTransferOnSpace
SpaceOwnershipError::<TestRuntime>::NoPendingTransfer
);
});
}
Expand All @@ -108,7 +108,7 @@ fn accept_pending_ownership_should_fail_if_origin_is_already_an_owner() {

assert_noop!(
_accept_pending_ownership(Some(RuntimeOrigin::signed(ACCOUNT1)), None),
SpaceOwnershipError::<TestRuntime>::AlreadyASpaceOwner
SpaceOwnershipError::<TestRuntime>::NotAllowedToAcceptOwnershipTransfer,
);
});
}
Expand Down Expand Up @@ -137,7 +137,7 @@ fn reject_pending_ownership_should_work() {
assert_eq!(space.owner, ACCOUNT1);

// Check whether storage state is correct
assert!(SpaceOwnership::pending_space_owner(SPACE1).is_none());
assert!(SpaceOwnership::pending_ownership_transfer(OwnableEntity::Space(SPACE1)).is_none());
});
}

Expand All @@ -153,7 +153,7 @@ fn reject_pending_ownership_should_work_when_proposal_rejected_by_current_space_
assert_eq!(space.owner, ACCOUNT1);

// Check whether storage state is correct
assert!(SpaceOwnership::pending_space_owner(SPACE1).is_none());
assert!(SpaceOwnership::pending_ownership_transfer(OwnableEntity::Space(SPACE1)).is_none());
});
}

Expand All @@ -172,7 +172,7 @@ fn reject_pending_ownership_should_fail_when_no_pending_transfer_on_space() {
ExtBuilder::build_with_space().execute_with(|| {
assert_noop!(
_reject_default_pending_ownership(),
SpaceOwnershipError::<TestRuntime>::NoPendingTransferOnSpace
SpaceOwnershipError::<TestRuntime>::NoPendingTransfer
); // Rejecting a transfer from ACCOUNT2
});
}
Expand Down
9 changes: 5 additions & 4 deletions integration-tests/src/utils/space_ownership_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
// Full license is available at https://github.com/dappforce/subsocial-parachain/blob/main/LICENSE

use frame_support::pallet_prelude::*;
use pallet_ownership::OwnableEntity;

use subsocial_support::SpaceId;

Expand All @@ -20,9 +21,9 @@ pub(crate) fn _transfer_space_ownership(
space_id: Option<SpaceId>,
transfer_to: Option<AccountId>,
) -> DispatchResult {
SpaceOwnership::transfer_space_ownership(
SpaceOwnership::transfer_ownership(
origin.unwrap_or_else(|| RuntimeOrigin::signed(ACCOUNT1)),
space_id.unwrap_or(SPACE1),
space_id.map_or(OwnableEntity::Space(SPACE1), |id| OwnableEntity::Space(id)),
transfer_to.unwrap_or(ACCOUNT2),
)
}
Expand All @@ -34,7 +35,7 @@ pub(crate) fn _accept_default_pending_ownership() -> DispatchResult {
pub(crate) fn _accept_pending_ownership(origin: Option<RuntimeOrigin>, space_id: Option<SpaceId>) -> DispatchResult {
SpaceOwnership::accept_pending_ownership(
origin.unwrap_or_else(|| RuntimeOrigin::signed(ACCOUNT2)),
space_id.unwrap_or(SPACE1),
space_id.map_or(OwnableEntity::Space(SPACE1), |id| OwnableEntity::Space(id)),
)
}

Expand All @@ -49,6 +50,6 @@ pub(crate) fn _reject_default_pending_ownership_by_current_owner() -> DispatchRe
pub(crate) fn _reject_pending_ownership(origin: Option<RuntimeOrigin>, space_id: Option<SpaceId>) -> DispatchResult {
SpaceOwnership::reject_pending_ownership(
origin.unwrap_or_else(|| RuntimeOrigin::signed(ACCOUNT2)),
space_id.unwrap_or(SPACE1),
space_id.map_or(OwnableEntity::Space(SPACE1), |id| OwnableEntity::Space(id)),
)
}
6 changes: 3 additions & 3 deletions pallets/creator-staking/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ pub mod pallet {
use sp_runtime::{traits::Zero, Perbill, Saturating};

use pallet_permissions::SpacePermissionsInfoOf;
use subsocial_support::{traits::{SpacesInterface, SpacePermissionsProvider}, SpaceId};
use subsocial_support::{traits::{SpacesProvider, SpacePermissionsProvider}, SpaceId};

pub use crate::types::*;

Expand All @@ -54,7 +54,7 @@ pub mod pallet {
type Currency: LockableCurrency<Self::AccountId, Moment = Self::BlockNumber>
+ ReservableCurrency<Self::AccountId>;

type SpacesInterface: SpacesInterface<Self::AccountId, SpaceId>;
type SpacesProvider: SpacesProvider<Self::AccountId, SpaceId>;

type SpacePermissionsProvider: SpacePermissionsProvider<
Self::AccountId,
Expand Down Expand Up @@ -341,7 +341,7 @@ pub mod pallet {
Error::<T>::CreatorAlreadyRegistered,
);

let space_owner = T::SpacesInterface::get_space_owner(space_id)?;
let space_owner = T::SpacesProvider::get_space_owner(space_id)?;
T::Currency::reserve(&space_owner, T::CreatorRegistrationDeposit::get())?;

RegisteredCreators::<T>::insert(space_id, CreatorInfo::new(space_owner.clone()));
Expand Down
Loading

0 comments on commit ed4da8c

Please sign in to comment.