Skip to content
  1. Mar 01, 2024
  3. Feb 28, 2024
    • Oliver Tale-Yazdi's avatar
      Multi-Block-Migrations, `poll` hook and new System callbacks (#1781) · eefd5fe4
      Oliver Tale-Yazdi authored 
      This MR is the merge of
https://github.com/paritytech/substrate/pull/14414 and
https://github.com/paritytech/substrate/pull/14275. It implements
[RFC#13](https://github.com/polkadot-fellows/RFCs/pull/13), closes
https://github.com/paritytech/polkadot-sdk/issues/198.

----- 

This Merge request introduces three major topicals:

1. Multi-Block-Migrations
1. New pallet `poll` hook for periodic service work
1. Replacement hooks for `on_initialize` and `on_finalize` in cases
where `poll` cannot be used

and some more general changes to FRAME.  
The changes for each topical span over multiple crates. They are listed
in topical order below.

# 1.) Multi-Block-Migrations

Multi-Block-Migrations are facilitated by creating `pallet_migrations`
and configuring `System::Config::MultiBlockMigrator` to point to it.
Executive picks this up and triggers one step of the migrations pallet
per block.
The chain is in lockdown mode for as long as an MBM is ongoing.
Executive does this by polling `MultiBlockMigrator::ongoing` and not
allowing any transaction in a block, if true.

A MBM is defined through trait `SteppedMigration`. A condensed version
looks like this:
```rust
/// A migration that can proceed in multiple steps.
pub trait SteppedMigration {
	type Cursor: FullCodec + MaxEncodedLen;
	type Identifier: FullCodec + MaxEncodedLen;

	fn id() -> Self::Identifier;

	fn max_steps() -> Option<u32>;

	fn step(
		cursor: Option<Self::Cursor>,
		meter: &mut WeightMeter,
	) -> Result<Option<Self::Cursor>, SteppedMigrationError>;
}
```

`pallet_migrations` can be configured with an aggregated tuple of these
migrations. It then starts to migrate them one-by-one on the next
runtime upgrade.
Two things are important here:
- 1. Doing another runtime upgrade while MBMs are ongoing is not a good
idea and can lead to messed up state.
- 2. **Pallet Migrations MUST BE CONFIGURED IN `System::Config`,
otherwise it is not used.**

The pallet supports an `UpgradeStatusHandler` that can be used to notify
external logic of upgrade start/finish (for example to pause XCM
dispatch).

Error recovery is very limited in the case that a migration errors or
times out (exceeds its `max_steps`). Currently the runtime dev can
decide in `FailedMigrationHandler::failed` how to handle this. One
follow-up would be to pair this with the `SafeMode` pallet and enact
safe mode when an upgrade fails, to allow governance to rescue the
chain. This is currently not possible, since governance is not
`Mandatory`.

## Runtime API

- `Core`: `initialize_block` now returns `ExtrinsicInclusionMode` to
inform the Block Author whether they can push transactions.

### Integration

Add it to your runtime implementation of `Core` and `BlockBuilder`:
```patch
diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs
@@ impl_runtime_apis! {
	impl sp_block_builder::Core<Block> for Runtime {
-		fn initialize_block(header: &<Block as BlockT>::Header) {
+		fn initialize_block(header: &<Block as BlockT>::Header) -> RuntimeExecutiveMode {
			Executive::initialize_block(header)
		}

		...
	}
```

# 2.) `poll` hook

A new pallet hook is introduced: `poll`. `Poll` is intended to replace
mostly all usage of `on_initialize`.
The reason for this is that any code that can be called from
`on_initialize` cannot be migrated through an MBM. Currently there is no
way to statically check this; the implication is to use `on_initialize`
as rarely as possible.
Failing to do so can result in broken storage invariants.

The implementation of the poll hook depends on the `Runtime API` changes
that are explained above.

# 3.) Hard-Deadline callbacks

Three new callbacks are introduced and configured on `System::Config`:
`PreInherents`, `PostInherents` and `PostTransactions`.
These hooks are meant as replacement for `on_initialize` and
`on_finalize` in cases where the code that runs cannot be moved to
`poll`.
The reason for this is to make the usage of HD-code (hard deadline) more
explicit - again to prevent broken invariants by MBMs.

# 4.) FRAME (general changes)

## `frame_system` pallet

A new memorize storage item `InherentsApplied` is added. It is used by
executive to track whether inherents have already been applied.
Executive and can then execute the MBMs directly between inherents and
transactions.

The `Config` gets five new items:
- `SingleBlockMigrations` this is the new way of configuring migrations
that run in a single block. Previously they were defined as last generic
argument of `Executive`. This shift is brings all central configuration
about migrations closer into view of the developer (migrations that are
configured in `Executive` will still work for now but is deprecated).
- `MultiBlockMigrator` this can be configured to an engine that drives
MBMs. One example would be the `pallet_migrations`. Note that this is
only the engine; the exact MBMs are injected into the engine.
- `PreInherents` a callback that executes after `on_initialize` but
before inherents.
- `PostInherents` a callback that executes after all inherents ran
(including MBMs and `poll`).
- `PostTransactions` in symmetry to `PreInherents`, this one is called
before `on_finalize` but after all transactions.

A sane default is to set all of these to `()`. Example diff suitable for
any chain:
```patch
@@ impl frame_system::Config for Test {
 	type MaxConsumers = ConstU32<16>;
+	type SingleBlockMigrations = ();
+	type MultiBlockMigrator = ();
+	type PreInherents = ();
+	type PostInherents = ();
+	type PostTransactions = ();
 }
```

An overview of how the block execution now looks like is here. The same
graph is also in the rust doc.

<details><summary>Block Execution Flow</summary>
<p>

![Screenshot 2023-12-04 at 19 11
29](https://github.com/paritytech/polkadot-sdk/assets/10380170/e88a80c4-ef11-4faa-8df5-8b33a724c054)

</p>
</details> 

## Inherent Order

Moved to https://github.com/paritytech/polkadot-sdk/pull/2154



---------------


## TODO

- [ ] Check that `try-runtime` still works
- [ ] Ensure backwards compatibility with old Runtime APIs
- [x] Consume weight correctly
- [x] Cleanup

---------

Signed-off-by: default avatarOliver Tale-Yazdi <[email protected]>
Co-authored-by: default avatarLiam Aharon <[email protected]>
Co-authored-by: default avatarJuan Girini <[email protected]>
Co-authored-by: command-bot <>
Co-authored-by: default avatarFrancisco Aguirre <[email protected]>
Co-authored-by: default avatarGavin Wood <[email protected]>
Co-authored-by: default avatarBastian Köcher <[email protected]>
      eefd5fe4
    • Clara van Staden's avatar
      Snowbridge - Extract Ethereum Chain ID (#3501) · 576681b8
      Clara van Staden authored 
      While adding runtime tests to
https://github.com/polkadot-fellows/runtimes/pull/130, I noticed the
Ethereum chain ID was hardcoded. For Kusama + Polkadot, the Ethereum
chain ID should 1 (Mainnet), whereas on Rococo it is 11155111 (Sepolia).

This PR also updates the Snowbridge crates versions to the current
versions on crates.io.

---------

Co-authored-by: claravanstaden <Cats 4 life!>
      576681b8
    • Liam Aharon's avatar
      Runtime Upgrade ref docs and Single Block Migration example pallet (#1554) · 12ce4f7d
      Liam Aharon authored 
      Closes https://github.com/paritytech/polkadot-sdk-docs/issues/55

- Changes 'current storage version' terminology to less ambiguous
'in-code storage version' (suggestion by @ggwpez)
- Adds a new example pallet `pallet-example-single-block-migrations`
- Adds a new reference doc to replace
https://docs.substrate.io/maintain/runtime-upgrades/ (temporarily living
in the pallet while we wait for developer hub PR to merge)
- Adds documentation for the `storage_alias` macro
- Improves `trait Hooks` docs 
- Improves `trait GetStorageVersion` docs
- Update the suggested patterns for using `VersionedMigration`, so that
version unchecked migrations are never exported
- Prevents accidental usage of version unchecked migrations in runtimes

https://github.com/paritytech/substrate/pull/14421#discussion_r1255467895
- Unversioned migration code is kept inside `mod version_unchecked`,
versioned code is kept in `pub mod versioned`
- It is necessary to use modules to limit visibility because the inner
migration must be `pub`. See
https://github.com/rust-lang/rust/issues/30905 and

https://internals.rust-lang.org/t/lang-team-minutes-private-in-public-rules/4504/40
for more.

### todo

- [x] move to reference docs to proper place within sdk-docs (now that
https://github.com/paritytech/polkadot-sdk/pull/2102

 is merged)
- [x] prdoc

---------

Co-authored-by: default avatarKian Paimani <[email protected]>
Co-authored-by: default avatarJuan <[email protected]>
Co-authored-by: default avatarOliver Tale-Yazdi <[email protected]>
Co-authored-by: command-bot <>
Co-authored-by: default avatargupnik <[email protected]>
      12ce4f7d
  5. Feb 27, 2024
    • Clara van Staden's avatar
      Snowbridge benchmark tests fix (#3424) · 94c54d50
      Clara van Staden authored 
      When running `cargo test -p bridge-hub-rococo-runtime --features
runtime-benchmarks`, two of the Snowbridge benchmark tests fails. The
reason is that when the runtime-benchmarks feature is enabled, the
`NoopMessageProcessor` message processor is used. The Snowbridge tests
rely on the outbound messages to be processed using the message queue,
so that we can check the expected nonce and block digest logs.

This PR changes the conditional compilation to only use
`NoopMessageProcessor` when compiling the executable to run benchmarks
against, not when running tests.

---------

Co-authored-by: claravanstaden <Cats 4 life!>
      94c54d50
  7. Feb 26, 2024
  9. Feb 23, 2024
  11. Feb 21, 2024
    • Clara van Staden's avatar
      Snowbridge - Test pallet order (#3381) · 5a06771e
      Clara van Staden authored 
      - Adds a test to check the correct digest for Snowbridge outbound
messages. For the correct digest to be in the block, the the
MessageQueue pallet should be configured after the EthereumOutbound
queue pallet. The added test fails if the EthereumOutbound is configured
after the MessageQueue pallet.
- Adds a helper method `run_to_block_with_finalize` to simulate the
block finalizing. The existing `run_to_block` method does not finalize
and so it cannot successfully test this condition.

Closes: https://github.com/paritytech/polkadot-sdk/issues/3208

---------

Co-authored-by: claravanstaden <Cats 4 life!>
      5a06771e
  13. Feb 20, 2024
  15. Feb 16, 2024
  17. Feb 15, 2024
  19. Feb 13, 2024
    • Branislav Kontur's avatar
      [xcm-builder] Replaced deprecated CurrencyAdapter with FungibleAdapter (#3287) · e0c902e3
      Branislav Kontur authored 
      I found out during the cleanup of this deprecation message in the
`polkadot-fellows` repository that we deprecated `CurrencyAdapter`
without making the recommended changes.


## TODO
- [ ] fix `polkadot-fellows` bump to 1.6.0
https://github.com/polkadot-fellows/runtimes/pull/159



---------

Co-authored-by: default avatarFrancisco Aguirre <[email protected]>
      e0c902e3
    • s0me0ne-unkn0wn's avatar
      Use dynamic aura slot duration in lookahead collator (#3211) · ec6bf5d0
      s0me0ne-unkn0wn authored 
      It's a follow-up of #2949. It enables the lookahead collator to
dynamically adjust the aura slot size, which may change during the
runtime upgrade. It also addressed a couple of issues with time
constants I missed in the original PR.

Good news: it works. The parachain successfully switches from sync
backing with 12s slots to async backing with 6s slots.

Bad news: during the transitional period of a single block in which the
actual runtime upgrade is performed, it still gets the old slot duration
of 12s (as it gets it from the best block), resulting in a runtime panic
(logs follow). That doesn't affect the following block production of the
parachain. Ideas on how to improve the situation are appreciated.

<details>

```
2024-02-05 12:59:36.373  INFO tokio-runtime-worker sc_basic_authorship::basic_authorship: [Parachain] 🙌 Starting consensus session on top of parent 0x6fd2d8f904f12c22531bfabf77b16dc84a6a29e45d9ae358aa6547fbf3f0438b    
2024-02-05 12:59:36.373 ERROR tokio-runtime-worker runtime: [Parachain] panicked at /home/s0me0ne/wrk/parity/polkadot-sdk/cumulus/pallets/aura-ext/src/consensus_hook.rs:69:9:
assertion `left == right` failed: slot number mismatch
  left: Slot(142261198)
 right: Slot(284522396)    
2024-02-05 12:59:36.373  WARN tokio-runtime-worker sp_state_machine::overlayed_changes::changeset: [Parachain] 1 storage transactions are left open by the runtime. Those will be rolled back.    
2024-02-05 12:59:36.373  WARN tokio-runtime-worker sp_state_machine::overlayed_changes::changeset: [Parachain] 1 storage transactions are left open by the runtime. Those will be rolled back.    
2024-02-05 12:59:36.373  WARN tokio-runtime-worker basic-authorship: [Parachain] ❗ Inherent extrinsic returned unexpected error: Error at calling runtime api: Execution failed: Execution aborted due to trap: wasm trap: wasm `unreachable` instruction executed
WASM backtrace:
error while executing at wasm backtrace:
    0: 0x4e4a3b - <unknown>!rust_begin_unwind
    1: 0x46cf57 - <unknown>!core::panicking::panic_fmt::h3c280dba88683724
    2: 0x46d238 - <unknown>!core::panicking::assert_failed_inner::hebac5970933beb4d
    3: 0x3d00fc - <unknown>!core::panicking::assert_failed::h640a47e2fb5dfb4b
    4: 0xd0db3 - <unknown>!frame_support::storage::transactional::with_transaction::hcbc31515f81b2ee1
    5: 0x34d654 - <unknown>!<cumulus_pallet_parachain_system::pallet::Call<T> as frame_support::traits::dispatch::UnfilteredDispatchable>::dispatch_bypass_filter::{{closure}}::hb7c2c9a11fa88301
    6: 0x3547db - <unknown>!environmental::local_key::LocalKey<T>::with::h783f2605ae27d6d3
    7: 0x7f454 - <unknown>!<asset_hub_rococo_runtime::RuntimeCall as frame_support::traits::dispatch::UnfilteredDispatchable>::dispatch_bypass_filter::h5e11a01ab97c06c7
    8: 0x7f237 - <unknown>!<asset_hub_rococo_runtime::RuntimeCall as sp_runtime::traits::Dispatchable>::dispatch::h7f8ae4a8fede71ca
    9: 0x26a0f3 - <unknown>!frame_executive::Executive<System,Block,Context,UnsignedValidator,AllPalletsWithSystem,COnRuntimeUpgrade>::apply_extrinsic::h75e524ff34738391
   10: 0x282211 - <unknown>!BlockBuilder_apply_extrinsic. Dropping.    
2024-02-05 12:59:36.374 ERROR tokio-runtime-worker runtime: [Parachain] panicked at /home/s0me0ne/wrk/parity/polkadot-sdk/substrate/frame/aura/src/lib.rs:416:9:
assertion `left == right` failed: Timestamp slot must match `CurrentSlot`
  left: Slot(142261198)
 right: Slot(284522396)    
2024-02-05 12:59:36.374  WARN tokio-runtime-worker sp_state_machine::overlayed_changes::changeset: [Parachain] 1 storage transactions are left open by the runtime. Those will be rolled back.    
2024-02-05 12:59:36.374  WARN tokio-runtime-worker sp_state_machine::overlayed_changes::changeset: [Parachain] 1 storage transactions are left open by the runtime. Those will be rolled back.    
2024-02-05 12:59:36.374  WARN tokio-runtime-worker basic-authorship: [Parachain] ❗

 Inherent extrinsic returned unexpected error: Error at calling runtime api: Execution failed: Execution aborted due to trap: wasm trap: wasm `unreachable` instruction executed
WASM backtrace:
error while executing at wasm backtrace:
    0: 0x4e4a3b - <unknown>!rust_begin_unwind
    1: 0x46cf57 - <unknown>!core::panicking::panic_fmt::h3c280dba88683724
    2: 0x46d238 - <unknown>!core::panicking::assert_failed_inner::hebac5970933beb4d
    3: 0x3d00fc - <unknown>!core::panicking::assert_failed::h640a47e2fb5dfb4b
    4: 0x9ece6 - <unknown>!frame_support::storage::transactional::with_transaction::h26f75cb9f9462088
    5: 0x356d7e - <unknown>!environmental::local_key::LocalKey<T>::with::hbcf2d4e17b48fdb5
    6: 0x7f507 - <unknown>!<asset_hub_rococo_runtime::RuntimeCall as frame_support::traits::dispatch::UnfilteredDispatchable>::dispatch_bypass_filter::h5e11a01ab97c06c7
    7: 0x7f237 - <unknown>!<asset_hub_rococo_runtime::RuntimeCall as sp_runtime::traits::Dispatchable>::dispatch::h7f8ae4a8fede71ca
    8: 0x26a0f3 - <unknown>!frame_executive::Executive<System,Block,Context,UnsignedValidator,AllPalletsWithSystem,COnRuntimeUpgrade>::apply_extrinsic::h75e524ff34738391
    9: 0x282211 - <unknown>!BlockBuilder_apply_extrinsic. Dropping.    
2024-02-05 12:59:36.374 DEBUG tokio-runtime-worker runtime::xcmp-queue-migration: [Parachain] Lazy migration finished: item gone    
2024-02-05 12:59:36.374 ERROR tokio-runtime-worker runtime: [Parachain] panicked at /home/s0me0ne/wrk/parity/polkadot-sdk/cumulus/pallets/parachain-system/src/lib.rs:265:18:
set_validation_data inherent needs to be present in every block!    
2024-02-05 12:59:36.374 ERROR tokio-runtime-worker aura::cumulus: [Parachain] err=Error { inner: Proposing

Caused by:
    0: Error at calling runtime api: Execution failed: Execution aborted due to trap: wasm trap: wasm `unreachable` instruction executed
       WASM backtrace:
       error while executing at wasm backtrace:
           0: 0x4e4a3b - <unknown>!rust_begin_unwind
           1: 0x46cf57 - <unknown>!core::panicking::panic_fmt::h3c280dba88683724
           2: 0x46da8b - <unknown>!core::option::expect_failed::hdf18d99c3adabca7
           3: 0x2134cb - <unknown>!<cumulus_pallet_parachain_system::pallet::Pallet<T> as frame_support::traits::hooks::OnFinalize<<<<T as frame_system::pallet::Config>::Block as sp_runtime::traits::HeaderProvider>::HeaderT as sp_runtime::traits::Header>::Number>>::on_finalize::hf98aac39802896ba
           4: 0x26a9d6 - <unknown>!frame_executive::Executive<System,Block,Context,UnsignedValidator,AllPalletsWithSystem,COnRuntimeUpgrade>::idle_and_finalize_hook::h32775c0df0749d92
           5: 0x26ad9f - <unknown>!frame_executive::Executive<System,Block,Context,UnsignedValidator,AllPalletsWithSystem,COnRuntimeUpgrade>::finalize_block::h15e5a1a6b9ca8032
           6: 0x2822bd - <unknown>!BlockBuilder_finalize_block
    1: Execution failed: Execution aborted due to trap: wasm trap: wasm `unreachable` instruction executed
       WASM backtrace:
       error while executing at wasm backtrace:
           0: 0x4e4a3b - <unknown>!rust_begin_unwind
           1: 0x46cf57 - <unknown>!core::panicking::panic_fmt::h3c280dba88683724
           2: 0x46da8b - <unknown>!core::option::expect_failed::hdf18d99c3adabca7
           3: 0x2134cb - <unknown>!<cumulus_pallet_parachain_system::pallet::Pallet<T> as frame_support::traits::hooks::OnFinalize<<<<T as frame_system::pallet::Config>::Block as sp_runtime::traits::HeaderProvider>::HeaderT as sp_runtime::traits::Header>::Number>>::on_finalize::hf98aac39802896ba
           4: 0x26a9d6 - <unknown>!frame_executive::Executive<System,Block,Context,UnsignedValidator,AllPalletsWithSystem,COnRuntimeUpgrade>::idle_and_finalize_hook::h32775c0df0749d92
           5: 0x26ad9f - <unknown>!frame_executive::Executive<System,Block,Context,UnsignedValidator,AllPalletsWithSystem,COnRuntimeUpgrade>::finalize_block::h15e5a1a6b9ca8032
           6: 0x2822bd - <unknown>!BlockBuilder_finalize_block }
```

</details>

---------

Co-authored-by: default avatarBastian Köcher <[email protected]>
      ec6bf5d0
  21. Feb 12, 2024
    • Dónal Murray's avatar
      Bump coretime-rococo to get leases fix (#3289) · cecdf760
      Dónal Murray authored 
      Leases can be force set, but since Leases is a StorageValue, if a lease
misses its sale rotation in which it should expire, it can never be
cleared.

This can happen if a lease is added with an until timeslice that lies in
a region whose sale has already started or has passed, even if the
timeslice itself hasn't passed.

Trappist is currently trapped in a lease that will never end, so this
will remove it at the next sale rotation.

A fix was introduced in
https://github.com/paritytech/polkadot-sdk/pull/3213 but this missed the
1.7 release. This PR bumps the `coretime-rococo` version to get these
changes on Rococo.
      cecdf760
    • Oliver Tale-Yazdi's avatar
      Lift dependencies to the workspace (Part 1) (#2070) · e80c2473
      Oliver Tale-Yazdi authored 
      Changes (partial https://github.com/paritytech/polkadot-sdk/issues/994):
- Set log to `0.4.20` everywhere
- Lift `log` to the workspace

Starting with a simpler one after seeing
https://github.com/paritytech/polkadot-sdk/pull/2065 from @jsdw

.
This sets the `default-features` to `false` in the root and then
overwrites that in each create to its original value. This is necessary
since otherwise the `default` features are additive and its impossible
to disable them in the crate again once they are enabled in the
workspace.

I am using a tool to do this, so its mostly a test to see that it works
as expected.

---------

Signed-off-by: default avatarOliver Tale-Yazdi <[email protected]>
      e80c2473
    • Serban Iorga's avatar
      Bridge zombienet tests refactoring (#3260) · dfc8e469
      Serban Iorga authored 
      Related to https://github.com/paritytech/polkadot-sdk/issues/3242

Reorganizing the bridge zombienet tests in order to:
- separate the environment spawning from the actual tests
- offer better control over the tests and some possibility to
orchestrate them as opposed to running everything from the zndsl file

Only rewrote the asset transfer test using this new "framework". The old
logic and old tests weren't functionally modified or deleted. The plan
is to get feedback on this approach first and if this is agreed upon,
migrate the other 2 tests later in separate PRs and also do other
improvements later.
      dfc8e469
  23. Feb 09, 2024
  25. Feb 06, 2024
  27. Feb 02, 2024
  29. Jan 31, 2024
    • Oliver Tale-Yazdi's avatar
      [FRAME] Make `core-fellowship` ans `salary` work for swapped members (#3156) · 07e55006
      Oliver Tale-Yazdi authored 
      Fixup for https://github.com/paritytech/polkadot-sdk/pull/2587 to make
the `core-fellowship` crate work with swapped members.

Adds a `MemberSwappedHandler` to the `ranked-collective` pallet that are
implemented by `core-fellowship+salary`.
There is are exhaustive tests
[here](https://github.com/paritytech/polkadot-sdk/blob/72aa7ac17a0e5b16faab5d2992aa2db2e01b05d0/substrate/frame/core-fellowship/src/tests/integration.rs#L338)
and
[here](https://github.com/paritytech/polkadot-sdk/blob/ab3cdb05a5ebc1ff841f8dda67edef0ea40bbba5/substrate/frame/salary/src/tests/integration.rs#L224

)
to check that adding member `1` is equivalent to adding member `0` and
then swapping.

---------

Signed-off-by: default avatarOliver Tale-Yazdi <[email protected]>
      07e55006
    • Branislav Kontur's avatar
      [xcm] Fix `SovereignPaidRemoteExporter` and `DepositAsset` handling (#3157) · 6ea472ad
      Branislav Kontur authored 
      This PR addresses two issues:
- It modifies `DepositAsset`'s asset filter from `All` to
`AllCounted(1)` to prevent potentially charging excessive weight/fees.
This adjustment avoids situations where fees could be calculated based
on the count of assets, as illustrated
[here](https://github.com/paritytech/polkadot-sdk/blob/master/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/weights/xcm/mod.rs#L38-L46).
- It encapsulates `DepositAsset` with `SetAppendix` to ensure that
`fees` are not trapped in any case. For instance, this prevents issues
when `ExportXcm::validate` encounters an error during the processing of
`ExportMessage`.
      6ea472ad
    • Adrian Catangiu's avatar
      xcm-executor: DepositReserveAsset charges delivery fees from inner assets (#3142) · 5354097a
      Adrian Catangiu authored 
      

This fix aims to solve an issue in Kusama that resulted in failed
reserve asset transfers.

During multi-hop XCMs, like reserve asset transfers where the reserve is
not the sender nor the destination, but a third remote chain, the origin
is not available to pay for delivery fees out of their account directly,
so delivery fees should be paid out of transferred assets.

This commit also adds an xcm-emulator regression test that validates
this scenario is now working.

Signed-off-by: default avatarAdrian Catangiu <[email protected]>
Co-authored-by: default avatarFrancisco Aguirre <[email protected]>
      5354097a
    • Branislav Kontur's avatar
      [frame] `#[pallet::composite_enum]` improved variant count handling + removed... · bb8ddc46
      Branislav Kontur authored 
      [frame] `#[pallet::composite_enum]` improved variant count handling + removed `pallet_balances`'s `MaxHolds` config (#2657)

I started this investigation/issue based on @liamaharon question
[here](https://github.com/paritytech/polkadot-sdk/pull/1801#discussion_r1410452499).

## Problem

The `pallet_balances` integrity test should correctly detect that the
runtime has correct distinct `HoldReasons` variant count. I assume the
same situation exists for RuntimeFreezeReason.

It is not a critical problem, if we set `MaxHolds` with a sufficiently
large value, everything should be ok. However, in this case, the
integrity_test check becomes less useful.

**Situation for "any" runtime:**
- `HoldReason` enums from different pallets:
```rust
        /// from pallet_nis
        #[pallet::composite_enum]
	pub enum HoldReason {
		NftReceipt,
	}

        /// from pallet_preimage
        #[pallet::composite_enum]
	pub enum HoldReason {
		Preimage,
	}

        // from pallet_state-trie-migration
        #[pallet::composite_enum]
	pub enum HoldReason {
		SlashForContinueMigrate,
		SlashForMigrateCustomTop,
		SlashForMigrateCustomChild,
	}
```

- generated `RuntimeHoldReason` enum looks like:
```rust
pub enum RuntimeHoldReason {

    #[codec(index = 32u8)]
    Preimage(pallet_preimage::HoldReason),

    #[codec(index = 38u8)]
    Nis(pallet_nis::HoldReason),

    #[codec(index = 42u8)]
    StateTrieMigration(pallet_state_trie_migration::HoldReason),
}
```

- composite enum `RuntimeHoldReason` variant count is detected as `3`
- we set `type MaxHolds = ConstU32<3>`
- `pallet_balances::integrity_test` is ok with `3`(at least 3)

However, the real problem can occur in a live runtime where some
functionality might stop working. This is due to a total of 5 distinct
hold reasons (for pallets with multi-instance support, it is even more),
and not all of them can be used because of an incorrect `MaxHolds`,
which is deemed acceptable according to the `integrity_test`:
  ```
  // pseudo-code - if we try to call all of these:

T::Currency::hold(&pallet_nis::HoldReason::NftReceipt.into(),
&nft_owner, deposit)?;
T::Currency::hold(&pallet_preimage::HoldReason::Preimage.into(),
&nft_owner, deposit)?;

T::Currency::hold(&pallet_state_trie_migration::HoldReason::SlashForContinueMigrate.into(),
&nft_owner, deposit)?;

  // With `type MaxHolds = ConstU32<3>` these two will fail

T::Currency::hold(&pallet_state_trie_migration::HoldReason::SlashForMigrateCustomTop.into(),
&nft_owner, deposit)?;

T::Currency::hold(&pallet_state_trie_migration::HoldReason::SlashForMigrateCustomChild.into(),
&nft_owner, deposit)?;
  ```  


## Solutions

A macro `#[pallet::*]` expansion is extended of `VariantCount`
implementation for the `#[pallet::composite_enum]` enum type. This
expansion generates the `VariantCount` implementation for pallets'
`HoldReason`, `FreezeReason`, `LockId`, and `SlashReason`. Enum variants
must be plain enum values without fields to ensure a deterministic
count.

The composite runtime enum, `RuntimeHoldReason` and
`RuntimeFreezeReason`, now sets `VariantCount::VARIANT_COUNT` as the sum
of pallets' enum `VariantCount::VARIANT_COUNT`:
```rust
#[frame_support::pallet(dev_mode)]
mod module_single_instance {

	#[pallet::composite_enum]
	pub enum HoldReason {
		ModuleSingleInstanceReason1,
		ModuleSingleInstanceReason2,
	}
...
}

#[frame_support::pallet(dev_mode)]
mod module_multi_instance {

	#[pallet::composite_enum]
	pub enum HoldReason<I: 'static = ()> {
		ModuleMultiInstanceReason1,
		ModuleMultiInstanceReason2,
		ModuleMultiInstanceReason3,
	}
...
}


impl self::sp_api_hidden_includes_construct_runtime::hidden_include::traits::VariantCount
    for RuntimeHoldReason
{
    const VARIANT_COUNT: u32 = 0
        + module_single_instance::HoldReason::VARIANT_COUNT
        + module_multi_instance::HoldReason::<module_multi_instance::Instance1>::VARIANT_COUNT
        + module_multi_instance::HoldReason::<module_multi_instance::Instance2>::VARIANT_COUNT
        + module_multi_instance::HoldReason::<module_multi_instance::Instance3>::VARIANT_COUNT;
}
```

In addition, `MaxHolds` is removed (as suggested
[here](https://github.com/paritytech/polkadot-sdk/pull/2657#discussion_r1443324573))
from `pallet_balances`, and its `Holds` are now bounded to
`RuntimeHoldReason::VARIANT_COUNT`. Therefore, there is no need to let
the runtime specify `MaxHolds`.


## For reviewers

Relevant changes can be found here:
- `substrate/frame/support/procedural/src/lib.rs` 
-  `substrate/frame/support/procedural/src/pallet/parse/composite.rs`
-  `substrate/frame/support/procedural/src/pallet/expand/composite.rs`
-
`substrate/frame/support/procedural/src/construct_runtime/expand/composite_helper.rs`
-
`substrate/frame/support/procedural/src/construct_runtime/expand/hold_reason.rs`
-
`substrate/frame/support/procedural/src/construct_runtime/expand/freeze_reason.rs`
- `substrate/frame/support/src/traits/misc.rs`

And the rest of the files is just about removed `MaxHolds` from
`pallet_balances`

## Next steps

Do the same for `MaxFreezes`
https://github.com/paritytech/polkadot-sdk/issues/2997

.

---------

Co-authored-by: command-bot <>
Co-authored-by: default avatarBastian Köcher <[email protected]>
Co-authored-by: default avatarDónal Murray <[email protected]>
Co-authored-by: default avatargupnik <[email protected]>
      bb8ddc46
    • Branislav Kontur's avatar
      [cumulus] `parachains-common` testnet constants cleaning (#3134) · 4450b615
      Branislav Kontur authored 
      The `parachains-common` contains a lots of constants and type
definitions which are used for `polkadot-sdk`'s testnet runtimes and
also for `polkadot-fellows`'s production [SP
runtimes](https://github.com/polkadot-fellows/runtimes/tree/main/system-parachains/constants).
This PR cleans `parachains-common` module to contain only common and
generic functionality.

Testnet-specific constants have been moved to the separate module
dedicated just for testnets:
`polkadot-sdk/cumulus/parachains/runtimes/constants/`


Part of: https://github.com/paritytech/polkadot-sdk/issues/3054



---------

Co-authored-by: default avatargeorgepisaltu <[email protected]>
      4450b615
  31. Jan 30, 2024
  33. Jan 29, 2024
  35. Jan 27, 2024
  37. Jan 26, 2024