diff --git a/.github/workflows/e2e-tests-main-devnet.yml b/.github/workflows/e2e-tests-main-devnet.yml index a1859c6839..a61e5a6aa0 100644 --- a/.github/workflows/e2e-tests-main-devnet.yml +++ b/.github/workflows/e2e-tests-main-devnet.yml @@ -457,6 +457,21 @@ jobs: follow-up-finalization-check: true timeout-minutes: 15 + run-e2e-ban-manual: + needs: [build-test-docker, build-test-client] + name: Run ban manual test + runs-on: ubuntu-20.04 + steps: + - name: Checkout source code + uses: actions/checkout@v2 + + - name: Run e2e test + uses: ./.github/actions/run-e2e-test + with: + test-case: ban_manual + follow-up-finalization-check: true + timeout-minutes: 15 + run-e2e-ban-counter-clearing: needs: [build-test-docker, build-test-client] name: Run ban counter clearing test @@ -472,6 +487,21 @@ jobs: follow-up-finalization-check: true timeout-minutes: 15 + run-e2e-ban-threshold: + needs: [build-test-docker, build-test-client] + name: Run ban threshold test + runs-on: ubuntu-20.04 + steps: + - name: Checkout source code + uses: actions/checkout@v2 + + - name: Run e2e test + uses: ./.github/actions/run-e2e-test + with: + test-case: ban_threshold + follow-up-finalization-check: true + timeout-minutes: 15 + run-e2e-version-upgrade: needs: [build-test-docker, build-test-client] name: Run basic (positive) version-upgrade test @@ -510,21 +540,6 @@ jobs: ONLY_LEGACY: true timeout-minutes: 10 - run-e2e-ban-manual: - needs: [build-test-docker, build-test-client] - name: Run ban manual test - runs-on: ubuntu-20.04 - steps: - - name: Checkout source code - uses: actions/checkout@v2 - - - name: Run e2e test - uses: ./.github/actions/run-e2e-test - with: - test-case: ban_manual - follow-up-finalization-check: true - timeout-minutes: 15 - run-e2e-version-upgrade-catchup: needs: [build-test-docker, build-cliain-image] name: Run series of tests where some of the nodes need to do version-upgrade during catch-up @@ -606,8 +621,9 @@ jobs: run-e2e-rewards-points-basic, run-e2e-authorities-are-staking, run-e2e-ban-automatic, - run-e2e-ban-counter-clearing, run-e2e-ban-manual, + run-e2e-ban-counter-clearing, + run-e2e-ban-threshold, run-e2e-version-upgrade, run-e2e-failing-version-upgrade, run-e2e-version-upgrade-catchup, diff --git a/aleph-client/Cargo.lock b/aleph-client/Cargo.lock index e290c25710..b1d80b1dd6 100644 --- a/aleph-client/Cargo.lock +++ b/aleph-client/Cargo.lock @@ -98,7 +98,7 @@ dependencies = [ [[package]] name = "aleph_client" -version = "1.11.0" +version = "1.12.0" dependencies = [ "ac-node-api", "ac-primitives", diff --git a/aleph-client/Cargo.toml b/aleph-client/Cargo.toml index 9439f1960a..b066ee4c15 100644 --- a/aleph-client/Cargo.toml +++ b/aleph-client/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "aleph_client" -version = "1.12.0" +version = "1.13.0" edition = "2021" license = "Apache 2.0" diff --git a/aleph-client/src/elections.rs b/aleph-client/src/elections.rs index 94961c7162..1a98a9cca9 100644 --- a/aleph-client/src/elections.rs +++ b/aleph-client/src/elections.rs @@ -1,3 +1,4 @@ +use log::info; use primitives::{ BanConfig, BanInfo, CommitteeSeats, EraIndex, EraValidators, SessionCount, SessionIndex, }; @@ -76,18 +77,19 @@ pub fn get_ban_config(connection: &C) -> BanConfig { pub fn get_underperformed_validator_session_count( connection: &C, account_id: &AccountId, + block_hash: Option, ) -> SessionCount { connection .read_storage_map( PALLET, "UnderperformedValidatorSessionCount", account_id, - None, + block_hash, ) .unwrap_or(0) } -pub fn get_ban_reason_for_validator( +pub fn get_ban_info_for_validator( connection: &C, account_id: &AccountId, ) -> Option { @@ -129,15 +131,19 @@ pub fn change_ban_config( ban_period: Option, status: XtStatus, ) { + info!(target: "aleph-client", "Changing ban config"); + let call_name = "set_ban_config"; + let call = compose_call!( sudo_connection.as_connection().metadata, PALLET, - "set_ban_config", + call_name, minimal_expected_performance, underperformed_session_count_threshold, clean_session_counter_delay, ban_period ); + let xt = compose_extrinsic!( sudo_connection.as_connection(), "Sudo", @@ -145,5 +151,6 @@ pub fn change_ban_config( call, 0_u64 ); - send_xt(sudo_connection, xt, Some("set_ban_config"), status); + + send_xt(sudo_connection, xt, Some(call_name), status); } diff --git a/aleph-client/src/lib.rs b/aleph-client/src/lib.rs index 6b327c5350..145367e0e5 100644 --- a/aleph-client/src/lib.rs +++ b/aleph-client/src/lib.rs @@ -6,7 +6,7 @@ pub use balances::total_issuance; use codec::{Decode, Encode}; pub use debug::print_storages; pub use elections::{ - ban_from_committee, change_ban_config, get_ban_config, get_ban_reason_for_validator, + ban_from_committee, change_ban_config, get_ban_config, get_ban_info_for_validator, get_committee_seats, get_current_era_non_reserved_validators, get_current_era_reserved_validators, get_current_era_validators, get_era_validators, get_next_era_committee_seats, get_next_era_non_reserved_validators, diff --git a/benches/payout-stakers/Cargo.lock b/benches/payout-stakers/Cargo.lock index 8d216c22b4..6f5ea1f226 100644 --- a/benches/payout-stakers/Cargo.lock +++ b/benches/payout-stakers/Cargo.lock @@ -98,7 +98,7 @@ dependencies = [ [[package]] name = "aleph_client" -version = "1.11.0" +version = "1.12.0" dependencies = [ "ac-node-api", "ac-primitives", diff --git a/bin/cliain/Cargo.lock b/bin/cliain/Cargo.lock index a32dca162d..9b6ec5cad9 100644 --- a/bin/cliain/Cargo.lock +++ b/bin/cliain/Cargo.lock @@ -98,7 +98,7 @@ dependencies = [ [[package]] name = "aleph_client" -version = "1.11.0" +version = "1.12.0" dependencies = [ "ac-node-api", "ac-primitives", diff --git a/e2e-tests/Cargo.lock b/e2e-tests/Cargo.lock index 30eb393cb1..e2cb7f60ad 100644 --- a/e2e-tests/Cargo.lock +++ b/e2e-tests/Cargo.lock @@ -98,7 +98,7 @@ dependencies = [ [[package]] name = "aleph-e2e-client" -version = "0.7.0" +version = "0.8.0" dependencies = [ "aleph_client", "anyhow", diff --git a/e2e-tests/Cargo.toml b/e2e-tests/Cargo.toml index 0a14771af6..64e3bb1d6d 100644 --- a/e2e-tests/Cargo.toml +++ b/e2e-tests/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "aleph-e2e-client" -version = "0.7.0" +version = "0.8.0" edition = "2021" license = "Apache 2.0" diff --git a/e2e-tests/src/ban.rs b/e2e-tests/src/ban.rs index b8b45ba2e0..227a252e53 100644 --- a/e2e-tests/src/ban.rs +++ b/e2e-tests/src/ban.rs @@ -1,18 +1,30 @@ use aleph_client::{ - change_validators, get_ban_config, get_ban_reason_for_validator, - get_underperformed_validator_session_count, wait_for_event, wait_for_full_era_completion, - AccountId, AnyConnection, RootConnection, XtStatus, + change_validators, get_ban_config, get_ban_info_for_validator, get_block_hash, + get_session_period, get_underperformed_validator_session_count, wait_for_event, + wait_for_full_era_completion, AccountId, AnyConnection, RootConnection, XtStatus, }; use codec::Decode; use log::info; -use primitives::{BanConfig, BanInfo, CommitteeSeats, EraValidators, SessionCount}; +use primitives::{BanConfig, BanInfo, CommitteeSeats, EraValidators, SessionCount, SessionIndex}; +use sp_core::H256; use sp_runtime::Perbill; -use crate::{accounts::account_ids_from_keys, validators::get_test_validators, Config}; +use crate::{ + accounts::account_ids_from_keys, elections::get_members_subset_for_session, + validators::get_test_validators, Config, +}; + +const RESERVED_SEATS: u32 = 2; +const NON_RESERVED_SEATS: u32 = 2; -pub fn setup_test( - config: &Config, -) -> anyhow::Result<(RootConnection, Vec, Vec)> { +type BanTestSetup = ( + RootConnection, + Vec, + Vec, + CommitteeSeats, +); + +pub fn setup_test(config: &Config) -> anyhow::Result { let root_connection = config.create_root_connection(); let validator_keys = get_test_validators(config); @@ -20,8 +32,8 @@ pub fn setup_test( let non_reserved_validators = account_ids_from_keys(&validator_keys.non_reserved); let seats = CommitteeSeats { - reserved_seats: 2, - non_reserved_seats: 2, + reserved_seats: RESERVED_SEATS, + non_reserved_seats: NON_RESERVED_SEATS, }; change_validators( @@ -38,6 +50,7 @@ pub fn setup_test( root_connection, reserved_validators, non_reserved_validators, + seats, )) } @@ -83,9 +96,10 @@ pub fn check_underperformed_validator_session_count( connection: &C, validator: &AccountId, expected_session_count: &SessionCount, + block_hash: Option, ) -> SessionCount { let underperformed_validator_session_count = - get_underperformed_validator_session_count(connection, validator); + get_underperformed_validator_session_count(connection, validator, block_hash); assert_eq!( &underperformed_validator_session_count, @@ -100,7 +114,7 @@ pub fn check_ban_info_for_validator( validator: &AccountId, expected_info: Option<&BanInfo>, ) -> Option { - let validator_ban_info = get_ban_reason_for_validator(connection, validator); + let validator_ban_info = get_ban_info_for_validator(connection, validator); assert_eq!(validator_ban_info.as_ref(), expected_info); @@ -124,3 +138,90 @@ pub fn check_ban_event( Ok(event) } + +pub fn get_members_for_session( + reserved_validators: &[AccountId], + non_reserved_validators: &[AccountId], + seats: &CommitteeSeats, + session: SessionIndex, +) -> Vec { + let reserved_members = + get_members_subset_for_session(seats.reserved_seats, reserved_validators, session); + let non_reserved_members = + get_members_subset_for_session(seats.non_reserved_seats, non_reserved_validators, session); + reserved_members + .into_iter() + .chain(non_reserved_members.into_iter()) + .collect() +} + +/// Checks whether underperformed counts for validators change predictably. Assumes: (a) that the +/// sessions checked are in the past, (b) that all the checked validators are underperforming in +/// those sessions (e.g. due to a prohibitively high performance threshold). +pub fn check_underperformed_count_for_sessions( + connection: &C, + reserved_validators: &[AccountId], + non_reserved_validators: &[AccountId], + seats: &CommitteeSeats, + start_session: SessionIndex, + end_session: SessionIndex, + ban_session_threshold: SessionCount, +) -> anyhow::Result<()> { + let session_period = get_session_period(connection); + + let validators: Vec<_> = reserved_validators + .iter() + .chain(non_reserved_validators.iter()) + .collect(); + + for session in start_session..end_session { + let session_end_block = (session + 1) * session_period; + let session_end_block_hash = get_block_hash(connection, session_end_block); + + let previous_session_end_block = session_end_block - session_period; + let previous_session_end_block_hash = + get_block_hash(connection, previous_session_end_block); + + let members = + get_members_for_session(reserved_validators, non_reserved_validators, seats, session); + + validators.iter().for_each(|&val| { + info!( + "Checking session count | session {} | validator {}", + session, val + ); + let session_underperformed_count = get_underperformed_validator_session_count( + connection, + val, + Some(session_end_block_hash), + ); + let previous_session_underperformed_count = get_underperformed_validator_session_count( + connection, + val, + Some(previous_session_end_block_hash), + ); + + let underperformed_diff = + session_underperformed_count.abs_diff(previous_session_underperformed_count); + + if members.contains(val) { + // Counter for committee members legally incremented by 1 or reset to 0 (decremented + // by ban_session_threshold - 1). + if underperformed_diff != 1 && underperformed_diff != (ban_session_threshold - 1) { + panic!( + "Underperformed session count for committee validator {} for session {} changed from {} to {}.", + val, session, previous_session_underperformed_count, session_underperformed_count + ); + } + } else if underperformed_diff != 0 { + // Counter for validators on the bench should stay the same. + panic!( + "Underperformed session count for non-committee validator {} for session {} changed from {} to {}.", + val, session, previous_session_underperformed_count, session_underperformed_count + ); + } + }); + } + + Ok(()) +} diff --git a/e2e-tests/src/cases.rs b/e2e-tests/src/cases.rs index 586f481442..dd0b626eab 100644 --- a/e2e-tests/src/cases.rs +++ b/e2e-tests/src/cases.rs @@ -3,7 +3,7 @@ use crate::{ test::{ authorities_are_staking as test_authorities_are_staking, ban_automatic as test_ban_automatic, ban_manual as test_ban_manual, - batch_transactions as test_batch_transactions, + ban_threshold as test_ban_threshold, batch_transactions as test_batch_transactions, change_stake_and_force_new_era as test_change_stake_and_force_new_era, change_validators as test_change_validators, channeling_fee_and_tip as test_channeling_fee_and_tip, @@ -70,5 +70,6 @@ pub fn possible_test_cases() -> PossibleTestCases { "clearing_session_count", test_clearing_session_count as TestCase, ), + ("ban_threshold", test_ban_threshold as TestCase), ] } diff --git a/e2e-tests/src/rewards.rs b/e2e-tests/src/rewards.rs index 132175b169..ea87a52ced 100644 --- a/e2e-tests/src/rewards.rs +++ b/e2e-tests/src/rewards.rs @@ -163,12 +163,12 @@ pub fn check_points( info!("Era: {} | session: {}.", era, session); - let beggining_of_session_block = session * session_period; - let end_of_session_block = beggining_of_session_block + session_period; + let beginning_of_session_block = session * session_period; + let end_of_session_block = beginning_of_session_block + session_period; info!("Waiting for block: {}.", end_of_session_block); wait_for_finalized_block(connection, end_of_session_block)?; - let beggining_of_session_block_hash = get_block_hash(connection, beggining_of_session_block); + let beginning_of_session_block_hash = get_block_hash(connection, beginning_of_session_block); let end_of_session_block_hash = get_block_hash(connection, end_of_session_block); let before_end_of_session_block_hash = get_block_hash(connection, end_of_session_block - 1); info!("End-of-session block hash: {}.", end_of_session_block_hash); @@ -188,7 +188,7 @@ pub fn check_points( .individual; let validator_reward_points_previous_session = - get_era_reward_points(connection, era, Some(beggining_of_session_block_hash)) + get_era_reward_points(connection, era, Some(beginning_of_session_block_hash)) .unwrap_or_default() .individual; @@ -229,7 +229,7 @@ pub fn check_points( let members_count = reward_points.len() as f64; for (account_id, reward_points) in reward_points.iter_mut() { let exposure = - download_exposure(connection, era, account_id, beggining_of_session_block_hash); + download_exposure(connection, era, account_id, beginning_of_session_block_hash); *reward_points *= exposure as f64 / members_count; } diff --git a/e2e-tests/src/test/ban.rs b/e2e-tests/src/test/ban.rs index 51ff27fbe5..ea206ca7a8 100644 --- a/e2e-tests/src/test/ban.rs +++ b/e2e-tests/src/test/ban.rs @@ -14,12 +14,15 @@ use crate::{ accounts::{get_validator_seed, NodeKeys}, ban::{ check_ban_config, check_ban_event, check_ban_info_for_validator, - check_underperformed_validator_session_count, check_validators, setup_test, + check_underperformed_count_for_sessions, check_underperformed_validator_session_count, + check_validators, setup_test, }, rewards::set_invalid_keys_for_validator, Config, }; +const SESSIONS_TO_CHECK: SessionCount = 5; + const VALIDATOR_TO_DISABLE_NON_RESERVED_INDEX: u32 = 0; const VALIDATOR_TO_DISABLE_OVERALL_INDEX: u32 = 2; // Address for //2 (Node2). Depends on the infrastructure setup. @@ -29,6 +32,8 @@ const SESSIONS_TO_MEET_BAN_THRESHOLD: SessionCount = 4; const VALIDATOR_TO_MANUALLY_BAN_NON_RESERVED_INDEX: u32 = 1; const MANUAL_BAN_REASON: &str = "Manual ban reason"; +const MIN_EXPECTED_PERFORMANCE: u8 = 100; + fn disable_validator(validator_address: &str, validator_seed: u32) -> anyhow::Result<()> { let validator_seed = get_validator_seed(validator_seed); let stash_controller = NodeKeys::from(validator_seed); @@ -45,7 +50,7 @@ fn disable_validator(validator_address: &str, validator_seed: u32) -> anyhow::Re /// producing blocks. Verifies that the offending validator has in fact been banned out for the /// appropriate reason. pub fn ban_automatic(config: &Config) -> anyhow::Result<()> { - let (root_connection, reserved_validators, non_reserved_validators) = setup_test(config)?; + let (root_connection, reserved_validators, non_reserved_validators, _) = setup_test(config)?; // Check current era validators. check_validators( @@ -67,7 +72,7 @@ pub fn ban_automatic(config: &Config) -> anyhow::Result<()> { info!("Validator to disable: {}", validator_to_disable); - check_underperformed_validator_session_count(&root_connection, validator_to_disable, &0); + check_underperformed_validator_session_count(&root_connection, validator_to_disable, &0, None); check_ban_info_for_validator(&root_connection, validator_to_disable, None); disable_validator(NODE_TO_DISABLE_ADDRESS, VALIDATOR_TO_DISABLE_OVERALL_INDEX)?; @@ -81,7 +86,7 @@ pub fn ban_automatic(config: &Config) -> anyhow::Result<()> { // The session count for underperforming validators is reset to 0 immediately on reaching the // threshold. - check_underperformed_validator_session_count(&root_connection, validator_to_disable, &0); + check_underperformed_validator_session_count(&root_connection, validator_to_disable, &0, None); let reason = BanReason::InsufficientUptime(DEFAULT_BAN_SESSION_COUNT_THRESHOLD); let start = get_current_era(&root_connection) + 1; @@ -114,7 +119,7 @@ pub fn ban_automatic(config: &Config) -> anyhow::Result<()> { /// from the committee with a specific reason. Verifies that validator marked for ban has in /// fact been banned for the given reason. pub fn ban_manual(config: &Config) -> anyhow::Result<()> { - let (root_connection, reserved_validators, non_reserved_validators) = setup_test(config)?; + let (root_connection, reserved_validators, non_reserved_validators, _) = setup_test(config)?; // Check current era validators. check_validators( @@ -136,7 +141,12 @@ pub fn ban_manual(config: &Config) -> anyhow::Result<()> { info!("Validator to manually ban: {}", validator_to_manually_ban); - check_underperformed_validator_session_count(&root_connection, validator_to_manually_ban, &0); + check_underperformed_validator_session_count( + &root_connection, + validator_to_manually_ban, + &0, + None, + ); check_ban_info_for_validator(&root_connection, validator_to_manually_ban, None); let bounded_reason: BoundedVec<_, _> = MANUAL_BAN_REASON @@ -187,9 +197,8 @@ pub fn ban_manual(config: &Config) -> anyhow::Result<()> { /// Disable one non_reserved validator. Check if the disabled validator is still in the committee /// and his underperformed session count is less or equal to 2. pub fn clearing_session_count(config: &Config) -> anyhow::Result<()> { - let (root_connection, reserved_validators, non_reserved_validators) = setup_test(config)?; + let (root_connection, reserved_validators, non_reserved_validators, _) = setup_test(config)?; - info!("changing ban config"); change_ban_config( &root_connection, None, @@ -209,7 +218,7 @@ pub fn clearing_session_count(config: &Config) -> anyhow::Result<()> { wait_for_at_least_session(&root_connection, current_session + 5)?; let underperformed_validator_session_count = - get_underperformed_validator_session_count(&root_connection, validator_to_disable); + get_underperformed_validator_session_count(&root_connection, validator_to_disable, None); // it only has to be ge than 0 and should be cleared before reaching values larger than 3. assert!(underperformed_validator_session_count <= 2); @@ -223,3 +232,55 @@ pub fn clearing_session_count(config: &Config) -> anyhow::Result<()> { Ok(()) } + +/// Runs a chain, sets up a committee and validators. Changes the ban config to require 100% +/// performance. Checks that each validator has all the sessions in which they were chosen for the +/// committee marked as ones in which they underperformed. +pub fn ban_threshold(config: &Config) -> anyhow::Result<()> { + let (root_connection, reserved_validators, non_reserved_validators, seats) = + setup_test(config)?; + + // Check current era validators. + check_validators( + &root_connection, + &reserved_validators, + &non_reserved_validators, + get_current_era_validators, + ); + + check_ban_config( + &root_connection, + DEFAULT_BAN_MINIMAL_EXPECTED_PERFORMANCE, + DEFAULT_BAN_SESSION_COUNT_THRESHOLD, + DEFAULT_CLEAN_SESSION_COUNTER_DELAY, + ); + + // Change ban config to require prohibitively high performance from all validators. + change_ban_config( + &root_connection, + Some(MIN_EXPECTED_PERFORMANCE), + None, + None, + None, + XtStatus::InBlock, + ); + + let ban_config_change_session = get_current_session(&root_connection); + let check_start_session = ban_config_change_session + 1; + let check_end_session = check_start_session + SESSIONS_TO_CHECK - 1; + + // Wait until all the sessions to be checked are in the past. + wait_for_at_least_session(&root_connection, check_end_session + 1)?; + + check_underperformed_count_for_sessions( + &root_connection, + &reserved_validators, + &non_reserved_validators, + &seats, + check_start_session, + check_end_session, + DEFAULT_BAN_SESSION_COUNT_THRESHOLD, + )?; + + Ok(()) +} diff --git a/e2e-tests/src/test/mod.rs b/e2e-tests/src/test/mod.rs index 6946aae617..79e4d51b3b 100644 --- a/e2e-tests/src/test/mod.rs +++ b/e2e-tests/src/test/mod.rs @@ -1,4 +1,4 @@ -pub use ban::{ban_automatic, ban_manual, clearing_session_count}; +pub use ban::{ban_automatic, ban_manual, ban_threshold, clearing_session_count}; pub use electing_validators::authorities_are_staking; pub use era_payout::era_payouts_calculated_correctly; pub use era_validators::era_validators; diff --git a/flooder/Cargo.lock b/flooder/Cargo.lock index 39ec0dda25..d71dccc95b 100644 --- a/flooder/Cargo.lock +++ b/flooder/Cargo.lock @@ -98,7 +98,7 @@ dependencies = [ [[package]] name = "aleph_client" -version = "1.11.0" +version = "1.12.0" dependencies = [ "ac-node-api", "ac-primitives",