Skip to content

[stable2503] Backport #7200#7773

Merged
EgorPopelyaev merged 1 commit intostable2503from
backport-7200-to-stable2503
Mar 3, 2025
Merged

[stable2503] Backport #7200#7773
EgorPopelyaev merged 1 commit intostable2503from
backport-7200-to-stable2503

Conversation

@paritytech-cmd-bot-polkadot-sdk
Copy link
Copy Markdown
Contributor

Backport #7200 into stable2503 from bkontur.

See the documentation on how to use this bot.

…o be executed on the local chain (#7200)

Resolves (partially):
#7148
Depends on: #7169

# Description

This PR addresses partially #7148 (Problem 2) and ensures the proper
checking of nested local instructions. It introduces a new barrier -
`DenyRecursively` - to provide more refined control over instruction
denial. The main change is the replacement of `DenyThenTry<Deny, Allow>`
with `DenyThenTry<DenyRecursively<Deny>, Allow>` which handles both
top-level and nested local instructions by applying allow condition
after denial.

For context and additional information, please refer to [_Problem 2 -
Barrier vs nested XCM
validation_](#7148).

# TODO
- [x] Evaluate PoC, more details at #7351:
    - **DenyNestedXcmInstructions**: Keep it as it is and be explicit:
        1. Name the Deny barriers for the top level.
2. Name the Deny barrier for nested with `DenyInstructionsWithXcm`.
- **DenyThenTry<DenyInstructionsWithXcm<Deny>, Allow>**: Alternatively,
hard-code those three instructions in `DenyThenTry`, so we wouldn’t need
`DenyInstructionsWithXcm`. However, this approach wouldn’t be as
general.
- **DenyInstructionsWithXcmFor**: Another possibility is to check
`DenyInstructionsWithXcm::Inner` for the actual `message`, so we don’t
need duplication for top-level and nested (not sure, maybe be explicit
is good thing) - see _Problem2 - example_. Instead of this:
   ```
  DenyThenTry<
                (
                               // Deny for top level XCM program
                               DenyReserveTransferToRelayChain,
// Dedicated barrier for nested XCM programs
                               DenyInstructionsWithXcmFor<
// Repeat all Deny filters here
DenyReserveTransferToRelayChain,
                                >
                ),
   ```
  we could just use:
   ```
  DenyThenTry<
                (
                               // Dedicated barrier for XCM programs
                               DenyInstructionsWithXcmFor<
// Add all `Deny` filters here
DenyReserveTransferToRelayChain,
                                                ...
                                >
                ),
   ```
- [POC
Evaluation](#7200 (comment))
- [x] Consider better name `DenyInstructionsWithXcm` =>
`DenyRecursively`, more details at
[here](#7200 (comment))
- [x] Clean-up and docs
- [x] Merge #7169 or
rebase this branch on the top of `yrong:fix-for-deny-then-try`
- [x] Set for the runtimes where we use `DenyThenTry<Deny, Allow>` =>
`DenyThenTry<DenyRecursively<Deny>, Allow>`
- [ ] Schedule sec.audit

---------

Co-authored-by: Raymond Cheung <178801527+raymondkfcheung@users.noreply.github.com>
Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: ron <yrong1997@gmail.com>
Co-authored-by: Francisco Aguirre <franciscoaguirreperez@gmail.com>
Co-authored-by: Clara van Staden <claravanstaden64@gmail.com>
Co-authored-by: Adrian Catangiu <adrian@parity.io>
(cherry picked from commit bd7cf11)
@bkontur bkontur enabled auto-merge (squash) March 3, 2025 10:31
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 3, 2025

This pull request is amending an existing release. Please proceed with extreme caution,
as to not impact downstream teams that rely on the stability of it. Some things to consider:

  • Backports are only for 'patch' or 'minor' changes. No 'major' or other breaking change.
  • Should be a legit fix for some bug, not adding tons of new features.
  • Must either be already audited or not need an audit.
Emergency Bypass

If you really need to bypass this check: add validate: false to each crate
in the Prdoc where a breaking change is introduced. This will release a new major
version of that crate and all its reverse dependencies and basically break the release.

@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/13628732606
Failed job name: check-runtime-migration

@EgorPopelyaev EgorPopelyaev disabled auto-merge March 3, 2025 11:32
@EgorPopelyaev EgorPopelyaev merged commit afbf60b into stable2503 Mar 3, 2025
241 of 257 checks passed
@EgorPopelyaev EgorPopelyaev deleted the backport-7200-to-stable2503 branch March 3, 2025 11:32
@raymondkfcheung raymondkfcheung moved this to SDK Released - Needs Integration in fellowship/runtimes integrations queue May 29, 2025
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.

Projects

Status: Done
Status: SDK Released - Needs Integration

Development

Successfully merging this pull request may close these issues.

3 participants