From 455e440290b887f7480342b287a3ed618b3ea826 Mon Sep 17 00:00:00 2001 From: Gav Date: Tue, 21 Mar 2023 15:03:03 +0000 Subject: [PATCH 1/5] Initial stuff --- frame/support/src/traits.rs | 2 +- frame/support/src/traits/messages.rs | 15 +++++++- frame/support/src/traits/storage.rs | 31 ++++++++++++++++ .../src/traits/tokens/fungible/hold.rs | 35 +++++++++++++++++++ .../support/src/traits/tokens/fungible/mod.rs | 33 +++++++++++++++++ 5 files changed, 114 insertions(+), 2 deletions(-) diff --git a/frame/support/src/traits.rs b/frame/support/src/traits.rs index 4b92cfbaee69c..9b8b93454ad75 100644 --- a/frame/support/src/traits.rs +++ b/frame/support/src/traits.rs @@ -91,7 +91,7 @@ pub mod schedule; mod storage; pub use storage::{ Instance, PartialStorageInfoTrait, StorageInfo, StorageInfoTrait, StorageInstance, - TrackedStorageKey, WhitelistedStorageKeys, + TrackedStorageKey, WhitelistedStorageKeys, Consideration, }; mod dispatch; diff --git a/frame/support/src/traits/messages.rs b/frame/support/src/traits/messages.rs index 781da3ed6c704..60ec258b16373 100644 --- a/frame/support/src/traits/messages.rs +++ b/frame/support/src/traits/messages.rs @@ -104,13 +104,26 @@ impl ServiceQueues for NoopServiceQueues { } } -/// The resource footprint of a queue. +/// The resource footprint of a bunch of blobs. We assume only the number of blobs and their total +/// size in bytes matter. #[derive(Default, Copy, Clone, Eq, PartialEq, RuntimeDebug)] pub struct Footprint { + /// The number of blobs. pub count: u64, + /// The total size of the blobs in bytes. pub size: u64, } +impl Footprint { + pub fn from_parts(items: usize, len: usize) -> Self { + Self { count: items as u64, size: len as u64 } + } + + pub fn from_encodable(e: impl Encode) -> Self { + Self::from_parts(1, e.encoded_size()) + } +} + /// Can enqueue messages for multiple origins. pub trait EnqueueMessage { /// The maximal length any enqueued message may have. diff --git a/frame/support/src/traits/storage.rs b/frame/support/src/traits/storage.rs index c3394185a7743..e39db43465859 100644 --- a/frame/support/src/traits/storage.rs +++ b/frame/support/src/traits/storage.rs @@ -18,9 +18,13 @@ //! Traits for encoding data related to pallet's storage items. use crate::sp_std::collections::btree_set::BTreeSet; +use codec::{Encode, FullCodec, MaxEncodedLen}; use impl_trait_for_tuples::impl_for_tuples; +use scale_info::TypeInfo; pub use sp_core::storage::TrackedStorageKey; +use sp_runtime::{traits::Member, DispatchError}; use sp_std::prelude::*; +use super::Footprint; /// An instance of a pallet in the storage. /// @@ -120,3 +124,30 @@ impl WhitelistedStorageKeys for Tuple { combined_keys.into_iter().collect::>() } } + +pub trait Consideration { + type Ticket: Member + FullCodec + TypeInfo + MaxEncodedLen; + + fn update( + who: &AccountId, + old: Option, + new: Option, + ) -> Result; + + fn new( + who: &AccountId, + new: Option, + ) -> Result { + Self::update(who, None, new) + } + + fn drop( + who: &AccountId, + old: Option, + ) -> Result { + Self::update(who, old, None) + } +} + +#[test] +fn it_builds() {} \ No newline at end of file diff --git a/frame/support/src/traits/tokens/fungible/hold.rs b/frame/support/src/traits/tokens/fungible/hold.rs index ddcb8c6ac1da8..9a490139d57ce 100644 --- a/frame/support/src/traits/tokens/fungible/hold.rs +++ b/frame/support/src/traits/tokens/fungible/hold.rs @@ -238,6 +238,41 @@ pub trait Mutate: Ok(actual) } + /// Hold or release funds in the account of `who` to bring the balance on hold for `reason` to + /// exactly `amount`. + fn set_on_hold( + reason: &Self::Reason, + who: &AccountId, + amount: Self::Balance, + ) -> DispatchResult { + let current_amount = Self::balance_on_hold(reason, who); + if current_amount < amount { + Self::hold(reason, who, amount - current_amount) + } else if current_amount > amount { + Self::release(reason, who, current_amount - amount, Precision::Exact).map(|_| ()) + } else { + Ok(()) + } + } + + /// Release all funds in the account of `who` on hold for `reason`. + /// + /// The actual amount released is returned with `Ok`. + /// + /// If `precision` is `BestEffort`, then the amount actually unreserved and returned as the + /// inner value of `Ok` may be smaller than the `amount` passed. + /// + /// NOTE! The inner of the `Ok` result variant returns the *actual* amount released. This is the + /// opposite of the `ReservableCurrency::unreserve()` result, which gives the amount not able + /// to be released! + fn release_all( + reason: &Self::Reason, + who: &AccountId, + precision: Precision, + ) -> Result { + Self::release(reason, who, Self::balance_on_hold(reason, who), precision) + } + /// Attempt to decrease the balance of `who` which is held for the given `reason` by `amount`. /// /// If `precision` is `BestEffort`, then as much as possible is reduced, up to `amount`, and the diff --git a/frame/support/src/traits/tokens/fungible/mod.rs b/frame/support/src/traits/tokens/fungible/mod.rs index 204b85ac29448..9c9eb79e43e05 100644 --- a/frame/support/src/traits/tokens/fungible/mod.rs +++ b/frame/support/src/traits/tokens/fungible/mod.rs @@ -54,3 +54,36 @@ pub use item_of::ItemOf; pub use regular::{ Balanced, DecreaseIssuance, Dust, IncreaseIssuance, Inspect, Mutate, Unbalanced, }; +use sp_runtime::traits::Convert; + +use crate::traits::{Consideration, Footprint}; + +pub struct FreezeConsideration(sp_std::marker::PhantomData<(A, F, R, D)>); +impl, R: F::Id, D: Convert> Consideration for FreezeConsideration { + type Ticket = (); + fn update( + who: &A, + _old: Option, + new: Option, + ) -> Result { + match new { + Some(footprint) => F::set_freeze(&R::get(), who, D::convert(footprint)), + None => F::thaw(&R::get(), who), + } + } +} + +pub struct HoldConsideration(sp_std::marker::PhantomData<(A, F, R, D)>); +impl, R: F::Id, D: Convert> Consideration for HoldConsideration { + type Ticket = (); + fn update( + who: &A, + _old: Option, + new: Option, + ) -> Result { + match (old, new) { + Some(footprint) => F::set_on_hold(&R::get(), who, D::convert(footprint)), + None => F::release_all(&R::get(), who, super::Precision::BestEffort), + } + } +} \ No newline at end of file From d5f68828f5ae23530316c4a16c8d22b685a7dcd2 Mon Sep 17 00:00:00 2001 From: Gav Date: Tue, 21 Mar 2023 17:27:19 +0000 Subject: [PATCH 2/5] Fixes --- frame/support/src/traits.rs | 4 ++-- frame/support/src/traits/storage.rs | 16 +++++----------- frame/support/src/traits/tokens/fungible/mod.rs | 15 ++++++++++----- 3 files changed, 17 insertions(+), 18 deletions(-) diff --git a/frame/support/src/traits.rs b/frame/support/src/traits.rs index 9b8b93454ad75..4cd56ea0ac34d 100644 --- a/frame/support/src/traits.rs +++ b/frame/support/src/traits.rs @@ -90,8 +90,8 @@ pub use hooks::{ pub mod schedule; mod storage; pub use storage::{ - Instance, PartialStorageInfoTrait, StorageInfo, StorageInfoTrait, StorageInstance, - TrackedStorageKey, WhitelistedStorageKeys, Consideration, + Consideration, Instance, PartialStorageInfoTrait, StorageInfo, StorageInfoTrait, + StorageInstance, TrackedStorageKey, WhitelistedStorageKeys, }; mod dispatch; diff --git a/frame/support/src/traits/storage.rs b/frame/support/src/traits/storage.rs index e39db43465859..606de9435fffe 100644 --- a/frame/support/src/traits/storage.rs +++ b/frame/support/src/traits/storage.rs @@ -17,14 +17,14 @@ //! Traits for encoding data related to pallet's storage items. +use super::Footprint; use crate::sp_std::collections::btree_set::BTreeSet; -use codec::{Encode, FullCodec, MaxEncodedLen}; +use codec::{FullCodec, MaxEncodedLen}; use impl_trait_for_tuples::impl_for_tuples; use scale_info::TypeInfo; pub use sp_core::storage::TrackedStorageKey; use sp_runtime::{traits::Member, DispatchError}; use sp_std::prelude::*; -use super::Footprint; /// An instance of a pallet in the storage. /// @@ -134,20 +134,14 @@ pub trait Consideration { new: Option, ) -> Result; - fn new( - who: &AccountId, - new: Option, - ) -> Result { + fn new(who: &AccountId, new: Option) -> Result { Self::update(who, None, new) } - fn drop( - who: &AccountId, - old: Option, - ) -> Result { + fn drop(who: &AccountId, old: Option) -> Result { Self::update(who, old, None) } } #[test] -fn it_builds() {} \ No newline at end of file +fn it_builds() {} diff --git a/frame/support/src/traits/tokens/fungible/mod.rs b/frame/support/src/traits/tokens/fungible/mod.rs index 9c9eb79e43e05..2a299735801a4 100644 --- a/frame/support/src/traits/tokens/fungible/mod.rs +++ b/frame/support/src/traits/tokens/fungible/mod.rs @@ -54,12 +54,15 @@ pub use item_of::ItemOf; pub use regular::{ Balanced, DecreaseIssuance, Dust, IncreaseIssuance, Inspect, Mutate, Unbalanced, }; +use sp_core::Get; use sp_runtime::traits::Convert; use crate::traits::{Consideration, Footprint}; pub struct FreezeConsideration(sp_std::marker::PhantomData<(A, F, R, D)>); -impl, R: F::Id, D: Convert> Consideration for FreezeConsideration { +impl, R: Get, D: Convert> Consideration + for FreezeConsideration +{ type Ticket = (); fn update( who: &A, @@ -74,16 +77,18 @@ impl, R: F::Id, D: Convert> Conside } pub struct HoldConsideration(sp_std::marker::PhantomData<(A, F, R, D)>); -impl, R: F::Id, D: Convert> Consideration for HoldConsideration { +impl, R: Get, D: Convert> Consideration + for HoldConsideration +{ type Ticket = (); fn update( who: &A, _old: Option, new: Option, ) -> Result { - match (old, new) { + match new { Some(footprint) => F::set_on_hold(&R::get(), who, D::convert(footprint)), - None => F::release_all(&R::get(), who, super::Precision::BestEffort), + None => F::release_all(&R::get(), who, super::Precision::BestEffort).map(|_| ()), } } -} \ No newline at end of file +} From b8a6c7c689634c11a39a38ed72e02fd1d42492a7 Mon Sep 17 00:00:00 2001 From: Gav Date: Tue, 21 Mar 2023 17:42:48 +0000 Subject: [PATCH 3/5] More stuff --- frame/balances/src/tests/currency_tests.rs | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/frame/balances/src/tests/currency_tests.rs b/frame/balances/src/tests/currency_tests.rs index b0a38f4ac4ed7..867fd5a2b260e 100644 --- a/frame/balances/src/tests/currency_tests.rs +++ b/frame/balances/src/tests/currency_tests.rs @@ -22,7 +22,7 @@ use crate::NegativeImbalance; use frame_support::traits::{ BalanceStatus::{Free, Reserved}, Currency, - ExistenceRequirement::{self, AllowDeath}, + ExistenceRequirement::{self, AllowDeath, KeepAlive}, LockIdentifier, LockableCurrency, NamedReservableCurrency, ReservableCurrency, WithdrawReasons, }; @@ -32,6 +32,17 @@ const ID_2: LockIdentifier = *b"2 "; pub const CALL: &::RuntimeCall = &RuntimeCall::Balances(crate::Call::transfer_allow_death { dest: 0, value: 0 }); +#[test] +fn ed_should_work() { + ExtBuilder::default() + .existential_deposit(1) + .build_and_execute_with(|| { + assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), 1, 1000)); + assert_noop!(>::transfer(&1, &10, 1000, KeepAlive), TokenError::NotExpendable); + assert_ok!(>::transfer(&1, &10, 1000, AllowDeath)); + }); +} + #[test] fn basic_locking_should_work() { ExtBuilder::default() From af4f14c67d5f4542cbc0ace823cde279faa4b334 Mon Sep 17 00:00:00 2001 From: Gav Date: Tue, 21 Mar 2023 21:16:43 +0000 Subject: [PATCH 4/5] Example of usage --- frame/preimage/src/lib.rs | 62 +++++++++++++++++++++++++++-- frame/support/src/traits/storage.rs | 10 ++--- 2 files changed, 63 insertions(+), 9 deletions(-) diff --git a/frame/preimage/src/lib.rs b/frame/preimage/src/lib.rs index 60208424db953..4ad0685259a7f 100644 --- a/frame/preimage/src/lib.rs +++ b/frame/preimage/src/lib.rs @@ -46,8 +46,8 @@ use frame_support::{ ensure, pallet_prelude::Get, traits::{ - Currency, Defensive, FetchResult, Hash as PreimageHash, PreimageProvider, - PreimageRecipient, QueryPreimage, ReservableCurrency, StorePreimage, + Consideration, Currency, Defensive, FetchResult, Hash as PreimageHash, PreimageProvider, + PreimageRecipient, QueryPreimage, ReservableCurrency, StorePreimage, Footprint, }, BoundedSlice, BoundedVec, }; @@ -61,7 +61,7 @@ pub use pallet::*; /// A type to note whether a preimage is owned by a user or the system. #[derive(Clone, Eq, PartialEq, Encode, Decode, TypeInfo, MaxEncodedLen, RuntimeDebug)] -pub enum RequestStatus { +pub enum OldRequestStatus { /// The associated preimage has not yet been requested by the system. The given deposit (if /// some) is being held until either it becomes requested or the user retracts the preimage. Unrequested { deposit: (AccountId, Balance), len: u32 }, @@ -71,8 +71,22 @@ pub enum RequestStatus { Requested { deposit: Option<(AccountId, Balance)>, count: u32, len: Option }, } +/// A type to note whether a preimage is owned by a user or the system. +#[derive(Clone, Eq, PartialEq, Encode, Decode, TypeInfo, MaxEncodedLen, RuntimeDebug)] +pub enum RequestStatus { + /// The associated preimage has not yet been requested by the system. The given deposit (if + /// some) is being held until either it becomes requested or the user retracts the preimage. + Unrequested { ticket: (AccountId, Ticket), len: u32 }, + /// There are a non-zero number of outstanding requests for this hash by this chain. If there + /// is a preimage registered, then `len` is `Some` and it may be removed iff this counter + /// becomes zero. + Requested { maybe_ticket: Option<(AccountId, Ticket)>, count: u32, len: Option }, +} + type BalanceOf = <::Currency as Currency<::AccountId>>::Balance; +type TicketOf = + <::Consideration as Consideration<::AccountId>>::Ticket; /// Maximum size of preimage we can store is 4mb. const MAX_SIZE: u32 = 4 * 1024 * 1024; @@ -104,6 +118,8 @@ pub mod pallet { /// The per-byte deposit for placing a preimage on chain. type ByteDeposit: Get>; + + type Consideration: Consideration; } #[pallet::pallet] @@ -140,7 +156,12 @@ pub mod pallet { /// The request status of a given hash. #[pallet::storage] pub(super) type StatusFor = - StorageMap<_, Identity, T::Hash, RequestStatus>>; + StorageMap<_, Identity, T::Hash, OldRequestStatus>>; + + /// The request status of a given hash. + #[pallet::storage] + pub(super) type RequestStatusFor = + StorageMap<_, Identity, T::Hash, RequestStatus>>; #[pallet::storage] pub(super) type PreimageFor = @@ -204,6 +225,38 @@ pub mod pallet { } impl Pallet { + fn ensure_updated(h: T::Hash) -> bool { + let r = match StatusFor::::take(&h) { + Some(r) => r, + None => return false, + }; + let n = match r { + OldRequestStatus::Unrequested { deposit: (who, amount), len } => { + // unreserve deposit + T::Currency::unreserve(&who, amount); + // take consideration + let ticket = T::Consideration::new(&who, Footprint::from_parts(1, len)) + .unwrap_or_default(); + RequestStatus::Unrequested { ticket: (who, ticket), len } + }, + OldRequestStatus::Requested { deposit: maybe_deposit, count, len } => { + let maybe_ticket = if let Some((who, deposit)) = maybe_deposit { + // unreserve deposit + T::Currency::unreserve(&who, amount); + // take consideration + let ticket = T::Consideration::new(&who, Footprint::from_parts(1, len)) + .unwrap_or_default(); + Some((who, ticket)) + } else { + None + }; + RequestStatus::Requested { maybe_ticket, count, len } + } + }; + RequestStatusFor::::insert(&h, n); + true + } + /// Ensure that the origin is either the `ManagerOrigin` or a signed origin. fn ensure_signed_or_manager( origin: T::RuntimeOrigin, @@ -245,6 +298,7 @@ impl Pallet { let deposit = T::BaseDeposit::get() .saturating_add(T::ByteDeposit::get().saturating_mul(length.into())); T::Currency::reserve(depositor, deposit)?; + T::Consideration::new(depositor, ) RequestStatus::Unrequested { deposit: (depositor.clone(), deposit), len } }, }; diff --git a/frame/support/src/traits/storage.rs b/frame/support/src/traits/storage.rs index 606de9435fffe..9cd118ee1e2d5 100644 --- a/frame/support/src/traits/storage.rs +++ b/frame/support/src/traits/storage.rs @@ -126,7 +126,7 @@ impl WhitelistedStorageKeys for Tuple { } pub trait Consideration { - type Ticket: Member + FullCodec + TypeInfo + MaxEncodedLen; + type Ticket: Member + FullCodec + TypeInfo + MaxEncodedLen + Default; fn update( who: &AccountId, @@ -134,12 +134,12 @@ pub trait Consideration { new: Option, ) -> Result; - fn new(who: &AccountId, new: Option) -> Result { - Self::update(who, None, new) + fn new(who: &AccountId, new: Footprint) -> Result { + Self::update(who, None, Some(new)) } - fn drop(who: &AccountId, old: Option) -> Result { - Self::update(who, old, None) + fn drop(who: &AccountId, old: Self::Ticket) -> Result { + Self::update(who, Some(old), None) } } From 8bc352209cccd8c1003aa343ae14caeaee21a86d Mon Sep 17 00:00:00 2001 From: Gav Date: Wed, 22 Mar 2023 09:41:00 +0000 Subject: [PATCH 5/5] Move preimages pallet over to freezes and consideration --- frame/balances/src/tests/currency_tests.rs | 15 +- frame/preimage/src/lib.rs | 136 +++++++++++------- frame/preimage/src/migration.rs | 31 ++-- frame/preimage/src/mock.rs | 14 +- frame/preimage/src/tests.rs | 45 +++--- .../src/traits/tokens/fungible/freeze.rs | 25 +++- .../support/src/traits/tokens/fungible/mod.rs | 34 +++++ frame/support/src/traits/tokens/misc.rs | 16 ++- 8 files changed, 211 insertions(+), 105 deletions(-) diff --git a/frame/balances/src/tests/currency_tests.rs b/frame/balances/src/tests/currency_tests.rs index 867fd5a2b260e..e9b0f04deb200 100644 --- a/frame/balances/src/tests/currency_tests.rs +++ b/frame/balances/src/tests/currency_tests.rs @@ -34,13 +34,14 @@ pub const CALL: &::RuntimeCall = #[test] fn ed_should_work() { - ExtBuilder::default() - .existential_deposit(1) - .build_and_execute_with(|| { - assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), 1, 1000)); - assert_noop!(>::transfer(&1, &10, 1000, KeepAlive), TokenError::NotExpendable); - assert_ok!(>::transfer(&1, &10, 1000, AllowDeath)); - }); + ExtBuilder::default().existential_deposit(1).build_and_execute_with(|| { + assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), 1, 1000)); + assert_noop!( + >::transfer(&1, &10, 1000, KeepAlive), + TokenError::NotExpendable + ); + assert_ok!(>::transfer(&1, &10, 1000, AllowDeath)); + }); } #[test] diff --git a/frame/preimage/src/lib.rs b/frame/preimage/src/lib.rs index 4ad0685259a7f..0e25d52a13ddb 100644 --- a/frame/preimage/src/lib.rs +++ b/frame/preimage/src/lib.rs @@ -46,8 +46,8 @@ use frame_support::{ ensure, pallet_prelude::Get, traits::{ - Consideration, Currency, Defensive, FetchResult, Hash as PreimageHash, PreimageProvider, - PreimageRecipient, QueryPreimage, ReservableCurrency, StorePreimage, Footprint, + Consideration, Currency, Defensive, FetchResult, Footprint, Hash as PreimageHash, + PreimageProvider, PreimageRecipient, QueryPreimage, ReservableCurrency, StorePreimage, }, BoundedSlice, BoundedVec, }; @@ -80,7 +80,7 @@ pub enum RequestStatus { /// There are a non-zero number of outstanding requests for this hash by this chain. If there /// is a preimage registered, then `len` is `Some` and it may be removed iff this counter /// becomes zero. - Requested { maybe_ticket: Option<(AccountId, Ticket)>, count: u32, len: Option }, + Requested { maybe_ticket: Option<(AccountId, Ticket)>, count: u32, maybe_len: Option }, } type BalanceOf = @@ -92,6 +92,7 @@ type TicketOf = const MAX_SIZE: u32 = 4 * 1024 * 1024; #[frame_support::pallet] +#[allow(deprecated)] pub mod pallet { use super::*; @@ -154,6 +155,7 @@ pub mod pallet { } /// The request status of a given hash. + #[deprecated = "RequestStatusFor"] #[pallet::storage] pub(super) type StatusFor = StorageMap<_, Identity, T::Hash, OldRequestStatus>>; @@ -225,8 +227,9 @@ pub mod pallet { } impl Pallet { - fn ensure_updated(h: T::Hash) -> bool { - let r = match StatusFor::::take(&h) { + fn ensure_updated(h: &T::Hash) -> bool { + #[allow(deprecated)] + let r = match StatusFor::::take(h) { Some(r) => r, None => return false, }; @@ -235,25 +238,30 @@ impl Pallet { // unreserve deposit T::Currency::unreserve(&who, amount); // take consideration - let ticket = T::Consideration::new(&who, Footprint::from_parts(1, len)) + let ticket = T::Consideration::new(&who, Footprint::from_parts(1, len as usize)) .unwrap_or_default(); RequestStatus::Unrequested { ticket: (who, ticket), len } }, - OldRequestStatus::Requested { deposit: maybe_deposit, count, len } => { + OldRequestStatus::Requested { deposit: maybe_deposit, count, len: maybe_len } => { let maybe_ticket = if let Some((who, deposit)) = maybe_deposit { // unreserve deposit - T::Currency::unreserve(&who, amount); + T::Currency::unreserve(&who, deposit); // take consideration - let ticket = T::Consideration::new(&who, Footprint::from_parts(1, len)) - .unwrap_or_default(); - Some((who, ticket)) + if let Some(len) = maybe_len { + let ticket = + T::Consideration::new(&who, Footprint::from_parts(1, len as usize)) + .unwrap_or_default(); + Some((who, ticket)) + } else { + None + } } else { None }; - RequestStatus::Requested { maybe_ticket, count, len } - } + RequestStatus::Requested { maybe_ticket, count, maybe_len } + }, }; - RequestStatusFor::::insert(&h, n); + RequestStatusFor::::insert(h, n); true } @@ -283,27 +291,29 @@ impl Pallet { let len = preimage.len() as u32; ensure!(len <= MAX_SIZE, Error::::TooBig); + Self::ensure_updated(&hash); // We take a deposit only if there is a provided depositor and the preimage was not // previously requested. This also allows the tx to pay no fee. - let status = match (StatusFor::::get(hash), maybe_depositor) { - (Some(RequestStatus::Requested { count, deposit, .. }), _) => - RequestStatus::Requested { count, deposit, len: Some(len) }, + let status = match (RequestStatusFor::::get(hash), maybe_depositor) { + (Some(RequestStatus::Requested { maybe_ticket, count, .. }), _) => + RequestStatus::Requested { maybe_ticket, count, maybe_len: Some(len) }, (Some(RequestStatus::Unrequested { .. }), Some(_)) => return Err(Error::::AlreadyNoted.into()), - (Some(RequestStatus::Unrequested { len, deposit }), None) => - RequestStatus::Requested { deposit: Some(deposit), count: 1, len: Some(len) }, - (None, None) => RequestStatus::Requested { count: 1, len: Some(len), deposit: None }, + (Some(RequestStatus::Unrequested { ticket, len }), None) => RequestStatus::Requested { + maybe_ticket: Some(ticket), + count: 1, + maybe_len: Some(len), + }, + (None, None) => + RequestStatus::Requested { maybe_ticket: None, count: 1, maybe_len: Some(len) }, (None, Some(depositor)) => { - let length = preimage.len() as u32; - let deposit = T::BaseDeposit::get() - .saturating_add(T::ByteDeposit::get().saturating_mul(length.into())); - T::Currency::reserve(depositor, deposit)?; - T::Consideration::new(depositor, ) - RequestStatus::Unrequested { deposit: (depositor.clone(), deposit), len } + let ticket = + T::Consideration::new(depositor, Footprint::from_parts(1, len as usize))?; + RequestStatus::Unrequested { ticket: (depositor.clone(), ticket), len } }, }; let was_requested = matches!(status, RequestStatus::Requested { .. }); - StatusFor::::insert(hash, status); + RequestStatusFor::::insert(hash, status); let _ = Self::insert(&hash, preimage) .defensive_proof("Unable to insert. Logic error in `note_bytes`?"); @@ -318,15 +328,19 @@ impl Pallet { // If the preimage already exists before the request is made, the deposit for the preimage is // returned to the user, and removed from their management. fn do_request_preimage(hash: &T::Hash) { - let (count, len, deposit) = - StatusFor::::get(hash).map_or((1, None, None), |x| match x { - RequestStatus::Requested { mut count, len, deposit } => { + Self::ensure_updated(&hash); + let (count, maybe_len, maybe_ticket) = + RequestStatusFor::::get(hash).map_or((1, None, None), |x| match x { + RequestStatus::Requested { maybe_ticket, mut count, maybe_len } => { count.saturating_inc(); - (count, len, deposit) + (count, maybe_len, maybe_ticket) }, - RequestStatus::Unrequested { deposit, len } => (1, Some(len), Some(deposit)), + RequestStatus::Unrequested { ticket, len } => (1, Some(len), Some(ticket)), }); - StatusFor::::insert(hash, RequestStatus::Requested { count, len, deposit }); + RequestStatusFor::::insert( + hash, + RequestStatus::Requested { maybe_ticket, count, maybe_len }, + ); if count == 1 { Self::deposit_event(Event::Requested { hash: *hash }); } @@ -342,24 +356,25 @@ impl Pallet { hash: &T::Hash, maybe_check_owner: Option, ) -> DispatchResult { - match StatusFor::::get(hash).ok_or(Error::::NotNoted)? { - RequestStatus::Requested { deposit: Some((owner, deposit)), count, len } => { + Self::ensure_updated(&hash); + match RequestStatusFor::::get(hash).ok_or(Error::::NotNoted)? { + RequestStatus::Requested { maybe_ticket: Some((owner, ticket)), count, maybe_len } => { ensure!(maybe_check_owner.map_or(true, |c| c == owner), Error::::NotAuthorized); - T::Currency::unreserve(&owner, deposit); - StatusFor::::insert( + let _ = T::Consideration::drop(&owner, ticket); + RequestStatusFor::::insert( hash, - RequestStatus::Requested { deposit: None, count, len }, + RequestStatus::Requested { maybe_ticket: None, count, maybe_len }, ); Ok(()) }, - RequestStatus::Requested { deposit: None, .. } => { + RequestStatus::Requested { maybe_ticket: None, .. } => { ensure!(maybe_check_owner.is_none(), Error::::NotAuthorized); Self::do_unrequest_preimage(hash) }, - RequestStatus::Unrequested { deposit: (owner, deposit), len } => { + RequestStatus::Unrequested { ticket: (owner, ticket), len } => { ensure!(maybe_check_owner.map_or(true, |c| c == owner), Error::::NotAuthorized); - T::Currency::unreserve(&owner, deposit); - StatusFor::::remove(hash); + let _ = T::Consideration::drop(&owner, ticket); + RequestStatusFor::::remove(hash); Self::remove(hash, len); Self::deposit_event(Event::Cleared { hash: *hash }); @@ -370,25 +385,32 @@ impl Pallet { /// Clear a preimage request. fn do_unrequest_preimage(hash: &T::Hash) -> DispatchResult { - match StatusFor::::get(hash).ok_or(Error::::NotRequested)? { - RequestStatus::Requested { mut count, len, deposit } if count > 1 => { + Self::ensure_updated(&hash); + match RequestStatusFor::::get(hash).ok_or(Error::::NotRequested)? { + RequestStatus::Requested { mut count, maybe_len, maybe_ticket } if count > 1 => { count.saturating_dec(); - StatusFor::::insert(hash, RequestStatus::Requested { count, len, deposit }); + RequestStatusFor::::insert( + hash, + RequestStatus::Requested { maybe_ticket, count, maybe_len }, + ); }, - RequestStatus::Requested { count, len, deposit } => { + RequestStatus::Requested { count, maybe_len, maybe_ticket } => { debug_assert!(count == 1, "preimage request counter at zero?"); - match (len, deposit) { + match (maybe_len, maybe_ticket) { // Preimage was never noted. - (None, _) => StatusFor::::remove(hash), + (None, _) => RequestStatusFor::::remove(hash), // Preimage was noted without owner - just remove it. (Some(len), None) => { Self::remove(hash, len); - StatusFor::::remove(hash); + RequestStatusFor::::remove(hash); Self::deposit_event(Event::Cleared { hash: *hash }); }, // Preimage was noted with owner - move to unrequested so they can get refund. - (Some(len), Some(deposit)) => { - StatusFor::::insert(hash, RequestStatus::Unrequested { deposit, len }); + (Some(len), Some(ticket)) => { + RequestStatusFor::::insert( + hash, + RequestStatus::Unrequested { ticket, len }, + ); }, } }, @@ -413,8 +435,10 @@ impl Pallet { fn len(hash: &T::Hash) -> Option { use RequestStatus::*; - match StatusFor::::get(hash) { - Some(Requested { len: Some(len), .. }) | Some(Unrequested { len, .. }) => Some(len), + Self::ensure_updated(&hash); + match RequestStatusFor::::get(hash) { + Some(Requested { maybe_len: Some(len), .. }) | Some(Unrequested { len, .. }) => + Some(len), _ => None, } } @@ -434,7 +458,8 @@ impl PreimageProvider for Pallet { } fn preimage_requested(hash: &T::Hash) -> bool { - matches!(StatusFor::::get(hash), Some(RequestStatus::Requested { .. })) + Self::ensure_updated(hash); + matches!(RequestStatusFor::::get(hash), Some(RequestStatus::Requested { .. })) } fn get_preimage(hash: &T::Hash) -> Option> { @@ -477,7 +502,8 @@ impl> QueryPreimage for Pallet { } fn is_requested(hash: &T::Hash) -> bool { - matches!(StatusFor::::get(hash), Some(RequestStatus::Requested { .. })) + Self::ensure_updated(&hash); + matches!(RequestStatusFor::::get(hash), Some(RequestStatus::Requested { .. })) } fn request(hash: &T::Hash) { diff --git a/frame/preimage/src/migration.rs b/frame/preimage/src/migration.rs index be352201da6cd..cff561c1e276e 100644 --- a/frame/preimage/src/migration.rs +++ b/frame/preimage/src/migration.rs @@ -32,7 +32,7 @@ mod v0 { use super::*; #[derive(Clone, Eq, PartialEq, Encode, Decode, TypeInfo, MaxEncodedLen, RuntimeDebug)] - pub enum RequestStatus { + pub enum OldRequestStatus { Unrequested(Option<(AccountId, Balance)>), Requested(u32), } @@ -50,7 +50,7 @@ mod v0 { Pallet, Identity, ::Hash, - RequestStatus<::AccountId, BalanceOf>, + OldRequestStatus<::AccountId, BalanceOf>, >; /// Returns the number of images or `None` if the storage is corrupted. @@ -122,21 +122,22 @@ pub mod v1 { } let status = match status { - v0::RequestStatus::Unrequested(deposit) => match deposit { - Some(deposit) => RequestStatus::Unrequested { deposit, len }, + v0::OldRequestStatus::Unrequested(deposit) => match deposit { + Some(deposit) => OldRequestStatus::Unrequested { deposit, len }, // `None` depositor becomes system-requested. None => - RequestStatus::Requested { deposit: None, count: 1, len: Some(len) }, + OldRequestStatus::Requested { deposit: None, count: 1, len: Some(len) }, }, - v0::RequestStatus::Requested(count) if count == 0 => { + v0::OldRequestStatus::Requested(count) if count == 0 => { log::error!(target: TARGET, "preimage has counter of zero: {:?}", hash); continue }, - v0::RequestStatus::Requested(count) => - RequestStatus::Requested { deposit: None, count, len: Some(len) }, + v0::OldRequestStatus::Requested(count) => + OldRequestStatus::Requested { deposit: None, count, len: Some(len) }, }; log::trace!(target: TARGET, "Moving preimage {:?} with len {}", hash, len); + #[allow(deprecated)] crate::StatusFor::::insert(hash, status); crate::PreimageFor::::insert(&(hash, len), preimage); @@ -198,19 +199,19 @@ mod test { // Case 1: Unrequested without deposit let (p, h) = preimage::(128); v0::PreimageFor::::insert(h, p); - v0::StatusFor::::insert(h, v0::RequestStatus::Unrequested(None)); + v0::StatusFor::::insert(h, v0::OldRequestStatus::Unrequested(None)); // Case 2: Unrequested with deposit let (p, h) = preimage::(1024); v0::PreimageFor::::insert(h, p); - v0::StatusFor::::insert(h, v0::RequestStatus::Unrequested(Some((1, 1)))); + v0::StatusFor::::insert(h, v0::OldRequestStatus::Unrequested(Some((1, 1)))); // Case 3: Requested by 0 (invalid) let (p, h) = preimage::(8192); v0::PreimageFor::::insert(h, p); - v0::StatusFor::::insert(h, v0::RequestStatus::Requested(0)); + v0::StatusFor::::insert(h, v0::OldRequestStatus::Requested(0)); // Case 4: Requested by 10 let (p, h) = preimage::(65536); v0::PreimageFor::::insert(h, p); - v0::StatusFor::::insert(h, v0::RequestStatus::Requested(10)); + v0::StatusFor::::insert(h, v0::OldRequestStatus::Requested(10)); assert_eq!(v0::image_count::(), Some(4)); assert_eq!(v1::image_count::(), None, "V1 storage should be corrupted"); @@ -229,14 +230,14 @@ mod test { assert_eq!(crate::PreimageFor::::get(&(h, 128)), Some(p)); assert_eq!( crate::StatusFor::::get(h), - Some(RequestStatus::Requested { deposit: None, count: 1, len: Some(128) }) + Some(OldRequestStatus::Requested { deposit: None, count: 1, len: Some(128) }) ); // Case 2: Unrequested with deposit becomes unrequested let (p, h) = preimage::(1024); assert_eq!(crate::PreimageFor::::get(&(h, 1024)), Some(p)); assert_eq!( crate::StatusFor::::get(h), - Some(RequestStatus::Unrequested { deposit: (1, 1), len: 1024 }) + Some(OldRequestStatus::Unrequested { deposit: (1, 1), len: 1024 }) ); // Case 3: Requested by 0 should be skipped let (_, h) = preimage::(8192); @@ -247,7 +248,7 @@ mod test { assert_eq!(crate::PreimageFor::::get(&(h, 65536)), Some(p)); assert_eq!( crate::StatusFor::::get(h), - Some(RequestStatus::Requested { deposit: None, count: 10, len: Some(65536) }) + Some(OldRequestStatus::Requested { deposit: None, count: 10, len: Some(65536) }) ); }); } diff --git a/frame/preimage/src/mock.rs b/frame/preimage/src/mock.rs index 5054a77a8123f..c4ccd191c4ba5 100644 --- a/frame/preimage/src/mock.rs +++ b/frame/preimage/src/mock.rs @@ -22,14 +22,14 @@ use super::*; use crate as pallet_preimage; use frame_support::{ ord_parameter_types, - traits::{ConstU32, ConstU64, Everything}, + traits::{fungible::FreezeMultiConsideration, ConstU32, ConstU64, Everything}, weights::constants::RocksDbWeight, }; use frame_system::EnsureSignedBy; use sp_core::H256; use sp_runtime::{ testing::Header, - traits::{BlakeTwo256, IdentityLookup}, + traits::{BlakeTwo256, Convert, IdentityLookup}, }; type UncheckedExtrinsic = frame_system::mocking::MockUncheckedExtrinsic; @@ -85,7 +85,7 @@ impl pallet_balances::Config for Test { type MaxReserves = ConstU32<50>; type ReserveIdentifier = [u8; 8]; type FreezeIdentifier = (); - type MaxFreezes = (); + type MaxFreezes = ConstU32<1>; type HoldIdentifier = (); type MaxHolds = (); } @@ -94,6 +94,13 @@ ord_parameter_types! { pub const One: u64 = 1; } +pub struct ConvertDeposit; +impl Convert for ConvertDeposit { + fn convert(a: Footprint) -> u64 { + a.count * 2 + a.size + } +} + impl Config for Test { type WeightInfo = (); type RuntimeEvent = RuntimeEvent; @@ -101,6 +108,7 @@ impl Config for Test { type ManagerOrigin = EnsureSignedBy; type BaseDeposit = ConstU64<2>; type ByteDeposit = ConstU64<1>; + type Consideration = FreezeMultiConsideration; } pub fn new_test_ext() -> sp_io::TestExternalities { diff --git a/frame/preimage/src/tests.rs b/frame/preimage/src/tests.rs index 63a178c408a3f..73d56121b409b 100644 --- a/frame/preimage/src/tests.rs +++ b/frame/preimage/src/tests.rs @@ -24,11 +24,11 @@ use crate::mock::*; use frame_support::{ assert_err, assert_noop, assert_ok, assert_storage_noop, bounded_vec, - traits::{Bounded, BoundedInline, Hash as PreimageHash}, + traits::{fungible::InspectFreeze, Bounded, BoundedInline, Hash as PreimageHash}, StorageNoopGuard, }; -use pallet_balances::Error as BalancesError; use sp_core::{blake2_256, H256}; +use sp_runtime::TokenError; /// Returns one `Inline`, `Lookup` and `Legacy` item each with different data and hash. pub fn make_bounded_values() -> (Bounded>, Bounded>, Bounded>) { @@ -51,8 +51,8 @@ pub fn make_bounded_values() -> (Bounded>, Bounded>, Bounded::AlreadyNoted + Error::::AlreadyNoted, ); assert_noop!( Preimage::note_preimage(RuntimeOrigin::signed(0), vec![2]), - BalancesError::::InsufficientBalance + TokenError::FundsUnavailable, ); }); } @@ -170,7 +170,7 @@ fn request_note_order_makes_no_difference() { assert_ok!(Preimage::request_preimage(RuntimeOrigin::signed(1), hashed([1]))); assert_ok!(Preimage::note_preimage(RuntimeOrigin::signed(2), vec![1])); ( - StatusFor::::iter().collect::>(), + RequestStatusFor::::iter().collect::>(), PreimageFor::::iter().collect::>(), ) }); @@ -179,7 +179,7 @@ fn request_note_order_makes_no_difference() { assert_ok!(Preimage::request_preimage(RuntimeOrigin::signed(1), hashed([1]))); assert_ok!(Preimage::unnote_preimage(RuntimeOrigin::signed(2), hashed([1]))); let other_way = ( - StatusFor::::iter().collect::>(), + RequestStatusFor::::iter().collect::>(), PreimageFor::::iter().collect::>(), ); assert_eq!(one_way, other_way); @@ -206,7 +206,7 @@ fn request_user_note_order_makes_no_difference() { assert_ok!(Preimage::request_preimage(RuntimeOrigin::signed(1), hashed([1]))); assert_ok!(Preimage::note_preimage(RuntimeOrigin::signed(2), vec![1])); ( - StatusFor::::iter().collect::>(), + RequestStatusFor::::iter().collect::>(), PreimageFor::::iter().collect::>(), ) }); @@ -215,7 +215,7 @@ fn request_user_note_order_makes_no_difference() { assert_ok!(Preimage::request_preimage(RuntimeOrigin::signed(1), hashed([1]))); assert_ok!(Preimage::unnote_preimage(RuntimeOrigin::signed(2), hashed([1]))); let other_way = ( - StatusFor::::iter().collect::>(), + RequestStatusFor::::iter().collect::>(), PreimageFor::::iter().collect::>(), ); assert_eq!(one_way, other_way); @@ -248,13 +248,14 @@ fn unrequest_preimage_works() { fn user_noted_then_requested_preimage_is_refunded_once_only() { new_test_ext().execute_with(|| { assert_ok!(Preimage::note_preimage(RuntimeOrigin::signed(2), vec![1; 3])); + assert_eq!(Balances::balance_frozen(&(), &2), 5); assert_ok!(Preimage::note_preimage(RuntimeOrigin::signed(2), vec![1])); + assert_eq!(Balances::balance_frozen(&(), &2), 8); assert_ok!(Preimage::request_preimage(RuntimeOrigin::signed(1), hashed([1]))); assert_ok!(Preimage::unrequest_preimage(RuntimeOrigin::signed(1), hashed([1]))); assert_ok!(Preimage::unnote_preimage(RuntimeOrigin::signed(2), hashed([1]))); - // Still have reserve from `vec[1; 3]`. - assert_eq!(Balances::reserved_balance(2), 5); - assert_eq!(Balances::free_balance(2), 95); + // Still have freeze from `vec[1; 3]`. + assert_eq!(Balances::balance_frozen(&(), &2), 5); }); } @@ -269,7 +270,7 @@ fn noted_preimage_use_correct_map() { assert_eq!(PreimageFor::::iter().count(), 8); // All are present - assert_eq!(StatusFor::::iter().count(), 8); + assert_eq!(RequestStatusFor::::iter().count(), 8); // Now start removing them again... for i in 0..7 { @@ -286,7 +287,7 @@ fn noted_preimage_use_correct_map() { assert_eq!(PreimageFor::::iter().count(), 0); // All are gone - assert_eq!(StatusFor::::iter().count(), 0); + assert_eq!(RequestStatusFor::::iter().count(), 0); }); } @@ -326,20 +327,20 @@ fn query_and_store_preimage_workflow() { Preimage::request(&hash); // It is requested thrice. assert!(matches!( - StatusFor::::get(&hash).unwrap(), + RequestStatusFor::::get(&hash).unwrap(), RequestStatus::Requested { count: 3, .. } )); // It can be realized and decoded correctly. assert_eq!(Preimage::realize::>(&bound).unwrap(), (data.clone(), Some(len))); assert!(matches!( - StatusFor::::get(&hash).unwrap(), + RequestStatusFor::::get(&hash).unwrap(), RequestStatus::Requested { count: 2, .. } )); // Dropping should unrequest. Preimage::drop(&bound); assert!(matches!( - StatusFor::::get(&hash).unwrap(), + RequestStatusFor::::get(&hash).unwrap(), RequestStatus::Requested { count: 1, .. } )); @@ -380,7 +381,7 @@ fn query_preimage_request_works() { assert!(::len(&hash).is_none()); assert_noop!(::fetch(&hash, None), DispatchError::Unavailable); // But there is only one entry in the map. - assert_eq!(StatusFor::::iter().count(), 1); + assert_eq!(RequestStatusFor::::iter().count(), 1); // Un-request the preimage. ::unrequest(&hash); @@ -391,7 +392,7 @@ fn query_preimage_request_works() { // It is not requested anymore. assert!(!::is_requested(&hash)); // And there is no entry in the map. - assert_eq!(StatusFor::::iter().count(), 0); + assert_eq!(RequestStatusFor::::iter().count(), 0); }); } @@ -412,7 +413,7 @@ fn query_preimage_hold_and_drop_work() { assert!(::is_requested(&legacy.hash())); // There are two values requested in total. - assert_eq!(StatusFor::::iter().count(), 2); + assert_eq!(RequestStatusFor::::iter().count(), 2); // Cleanup by dropping both. ::drop(&lookup); @@ -421,7 +422,7 @@ fn query_preimage_hold_and_drop_work() { assert!(!::is_requested(&legacy.hash())); // There are no values requested anymore. - assert_eq!(StatusFor::::iter().count(), 0); + assert_eq!(RequestStatusFor::::iter().count(), 0); }); } diff --git a/frame/support/src/traits/tokens/fungible/freeze.rs b/frame/support/src/traits/tokens/fungible/freeze.rs index 1ec3a5fadf555..fc26a3adc152e 100644 --- a/frame/support/src/traits/tokens/fungible/freeze.rs +++ b/frame/support/src/traits/tokens/fungible/freeze.rs @@ -18,7 +18,13 @@ //! The traits for putting freezes within a single fungible token class. use scale_info::TypeInfo; -use sp_runtime::DispatchResult; +use sp_arithmetic::{ + traits::{CheckedAdd, CheckedSub}, + ArithmeticError, +}; +use sp_runtime::{DispatchResult, TokenError}; + +use crate::ensure; /// Trait for inspecting a fungible asset which can be frozen. Freezing is essentially setting a /// minimum balance bellow which the total balance (inclusive of any funds placed on hold) may not @@ -65,4 +71,21 @@ pub trait Mutate: Inspect { /// Remove an existing lock. fn thaw(id: &Self::Id, who: &AccountId) -> DispatchResult; + + /// Reduce an existing lock. + fn decrease_frozen(id: &Self::Id, who: &AccountId, amount: Self::Balance) -> DispatchResult { + let a = Self::balance_frozen(id, who) + .checked_sub(&amount) + .ok_or(ArithmeticError::Underflow)?; + Self::set_freeze(id, who, a) + } + + /// Reduce an existing lock. + fn increase_frozen(id: &Self::Id, who: &AccountId, amount: Self::Balance) -> DispatchResult { + let a = Self::balance_frozen(id, who) + .checked_add(&amount) + .ok_or(ArithmeticError::Overflow)?; + ensure!(Self::balance_freezable(who) >= a, TokenError::FundsUnavailable); + Self::set_freeze(id, who, a) + } } diff --git a/frame/support/src/traits/tokens/fungible/mod.rs b/frame/support/src/traits/tokens/fungible/mod.rs index 2a299735801a4..4b6ce07b5d643 100644 --- a/frame/support/src/traits/tokens/fungible/mod.rs +++ b/frame/support/src/traits/tokens/fungible/mod.rs @@ -76,6 +76,40 @@ impl, R: Get, D: Convert> Co } } +pub struct FreezeMultiConsideration(sp_std::marker::PhantomData<(A, F, R, D)>); +impl, R: Get, D: Convert> Consideration + for FreezeMultiConsideration +{ + type Ticket = F::Balance; + fn update( + who: &A, + old: Option, + new: Option, + ) -> Result { + match (old, new) { + (None, Some(footprint)) => { + let new = D::convert(footprint); + F::increase_frozen(&R::get(), who, new)?; + Ok(new) + }, + (Some(old), Some(footprint)) => { + let new = D::convert(footprint); + if old > new { + F::decrease_frozen(&R::get(), who, old - new)?; + } else if new > old { + F::extend_freeze(&R::get(), who, new - old)?; + } + Ok(new) + }, + (Some(old), None) => { + F::decrease_frozen(&R::get(), who, old)?; + Ok(Default::default()) + }, + (None, None) => Ok(Default::default()), + } + } +} + pub struct HoldConsideration(sp_std::marker::PhantomData<(A, F, R, D)>); impl, R: Get, D: Convert> Consideration for HoldConsideration diff --git a/frame/support/src/traits/tokens/misc.rs b/frame/support/src/traits/tokens/misc.rs index 8ad3a9535fd0d..c3f8424b8ea8d 100644 --- a/frame/support/src/traits/tokens/misc.rs +++ b/frame/support/src/traits/tokens/misc.rs @@ -228,7 +228,16 @@ impl Balance for T { }