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

Giza. Add storage buckets bag assignments. #3088

Merged

Conversation

shamil-gadelshin
Copy link
Contributor

@shamil-gadelshin shamil-gadelshin commented Jan 24, 2022

We need to fix a bug in the storage pallet: storage buckets deletion doesn't change the bag.

Changes

  • runtime(storage pallet)
  • types
  • chain-spec.json

Storage pallet changes

  • added assigned_bag field to the StorageBucket
  • updated tests
  • modified extrinsics and methods
    • delete_storage_bucket
    • delete_dynamic_bag
    • create_dynamic_bag

@vercel
Copy link

vercel bot commented Jan 24, 2022

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/joystream/pioneer-testnet/EPacZXTgPzab2oFbruWnFUevBqL7
✅ Preview: Canceled

[Deployment for 708bb07 canceled]

@shamil-gadelshin shamil-gadelshin self-assigned this Jan 24, 2022
@shamil-gadelshin shamil-gadelshin added bug Something isn't working giza joystream/types Joystream Polkadotjs Types Library runtime storage-pallet labels Jan 24, 2022
yarn.lock Outdated
@@ -3142,7 +3142,7 @@
lodash "^4.17.15"
moment "^2.24.0"

"@joystream/warthog@2.41.2", "@joystream/warthog@~2.41.2":
"@joystream/warthog@2.41.2", "@joystream/warthog@^2.40.0":
Copy link
Member

Choose a reason for hiding this comment

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

this change doesn't seem correct. When I ran yarn build:packages in your branch it reverted to:
"@joystream/warthog@2.41.2", "@joystream/warthog@~2.41.2":

Copy link
Member

Choose a reason for hiding this comment

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

perhaps you are not using the same version of yarn. (which may be the case if you haven't installed volta)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

$ yarn --version
1.22.15

$ volta --version
1.0.5

@mnaamani
Copy link
Member

Implementation looks correct.
I think we should bump the runtime spec to 14, node to 5.14.0 and chainspec builder to 3.4.0 as usual

Crates:
- storage
- runtime
- node
- chain-spec-builder
Updated the yarn.lock file with command: yarn build:packages
Copy link
Member

@bedeho bedeho left a comment

Choose a reason for hiding this comment

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

Looks good :)

Self::change_bag_assignments(&BTreeSet::new(), &deleted_dynamic_bag.distributed_by);
Self::change_bag_assignments_for_distribution_buckets(
&BTreeSet::new(),
&deleted_dynamic_bag.distributed_by,
Copy link
Member

Choose a reason for hiding this comment

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

Should we, perhaps in a future iteration, add commitment in extrinsic to this length, as in principle this set could be different to what caller thinks, for the purposes of weight function? Same for stored_by, below.

If you agree, please make new issues for this with correct labels, in particular storage-pallet, runtime and mainnet.

@mnaamani mnaamani merged commit 96b2ca6 into Joystream:giza Jan 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working giza joystream/types Joystream Polkadotjs Types Library runtime storage-pallet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants