Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Move LockableCurrency trait to fungible::Lockable and deprecate LockableCurrency #12887

Closed
wants to merge 36 commits into from

Conversation

tonyalaribe
Copy link
Contributor

Priliminary efforts to introduce a Lockable trait based off LockableCurrency.

This moves us closer to the points in paritytech/polkadot-sdk#327

tonyalaribe and others added 28 commits November 28, 2022 19:54
Co-authored-by: Squirrel <gilescope@gmail.com>
Co-authored-by: Squirrel <gilescope@gmail.com>
@github-actions github-actions bot added the A3-in_progress Pull request is in progress. No review needed at this stage. label Dec 9, 2022
@tonyalaribe tonyalaribe marked this pull request as ready for review December 9, 2022 12:45
@github-actions github-actions bot added A0-please_review Pull request needs code review. and removed A3-in_progress Pull request is in progress. No review needed at this stage. labels Dec 9, 2022
@tonyalaribe tonyalaribe changed the title Move LockableCurrency trait to fungibles::Lockable and deprecate LockableCurrency Move LockableCurrency trait to fungible::Lockable and deprecate LockableCurrency Dec 9, 2022
@@ -28,15 +28,15 @@ macro_rules! decl_tests {
use frame_support::{
assert_noop, assert_storage_noop, assert_ok, assert_err,
traits::{
LockableCurrency, LockIdentifier, WithdrawReasons,
fungible, fungible::Lockable, WithdrawReasons,
Copy link
Contributor

Choose a reason for hiding this comment

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

What would happen if you don't include fungible::Lockable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I get this, if fungible::Lockable isn't included:
image

Copy link
Contributor

Choose a reason for hiding this comment

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

ouch, got it :)

@kianenigma
Copy link
Contributor

How does this work link to paritytech/polkadot-sdk#236? has it been discussed/coordinated?

Copy link
Contributor

@melekes melekes left a comment

Choose a reason for hiding this comment

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

👍 (code-wise; don't know enough to reason about business logic)

@@ -20,6 +20,7 @@
//! NOTE: If you're looking for `parameter_types`, it has moved in to the top-level module.

pub mod tokens;
#[allow(deprecated)]
Copy link
Contributor

Choose a reason for hiding this comment

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

do we really need this?

@tonyalaribe
Copy link
Contributor Author

How does this work link to paritytech/polkadot-sdk#236? has it been discussed/coordinated?

It's intended to be a step in that direction moving LockableCurrency to fungibles::* but given the work in #12951, simply renaming LockableCurrency is not a good enough solution. So i'll close this PR for now.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants