BoundedVec for AccountLocks #8580
Conversation
kianenigma
added
A0-please_review
frame/balances/src/lib.rs
Outdated
| @@ -859,7 +852,11 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> { | |||
| system::Pallet::<T>::dec_consumers(who); | |||
| } | |||
| } else { | |||
| Locks::<T, I>::insert(who, locks); | |||
| let bounded_locks = <BoundedVec<BalanceLock<T::Balance>, T::MaxLocks>>::force_from( | |||
There was a problem hiding this comment.
Haha we need to fix this at some point, but yeah this looks good.
There was a problem hiding this comment.
What alternative do you have in mind?
I tried to replicate the old code, and the old code was not strict, so I added this force_from.
There was a problem hiding this comment.
yeah the way this API is written historically is not strict. ideally something like set_lock would return a result, and we would check that wherever set_lock is used
But that is beyond the scope here.
|
Another example actually doing the check? |
kianenigma
added
the
D2-notlive 💤
frame/elections/src/mock.rs
Outdated
| @@ -257,7 +257,7 @@ pub(crate) fn balances(who: &u64) -> (u64, u64) { | |||
| } | |||
|
|
|||
| pub(crate) fn locks(who: &u64) -> Vec<u64> { | |||
| Balances::locks(who).iter().map(|l| l.amount).collect::<Vec<u64>>() | |||
| Balances::locks(who).inner().iter().map(|l| l.amount).collect::<Vec<u64>>() | |||
There was a problem hiding this comment.
This can be improved, I should be able to use the iter directly defined on BoundedVec
…rate into kiz-bounded-vec-example
|
Added an example for collective as well. The changes are quite small, lets merge this into the base PR? |
| proposals.len() <= T::MaxProposals::get() as usize, | ||
| Error::<T, I>::TooManyProposals | ||
| ); | ||
| proposals.try_push(proposal_hash).map_err(|_| Error::<T, I>::TooManyProposals)?; |
| } | ||
| impl pallet_balances::Config for Test { | ||
| type MaxLocks = (); | ||
| type MaxLocks = MaxLocks; |
There was a problem hiding this comment.
is there such a magic trick that () would be a boundless vec?
There was a problem hiding this comment.
No in this case it is the opposite. Default for () is 0 so it will be a non-usable vec.
* prototype for shawn * Clean and document it * Add more docs * Move imports * Some changes for easier compat. * revert exmaple pallet * rename * BoundedVec for AccountLocks (#8580) * Example with balances * Fix tests * Make it indexable * fix * Fix tests * fix test * Fix collective as well * Fix test * Update frame/support/src/storage/mod.rs Co-authored-by: Peter Goodspeed-Niklaus <coriolinus@users.noreply.github.com> * Repot and add for value * Add for map and double map * Final touches. * Update frame/support/src/storage/bounded_vec.rs Co-authored-by: Guillaume Thiolliere <gui.thiolliere@gmail.com> * Add a few more tests * Add import Co-authored-by: Peter Goodspeed-Niklaus <coriolinus@users.noreply.github.com> Co-authored-by: Guillaume Thiolliere <gui.thiolliere@gmail.com>
Example of using: #8556