Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat!: Refactor ownership pallet to transfer ownership of spaces, posts and domains #255

Merged
merged 26 commits into from
Mar 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
298342a
Add ownership transfers for domains
F3Joule Feb 22, 2024
389f38d
Rename pallet-space-ownership to pallet-ownership
F3Joule Feb 23, 2024
cdfb883
WIP: refactor ownership pallet to work with different entities
F3Joule Feb 23, 2024
9410f94
Provide interfaces for changing spaces and domains ownership
F3Joule Feb 23, 2024
fc76af7
Introduce PostsProvider
F3Joule Feb 27, 2024
172b49a
Remove redundant code from domains pallet
F3Joule Feb 27, 2024
d206aac
Implement transfer ownership logic in ownership pallet
F3Joule Feb 27, 2024
4f1cafb
Add migration to the ownership pallet
F3Joule Feb 27, 2024
ab8559f
Fix mocks
F3Joule Feb 27, 2024
35993d0
Add migration to the runtime
F3Joule Feb 27, 2024
149efe1
Update spec_version to 41
F3Joule Feb 27, 2024
a01fb06
Update transaction_version to 9
F3Joule Feb 27, 2024
beb18a6
Fix code formatting in migration file
F3Joule Feb 27, 2024
3212edd
chore: Add tests for ownership pallet (#256)
F3Joule Mar 14, 2024
05587d2
Add missing license headers
github-actions[bot] Mar 14, 2024
901a75c
Add migration from old to new ownership pallet
F3Joule Mar 14, 2024
1acdc6a
Merge branch 'main' into f3/transfer-domains
F3Joule Mar 20, 2024
18df14e
Update spec_version to 42
F3Joule Mar 20, 2024
7728026
Apply suggestions after a code review
F3Joule Mar 22, 2024
056cb68
Add missing license headers
github-actions[bot] Mar 22, 2024
f575b39
Apply suggestions from code review
F3Joule Mar 22, 2024
26aa859
Apply suggestions from code review
F3Joule Mar 27, 2024
2cb2a22
Change some functions arguments ordering
F3Joule Mar 27, 2024
6166d7f
Apply final suggestions after code review
F3Joule Mar 27, 2024
5460173
Update migration for ownership pallet
F3Joule Mar 27, 2024
a740d7e
Fix migration after review
F3Joule Mar 27, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why renamed?

Copy link
Member Author

@F3Joule F3Joule Mar 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously this error was related to space ownership, from now on we need more generalized error for it.
We can do CurrentOwnerCannotAcceptOwnershipTransfer, then this error will be more specific.

);
});
}
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
Loading