diff --git a/Cargo.lock b/Cargo.lock index 4d82d916dd..32e698551a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1165,6 +1165,7 @@ dependencies = [ "frame-system-rpc-runtime-api", "hex-literal", "libsecp256k1", + "log", "pallet-aura", "pallet-authorship", "pallet-cf-account-roles", @@ -1201,6 +1202,7 @@ dependencies = [ "sp-runtime", "sp-session", "sp-std 8.0.0 (git+https://github.com/chainflip-io/substrate.git?tag=chainflip-monthly-2023-08+2)", + "sp-timestamp", "sp-transaction-pool", "sp-version", "state-chain-runtime", diff --git a/state-chain/amm/src/common.rs b/state-chain/amm/src/common.rs index 38c98fa730..7dc5c8f706 100644 --- a/state-chain/amm/src/common.rs +++ b/state-chain/amm/src/common.rs @@ -335,13 +335,13 @@ pub(super) fn sqrt_price_at_tick(tick: Tick) -> SqrtPriceQ64F96 { /* Proof that r is never zero (therefore avoiding the divide by zero case here): We can think of an application of the `handle_tick_bit` macro as increasing the index I of r's MSB/`r.ilog2()` (mul by constant), and then decreasing it by 128 (the right shift). - Note the increase in I caused by the constant mul will be atleast constant.ilog2(). + Note the increase in I caused by the constant mul will be at least constant.ilog2(). Also note each application of `handle_tick_bit` decreases (if the if branch is entered) or else maintains r's value as all the constants are less than 2^128. Therefore the largest decrease would be caused if all the macros application's if branches where entered. - So we assuming all if branches are entered, after all the applications `I` would be atleast I_initial + bigsum(constant.ilog2()) - 19*128. + So we assuming all if branches are entered, after all the applications `I` would be at least I_initial + bigsum(constant.ilog2()) - 19*128. The test `r_non_zero` checks with value is >= 0, therefore imply the smallest value r could have is more than 0. */ diff --git a/state-chain/amm/src/limit_orders.rs b/state-chain/amm/src/limit_orders.rs index 63db143ccd..6a943d725a 100644 --- a/state-chain/amm/src/limit_orders.rs +++ b/state-chain/amm/src/limit_orders.rs @@ -75,7 +75,7 @@ impl FloatBetweenZeroAndOne { let (mul_div_normalised_mantissa, div_normalise_shift) = { // As the denominator <= U256::MAX, this div will not right-shift the mantissa more than - // 256 bits, so we maintain atleast 256 accurate bits in the result. + // 256 bits, so we maintain at least 256 accurate bits in the result. let (d, div_remainder) = U512::div_mod(mul_normalised_mantissa, U512::from(denominator)); let d = if div_remainder.is_zero() { d } else { d + U512::one() }; @@ -96,11 +96,11 @@ impl FloatBetweenZeroAndOne { Self { normalised_mantissa: mul_div_normalised_mantissa, negative_exponent } } else { // This bounding will cause swaps to get bad prices, but this case will effectively - // never happen, as atleast (U256::MAX / 256) (~10^74) swaps would have to happen to get - // into this situation. TODO: A possible solution is disabling minting for pools "close" - // to this minimum. With a small change to the swapping logic it would be possible to - // guarantee that the pool would be emptied before percent_remaining could reach this - // min bound. + // never happen, as at least (U256::MAX / 256) (~10^74) swaps would have to happen to + // get into this situation. TODO: A possible solution is disabling minting for pools + // "close" to this minimum. With a small change to the swapping logic it would be + // possible to guarantee that the pool would be emptied before percent_remaining could + // reach this min bound. Self { normalised_mantissa: U256::one() << 255, negative_exponent: U256::MAX } } } diff --git a/state-chain/cf-integration-tests/Cargo.toml b/state-chain/cf-integration-tests/Cargo.toml index f8fe9f5724..b4008d1170 100644 --- a/state-chain/cf-integration-tests/Cargo.toml +++ b/state-chain/cf-integration-tests/Cargo.toml @@ -13,6 +13,7 @@ targets = ['x86_64-unknown-linux-gnu'] [dependencies] state-chain-runtime = { path = '../runtime' } +log = { version = '0.4.16', default-features = false } [dev-dependencies] libsecp256k1 = { version = "0.7", features = ['static-context'] } @@ -79,3 +80,4 @@ sp-std = { git = "https://github.com/chainflip-io/substrate.git", tag = "chainfl sp-transaction-pool = { git = "https://github.com/chainflip-io/substrate.git", tag = "chainflip-monthly-2023-08+2" } sp-version = { git = "https://github.com/chainflip-io/substrate.git", tag = "chainflip-monthly-2023-08+2" } sp-consensus-grandpa = { git = "https://github.com/chainflip-io/substrate.git", tag = "chainflip-monthly-2023-08+2" } +sp-timestamp = { git = "https://github.com/chainflip-io/substrate.git", tag = "chainflip-monthly-2023-08+2" } diff --git a/state-chain/cf-integration-tests/src/account.rs b/state-chain/cf-integration-tests/src/account.rs index c31ba1e777..202344e768 100644 --- a/state-chain/cf-integration-tests/src/account.rs +++ b/state-chain/cf-integration-tests/src/account.rs @@ -44,6 +44,7 @@ fn account_deletion_removes_relevant_storage_items() { let vanity_names = VanityNames::::get(); assert_eq!(*vanity_names.get(&backup_node).unwrap(), elon_vanity_name.as_bytes().to_vec()); + // Redeem all network::Cli::redeem( &backup_node, RedemptionAmount::Max, diff --git a/state-chain/cf-integration-tests/src/authorities.rs b/state-chain/cf-integration-tests/src/authorities.rs index 8c4b43df33..940de5d7d9 100644 --- a/state-chain/cf-integration-tests/src/authorities.rs +++ b/state-chain/cf-integration-tests/src/authorities.rs @@ -1,9 +1,9 @@ use crate::{ - genesis, get_validator_state, network, ChainflipAccountState, NodeId, HEARTBEAT_BLOCK_INTERVAL, - VAULT_ROTATION_BLOCKS, + genesis, get_validator_state, network, AllVaults, ChainflipAccountState, NodeId, + HEARTBEAT_BLOCK_INTERVAL, VAULT_ROTATION_BLOCKS, }; -use frame_support::assert_ok; +use frame_support::{assert_err, assert_ok}; use sp_runtime::AccountId32; use std::collections::{BTreeSet, HashMap}; @@ -12,7 +12,8 @@ use cf_traits::{AsyncResult, EpochInfo, SafeMode, VaultRotator, VaultStatus}; use pallet_cf_environment::SafeModeUpdate; use pallet_cf_validator::{CurrentRotationPhase, RotationPhase}; use state_chain_runtime::{ - chainflip::RuntimeSafeMode, Environment, EthereumVault, Flip, Runtime, Validator, + chainflip::RuntimeSafeMode, BitcoinVault, Environment, EthereumInstance, EthereumVault, Flip, + PolkadotInstance, PolkadotVault, Runtime, RuntimeOrigin, Validator, }; // Helper function that creates a network, funds backup nodes, and have them join the auction. @@ -40,7 +41,7 @@ fn fund_authorities_and_join_auction( // Allow the funds to be registered, initialise the account keys and peer // ids, register as a validator, then start bidding. - testnet.move_forward_blocks(1); + testnet.move_forward_blocks(2); for node in &init_backup_nodes { network::Cli::register_as_validator(node); @@ -51,6 +52,88 @@ fn fund_authorities_and_join_auction( (testnet, genesis_authorities, init_backup_nodes) } +/// Tests that Validator and Vaults work together to complete a Authority Rotation +/// by going through the correct sequence in sync. +#[test] +fn authority_rotates_with_correct_sequence() { + const EPOCH_BLOCKS: u32 = 1000; + const MAX_AUTHORITIES: AuthorityCount = 10; + super::genesis::default() + .blocks_per_epoch(EPOCH_BLOCKS) + .max_authorities(MAX_AUTHORITIES) + .build() + .execute_with(|| { + let (mut testnet, _, _) = fund_authorities_and_join_auction(MAX_AUTHORITIES); + assert_eq!(GENESIS_EPOCH, Validator::epoch_index()); + + // Skip the first authority rotation, as key handover is guaranteed to succeed + // when rotating for the first time. + testnet.move_forward_blocks(EPOCH_BLOCKS); + testnet.submit_heartbeat_all_engines(); + + testnet.move_forward_blocks(VAULT_ROTATION_BLOCKS); + + assert!(matches!(Validator::current_rotation_phase(), RotationPhase::Idle)); + assert_eq!(AllVaults::status(), AsyncResult::Ready(VaultStatus::RotationComplete)); + assert_eq!(GENESIS_EPOCH + 1, Validator::epoch_index()); + + testnet.move_forward_blocks(EPOCH_BLOCKS); + testnet.submit_heartbeat_all_engines(); + + // Start the Authority and Vault rotation + // idle -> Keygen + testnet.move_forward_blocks(4); + assert!(matches!( + Validator::current_rotation_phase(), + RotationPhase::KeygensInProgress(..) + )); + // NOTE: This happens due to a bug in `move_forward_blocks`: keygen completes in the + // same block in which is was requested. + assert_eq!(AllVaults::status(), AsyncResult::Ready(VaultStatus::KeygenComplete)); + + // Key Handover complete. + testnet.move_forward_blocks(4); + assert!(matches!( + Validator::current_rotation_phase(), + RotationPhase::KeyHandoversInProgress(..) + )); + // NOTE: See above, we skip the pending state. + assert_eq!(AllVaults::status(), AsyncResult::Ready(VaultStatus::KeyHandoverComplete)); + + // Activate new key. + testnet.move_forward_blocks(2); + assert!(matches!( + Validator::current_rotation_phase(), + RotationPhase::ActivatingKeys(..) + )); + assert_eq!( + AllVaults::status(), + AsyncResult::Ready(VaultStatus::RotationComplete), + "Rotation should be complete but vault status is {:?}", + AllVaults::status() + ); + + // Rotating session + testnet.move_forward_blocks(1); + assert!(matches!( + Validator::current_rotation_phase(), + RotationPhase::SessionRotating(..) + )); + assert_eq!(AllVaults::status(), AsyncResult::Ready(VaultStatus::RotationComplete)); + + // Rotation Completed. + testnet.move_forward_blocks(1); + assert!(matches!(Validator::current_rotation_phase(), RotationPhase::Idle)); + assert_eq!(AllVaults::status(), AsyncResult::Ready(VaultStatus::RotationComplete)); + + assert_eq!( + GENESIS_EPOCH + 2, + Validator::epoch_index(), + "We should be in the next epoch." + ); + }); +} + #[test] fn authorities_earn_rewards_for_authoring_blocks() { // We want to have at least one heartbeat within our reduced epoch @@ -189,8 +272,12 @@ fn authority_rotation_can_succeed_after_aborted_by_safe_mode() { testnet.submit_heartbeat_all_engines(); // Run until key gen is completed. - testnet.move_forward_blocks(2); - matches!(EthereumVault::status(), AsyncResult::Ready(VaultStatus::KeygenComplete)); + testnet.move_forward_blocks(4); + assert!( + matches!(AllVaults::status(), AsyncResult::Ready(VaultStatus::KeygenComplete)), + "Keygen should be complete but is {:?}", + AllVaults::status() + ); // This is the last chance to abort validator rotation. Activate code red here. assert_ok!(Environment::update_safe_mode( @@ -201,13 +288,13 @@ fn authority_rotation_can_succeed_after_aborted_by_safe_mode() { // Ensure Validator and Vault rotation have been aborted. assert_eq!(CurrentRotationPhase::::get(), RotationPhase::Idle); - assert_eq!(EthereumVault::status(), AsyncResult::Void); + assert_eq!(AllVaults::status(), AsyncResult::Void); // Authority rotation does not start while in Safe Mode. testnet.move_forward_blocks(EPOCH_BLOCKS); assert_eq!(CurrentRotationPhase::::get(), RotationPhase::Idle); - assert_eq!(EthereumVault::status(), AsyncResult::Void); + assert_eq!(AllVaults::status(), AsyncResult::Void); // Changing to code green should restart the Authority rotation assert_ok!(Environment::update_safe_mode( @@ -238,11 +325,12 @@ fn authority_rotation_cannot_be_aborted_after_key_handover_but_stalls_on_safe_mo testnet.submit_heartbeat_all_engines(); // Run until key handover starts - testnet.move_forward_blocks(3); - assert!(matches!( - EthereumVault::status(), - AsyncResult::Ready(VaultStatus::KeyHandoverComplete) - )); + testnet.move_forward_blocks(5); + assert!( + matches!(AllVaults::status(), AsyncResult::Ready(VaultStatus::KeyHandoverComplete)), + "Key handover should be complete but is {:?}", + AllVaults::status() + ); assert_ok!(Environment::update_safe_mode( pallet_cf_governance::RawOrigin::GovernanceApproval.into(), @@ -253,23 +341,13 @@ fn authority_rotation_cannot_be_aborted_after_key_handover_but_stalls_on_safe_mo // Authority rotation is stalled while in Code Red because of disabling dispatching // witness extrinsics and so witnessing vault rotation will be stalled. - matches!(EthereumVault::status(), AsyncResult::Ready(VaultStatus::KeyHandoverComplete)); + assert!(matches!(AllVaults::status(), AsyncResult::Pending)); // We activate witnessing calls by setting safe mode to code green just for the // witnesser pallet. - let runtime_safe_mode_with_witnessing = RuntimeSafeMode { - emissions: pallet_cf_emissions::PalletSafeMode::CODE_RED, - funding: pallet_cf_funding::PalletSafeMode::CODE_RED, - swapping: pallet_cf_swapping::PalletSafeMode::CODE_RED, - liquidity_provider: pallet_cf_lp::PalletSafeMode::CODE_RED, - validator: pallet_cf_validator::PalletSafeMode::CODE_RED, - pools: pallet_cf_pools::PalletSafeMode::CODE_RED, - witnesser: pallet_cf_witnesser::PalletSafeMode::CODE_GREEN, /* code green for - * witnessing */ - reputation: pallet_cf_reputation::PalletSafeMode::CODE_RED, - vault: pallet_cf_vaults::PalletSafeMode::CODE_RED, - broadcast: pallet_cf_broadcast::PalletSafeMode::CODE_RED, - }; + let mut runtime_safe_mode_with_witnessing = RuntimeSafeMode::CODE_RED; + runtime_safe_mode_with_witnessing.witnesser = + pallet_cf_witnesser::PalletSafeMode::CODE_GREEN; assert_ok!(Environment::update_safe_mode( pallet_cf_governance::RawOrigin::GovernanceApproval.into(), @@ -278,7 +356,131 @@ fn authority_rotation_cannot_be_aborted_after_key_handover_but_stalls_on_safe_mo // rotation should now complete since the witness calls are now dispatched. testnet.move_forward_blocks(3); + assert_eq!(GENESIS_EPOCH + 1, Validator::epoch_index(), "We should be in a new epoch"); + }); +} - assert_eq!(GENESIS_EPOCH + 1, Validator::epoch_index(), "We should be in a new epoch"); +#[test] +fn authority_rotation_can_recover_after_keygen_fails() { + const EPOCH_BLOCKS: u32 = 1000; + const MAX_AUTHORITIES: AuthorityCount = 10; + super::genesis::default() + .blocks_per_epoch(EPOCH_BLOCKS) + .max_authorities(MAX_AUTHORITIES) + .build() + .execute_with(|| { + let (mut testnet, _, backup_nodes) = fund_authorities_and_join_auction(MAX_AUTHORITIES); + + backup_nodes.iter().for_each(|validator| { + testnet.set_active(validator, false); + }); + + // Begin the rotation, but make Keygen fail. + testnet.move_to_next_epoch(); + testnet.submit_heartbeat_all_engines(); + + testnet.move_forward_blocks(1); + assert!(matches!( + Validator::current_rotation_phase(), + RotationPhase::KeygensInProgress(..) + )); + assert_eq!(AllVaults::status(), AsyncResult::Pending); + backup_nodes.iter().for_each(|validator| { + assert_ok!(EthereumVault::report_keygen_outcome( + RuntimeOrigin::signed(validator.clone()), + EthereumVault::ceremony_id_counter(), + Err(BTreeSet::default()), + )); + assert_ok!(PolkadotVault::report_keygen_outcome( + RuntimeOrigin::signed(validator.clone()), + PolkadotVault::ceremony_id_counter(), + Err(BTreeSet::default()), + )); + assert_ok!(BitcoinVault::report_keygen_outcome( + RuntimeOrigin::signed(validator.clone()), + BitcoinVault::ceremony_id_counter(), + Err(BTreeSet::default()), + )); + }); + + // Authority rotation can recover and succeed. + backup_nodes.iter().for_each(|validator| { + testnet.set_active(validator, true); + }); + testnet.submit_heartbeat_all_engines(); + testnet.move_forward_blocks(VAULT_ROTATION_BLOCKS + 1); + assert_eq!(GENESIS_EPOCH + 1, Validator::epoch_index(), "We should be in a new epoch"); + }); +} + +#[test] +fn authority_rotation_can_recover_after_key_handover_fails() { + const EPOCH_BLOCKS: u32 = 1000; + const MAX_AUTHORITIES: AuthorityCount = 10; + super::genesis::default() + .blocks_per_epoch(EPOCH_BLOCKS) + .max_authorities(MAX_AUTHORITIES) + .build() + .execute_with(|| { + let (mut testnet, _, backup_nodes) = fund_authorities_and_join_auction(MAX_AUTHORITIES); + // Rotate authority at least once to ensure epoch keys are set. + testnet.move_to_next_epoch(); + testnet.submit_heartbeat_all_engines(); + testnet.move_forward_blocks(VAULT_ROTATION_BLOCKS); + assert_eq!(GENESIS_EPOCH + 1, Validator::epoch_index(), "We should be in a new epoch"); + + // Begin the second rotation. + testnet.move_forward_blocks(EPOCH_BLOCKS); + testnet.submit_heartbeat_all_engines(); + testnet.move_forward_blocks(4); + + // Make Key Handover fail. Only Bitcoin vault can fail during Key Handover. + // Ethereum and Polkadot do not need to wait for Key Handover. + backup_nodes.iter().for_each(|validator| { + testnet.set_active(validator, false); + }); + testnet.move_forward_blocks(1); + backup_nodes.iter().for_each(|validator| { + assert_ok!(BitcoinVault::report_key_handover_outcome( + RuntimeOrigin::signed(validator.clone()), + BitcoinVault::ceremony_id_counter(), + Err(BTreeSet::default()), + )); + assert_err!( + EthereumVault::report_key_handover_outcome( + RuntimeOrigin::signed(validator.clone()), + EthereumVault::ceremony_id_counter(), + Err(BTreeSet::default()), + ), + pallet_cf_vaults::Error::::InvalidRotationStatus + ); + assert_err!( + PolkadotVault::report_key_handover_outcome( + RuntimeOrigin::signed(validator.clone()), + EthereumVault::ceremony_id_counter(), + Err(BTreeSet::default()), + ), + pallet_cf_vaults::Error::::InvalidRotationStatus + ); + }); + + testnet.move_forward_blocks(1); + assert!(matches!( + Validator::current_rotation_phase(), + RotationPhase::KeyHandoversInProgress(..) + )); + assert_eq!( + AllVaults::status(), + AsyncResult::Ready(VaultStatus::Failed(BTreeSet::default())) + ); + + // Key handovers are retried after failure. + // Authority rotation can recover and succeed. + backup_nodes.iter().for_each(|validator| { + testnet.set_active(validator, true); + }); + testnet.submit_heartbeat_all_engines(); + testnet.move_forward_blocks(VAULT_ROTATION_BLOCKS); + assert_eq!(GENESIS_EPOCH + 2, Validator::epoch_index(), "We should be in a new epoch"); }); } diff --git a/state-chain/cf-integration-tests/src/lib.rs b/state-chain/cf-integration-tests/src/lib.rs index 6883461ec3..f8ba166ccb 100644 --- a/state-chain/cf-integration-tests/src/lib.rs +++ b/state-chain/cf-integration-tests/src/lib.rs @@ -1,5 +1,6 @@ #![cfg(test)] #![feature(exclusive_range_pattern)] +#![feature(local_key_cell_methods)] mod network; mod signer_nomination; @@ -25,8 +26,9 @@ use sp_consensus_aura::sr25519::AuthorityId as AuraId; use sp_consensus_grandpa::AuthorityId as GrandpaId; use sp_core::crypto::Pair; use state_chain_runtime::{ - constants::common::*, opaque::SessionKeys, AccountId, Emissions, Flip, Funding, Governance, - Reputation, Runtime, RuntimeOrigin, System, Validator, + constants::common::*, opaque::SessionKeys, AccountId, BitcoinVault, Emissions, EthereumVault, + Flip, Funding, Governance, PolkadotVault, Reputation, Runtime, RuntimeOrigin, System, + Validator, }; type NodeId = AccountId32; @@ -56,10 +58,13 @@ pub fn get_validator_state(account_id: &AccountId) -> ChainflipAccountState { } // The minimum number of blocks a vault rotation should last -const VAULT_ROTATION_BLOCKS: BlockNumber = 6; +// 4 (keygen + key verification) + 4(key handover) + 2(activating_key) + 2(session rotating) +const VAULT_ROTATION_BLOCKS: BlockNumber = 12; #[derive(PartialEq, Eq, Clone, Copy, Debug)] pub enum ChainflipAccountState { CurrentAuthority, Backup, } + +pub type AllVaults = ::VaultRotator; diff --git a/state-chain/cf-integration-tests/src/network.rs b/state-chain/cf-integration-tests/src/network.rs index 05cabb58b3..efc5a760ca 100644 --- a/state-chain/cf-integration-tests/src/network.rs +++ b/state-chain/cf-integration-tests/src/network.rs @@ -1,24 +1,30 @@ use super::*; -use crate::threshold_signing::{ - BtcKeyComponents, BtcThresholdSigner, DotKeyComponents, DotThresholdSigner, EthKeyComponents, - EthThresholdSigner, KeyUtils, ThresholdSigner, -}; -use cf_chains::{dot::PolkadotSignature, evm::SchnorrVerificationComponents, Chain, ChainCrypto}; +use crate::threshold_signing::{BtcThresholdSigner, DotThresholdSigner, EthThresholdSigner}; -use cf_primitives::{AccountRole, CeremonyId, EpochIndex, FlipBalance, TxId, GENESIS_EPOCH}; -use cf_traits::{AccountRoleRegistry, EpochInfo}; +use cf_primitives::{AccountRole, EpochIndex, FlipBalance, TxId, GENESIS_EPOCH}; +use cf_traits::{AccountRoleRegistry, EpochInfo, VaultRotator}; use chainflip_node::test_account_from_seed; use codec::Encode; -use frame_support::traits::{OnFinalize, OnIdle}; +use frame_support::{ + dispatch::UnfilteredDispatchable, + inherent::ProvideInherent, + pallet_prelude::InherentData, + traits::{IntegrityTest, OnFinalize, OnIdle}, +}; use pallet_cf_funding::{MinimumFunding, RedemptionAmount}; use pallet_cf_validator::RotationPhase; +use sp_consensus_aura::SlotDuration; use sp_std::collections::btree_set::BTreeSet; use state_chain_runtime::{ - AccountRoles, Authorship, BitcoinInstance, EthereumInstance, PolkadotInstance, RuntimeEvent, - RuntimeOrigin, Weight, + AccountRoles, AllPalletsWithSystem, BitcoinInstance, EthereumInstance, PolkadotInstance, + Runtime, RuntimeCall, RuntimeEvent, RuntimeOrigin, Weight, +}; +use std::{ + cell::RefCell, + collections::{HashMap, VecDeque}, + rc::Rc, }; -use std::{cell::RefCell, collections::HashMap, rc::Rc}; // TODO: Can we use the actual events here? // Events from ethereum contract @@ -53,7 +59,7 @@ pub fn create_testnet_with_new_funder() -> (Network, AccountId32) { GENESIS_EPOCH, ); // register the funds - testnet.move_forward_blocks(1); + testnet.move_forward_blocks(2); (testnet, new_backup) } @@ -122,12 +128,11 @@ impl Cli { } pub fn register_as_validator(account: &NodeId) { - assert_ok!( - >::register_account_role( - account, - AccountRole::Validator - ) - ); + System::inc_providers(account); + assert_ok!(>::register_account_role( + account, + AccountRole::Validator + )); } } @@ -167,35 +172,37 @@ impl Engine { if self.state() == ChainflipAccountState::CurrentAuthority && self.live { match event { ContractEvent::Funded { node_id: validator_id, amount, epoch, .. } => { - state_chain_runtime::Witnesser::witness_at_epoch( + queue_dispatch_extrinsic( + RuntimeCall::Witnesser(pallet_cf_witnesser::Call::witness_at_epoch { + call: Box::new( + pallet_cf_funding::Call::funded { + account_id: validator_id.clone(), + amount: *amount, + funder: ETH_ZERO_ADDRESS, + tx_hash: TX_HASH, + } + .into(), + ), + epoch_index: *epoch, + }), RuntimeOrigin::signed(self.node_id.clone()), - Box::new( - pallet_cf_funding::Call::funded { - account_id: validator_id.clone(), - amount: *amount, - funder: ETH_ZERO_ADDRESS, - tx_hash: TX_HASH, - } - .into(), - ), - *epoch, - ) - .expect("should be able to witness event for node"); + ); }, ContractEvent::Redeemed { node_id, amount, epoch } => { - state_chain_runtime::Witnesser::witness_at_epoch( + queue_dispatch_extrinsic( + RuntimeCall::Witnesser(pallet_cf_witnesser::Call::witness_at_epoch { + call: Box::new( + pallet_cf_funding::Call::redeemed { + account_id: node_id.clone(), + redeemed_amount: *amount, + tx_hash: TX_HASH, + } + .into(), + ), + epoch_index: *epoch, + }), RuntimeOrigin::signed(self.node_id.clone()), - Box::new( - pallet_cf_funding::Call::redeemed { - account_id: node_id.clone(), - redeemed_amount: *amount, - tx_hash: TX_HASH, - } - .into(), - ), - *epoch, - ) - .expect("should be able to witness event for node"); + ); }, } } @@ -207,7 +214,7 @@ impl Engine { if self.live { // Being a CurrentAuthority we would respond to certain events if self.state() == ChainflipAccountState::CurrentAuthority { - on_events!( + on_events! { events, RuntimeEvent::Validator( pallet_cf_validator::Event::NewEpoch(_epoch_index)) => { @@ -223,14 +230,15 @@ impl Engine { payload, .. }) => { - - // if we unwrap on this, we'll panic, because we will have already succeeded - // on a previous submission (all nodes submit this) - let _result = state_chain_runtime::EthereumThresholdSigner::signature_success( - RuntimeOrigin::none(), - *ceremony_id, - self.eth_threshold_signer.borrow().sign_with_key(*key, payload.as_fixed_bytes()), - ); + queue_dispatch_extrinsic( + RuntimeCall::EthereumThresholdSigner( + pallet_cf_threshold_signature::Call::signature_success{ + ceremony_id: *ceremony_id, + signature: self.eth_threshold_signer.borrow().sign_with_key(*key, payload.as_fixed_bytes()), + } + ), + RuntimeOrigin::none() + ); } RuntimeEvent::PolkadotThresholdSigner( @@ -240,11 +248,15 @@ impl Engine { payload, .. }) => { - let _result = state_chain_runtime::PolkadotThresholdSigner::signature_success( - RuntimeOrigin::none(), - *ceremony_id, - self.dot_threshold_signer.borrow().sign_with_key(*key, payload), - ); + queue_dispatch_extrinsic( + RuntimeCall::PolkadotThresholdSigner( + pallet_cf_threshold_signature::Call::signature_success{ + ceremony_id: *ceremony_id, + signature: self.dot_threshold_signer.borrow().sign_with_key(*key, payload), + } + ), + RuntimeOrigin::none() + ); } RuntimeEvent::BitcoinThresholdSigner( @@ -254,94 +266,72 @@ impl Engine { payload, .. }) => { - let _result = state_chain_runtime::BitcoinThresholdSigner::signature_success( - RuntimeOrigin::none(), - *ceremony_id, - vec![self.btc_threshold_signer.borrow().sign_with_key(*key, &(payload[0].1.clone()))], - ); + queue_dispatch_extrinsic( + RuntimeCall::BitcoinThresholdSigner( + pallet_cf_threshold_signature::Call::signature_success{ + ceremony_id: *ceremony_id, + signature: vec![self.btc_threshold_signer.borrow().sign_with_key(*key, &(payload[0].1.clone()))], + } + ), RuntimeOrigin::none() + ); } - RuntimeEvent::Validator( + RuntimeEvent::Validator(pallet_cf_validator::Event::RotationPhaseUpdated { new_phase: RotationPhase::ActivatingKeys(_) }) => { // NOTE: This is a little inaccurate a representation of how it actually works. An event is emitted // which contains the transaction to broadcast for the rotation tx, which the CFE then broadcasts. // This is a simpler way to represent this in the tests. Representing in this way in the tests also means // that for dot, given we don't have a key to sign with initially, it will work without extra test boilerplate. - pallet_cf_validator::Event::RotationPhaseUpdated { new_phase: RotationPhase::ActivatingKeys(_) }) => { - // If we rotating let's witness the keys being rotated on the contract - let _result = state_chain_runtime::Witnesser::witness_at_epoch( - RuntimeOrigin::signed(self.node_id.clone()), - Box::new(pallet_cf_vaults::Call::<_, EthereumInstance>::vault_key_rotated { + // If we rotating let's witness the keys being rotated on the contract + queue_dispatch_extrinsic( + RuntimeCall::Witnesser( + pallet_cf_witnesser::Call::witness_at_epoch { + call: Box::new(pallet_cf_vaults::Call::<_, EthereumInstance>::vault_key_rotated { block_number: 100, tx_id: [1u8; 32].into(), }.into()), - Validator::epoch_index() - ); - - if Validator::epoch_index() == GENESIS_EPOCH { - let _result = state_chain_runtime::Environment::witness_polkadot_vault_creation( - pallet_cf_governance::RawOrigin::GovernanceApproval.into(), - // Use a dummy key for polkadot - we don't sign anything with it - // in these tests. - Default::default(), - TxId { - block_number: 1, - extrinsic_index: 0, - }, - ); - }else { - let _result = state_chain_runtime::Witnesser::witness_at_epoch( - RuntimeOrigin::signed(self.node_id.clone()), - Box::new(pallet_cf_vaults::Call::<_, PolkadotInstance>::vault_key_rotated { - block_number: 100, - tx_id: TxId { - block_number: 2, - extrinsic_index: 1, - }, - }.into()), - Validator::epoch_index() - ); - } - + epoch_index: Validator::epoch_index(), + }), + RuntimeOrigin::signed(self.node_id.clone()) + ); - let _result = state_chain_runtime::Witnesser::witness_at_epoch( - RuntimeOrigin::signed(self.node_id.clone()), - Box::new(pallet_cf_vaults::Call::<_, BitcoinInstance>::vault_key_rotated { + queue_dispatch_extrinsic( + RuntimeCall::Witnesser( + pallet_cf_witnesser::Call::witness_at_epoch { + call: Box::new(pallet_cf_vaults::Call::<_, BitcoinInstance>::vault_key_rotated { block_number: 100, tx_id: [2u8; 32], }.into()), - Validator::epoch_index() - ); - } - ); - } + epoch_index: Validator::epoch_index() + }), + RuntimeOrigin::signed(self.node_id.clone()) + ); - fn report_keygen_outcome_for_chain< - K: KeyUtils< - SigVerification = S, - AggKey = <<>::Chain as Chain>::ChainCrypto as ChainCrypto>::AggKey, - > + Clone, - S, - T: pallet_cf_vaults::Config, - I: 'static, - >( - ceremony_id: CeremonyId, - authorities: &BTreeSet, - threshold_signer: Rc>>, - node_id: NodeId, - ) where - ::RuntimeOrigin: - From, - { - if authorities.contains(&node_id) { - pallet_cf_vaults::Pallet::::report_keygen_outcome( - RuntimeOrigin::signed(node_id.clone()).into(), - ceremony_id, - Ok(threshold_signer.borrow_mut().propose_new_key()), - ) - .unwrap_or_else(|_| { - panic!("should be able to report keygen outcome: {node_id}") - }); - } + if Validator::epoch_index() == GENESIS_EPOCH { + queue_dispatch_extrinsic( + RuntimeCall::Environment(pallet_cf_environment::Call::witness_polkadot_vault_creation { + dot_pure_proxy_vault_key: Default::default(), + tx_id: TxId { + block_number: 1, + extrinsic_index: 0, + }, + } + ), pallet_cf_governance::RawOrigin::GovernanceApproval.into()); + } else { + queue_dispatch_extrinsic(RuntimeCall::Witnesser( + pallet_cf_witnesser::Call::witness_at_epoch { + call: Box::new(pallet_cf_vaults::Call::<_, PolkadotInstance>::vault_key_rotated { + block_number: 100, + tx_id: TxId { + block_number: 2, + extrinsic_index: 1, + }, + }.into()), + epoch_index: Validator::epoch_index() + } + ), RuntimeOrigin::signed(self.node_id.clone())); + } + } + }; } // Being funded we would be required to respond to keygen requests @@ -349,30 +339,49 @@ impl Engine { events, RuntimeEvent::EthereumVault( pallet_cf_vaults::Event::KeygenRequest {ceremony_id, participants, .. }) => { - report_keygen_outcome_for_chain::(*ceremony_id, participants, self.eth_threshold_signer.clone(), self.node_id.clone()); + if participants.contains(&self.node_id) { + queue_dispatch_extrinsic(RuntimeCall::EthereumVault( + pallet_cf_vaults::Call::report_keygen_outcome { + ceremony_id: *ceremony_id, + reported_outcome: Ok(self.eth_threshold_signer.borrow_mut().propose_new_key()), + } + ), RuntimeOrigin::signed(self.node_id.clone())); + } } RuntimeEvent::PolkadotVault( pallet_cf_vaults::Event::KeygenRequest {ceremony_id, participants, .. }) => { - report_keygen_outcome_for_chain::(*ceremony_id, participants, self.dot_threshold_signer.clone(), self.node_id.clone()); + if participants.contains(&self.node_id) { + queue_dispatch_extrinsic(RuntimeCall::PolkadotVault( + pallet_cf_vaults::Call::report_keygen_outcome { + ceremony_id: *ceremony_id, + reported_outcome: Ok(self.dot_threshold_signer.borrow_mut().propose_new_key()), + } + ), RuntimeOrigin::signed(self.node_id.clone())); + } } RuntimeEvent::BitcoinVault( pallet_cf_vaults::Event::KeygenRequest {ceremony_id, participants, .. }) => { - report_keygen_outcome_for_chain::(*ceremony_id, participants, self.btc_threshold_signer.clone(), self.node_id.clone()); + if participants.contains(&self.node_id) { + queue_dispatch_extrinsic(RuntimeCall::BitcoinVault( + pallet_cf_vaults::Call::report_keygen_outcome { + ceremony_id: *ceremony_id, + reported_outcome: Ok(self.btc_threshold_signer.borrow_mut().propose_new_key()), + } + ), RuntimeOrigin::signed(self.node_id.clone())); + } } RuntimeEvent::BitcoinVault( pallet_cf_vaults::Event::KeyHandoverRequest {ceremony_id, sharing_participants, receiving_participants, .. }) => { let all_participants = sharing_participants.union(receiving_participants).cloned().collect::>(); if all_participants.contains(&self.node_id) { - pallet_cf_vaults::Pallet::::report_key_handover_outcome( - RuntimeOrigin::signed(self.node_id.clone()), - *ceremony_id, - Ok(self.btc_threshold_signer.borrow_mut().current_agg_key()), - ).unwrap_or_else(|_| { - panic!("should be able to report key handover outcome: {}", self.node_id) - }); + queue_dispatch_extrinsic(RuntimeCall::BitcoinVault( + pallet_cf_vaults::Call::report_key_handover_outcome { + ceremony_id: *ceremony_id, + reported_outcome: Ok(self.btc_threshold_signer.borrow_mut().propose_new_key()), + } + ), RuntimeOrigin::signed(self.node_id.clone())); } - } ); } @@ -390,7 +399,7 @@ pub(crate) fn setup_account(node_id: &NodeId) { let seed = &node_id.clone().to_string(); assert_ok!(state_chain_runtime::Session::set_keys( - state_chain_runtime::RuntimeOrigin::signed(node_id.clone()), + RuntimeOrigin::signed(node_id.clone()), SessionKeys { aura: test_account_from_seed::(seed), grandpa: test_account_from_seed::(seed), @@ -404,7 +413,7 @@ pub(crate) fn setup_peer_mapping(node_id: &NodeId) { let peer_keypair = sp_core::ed25519::Pair::from_legacy_string(seed, None); assert_ok!(state_chain_runtime::Validator::register_peer_id( - state_chain_runtime::RuntimeOrigin::signed(node_id.clone()), + RuntimeOrigin::signed(node_id.clone()), peer_keypair.public(), 0, 0, @@ -416,7 +425,6 @@ pub(crate) fn setup_peer_mapping(node_id: &NodeId) { pub struct Network { engines: HashMap, pub state_chain_gateway_contract: ScGatewayContract, - last_event: usize, node_counter: u32, // Used to initialised the threshold signers of the engines added @@ -425,6 +433,65 @@ pub struct Network { pub btc_threshold_signer: Rc>, } +thread_local! { + // TODO: USE THIS IN WHEN HANDLING EVENTS INSTEAD OF DISPATCHING CALLS DIRECTLY + static PENDING_EXTRINSICS: RefCell> = RefCell::default(); + static TIMESTAMP: RefCell = RefCell::new(SLOT_DURATION); +} + +fn queue_dispatch_extrinsic(call: impl Into, origin: RuntimeOrigin) { + PENDING_EXTRINSICS.with_borrow_mut(|v| { + v.push_back((call.into(), origin)); + }); +} + +/// Dispatch all pending extrinsics in the queue. +pub fn dispatch_all_pending_extrinsics() { + PENDING_EXTRINSICS.with_borrow_mut(|v| { + v.drain(..).for_each(|(call, origin)| { + let expect_ok = match call { + RuntimeCall::EthereumThresholdSigner(..) | + RuntimeCall::PolkadotThresholdSigner(..) | + RuntimeCall::BitcoinThresholdSigner(..) | + RuntimeCall::Environment(..) => { + // These are allowed to fail, since it is possible to sign things + // that have already succeeded + false + }, + _ => true, + }; + let res = call.clone().dispatch_bypass_filter(origin); + if expect_ok && res.is_err() { + // An extrinsic failed. Log as much info as needed to help debugging. + match call { + RuntimeCall::EthereumVault(..) => log::info!( + "Validator status: {:?}\nVault Status: {:?}", + Validator::current_rotation_phase(), + EthereumVault::pending_vault_rotations() + ), + RuntimeCall::PolkadotVault(..) => log::info!( + "Validator status: {:?}\nVault Status: {:?}", + Validator::current_rotation_phase(), + PolkadotVault::pending_vault_rotations() + ), + RuntimeCall::BitcoinVault(..) => log::info!( + "Validator status: {:?}\nVault Status: {:?}", + Validator::current_rotation_phase(), + BitcoinVault::pending_vault_rotations() + ), + RuntimeCall::Validator(..) => log::info!( + "Validator status: {:?}\nAllVaults Status: {:?}", + Validator::current_rotation_phase(), + AllVaults::status() + ), + _ => {}, + } + panic!("Extrinsic Failed. Call: {:?} \n Result: {:?}", call, res); + } + }); + }); +} + impl Network { pub fn next_node_id(&mut self) -> NodeId { self.node_counter += 1; @@ -493,52 +560,66 @@ impl Network { pub fn submit_heartbeat_all_engines(&self) { for engine in self.engines.values() { - let _result = Reputation::heartbeat(state_chain_runtime::RuntimeOrigin::signed( - engine.node_id.clone(), - )); + let _result = Reputation::heartbeat(RuntimeOrigin::signed(engine.node_id.clone())); } } pub fn move_forward_blocks(&mut self, n: u32) { - let current_block_number = System::block_number(); - while System::block_number() < current_block_number + n { - let block_number = System::block_number() + 1; - - System::initialize(&block_number, &System::block_hash(block_number), &{ - let mut digest = sp_runtime::Digest::default(); - digest.push(sp_runtime::DigestItem::PreRuntime( - sp_consensus_aura::AURA_ENGINE_ID, - sp_consensus_aura::Slot::from(block_number as u64).encode(), - )); - digest - }); - - state_chain_runtime::AllPalletsWithoutSystem::on_initialize(block_number); - // We must finalise this to clear the previous author which is otherwise cached - Authorship::on_finalize(block_number); - - // Provide very large weight to ensure all on_idle processing can occur - state_chain_runtime::AllPalletsWithoutSystem::on_idle( - block_number, - Weight::from_parts(1_000_000_000_000, 0), - ); - + let start_block = System::block_number() + 1; + for block_number in start_block..(start_block + n) { + // Process any external events that have occurred. for event in self.state_chain_gateway_contract.events() { for engine in self.engines.values() { engine.on_contract_event(&event); } } - self.state_chain_gateway_contract.clear(); + // Inherent data. + let timestamp = TIMESTAMP.with_borrow_mut(|t| std::mem::replace(t, *t + SLOT_DURATION)); + let slot = sp_consensus_aura::Slot::from_timestamp( + sp_timestamp::Timestamp::new(timestamp), + SlotDuration::from_millis(SLOT_DURATION), + ); + let mut inherent_data = InherentData::new(); + inherent_data.put_data(sp_timestamp::INHERENT_IDENTIFIER, ×tamp).unwrap(); + inherent_data + .put_data(sp_consensus_aura::inherents::INHERENT_IDENTIFIER, &slot) + .unwrap(); + + // Header digest. + let mut digest = sp_runtime::Digest::default(); + digest.push(sp_runtime::DigestItem::PreRuntime( + sp_consensus_aura::AURA_ENGINE_ID, + slot.encode(), + )); + + // Reset events before on_initialise, same as in frame_executive. + System::reset_events(); + + // Initialize + System::initialize(&block_number, &System::block_hash(block_number), &digest); + AllPalletsWithSystem::on_initialize(block_number); + + // Inherents + assert_ok!(state_chain_runtime::Timestamp::create_inherent(&inherent_data) + .unwrap() + .dispatch_bypass_filter(RuntimeOrigin::none())); + + dispatch_all_pending_extrinsics(); + + // Provide very large weight to ensure all on_idle processing can occur + AllPalletsWithSystem::on_idle(block_number, Weight::from_parts(1_000_000_000_000, 0)); + + // We must finalise this to clear the previous author which is otherwise cached + AllPalletsWithSystem::on_finalize(block_number); + AllPalletsWithSystem::integrity_test(); + + // Engine reacts to events from the State Chain. let events = frame_system::Pallet::::events() .into_iter() .map(|e| e.event) - .skip(self.last_event) .collect::>(); - - self.last_event += events.len(); - for engine in self.engines.values_mut() { engine.handle_state_chain_events(&events); } diff --git a/state-chain/cf-integration-tests/src/new_epoch.rs b/state-chain/cf-integration-tests/src/new_epoch.rs index 34479d418f..f9b4e523bb 100644 --- a/state-chain/cf-integration-tests/src/new_epoch.rs +++ b/state-chain/cf-integration-tests/src/new_epoch.rs @@ -88,8 +88,7 @@ fn auction_repeats_after_failure_because_of_liveness() { } #[test] -// An epoch has completed. We have a genesis where the blocks per epoch are -// set to 100 +// An epoch has completed. We have a genesis where the blocks per epoch are set to 100 // - When the epoch is reached an auction is started and completed // - All nodes add funds above the MAB // - We have two nodes that haven't registered their session keys diff --git a/state-chain/cf-integration-tests/src/threshold_signing.rs b/state-chain/cf-integration-tests/src/threshold_signing.rs index 20e8155f59..9325d01d87 100644 --- a/state-chain/cf-integration-tests/src/threshold_signing.rs +++ b/state-chain/cf-integration-tests/src/threshold_signing.rs @@ -98,10 +98,6 @@ where } } - pub fn current_agg_key(&self) -> AggKey { - self.key_components.agg_key() - } - pub fn propose_new_key(&mut self) -> AggKey { let new_key = KeyComponents::generate_next(&self.key_components); let agg_key = new_key.agg_key(); @@ -205,7 +201,10 @@ impl KeyUtils for BtcKeyComponents { } fn generate_next(&self) -> Self { - Self::generate(self.seed + 1, self.epoch_index + 1) + let prev_agg_key = self.agg_key.current; + let mut generated = Self::generate(self.seed + 1, self.epoch_index + 1); + generated.agg_key.previous = Some(prev_agg_key); + generated } fn agg_key(&self) -> Self::AggKey { diff --git a/state-chain/pallets/cf-vaults/src/lib.rs b/state-chain/pallets/cf-vaults/src/lib.rs index 397a7c5174..5ef4649c50 100644 --- a/state-chain/pallets/cf-vaults/src/lib.rs +++ b/state-chain/pallets/cf-vaults/src/lib.rs @@ -63,7 +63,7 @@ pub type KeyHandoverResponseStatus = impl_pallet_safe_mode!(PalletSafeMode; slashing_enabled); /// The current status of a vault rotation. -#[derive(PartialEq, Eq, Clone, Encode, Decode, TypeInfo, RuntimeDebug, EnumVariant)] +#[derive(PartialEq, Eq, Clone, Encode, Decode, TypeInfo, RuntimeDebugNoBound, EnumVariant)] #[scale_info(skip_type_params(T, I))] pub enum VaultRotationStatus, I: 'static = ()> { /// We are waiting for nodes to generate a new aggregate key. diff --git a/state-chain/pallets/cf-vaults/src/vault_rotator.rs b/state-chain/pallets/cf-vaults/src/vault_rotator.rs index a20f11f428..97ab9e1cce 100644 --- a/state-chain/pallets/cf-vaults/src/vault_rotator.rs +++ b/state-chain/pallets/cf-vaults/src/vault_rotator.rs @@ -98,12 +98,14 @@ impl, I: 'static> VaultRotator for Pallet { Self::deposit_event(Event::::NoKeyHandover); }, }, - _ => { - debug_assert!( - false, - "We should have completed keygen verification before starting key handover." + Some(VaultRotationStatus::::KeyHandoverComplete { .. }) => { + log::info!( + target: Pallet::::name(), + "Key handover already complete." ); - log::error!("Key handover called during wrong wrong state."); + }, + other => { + log_or_panic!("Key handover initiated during invalid state {:?}.", other); }, } } diff --git a/state-chain/test-utilities/src/rich_test_externalities.rs b/state-chain/test-utilities/src/rich_test_externalities.rs index 3a2d7d26da..9d56899fd5 100644 --- a/state-chain/test-utilities/src/rich_test_externalities.rs +++ b/state-chain/test-utilities/src/rich_test_externalities.rs @@ -2,7 +2,7 @@ use frame_support::{ assert_noop, assert_ok, dispatch::UnfilteredDispatchable, pallet_prelude::DispatchResult, - traits::{OnFinalize, OnIdle, OnInitialize}, + traits::{IntegrityTest, OnFinalize, OnIdle, OnInitialize}, weights::Weight, }; use frame_system::pallet_prelude::BlockNumberFor; @@ -12,7 +12,8 @@ use sp_runtime::BuildStorage; pub trait HasAllPallets: frame_system::Config { type AllPalletsWithSystem: OnInitialize> + OnIdle> - + OnFinalize>; + + OnFinalize> + + IntegrityTest; fn on_initialize(block_number: BlockNumberFor) { >>::on_initialize( @@ -25,6 +26,9 @@ pub trait HasAllPallets: frame_system::Config { fn on_finalize(block_number: BlockNumberFor) { >>::on_finalize(block_number); } + fn integrity_test() { + ::integrity_test(); + } } /// Basic [sp_io::TestExternalities] wrapper that provides a richer API for testing pallets. @@ -70,6 +74,7 @@ impl RichExternalities { let context = f(); Runtime::on_idle(block_number, Weight::MAX); Runtime::on_finalize(block_number); + Runtime::integrity_test(); context }); TestExternalities { ext: self, context } @@ -105,6 +110,7 @@ where let mut ext: sp_io::TestExternalities = config.build_storage().unwrap().into(); ext.execute_with(|| { frame_system::Pallet::::set_block_number(1u32.into()); + Runtime::integrity_test(); }); TestExternalities { ext: RichExternalities::new(ext), context: () } }