Skip to content

Commit

Permalink
Fix collators not producing blocks when selected candidates is zero (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
tmpolaczyk authored Apr 21, 2023
1 parent e8eef18 commit fb0c237
Show file tree
Hide file tree
Showing 8 changed files with 294 additions and 12 deletions.
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

0 comments on commit fb0c237

Please sign in to comment.