Refactor pallet recovery to fungibles #1807
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -68,10 +68,10 @@ | |
| //! configuration on the recovered account and reclaim the recovery configuration deposit they | ||
| //! placed. | ||
| //! 9. Using `as_recovered`, the account owner is able to call any other pallets to clean up their | ||
| //! state and reclaim any held or locked funds. They can then transfer all funds from the | ||
| //! recovered account to the new account. | ||
| //! 10. When the recovered account becomes reaped (i.e. its free and held balance drops to zero), | ||
| //! the final recovery link is removed. | ||
| //! | ||
| //! ### Malicious Recovery Attempts | ||
| //! | ||
|
|
@@ -158,9 +158,14 @@ use sp_runtime::{ | |
| }; | ||
| use sp_std::prelude::*; | ||
|
|
||
| #[cfg(feature = "runtime-benchmarks")] | ||
| use frame_support::traits::fungible::Mutate; | ||
| use frame_support::{ | ||
| dispatch::{GetDispatchInfo, PostDispatchInfo}, | ||
| traits::{ | ||
| fungible::{Inspect, MutateHold}, | ||
| tokens::{Fortitude, Precision, Restriction}, | ||
| }, | ||
| BoundedVec, | ||
| }; | ||
|
|
||
|
|
@@ -177,7 +182,7 @@ mod tests; | |
| pub mod weights; | ||
|
|
||
| type BalanceOf<T> = | ||
| <<T as Config>::Currency as Inspect<<T as frame_system::Config>::AccountId>>::Balance; | ||
|
|
||
| type FriendsOf<T> = BoundedVec<<T as frame_system::Config>::AccountId, <T as Config>::MaxFriends>; | ||
| type AccountIdLookupOf<T> = <<T as frame_system::Config>::Lookup as StaticLookup>::Source; | ||
|
|
@@ -187,7 +192,7 @@ type AccountIdLookupOf<T> = <<T as frame_system::Config>::Lookup as StaticLookup | |
| pub struct ActiveRecovery<BlockNumber, Balance, Friends> { | ||
| /// The block number when the recovery process started. | ||
| created: BlockNumber, | ||
| /// The amount held the `depositor`, | ||
| /// To be returned once this recovery process is closed. | ||
| deposit: Balance, | ||
| /// The friends which have vouched so far. Always sorted. | ||
|
|
@@ -200,7 +205,7 @@ pub struct RecoveryConfig<BlockNumber, Balance, Friends> { | |
| /// The minimum number of blocks since the start of the recovery process before the account | ||
| /// can be recovered. | ||
| delay_period: BlockNumber, | ||
| /// The amount held of the `depositor`, | ||
| /// to be returned once this configuration is removed. | ||
| deposit: Balance, | ||
| /// The list of friends which can help recover an account. Always sorted. | ||
|
|
@@ -235,9 +240,15 @@ pub mod pallet { | |
| + From<frame_system::Call<Self>>; | ||
|
|
||
| /// The currency mechanism. | ||
| #[cfg(feature = "runtime-benchmarks")] | ||
| type Currency: Mutate<Self::AccountId> | ||
| + MutateHold<Self::AccountId, Reason = Self::RuntimeHoldReason>; | ||
|
Comment on lines
+243
to
+245
There was a problem hiding this comment. We need to There was a problem hiding this comment. It is possible to set custom where bounds for benchmarks, like this. |
||
|
|
||
| /// The currency mechanism. | ||
| #[cfg(not(feature = "runtime-benchmarks"))] | ||
| type Currency: MutateHold<Self::AccountId, Reason = Self::RuntimeHoldReason>; | ||
|
|
||
| /// The base amount of currency needed to hold for creating a recovery configuration. | ||
| /// | ||
| /// This is held for an additional storage item whose value size is | ||
| /// `2 + sizeof(BlockNumber, Balance)` bytes. | ||
|
|
@@ -254,14 +265,14 @@ pub mod pallet { | |
|
|
||
| /// The maximum amount of friends allowed in a recovery configuration. | ||
| /// | ||
| /// NOTE: The threshold programmed in this Pallet uses u32, so it does | ||
| /// not really make sense to have a limit here greater than u32::MAX. | ||
| /// But also, that is a lot more than you should probably set this value | ||
| /// to anyway... | ||
| #[pallet::constant] | ||
| type MaxFriends: Get<u32>; | ||
|
|
||
| /// The base amount of currency needed to hold for starting a recovery. | ||
| /// | ||
| /// This is primarily held for deterring malicious recovery attempts, and should | ||
| /// have a value large enough that a bad actor would choose not to place this | ||
|
|
@@ -270,6 +281,9 @@ pub mod pallet { | |
| /// threshold. | ||
| #[pallet::constant] | ||
| type RecoveryDeposit: Get<BalanceOf<Self>>; | ||
|
|
||
| /// The overarching hold reason. | ||
| type RuntimeHoldReason: From<HoldReason>; | ||
| } | ||
|
|
||
| /// Events type. | ||
|
|
@@ -330,6 +344,16 @@ pub mod pallet { | |
| BadState, | ||
| } | ||
|
|
||
| /// The reasons for the pallet recovery placing holds on funds. | ||
| #[pallet::composite_enum] | ||
| pub enum HoldReason { | ||
| /// The Pallet holds it as the initial Configuration Deposit, when the Recovery | ||
| /// Configuration is created. | ||
| ConfigurationDeposit, | ||
| /// The Pallet holds it as the Deposit for starting a Recovery Process. | ||
| RecoveryProcessDeposit, | ||
| } | ||
|
|
||
| /// The set of recoverable accounts and their recovery configuration. | ||
| #[pallet::storage] | ||
| #[pallet::getter(fn recovery_config)] | ||
|
|
@@ -424,7 +448,7 @@ pub mod pallet { | |
| /// Create a recovery configuration for your account. This makes your account recoverable. | ||
| /// | ||
| /// Payment: `ConfigDepositBase` + `FriendDepositFactor` * #_of_friends balance | ||
| /// will be held for storing the recovery configuration. This deposit is returned | ||
| /// in full when the user calls `remove_recovery`. | ||
| /// | ||
| /// The dispatch origin for this call must be _Signed_. | ||
|
|
@@ -462,8 +486,8 @@ pub mod pallet { | |
| let total_deposit = T::ConfigDepositBase::get() | ||
| .checked_add(&friend_deposit) | ||
| .ok_or(ArithmeticError::Overflow)?; | ||
| // Hold the deposit | ||
| T::Currency::hold(&HoldReason::ConfigurationDeposit.into(), &who, total_deposit)?; | ||
| // Create the recovery configuration | ||
| let recovery_config = RecoveryConfig { | ||
| delay_period, | ||
|
|
@@ -480,7 +504,7 @@ pub mod pallet { | |
|
|
||
| /// Initiate the process for recovering a recoverable account. | ||
| /// | ||
| /// Payment: `RecoveryDeposit` balance will be held for initiating the | ||
| /// recovery process. This deposit will always be repatriated to the account | ||
| /// trying to be recovered. See `close_recovery`. | ||
| /// | ||
|
|
@@ -506,7 +530,7 @@ pub mod pallet { | |
| ); | ||
| // Take recovery deposit | ||
| let recovery_deposit = T::RecoveryDeposit::get(); | ||
| T::Currency::hold(&HoldReason::RecoveryProcessDeposit.into(), &who, recovery_deposit)?; | ||
| // Create an active recovery status | ||
| let recovery_status = ActiveRecovery { | ||
| created: <frame_system::Pallet<T>>::block_number(), | ||
|
|
@@ -637,13 +661,16 @@ pub mod pallet { | |
| // Take the active recovery process started by the rescuer for this account. | ||
| let active_recovery = | ||
| <ActiveRecoveries<T>>::take(&who, &rescuer).ok_or(Error::<T>::NotStarted)?; | ||
| // Move the held funds from the rescuer to the rescued account. | ||
| // Acts like a slashing mechanism for those who try to maliciously recover accounts. | ||
| let res = T::Currency::transfer_on_hold( | ||
|
There was a problem hiding this comment. I am thinking about how to do a lazy migration here. Maybe you can require the Then in this function you can first try to use the new There was a problem hiding this comment. I am not a big fan of lazy migrations here, cause that will generate some "garbage" code that will live there forever. It seems more like a patch to me than a real solution. It's been a while already since we wanted the traits to be migrated that I would rather wait for the MBM to be ready and apply a proper solution |
||
| &HoldReason::RecoveryProcessDeposit.into(), | ||
| &rescuer, | ||
| &who, | ||
| active_recovery.deposit, | ||
| Precision::BestEffort, | ||
| Restriction::Free, | ||
| Fortitude::Force, | ||
| ); | ||
| debug_assert!(res.is_ok()); | ||
| Self::deposit_event(Event::<T>::RecoveryClosed { | ||
|
|
@@ -658,7 +685,7 @@ pub mod pallet { | |
| /// NOTE: The user must make sure to call `close_recovery` on all active | ||
| /// recovery attempts before calling this function else it will fail. | ||
| /// | ||
| /// Payment: By calling this function the recoverable account will release | ||
| /// their recovery configuration deposit. | ||
| /// (`ConfigDepositBase` + `FriendDepositFactor` * #_of_friends) | ||
| /// | ||
|
|
@@ -674,8 +701,13 @@ pub mod pallet { | |
| // Take the recovery configuration for this account. | ||
| let recovery_config = <Recoverable<T>>::take(&who).ok_or(Error::<T>::NotRecoverable)?; | ||
|
|
||
| // Release the initial deposit for the recovery configuration. | ||
| let _ = T::Currency::release( | ||
| &HoldReason::ConfigurationDeposit.into(), | ||
| &who, | ||
| recovery_config.deposit, | ||
| Precision::BestEffort, | ||
| ); | ||
| Self::deposit_event(Event::<T>::RecoveryRemoved { lost_account: who }); | ||
| Ok(()) | ||
| } | ||
|
|
||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.