Skip to content

Update Pallet Referenda to support Block Number Provider#6338

Merged
muharem merged 20 commits intoparitytech:masterfrom
dharjeezy:dami/pallet_referenda_block_number_provider
Feb 7, 2025
Merged

Update Pallet Referenda to support Block Number Provider#6338
muharem merged 20 commits intoparitytech:masterfrom
dharjeezy:dami/pallet_referenda_block_number_provider

Conversation

@dharjeezy
Copy link
Copy Markdown
Contributor

@dharjeezy dharjeezy commented Nov 2, 2024

This PR introduces BlockNumberProvider config for the referenda pallet.
closes part of #6297

Polkadot address: 12GyGD3QhT4i2JJpNzvMf96sxxBLWymz4RdGCxRH5Rj5agKW

@dharjeezy dharjeezy requested a review from a team as a code owner November 2, 2024 11:19
@gui1117 gui1117 added the T2-pallets This PR/Issue is related to a particular pallet. label Nov 4, 2024
@paritytech-review-bot paritytech-review-bot bot requested a review from a team November 18, 2024 14:53
@dharjeezy dharjeezy requested a review from gui1117 November 18, 2024 15:19
@dharjeezy
Copy link
Copy Markdown
Contributor Author

can you help review this again @gui1117

Copy link
Copy Markdown
Member

@bkchr bkchr left a comment

Choose a reason for hiding this comment

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

Could you also provide a migration that migrates from one block number type to another? Especially take into account the different block times.

Copy link
Copy Markdown
Contributor

@gui1117 gui1117 left a comment

Choose a reason for hiding this comment

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

looks good to me.

@acatangiu
Copy link
Copy Markdown
Contributor

@dharjeezy this is nearly done, will you take it over the finish line?

@dharjeezy
Copy link
Copy Markdown
Contributor Author

dharjeezy commented Jan 14, 2025

@dharjeezy this is nearly done, will you take it over the finish line?

i have updated the PR with the migration written. @acatangiu @bkchr

/// The preimage provider.
type Preimages: QueryPreimage<H = Self::Hashing> + StorePreimage;

/// Provider for the block number. Normally this is the `frame_system` pallet.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
/// Provider for the block number. Normally this is the `frame_system` pallet.
/// Provider for the block number.
///
/// Normally this is the `frame_system` pallet.

}
}

pub mod v2 {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
pub mod v2 {
/// Migration for when changing the block number provider.
pub mod switch_block_number_provider {

use super::*;

/// The log target.
const TARGET: &'static str = "runtime::referenda::migration::v2";
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
const TARGET: &'static str = "runtime::referenda::migration::v2";
const TARGET: &'static str = "runtime::referenda::migration::change_block_number_provider";

Comment on lines +187 to +191
pub trait BlockToRelayHeightConversion<T: Config<I>, I: 'static> {
fn convert_block_number_to_relay_height(
block_number: SystemBlockNumberFor<T>,
) -> BlockNumberFor<T, I>;
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
pub trait BlockToRelayHeightConversion<T: Config<I>, I: 'static> {
fn convert_block_number_to_relay_height(
block_number: SystemBlockNumberFor<T>,
) -> BlockNumberFor<T, I>;
}
/// Convert from one to another block number provider/type.
pub trait BlockNumberConversion<Old, New> {
/// Convert the `old` block number type to the new block number type.
///
/// Any changes in the rate of blocks need to be taken into account.
fn convert_block_number(
block_number: Old,
) -> New;
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this is not addressed. @bkchr

Comment on lines +222 to +225
if on_chain_version != 1 {
log::warn!(target: TARGET, "skipping migration from v1 to v2.");
return weight
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
if on_chain_version != 1 {
log::warn!(target: TARGET, "skipping migration from v1 to v2.");
return weight
}

Please add a comment to the migration that this is not guarded.

}

/// Transforms SystemBlockNumberFor<T> to BlockNumberFor<T,I>
pub struct MigrateV1ToV2<BlockConversion, T, I = ()>(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

IMO this should probably just be a standalone function. Downstream people need to write their own migration with some custom guarding or whatever to ensure that they don't apply it multiple times.

@dharjeezy dharjeezy requested a review from bkchr January 16, 2025 14:32
@gui1117 gui1117 self-requested a review January 25, 2025 10:26

impl BlockNumberConversion<SystemBlockNumberFor<T>, BlockNumberFor<T, ()>> for MockBlockConverter {
fn convert_block_number(block_number: SystemBlockNumberFor<T>) -> BlockNumberFor<T, ()> {
block_number as u64
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.

Better to make it not constant ? add a delay like block_number + 10

pub struct MockBlockConverter;

impl BlockNumberConversion<SystemBlockNumberFor<T>, BlockNumberFor<T, ()>> for MockBlockConverter {
fn convert_block_number(block_number: SystemBlockNumberFor<T>) -> BlockNumberFor<T, ()> {
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.

In the mock both are System but I think it is ok.

@gui1117
Copy link
Copy Markdown
Contributor

gui1117 commented Feb 1, 2025

CI is failing because some unused variable

doc:
- audience: Runtime Dev
description: |
This PR makes the referenda pallet uses the relay chain as a block provider for a parachain on a regular schedule.
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.

not sure what you mean with this sentence. the PR provides a new configuration

@muharem
Copy link
Copy Markdown
Contributor

muharem commented Feb 3, 2025

@dharjeezy please check this comment - #6297 (comment)

@muharem muharem added this pull request to the merge queue Feb 7, 2025
Merged via the queue into paritytech:master with commit f08bf1a Feb 7, 2025
140 of 143 checks passed
@gui1117
Copy link
Copy Markdown
Contributor

gui1117 commented Feb 11, 2025

/tip small

@substrate-tip-bot
Copy link
Copy Markdown

Only members of paritytech/tip-bot-approvers have permission to request the creation of the tip referendum from the bot.

However, you can create the tip referendum yourself using Polkassembly or PolkadotJS Apps.

@ggwpez
Copy link
Copy Markdown
Member

ggwpez commented Feb 11, 2025

/tip medium

@substrate-tip-bot
Copy link
Copy Markdown

@ggwpez A referendum for a medium (80 DOT) tip was successfully submitted for @dharjeezy (12GyGD3QhT4i2JJpNzvMf96sxxBLWymz4RdGCxRH5Rj5agKW on polkadot).

Referendum number: 1429.
tip

@substrate-tip-bot
Copy link
Copy Markdown

The referendum has appeared on Polkassembly.

clangenb pushed a commit to clangenb/polkadot-sdk that referenced this pull request Feb 19, 2025
…6338)

This PR introduces BlockNumberProvider config for the referenda pallet.
closes part of paritytech#6297

Polkadot address: 12GyGD3QhT4i2JJpNzvMf96sxxBLWymz4RdGCxRH5Rj5agKW

---------

Co-authored-by: muharem <ismailov.m.h@gmail.com>
gonzamontiel pushed a commit to datahaven-xyz/datahaven that referenced this pull request Mar 26, 2026
## Polkadot upgrade 2503

In this PR, we upgrade form Polkadot SDK 2412 to Polkadot SDK 2503. In
order to upgrade the SDK we need to upgrade some dependencies :
StorageHub and frontier simultaneously.


## What changes 

### Trivial changes

* paritytech/polkadot-sdk#7634 -> The new trait
is required in all the pallets using scale encoding.

* paritytech/polkadot-sdk#7043 -> Remove
deprecated `sp-std` and replace with `alloc` or `core`.

* paritytech/polkadot-sdk#6140 -> Accurate
weight reclaim with frame_system::WeightReclaim


### Breaking changes

* paritytech/polkadot-sdk#2072 -> There is a
change in `pallet-referenda`. Now, the tracks are retrieved as a list of
`Track`s. Also, the names of the tracks might have some trailing null
values (`\0`). This means display representation of the tracks' names
must be sanitized.

* paritytech/polkadot-sdk#5723 -> adds the
ability for these pallets to specify their source of the block number.
This is useful when these pallets are migrated from the relay chain to a
parachain and vice versa. (Not entirely sure it is breaking as it is
being marked as backward compatible).

* paritytech/polkadot-sdk#6338 -> Update
Referenda to Support Block Number Provider

### Notable changes

* paritytech/polkadot-sdk#5703 -> Not changes
required in the codebase but impact fastSync mode. Should improve
testing.

* paritytech/polkadot-sdk#5842 -> Removes
`libp2p` types in authority-discovery, and replace them with network
backend agnostic types from `sc-network-types`. The `sc-network`
interface is therefore updated accordingly.

## What changes in Frontier 

* polkadot-evm/frontier#1693 -> Add support for
EIP 7702 which has been enable with Pectra. This EIP add a new field
`AuthorizationList` in Ethereum transaction.

Changes example :

```rust
#[test]
fn validate_transaction_fails_on_filtered_call() {
...
            pallet_evm::Call::<Runtime>::call {
                source: H160::default(),
                target: H160::default(),
                input: Vec::new(),
                value: sp_core::U256::zero(),
                gas_limit: 21000,
                max_fee_per_gas: sp_core::U256::zero(),
                max_priority_fee_per_gas: Some(sp_core::U256::zero()),
                nonce: None,
                access_list: Vec::new(),
                authorization_list: Vec::new(),
            }
            .into(),
```

## ⚠️ Breaking Changes ⚠️

* Upgrade to Stirage hub v0.5.1
* Support for Ethereum latest upgrade (txs now have the
`authoriation_list` for support EIP 7702)

---------

Co-authored-by: Steve Degosserie <723552+stiiifff@users.noreply.github.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T2-pallets This PR/Issue is related to a particular pallet.

Projects

Status: Audited
Status: Done

Development

Successfully merging this pull request may close these issues.

8 participants