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

Introduce WeakBoundedVec, StorageTryAppend, and improve BoundedVec API #8842

Merged
8 commits merged into from
May 21, 2021

Conversation

gui1117
Copy link
Contributor

@gui1117 gui1117 commented May 17, 2021

Fix #8839

This PR does:

  • introduce WeakBoundedVec: it is exactly same as BoundedVec previous to this PR, i.e. the lenght is not enforced by force_from nor decode, but it warns when exceeded.
  • improve BoundedVec API: force_from is removed and decode only accept vec with correct size.
  • introduce StorageTryAppend: allow to specify that a storage value have a maximum bound. It is used to implement try_append on storage value, storage map and storage double map now.
    Previous to this PR, try_append was only implemented for BoundedVec explicitly. The StorageTryAppend allow to implement try_append on various types, such as BoundedVec and WeakBoundedVec.

@github-actions github-actions bot added the A0-please_review Pull request needs code review. label May 17, 2021
Copy link
Contributor

@kianenigma kianenigma left a comment

Choose a reason for hiding this comment

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

I think BoundedBTreeMap etc. also have similar confusion + use of unsafe.

@gui1117
Copy link
Contributor Author

gui1117 commented May 18, 2021

I think BoundedBTreeMap etc. also have similar confusion + use of unsafe.

ah yes, I changed it too now.

@gui1117 gui1117 added A3-in_progress Pull request is in progress. No review needed at this stage. and removed A0-please_review Pull request needs code review. labels May 20, 2021
@gui1117
Copy link
Contributor Author

gui1117 commented May 20, 2021

after discussion I will implement WeakBoundedVec and make BoundedVec API complete by changing Decode/Encode

/// Marker trait that will be implemented for types that support the `storage::try_append` api.
///
/// This trait is sealed.
pub trait StorageTryAppend<Item>: StorageDecodeLength + private::Sealed {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is a new trait in order to be able to implement try_append for both WeakBoudedVec and BoundedVec without too much code duplication.

@gui1117 gui1117 requested a review from kianenigma May 20, 2021 17:47
@gui1117 gui1117 added A0-please_review Pull request needs code review. B3-apinoteworthy C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit D5-nicetohaveaudit ⚠️ PR contains trivial changes to logic that should be properly reviewed. and removed A3-in_progress Pull request is in progress. No review needed at this stage. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit labels May 20, 2021
@gui1117 gui1117 changed the title Fix bounded vec doc and unsafe usage Introduce WeakBoundedVec and StorageTryAppend May 20, 2021
@gui1117 gui1117 changed the title Introduce WeakBoundedVec and StorageTryAppend Introduce WeakBoundedVec, StorageTryAppend, and improve BoundedVec API May 20, 2021
@shawntabrizi
Copy link
Member

thank you!

gui1117 and others added 2 commits May 21, 2021 10:16
Co-authored-by: Peter Goodspeed-Niklaus <coriolinus@users.noreply.github.com>
@gui1117
Copy link
Contributor Author

gui1117 commented May 21, 2021

bot merge

@ghost
Copy link

ghost commented May 21, 2021

Trying merge.

@ghost ghost merged commit 99164cb into master May 21, 2021
@ghost ghost deleted the gui-fix-bounded-vec branch May 21, 2021 09:07
nazar-pc pushed a commit to autonomys/substrate that referenced this pull request Aug 8, 2021
paritytech#8842)

* fix bounded vec doc and unsafe

* fix btree map and set and tests

* introduce weak_bounded_vec and StorageTryAppend

* fix tests and reorganize tests

* improve doc

* add doc

* Update frame/support/src/storage/weak_bounded_vec.rs

Co-authored-by: Peter Goodspeed-Niklaus <coriolinus@users.noreply.github.com>

* fix inner doc

Co-authored-by: Peter Goodspeed-Niklaus <coriolinus@users.noreply.github.com>
This pull request was closed.
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. C1-low PR touches the given topic and has a low impact on builders. D5-nicetohaveaudit ⚠️ PR contains trivial changes to logic that should be properly reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

inconsistency in BoundedVec type
5 participants