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

Add BEEFY capabilities to the Polkadot runtime #65

Merged

Conversation

serban300
Copy link
Contributor

@serban300 serban300 commented Oct 17, 2023

This adds BEEFY capabilities to the Polkadot runtime.

Governance/sudo call is later required to enable/start consensus.

Part of paritytech/parity-bridges-common#2420

Copy link
Contributor

@acatangiu acatangiu left a comment

Choose a reason for hiding this comment

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

Enabling BEEFY on Polkadot will happen after a couple of weeks of it running on Kusama without problems, but let's get the code in a release sooner rather than later to be ready for it.

There is a GenesisBlock storage item in pallet-beefy that defaults to None meaning BEEFY consensus is not enabled/running.

Privileged set_new_genesis() will be called through governance at a later point when we decide to enable BEEFY.

serban300 added a commit to paritytech/polkadot-sdk that referenced this pull request Oct 18, 2023
Fellowship companion:
polkadot-fellows/runtimes#65

This starts the BEEFY client by default for Polkadot nodes.

Governance/sudo call is later required to enable/start consensus.

Part of paritytech/parity-bridges-common#2420
@tomaka
Copy link
Contributor

tomaka commented Oct 18, 2023

Is there some specification or document detailing how Beefy works available somewhere?

@acatangiu
Copy link
Contributor

acatangiu commented Oct 18, 2023

Is there some specification or document detailing how Beefy works available somewhere?

Also not yet merged anywhere but pretty comprehensive:

@acatangiu acatangiu mentioned this pull request Oct 19, 2023
@joepetrowski joepetrowski mentioned this pull request Oct 20, 2023
tdimitrov pushed a commit to paritytech/polkadot-sdk that referenced this pull request Oct 23, 2023
Fellowship companion:
polkadot-fellows/runtimes#65

This starts the BEEFY client by default for Polkadot nodes.

Governance/sudo call is later required to enable/start consensus.

Part of paritytech/parity-bridges-common#2420
Comment on lines +1737 to +1745
/// Upgrade Session keys to include BEEFY key.
/// When this is removed, should also remove `OldSessionKeys`.
pub struct UpgradeSessionKeys;
impl frame_support::traits::OnRuntimeUpgrade for UpgradeSessionKeys {
fn on_runtime_upgrade() -> Weight {
Session::upgrade_keys::<OldSessionKeys, _>(transform_session_keys);
Perbill::from_percent(50) * BlockWeights::get().max_block
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this have any protection to not run twice?

Copy link
Contributor

Choose a reason for hiding this comment

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

Take a look at https://github.com/paritytech/polkadot-sdk/pull/2265/files#diff-da220c6c064cc047fef5d3b4a594ae30b8bbf48a29b740742bc9858285c7ba57R1572 incl. try-runtime checks.
That might require more recent version of the system pallet though.

Copy link
Contributor

Choose a reason for hiding this comment

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

Problem is that last_runtime_upgrade_spec_version() is not in this release of polkadot-sdk AFAIK.

Copy link
Contributor

Choose a reason for hiding this comment

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

But we can copy the try runtime stuff.

Copy link
Contributor

Choose a reason for hiding this comment

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

type MaxNominators = MaxNominatorRewardedPerValidator;
type MaxSetIdSessionEntries = BeefySetIdSessionEntries;
type OnNewValidatorSet = BeefyMmrLeaf;
type WeightInfo = ();
Copy link
Contributor

Choose a reason for hiding this comment

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

Why does this use the default and not add Beefy to runtime benchmarks?

Copy link
Contributor

Choose a reason for hiding this comment

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

Same question for Kusama.

Copy link
Contributor

Choose a reason for hiding this comment

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

const INDEXING_PREFIX: &'static [u8] = mmr::INDEXING_PREFIX;
type Hashing = Keccak256;
type OnNewRoot = pallet_beefy_mmr::DepositBeefyDigest<Runtime>;
type WeightInfo = ();
Copy link
Contributor

Choose a reason for hiding this comment

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

Same

Copy link
Contributor

Choose a reason for hiding this comment

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

@@ -2501,7 +2668,7 @@ mod test {

#[test]
fn call_size() {
RuntimeCall::assert_size_under(230);
RuntimeCall::assert_size_under(256);
Copy link
Member

Choose a reason for hiding this comment

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

It probably increased from one of the pallet args? In the past we used Box to keep it small.

@bkchr
Copy link
Contributor

bkchr commented Dec 8, 2023

/merge

@fellowship-merge-bot fellowship-merge-bot bot enabled auto-merge (squash) December 8, 2023 21:47
@fellowship-merge-bot
Copy link
Contributor

Enabled auto-merge in Pull Request

Available commands
  • /merge: Enables auto-merge for Pull Request
  • /merge cancel: Cancels auto-merge for Pull Request
  • /merge help: Shows this menu

For more information see the documentation

@fellowship-merge-bot fellowship-merge-bot bot merged commit 9bbd9d2 into polkadot-fellows:main Dec 8, 2023
14 checks passed
@acatangiu acatangiu mentioned this pull request Jan 18, 2024
19 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants