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

Nest errors in pallet-xcm #7730

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Conversation

franciscoaguirre
Copy link
Contributor

Errors in pallet-xcm are very bad. You execute something and all you get is LocalExecutionIncomplete. That's it, nothing else. You don't know if you had insufficient balance, if your account didn't convert, nothing.

One thing we can do to improve this is to nest the error from the executor inside of LocalExecutionIncomplete. That way, we can provide more information in the event of an error. Errors in FRAME are 4 bytes long and right now we're only using one of them. By using more bytes we can have nested enums that give the end user (or application) a better error.

Still, in order to make this really good, we would need to rework the errors themselves. For example, FailedToTransactAsset takes a string that we can't encode in less than 4 bytes. So we need to change that or convert that to a relevant enum that we can provide in the error.

@franciscoaguirre franciscoaguirre requested a review from a team as a code owner February 26, 2025 14:23
@franciscoaguirre franciscoaguirre added the T6-XCM This PR/Issue is related to XCM. label Feb 26, 2025
@franciscoaguirre franciscoaguirre marked this pull request as draft February 26, 2025 14:23
@paritytech-workflow-stopper
Copy link

All GitHub workflows were cancelled due to failure one of the required jobs.
Failed workflow url: https://github.com/paritytech/polkadot-sdk/actions/runs/13546080272
Failed job name: fmt

Comment on lines 591 to +592
#[codec(index = 24)]
LocalExecutionIncomplete,
LocalExecutionIncomplete(ExecutionError),
Copy link
Contributor

Choose a reason for hiding this comment

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

This should stay stable, no change, otherwise breaks PJS decoding.

Just use new (index=25) error variant.

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.

3 participants