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

F3Joule · 2024-02-22T14:22:14Z

Release Checklist

Verify spec_version has been incremented since the last release.
Verify pallet and extrinsic ordering has stayed the same. Bump transaction_version if not.
Verify new extrinsics have been correctly whitelisted/blacklisted for BaseCallFilter
( refactor: Adjust fees configuration and refactor social related proxy types #257 )
There are benchmarks for newly created extrinsics.
Verify weights have been updated for any modified runtime logic.
( chore: Rename space-ownership to ownership #259 )
There are tests for newly created logic.
All the tests pass.
Ensure migrations run correctly.

- Refactor other providers traits

Fix runtime

* Add tests for ownership pallet * Add utility function to ownership for benchmarking * Update benchmarks in ownership pallet --------- Co-authored-by: Oleh Mell <[email protected]>

F3Joule · 2024-03-21T13:12:34Z

Extirnsics ordering (metadata comparison) check results:

----------------------------------------------------------------------
Metadata comparison:
Date: Thu 21 Mar 2024 15:11:08 EET
Reference: wss://para.subsocial.network
Target version: subsocial-collator 0.2.2-18df14eead0
Chain: dev
----------------------------------------------------------------------
              [Spec] name: subsocial-parachain
                     spec_version: 41 -> 42
                     transaction_version: 8 -> 9
          [Metadata] version: 14
           [Modules] num: 38
                     [+] modules: Ownership
                     [-] modules: SpaceOwnership
                          [System] idx: 0 (calls: 8, storage: 17)
                 [ParachainSystem] idx: 1 (calls: 4, storage: 22)
                       [Timestamp] idx: 3 (calls: 1, storage: 3)
                        [Balances] idx: 10 (calls: 6, storage: 6)
                        [Treasury] idx: 12 (calls: 5, storage: 5)
               [CollatorSelection] idx: 21 (calls: 5, storage: 6)
                         [Session] idx: 22 (calls: 2, storage: 8)
                         [Vesting] idx: 26 (calls: 5, storage: 3)
                           [Proxy] idx: 27 (calls: 10, storage: 3)
                        [Multisig] idx: 29 (calls: 4, storage: 2)
                       [XcmpQueue] idx: 30 (calls: 9, storage: 11)
                     [PolkadotXcm] idx: 31 (calls: 10, storage: 12)
                        [DmpQueue] idx: 33 (calls: 1, storage: 6)
                         [Domains] idx: 60 (calls: 9, storage: 8)
                          [Energy] idx: 61 (calls: 2, storage: 4)
                       [FreeProxy] idx: 62 (calls: 1, storage: 2)
                  [CreatorStaking] idx: 63 (calls: 14, storage: 13)
                    [EvmAddresses] idx: 64 (calls: 2, storage: 3)
             [ResourceDiscussions] idx: 65 (calls: 2, storage: 2)
                           [Roles] idx: 71 (calls: 8, storage: 6)
                  [AccountFollows] idx: 72 (calls: 3, storage: 4)
                        [Profiles] idx: 73 (calls: 3, storage: 2)
                    [SpaceFollows] idx: 74 (calls: 3, storage: 4)
                          [Spaces] idx: 76 (calls: 4, storage: 4)
                     [PostFollows] idx: 77 (calls: 2, storage: 4)
                           [Posts] idx: 78 (calls: 6, storage: 6)
                       [Reactions] idx: 79 (calls: 6, storage: 5)
                            [Sudo] idx: 255 (calls: 4, storage: 2)

------------------------------ SUMMARY -------------------------------
⚠️ This filter is here to help spotting changes that should be reviewed carefully.
⚠️ It catches only index changes, deletions and value decreases.
## Deletions
 13: [-] modules: SpaceOwnership
46: 13: [-] modules: SpaceOwnership
71: [-] modules: SpaceOwnership

## Index changes
 n/a
## Decreases
 n/a
----------------------------------------------------------------------

siman · 2024-02-23T17:05:06Z

integration-tests/src/utils/space_ownership_utils.rs

Rename to ownership_utils.rs?

All appropriate renames will be considered in a separate pull request.

siman · 2024-02-23T17:22:11Z

pallets/space-ownership/src/lib.rs

-        /// Account is already an owner of a space.
-        AlreadyASpaceOwner,
+        /// Account is already an owner of an entity.
+        AlreadyOwner,
        /// Cannot transfer ownership, because a space is registered as an active creator.
        ActiveCreatorCannotTransferOwnership,


This error is pretty specific to Creator Staking.
But from another side, we are cleaning up creator staking and this should not be relevant anymore.

As the creator staking logic still exists on a chain, we should consider it properly. Once creator-staking is removed/refactored, we will eliminate this error case.

siman · 2024-03-21T13:37:51Z

integration-tests/src/tests/space_ownership.rs

@@ -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,


why renamed?

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.

pallets/space-ownership/tests/src/tests.rs

runtime/src/lib.rs

pallets/posts/src/functions.rs

pallets/space-ownership/src/lib.rs

pallets/domains/src/lib.rs

Co-Authored-By: Alex Siman <[email protected]>

Co-authored-by: Alex Siman <[email protected]>

Co-Authored-By: Alex Siman <[email protected]>

F3Joule · 2024-03-27T14:33:25Z

Try-runtime results:

2024-03-27 16:29:39 initialized state externalities with storage root 0xbd86c261ca929f32d420c60785413d9549c8b14244b6a0c44e20de711f429c46 and state_version V0
2024-03-27 16:29:40 original spec: RuntimeString::Owned("subsocial-parachain")-41, code hash: 90df05114de5eab279cbf617e0a797f55d66ed586ad54806deba4fa144cf3b69
2024-03-27 16:29:40 new spec: RuntimeString::Owned("subsocial-parachain")-42, code hash: ea701508ee019da2283b7b5d3001ba80bf3aa41484dd0c51ba9445c71e9b1019
2024-03-27 16:29:40 Running migration to clean-up the old pallet name SpaceOwnership
2024-03-27 16:29:40 Removed 6 records from the old pallet SpaceOwnership
2024-03-27 16:29:40 ⚠️ ParachainSystem declares internal migrations (which *might* execute). On-chain `StorageVersion(2)` vs current storage version `StorageVersion(2)`
2024-03-27 16:29:40 ⚠️ XcmpQueue declares internal migrations (which *might* execute). On-chain `StorageVersion(3)` vs current storage version `StorageVersion(2)`
2024-03-27 16:29:40 ⚠️ PolkadotXcm declares internal migrations (which *might* execute). On-chain `StorageVersion(0)` vs current storage version `StorageVersion(1)`
2024-03-27 16:29:40 ⚠️ DmpQueue declares internal migrations (which *might* execute). On-chain `StorageVersion(2)` vs current storage version `StorageVersion(1)`
2024-03-27 16:29:41 TryRuntime_on_runtime_upgrade executed without errors. Consumed weight = (500200000000 ps, 5242880 byte), total weight = (500000000000 ps, 5242880 byte) (100.04 %, 100.00 %).

Add ownership transfers for domains

298342a

F3Joule self-assigned this Feb 22, 2024

F3Joule requested review from olehmell and siman February 22, 2024 14:22

F3Joule added the enhancement New feature or request label Feb 22, 2024

F3Joule added 8 commits February 23, 2024 12:55

Rename pallet-space-ownership to pallet-ownership

389f38d

WIP: refactor ownership pallet to work with different entities

cdfb883

Provide interfaces for changing spaces and domains ownership

9410f94

Introduce PostsProvider

fc76af7

- Refactor other providers traits

Remove redundant code from domains pallet

172b49a

Implement transfer ownership logic in ownership pallet

d206aac

Add migration to the ownership pallet

4f1cafb

Fix runtime

Fix mocks

ab8559f

F3Joule marked this pull request as ready for review February 27, 2024 10:53

F3Joule changed the title ~~feat: Add ownership transfers for domains~~ feat!: Refactor ownership pallet to transfer ownership of spaces, posts and domains Feb 27, 2024

F3Joule and others added 9 commits February 27, 2024 14:04

Add migration to the runtime

35993d0

Update spec_version to 41

149efe1

Update transaction_version to 9

a01fb06

Fix code formatting in migration file

beb18a6

chore: Add tests for ownership pallet (#256)

3212edd

* Add tests for ownership pallet * Add utility function to ownership for benchmarking * Update benchmarks in ownership pallet --------- Co-authored-by: Oleh Mell <[email protected]>

Add missing license headers

05587d2

Add migration from old to new ownership pallet

901a75c

Merge branch 'main' into f3/transfer-domains

1acdc6a

Update spec_version to 42

18df14e

siman requested changes Mar 21, 2024

View reviewed changes

F3Joule and others added 3 commits March 22, 2024 13:34

Apply suggestions after a code review

7728026

Co-Authored-By: Alex Siman <[email protected]>

Add missing license headers

056cb68

Apply suggestions from code review

f575b39

Co-authored-by: Alex Siman <[email protected]>

F3Joule requested a review from siman March 27, 2024 11:30

F3Joule and others added 4 commits March 27, 2024 15:17

Apply suggestions from code review

26aa859

Co-Authored-By: Alex Siman <[email protected]>

Change some functions arguments ordering

2cb2a22

Apply final suggestions after code review

6166d7f

Co-Authored-By: Alex Siman <[email protected]>

Update migration for ownership pallet

5460173

Fix migration after review

a740d7e

siman approved these changes Mar 27, 2024

View reviewed changes

olehmell approved these changes Mar 27, 2024

View reviewed changes

F3Joule merged commit ed4da8c into main Mar 27, 2024
1 of 2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

F3Joule commented Feb 22, 2024 •

edited

Loading

F3Joule commented Mar 21, 2024

siman Feb 23, 2024

F3Joule Mar 22, 2024 •

edited

Loading

siman Feb 23, 2024

F3Joule Mar 22, 2024

siman Mar 21, 2024

F3Joule Mar 22, 2024 •

edited

Loading

F3Joule commented Mar 27, 2024

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

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

Conversation

F3Joule commented Feb 22, 2024 • edited Loading

Release Checklist

F3Joule commented Mar 21, 2024

siman Feb 23, 2024

Choose a reason for hiding this comment

F3Joule Mar 22, 2024 • edited Loading

Choose a reason for hiding this comment

siman Feb 23, 2024

Choose a reason for hiding this comment

F3Joule Mar 22, 2024

Choose a reason for hiding this comment

siman Mar 21, 2024

Choose a reason for hiding this comment

F3Joule Mar 22, 2024 • edited Loading

Choose a reason for hiding this comment

F3Joule commented Mar 27, 2024

F3Joule commented Feb 22, 2024 •

edited

Loading

F3Joule Mar 22, 2024 •

edited

Loading

F3Joule Mar 22, 2024 •

edited

Loading