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: Bucket level read access #79

Merged
merged 42 commits into from
May 31, 2024
Merged

feat: Bucket level read access #79

merged 42 commits into from
May 31, 2024

Conversation

snowmead
Copy link
Contributor

@snowmead snowmead commented May 28, 2024

This PR adds pallet_nfts (FRAME pallet) and pallet_bucket_nfts (wrapper around the NFTs pallet) to the runtime, fulfilling the need to distribute NFTs/items to users whom a bucket owner wishes to grant read access to.

Bucket NFTs Pallet

The reason for creating a new bucket NFTs pallet, is to provide ergonomic extrinsics which dApps and parachains can call and be certain they have not missed any underlying requirements which StorageHub imposes related to managing NFTs. While the pallet is small, and is mostly a wrapper around the NFTs pallet, it could potentially grow overtime catering to new unforseen usecases.

As of right now, this first iteration ensures the proper metadata is set to the items created within collections corresponding to a bucket. The only metadata required for MSPs to properly respond to user requests, is the read_access_regex. A finite byte field which determines which files a user owning this item has access to. Without this particular field, users would be denied all read requests sent to MSPs.

NFTs Pallet (FRAME)

While we do offer ergonomic extrinsics within bucket NFTs pallet, users and bucket owners can always directly call into the NFTs pallet. While it was first decided to disable the NFTs pallet extrinsics which would force our pallet to implement a lot of redundant logic, it is not insecure nor breaking to have users bypass the bucket NFTs pallet. They would only have to ensure they meet the requirements set by StorageHub (e.g. read_access_regex).

This enables all the features already offered by the pallet and greatly increases the potential use cases of StorageHub.

Notable Features

File System Pallet Extrinsics:

  • create_bucket: Allows users to create private or public buckets, which determines whether to create a corresponding collection in the NFTs pallet.
  • update_bucket_privacy: Enables users to update the privacy settings of their bucket post-creation.
  • create_and_associate_collection_with_bucket: Allow users to create a new collection and associate it with a bucket. This is helpful when you want to overwrite a collection or a set one when there is none present set to the bucket (this can happen when the bucket is private and a user deletes the collection by calling the NFTs pallet directly).

Bucket NFTs Pallet Extrinsics:

  • share_access: Allows bucket owners to share access by setting a read_access_regex on a newly created item in the NFTs pallet, owned by the recipient account.
  • update_read_access: Lets bucket owners update the read_access_regex of any item in the collection. Options include None (no access) and Some(regex).

Bucket ID Generation:

  • Bucket IDs are derived using the derive_bucket_id trait method from ReadProvidersInterface, implemented by the Providers pallet. This method hashes (Hasher - $H$) the concatenation of the owner (AccountId - $A$) and bucket name (Bucket name - $B$):
    $$\text{BucketId} = H(A, B)$$

Other unrelated features

File system pallet now uses provider Ids across the board instead of the base provider account Ids.

snowmead added 21 commits May 16, 2024 19:28
…nfts to runtime and to file system pallet, impl do_create_bucket
Base addition of the BucketNfts pallet
Implements basic share_access extrinsic
ReadProvidersInterface trait fn derive_bucket_id implemented by Providers pallet. Test success share_access extrinsic.
Removed and added redundant checks across do_* methods
@snowmead snowmead force-pushed the feat/create-bucket-with-nfts branch from e59fb3b to c1e2d15 Compare May 28, 2024 19:00
@snowmead snowmead marked this pull request as ready for review May 29, 2024 11:35
@ffarall ffarall self-requested a review May 29, 2024 20:58
support/traits/src/lib.rs Outdated Show resolved Hide resolved
support/traits/src/lib.rs Outdated Show resolved Hide resolved
runtime/src/lib.rs Show resolved Hide resolved
runtime/src/configs/mod.rs Outdated Show resolved Hide resolved
pallets/providers/src/lib.rs Outdated Show resolved Hide resolved
pallets/bucket-nfts/src/utils.rs Outdated Show resolved Hide resolved
pallets/bucket-nfts/src/utils.rs Outdated Show resolved Hide resolved
pallets/bucket-nfts/src/utils.rs Outdated Show resolved Hide resolved
pallets/bucket-nfts/src/lib.rs Show resolved Hide resolved
pallets/bucket-nfts/src/tests.rs Show resolved Hide resolved
@TDemeco TDemeco self-requested a review May 30, 2024 11:23
pallets/bucket-nfts/src/lib.rs Outdated Show resolved Hide resolved
pallets/bucket-nfts/src/utils.rs Outdated Show resolved Hide resolved
pallets/bucket-nfts/src/utils.rs Outdated Show resolved Hide resolved
pallets/file-system/src/lib.rs Outdated Show resolved Hide resolved
@snowmead snowmead requested review from TDemeco and ffarall May 31, 2024 17:54
Copy link
Contributor

@ffarall ffarall left a comment

Choose a reason for hiding this comment

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

Merge from main, solve conflicts and ready to go. Great work!

runtime/src/lib.rs Outdated Show resolved Hide resolved
primitives/traits/src/lib.rs Show resolved Hide resolved
@snowmead snowmead merged commit 3d64f51 into main May 31, 2024
11 checks passed
@snowmead snowmead deleted the feat/create-bucket-with-nfts branch May 31, 2024 23:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants