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: added useful error logs (#2408) #4982

Merged
merged 46 commits into from
Oct 15, 2024

Conversation

ayevbeosa
Copy link
Contributor

@ayevbeosa ayevbeosa commented Jul 9, 2024

Added error logs in pallet-xcm to help in debugging, fixes #2408

TODO

  • change log::error to tracing::error format for xcm-executor
  • check existing logs, e.g. this one can be extended with more info tracing::error!(target: "xcm::reanchor", ?error, "Failed reanchoring with error");
  • use tracing instead of log for pallet-xcm/src/lib.rs

Polkadot Address: 1Gz5aLtEu2n4jsfA6XwtZnuaRymJrDDw4kEGdNHTdxrpzrc

@ayevbeosa ayevbeosa requested a review from a team as a code owner July 9, 2024 08:34
@acatangiu
Copy link
Contributor

@ayevbeosa this is good, but does not yet fix #2408 - this PR only covers pallet-xcm right now - #2408 is referring to XCM errors across the whole stack.
A good starting point is to do the same for everything under https://github.com/paritytech/polkadot-sdk/tree/master/polkadot/xcm.

@ayevbeosa
Copy link
Contributor Author

A good starting point is to do the same for everything under https://github.com/paritytech/polkadot-sdk/tree/master/polkadot/xcm.

Thanks @acatangiu, is it okay to do everything in this PR or I should break it in parts?

@acatangiu
Copy link
Contributor

I think you can do it in same PR since they're not that many changes

@github-actions github-actions bot requested a review from acatangiu July 11, 2024 06:36
Copy link

Review required! Latest push from author must always be reviewed

polkadot/xcm/xcm-executor/src/lib.rs Outdated Show resolved Hide resolved
polkadot/xcm/xcm-executor/src/lib.rs Outdated Show resolved Hide resolved
polkadot/xcm/xcm-executor/src/lib.rs Outdated Show resolved Hide resolved
polkadot/xcm/xcm-executor/src/lib.rs Outdated Show resolved Hide resolved
Co-authored-by: Francisco Aguirre <[email protected]>
polkadot/xcm/xcm-executor/src/lib.rs Outdated Show resolved Hide resolved
polkadot/xcm/xcm-executor/src/lib.rs Outdated Show resolved Hide resolved
@acatangiu
Copy link
Contributor

@ayevbeosa do you still plan on getting this over the finish line?

@ayevbeosa
Copy link
Contributor Author

@ayevbeosa do you still plan on getting this over the finish line?

Sure! Asides resolving the merge conflict, is there anything else left?

@acatangiu
Copy link
Contributor

Sure! Asides resolving the merge conflict, is there anything else left?

Nope, merge master and have CI passing and we'll merge it.

@bkontur bkontur added the R0-silent Changes should not be mentioned in any release notes label Oct 3, 2024
@acatangiu acatangiu removed the R0-silent Changes should not be mentioned in any release notes label Oct 14, 2024
@acatangiu
Copy link
Contributor

@ayevbeosa you need to add a new pr_4982.prdoc file with this PR, explaining the changes and "bumping" the affected crates.

e.g. https://github.com/paritytech/polkadot-sdk/blob/master/prdoc/pr_6023.prdoc

In your case you should have following crates bumping:

crates:
  - name: staging-xcm-executor
    bump: patch
  - name: pallet-xcm
    bump: patch

@ayevbeosa
Copy link
Contributor Author

@ayevbeosa you need to add a new pr_4982.prdoc file with this PR, explaining the changes and "bumping" the affected crates.

e.g. https://github.com/paritytech/polkadot-sdk/blob/master/prdoc/pr_6023.prdoc

In your case you should have following crates bumping:

crates:
  - name: staging-xcm-executor
    bump: patch
  - name: pallet-xcm
    bump: patch

Alright, thanks

prdoc/pr_4982.prdoc Outdated Show resolved Hide resolved
@bkontur bkontur enabled auto-merge October 14, 2024 11:33
prdoc/pr_4982.prdoc Outdated Show resolved Hide resolved
@bkontur bkontur added this pull request to the merge queue Oct 14, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 14, 2024
@bkontur bkontur added this pull request to the merge queue Oct 14, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 14, 2024
@bkontur bkontur enabled auto-merge October 15, 2024 05:33
@bkontur bkontur added this pull request to the merge queue Oct 15, 2024
Merged via the queue into paritytech:master with commit b20be7c Oct 15, 2024
193 of 196 checks passed
@bkontur
Copy link
Contributor

bkontur commented Oct 15, 2024

@ayevbeosa Thank you for your contribution, please include your Polkadot address in the PR description. https://github.com/paritytech/substrate-tip-bot

@ayevbeosa
Copy link
Contributor Author

@ayevbeosa Thank you for your contribution, please include your Polkadot address in the PR description. https://github.com/paritytech/substrate-tip-bot

Thank you, will do that!

@acatangiu
Copy link
Contributor

/tip small

Copy link

@acatangiu A referendum for a small (20 DOT) tip was successfully submitted for @ayevbeosa (1Gz5aLtEu2n4jsfA6XwtZnuaRymJrDDw4kEGdNHTdxrpzrc on polkadot).

Referendum number: 1233.
tip

Copy link

The referendum has appeared on Polkassembly.

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.

[XCM] don't ignore errors
5 participants