Skip to content

Commit a84dd0d

Browse files
authored
Approve multiple candidates with a single signature (#1191)
Initial implementation for the plan discussed here: #701 Built on top of #1178 v0: paritytech/polkadot#7554, ## Overall idea When approval-voting checks a candidate and is ready to advertise the approval, defer it in a per-relay chain block until we either have MAX_APPROVAL_COALESCE_COUNT candidates to sign or a candidate has stayed MAX_APPROVALS_COALESCE_TICKS in the queue, in both cases we sign what candidates we have available. This should allow us to reduce the number of approvals messages we have to create/send/verify. The parameters are configurable, so we should find some values that balance: - Security of the network: Delaying broadcasting of an approval shouldn't but the finality at risk and to make sure that never happens we won't delay sending a vote if we are past 2/3 from the no-show time. - Scalability of the network: MAX_APPROVAL_COALESCE_COUNT = 1 & MAX_APPROVALS_COALESCE_TICKS =0, is what we have now and we know from the measurements we did on versi, it bottlenecks approval-distribution/approval-voting when increase significantly the number of validators and parachains - Block storage: In case of disputes we have to import this votes on chain and that increase the necessary storage with MAX_APPROVAL_COALESCE_COUNT * CandidateHash per vote. Given that disputes are not the normal way of the network functioning and we will limit MAX_APPROVAL_COALESCE_COUNT in the single digits numbers, this should be good enough. Alternatively, we could try to create a better way to store this on-chain through indirection, if that's needed. ## Other fixes: - Fixed the fact that we were sending random assignments to non-validators, that was wrong because those won't do anything with it and they won't gossip it either because they do not have a grid topology set, so we would waste the random assignments. - Added metrics to be able to debug potential no-shows and mis-processing of approvals/assignments. ## TODO: - [x] Get feedback, that this is moving in the right direction. @ordian @sandreim @eskimor @burdges, let me know what you think. - [x] More and more testing. - [x] Test in versi. - [x] Make MAX_APPROVAL_COALESCE_COUNT & MAX_APPROVAL_COALESCE_WAIT_MILLIS a parachain host configuration. - [x] Make sure the backwards compatibility works correctly - [x] Make sure this direction is compatible with other streams of work: #635 & #742 - [x] Final versi burn-in before merging --------- Signed-off-by: Alexandru Gheorghe <[email protected]>
1 parent d18a682 commit a84dd0d

File tree

82 files changed

+5878
-1478
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

82 files changed

+5878
-1478
lines changed

.gitlab/pipeline/zombienet/polkadot.yml

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,14 @@ zombienet-polkadot-functional-0008-dispute-old-finalized:
131131
--local-dir="${LOCAL_DIR}/functional"
132132
--test="0008-dispute-old-finalized.zndsl"
133133

134+
zombienet-polkadot-functional-0009-approval-voting-coalescing:
135+
extends:
136+
- .zombienet-polkadot-common
137+
script:
138+
- /home/nonroot/zombie-net/scripts/ci/run-test-local-env-manager.sh
139+
--local-dir="${LOCAL_DIR}/functional"
140+
--test="0009-approval-voting-coalescing.zndsl"
141+
134142
zombienet-polkadot-smoke-0001-parachains-smoke-test:
135143
extends:
136144
- .zombienet-polkadot-common

cumulus/client/relay-chain-minimal-node/src/blockchain_rpc_client.rs

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ use polkadot_overseer::{ChainApiBackend, RuntimeApiSubsystemClient};
2424
use polkadot_primitives::{
2525
async_backing::{AsyncBackingParams, BackingState},
2626
slashing,
27-
vstaging::NodeFeatures,
27+
vstaging::{ApprovalVotingParams, NodeFeatures},
2828
};
2929
use sc_authority_discovery::{AuthorityDiscovery, Error as AuthorityDiscoveryError};
3030
use sc_client_api::AuxStore;
@@ -427,6 +427,18 @@ impl RuntimeApiSubsystemClient for BlockChainRpcClient {
427427
Ok(self.rpc_client.parachain_host_para_backing_state(at, para_id).await?)
428428
}
429429

430+
/// Approval voting configuration parameters
431+
async fn approval_voting_params(
432+
&self,
433+
at: Hash,
434+
session_index: polkadot_primitives::SessionIndex,
435+
) -> Result<ApprovalVotingParams, ApiError> {
436+
Ok(self
437+
.rpc_client
438+
.parachain_host_staging_approval_voting_params(at, session_index)
439+
.await?)
440+
}
441+
430442
async fn node_features(&self, at: Hash) -> Result<NodeFeatures, ApiError> {
431443
Ok(self.rpc_client.parachain_host_node_features(at).await?)
432444
}

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

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ use cumulus_primitives_core::{
3232
relay_chain::{
3333
async_backing::{AsyncBackingParams, BackingState},
3434
slashing,
35-
vstaging::NodeFeatures,
35+
vstaging::{ApprovalVotingParams, NodeFeatures},
3636
BlockNumber, CandidateCommitments, CandidateEvent, CandidateHash,
3737
CommittedCandidateReceipt, CoreState, DisputeState, ExecutorParams, GroupRotationInfo,
3838
Hash as RelayHash, Header as RelayHeader, InboundHrmpMessage, OccupiedCoreAssumption,
@@ -625,6 +625,19 @@ impl RelayChainRpcClient {
625625
}
626626

627627
#[allow(missing_docs)]
628+
pub async fn parachain_host_staging_approval_voting_params(
629+
&self,
630+
at: RelayHash,
631+
_session_index: SessionIndex,
632+
) -> Result<ApprovalVotingParams, RelayChainError> {
633+
self.call_remote_runtime_function(
634+
"ParachainHost_staging_approval_voting_params",
635+
at,
636+
None::<()>,
637+
)
638+
.await
639+
}
640+
628641
pub async fn parachain_host_para_backing_state(
629642
&self,
630643
at: RelayHash,

cumulus/parachains/integration-tests/emulated/chains/relays/rococo/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ use emulated_integration_tests_common::{
2424

2525
// Rococo declaration
2626
decl_test_relay_chains! {
27-
#[api_version(9)]
27+
#[api_version(10)]
2828
pub struct Rococo {
2929
genesis = genesis::genesis(),
3030
on_init = (),

cumulus/parachains/integration-tests/emulated/chains/relays/westend/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ use emulated_integration_tests_common::{
2424

2525
// Westend declaration
2626
decl_test_relay_chains! {
27-
#[api_version(9)]
27+
#[api_version(10)]
2828
pub struct Westend {
2929
genesis = genesis::genesis(),
3030
on_init = (),

polkadot/Cargo.toml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,6 @@ jemalloc-allocator = [
6464
"polkadot-node-core-pvf/jemalloc-allocator",
6565
"polkadot-overseer/jemalloc-allocator",
6666
]
67-
network-protocol-staging = ["polkadot-cli/network-protocol-staging"]
6867

6968

7069
# Enables timeout-based tests supposed to be run only in CI environment as they may be flaky

polkadot/cli/Cargo.toml

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,5 +75,3 @@ runtime-metrics = [
7575
"polkadot-node-metrics/runtime-metrics",
7676
"service/runtime-metrics",
7777
]
78-
79-
network-protocol-staging = ["service/network-protocol-staging"]

0 commit comments

Comments
 (0)