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

Improve BoundedVec API (extracted from #10195)#10656

Merged
gavofyork merged 6 commits intomasterfrom
shawntabrizi-gav-breakdown-boundedvec
Jan 17, 2022
Merged

Improve BoundedVec API (extracted from #10195)#10656
gavofyork merged 6 commits intomasterfrom
shawntabrizi-gav-breakdown-boundedvec

Conversation

@shawntabrizi
Copy link
Copy Markdown
Member

This PR improves the BoundedVec API, porting some existing features of the standard Vec and adding some new features such as force_push, slide, etc... (see code for details)

Trying to simplify the diff in #10195, original author is @gavofyork

@github-actions github-actions Bot added the A0-please_review Pull request needs code review. label Jan 14, 2022
@shawntabrizi shawntabrizi added B0-silent Changes should not be mentioned in any release notes 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. labels Jan 14, 2022
@shawntabrizi shawntabrizi added D9-needsaudit 👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited and removed D5-nicetohaveaudit ⚠️ PR contains trivial changes to logic that should be properly reviewed. labels Jan 14, 2022
Comment thread frame/support/src/storage/bounded_vec.rs
return true
}

debug_assert!(false, "all noop conditions should have been covered above");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Wonder if we can try fuzzing this function to ensure that this assertion is never hit.

Copy link
Copy Markdown
Member

@gavofyork gavofyork Jan 17, 2022

Choose a reason for hiding this comment

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

no need to fuzz it; you can just prove it logically.

Comment thread frame/support/src/storage/bounded_vec.rs Outdated
Copy link
Copy Markdown
Contributor

@KiChjang KiChjang left a comment

Choose a reason for hiding this comment

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

Don't see major issues; I think it does need an audit though.

Co-authored-by: Keith Yeung <kungfukeith11@gmail.com>
@gavofyork gavofyork merged commit fe3f379 into master Jan 17, 2022
@gavofyork gavofyork deleted the shawntabrizi-gav-breakdown-boundedvec branch January 17, 2022 09:28
@jakoblell jakoblell added D1-audited 👍 PR contains changes to fund-managing logic that has been properly reviewed and externally audited and removed D9-needsaudit 👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited labels Jan 19, 2022
grishasobol pushed a commit to gear-tech/substrate that referenced this pull request Mar 28, 2022
…10656)

* Gav wrote this code in pull paritytech#10195. Extracting to simplify that PR.

* fix potential panics

* prevent panics in slide

* update doc

* fmt

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

Co-authored-by: Keith Yeung <kungfukeith11@gmail.com>

Co-authored-by: Gav Wood <gavin@parity.io>
Co-authored-by: Keith Yeung <kungfukeith11@gmail.com>
ark0f pushed a commit to gear-tech/substrate that referenced this pull request Feb 27, 2023
…10656)

* Gav wrote this code in pull paritytech#10195. Extracting to simplify that PR.

* fix potential panics

* prevent panics in slide

* update doc

* fmt

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

Co-authored-by: Keith Yeung <kungfukeith11@gmail.com>

Co-authored-by: Gav Wood <gavin@parity.io>
Co-authored-by: Keith Yeung <kungfukeith11@gmail.com>
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. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D1-audited 👍 PR contains changes to fund-managing logic that has been properly reviewed and externally audited

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants