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.

Sign up for GitHub

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

Jump to bottom

NFT Lock Mechanism #76

Merged
merged 21 commits into from
May 19, 2022
Merged

NFT Lock Mechanism #76

merged 21 commits into from
May 19, 2022

Conversation

HashWarlock
Copy link
Contributor

@HashWarlock HashWarlock commented Mar 1, 2022 •
edited
Loading

Targets

  • Temporarily add Uniques pallet
  • Add a dependency injection Trait to Uniques pallet for RMRK Core pallet to implement
  • Implement Lock logic in RMRK Core
  • Add tests to ensure Locks operate correctly
  • Rebase changes with main branch
  • Submit PR for Uniques pallet in Substrate repo to add the dependency injection Trait
  • After PR accepted remove Uniques pallet from RMRK repo
  • Make PR suggested changes
  • Update Cargo to polkadot-v0.9.22 when it is released with Locker Trait

Procedure

  • Temporarily add a local copy of the Uniques pallet to the repo
  • Implement the dependency injection Trait for the Lock and implement a function to default to false
  • Implement the new Lock Trait in RMRK Core and add logic to configure the Lock status as a bool
  • Add error checks and tests to ensure a NFT has the following limitations:
    • no adding children
    • no changing priority
    • no adding resources
    • no equip/unequip
    • no mutating attributes or properties
  • Psuedo code ex: https://gist.github.com/h4x3rotab/152d05c9b5a18462e8dfe2b299b58db8

RMRK Spec

HashWarlock and others added 9 commits March 1, 2022 13:00
@HashWarlock
Add the Uniques pallet temporarily for dev purposes
935ba04
@HashWarlock
Create the dependency injection trait Locker and default check_should…
644475b 
…_lock function to false
@HashWarlock
Implement Locker trait in RMRK core pallet and add lock logic to down…
8a2cb9c 
…stream RMRK market pallet whenever a list, unlist, and do_buy are executed
@HashWarlock
Merge branch 'main' into feat/nft-lock-logic
cc03548
@HashWarlock
Fix merge errors and update cargo.toml file dependencies
ba1f9dd
@HashWarlock
Add check_should_lock function to do_equip and check for both collect…
6fc947a 
…ion id and nft id of item and equipper. Cargo fmt
@HashWarlock
Add Locker trait to mock for equip pallet
f208429
@HashWarlock
Add a test to directly call Uniques do_transfer function
bf8279f
@HashWarlock
Temporarily use substrate branch with Uniques locker trait implementa…
28f7890 
…tion that branches off stable build polkadot-v0.19.17. Made small changes Cargo.toml files
@HashWarlock HashWarlock mentioned this pull request Mar 13, 2022
Merged
@HashWarlock HashWarlock marked this pull request as ready for review March 13, 2022 22:51
@HashWarlock HashWarlock requested a review from ilionic March 13, 2022 22:51
@HashWarlock
Copy link
Contributor Author

Substrate PR 11025
Cumulus PR 1085

@ilionic ilionic requested a review from bmacer March 17, 2022 03:12
ilionic
ilionic requested changes Mar 18, 2022
View reviewed changes
Copy link
Contributor

@ilionic ilionic left a comment

Choose a reason for hiding this comment

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

Great work, would be good to merge it to avoid conflict. One question before we can merge.

traits/Cargo.toml Outdated
sp-runtime = { default-features = false, version = "5.0.0", git = "https://github.com/paritytech/substrate.git", branch = "polkadot-v0.9.17" }
sp-std = { default-features = false, version = "4.0.0", git = "https://github.com/paritytech/substrate.git", branch = "polkadot-v0.9.17" }
sp-runtime = { default-features = false, version = "5.0.0", git = "https://github.com/HashWarlock/substrate.git", branch = "hw-uniques-locker-trait" }
sp-std = { default-features = false, version = "4.0.0", git = "https://github.com/HashWarlock/substrate.git", branch = "hw-uniques-locker-trait" }
Copy link
Contributor

Choose a reason for hiding this comment

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

can we switch back to paritytech repo yet?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hasn't been merged, yet. We could create a substrate repo for rmrk-team and put the fix there until the code is merged to substrate. I have the 2 PRs here paritytech/substrate#11025
paritytech/cumulus#1085

I'm gonna reach out on the Matrix server to see if we can get another review.

bmacer
bmacer approved these changes Apr 22, 2022
View reviewed changes
Copy link
Contributor

@bmacer bmacer left a comment

Choose a reason for hiding this comment

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

LGTM

@bmacer
Copy link
Contributor

bmacer commented Apr 22, 2022

looks like your parity PRs are slowly making progress. do we want to merge this before the switch back to parity is possible? getting a little scared of the conflicts we'll be creating with getting too deep in multiple branches.

@HashWarlock
Copy link
Contributor Author

looks like your parity PRs are slowly making progress. do we want to merge this before the switch back to parity is possible? getting a little scared of the conflicts we'll be creating with getting too deep in multiple branches.

I will update the packages and rebase once it is put into the release. I don't think the changes will be too hard.

@HashWarlock
Copy link
Contributor Author

Code is merged now. I'll see which release it will be in before updating code.
substrate - paritytech/substrate#11025
cumulus - paritytech/cumulus#1085

@bmacer bmacer mentioned this pull request May 3, 2022
Closed
@HashWarlock
Copy link
Contributor Author

Not currently included in polkadot-v0.9.20 will wait until included in release. Code is rebased. I'll also be upgrading the toolchain whenever I merge.

@HashWarlock HashWarlock mentioned this pull request May 18, 2022
Closed
@HashWarlock HashWarlock requested a review from ilionic May 18, 2022 06:48
@HashWarlock
Copy link
Contributor Author

HashWarlock commented May 18, 2022 •
edited
Loading

@ilionic All checks pass, but hold off on merging for now

@HashWarlock HashWarlock mentioned this pull request May 18, 2022
Closed
@HashWarlock
Copy link
Contributor Author

Officially released. We can merge now

@ilionic ilionic merged commit 8096ad3 into rmrk-team:main May 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bmacer bmacer bmacer approved these changes

@ilionic ilionic ilionic approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@HashWarlock @bmacer @ilionic