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

Construct Runtime v2 #1378

Merged
merged 64 commits into from
Mar 13, 2024
Merged

Construct Runtime v2 #1378

merged 64 commits into from
Mar 13, 2024

Conversation

gupnik
Copy link
Contributor

@gupnik gupnik commented Sep 4, 2023

Moved from paritytech/substrate#14788


Fixes #232

This PR introduces outer-macro approach for construct_runtime as discussed in the linked issue. It looks like the following:

#[frame_support::runtime]
mod runtime {
	#[runtime::runtime]
        #[runtime::derive(
		RuntimeCall,
		RuntimeEvent,
		RuntimeError,
		RuntimeOrigin,
		RuntimeFreezeReason,
		RuntimeHoldReason,
		RuntimeSlashReason,
		RuntimeLockId,
                RuntimeTask,
	)]
	pub struct Runtime;

	#[runtime::pallet_index(0)]
	pub type System = frame_system;

	#[runtime::pallet_index(1)]
	pub type Timestamp = pallet_timestamp;

	#[runtime::pallet_index(2)]
	pub type Aura = pallet_aura;

	#[runtime::pallet_index(3)]
	pub type Grandpa = pallet_grandpa;

	#[runtime::pallet_index(4)]
	pub type Balances = pallet_balances;

	#[runtime::pallet_index(5)]
	pub type TransactionPayment = pallet_transaction_payment;

	#[runtime::pallet_index(6)]
	pub type Sudo = pallet_sudo;

	// Include the custom logic from the pallet-template in the runtime.
	#[runtime::pallet_index(7)]
	pub type TemplateModule = pallet_template;
}

Features

  • #[runtime::runtime] attached to a struct defines the main runtime
  • #[runtime::derive] attached to this struct defines the types generated by runtime
  • #[runtime::pallet_index] must be attached to a pallet to define its index
  • #[runtime::disable_call] can be optionally attached to a pallet to disable its calls
  • #[runtime::disable_unsigned] can be optionally attached to a pallet to disable unsigned calls
  • A pallet instance can be defined as TemplateModule: pallet_template<Instance>
  • An optional attribute can be defined as #[frame_support::runtime(legacy_ordering)] to ensure that the order of hooks is same as the order of pallets (and not based on the pallet_index). This is to support legacy runtimes and should be avoided for new ones.

Todo

  • Update the latest syntax in kitchensink and tests
  • Update UI tests
  • Docs

Extension

@gupnik gupnik added the T1-FRAME This PR/Issue is related to core FRAME, the framework. label Sep 4, 2023
@gilescope
Copy link
Contributor

I'd like to see that we fully respect the ordering based on pallet-index. I.e. the order of AllPallets tuple etc. should be driven off pallet-index rather than having some things sorted by pallet-index and some by lexical. It would be a breaking change but the current mixture of orderings is a footgun.

@gupnik gupnik changed the title [WIP]: Construct Runtime v2 Construct Runtime v2 Sep 13, 2023
@gupnik gupnik marked this pull request as ready for review September 13, 2023 06:20
@gupnik gupnik requested review from a team September 13, 2023 06:20
@KiChjang
Copy link
Contributor

Could we in fact use #[derive] directly and remove #[frame::derive]? Feels like we can replace it with derive macros.

@gupnik
Copy link
Contributor Author

gupnik commented Sep 14, 2023

Could we in fact use #[derive] directly and remove #[frame::derive]? Feels like we can replace it with derive macros.

I saw a couple of issues with that:

  1. They won't be able to access outside info (for now it's just the struct Runtime but we will probably have more down the line like Executive, etc.)
  2. Each of them would need to parse the Pallets struct with both intermediate and final expansion (using tt_default_parts). That might increase the build time and is probably unneccessary?

@sam0x17
Copy link
Contributor

sam0x17 commented Sep 15, 2023

  1. They won't be able to access outside info (for now it's just the struct Runtime but we will probably have more down the line like Executive, etc.)

Well we're in an outer macro pattern already so it doesn't have to be a real derive macro, it's just syntactic sugar. You could have something in construct_runtime_v2 that looks for #[derive()] containing this, pulls it out of the list of derives for that item, and then expands accordingly

Copy link
Contributor

@sam0x17 sam0x17 left a comment

Choose a reason for hiding this comment

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

Looking great, very cool this is happening, few comments and things to discuss 👍🏻

@gupnik
Copy link
Contributor Author

gupnik commented Feb 28, 2024

Quick question, Does that new syntax let me define a generic type on the Runtime?

// pub struct Runtime;
pub struct Runtime<T>(core::marker::PhantomData<T>);

This could be handy in tests to easily test various associated types

impl<T> Config for Runtime<T> {
    type Foo = T;
}

Not sure I completely follow as Runtime is supposed to be a concrete type that provides an implementation for all pallet configs.

What would be an example of the associated type that you want to test here?

@pgherveou
Copy link
Contributor

Not sure I completely follow as Runtime is supposed to be a concrete type that provides an implementation for all pallet configs.

What would be an example of the associated type that you want to test here?

I mean the runtime would be concrete type such as Runtime<()>
I recently stumbled upon this code that creates a test runtime inside a macro. The only reason this macro exists instead of calling it directly is to let the user specify a custom ChainExtension type. Thus I was wondering if this could be avoided with v2

@gupnik
Copy link
Contributor Author

gupnik commented Feb 29, 2024

Not sure I completely follow as Runtime is supposed to be a concrete type that provides an implementation for all pallet configs.
What would be an example of the associated type that you want to test here?

I mean the runtime would be concrete type such as Runtime<()> I recently stumbled upon this code that creates a test runtime inside a macro. The only reason this macro exists instead of calling it directly is to let the user specify a custom ChainExtension type. Thus I was wondering if this could be avoided with v2

Thanks - I think this is a very specific use-case that should probably be handled the way it's done currently?

Copy link
Member

@ggwpez ggwpez left a comment

Choose a reason for hiding this comment

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

Can we still mark this as experimental by gating it behind that feature flag?
Just until we tested it on Rococo/Westend to be sure that it works.

@gupnik gupnik added this pull request to the merge queue Mar 13, 2024
Merged via the queue into master with commit 82f3c3e Mar 13, 2024
129 of 130 checks passed
@gupnik gupnik deleted the gupnik/crv2 branch March 13, 2024 07:35
ordian added a commit that referenced this pull request Mar 16, 2024
* master: (65 commits)
  collator protocol changes for elastic scaling (validator side) (#3302)
  Contracts use polkavm workspace deps (#3715)
  Add elastic scaling support in ParaInherent BenchBuilder  (#3690)
  Removes `as [disambiguation_path]` from `derive_impl` usage (#3652)
  fix(paseo-spec): New Paseo Bootnodes (#3674)
  Improve Penpal runtime + emulated tests (#3543)
  Staking ledger bonding fixes (#3639)
  DescribeAllTerminal for HashedDescription (#3349)
  Increase timeout for assertions (#3680)
  Add subsystems regression tests to CI (#3527)
  Always print connectivity report (#3677)
  Revert "FRAME: Create `TransactionExtension` as a replacement for `SignedExtension` (#2280)" (#3665)
  authority-discovery: Add log for debugging DHT authority records (#3668)
  Construct Runtime v2  (#1378)
  Support for `keyring` in runtimes (#2044)
  Add api-name in `cannot query the runtime API version` warning (#3653)
  Add a PolkaVM-based executor (#3458)
  Adds default config for assets pallet (#3637)
  Bump handlebars from 4.3.7 to 5.1.0 (#3248)
  [Collator Selection] Fix weight refund for `set_candidacy_bond` (#3643)
  ...
dharjeezy pushed a commit to dharjeezy/polkadot-sdk that referenced this pull request Mar 24, 2024
Moved from paritytech/substrate#14788

----

Fixes paritytech#232

This PR introduces outer-macro approach for `construct_runtime` as
discussed in the linked issue. It looks like the following:
```rust
#[frame_support::runtime]
mod runtime {
	#[runtime::runtime]
        #[runtime::derive(
		RuntimeCall,
		RuntimeEvent,
		RuntimeError,
		RuntimeOrigin,
		RuntimeFreezeReason,
		RuntimeHoldReason,
		RuntimeSlashReason,
		RuntimeLockId,
                RuntimeTask,
	)]
	pub struct Runtime;

	#[runtime::pallet_index(0)]
	pub type System = frame_system;

	#[runtime::pallet_index(1)]
	pub type Timestamp = pallet_timestamp;

	#[runtime::pallet_index(2)]
	pub type Aura = pallet_aura;

	#[runtime::pallet_index(3)]
	pub type Grandpa = pallet_grandpa;

	#[runtime::pallet_index(4)]
	pub type Balances = pallet_balances;

	#[runtime::pallet_index(5)]
	pub type TransactionPayment = pallet_transaction_payment;

	#[runtime::pallet_index(6)]
	pub type Sudo = pallet_sudo;

	// Include the custom logic from the pallet-template in the runtime.
	#[runtime::pallet_index(7)]
	pub type TemplateModule = pallet_template;
}
```

## Features
- `#[runtime::runtime]` attached to a struct defines the main runtime
- `#[runtime::derive]` attached to this struct defines the types
generated by runtime
- `#[runtime::pallet_index]` must be attached to a pallet to define its
index
- `#[runtime::disable_call]` can be optionally attached to a pallet to
disable its calls
- `#[runtime::disable_unsigned]` can be optionally attached to a pallet
to disable unsigned calls
- A pallet instance can be defined as `TemplateModule:
pallet_template<Instance>`
- An optional attribute can be defined as
`#[frame_support::runtime(legacy_ordering)]` to ensure that the order of
hooks is same as the order of pallets (and not based on the
pallet_index). This is to support legacy runtimes and should be avoided
for new ones.

## Todo
- [x] Update the latest syntax in kitchensink and tests
- [x] Update UI tests
- [x] Docs

## Extension
- Abstract away the Executive similar to
paritytech/substrate#14742
- Optionally avoid the need to specify all runtime types (TBD)

---------

Co-authored-by: Francisco Aguirre <[email protected]>
Co-authored-by: Nikhil Gupta <>
bgallois pushed a commit to duniter/duniter-polkadot-sdk that referenced this pull request Mar 25, 2024
Moved from paritytech/substrate#14788

----

Fixes paritytech#232

This PR introduces outer-macro approach for `construct_runtime` as
discussed in the linked issue. It looks like the following:
```rust
#[frame_support::runtime]
mod runtime {
	#[runtime::runtime]
        #[runtime::derive(
		RuntimeCall,
		RuntimeEvent,
		RuntimeError,
		RuntimeOrigin,
		RuntimeFreezeReason,
		RuntimeHoldReason,
		RuntimeSlashReason,
		RuntimeLockId,
                RuntimeTask,
	)]
	pub struct Runtime;

	#[runtime::pallet_index(0)]
	pub type System = frame_system;

	#[runtime::pallet_index(1)]
	pub type Timestamp = pallet_timestamp;

	#[runtime::pallet_index(2)]
	pub type Aura = pallet_aura;

	#[runtime::pallet_index(3)]
	pub type Grandpa = pallet_grandpa;

	#[runtime::pallet_index(4)]
	pub type Balances = pallet_balances;

	#[runtime::pallet_index(5)]
	pub type TransactionPayment = pallet_transaction_payment;

	#[runtime::pallet_index(6)]
	pub type Sudo = pallet_sudo;

	// Include the custom logic from the pallet-template in the runtime.
	#[runtime::pallet_index(7)]
	pub type TemplateModule = pallet_template;
}
```

## Features
- `#[runtime::runtime]` attached to a struct defines the main runtime
- `#[runtime::derive]` attached to this struct defines the types
generated by runtime
- `#[runtime::pallet_index]` must be attached to a pallet to define its
index
- `#[runtime::disable_call]` can be optionally attached to a pallet to
disable its calls
- `#[runtime::disable_unsigned]` can be optionally attached to a pallet
to disable unsigned calls
- A pallet instance can be defined as `TemplateModule:
pallet_template<Instance>`
- An optional attribute can be defined as
`#[frame_support::runtime(legacy_ordering)]` to ensure that the order of
hooks is same as the order of pallets (and not based on the
pallet_index). This is to support legacy runtimes and should be avoided
for new ones.

## Todo
- [x] Update the latest syntax in kitchensink and tests
- [x] Update UI tests
- [x] Docs

## Extension
- Abstract away the Executive similar to
paritytech/substrate#14742
- Optionally avoid the need to specify all runtime types (TBD)

---------

Co-authored-by: Francisco Aguirre <[email protected]>
Co-authored-by: Nikhil Gupta <>
@Polkadot-Forum
Copy link

This pull request has been mentioned on Polkadot Forum. There might be relevant details there:

https://forum.polkadot.network/t/parity-tech-update-for-march/7226/1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T1-FRAME This PR/Issue is related to core FRAME, the framework.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Make construct_runtime macro more flexible