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

Enforce max length for MultiAssets for some instructions #7615

Closed
wants to merge 1 commit into from
Closed

Enforce max length for MultiAssets for some instructions #7615

wants to merge 1 commit into from

Conversation

franciscoaguirre
Copy link
Contributor

@franciscoaguirre franciscoaguirre commented Aug 14, 2023 •
edited
Loading

MultiAssets had a recommended MAX_ITEMS_IN_MULTIASSETS set to 20 but it was not being enforced.

This PR enforces this check on the following instructions:

  • WithdrawAsset
  • ClaimAsset
  • ReserveAssetDeposited
  • ReceiveTeleportedAsset

@franciscoaguirre franciscoaguirre added A0-please_review Pull request needs code review. 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. F3-breaks_API This PR changes public API; next release should be major. B1-note_worthy Changes should be noted in the release notes labels Aug 14, 2023
@franciscoaguirre franciscoaguirre added T6-XCM This PR/Issue is related to XCM. T2-API This PR/Issue is related to APIs. D5-nicetohaveaudit ⚠️ PR contains trivial changes to logic that should be properly reviewed. and removed T6-XCM This PR/Issue is related to XCM. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit. labels Aug 14, 2023
KiChjang
KiChjang reviewed Aug 14, 2023
View reviewed changes
xcm/src/v3/multiasset.rs Outdated Show resolved Hide resolved
@KiChjang KiChjang added T6-XCM This PR/Issue is related to XCM. T1-runtime This PR/Issue is related to the topic “runtime”. and removed T2-API This PR/Issue is related to APIs. T6-XCM This PR/Issue is related to XCM. labels Aug 14, 2023
@franciscoaguirre franciscoaguirre force-pushed the cisco-max-items-in-multiasset-enforcement branch from 5b46850 to c6ea9a9 Compare August 17, 2023 15:37
@franciscoaguirre franciscoaguirre added 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. T1-runtime This PR/Issue is related to the topic “runtime”. and removed D5-nicetohaveaudit ⚠️ PR contains trivial changes to logic that should be properly reviewed. F3-breaks_API This PR changes public API; next release should be major. T1-runtime This PR/Issue is related to the topic “runtime”. T6-XCM This PR/Issue is related to XCM. labels Aug 17, 2023
@franciscoaguirre
Copy link
Contributor Author

bot rebase

@paritytech-processbot
Copy link

Branch is already up-to-date

@franciscoaguirre franciscoaguirre changed the title Enforce max length for MultiAssets Enforce max length for MultiAssets for some instructions Aug 17, 2023
xlc
xlc reviewed Aug 18, 2023
View reviewed changes
xcm/src/v3/multiasset.rs
@@ -508,7 +508,7 @@ pub struct MultiAssets(Vec<MultiAsset>);

/// Maximum number of items we expect in a single `MultiAssets` value. Note this is not (yet)
Copy link
Contributor

Choose a reason for hiding this comment

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

comments needs update

@gavofyork
Copy link
Member

Not needed any more after #7641 .

@gavofyork gavofyork closed this Aug 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@xlc xlc xlc left review comments

@KiChjang KiChjang KiChjang left review comments

@vstam1 vstam1 Awaiting requested review from vstam1

Assignees
No one assigned
Labels
A0-please_review Pull request needs code review. B1-note_worthy Changes should be noted in the 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. T1-runtime This PR/Issue is related to the topic “runtime”.
Projects
None yet
Milestone
No milestone
Development

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