Skip to content
  1. Feb 28, 2024
  3. Feb 26, 2024
  5. Feb 20, 2024
  7. Feb 16, 2024
  9. Feb 13, 2024
  11. Feb 12, 2024
  13. Feb 09, 2024
  15. Feb 06, 2024
  17. Feb 02, 2024
  19. Jan 31, 2024
    • 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
  21. Jan 30, 2024
  23. Jan 29, 2024
  25. Jan 26, 2024
  27. Jan 24, 2024
  29. Jan 23, 2024
  31. Jan 22, 2024
  33. Jan 19, 2024
  35. Jan 17, 2024
  37. Jan 16, 2024
    • Francisco Aguirre's avatar
      XCMv4 (#1230) · 8428f678
      Francisco Aguirre authored 
      

# Note for reviewer

Most changes are just syntax changes necessary for the new version.
Most important files should be the ones under the `xcm` folder.

# Description 

Added XCMv4.

## Removed `Multi` prefix
The following types have been renamed:
- MultiLocation -> Location
- MultiAsset -> Asset
- MultiAssets -> Assets
- InteriorMultiLocation -> InteriorLocation
- MultiAssetFilter -> AssetFilter
- VersionedMultiAsset -> VersionedAsset
- WildMultiAsset -> WildAsset
- VersionedMultiLocation -> VersionedLocation

In order to fix a name conflict, the `Assets` in `xcm-executor` were
renamed to `HoldingAssets`, as they represent assets in holding.

## Removed `Abstract` asset id

It was not being used anywhere and this simplifies the code.

Now assets are just constructed as follows:

```rust
let asset: Asset = (AssetId(Location::new(1, Here)), 100u128).into();
```

No need for specifying `Concrete` anymore.

## Outcome is now a named fields struct

Instead of

```rust
pub enum Outcome {
  Complete(Weight),
  Incomplete(Weight, Error),
  Error(Error),
}
```

we now have

```rust
pub enum Outcome {
  Complete { used: Weight },
  Incomplete { used: Weight, error: Error },
  Error { error: Error },
}
```

## Added Reanchorable trait

Now both locations and assets implement this trait, making it easier to
reanchor both.

## New syntax for building locations and junctions

Now junctions are built using the following methods:

```rust
let location = Location {
    parents: 1,
    interior: [Parachain(1000), PalletInstance(50), GeneralIndex(1984)].into()
};
```

or

```rust
let location = Location::new(1, [Parachain(1000), PalletInstance(50), GeneralIndex(1984)]);
```

And they are matched like so:

```rust
match location.unpack() {
  (1, [Parachain(id)]) => ...
  (0, Here) => ...,
  (1, [_]) => ...,
}
```

This syntax is mandatory in v4, and has been also implemented for v2 and
v3 for easier migration.

This was needed to make all sizes smaller.

# TODO
- [x] Scaffold v4
- [x] Port github.com/paritytech/polkadot/pull/7236
- [x] Remove `Multi` prefix
- [x] Remove `Abstract` asset id

---------

Co-authored-by: command-bot <>
Co-authored-by: default avatarKeith Yeung <[email protected]>
      8428f678
  39. Jan 07, 2024
  41. Jan 04, 2024
  43. Dec 22, 2023
  45. Dec 21, 2023
  47. Dec 20, 2023
    • joe petrowski's avatar
      Add Authorize Upgrade Pattern to Frame System (#2682) · 280aa0b5
      joe petrowski authored 
      Adds the `authorize_upgrade` -> `enact_authorized_upgrade` pattern to
`frame-system`. This will be useful for upgrading bridged chains that
are under the governance of Polkadot without passing entire runtime Wasm
blobs over a bridge.

Notes:

- Changed `enact_authorized_upgrade` to `apply_authorized_upgrade`.
Personal opinion, "apply" more accurately expresses what it's doing. Can
change back if outvoted.
- Remove `check_version` in favor of two extrinsics, so as to make
_checked_ the default.
- Left calls in `parachain-system` and marked as deprecated to prevent
breaking the API. They just call into the `frame-system` functions.
- Updated `frame-system` benchmarks to v2 syntax.

---------

Co-authored-by: command-bot <>
      280aa0b5
  49. Dec 18, 2023
  51. Dec 15, 2023
  53. Dec 14, 2023
    • Francisco Aguirre's avatar
      Add FungibleAdapter (#2684) · 10a91f82
      Francisco Aguirre authored 
      In the move from the old `Currency` traits to the new `fungible/s`
family of traits, we already had the `FungiblesAdapter` and
`NonFungiblesAdapter` for multiple fungible and non fungible assets
respectively. However, for handling only one fungible asset, we were
missing a `FungibleAdapter`, and so used the old `CurrencyAdapter`
instead. This PR aims to fill in that gap, and provide the new adapter
for more updated examples.

I marked the old `CurrencyAdapter` as deprecated as part of this PR, and
I'll change it to the new `FungibleAdapter` in a following PR.
The two stages are separated so as to not bloat this PR with some name
fixes in tests.

---------

Co-authored-by: command-bot <>
      10a91f82
    • Egor_P's avatar
      [Backport] txn version bump from 1.5.0 (#2709) · 3e4e8c0b
      Egor_P authored 
      This PR backports `transaction_version` bump from `1.5.0` release back
to `master`
      3e4e8c0b
  55. Dec 13, 2023
    • Squirrel's avatar
      Set clippy lints in workspace (requires rust 1.74) (#2390) · be8e6268
      Squirrel authored 
      

We currently use a bit of a hack in `.cargo/config` to make sure that
clippy isn't too annoying by specifying the list of lints.

There is now a stable way to define lints for a workspace. The only down
side is that every crate seems to have to opt into this so there's a
*few* files modified in this PR.

Dependencies:

- [x] PR that upgrades CI to use rust 1.74 is merged.

---------

Co-authored-by: default avatarjoe petrowski <[email protected]>
Co-authored-by: default avatarBranislav Kontur <[email protected]>
Co-authored-by: default avatarLiam Aharon <[email protected]>
      be8e6268
    • Branislav Kontur's avatar
      Updated benchmarks related to the Rococo/Westend bridge (#2639) · 4c4407a8
      Branislav Kontur authored 
      Co-authored-by: command-bot <>
      4c4407a8
    • Svyatoslav Nikolsky's avatar
      Tests for BridgeHub(s) <> remote GRANDPA chain (#2692) · 9ecb2d33
      Svyatoslav Nikolsky authored 
      So far the `bridge-hub-test-utils` contained a tests set for testing
BridgeHub runtime that is bridging with the remote **parachain**. But we
have https://github.com/paritytech/polkadot-sdk/pull/2540 coming, which
would add Rococo <> Bulletin chain bridge (where Bulletin = standalone
chain that is using GRANDPA finality). Then it'll be expanded to
Polkadot BH as well.

So this PR adds the same set of tests to the `bridge-hub-test-utils`,
but for the case when remote chain is the chain with GRANDPA finality.
There's a lot of changes in this PR - I'll describe some:
- I've added `BasicParachainRuntime` trait to decrease number of lines
we need to add to `where` clause. Could revert, but imo it is useful;
- `cumulus/parachains/runtimes/bridge-hubs/test-utils/src/test_data` is
a submodule for generating test data for the test sets.
`from_parachain.rs` is used in tests for the case when remote chain is a
parachain, `from_grandpa_chain.rs` - for the bridges with remote GRANDPA
chains. `mod.rs` has some code, shared by both types of tests;
- `cumulus/parachains/runtimes/bridge-hubs/test-utils/src/test_data` is
a submodule with all test cases. The `mod.rs` has tests, suitable for
all cases. There's also `wth_parachain.rs` and `with_grandpa_chain.rs`
with the same meaning as above;
- I've merged the "core" code of two previous tests -
`relayed_incoming_message_works` and `complex_relay_extrinsic_works`
into one single `relayed_incoming_message_works` test. So now we are
always constructing extrinsics and are dispatching them using executive
module (meaning all signed extensions are also tested).

New test set is used here:
https://github.com/paritytech/polkadot-sdk/pull/2540. Once this PR is
merged, I'll merge that other PR with master to remove duplicate
changes.

I'm also planning to cleanup generic constraints + remove some
unnecessary assumptions about used chains in a follow-up PRs. But for
now I think this PR has enough changes, so don't want to complicate it
even more.

---

Breaking changes for the code that have used those tests before:
- the `construct_and_apply_extrinsic` callback now accepts the
`RuntimeCall` instead of the `pallet_utility::Call`;
- the `construct_and_apply_extrinsic` now may be called multiple times
for the single test, so make sure the `frame_system::CheckNonce` is
correctly constructed;
- all previous tests have been moved from
`bridge_hub_test_utils::test_cases` to
`bridge_hub_test_utils::test_cases::from_parachain` module;
- there are several changes in test arguments - please refer to
https://github.com/paritytech/polkadot-sdk/compare/sv-tests-for-bridge-with-remote-grandpa-chain?expand=1#diff-79a28d4d3e1749050341c2424f00c4c139825b1a20937767f83e58b95166735c
for details.
      9ecb2d33
  57. Dec 12, 2023