-
Notifications
You must be signed in to change notification settings - Fork 115
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
Content dir 2 #396
Content dir 2 #396
Conversation
Remove entity_id from Entity structure
…substrate-runtime-joystream into content_dir_2_issue_189
…r PropertyValue::Internal
…Map<u16, PropertyValue> where key is index of property in class
… representation, renamed to supported_schemas
…em configurable through runtime trait
…mentation. Begun work on insert_at implementation
…om_group extrinsic
…container instead of BtreeMap
…ropertyValue under in_class_schema_property_id is VecPropertyValue
…reluctant entity_creation_vouchers lookup, wrap EntityCreationVouchers runtime storage value in Option
…ation_voucher_upper_bound, update_entity_creation_voucher extrinsic: fix constraint check
…n_voucher after entity removal performed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See my two comments to prior discussions, others are closed. Good job on trying to show where the change was made, that was very helpful in many cases.
As discussed in the chat, most of the calls to improve the clarity and reduce the usage of mutables did not result in sufficiently large improvements, and the introduction of EntitiesRc
was a further step in this direction. So I think we should do a separate set of PRs to try to correct this step by step. This PR is already way to big.
Will merge this once we have concluded on the two replies I made.
Long-running PR for v2.0 of the content directory
VLOS
https://github.com/Joystream/joystream-landing/issues/175
Drop all events in VLOS
Enforce unique class names. <= Why? perhaps we should even drop them?
Enforce unique property names within each class. <= Why? perhaps we should even drop them?
Rename
PropertyType::Internal
toPropertyType::Reference
, and same forPropertyValue::Internal
, as we are dropping the no'tion of external references.Remove
PropertyType::None
, was added to have some value for required Default trait, but has no semantic content. Instead just return some other value needed, we should not sacrifice our type safety due to a bad Substrate design decision.Remove
Class::id
andEntity::id
, as per convention Fix test for storage module and add new #36 (comment).Make
ClassId
andEntityId
, configurable through runtime.Raw index types u16 are used for index types, replace with aliases for each use case, e.g.
ClassId
,EntityId
,PropertyId
etc. for clarity. Also make publicAdd boolean field in
ClassSchema
which indicates whether a schema is active or not. It is only possible to add schema support to an entity for an active schema. Introduce method for updating this status also.ClassSchema
->Schema
Change values field of Entity to
BTreeMap<u16, PropertyValue>
where key is index of property in class, and killClassPropertyValue
.Change
in_class_schema_indexes
inEntity
toBTreeSet
, and give better name to field, likesupported_schemas
.Make vector properties updatable as vectors substrate-versioned-store-module#13
Entity removal substrate-versioned-store-module#22
Drop from storage these from storage, they need not be tuneable at runtime, just make them configurable through runtime trait.
paritytech/substrate#3263 - is not implemented yet
Remove
InputValidationLengthConstraint
, as it is no longer needed.Introduce a new trait for Module, so that other modules depending on VLOS can consume this trait on runtime, which also allows for mocking.
Proposal: Merge permission and version store #380
Merge permission and version store
Class
andClassPermission
, only have one instance per class, and only one mapEntity
andEntityPermission
, only have one instance per entity, and only one map.Drop
ClassPermission::properties_locked_from_controller
, and instead enrichPropertyType
to include a locking flag for controllers.Drop
ClassPermission::properties_locked_from_maintainers
, and instead enrichPropertyType
to include a locking flag for maintainers.Drop
ClassPermission::referenced_entity_must_have_same_controller
, and instead enrichPropertyType::Reference
to include a same owner boolean flag.VLOS Permissions
https://github.com/Joystream/joystream-landing/issues/178
Security/configuration constraints for:
Closes Proposal: Merge permission and version store #380, Proposal: Efficient entity ownership transfer #379, Content directory: introduce events for all extrinsics #453
Resolves Make vector properties updatable as vectors substrate-versioned-store-module#13, resolves Entity removal substrate-versioned-store-module#22