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

XCM: Limit the max number of assets weighable in MultiAssets #6786

Merged
merged 6 commits into from
Feb 28, 2023
Merged

XCM: Limit the max number of assets weighable in MultiAssets #6786

merged 6 commits into from
Feb 28, 2023

Conversation

KiChjang
Copy link
Contributor

To prevent unreasonably high count values from overweighing related XCM instructions, we should have an upper limit on how many assets we would weight during the weight calculation of XCM.

@KiChjang KiChjang added 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. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit. T6-XCM This PR/Issue is related to XCM. labels Feb 25, 2023
gavofyork
gavofyork approved these changes Feb 25, 2023
View reviewed changes
Copy link
Member

@gavofyork gavofyork left a comment

Choose a reason for hiding this comment

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

After my suggestions, this seems ok.

@xlc
Copy link
Contributor

xlc commented Feb 25, 2023

Some tests will be helpful

KiChjang and others added 2 commits February 25, 2023 07:55
@KiChjang @gavofyork
Apply suggestions from code review
96719e9 
Co-authored-by: Gavin Wood <gavin@parity.io>
@KiChjang
Add tests
308c780
@paritytech-ci paritytech-ci requested a review from a team February 25, 2023 21:12
@paritytech-ci paritytech-ci requested a review from a team February 26, 2023 06:18
@paritytech-ci paritytech-ci requested a review from a team February 26, 2023 06:19
@paritytech-ci paritytech-ci requested a review from a team February 26, 2023 06:20
@gavofyork
Copy link
Member

Looks fine - will need another review though.

@gavofyork gavofyork merged commit 9d74985 into master Feb 28, 2023
@gavofyork gavofyork deleted the kckyeung/limit-max-all-counted branch February 28, 2023 09:12
@kiltbot kiltbot mentioned this pull request Apr 3, 2023
Closed
@librelois librelois mentioned this pull request Apr 6, 2023
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@gavofyork gavofyork gavofyork approved these changes

@bkontur bkontur bkontur approved these changes

@shawntabrizi shawntabrizi Awaiting requested review from shawntabrizi

Assignees
No one assigned
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. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit. T6-XCM This PR/Issue is related to XCM.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@KiChjang @xlc @gavofyork @bkontur