Skip to content

Conversation

@bkontur
Copy link
Contributor

@bkontur bkontur commented Apr 22, 2024

This PR:

  • moves validate_xcm_nesting from XcmpQueue into the VersionedXcm
  • adds validate_xcm_nesting to the ParentAsUmp
  • adds validate_xcm_nesting to the ChildParachainRouter

Based on discussion here and/or here and/or here

Question/TODO

  • To the comment - Why was validate_xcm_nesting added just to the XcmpQueue router and nowhere else? What kind of problem MAX_XCM_DECODE_DEPTH is solving? (see comment)

@bkontur bkontur added R0-no-crate-publish-required The change does not require any crates to be re-published. T6-XCM This PR/Issue is related to XCM. labels Apr 22, 2024
@bkontur bkontur requested a review from a team as a code owner April 22, 2024 09:03
@bkontur bkontur self-assigned this Apr 22, 2024
@acatangiu
Copy link
Contributor

moves validate_xcm_nesting from XcmpQueue into the VersionedXcm

this is not accurate anymore, it is also done inside XcmpQueue validations

Copy link
Contributor

@acatangiu acatangiu left a comment

Choose a reason for hiding this comment

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

This is good!

@bkontur bkontur enabled auto-merge April 22, 2024 15:17
@bkontur bkontur disabled auto-merge April 22, 2024 15:18
@bkontur bkontur enabled auto-merge April 22, 2024 15:43
@bkontur bkontur added this pull request to the merge queue Apr 23, 2024
Merged via the queue into master with commit 7f1646e Apr 23, 2024
@bkontur bkontur deleted the bko-validate-xcm-nesting branch April 23, 2024 09:07
github-merge-queue bot pushed a commit that referenced this pull request Apr 23, 2024
…Xcm` router (for testing purposes) (#4186)

This PR:
- adds `EnsureDecodableXcm` (testing) router that attempts to *encode*
and *decode* passed XCM `message` to ensure that the receiving side will
be able to decode, at least with the same XCM version.
- fixes `pallet_xcm` / `pallet_xcm_benchmarks` assets data generation

Relates to investigation of
https://substrate.stackexchange.com/questions/11288 and missing fix
#2129 which did not get
into the fellows 1.1.X release.

## Questions/TODOs

- [x] fix XCM benchmarks, which produces undecodable data - new router
catched at least two cases
  - `BoundedVec exceeds its limit`
  - `Fungible asset of zero amount is not allowed`  
- [x] do we need to add `sort` to the `prepend_with` as we did for
reanchor [here](#2129)?
@serban300 (**created separate/follow-up PR**:
#4235)
- [x] We added decoding check to `XcmpQueue` -> `validate_xcm_nesting`,
why not to added to the `ParentAsUmp` or `ChildParachainRouter`?
@franciscoaguirre (**created separate/follow-up PR**:
#4236)
- [ ] `SendController::send_blob` replace `VersionedXcm::<()>::decode(`
with `VersionedXcm::<()>::decode_with_depth_limit(MAX_XCM_DECODE_DEPTH,
data)` ?

---------

Co-authored-by: Adrian Catangiu <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

R0-no-crate-publish-required The change does not require any crates to be re-published. T6-XCM This PR/Issue is related to XCM.

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

5 participants