Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pallet-xcm: Deprecate execute and send in favor of execute_blob and send_blob #3749

Merged
merged 53 commits into from
Mar 27, 2024

Conversation

franciscoaguirre
Copy link
Contributor

@franciscoaguirre franciscoaguirre commented Mar 19, 2024

execute and send try to decode the xcm in the parameters before reaching the filter line.
The new extrinsics decode only after the filter line.
These should be used instead of the old ones.

TODO

This would make writing XCMs in PJS Apps more difficult, but here's the fix for that: polkadot-js/apps#10350.
Already deployed! https://polkadot.js.org/apps/#/utilities/xcm

Supersedes #1798

@franciscoaguirre
Copy link
Contributor Author

@pgherveou Could you take a look at ExecuteController and SendController and make sure the changes make sense? I don't want to break anything on the contracts side

polkadot/xcm/xcm-builder/src/controller.rs Outdated Show resolved Hide resolved
polkadot/xcm/xcm-builder/src/controller.rs Outdated Show resolved Hide resolved
polkadot/xcm/pallet-xcm/src/lib.rs Outdated Show resolved Hide resolved
polkadot/xcm/pallet-xcm/src/lib.rs Outdated Show resolved Hide resolved
@pgherveou
Copy link
Contributor

@pgherveou Could you take a look at ExecuteController and SendController and make sure the changes make sense? I don't want to break anything on the contracts side

this should work, I am not too sure where your new MaxXcmEncodedSize is defined though. If that helps we can also not add the legacy function to the traits since at that point the contract implementation is still unstable and ok to change.

@franciscoaguirre
Copy link
Contributor Author

I'll put only the new function in the traits then and ping you later to check everything would be fine with contracts.

@franciscoaguirre franciscoaguirre added the T6-XCM This PR/Issue is related to XCM. label Mar 20, 2024
@acatangiu
Copy link
Contributor

New PJS utility to allow same UX for these new extrinsics is already available, thanks to @serban300 👍
https://polkadot.js.org/apps/#/utilities/xcm

@franciscoaguirre franciscoaguirre marked this pull request as ready for review March 20, 2024 16:52
@franciscoaguirre franciscoaguirre requested a review from a team as a code owner March 20, 2024 16:52
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.

looks good! waiting for TODOs to be finished before approving.

polkadot/xcm/pallet-xcm/src/lib.rs Outdated Show resolved Hide resolved
polkadot/runtime/westend/src/weights/pallet_xcm.rs Outdated Show resolved Hide resolved
polkadot/xcm/pallet-xcm/src/lib.rs Outdated Show resolved Hide resolved
polkadot/xcm/pallet-xcm/src/lib.rs Outdated Show resolved Hide resolved
polkadot/xcm/pallet-xcm/src/lib.rs Outdated Show resolved Hide resolved
polkadot/xcm/pallet-xcm/src/lib.rs Outdated Show resolved Hide resolved
@franciscoaguirre franciscoaguirre requested a review from athei as a code owner March 20, 2024 20:00
@pgherveou
Copy link
Contributor

pgherveou commented Mar 21, 2024

Can you tell a bit more about the motivations

execute and send try to decode the xcm in the parameters before reaching the filter line.

The new interface is a bit awkward, the caller needs to create an XCM, then encode it, so that it's later decoded again and processed inside the pallet.

Couldn't we just get away with an extra encoded_size() check on the passed message to see if it fits the constraint?

@franciscoaguirre
Copy link
Contributor Author

franciscoaguirre commented Mar 21, 2024

You only encode and then decode because you're writing the code in rust, but normal users would already have to encode their XCM.
Taking it encoded sounds like a good pattern, and it only requires .encode(), and an optional .try_into() and error handling if it's too big, in rust code.

@franciscoaguirre
Copy link
Contributor Author

bot help

@acatangiu acatangiu enabled auto-merge March 25, 2024 17:46
@acatangiu
Copy link
Contributor

@pgherveou @bkontur this is ready, waiting for final review

@acatangiu acatangiu added this pull request to the merge queue Mar 27, 2024
Merged via the queue into master with commit feee773 Mar 27, 2024
128 of 134 checks passed
@acatangiu acatangiu deleted the xcm-execute-blob branch March 27, 2024 08:55
dharjeezy pushed a commit to dharjeezy/polkadot-sdk that referenced this pull request Apr 9, 2024
… and `send_blob` (paritytech#3749)

`execute` and `send` try to decode the xcm in the parameters before
reaching the filter line.
The new extrinsics decode only after the filter line.
These should be used instead of the old ones.

## TODO
- [x] Tests
- [x] Generate weights
- [x] Deprecation issue ->
paritytech#3771
- [x] PRDoc
- [x] Handle error in pallet-contracts

This would make writing XCMs in PJS Apps more difficult, but here's the
fix for that: polkadot-js/apps#10350.
Already deployed! https://polkadot.js.org/apps/#/utilities/xcm

Supersedes paritytech#1798

---------

Co-authored-by: PG Herveou <[email protected]>
Co-authored-by: command-bot <>
Co-authored-by: Adrian Catangiu <[email protected]>
franciscoaguirre added a commit that referenced this pull request Apr 24, 2024
…te_blob` and `send_blob` (#3749)"

This reverts commit feee773.
github-merge-queue bot pushed a commit that referenced this pull request Apr 24, 2024
Revert "pallet-xcm: Deprecate `execute` and `send` in favor of
`execute_blob` and `send_blob` (#3749)"

This reverts commit feee773.

---------

Co-authored-by: Adrian Catangiu <[email protected]>
Co-authored-by: Javier Bullrich <[email protected]>
franciscoaguirre added a commit that referenced this pull request Apr 25, 2024
Revert "pallet-xcm: Deprecate `execute` and `send` in favor of
`execute_blob` and `send_blob` (#3749)"

This reverts commit feee773.

---------

Co-authored-by: Adrian Catangiu <[email protected]>
Co-authored-by: Javier Bullrich <[email protected]>
acatangiu added a commit that referenced this pull request Apr 29, 2024
Revert "pallet-xcm: Deprecate `execute` and `send` in favor of
`execute_blob` and `send_blob` (#3749)"

This reverts commit feee773.

---------

---------

Co-authored-by: Adrian Catangiu <[email protected]>
Co-authored-by: Javier Bullrich <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T6-XCM This PR/Issue is related to XCM.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants