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

Fix collators not producing blocks when selected candidates is zero #2231

Merged
merged 6 commits into from
Apr 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion pallets/parachain-staking/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1716,7 +1716,9 @@ pub mod pallet {
}

/// Compute the top `TotalSelected` candidates in the CandidatePool and return
/// a vec of their AccountIds (sorted by AccountId)
/// a vec of their AccountIds (sorted by AccountId).
///
/// If the returned vec is empty, the previous candidates should be used.
pub fn compute_top_candidates() -> Vec<T::AccountId> {
let top_n = <TotalSelected<T>>::get() as usize;
if top_n == 0 {
Expand Down
9 changes: 8 additions & 1 deletion runtime/common/src/apis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -420,10 +420,17 @@ macro_rules! impl_runtime_apis_plus_common {
// return false if author mapping not registered like in can_author impl
return false
};
let candidates = pallet_parachain_staking::Pallet::<Self>::compute_top_candidates();
if candidates.is_empty() {
// If there are zero selected candidates, we use the same eligibility
// as the previous round
return AuthorInherent::can_author(&author, &slot);
}

// predict eligibility post-selection by computing selection results now
let (eligible, _) =
pallet_author_slot_filter::compute_pseudo_random_subset::<Self>(
pallet_parachain_staking::Pallet::<Self>::compute_top_candidates(),
candidates,
&slot
);
eligible.contains(&author_account_id)
Expand Down
6 changes: 3 additions & 3 deletions runtime/moonbase/tests/common/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@ use moonbase_runtime::{asset_config::AssetRegistrarMetadata, xcm_config::AssetTy
pub use moonbase_runtime::{
currency::{GIGAWEI, SUPPLY_FACTOR, UNIT, WEI},
AccountId, AssetId, AssetManager, Assets, AuthorInherent, Balance, Balances, CrowdloanRewards,
Ethereum, Executive, InflationInfo, LocalAssets, ParachainStaking, Range, Runtime, RuntimeCall,
RuntimeEvent, System, TransactionConverter, TransactionPaymentAsGasPrice, UncheckedExtrinsic,
HOURS, WEEKS,
Ethereum, Executive, Header, InflationInfo, LocalAssets, ParachainStaking, Range, Runtime,
RuntimeCall, RuntimeEvent, System, TransactionConverter, TransactionPaymentAsGasPrice,
UncheckedExtrinsic, HOURS, WEEKS,
};
use nimbus_primitives::{NimbusId, NIMBUS_ENGINE_ID};
use sp_core::{Encode, H160};
Expand Down
91 changes: 91 additions & 0 deletions runtime/moonbase/tests/runtime_apis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,14 @@ mod common;
use common::*;

use fp_evm::GenesisAccount;
use frame_support::assert_ok;
use nimbus_primitives::NimbusId;
use pallet_evm::{Account as EVMAccount, AddressMapping, FeeCalculator};
use sp_core::{ByteArray, H160, H256, U256};

use fp_rpc::runtime_decl_for_EthereumRuntimeRPCApi::EthereumRuntimeRPCApi;
use moonbeam_rpc_primitives_txpool::runtime_decl_for_TxPoolRuntimeApi::TxPoolRuntimeApi;
use nimbus_primitives::runtime_decl_for_NimbusApi::NimbusApi;
use std::{collections::BTreeMap, str::FromStr};

#[test]
Expand Down Expand Up @@ -298,6 +300,95 @@ fn txpool_runtime_api_extrinsic_filter() {
});
}

#[test]
fn can_author_when_selected_is_empty() {
ExtBuilder::default()
.with_balances(vec![
(AccountId::from(ALICE), 20_000_000 * UNIT),
(AccountId::from(BOB), 10_000_000 * UNIT),
])
.with_collators(vec![(AccountId::from(ALICE), 2_000_000 * UNIT)])
.with_mappings(vec![(
NimbusId::from_slice(&ALICE_NIMBUS).unwrap(),
AccountId::from(ALICE),
)])
.build()
.execute_with(|| {
set_parachain_inherent_data();
run_to_block(2, Some(NimbusId::from_slice(&ALICE_NIMBUS).unwrap()));

assert_eq!(ParachainStaking::candidate_pool().0.len(), 1);

let slot_number = 0;
let parent = Header {
digest: Default::default(),
extrinsics_root: Default::default(),
number: Default::default(),
parent_hash: Default::default(),
state_root: Default::default(),
};

// Base case: ALICE can author blocks when she is the only candidate
let can_author_block = Runtime::can_author(
NimbusId::from_slice(&ALICE_NIMBUS).unwrap(),
slot_number,
&parent,
);

assert!(can_author_block);

// Remove ALICE from candidate pool, leaving the candidate_pool empty
assert_ok!(ParachainStaking::go_offline(origin_of(AccountId::from(
ALICE
))));

// Need to fast forward to right before the next session, which is when selected candidates
// will be updated. We want to test the creation of the first block of the next session.
run_to_block(1799, Some(NimbusId::from_slice(&ALICE_NIMBUS).unwrap()));

assert_eq!(ParachainStaking::candidate_pool().0.len(), 0);

let slot_number = 0;
let parent = Header {
digest: Default::default(),
extrinsics_root: Default::default(),
number: 1799,
parent_hash: Default::default(),
state_root: Default::default(),
};

let can_author_block = Runtime::can_author(
NimbusId::from_slice(&ALICE_NIMBUS).unwrap(),
slot_number,
&parent,
);

assert!(can_author_block);

// Check that it works as expected after session update
run_to_block(1800, Some(NimbusId::from_slice(&ALICE_NIMBUS).unwrap()));

assert_eq!(ParachainStaking::candidate_pool().0.len(), 0);

let slot_number = 0;
let parent = Header {
digest: Default::default(),
extrinsics_root: Default::default(),
number: 1800,
parent_hash: Default::default(),
state_root: Default::default(),
};

let can_author_block = Runtime::can_author(
NimbusId::from_slice(&ALICE_NIMBUS).unwrap(),
slot_number,
&parent,
);

assert!(can_author_block);
});
}

// Some Priority-related test ideas
// 1. Eth balance transfer with various gas prices. Priority == gas price
// 2. Eth contract call with various gas prices. Priority == gas price
Expand Down
8 changes: 4 additions & 4 deletions runtime/moonbeam/tests/common/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,9 @@ pub use moonbeam_runtime::{
currency::{GIGAWEI, GLMR, SUPPLY_FACTOR, WEI},
xcm_config::AssetType,
AccountId, AssetId, AssetManager, Assets, AuthorInherent, Balance, Balances, CrowdloanRewards,
Ethereum, Executive, InflationInfo, LocalAssets, ParachainStaking, Range, Runtime, RuntimeCall,
RuntimeEvent, System, TransactionConverter, TransactionPaymentAsGasPrice, UncheckedExtrinsic,
HOURS, WEEKS,
Ethereum, Executive, Header, InflationInfo, LocalAssets, ParachainStaking, Range, Runtime,
RuntimeCall, RuntimeEvent, System, TransactionConverter, TransactionPaymentAsGasPrice,
UncheckedExtrinsic, HOURS, WEEKS,
};
use nimbus_primitives::{NimbusId, NIMBUS_ENGINE_ID};
use sp_core::{Encode, H160};
Expand Down Expand Up @@ -132,7 +132,7 @@ pub struct ExtBuilder {
delegations: Vec<(AccountId, AccountId, Balance, Percent)>,
// per-round inflation config
inflation: InflationInfo<Balance>,
// AuthorId -> AccoutId mappings
// AuthorId -> AccountId mappings
mappings: Vec<(NimbusId, AccountId)>,
// Crowdloan fund
crowdloan_fund: Balance,
Expand Down
91 changes: 91 additions & 0 deletions runtime/moonbeam/tests/runtime_apis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,14 @@ mod common;
use common::*;

use fp_evm::GenesisAccount;
use frame_support::assert_ok;
use nimbus_primitives::NimbusId;
use pallet_evm::{Account as EVMAccount, AddressMapping, FeeCalculator};
use sp_core::{ByteArray, H160, H256, U256};

use fp_rpc::runtime_decl_for_EthereumRuntimeRPCApi::EthereumRuntimeRPCApi;
use moonbeam_rpc_primitives_txpool::runtime_decl_for_TxPoolRuntimeApi::TxPoolRuntimeApi;
use nimbus_primitives::runtime_decl_for_NimbusApi::NimbusApi;
use std::{collections::BTreeMap, str::FromStr};

#[test]
Expand Down Expand Up @@ -300,3 +302,92 @@ fn txpool_runtime_api_extrinsic_filter() {
assert_eq!(txpool.future.len(), 1);
});
}

#[test]
fn can_author_when_selected_is_empty() {
ExtBuilder::default()
.with_balances(vec![
(AccountId::from(ALICE), 20_000_000 * GLMR),
(AccountId::from(BOB), 10_000_000 * GLMR),
])
.with_collators(vec![(AccountId::from(ALICE), 2_000_000 * GLMR)])
.with_mappings(vec![(
NimbusId::from_slice(&ALICE_NIMBUS).unwrap(),
AccountId::from(ALICE),
)])
.build()
.execute_with(|| {
set_parachain_inherent_data();
run_to_block(2, Some(NimbusId::from_slice(&ALICE_NIMBUS).unwrap()));

assert_eq!(ParachainStaking::candidate_pool().0.len(), 1);

let slot_number = 0;
let parent = Header {
digest: Default::default(),
extrinsics_root: Default::default(),
number: Default::default(),
parent_hash: Default::default(),
state_root: Default::default(),
};

// Base case: ALICE can author blocks when she is the only candidate
let can_author_block = Runtime::can_author(
NimbusId::from_slice(&ALICE_NIMBUS).unwrap(),
slot_number,
&parent,
);

assert!(can_author_block);

// Remove ALICE from candidate pool, leaving the candidate_pool empty
assert_ok!(ParachainStaking::go_offline(origin_of(AccountId::from(
ALICE
))));

// Need to fast forward to right before the next session, which is when selected candidates
// will be updated. We want to test the creation of the first block of the next session.
run_to_block(1799, Some(NimbusId::from_slice(&ALICE_NIMBUS).unwrap()));

assert_eq!(ParachainStaking::candidate_pool().0.len(), 0);

let slot_number = 0;
let parent = Header {
digest: Default::default(),
extrinsics_root: Default::default(),
number: 1799,
parent_hash: Default::default(),
state_root: Default::default(),
};

let can_author_block = Runtime::can_author(
NimbusId::from_slice(&ALICE_NIMBUS).unwrap(),
slot_number,
&parent,
);

assert!(can_author_block);

// Check that it works as expected after session update
run_to_block(1800, Some(NimbusId::from_slice(&ALICE_NIMBUS).unwrap()));

assert_eq!(ParachainStaking::candidate_pool().0.len(), 0);

let slot_number = 0;
let parent = Header {
digest: Default::default(),
extrinsics_root: Default::default(),
number: 1800,
parent_hash: Default::default(),
state_root: Default::default(),
};

let can_author_block = Runtime::can_author(
NimbusId::from_slice(&ALICE_NIMBUS).unwrap(),
slot_number,
&parent,
);

assert!(can_author_block);
});
}
6 changes: 3 additions & 3 deletions runtime/moonriver/tests/common/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,9 @@ pub use moonriver_runtime::{
currency::{GIGAWEI, MOVR, SUPPLY_FACTOR, WEI},
xcm_config::AssetType,
AccountId, AssetId, AssetManager, Assets, AuthorInherent, Balance, Balances, CrowdloanRewards,
Ethereum, Executive, InflationInfo, LocalAssets, ParachainStaking, Range, Runtime, RuntimeCall,
RuntimeEvent, System, TransactionConverter, TransactionPaymentAsGasPrice, UncheckedExtrinsic,
HOURS, WEEKS,
Ethereum, Executive, Header, InflationInfo, LocalAssets, ParachainStaking, Range, Runtime,
RuntimeCall, RuntimeEvent, System, TransactionConverter, TransactionPaymentAsGasPrice,
UncheckedExtrinsic, HOURS, WEEKS,
};
use nimbus_primitives::{NimbusId, NIMBUS_ENGINE_ID};
use sp_core::{Encode, H160};
Expand Down
Loading