backing: move the min votes threshold to the runtime #1200

alindima · 2023-08-28T06:51:34Z

Copy of paritytech/polkadot#7577
Also addresses review comments.

For previous reviewers: I maintained the list of commits from previous PR. The only unreviewed commits, are the ones starting with: e7e82bc, which address the review comments

also cache it per-session in the backing subsystem Signed-off-by: alindima <[email protected]>

also enable it for rococo/versi for testing

this dependency has been recently introduced by async backing

this is not needed, as the RuntimeAPISubsystem already takes care of versioning and will return NotSupported if the version is not right.

- parametrise backing votes runtime API with session index - remove RuntimeInfo usage in backing subsystem, as runtime API caches the min backing votes by session index anyway. - move the logic for adjusting the configured needed backing votes with the size of the backing group to a primitives helper. - move the legacy min backing votes value to a primitives helper. - mark JoinMultiple error as fatal, since the Canceled (non-multiple) counterpart is also fatal. - make backing subsystem handle fatal errors for new leaves update. - add HostConfiguration consistency check for zeroed backing votes threshold - add cumulus accompanying change

polkadot/runtime/parachains/src/configuration.rs

…cking-votes-to-runtime

alindima · 2023-08-30T12:14:54Z

@sandreim @eskimor, could you please have another look? I addressed the review comments and I think that if it looks good, it could be merged.

Runtime upgrades are currently failing on Versi even with master code, so the reason is unrelated to this PR.

alindima · 2023-08-30T12:17:35Z

The CI pipeline was cancelled due to failure one of the required jobs. Job name: cargo-clippy Logs: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/3494434

wut? the job passed..

alindima · 2023-08-30T15:02:36Z

I have tested the runtime upgrade locally with zombienet. it works as expected

eskimor · 2023-08-31T09:04:01Z

cumulus/client/relay-chain-rpc-interface/src/rpc_client.rs

@@ -588,6 +588,16 @@ impl RelayChainRpcClient {
 .await
 }

+ /// Get the minimum number of backing votes for a candidate.
+ pub async fn parachain_host_minimum_backing_votes(


Is this even needed in cumulus? It should not.

It doesn't compile without it, I see all runtime APIs have counterparts here.
I'm not sure why they're needed though

I had similar question some time ago, and got this suggestion from @skunert: paritytech/cumulus#2160 (comment)

I don't fully agree with that. For UX simplicity, I think the interface should expose the bare necessities for collators.

I think we can open a separate issue to discuss this further

sandreim · 2023-08-31T09:34:11Z

cumulus/client/relay-chain-rpc-interface/src/rpc_client.rs

@@ -588,6 +588,16 @@ impl RelayChainRpcClient {
 .await
 }

+ /// Get the minimum number of backing votes for a candidate.
+ pub async fn parachain_host_minimum_backing_votes(


I don't fully agree with that. For UX simplicity, I think the interface should expose the bare necessities for collators.

sandreim · 2023-08-31T10:22:44Z

polkadot/runtime/kusama/src/lib.rs

@@ -1741,6 +1741,8 @@ pub mod migrations {

 // Upgrade SessionKeys to include BEEFY key
 UpgradeSessionKeys,
+


nit: a comment describing what the migration does is useful.

* master: Sassafras primitives (#1249) Restructure `dispatch` macro related exports (#1162) backing: move the min votes threshold to the runtime (#1200) Bump zstd from 0.11.2+zstd.1.5.2 to 0.12.4 (#1326) Remove `substrate_test_utils::test` (#1321) remove disable-runtime-api (#1328)

* master: (25 commits) fix typos (#1339) Use bandersnatch-vrfs with locked dependencies ref (#1342) Bump bs58 from 0.4.0 to 0.5.0 (#1293) Contracts: `seal0::balance` should return the free balance (#1254) Logs: add extra debug log for negative rep changes (#1205) Added short-benchmarks for cumulus (#1183) [xcm-emulator] Improve hygiene and clean up (#1301) Bump the known_good_semver group with 1 update (#1347) Renames API (#1186) Rename `polkadot-parachain` to `polkadot-parachain-primitives` (#1334) Add README to project root (#1253) Add environmental variable to track decoded instructions (#1320) Fix polkadot-node-core-pvf-prepare-worker build with jemalloc (#1315) Sassafras primitives (#1249) Restructure `dispatch` macro related exports (#1162) backing: move the min votes threshold to the runtime (#1200) Bump zstd from 0.11.2+zstd.1.5.2 to 0.12.4 (#1326) Remove `substrate_test_utils::test` (#1321) remove disable-runtime-api (#1328) [ci] add more jobs for pipeline cancel, cleanup (#1314) ...

* move min backing votes const to runtime also cache it per-session in the backing subsystem Signed-off-by: alindima <[email protected]> * add runtime migration * introduce api versioning for min_backing votes also enable it for rococo/versi for testing * also add min_backing_votes runtime calls to statement-distribution this dependency has been recently introduced by async backing * remove explicit version runtime API call this is not needed, as the RuntimeAPISubsystem already takes care of versioning and will return NotSupported if the version is not right. * address review comments - parametrise backing votes runtime API with session index - remove RuntimeInfo usage in backing subsystem, as runtime API caches the min backing votes by session index anyway. - move the logic for adjusting the configured needed backing votes with the size of the backing group to a primitives helper. - move the legacy min backing votes value to a primitives helper. - mark JoinMultiple error as fatal, since the Canceled (non-multiple) counterpart is also fatal. - make backing subsystem handle fatal errors for new leaves update. - add HostConfiguration consistency check for zeroed backing votes threshold - add cumulus accompanying change * fix cumulus test compilation * fix tests * more small fixes * fix merge * bump runtime api version for westend and rollback version for rococo --------- Signed-off-by: alindima <[email protected]> Co-authored-by: Javier Viola <[email protected]>

alindima added 6 commits August 28, 2023 09:20

move min backing votes const to runtime

f140a37

also cache it per-session in the backing subsystem Signed-off-by: alindima <[email protected]>

add runtime migration

a6664a4

introduce api versioning for min_backing votes

62783af

also enable it for rococo/versi for testing

also add min_backing_votes runtime calls to statement-distribution

8718818

this dependency has been recently introduced by async backing

remove explicit version runtime API call

be44e33

this is not needed, as the RuntimeAPISubsystem already takes care of versioning and will return NotSupported if the version is not right.

paritytech-ci requested review from a team August 28, 2023 06:53

fix cumulus test compilation

533f6c9

alindima added A0-needs_burnin Pull request needs to be tested on a live validator node before merge. DevOps is notified via matrix T4-runtime_API This PR/Issue is related to runtime APIs. T8-polkadot This PR/Issue is related to/affects the Polkadot network. labels Aug 28, 2023

fix tests

8adf595

eskimor reviewed Aug 28, 2023

View reviewed changes

polkadot/runtime/parachains/src/configuration.rs Outdated Show resolved Hide resolved

paritytech-ci requested review from a team August 28, 2023 08:38

alindima and others added 5 commits August 28, 2023 11:53

more small fixes

34fa138

Merge remote-tracking branch 'origin/master' into alindima-add-min-ba…

200537e

…cking-votes-to-runtime

fix merge

3f594a0

Merge branch 'master' into alindima-add-min-backing-votes-to-runtime

fa0a515

bump runtime api version for westend and rollback version for rococo

cd6485b

Merge branch 'master' into alindima-add-min-backing-votes-to-runtime

75bc7e6

Merge branch 'master' into alindima-add-min-backing-votes-to-runtime

8c1cb7f

eskimor reviewed Aug 31, 2023

View reviewed changes

eskimor approved these changes Aug 31, 2023

View reviewed changes

sandreim approved these changes Aug 31, 2023

View reviewed changes

alindima merged commit d6af073 into master Aug 31, 2023
103 of 105 checks passed

alindima deleted the alindima-add-min-backing-votes-to-runtime branch August 31, 2023 11:01

alindima mentioned this pull request Aug 31, 2023

Consider restricting cumulus access to unnecessary runtime APIs #1332

Open

crystalin mentioned this pull request Oct 20, 2023

Update substrate/polkadot/cumulus from v0.9.43 to v1.1.0 moonbeam-foundation/moonbeam#2535

Closed

serban300 pushed a commit to serban300/polkadot-sdk that referenced this pull request Apr 8, 2024

fix clippy things (paritytech#1200)

d39727f

serban300 pushed a commit to serban300/polkadot-sdk that referenced this pull request Apr 8, 2024

fix clippy things (paritytech#1200)

b86b182

serban300 pushed a commit to serban300/polkadot-sdk that referenced this pull request Apr 8, 2024

fix clippy things (paritytech#1200)

f128adc

serban300 pushed a commit to serban300/polkadot-sdk that referenced this pull request Apr 8, 2024

fix clippy things (paritytech#1200)

db2172b

serban300 pushed a commit to serban300/polkadot-sdk that referenced this pull request Apr 9, 2024

fix clippy things (paritytech#1200)

1fb9382

serban300 pushed a commit to serban300/polkadot-sdk that referenced this pull request Apr 9, 2024

fix clippy things (paritytech#1200)

bb43ca6

serban300 pushed a commit to serban300/polkadot-sdk that referenced this pull request Apr 9, 2024

fix clippy things (paritytech#1200)

3e1cd3c

serban300 pushed a commit to serban300/polkadot-sdk that referenced this pull request Apr 9, 2024

fix clippy things (paritytech#1200)

2367e97

serban300 pushed a commit to serban300/polkadot-sdk that referenced this pull request Apr 9, 2024

fix clippy things (paritytech#1200)

2f06184

serban300 pushed a commit to serban300/polkadot-sdk that referenced this pull request Apr 9, 2024

fix clippy things (paritytech#1200)

488b35f

serban300 pushed a commit to serban300/polkadot-sdk that referenced this pull request Apr 10, 2024

fix clippy things (paritytech#1200)

2b96209

serban300 pushed a commit to serban300/polkadot-sdk that referenced this pull request Apr 10, 2024

fix clippy things (paritytech#1200)

bd73d85

bkchr pushed a commit that referenced this pull request Apr 10, 2024

fix clippy things (#1200)

c2b38ba

TomaszWaszczyk pushed a commit to TomaszWaszczyk/polkadot-sdk that referenced this pull request May 27, 2024

fix clippy things (paritytech#1200)

534af22

This was referenced Jun 5, 2024

Update polkadot-sdk from v1.7.0 to v1.11.0 moondance-labs/tanssi#573

Closed

Update polkadot-sdk from v1.10.0 to v1.11.0 moondance-labs/tanssi#577

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

backing: move the min votes threshold to the runtime #1200

backing: move the min votes threshold to the runtime #1200

alindima commented Aug 28, 2023 •

edited

alindima commented Aug 30, 2023

alindima commented Aug 30, 2023

alindima commented Aug 30, 2023

eskimor Aug 31, 2023

alindima Aug 31, 2023

ordian Aug 31, 2023

sandreim Aug 31, 2023

alindima Aug 31, 2023

alindima Aug 31, 2023

sandreim Aug 31, 2023

sandreim Aug 31, 2023

		@@ -1741,6 +1741,8 @@ pub mod migrations {

		// Upgrade SessionKeys to include BEEFY key
		UpgradeSessionKeys,

backing: move the min votes threshold to the runtime #1200

backing: move the min votes threshold to the runtime #1200

Conversation

alindima commented Aug 28, 2023 • edited

alindima commented Aug 30, 2023

alindima commented Aug 30, 2023

alindima commented Aug 30, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alindima commented Aug 28, 2023 •

edited