Skip to content

Make WeightBounds return XcmError to surface failures#8535

Merged
raymondkfcheung merged 31 commits intomasterfrom
ray-check-weight-with-limit
May 29, 2025
Merged

Make WeightBounds return XcmError to surface failures#8535
raymondkfcheung merged 31 commits intomasterfrom
ray-check-weight-with-limit

Conversation

@raymondkfcheung
Copy link
Copy Markdown
Contributor

@raymondkfcheung raymondkfcheung commented May 14, 2025

This PR partially addresses #8419 by improving error handling and traceability for XCM weight calculation failures in WeightInfoBounds and FixedWeightBounds.

Why change

The vague "Failed to prepare message" error in #8419 makes debugging extremely difficult. The current implementation drops detailed errors and omits log context, especially for cases like decoding instructions or violating instruction count limits. Previously, XCM weight calculation errors were surfaced only as Result<Weight, ()>, losing valuable error information and forcing all upstream consumers to treat them as opaque failures.

How change

  • Updates return types: Changes WeightBounds::weight and instr_weight to return Result<Weight, XcmError> instead of Result<Weight, ()>
  • Improves internal error handling: Uses Result<Weight, XcmError> internally for:
    • Instruction decoding failures → FailedToDecode
    • Message/instruction weight overflows → Overflow
    • Excessive instruction count → ExceedsStackLimit
  • Adds structured debug logging:
    • weight: logs error type, instructions remaining, message length
    • instr_weight: logs error type, instruction, and limit
  • Updates consumers: Ensures downstream callers (e.g., XcmExecutor, ExecuteXcm) properly handle the structured errors

This enables better structured error handling and debugging while providing detailed context for XCM weight calculation failures.

@raymondkfcheung raymondkfcheung self-assigned this May 14, 2025
@raymondkfcheung raymondkfcheung requested a review from a team as a code owner May 14, 2025 15:20
@raymondkfcheung raymondkfcheung marked this pull request as draft May 14, 2025 15:20
@raymondkfcheung raymondkfcheung added the T6-XCM This PR/Issue is related to XCM. label May 14, 2025
@raymondkfcheung raymondkfcheung moved this to In Progress in Bridges + XCM May 14, 2025
@raymondkfcheung
Copy link
Copy Markdown
Contributor Author

/cmd prdoc --audience runtime_dev --bump patch

@raymondkfcheung raymondkfcheung marked this pull request as ready for review May 15, 2025 09:03
@raymondkfcheung raymondkfcheung moved this from In Progress to In-Review in Bridges + XCM May 15, 2025
@paritytech-workflow-stopper
Copy link
Copy Markdown

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

@raymondkfcheung raymondkfcheung marked this pull request as ready for review May 27, 2025 09:55
@acatangiu acatangiu requested a review from a team May 27, 2025 14:00
@raymondkfcheung raymondkfcheung enabled auto-merge May 27, 2025 15:31
/// Return the maximum amount of weight that an attempted execution of this message could
/// consume.
fn weight(message: &mut Xcm<RuntimeCall>) -> Result<Weight, ()>;
fn weight(message: &mut Xcm<RuntimeCall>) -> Result<Weight, XcmError>;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we make this return the index of the problematic instruction as well? It'd be very useful to bubble it up in #7730

polkadot-runtime-parachains = { workspace = true, default-features = true }
polkadot-test-runtime = { workspace = true }
primitive-types = { features = ["codec", "num-traits", "scale-info"], workspace = true }
sp-tracing = { features = ["test-utils"], workspace = true, default-features = true }
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

just curious why you add sp-tracing as a dev dependency not as a regular one?

@raymondkfcheung raymondkfcheung added this pull request to the merge queue May 29, 2025
Merged via the queue into master with commit fed35af May 29, 2025
314 of 316 checks passed
@raymondkfcheung raymondkfcheung deleted the ray-check-weight-with-limit branch May 29, 2025 06:45
@github-project-automation github-project-automation bot moved this from To be released (SDK) to Integrated - Done in fellowship/runtimes integrations queue May 29, 2025
@github-project-automation github-project-automation bot moved this from In-Review to Done in Bridges + XCM May 29, 2025
@bkontur bkontur moved this from Integrated - Done to To be released (SDK) in fellowship/runtimes integrations queue May 29, 2025
pgherveou pushed a commit that referenced this pull request Jun 11, 2025
This PR partially addresses #8419 by improving error handling and
traceability for XCM weight calculation failures in `WeightInfoBounds`
and `FixedWeightBounds`.

## Why change

The vague "Failed to prepare message" error in #8419 makes debugging
extremely difficult. The current implementation drops detailed errors
and omits log context, especially for cases like decoding instructions
or violating instruction count limits. Previously, XCM weight
calculation errors were surfaced only as `Result<Weight, ()>`, losing
valuable error information and forcing all upstream consumers to treat
them as opaque failures.

## How change

* Updates return types: Changes `WeightBounds::weight` and
`instr_weight` to return `Result<Weight, XcmError>` instead of
`Result<Weight, ()>`
* Improves internal error handling: Uses `Result<Weight, XcmError>`
internally for:
    - Instruction decoding failures → `FailedToDecode`
    - Message/instruction weight overflows → `Overflow`
    - Excessive instruction count → `ExceedsStackLimit`
* Adds structured debug logging:
    - `weight`: logs error type, instructions remaining, message length
    - `instr_weight`: logs error type, instruction, and limit
* Updates consumers: Ensures downstream callers (e.g., `XcmExecutor`,
`ExecuteXcm`) properly handle the structured errors

This enables better structured error handling and debugging while
providing detailed context for XCM weight calculation failures.

---------

Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com>
alvicsam pushed a commit that referenced this pull request Oct 17, 2025
This PR partially addresses #8419 by improving error handling and
traceability for XCM weight calculation failures in `WeightInfoBounds`
and `FixedWeightBounds`.

## Why change

The vague "Failed to prepare message" error in #8419 makes debugging
extremely difficult. The current implementation drops detailed errors
and omits log context, especially for cases like decoding instructions
or violating instruction count limits. Previously, XCM weight
calculation errors were surfaced only as `Result<Weight, ()>`, losing
valuable error information and forcing all upstream consumers to treat
them as opaque failures.

## How change

* Updates return types: Changes `WeightBounds::weight` and
`instr_weight` to return `Result<Weight, XcmError>` instead of
`Result<Weight, ()>`
* Improves internal error handling: Uses `Result<Weight, XcmError>`
internally for:
    - Instruction decoding failures → `FailedToDecode`
    - Message/instruction weight overflows → `Overflow`
    - Excessive instruction count → `ExceedsStackLimit`
* Adds structured debug logging:
    - `weight`: logs error type, instructions remaining, message length
    - `instr_weight`: logs error type, instruction, and limit
* Updates consumers: Ensures downstream callers (e.g., `XcmExecutor`,
`ExecuteXcm`) properly handle the structured errors

This enables better structured error handling and debugging while
providing detailed context for XCM weight calculation failures.

---------

Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com>
snowmead added a commit to Moonsong-Labs/storage-hub that referenced this pull request Mar 16, 2026
Upgrade all polkadot-sdk git dependencies from branch `stable2503` to
`stable2506`. Upgrade all Frontier EVM dependencies from branch
`stable2503` to tag `frontier-stable2506`.

Fix all compilation errors and test failures caused by upstream breaking
API changes in polkadot-sdk stable2506 (134 PRs) and 9 patch releases.

Upstream breaking changes addressed:
- Remove local `AccountIdFor` alias, now provided by
  `frame_system::pallet_prelude` (paritytech/polkadot-sdk#7229)
- Add `RelayParentOffset = ConstU32<0>` to parachain system config
  (paritytech/polkadot-sdk#8299)
- Fix `Outcome` enum pattern matching: `Error` is now tuple variant
  wrapping `InstructionError` (paritytech/polkadot-sdk#8535)
- Remove `RuntimeEvent` from `pallet_evm::Config` and
  `pallet_ethereum::Config` (frontier stable2506)
- Add `RuntimeEvent` to `pallet_session::historical::Config`
- Remove `PassByInner` import, use `.0` for `H256` inner access
  (paritytech/polkadot-sdk#7375)
- Destructure 4 return values from `build_relay_chain_interface`
  (paritytech/polkadot-sdk#8072)
- Add `metrics` field to `BuildNetworkParams`
  (paritytech/polkadot-sdk#8332)
- Add `prometheus_registry` to `StartRelayChainTasksParams`
  (paritytech/polkadot-sdk#8332)

Release: https://github.com/paritytech/polkadot-sdk/releases/tag/polkadot-stable2506

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A3-backport Pull request is already reviewed well in another branch. T6-XCM This PR/Issue is related to XCM.

Projects

Status: Done
Status: Done
Status: To be released (SDK)

Development

Successfully merging this pull request may close these issues.

6 participants