This repository was archived by the owner on Nov 15, 2023. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Simple MaxBoundedLen Implementations
#8793
Merged
Merged
Changes from 9 commits
Commits
Show all changes
60 commits
Select commit
Hold shift + click to select a range
0de4775
implement max_values + storages info
gui1117 e7c14fb
Merge remote-tracking branch 'origin/master' into gui-max_values-macro
gui1117 42c4f48
some formatting + doc
gui1117 d1032a3
sudo sanity check
shawntabrizi f2582d5
timestamp
shawntabrizi bf354b0
assets (not working)
shawntabrizi 38cecd6
fix assets
shawntabrizi 06b872a
impl for proxy
shawntabrizi 43edcac
update balances
shawntabrizi 0ca09ee
Merge remote-tracking branch 'origin/master' into gui-max_values-macro
gui1117 ada719b
rename StoragesInfo -> PalletStorageInfo
gui1117 341d3f5
merge both StorageInfoTrait and PalletStorageInfo
gui1117 5521953
Update frame/support/procedural/src/storage/parse.rs
gui1117 02b0cbb
Update frame/support/procedural/src/storage/storage_struct.rs
gui1117 999640b
Fix max_size using hasher information
gui1117 a7909d7
fix tests
gui1117 e5964c0
fix ui tests
gui1117 2001d32
Move `MaxBoundedLen` into its own crate (#8814)
coriolinus 566425d
nits
shawntabrizi bb1d521
Merge branch 'master' into shawntabrizi-maxboundedlen-start
shawntabrizi 1a398cb
fix compile
shawntabrizi 3685d3c
line width
shawntabrizi 15918ff
Merge commit '8d02bb0bfc6136f6a3c805db19f51e43090a7cd4' into gui-max_…
gui1117 4fc7829
Merge remote-tracking branch 'origin/master' into gui-max_values-macro
gui1117 9c42372
Merge remote-tracking branch 'origin/gui-max_values-macro' into shawn…
gui1117 e8841f5
fix max-values-macro merge
gui1117 c12be40
Merge remote-tracking branch 'origin/master' into shawntabrizi-maxbou…
gui1117 1a3e03e
Add some derive, needed for test and other purpose
gui1117 9a45be6
Merge remote-tracking branch 'origin/master' into shawntabrizi-maxbou…
gui1117 5af7c4c
use weak bounded vec in some cases
gui1117 0e50fe0
Merge branch 'master' into shawntabrizi-maxboundedlen-start
shawntabrizi 7683f73
Update lib.rs
shawntabrizi 0c09750
move max-encoded-len crate
shawntabrizi d93b503
fix
shawntabrizi 73623e9
remove app crypto for now
shawntabrizi db766a8
Merge branch 'master' into shawntabrizi-maxboundedlen-start
shawntabrizi 1bb36fd
width
shawntabrizi f1e3ced
Revert "remove app crypto for now"
shawntabrizi 562f6b0
Merge branch 'master' into shawntabrizi-maxboundedlen-start
shawntabrizi 747e613
unused variable
shawntabrizi 7af3dcc
more unused variables
shawntabrizi 0dd6403
more fixes
shawntabrizi b7620ef
Add #[max_encoded_len_crate(...)] helper attribute
coriolinus 8a812bb
fix a ui test
coriolinus 04375ac
use #[max_encoded_len_crate(...)] helper in app_crypto
coriolinus d5f3ee6
remove max_encoded_len import where not necessary
coriolinus 8971223
Merge remote-tracking branch 'origin/master' into shawntabrizi-maxbou…
coriolinus 016cc16
update lockfile
coriolinus 478c742
Merge branch 'master' into shawntabrizi-maxboundedlen-start
shawntabrizi e2a16cb
fix ui test
shawntabrizi b71b2a7
ui
shawntabrizi 3acfa75
newline
shawntabrizi 62d6410
Merge branch 'master' into shawntabrizi-maxboundedlen-start
shawntabrizi cf92608
fix merge
shawntabrizi 2f739cd
try fix ui again
shawntabrizi 54839b7
Update max-encoded-len/derive/src/lib.rs
shawntabrizi 1fd92a4
extract generate_crate_access_2018
shawntabrizi f41c90a
Merge branch 'shawntabrizi-maxboundedlen-start' of https://github.com…
shawntabrizi f6d5db3
Update lib.rs
shawntabrizi c329b06
compiler isnt smart enough
shawntabrizi File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or 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 hidden or 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 hidden or 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 |
|---|---|---|
|
|
@@ -159,9 +159,9 @@ use sp_std::prelude::*; | |
| use sp_std::{cmp, result, mem, fmt::Debug, ops::BitOr}; | ||
| use codec::{Codec, Encode, Decode}; | ||
| use frame_support::{ | ||
| ensure, | ||
| ensure, BoundedVec, | ||
| traits::{ | ||
| Currency, OnUnbalanced, TryDrop, StoredMap, | ||
| Currency, OnUnbalanced, TryDrop, StoredMap, MaxEncodedLen, | ||
| WithdrawReasons, LockIdentifier, LockableCurrency, ExistenceRequirement, | ||
| Imbalance, SignedImbalance, ReservableCurrency, Get, ExistenceRequirement::KeepAlive, | ||
| ExistenceRequirement::AllowDeath, | ||
|
|
@@ -193,7 +193,7 @@ pub mod pallet { | |
| pub trait Config<I: 'static = ()>: frame_system::Config { | ||
| /// The balance of an account. | ||
| type Balance: Parameter + Member + AtLeast32BitUnsigned + Codec + Default + Copy + | ||
| MaybeSerializeDeserialize + Debug; | ||
| MaybeSerializeDeserialize + Debug + MaxEncodedLen; | ||
|
|
||
| /// Handler for the unbalanced reduction when removing a dust account. | ||
| type DustRemoval: OnUnbalanced<NegativeImbalance<Self, I>>; | ||
|
|
@@ -218,6 +218,7 @@ pub mod pallet { | |
|
|
||
| #[pallet::pallet] | ||
| #[pallet::generate_store(pub(super) trait Store)] | ||
| #[pallet::generate_storages_info] | ||
| pub struct Pallet<T, I=()>(PhantomData<(T, I)>); | ||
|
|
||
| #[pallet::hooks] | ||
|
|
@@ -428,7 +429,9 @@ pub mod pallet { | |
| Blake2_128Concat, | ||
| T::AccountId, | ||
| AccountData<T::Balance>, | ||
| ValueQuery | ||
| ValueQuery, | ||
| GetDefault, | ||
| ConstU32<300_000>, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess this will need to be configurable in the future, because chains can have very different number of account |
||
| >; | ||
|
|
||
| /// Any liquidity locks on some account balances. | ||
|
|
@@ -439,8 +442,10 @@ pub mod pallet { | |
| _, | ||
| Blake2_128Concat, | ||
| T::AccountId, | ||
| Vec<BalanceLock<T::Balance>>, | ||
| ValueQuery | ||
| BoundedVec<BalanceLock<T::Balance>, T::MaxLocks>, | ||
| ValueQuery, | ||
| GetDefault, | ||
| ConstU32<300_000>, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same, it will need to be configurable |
||
| >; | ||
|
|
||
| /// Storage version of the pallet. | ||
|
|
@@ -517,7 +522,7 @@ impl<T: Config<I>, I: 'static> GenesisConfig<T, I> { | |
| } | ||
|
|
||
| /// Simplified reasons for withdrawing balance. | ||
| #[derive(Encode, Decode, Clone, Copy, PartialEq, Eq, RuntimeDebug)] | ||
| #[derive(Encode, Decode, Clone, Copy, PartialEq, Eq, RuntimeDebug, MaxEncodedLen)] | ||
| pub enum Reasons { | ||
| /// Paying system transaction fees. | ||
| Fee = 0, | ||
|
|
@@ -549,7 +554,7 @@ impl BitOr for Reasons { | |
|
|
||
| /// A single lock on a balance. There can be many of these on an account and they "overlap", so the | ||
| /// same balance is frozen by multiple locks. | ||
| #[derive(Encode, Decode, Clone, PartialEq, Eq, RuntimeDebug)] | ||
| #[derive(Encode, Decode, Clone, PartialEq, Eq, RuntimeDebug, MaxEncodedLen)] | ||
| pub struct BalanceLock<Balance> { | ||
| /// An identifier for this lock. Only one lock may be in existence for each identifier. | ||
| pub id: LockIdentifier, | ||
|
|
@@ -560,7 +565,7 @@ pub struct BalanceLock<Balance> { | |
| } | ||
|
|
||
| /// All balance information for an account. | ||
| #[derive(Encode, Decode, Clone, PartialEq, Eq, Default, RuntimeDebug)] | ||
| #[derive(Encode, Decode, Clone, PartialEq, Eq, Default, RuntimeDebug, MaxEncodedLen)] | ||
| pub struct AccountData<Balance> { | ||
| /// Non-reserved part of the balance. There may still be restrictions on this, but it is the | ||
| /// total pool what may in principle be transferred, reserved and used for tipping. | ||
|
|
@@ -606,7 +611,7 @@ impl<Balance: Saturating + Copy + Ord> AccountData<Balance> { | |
| // A value placed in storage that represents the current version of the Balances storage. | ||
| // This value is used by the `on_runtime_upgrade` logic to determine whether we run | ||
| // storage migration logic. This should match directly with the semantic versions of the Rust crate. | ||
| #[derive(Encode, Decode, Clone, Copy, PartialEq, Eq, RuntimeDebug)] | ||
| #[derive(Encode, Decode, Clone, Copy, PartialEq, Eq, RuntimeDebug, MaxEncodedLen)] | ||
| enum Releases { | ||
| V1_0_0, | ||
| V2_0_0, | ||
|
|
@@ -826,48 +831,52 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> { | |
|
|
||
| /// Update the account entry for `who`, given the locks. | ||
| fn update_locks(who: &T::AccountId, locks: &[BalanceLock<T::Balance>]) { | ||
| if locks.len() as u32 > T::MaxLocks::get() { | ||
| log::warn!( | ||
| target: "runtime::balances", | ||
| "Warning: A user has more currency locks than expected. \ | ||
| A runtime configuration adjustment may be needed." | ||
| ); | ||
| } | ||
| // No way this can fail since we do not alter the existential balances. | ||
| let res = Self::mutate_account(who, |b| { | ||
| b.misc_frozen = Zero::zero(); | ||
| b.fee_frozen = Zero::zero(); | ||
| for l in locks.iter() { | ||
| if l.reasons == Reasons::All || l.reasons == Reasons::Misc { | ||
| b.misc_frozen = b.misc_frozen.max(l.amount); | ||
| unsafe { | ||
| let bounded_locks = BoundedVec::<_, T::MaxLocks>::force_from(locks.to_vec(), Some("Balances Update Locks")); | ||
shawntabrizi marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| if locks.len() as u32 > T::MaxLocks::get() { | ||
| log::warn!( | ||
| target: "runtime::balances", | ||
| "Warning: A user has more currency locks than expected. \ | ||
| A runtime configuration adjustment may be needed." | ||
| ); | ||
| } | ||
| // No way this can fail since we do not alter the existential balances. | ||
| let res = Self::mutate_account(who, |b| { | ||
| b.misc_frozen = Zero::zero(); | ||
| b.fee_frozen = Zero::zero(); | ||
| for l in locks.iter() { | ||
| if l.reasons == Reasons::All || l.reasons == Reasons::Misc { | ||
| b.misc_frozen = b.misc_frozen.max(l.amount); | ||
| } | ||
| if l.reasons == Reasons::All || l.reasons == Reasons::Fee { | ||
| b.fee_frozen = b.fee_frozen.max(l.amount); | ||
| } | ||
| } | ||
| if l.reasons == Reasons::All || l.reasons == Reasons::Fee { | ||
| b.fee_frozen = b.fee_frozen.max(l.amount); | ||
| }); | ||
| debug_assert!(res.is_ok()); | ||
|
|
||
| let existed = Locks::<T, I>::contains_key(who); | ||
| if locks.is_empty() { | ||
| Locks::<T, I>::remove(who); | ||
| if existed { | ||
| // TODO: use Locks::<T, I>::hashed_key | ||
| // https://github.com/paritytech/substrate/issues/4969 | ||
| system::Pallet::<T>::dec_consumers(who); | ||
| } | ||
| } | ||
| }); | ||
| debug_assert!(res.is_ok()); | ||
|
|
||
| let existed = Locks::<T, I>::contains_key(who); | ||
| if locks.is_empty() { | ||
| Locks::<T, I>::remove(who); | ||
| if existed { | ||
| // TODO: use Locks::<T, I>::hashed_key | ||
| // https://github.com/paritytech/substrate/issues/4969 | ||
| system::Pallet::<T>::dec_consumers(who); | ||
| } | ||
| } else { | ||
| Locks::<T, I>::insert(who, locks); | ||
| if !existed { | ||
| if system::Pallet::<T>::inc_consumers(who).is_err() { | ||
| // No providers for the locks. This is impossible under normal circumstances | ||
| // since the funds that are under the lock will themselves be stored in the | ||
| // account and therefore will need a reference. | ||
| log::warn!( | ||
| target: "runtime::balances", | ||
| "Warning: Attempt to introduce lock consumer reference, yet no providers. \ | ||
| This is unexpected but should be safe." | ||
| ); | ||
| } else { | ||
| Locks::<T, I>::insert(who, bounded_locks); | ||
| if !existed { | ||
| if system::Pallet::<T>::inc_consumers(who).is_err() { | ||
| // No providers for the locks. This is impossible under normal circumstances | ||
| // since the funds that are under the lock will themselves be stored in the | ||
| // account and therefore will need a reference. | ||
| log::warn!( | ||
| target: "runtime::balances", | ||
| "Warning: Attempt to introduce lock consumer reference, yet no providers. \ | ||
| This is unexpected but should be safe." | ||
| ); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
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.
I think in the future (or now) we should say how the associated types can be modified.
For instance here "T::StringLimit" is used for stored item, thus changing the limit requires a migration of those items.
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.
Only when the value decreases. Increases should be backwards compatible.