diff --git a/Cargo.lock b/Cargo.lock index 6b284f469b..f2de3243c1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7251,6 +7251,7 @@ dependencies = [ "cf-chains", "cf-primitives", "cf-runtime-upgrade-utilities", + "cf-runtime-utilities", "cf-test-utilities", "cf-traits", "frame-benchmarking", diff --git a/bouncer/commands/go_bananas.ts b/bouncer/commands/go_bananas.ts index bcfa660774..d1f1bc7494 100755 --- a/bouncer/commands/go_bananas.ts +++ b/bouncer/commands/go_bananas.ts @@ -133,7 +133,7 @@ async function playLp(asset: Asset, price: number, liquidity: number) { async function launchTornado() { const chainflip = await getChainflipApi(); - const epoch = (await chainflip.query.bitcoinVault.currentVaultEpochAndState()).toJSON()! + const epoch = (await chainflip.query.bitcoinVault.currentVaultEpoch()).toJSON()! .epochIndex as number; const pubkey = ( (await chainflip.query.bitcoinVault.vaults(epoch)).toJSON()!.publicKey.current as string diff --git a/bouncer/pnpm-lock.yaml b/bouncer/pnpm-lock.yaml index 2d03e99181..3b726dd4dc 100644 --- a/bouncer/pnpm-lock.yaml +++ b/bouncer/pnpm-lock.yaml @@ -1,4 +1,4 @@ -lockfileVersion: '6.0' +lockfileVersion: '6.1' settings: autoInstallPeers: true diff --git a/engine/src/witness/eth/key_manager.rs b/engine/src/witness/eth/key_manager.rs index 21a6ab27f4..9eef32ab84 100644 --- a/engine/src/witness/eth/key_manager.rs +++ b/engine/src/witness/eth/key_manager.rs @@ -84,14 +84,6 @@ impl ChunkedByVaultBuilder { { info!("Handling event: {event}"); let call: state_chain_runtime::RuntimeCall = match event.event_parameters { - KeyManagerEvents::AggKeySetByAggKeyFilter(_) => pallet_cf_vaults::Call::< - _, - ::Instance, - >::vault_key_rotated { - block_number: header.index, - tx_id: event.tx_hash, - } - .into(), KeyManagerEvents::AggKeySetByGovKeyFilter(AggKeySetByGovKeyFilter { new_agg_key, .. diff --git a/state-chain/cf-integration-tests/src/authorities.rs b/state-chain/cf-integration-tests/src/authorities.rs index 6e42f3454d..fc61b665a0 100644 --- a/state-chain/cf-integration-tests/src/authorities.rs +++ b/state-chain/cf-integration-tests/src/authorities.rs @@ -8,12 +8,12 @@ use sp_runtime::AccountId32; use std::collections::{BTreeSet, HashMap}; use cf_primitives::{AuthorityCount, FlipBalance, GENESIS_EPOCH}; -use cf_traits::{AsyncResult, EpochInfo, SafeMode, VaultRotator, VaultStatus}; +use cf_traits::{AsyncResult, EpochInfo, VaultRotator, VaultStatus}; use pallet_cf_environment::SafeModeUpdate; use pallet_cf_validator::{CurrentRotationPhase, RotationPhase}; use state_chain_runtime::{ - safe_mode::RuntimeSafeMode, BitcoinVault, Environment, EthereumInstance, EthereumVault, Flip, - PolkadotInstance, PolkadotVault, Runtime, RuntimeOrigin, Validator, + 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. @@ -97,11 +97,13 @@ fn authority_rotates_with_correct_sequence() { assert_eq!(AllVaults::status(), AsyncResult::Ready(VaultStatus::KeyHandoverComplete)); // Activate new key. - testnet.move_forward_blocks(2); + // The key is immediately activated in the next block + testnet.move_forward_blocks(1); assert!(matches!( Validator::current_rotation_phase(), RotationPhase::ActivatingKeys(..) )); + assert_eq!( AllVaults::status(), AsyncResult::Ready(VaultStatus::RotationComplete), @@ -294,7 +296,8 @@ fn authority_rotation_can_succeed_after_aborted_by_safe_mode() { } #[test] -fn authority_rotation_cannot_be_aborted_after_key_handover_but_stalls_on_safe_mode() { +fn authority_rotation_cannot_be_aborted_after_key_handover_and_completes_even_on_safe_mode_enabled() +{ const EPOCH_BLOCKS: u32 = 1000; const MAX_AUTHORITIES: AuthorityCount = 10; super::genesis::default() @@ -324,20 +327,10 @@ 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. - assert!(matches!(AllVaults::status(), AsyncResult::Pending)); - - // We activate witnessing calls by setting safe mode to code green just for the - // witnesser pallet. - 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(), - SafeModeUpdate::CodeAmber(runtime_safe_mode_with_witnessing) + assert!(matches!( + AllVaults::status(), + AsyncResult::Ready(VaultStatus::RotationComplete) )); - - // 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"); }); diff --git a/state-chain/cf-integration-tests/src/funding.rs b/state-chain/cf-integration-tests/src/funding.rs index ae998ba7c2..4845c24c59 100644 --- a/state-chain/cf-integration-tests/src/funding.rs +++ b/state-chain/cf-integration-tests/src/funding.rs @@ -156,33 +156,31 @@ fn backup_reward_is_calculated_linearly() { // 3 backup will split the backup reward. assert_eq!(Validator::highest_funded_qualified_backup_node_bids().count(), 3); + const N: u128 = 100; - let rewards_per_block = &calculate_backup_rewards::( + let rewards_per_heartbeat = &calculate_backup_rewards::( Validator::highest_funded_qualified_backup_node_bids().collect::>(), Validator::bond(), - 1u128, + HEARTBEAT_BLOCK_INTERVAL as u128, Emissions::backup_node_emission_per_block(), Emissions::current_authority_emission_per_block(), Validator::current_authority_count() as u128, ); - let rewards_per_heartbeat = &calculate_backup_rewards::( + let rewards_per_n_heartbeats = &calculate_backup_rewards::( Validator::highest_funded_qualified_backup_node_bids().collect::>(), Validator::bond(), - HEARTBEAT_BLOCK_INTERVAL as u128, + HEARTBEAT_BLOCK_INTERVAL as u128 * N, Emissions::backup_node_emission_per_block(), Emissions::current_authority_emission_per_block(), Validator::current_authority_count() as u128, ); - for i in 0..rewards_per_block.len() { + for i in 0..rewards_per_heartbeat.len() { // Validator account should match - assert_eq!(rewards_per_block[i].0, rewards_per_heartbeat[i].0); + assert_eq!(rewards_per_heartbeat[i].0, rewards_per_heartbeat[i].0); // Reward per heartbeat should be scaled linearly. - assert_eq!( - rewards_per_heartbeat[i].1, - rewards_per_block[i].1 * HEARTBEAT_BLOCK_INTERVAL as u128 - ); + assert_eq!(rewards_per_n_heartbeats[i].1, rewards_per_heartbeat[i].1 * N); } }); } @@ -270,7 +268,7 @@ fn apy_can_be_above_100_percent() { MAX_AUTHORITIES as u128; let apy_basis_point = FixedU64::from_rational(reward, total).checked_mul_int(10_000u32).unwrap(); - assert_eq!(apy_basis_point, 241_377_726u32); + assert_eq!(apy_basis_point, 241_377_727u32); assert_eq!(calculate_account_apy(&validator), Some(apy_basis_point)); }); } diff --git a/state-chain/cf-integration-tests/src/network.rs b/state-chain/cf-integration-tests/src/network.rs index 5dc89546f4..5ea02ad770 100644 --- a/state-chain/cf-integration-tests/src/network.rs +++ b/state-chain/cf-integration-tests/src/network.rs @@ -13,12 +13,11 @@ use frame_support::{ 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, AllPalletsWithSystem, BitcoinInstance, EthereumInstance, PolkadotInstance, - Runtime, RuntimeCall, RuntimeEvent, RuntimeOrigin, Weight, + AccountRoles, AllPalletsWithSystem, BitcoinInstance, PolkadotInstance, Runtime, RuntimeCall, + RuntimeEvent, RuntimeOrigin, Weight, }; use std::{ cell::RefCell, @@ -295,26 +294,6 @@ impl Engine { ), RuntimeOrigin::none() ); } - 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. - - // 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()), - epoch_index: Validator::epoch_index(), - }), - RuntimeOrigin::signed(self.node_id.clone()) - ); - } - RuntimeEvent::PolkadotVault(pallet_cf_vaults::Event::<_, PolkadotInstance>::AwaitingGovernanceActivation { .. }) => { queue_dispatch_extrinsic( RuntimeCall::Environment(pallet_cf_environment::Call::witness_polkadot_vault_creation { @@ -596,8 +575,7 @@ impl Network { true => true, false => engine.auto_submit_heartbeat && - engine.last_heartbeat + Validator::blocks_per_epoch() - 1 <= - current_block, + engine.last_heartbeat + HEARTBEAT_BLOCK_INTERVAL - 1 <= current_block, } { assert_ok!(Reputation::heartbeat(RuntimeOrigin::signed(engine.node_id.clone()))); engine.last_heartbeat = current_block; diff --git a/state-chain/cf-integration-tests/src/swapping.rs b/state-chain/cf-integration-tests/src/swapping.rs index 81df701cdd..947e8846d0 100644 --- a/state-chain/cf-integration-tests/src/swapping.rs +++ b/state-chain/cf-integration-tests/src/swapping.rs @@ -11,6 +11,7 @@ use cf_chains::{ address::{AddressConverter, AddressDerivationApi, EncodedAddress}, assets::eth::Asset as EthAsset, eth::{api::EthereumApi, EthereumTrackedData}, + evm::TransactionFee, CcmChannelMetadata, CcmDepositMetadata, Chain, ChainState, Ethereum, ExecutexSwapAndCall, ForeignChain, ForeignChainAddress, SwapOrigin, TransactionBuilder, TransferAssetParams, }; @@ -18,15 +19,15 @@ use cf_primitives::{ AccountId, AccountRole, Asset, AssetAmount, AuthorityCount, GENESIS_EPOCH, STABLE_ASSET, }; use cf_test_utilities::{assert_events_eq, assert_events_match}; -use cf_traits::{AccountRoleRegistry, EpochInfo, LpBalanceApi}; +use cf_traits::{AccountRoleRegistry, Chainflip, EpochInfo, LpBalanceApi}; use frame_support::{ assert_ok, instances::Instance1, traits::{OnFinalize, OnIdle, OnNewAccount}, }; use pallet_cf_broadcast::{ - AwaitingBroadcast, BroadcastAttemptId, RequestFailureCallbacks, RequestSuccessCallbacks, - ThresholdSignatureData, TransactionSigningAttempt, + AwaitingBroadcast, BroadcastAttemptId, BroadcastIdCounter, RequestFailureCallbacks, + RequestSuccessCallbacks, ThresholdSignatureData, TransactionSigningAttempt, }; use pallet_cf_ingress_egress::{DepositWitness, FailedCcm}; use pallet_cf_pools::{OrderId, RangeOrderSize}; @@ -770,6 +771,33 @@ fn can_resign_failed_ccm() { } testnet.move_to_the_next_epoch(); + let tx_out_id = AwaitingBroadcast::::get(BroadcastAttemptId { + broadcast_id: 1, + attempt_count: 0, + }) + .unwrap() + .broadcast_attempt + .transaction_out_id; + + for node in Validator::current_authorities() { + // Broadcast success for id 1, which is the rotation transaction. + // This needs to succeed because it's a barrier broadcast. + assert_ok!(Witnesser::witness_at_epoch( + RuntimeOrigin::signed(node), + Box::new(RuntimeCall::EthereumBroadcaster( + pallet_cf_broadcast::Call::transaction_succeeded { + tx_out_id, + signer_id: Default::default(), + tx_fee: TransactionFee { + effective_gas_price: Default::default(), + gas_used: Default::default() + }, + tx_metadata: Default::default(), + } + )), + ::EpochInfo::current_epoch() + )); + } setup_pool_and_accounts(vec![Asset::Eth, Asset::Flip]); // Deposit CCM and process the swap @@ -801,27 +829,27 @@ fn can_resign_failed_ccm() { } // Process the swap -> egress -> threshold sign -> broadcast - let broadcast_id = 2; - assert!(EthereumBroadcaster::threshold_signature_data(broadcast_id).is_none()); testnet.move_forward_blocks(3); - - // Threshold signature is ready and the call is ready to be broadcasted. - assert!(EthereumBroadcaster::threshold_signature_data(broadcast_id).is_some()); - - let validators = Validator::current_authorities(); + let broadcast_id = BroadcastIdCounter::::get(); let mut broadcast_attempt_id = BroadcastAttemptId { broadcast_id, attempt_count: 0 }; // Fail the broadcast - for _ in 0..validators.len() { + for _ in Validator::current_authorities() { let TransactionSigningAttempt { broadcast_attempt: _attempt, nominee } = - AwaitingBroadcast::::get(broadcast_attempt_id).unwrap(); + AwaitingBroadcast::::get(broadcast_attempt_id) + .unwrap_or_else(|| { + panic!( + "Failed to get the transaction signing attempt for {:?}.", + broadcast_attempt_id, + ) + }); assert_ok!(EthereumBroadcaster::transaction_signing_failure( RuntimeOrigin::signed(nominee), broadcast_attempt_id, )); testnet.move_forward_blocks(1); - broadcast_attempt_id = broadcast_attempt_id.next_attempt(); + broadcast_attempt_id = broadcast_attempt_id.peek_next(); } // Upon broadcast failure, the Failure callback is called, and failed CCM is stored. diff --git a/state-chain/cf-integration-tests/src/threshold_signing.rs b/state-chain/cf-integration-tests/src/threshold_signing.rs index 43c334de2e..85cea9163d 100644 --- a/state-chain/cf-integration-tests/src/threshold_signing.rs +++ b/state-chain/cf-integration-tests/src/threshold_signing.rs @@ -76,6 +76,7 @@ impl KeyUtils for EthKeyComponents { } pub struct ThresholdSigner { + previous_key_components: Option, key_components: KeyComponents, proposed_key_components: Option, _phantom: PhantomData, @@ -90,8 +91,14 @@ where if key == current_key { return self.key_components.sign(message) } - let next_key = self.proposed_key_components.as_ref().unwrap().agg_key(); - if key == next_key { + if self.previous_key_components.is_some() && + self.previous_key_components.as_ref().unwrap().agg_key() == key + { + return self.previous_key_components.as_ref().unwrap().sign(message) + } + if self.proposed_key_components.is_some() && + self.proposed_key_components.as_ref().unwrap().agg_key() == key + { self.proposed_key_components.as_ref().unwrap().sign(message) } else { panic!("Unknown key"); @@ -108,6 +115,7 @@ where // Rotate to the current proposed key and clear the proposed key pub fn use_proposed_key(&mut self) { if self.proposed_key_components.is_some() { + self.previous_key_components = Some(self.key_components.clone()); self.key_components = self.proposed_key_components.as_ref().expect("No key has been proposed").clone(); self.proposed_key_components = None; @@ -133,6 +141,7 @@ pub type EthThresholdSigner = ThresholdSigner Self { ThresholdSigner { + previous_key_components: None, key_components: EthKeyComponents::generate(GENESIS_KEY_SEED, GENESIS_EPOCH), proposed_key_components: None, _phantom: PhantomData, @@ -147,6 +156,7 @@ pub type DotThresholdSigner = ThresholdSigner Self { Self { + previous_key_components: None, key_components: DotKeyComponents::generate(GENESIS_KEY_SEED, GENESIS_EPOCH), proposed_key_components: None, _phantom: PhantomData, @@ -184,6 +194,7 @@ pub type BtcThresholdSigner = ThresholdSigner; impl Default for BtcThresholdSigner { fn default() -> Self { Self { + previous_key_components: None, key_components: BtcKeyComponents::generate(GENESIS_KEY_SEED, GENESIS_EPOCH), proposed_key_components: None, _phantom: PhantomData, diff --git a/state-chain/chains/src/btc.rs b/state-chain/chains/src/btc.rs index ff5e12583d..a128b98750 100644 --- a/state-chain/chains/src/btc.rs +++ b/state-chain/chains/src/btc.rs @@ -248,6 +248,13 @@ impl ChainCrypto for BitcoinCrypto { fn handover_key_matches(current_key: &Self::AggKey, new_key: &Self::AggKey) -> bool { new_key.previous.is_some_and(|previous| current_key.current == previous) } + + fn maybe_broadcast_barriers_on_rotation( + _rotation_broadcast_id: cf_primitives::BroadcastId, + ) -> Vec { + // we dont need to put broadcast barriers for Bitcoin + vec![] + } } fn verify_single_threshold_signature( diff --git a/state-chain/chains/src/dot.rs b/state-chain/chains/src/dot.rs index 99f5bc0a50..5bb1bbac6c 100644 --- a/state-chain/chains/src/dot.rs +++ b/state-chain/chains/src/dot.rs @@ -308,15 +308,12 @@ impl ChainCrypto for PolkadotCrypto { EncodedPolkadotPayload(Blake2_256::hash(&agg_key.aliased_ref()[..]).to_vec()) } - /// Once authored, polkadot extrinsic must be signed by the key whose nonce was incorporated - /// into the extrinsic. - fn sign_with_specific_key() -> bool { - true - } - - /// We sign with a specific key, so we can optimistically activate the next one. - fn optimistic_activation() -> bool { - true + fn maybe_broadcast_barriers_on_rotation( + rotation_broadcast_id: BroadcastId, + ) -> Vec { + // For polkadot, we need to pause future epoch broadcasts until all the previous epoch + // broadcasts (including the rotation tx) has successfully broadcasted. + vec![rotation_broadcast_id] } } diff --git a/state-chain/chains/src/evm.rs b/state-chain/chains/src/evm.rs index ce24121164..05489be01a 100644 --- a/state-chain/chains/src/evm.rs +++ b/state-chain/chains/src/evm.rs @@ -49,6 +49,23 @@ impl ChainCrypto for EvmCrypto { fn agg_key_to_payload(agg_key: Self::AggKey, _for_handover: bool) -> Self::Payload { H256(Blake2_256::hash(&agg_key.to_pubkey_compressed())) } + + fn maybe_broadcast_barriers_on_rotation( + rotation_broadcast_id: BroadcastId, + ) -> Vec { + // For Ethereum, we need to put 2 barriers, the first on the last non-rotation tx of the + // previous epoch, the second on the rotation tx itself. This is because before we execute + // the rotation tx for eth, we need to make sure all previous tx have successfully + // broadcast. Also, we need to pause future new epoch tx from broadcast until the rotation + // broadcast has successfully completed. + // + // If the rotation tx is the first broadcast ever, we dont need the first barrier. + if rotation_broadcast_id > 1 { + vec![rotation_broadcast_id - 1, rotation_broadcast_id] + } else { + vec![rotation_broadcast_id] + } + } } #[derive(Copy, Clone, RuntimeDebug, PartialEq, Eq)] diff --git a/state-chain/chains/src/lib.rs b/state-chain/chains/src/lib.rs index 019f9a4c96..947c895be5 100644 --- a/state-chain/chains/src/lib.rs +++ b/state-chain/chains/src/lib.rs @@ -5,7 +5,7 @@ use core::{fmt::Display, iter::Step}; use crate::benchmarking_value::{BenchmarkValue, BenchmarkValueExtended}; pub use address::ForeignChainAddress; use address::{AddressDerivationApi, AddressDerivationError, ToHumanreadableAddress}; -use cf_primitives::{AssetAmount, ChannelId, EthAmount, TransactionHash}; +use cf_primitives::{AssetAmount, BroadcastId, ChannelId, EthAmount, TransactionHash}; use codec::{Decode, Encode, FullCodec, MaxEncodedLen}; use frame_support::{ pallet_prelude::{MaybeSerializeDeserialize, Member}, @@ -179,27 +179,17 @@ pub trait ChainCrypto { true } - /// Determines whether threshold signatures are made with a specific fixed key, or whether the - /// key is refreshed if we need to retry the signature. - /// - /// By default, this is true for Utxo-based chains, false otherwise. - fn sign_with_specific_key() -> bool { - Self::UtxoChain::get() - } - - /// Determines whether the chain crypto allows for optimistic activation of new aggregate keys. - /// - /// By default, this is true for Utxo-based chains, false otherwise. - fn optimistic_activation() -> bool { - Self::UtxoChain::get() - } - /// Determines whether the chain crypto supports key handover. /// /// By default, this is true for Utxo-based chains, false otherwise. fn key_handover_is_required() -> bool { Self::UtxoChain::get() } + + /// Provides chain specific functionality for providing the broadcast barriers on rotation tx + /// broadcast + fn maybe_broadcast_barriers_on_rotation(rotation_broadcast_id: BroadcastId) + -> Vec; } /// Provides chain-specific replay protection data. @@ -247,11 +237,9 @@ where fn refresh_unsigned_data(tx: &mut C::Transaction); /// Checks if the payload is still valid for the call. - fn is_valid_for_rebroadcast( + fn requires_signature_refresh( call: &Call, payload: &<::ChainCrypto as ChainCrypto>::Payload, - current_key: &<::ChainCrypto as ChainCrypto>::AggKey, - signature: &<::ChainCrypto as ChainCrypto>::ThresholdSignature, ) -> bool; /// Calculate the Units of gas that is allowed to make this call. diff --git a/state-chain/chains/src/mocks.rs b/state-chain/chains/src/mocks.rs index 1f5c9ac284..72aaaa0954 100644 --- a/state-chain/chains/src/mocks.rs +++ b/state-chain/chains/src/mocks.rs @@ -14,11 +14,17 @@ pub struct MockEthereum; pub type MockEthereumChannelId = u128; +#[derive(Clone)] +pub enum ChainChoice { + Ethereum, + Polkadot, + Bitcoin, +} + thread_local! { static MOCK_KEY_HANDOVER_IS_REQUIRED: RefCell = RefCell::new(true); - static MOCK_OPTIMISTIC_ACTIVATION: RefCell = RefCell::new(false); - static MOCK_SIGN_WITH_SPECIFIC_KEY: RefCell = RefCell::new(false); static MOCK_VALID_METADATA: RefCell = RefCell::new(true); + static MOCK_BROADCAST_BARRIERS: RefCell = RefCell::new(ChainChoice::Ethereum); } pub struct MockKeyHandoverIsRequired; @@ -35,31 +41,17 @@ impl Get for MockKeyHandoverIsRequired { } } -pub struct MockOptimisticActivation; - -impl MockOptimisticActivation { - pub fn set(value: bool) { - MOCK_OPTIMISTIC_ACTIVATION.with(|v| *v.borrow_mut() = value); - } -} - -impl Get for MockOptimisticActivation { - fn get() -> bool { - MOCK_OPTIMISTIC_ACTIVATION.with(|v| *v.borrow()) - } -} - -pub struct MockFixedKeySigningRequests; +pub struct MockBroadcastBarriers; -impl MockFixedKeySigningRequests { - pub fn set(value: bool) { - MOCK_SIGN_WITH_SPECIFIC_KEY.with(|v| *v.borrow_mut() = value); +impl MockBroadcastBarriers { + pub fn set(value: ChainChoice) { + MOCK_BROADCAST_BARRIERS.with(|v| *v.borrow_mut() = value); } } -impl Get for MockFixedKeySigningRequests { - fn get() -> bool { - MOCK_SIGN_WITH_SPECIFIC_KEY.with(|v| *v.borrow()) +impl Get for MockBroadcastBarriers { + fn get() -> ChainChoice { + MOCK_BROADCAST_BARRIERS.with(|v| (*v.borrow()).clone()) } } @@ -260,17 +252,24 @@ impl ChainCrypto for MockEthereumChainCrypto { new_key != &BAD_AGG_KEY_POST_HANDOVER } - fn sign_with_specific_key() -> bool { - MockFixedKeySigningRequests::get() - } - - fn optimistic_activation() -> bool { - MockOptimisticActivation::get() - } - fn key_handover_is_required() -> bool { MockKeyHandoverIsRequired::get() } + + fn maybe_broadcast_barriers_on_rotation( + rotation_broadcast_id: BroadcastId, + ) -> Vec { + match MockBroadcastBarriers::get() { + ChainChoice::Ethereum => + if rotation_broadcast_id > 1 { + vec![rotation_broadcast_id - 1, rotation_broadcast_id] + } else { + vec![rotation_broadcast_id] + }, + ChainChoice::Polkadot => vec![rotation_broadcast_id], + ChainChoice::Bitcoin => vec![], + } + } } impl_default_benchmark_value!(MockAggKey); @@ -334,14 +333,14 @@ impl ApiCall for MockApiCall { } thread_local! { - pub static IS_VALID_BROADCAST: std::cell::RefCell = RefCell::new(true); + pub static REQUIRES_REFRESH: std::cell::RefCell = RefCell::new(false); } pub struct MockTransactionBuilder(PhantomData<(C, Call)>); impl MockTransactionBuilder { - pub fn set_invalid_for_rebroadcast() { - IS_VALID_BROADCAST.with(|is_valid| *is_valid.borrow_mut() = false) + pub fn set_requires_refresh() { + REQUIRES_REFRESH.with(|is_valid| *is_valid.borrow_mut() = true) } } @@ -356,12 +355,10 @@ impl, Call: ApiCall> // refresh nothing } - fn is_valid_for_rebroadcast( + fn requires_signature_refresh( _call: &Call, _payload: &<::ChainCrypto as ChainCrypto>::Payload, - _current_key: &<::ChainCrypto as ChainCrypto>::AggKey, - _signature: &<::ChainCrypto as ChainCrypto>::ThresholdSignature, ) -> bool { - IS_VALID_BROADCAST.with(|is_valid| *is_valid.borrow()) + REQUIRES_REFRESH.with(|is_valid| *is_valid.borrow()) } } diff --git a/state-chain/chains/src/none.rs b/state-chain/chains/src/none.rs index 119a8b2ec8..98b9f2ea7b 100644 --- a/state-chain/chains/src/none.rs +++ b/state-chain/chains/src/none.rs @@ -53,4 +53,10 @@ impl ChainCrypto for NoneChainCrypto { fn agg_key_to_payload(_agg_key: Self::AggKey, _for_handover: bool) -> Self::Payload { unimplemented!() } + + fn maybe_broadcast_barriers_on_rotation( + _rotation_broadcast_id: BroadcastId, + ) -> Vec { + unimplemented!() + } } diff --git a/state-chain/pallets/cf-broadcast/Cargo.toml b/state-chain/pallets/cf-broadcast/Cargo.toml index 96a0bb3dca..eca2ea6423 100644 --- a/state-chain/pallets/cf-broadcast/Cargo.toml +++ b/state-chain/pallets/cf-broadcast/Cargo.toml @@ -18,6 +18,7 @@ cf-chains = { path = '../../chains', default-features = false } cf-primitives = { path = '../../primitives', default-features = false } cf-runtime-upgrade-utilities = { path = '../../runtime-upgrade-utilities', default-features = false } cf-traits = { path = '../../traits', default-features = false } +cf-runtime-utilities = { path = '../../runtime-utilities', default-features = false } log = { version = '0.4.16', default-features = false } @@ -46,6 +47,7 @@ std = [ 'cf-chains/std', 'cf-primitives/std', 'cf-runtime-upgrade-utilities/std', + 'cf-runtime-utilities/std', 'cf-traits/std', 'codec/std', 'frame-benchmarking?/std', diff --git a/state-chain/pallets/cf-broadcast/src/benchmarking.rs b/state-chain/pallets/cf-broadcast/src/benchmarking.rs index c6a9adbc3f..e72ce83dfa 100644 --- a/state-chain/pallets/cf-broadcast/src/benchmarking.rs +++ b/state-chain/pallets/cf-broadcast/src/benchmarking.rs @@ -49,7 +49,7 @@ fn generate_on_signature_ready_call, I>() -> pallet::Call::benchmark_value(), api_call: Box::new(ApiCallFor::::benchmark_value()), - broadcast_id: 1, + broadcast_attempt_id: BroadcastAttemptId { broadcast_id: 1, attempt_count: 0 }, initiated_at: INITIATED_AT.into(), should_broadcast: true, } @@ -68,7 +68,6 @@ benchmarks_instance_pallet! { ThresholdSignatureData::::insert(i, (ApiCallFor::::benchmark_value(), ThresholdSignatureFor::::benchmark_value())) } let valid_key = AggKeyFor::::benchmark_value(); - T::KeyProvider::set_key(valid_key); } : { Pallet::::on_initialize(timeout_block); } @@ -86,7 +85,6 @@ benchmarks_instance_pallet! { generate_on_signature_ready_call::().dispatch_bypass_filter(T::EnsureThresholdSigned::try_successful_origin().unwrap())?; let expiry_block = frame_system::Pallet::::block_number() + T::BroadcastTimeout::get(); let valid_key = AggKeyFor::::benchmark_value(); - T::KeyProvider::set_key(valid_key); }: _(RawOrigin::Signed(caller), broadcast_attempt_id) verify { assert!(Timeouts::::contains_key(expiry_block)); @@ -101,7 +99,6 @@ benchmarks_instance_pallet! { insert_transaction_broadcast_attempt::(whitelisted_caller(), broadcast_attempt_id); let call = generate_on_signature_ready_call::(); let valid_key = AggKeyFor::::benchmark_value(); - T::KeyProvider::set_key(valid_key); } : { call.dispatch_bypass_filter(T::EnsureThresholdSigned::try_successful_origin().unwrap())? } verify { assert_eq!(BroadcastIdCounter::::get(), 0); @@ -111,15 +108,14 @@ benchmarks_instance_pallet! { start_next_broadcast_attempt { let api_call = ApiCallFor::::benchmark_value(); let signed_api_call = api_call.signed(&BenchmarkValue::benchmark_value()); - let broadcast_attempt_id = Pallet::::start_broadcast( + let broadcast_id = as Broadcaster<_>>::threshold_sign_and_broadcast( BenchmarkValue::benchmark_value(), - signed_api_call, - BenchmarkValue::benchmark_value(), - 1, - INITIATED_AT.into(), ); + let broadcast_attempt_id = BroadcastAttemptId { + broadcast_id, + attempt_count: BroadcastAttemptCount::::get(broadcast_id), + }; - T::KeyProvider::set_key(AggKeyFor::::benchmark_value()); let transaction_payload = TransactionFor::::benchmark_value(); } : { @@ -131,7 +127,7 @@ benchmarks_instance_pallet! { }) } verify { - assert!(AwaitingBroadcast::::contains_key(broadcast_attempt_id.next_attempt())); + assert!(AwaitingBroadcast::::contains_key(broadcast_attempt_id.peek_next())); } transaction_succeeded { let caller: T::AccountId = whitelisted_caller(); @@ -151,7 +147,6 @@ benchmarks_instance_pallet! { tx_metadata: TransactionMetadataFor::::benchmark_value(), }; let valid_key = AggKeyFor::::benchmark_value(); - T::KeyProvider::set_key(valid_key); } : { call.dispatch_bypass_filter(T::EnsureWitnessedAtCurrentEpoch::try_successful_origin().unwrap())? } verify { // We expect the unwrap to error if the extrinsic didn't fire an event - if an event has been emitted we reached the end of the extrinsic diff --git a/state-chain/pallets/cf-broadcast/src/lib.rs b/state-chain/pallets/cf-broadcast/src/lib.rs index 5e779979a3..21084c9d4f 100644 --- a/state-chain/pallets/cf-broadcast/src/lib.rs +++ b/state-chain/pallets/cf-broadcast/src/lib.rs @@ -1,6 +1,8 @@ #![cfg_attr(not(feature = "std"), no_std)] #![doc = include_str!("../README.md")] #![doc = include_str!("../../cf-doc-head.md")] +#![feature(extract_if)] +#![feature(is_sorted)] mod benchmarking; mod mock; @@ -8,10 +10,26 @@ mod tests; pub mod migrations; pub mod weights; +use cf_chains::{ + ApiCall, Chain, ChainCrypto, FeeRefundCalculator, TransactionBuilder, TransactionMetadata as _, +}; use cf_primitives::{BroadcastId, ThresholdSignatureRequestId}; -use cf_traits::{GetBlockHeight, SafeMode}; -use frame_support::{traits::OriginTrait, RuntimeDebug}; -use sp_std::marker; +use cf_traits::{ + offence_reporting::OffenceReporter, BroadcastNomination, Broadcaster, Chainflip, EpochInfo, + GetBlockHeight, SafeMode, ThresholdSigner, +}; +use codec::{Decode, Encode, MaxEncodedLen}; +use frame_support::{ + dispatch::DispatchResultWithPostInfo, + pallet_prelude::DispatchResult, + sp_runtime::traits::Saturating, + traits::{Defensive, Get, OriginTrait, StorageVersion, UnfilteredDispatchable}, + RuntimeDebug, Twox64Concat, +}; +use frame_system::pallet_prelude::OriginFor; +pub use pallet::*; +use scale_info::TypeInfo; +use sp_std::{collections::btree_set::BTreeSet, marker, marker::PhantomData, prelude::*, vec::Vec}; pub use weights::WeightInfo; #[derive(Encode, Decode, MaxEncodedLen, TypeInfo, Copy, Clone, PartialEq, Eq, RuntimeDebug)] @@ -28,29 +46,6 @@ impl SafeMode for PalletSafeMode { const CODE_GREEN: Self = PalletSafeMode { retry_enabled: true, _phantom: marker::PhantomData }; } -use cf_chains::{ - ApiCall, Chain, ChainCrypto, FeeRefundCalculator, TransactionBuilder, TransactionMetadata as _, -}; -use cf_traits::{ - offence_reporting::OffenceReporter, BroadcastNomination, Broadcaster, Chainflip, EpochInfo, - EpochKey, OnBroadcastReady, ThresholdSigner, -}; -use codec::{Decode, Encode, MaxEncodedLen}; -use frame_support::{ - dispatch::DispatchResultWithPostInfo, - pallet_prelude::DispatchResult, - sp_runtime::traits::Saturating, - traits::{Get, StorageVersion, UnfilteredDispatchable}, - Twox64Concat, -}; - -use cf_traits::KeyProvider; - -use frame_system::pallet_prelude::OriginFor; -pub use pallet::*; -use scale_info::TypeInfo; -use sp_std::{marker::PhantomData, prelude::*}; - /// The number of broadcast attempts that were made before this one. pub type AttemptCount = u32; @@ -62,9 +57,23 @@ pub struct BroadcastAttemptId { } impl BroadcastAttemptId { - /// Increment the attempt count for a particular BroadcastAttemptId - pub fn next_attempt(&self) -> Self { - Self { attempt_count: self.attempt_count.wrapping_add(1), ..*self } + /// Get the next BroadcastAttemptId. + pub fn peek_next(&self) -> Self { + Self { attempt_count: self.attempt_count + 1, ..*self } + } + + /// Increment the attempt counter and return the next BroadcastAttemptId. + pub fn into_next, I: 'static>(self) -> Self { + Self { + attempt_count: BroadcastAttemptCount::::mutate( + self.broadcast_id, + |attempt_count: &mut AttemptCount| { + *attempt_count += 1; + *attempt_count + }, + ), + ..self + } } } @@ -89,8 +98,12 @@ pub const PALLET_VERSION: StorageVersion = StorageVersion::new(2); pub mod pallet { use super::*; use cf_chains::benchmarking_value::BenchmarkValue; - use cf_traits::{AccountRoleRegistry, BroadcastNomination, KeyProvider, OnBroadcastReady}; - use frame_support::{ensure, pallet_prelude::*, traits::EnsureOrigin}; + use cf_traits::{AccountRoleRegistry, BroadcastNomination, OnBroadcastReady}; + use frame_support::{ + ensure, + pallet_prelude::{OptionQuery, ValueQuery, *}, + traits::EnsureOrigin, + }; use frame_system::pallet_prelude::*; /// Type alias for the instance's configured Transaction. @@ -208,9 +221,6 @@ pub mod pallet { #[pallet::constant] type BroadcastTimeout: Get>; - /// Something that provides the current key for signing. - type KeyProvider: KeyProvider<::ChainCrypto>; - /// Safe Mode access. type SafeMode: Get>; @@ -311,6 +321,22 @@ pub mod pallet { pub type TransactionFeeDeficit, I: 'static = ()> = StorageMap<_, Twox64Concat, SignerIdFor, ChainAmountFor, ValueQuery>; + /// Whether or not broadcasts are paused for broadcast ids greater than the given broadcast id. + #[pallet::storage] + #[pallet::getter(fn broadcast_barriers)] + pub type BroadcastBarriers = StorageValue<_, BTreeSet, ValueQuery>; + + /// List of broadcasts that are initiated but not witnessed on the external chain. + #[pallet::storage] + #[pallet::getter(fn pending_broadcasts)] + pub type PendingBroadcasts = StorageValue<_, BTreeSet, ValueQuery>; + + /// List of broadcasts that have been aborted because they were unsuccessful to be broadcast + /// after many retries. + #[pallet::storage] + #[pallet::getter(fn aborted_broadcasts)] + pub type AbortedBroadcasts = StorageValue<_, Vec, ValueQuery>; + #[pallet::event] #[pallet::generate_deposit(pub(super) fn deposit_event)] pub enum Event, I: 'static = ()> { @@ -334,7 +360,7 @@ pub mod pallet { transaction_out_id: TransactionOutIdFor, }, /// A broadcast's threshold signature is invalid, we will attempt to re-sign it. - ThresholdSignatureInvalid { broadcast_id: BroadcastId, retry_broadcast_id: BroadcastId }, + ThresholdSignatureInvalid { broadcast_attempt_id: BroadcastAttemptId }, /// A signature accepted event on the target chain has been witnessed and the callback was /// executed. BroadcastCallbackExecuted { broadcast_id: BroadcastId, result: DispatchResult }, @@ -379,11 +405,14 @@ pub mod pallet { let expiries = Timeouts::::take(block_number); if T::SafeMode::get().retry_enabled { for attempt_id in expiries.iter() { - if let Some(attempt) = Self::take_awaiting_broadcast(*attempt_id) { + if PendingBroadcasts::::get().contains(&attempt_id.broadcast_id) { Self::deposit_event(Event::::BroadcastAttemptTimeout { broadcast_attempt_id: *attempt_id, }); - Self::start_next_broadcast_attempt(attempt); + if let Some(broadcast_attempt) = Self::take_awaiting_broadcast(*attempt_id) + { + Self::start_next_broadcast_attempt(broadcast_attempt); + } } } } else { @@ -406,16 +435,26 @@ pub mod pallet { .expect("start_next_broadcast_attempt weight should not be 0") as usize; - let mut retries = BroadcastRetryQueue::::take(); - - if retries.len() > num_retries_that_fit { - BroadcastRetryQueue::::put(retries.split_off(num_retries_that_fit)); - } + let retries = BroadcastRetryQueue::::mutate(|retry_queue| { + let id_limit = BroadcastBarriers::::get() + .first() + .copied() + .unwrap_or(BroadcastId::max_value()); + retry_queue + .extract_if(|broadcast| { + broadcast.broadcast_attempt_id.broadcast_id <= id_limit + }) + .take(num_retries_that_fit) + .collect::>() + }); let retries_len = retries.len(); for retry in retries { - if Self::take_awaiting_broadcast(retry.broadcast_attempt_id).is_some() { + // Check if the broadcast is pending + if PendingBroadcasts::::get() + .contains(&retry.broadcast_attempt_id.broadcast_id) + { Self::start_next_broadcast_attempt(retry); } } @@ -461,7 +500,7 @@ pub mod pallet { // Schedule a failed attempt for retry when the next block is authored. // We will abort the broadcast once all authorities have attempt to sign the // transaction - if signing_attempt.broadcast_attempt.broadcast_attempt_id.attempt_count == + if signing_attempt.broadcast_attempt.broadcast_attempt_id.attempt_count >= T::EpochInfo::current_authority_count() .checked_sub(1) .expect("We must have at least one authority") @@ -497,6 +536,8 @@ pub mod pallet { .broadcast_attempt_id .broadcast_id, }); + Self::remove_pending_broadcast(&broadcast_attempt_id.broadcast_id); + AbortedBroadcasts::::append(broadcast_attempt_id.broadcast_id); } else { Self::schedule_for_retry(&signing_attempt.broadcast_attempt); } @@ -511,7 +552,7 @@ pub mod pallet { /// /// ## Events /// - /// - If `should_broadcast` see [Call::start_broadcast] + /// - [Event::CallResigned] If the call was re-signed. /// /// /// ## Errors @@ -524,7 +565,7 @@ pub mod pallet { threshold_request_id: ThresholdSignatureRequestId, threshold_signature_payload: PayloadFor, api_call: Box<>::ApiCall>, - broadcast_id: BroadcastId, + broadcast_attempt_id: BroadcastAttemptId, initiated_at: ChainBlockNumberFor, should_broadcast: bool, ) -> DispatchResultWithPostInfo { @@ -542,22 +583,42 @@ pub mod pallet { .expect("signature can not be unavailable"); let signed_api_call = api_call.signed(&signature); + ThresholdSignatureData::::insert( - broadcast_id, + broadcast_attempt_id.broadcast_id, (signed_api_call.clone(), signature), ); // If a signed call already exists, update the storage and do not broadcast. if should_broadcast { - Self::start_broadcast( - T::TransactionBuilder::build_transaction(&signed_api_call), - signed_api_call, - threshold_signature_payload, - broadcast_id, - initiated_at, + let transaction_out_id = signed_api_call.transaction_out_id(); + + T::BroadcastReadyProvider::on_broadcast_ready(&signed_api_call); + + // The Engine uses this. + TransactionOutIdToBroadcastId::::insert( + &transaction_out_id, + (broadcast_attempt_id.broadcast_id, initiated_at), ); + + let broadcast_attempt = BroadcastAttempt:: { + broadcast_attempt_id, + transaction_payload: T::TransactionBuilder::build_transaction(&signed_api_call), + threshold_signature_payload, + transaction_out_id, + }; + + if BroadcastBarriers::::get().first().is_some_and(|broadcast_barrier_id| { + broadcast_attempt_id.broadcast_id > *broadcast_barrier_id + }) { + Self::schedule_for_retry(&broadcast_attempt); + } else { + Self::start_broadcast_attempt(broadcast_attempt); + } } else { - Self::deposit_event(Event::::CallResigned { broadcast_id }); + Self::deposit_event(Event::::CallResigned { + broadcast_id: broadcast_attempt_id.broadcast_id, + }); } Ok(().into()) @@ -592,25 +653,34 @@ pub mod pallet { TransactionOutIdToBroadcastId::::take(&tx_out_id) .ok_or(Error::::InvalidPayload)?; + Self::remove_pending_broadcast(&broadcast_id); + if let Some(expected_tx_metadata) = TransactionMetadata::::take(broadcast_id) { if tx_metadata.verify_metadata(&expected_tx_metadata) { - let to_refund = AwaitingBroadcast::::get(BroadcastAttemptId { - broadcast_id, - attempt_count: BroadcastAttemptCount::::get(broadcast_id), - }) - .ok_or(Error::::InvalidBroadcastAttemptId)? - .broadcast_attempt - .transaction_payload - .return_fee_refund(tx_fee); - - TransactionFeeDeficit::::mutate(signer_id.clone(), |fee_deficit| { - *fee_deficit = fee_deficit.saturating_add(to_refund); - }); + if let Some(signing_attempt) = + AwaitingBroadcast::::get(BroadcastAttemptId { + broadcast_id, + attempt_count: BroadcastAttemptCount::::get(broadcast_id), + }) { + let to_refund = signing_attempt + .broadcast_attempt + .transaction_payload + .return_fee_refund(tx_fee); + + TransactionFeeDeficit::::mutate(signer_id.clone(), |fee_deficit| { + *fee_deficit = fee_deficit.saturating_add(to_refund); + }); - Self::deposit_event(Event::::TransactionFeeDeficitRecorded { - beneficiary: signer_id, - amount: to_refund, - }); + Self::deposit_event(Event::::TransactionFeeDeficitRecorded { + beneficiary: signer_id, + amount: to_refund, + }); + } else { + log::warn!( + "Unable to attribute transaction fee refund for broadcast {}.", + broadcast_id + ); + } } else { Self::deposit_event(Event::::TransactionFeeDeficitRefused { beneficiary: signer_id, @@ -678,26 +748,16 @@ pub mod pallet { impl, I: 'static> Pallet { pub fn clean_up_broadcast_storage(broadcast_id: BroadcastId) { - let first_attempt = AttemptCount::default(); - - if let Some(transaction_signing_attempt) = - AwaitingBroadcast::::get(BroadcastAttemptId { - broadcast_id, - attempt_count: first_attempt, - }) { - TransactionOutIdToBroadcastId::::remove( - transaction_signing_attempt.broadcast_attempt.transaction_out_id, - ); - }; - - for attempt_count in first_attempt..=(BroadcastAttemptCount::::take(broadcast_id)) { + for attempt_count in 0..=(BroadcastAttemptCount::::take(broadcast_id)) { AwaitingBroadcast::::remove(BroadcastAttemptId { broadcast_id, attempt_count }); } TransactionMetadata::::remove(broadcast_id); RequestSuccessCallbacks::::remove(broadcast_id); RequestFailureCallbacks::::remove(broadcast_id); - ThresholdSignatureData::::remove(broadcast_id); + if let Some((api_call, _)) = ThresholdSignatureData::::take(broadcast_id) { + TransactionOutIdToBroadcastId::::remove(api_call.transaction_out_id()); + } } pub fn take_awaiting_broadcast( @@ -715,6 +775,23 @@ impl, I: 'static> Pallet { } } + pub fn remove_pending_broadcast(broadcast_id: &BroadcastId) { + PendingBroadcasts::::mutate(|pending_broadcasts| { + if !pending_broadcasts.remove(broadcast_id) { + cf_runtime_utilities::log_or_panic!( + "The broadcast_id should exist in the pending broadcasts list since we added it to the list when the broadcast was initated" + ); + } + if let Some(broadcast_barrier_id) = BroadcastBarriers::::get().first() { + if pending_broadcasts.first().map_or(true, |id| *id > *broadcast_barrier_id) { + BroadcastBarriers::::mutate(|broadcast_barriers| { + broadcast_barriers.pop_first(); + }); + } + } + }); + } + /// Request a threshold signature, providing [Call::on_signature_ready] as the callback. pub fn threshold_sign_and_broadcast( api_call: >::ApiCall, @@ -722,11 +799,14 @@ impl, I: 'static> Pallet { maybe_failed_callback_generator: impl FnOnce( BroadcastId, ) -> Option<>::BroadcastCallable>, - ) -> (BroadcastId, ThresholdSignatureRequestId) { + ) -> BroadcastId { let broadcast_id = BroadcastIdCounter::::mutate(|id| { *id += 1; *id }); + + PendingBroadcasts::::append(broadcast_id); + if let Some(callback) = maybe_success_callback { RequestSuccessCallbacks::::insert(broadcast_id, callback); } @@ -734,7 +814,9 @@ impl, I: 'static> Pallet { RequestFailureCallbacks::::insert(broadcast_id, callback); } - (broadcast_id, Self::threshold_sign(api_call, broadcast_id, true)) + let _threshold_signature_id = Self::threshold_sign(api_call, broadcast_id, true); + + broadcast_id } /// Signs a API call, use `Call::on_signature_ready` as the callback, and returns the signature @@ -758,7 +840,7 @@ impl, I: 'static> Pallet { threshold_request_id, threshold_signature_payload, api_call: Box::new(api_call), - broadcast_id, + broadcast_attempt_id: BroadcastAttemptId { broadcast_id, attempt_count: 0 }, initiated_at, should_broadcast, } @@ -767,85 +849,51 @@ impl, I: 'static> Pallet { ) } - /// Begin the process of broadcasting a transaction. - /// - /// ## Events - /// - /// - [TransactionBroadcastRequest](Event::TransactionBroadcastRequest) - fn start_broadcast( - transaction_payload: TransactionFor, - api_call: >::ApiCall, - threshold_signature_payload: <::ChainCrypto as ChainCrypto>::Payload, - broadcast_id: BroadcastId, - initiated_at: ChainBlockNumberFor, - ) -> BroadcastAttemptId { - let transaction_out_id = api_call.transaction_out_id(); - - T::BroadcastReadyProvider::on_broadcast_ready(&api_call); - - TransactionOutIdToBroadcastId::::insert( - &transaction_out_id, - (broadcast_id, initiated_at), - ); - - let broadcast_attempt_id = BroadcastAttemptId { broadcast_id, attempt_count: 0 }; - Self::start_broadcast_attempt(BroadcastAttempt:: { - broadcast_attempt_id, - transaction_payload, - threshold_signature_payload, - transaction_out_id, - }); - broadcast_attempt_id - } - fn start_next_broadcast_attempt(broadcast_attempt: BroadcastAttempt) { let broadcast_id = broadcast_attempt.broadcast_attempt_id.broadcast_id; - if let Some((api_call, signature)) = ThresholdSignatureData::::get(broadcast_id) { - let EpochKey { key, .. } = T::KeyProvider::active_epoch_key() - .expect("Epoch key must exist if we made a broadcast."); + if let Some((api_call, _signature)) = ThresholdSignatureData::::get(broadcast_id) { + let next_broadcast_attempt_id = + broadcast_attempt.broadcast_attempt_id.into_next::(); - if T::TransactionBuilder::is_valid_for_rebroadcast( + if T::TransactionBuilder::requires_signature_refresh( &api_call, &broadcast_attempt.threshold_signature_payload, - &key, - &signature, ) { - let next_broadcast_attempt_id = - broadcast_attempt.broadcast_attempt_id.next_attempt(); + // We update the initiated_at here since as the tx is resigned and broadcast, it is + // not possible for it to be successfully broadcasted before this point. + // This `initiated_at` block will be associated with the new transaction_out_id + // so should not interfere with witnessing the previous one. + let initiated_at = T::ChainTracking::get_block_height(); - BroadcastAttemptCount::::mutate( - broadcast_id, - |attempt_count: &mut AttemptCount| { - *attempt_count += 1; + Self::deposit_event(Event::::ThresholdSignatureInvalid { + broadcast_attempt_id: broadcast_attempt.broadcast_attempt_id, + }); + + let threshold_signature_payload = api_call.threshold_signature_payload(); + T::ThresholdSigner::request_signature_with_callback( + threshold_signature_payload.clone(), + |threshold_request_id| { + Call::on_signature_ready { + threshold_request_id, + threshold_signature_payload, + api_call: Box::new(api_call), + broadcast_attempt_id: next_broadcast_attempt_id, + initiated_at, + should_broadcast: true, + } + .into() }, ); - debug_assert_eq!( - BroadcastAttemptCount::::get(broadcast_id), - next_broadcast_attempt_id.attempt_count, - ); - Self::start_broadcast_attempt(BroadcastAttempt:: { - broadcast_attempt_id: next_broadcast_attempt_id, - ..broadcast_attempt - }); - } - // If the signature verification fails, we want - // to retry from the threshold signing stage. - else { - Self::clean_up_broadcast_storage(broadcast_id); - let (retry_broadcast_id, _) = Self::threshold_sign_and_broadcast( - api_call, - RequestSuccessCallbacks::::get(broadcast_id), - |_broadcast_id| RequestFailureCallbacks::::get(broadcast_id), - ); log::info!( "Signature is invalid -> rescheduled threshold signature for broadcast id {}.", broadcast_id ); - Self::deposit_event(Event::::ThresholdSignatureInvalid { - broadcast_id, - retry_broadcast_id, + } else { + Self::start_broadcast_attempt(BroadcastAttempt:: { + broadcast_attempt_id: next_broadcast_attempt_id, + ..broadcast_attempt }); } } else { @@ -915,9 +963,7 @@ impl, I: 'static> Broadcaster for Pallet { type ApiCall = T::ApiCall; type Callback = >::BroadcastCallable; - fn threshold_sign_and_broadcast( - api_call: Self::ApiCall, - ) -> (BroadcastId, ThresholdSignatureRequestId) { + fn threshold_sign_and_broadcast(api_call: Self::ApiCall) -> BroadcastId { Self::threshold_sign_and_broadcast(api_call, None, |_| None) } @@ -925,20 +971,33 @@ impl, I: 'static> Broadcaster for Pallet { api_call: Self::ApiCall, success_callback: Option, failed_callback_generator: impl FnOnce(BroadcastId) -> Option, - ) -> (BroadcastId, ThresholdSignatureRequestId) { + ) -> BroadcastId { Self::threshold_sign_and_broadcast(api_call, success_callback, failed_callback_generator) } fn threshold_resign(broadcast_id: BroadcastId) -> Option { - if let Some((api_call, _signature)) = ThresholdSignatureData::::get(broadcast_id) { - Some(Self::threshold_sign(api_call, broadcast_id, false)) - } else { - None - } + ThresholdSignatureData::::get(broadcast_id) + .map(|(api_call, _signature)| Self::threshold_sign(api_call, broadcast_id, false)) } /// Clean up storage data related to a broadcast ID. fn clean_up_broadcast_storage(broadcast_id: BroadcastId) { Self::clean_up_broadcast_storage(broadcast_id); } + + fn threshold_sign_and_broadcast_rotation_tx(api_call: Self::ApiCall) -> BroadcastId { + let broadcast_id = >::threshold_sign_and_broadcast(api_call); + + if let Some(earliest_pending_broadcast_id) = PendingBroadcasts::::get() + .first() + .defensive_proof("Broadcast ID was just inserted, so at least this one must exist.") + { + for barrier in <<>::TargetChain as Chain>::ChainCrypto as ChainCrypto>::maybe_broadcast_barriers_on_rotation(broadcast_id) { + if barrier >= *earliest_pending_broadcast_id { + BroadcastBarriers::::append(barrier); + } + } + } + broadcast_id + } } diff --git a/state-chain/pallets/cf-broadcast/src/migrations.rs b/state-chain/pallets/cf-broadcast/src/migrations.rs index 3eaa166e9b..ca1b007b32 100644 --- a/state-chain/pallets/cf-broadcast/src/migrations.rs +++ b/state-chain/pallets/cf-broadcast/src/migrations.rs @@ -2,5 +2,6 @@ pub mod v2; use cf_runtime_upgrade_utilities::VersionedMigration; -pub type PalletMigration = - (VersionedMigration, v2::Migration, 1, 2>,); +use crate::Pallet; + +pub type PalletMigration = (VersionedMigration, v2::Migration, 1, 2>,); diff --git a/state-chain/pallets/cf-broadcast/src/migrations/v2.rs b/state-chain/pallets/cf-broadcast/src/migrations/v2.rs index 22c77cfc8b..2c0fb08817 100644 --- a/state-chain/pallets/cf-broadcast/src/migrations/v2.rs +++ b/state-chain/pallets/cf-broadcast/src/migrations/v2.rs @@ -1,47 +1,33 @@ use crate::*; -use frame_support::{ - migration, pallet_prelude::Weight, traits::OnRuntimeUpgrade, StoragePrefixedMap, -}; +#[cfg(feature = "try-runtime")] +use frame_support::sp_runtime::DispatchError; +use frame_support::{traits::OnRuntimeUpgrade, weights::Weight}; use sp_std::marker::PhantomData; - -mod old { - use super::*; - use frame_support::{pallet_prelude::OptionQuery, Twox64Concat}; - - #[frame_support::storage_alias] - pub type RequestCallbacks, I: 'static> = StorageMap< - Pallet, - Twox64Concat, - BroadcastId, - >::BroadcastCallable, - OptionQuery, - >; -} +#[cfg(feature = "try-runtime")] +use sp_std::prelude::Vec; pub struct Migration, I: 'static>(PhantomData<(T, I)>); impl, I: 'static> OnRuntimeUpgrade for Migration { fn on_runtime_upgrade() -> frame_support::weights::Weight { - migration::move_prefix( - old::RequestCallbacks::::storage_prefix(), - RequestSuccessCallbacks::::storage_prefix(), - ); + let pending_broadcasts = AwaitingBroadcast::::iter_keys() + .map(|BroadcastAttemptId { broadcast_id, .. }| broadcast_id) + .collect::>(); + PendingBroadcasts::::put(pending_broadcasts); Weight::zero() } #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result, frame_support::sp_runtime::TryRuntimeError> { - let count = old::RequestCallbacks::::iter().count() as u32; - Ok(count.encode()) + fn pre_upgrade() -> Result, DispatchError> { + Ok(Default::default()) } #[cfg(feature = "try-runtime")] - fn post_upgrade(state: Vec) -> Result<(), frame_support::sp_runtime::TryRuntimeError> { - let old_count = u32::decode(&mut &*state).expect("Invalid data passed from pre_upgrade"); - frame_support::ensure!( - old_count == RequestSuccessCallbacks::::iter().count() as u32, - "Count mismatch" - ); + fn post_upgrade(_state: Vec) -> Result<(), DispatchError> { + let pending_broadcasts = PendingBroadcasts::::get(); + for id in AwaitingBroadcast::::iter_keys() { + assert!(pending_broadcasts.contains(&id.broadcast_id),); + } Ok(()) } } diff --git a/state-chain/pallets/cf-broadcast/src/mock.rs b/state-chain/pallets/cf-broadcast/src/mock.rs index 0e0d05c0cf..0202e34ad5 100644 --- a/state-chain/pallets/cf-broadcast/src/mock.rs +++ b/state-chain/pallets/cf-broadcast/src/mock.rs @@ -6,9 +6,7 @@ use crate::{self as pallet_cf_broadcast, Instance1, PalletOffence, PalletSafeMod use cf_chains::{ eth::Ethereum, evm::EvmCrypto, - mocks::{ - MockAggKey, MockApiCall, MockEthereum, MockEthereumChainCrypto, MockTransactionBuilder, - }, + mocks::{MockApiCall, MockEthereum, MockEthereumChainCrypto, MockTransactionBuilder}, Chain, ChainCrypto, }; use cf_traits::{ @@ -17,7 +15,7 @@ use cf_traits::{ block_height_provider::BlockHeightProvider, signer_nomination::MockNominator, threshold_signer::MockThresholdSigner, }, - AccountRoleRegistry, EpochKey, KeyState, OnBroadcastReady, + AccountRoleRegistry, OnBroadcastReady, }; use codec::{Decode, Encode}; use frame_support::{parameter_types, traits::UnfilteredDispatchable}; @@ -79,10 +77,6 @@ parameter_types! { pub type MockOffenceReporter = cf_traits::mocks::offence_reporting::MockOffenceReporter; -pub const VALID_AGG_KEY: MockAggKey = MockAggKey([0, 0, 0, 0]); - -pub const INVALID_AGG_KEY: MockAggKey = MockAggKey([1, 1, 1, 1]); - thread_local! { pub static SIGNATURE_REQUESTS: RefCell::ChainCrypto as ChainCrypto>::Payload>> = RefCell::new(vec![]); pub static CALLBACK_CALLED: RefCell = RefCell::new(false); @@ -91,18 +85,6 @@ thread_local! { pub type EthMockThresholdSigner = MockThresholdSigner; -pub struct MockKeyProvider; - -impl cf_traits::KeyProvider for MockKeyProvider { - fn active_epoch_key() -> Option::AggKey>> { - Some(EpochKey { - key: if VALIDKEY.with(|cell| *cell.borrow()) { VALID_AGG_KEY } else { INVALID_AGG_KEY }, - epoch_index: Default::default(), - key_state: KeyState::Unlocked, - }) - } -} - #[derive(Debug, Clone, Default, PartialEq, Eq, Encode, Decode, TypeInfo)] pub struct MockCallback; @@ -124,12 +106,6 @@ impl UnfilteredDispatchable for MockCallback { } } -impl MockKeyProvider { - pub fn set_valid(valid: bool) { - VALIDKEY.with(|cell| *cell.borrow_mut() = valid); - } -} - pub struct MockBroadcastReadyProvider; impl OnBroadcastReady for MockBroadcastReadyProvider { type ApiCall = MockApiCall; @@ -150,7 +126,6 @@ impl pallet_cf_broadcast::Config for Test { type EnsureThresholdSigned = NeverFailingOriginCheck; type BroadcastTimeout = BroadcastTimeout; type WeightInfo = (); - type KeyProvider = MockKeyProvider; type RuntimeOrigin = RuntimeOrigin; type BroadcastCallable = MockCallback; type SafeMode = MockRuntimeSafeMode; @@ -164,7 +139,7 @@ cf_test_utilities::impl_test_helpers! { Test, RuntimeGenesisConfig::default(), || { - MockEpochInfo::next_epoch((0..3).collect()); + MockEpochInfo::next_epoch((0..4).collect()); MockNominator::use_current_authorities_as_nominees::(); for id in &MockEpochInfo::current_authorities() { >::register_as_validator(id).unwrap(); diff --git a/state-chain/pallets/cf-broadcast/src/tests.rs b/state-chain/pallets/cf-broadcast/src/tests.rs index 64b0436e25..4717ff541d 100644 --- a/state-chain/pallets/cf-broadcast/src/tests.rs +++ b/state-chain/pallets/cf-broadcast/src/tests.rs @@ -2,7 +2,7 @@ use crate::{ mock::*, AwaitingBroadcast, BroadcastAttempt, BroadcastAttemptCount, BroadcastAttemptId, - BroadcastId, BroadcastRetryQueue, Error, Event as BroadcastEvent, FailedBroadcasters, + BroadcastId, BroadcastRetryQueue, Config, Error, Event as BroadcastEvent, FailedBroadcasters, Instance1, PalletOffence, RequestFailureCallbacks, RequestSuccessCallbacks, ThresholdSignatureData, Timeouts, TransactionFeeDeficit, TransactionMetadata, TransactionOutIdToBroadcastId, TransactionSigningAttempt, WeightInfo, @@ -10,14 +10,16 @@ use crate::{ use cf_chains::{ evm::SchnorrVerificationComponents, mocks::{ - MockApiCall, MockEthereum, MockEthereumChainCrypto, MockEthereumTransactionMetadata, - MockTransaction, MockTransactionBuilder, ETH_TX_FEE, MOCK_TX_METADATA, + ChainChoice, MockApiCall, MockBroadcastBarriers, MockEthereum, MockEthereumChainCrypto, + MockEthereumTransactionMetadata, MockTransactionBuilder, ETH_TX_FEE, + MOCK_TRANSACTION_OUT_ID, MOCK_TX_METADATA, }, ChainCrypto, FeeRefundCalculator, }; use cf_traits::{ mocks::{signer_nomination::MockNominator, threshold_signer::MockThresholdSigner}, - AsyncResult, Chainflip, EpochInfo, SetSafeMode, ThresholdSigner, + AsyncResult, Broadcaster as BroadcasterTrait, Chainflip, EpochInfo, SetSafeMode, + ThresholdSigner, }; use frame_support::{ assert_noop, assert_ok, @@ -41,8 +43,6 @@ thread_local! { // When calling on_idle, we should broadcast everything with this excess weight. const LARGE_EXCESS_WEIGHT: Weight = Weight::from_parts(20_000_000_000, 0); -const MOCK_TRANSACTION_OUT_ID: [u8; 4] = [0xbc; 4]; - struct MockCfe; impl MockCfe { @@ -126,33 +126,25 @@ fn assert_broadcast_storage_cleaned_up(broadcast_id: BroadcastId) { } fn start_mock_broadcast_tx_out_id( - tx_out_id: ::TransactionOutId, -) -> BroadcastAttemptId { - let api_call = - MockApiCall { tx_out_id, payload: Default::default(), sig: Some(Default::default()) }; - let broadcast_id = 1; - // Insert threshold signature into storage. - ThresholdSignatureData::::insert( - broadcast_id, - (api_call.clone(), api_call.sig.unwrap()), - ); - Broadcaster::start_broadcast( - MockTransaction, - api_call, - Default::default(), - broadcast_id, - 100u64, - ) + i: u8, +) -> (BroadcastAttemptId, ::TransactionOutId) { + let (tx_out_id, apicall) = api_call(i); + let broadcast_id = initiate_and_sign_broadcast(&apicall, TxType::Normal); + (BroadcastAttemptId { broadcast_id, attempt_count: 0 }, tx_out_id) } fn start_mock_broadcast() -> BroadcastAttemptId { - start_mock_broadcast_tx_out_id(Default::default()) + start_mock_broadcast_tx_out_id(Default::default()).0 } #[test] fn transaction_succeeded_results_in_refund_for_signer() { new_test_ext().execute_with(|| { - let broadcast_attempt_id = start_mock_broadcast_tx_out_id(MOCK_TRANSACTION_OUT_ID); + let (tx_out_id, apicall) = api_call(1); + let broadcast_id = initiate_and_sign_broadcast(&apicall, TxType::Normal); + + let broadcast_attempt_id = BroadcastAttemptId { broadcast_id, attempt_count: 0 }; + let tx_sig_request = AwaitingBroadcast::::get(broadcast_attempt_id).unwrap(); @@ -160,13 +152,7 @@ fn transaction_succeeded_results_in_refund_for_signer() { assert_eq!(TransactionFeeDeficit::::get(nominee), 0); - assert_ok!(Broadcaster::transaction_succeeded( - RuntimeOrigin::root(), - MOCK_TRANSACTION_OUT_ID, - nominee, - ETH_TX_FEE, - MOCK_TX_METADATA, - )); + witness_broadcast(tx_out_id); let expected_refund = tx_sig_request .broadcast_attempt @@ -185,22 +171,23 @@ fn transaction_succeeded_results_in_refund_for_signer() { }) ); - assert_broadcast_storage_cleaned_up(broadcast_attempt_id.broadcast_id); + assert_broadcast_storage_cleaned_up(broadcast_id); }); } #[test] fn test_abort_after_number_of_attempts_is_equal_to_the_number_of_authorities() { new_test_ext().execute_with(|| { - let broadcast_attempt_id = start_mock_broadcast(); + let (_tx_out_id, apicall) = api_call(1); + let broadcast_id = initiate_and_sign_broadcast(&apicall, TxType::Normal); for i in 0..MockEpochInfo::current_authority_count() { // Nominated signer responds that they can't sign the transaction. // retry should kick off at end of block if sufficient block space is free. assert_eq!( - BroadcastAttemptCount::::get(broadcast_attempt_id.broadcast_id), - broadcast_attempt_id.attempt_count + i, - "Failed for {broadcast_attempt_id:?} at iteration {i}" + BroadcastAttemptCount::::get(broadcast_id), + i, + "Failed for {broadcast_id:?} at iteration {i}" ); MockCfe::respond(Scenario::SigningFailure); Broadcaster::on_idle(0, LARGE_EXCESS_WEIGHT); @@ -208,13 +195,65 @@ fn test_abort_after_number_of_attempts_is_equal_to_the_number_of_authorities() { assert_eq!( System::events().pop().expect("an event").event, - RuntimeEvent::Broadcaster(crate::Event::BroadcastAborted { - broadcast_id: broadcast_attempt_id.broadcast_id - }) + RuntimeEvent::Broadcaster(crate::Event::BroadcastAborted { broadcast_id }) ); }); } +#[test] +fn broadcasts_aborted_after_all_report_failures_after_retry() { + new_test_ext() + .execute_with(|| { + let (_tx_out_id1, api_call1) = api_call(1); + + // Mock when all the possible broadcasts have failed another broadcast, and are + // therefore suspended. + MockNominator::set_nominees(Some(Default::default())); + + let broadcast_id = initiate_and_sign_broadcast(&api_call1, TxType::Normal); + + // No nominees, so we need to reschedule + System::assert_last_event(RuntimeEvent::Broadcaster( + crate::Event::::BroadcastRetryScheduled { + broadcast_attempt_id: BroadcastAttemptId { broadcast_id, attempt_count: 0 }, + }, + )); + broadcast_id + }) + // schedule some retries within each block - these do not result in + // TransactionBroadcastRequests + .then_execute_at_next_block(|broadcast_id| broadcast_id) + .then_execute_at_next_block(|broadcast_id| broadcast_id) + .then_execute_at_next_block(|broadcast_id| { + // The nominees are no longer suspended, so the retry in on_idle will nominate a + // broadcaster + MockNominator::reset_last_nominee(); + MockNominator::use_current_authorities_as_nominees::(); + broadcast_id + }) + .then_execute_at_next_block(|broadcast_id| { + // we should have attempt 3 here now. + assert_ok!(Broadcaster::transaction_signing_failure( + RawOrigin::Signed(0).into(), + BroadcastAttemptId { broadcast_id, attempt_count: 3 }, + )); + assert_eq!( + System::events().pop().expect("an event").event, + RuntimeEvent::Broadcaster(crate::Event::BroadcastAborted { broadcast_id }) + ); + broadcast_id + }) + .then_execute_at_next_block(|broadcast_id| { + assert_noop!( + Broadcaster::transaction_signing_failure( + RawOrigin::Signed(1).into(), + BroadcastAttemptId { broadcast_id, attempt_count: 4 }, + ), + Error::::InvalidBroadcastAttemptId + ); + }); +} + #[test] fn on_idle_caps_broadcasts_when_not_enough_weight() { new_test_ext().execute_with(|| { @@ -236,8 +275,9 @@ fn on_idle_caps_broadcasts_when_not_enough_weight() { Broadcaster::on_initialize(0); // only the first one should have retried, incremented attempt count - assert!(AwaitingBroadcast::::get(broadcast_attempt_id.next_attempt()) - .is_some()); + assert!( + AwaitingBroadcast::::get(broadcast_attempt_id.peek_next()).is_some() + ); // the other should be still in the retry queue let retry_queue = BroadcastRetryQueue::::get(); assert_eq!(retry_queue.len(), 1); @@ -272,8 +312,9 @@ fn test_transaction_signing_failed() { Broadcaster::on_idle(0, LARGE_EXCESS_WEIGHT); Broadcaster::on_initialize(0); - assert!(AwaitingBroadcast::::get(broadcast_attempt_id.next_attempt()) - .is_some()); + assert!( + AwaitingBroadcast::::get(broadcast_attempt_id.peek_next()).is_some() + ); }); } @@ -317,7 +358,8 @@ fn test_sigdata_with_no_match_is_noop() { #[test] fn transaction_succeeded_after_timeout_reports_failed_nodes() { new_test_ext().execute_with(|| { - start_mock_broadcast_tx_out_id(MOCK_TRANSACTION_OUT_ID); + let (tx_out_id, apicall) = api_call(1); + initiate_and_sign_broadcast(&apicall, TxType::Normal); let mut failed_authorities = vec![]; // The last node succeeds @@ -331,13 +373,7 @@ fn transaction_succeeded_after_timeout_reports_failed_nodes() { Broadcaster::on_initialize(0); } - assert_ok!(Broadcaster::transaction_succeeded( - RuntimeOrigin::root(), - MOCK_TRANSACTION_OUT_ID, - Default::default(), - ETH_TX_FEE, - MOCK_TX_METADATA, - )); + witness_broadcast(tx_out_id); MockOffenceReporter::assert_reported( PalletOffence::FailedToBroadcastTransaction, @@ -389,7 +425,7 @@ fn test_signature_request_expiry() { // New attempt is live with same broadcast_id and incremented attempt_count. assert!({ let new_attempt = - AwaitingBroadcast::::get(broadcast_attempt_id.next_attempt()) + AwaitingBroadcast::::get(broadcast_attempt_id.peek_next()) .unwrap(); new_attempt.broadcast_attempt.broadcast_attempt_id.attempt_count == 1 && new_attempt.broadcast_attempt.broadcast_attempt_id.broadcast_id == @@ -432,7 +468,7 @@ fn test_transmission_request_expiry() { // New attempt is live with same broadcast_id and incremented attempt_count. assert!({ let new_attempt = - AwaitingBroadcast::::get(broadcast_attempt_id.next_attempt()) + AwaitingBroadcast::::get(broadcast_attempt_id.peek_next()) .unwrap(); new_attempt.broadcast_attempt.broadcast_attempt_id.attempt_count == 1 && new_attempt.broadcast_attempt.broadcast_attempt_id.broadcast_id == @@ -449,48 +485,37 @@ fn test_transmission_request_expiry() { check_end_state(); }); } - -fn threshold_signature_rerequested(broadcast_attempt_id: BroadcastAttemptId) { - // Expect the original broadcast to be deleted - assert!(AwaitingBroadcast::::get(broadcast_attempt_id).is_none()); - // Verify storage has been deleted - assert!( - TransactionOutIdToBroadcastId::::get(MOCK_TRANSACTION_OUT_ID).is_none() - ); - assert_eq!(BroadcastAttemptCount::::get(broadcast_attempt_id.broadcast_id), 0); - assert!( - ThresholdSignatureData::::get(broadcast_attempt_id.broadcast_id).is_none() - ); - // Verify that we have a new signature request in the pipeline - assert_eq!( - MockThresholdSigner::::signature_result(0), - AsyncResult::Pending - ); -} - // One particular case where this occurs is if the Polkadot Runtime upgrade occurs after we've // already signed a tx. In this case we know it will continue to fail if we keep rebroadcasting so // we should stop and rethreshold sign using the new runtime version. #[test] fn re_request_threshold_signature_on_invalid_tx_params() { new_test_ext().execute_with(|| { - let broadcast_attempt_id = start_mock_broadcast(); + let (_, apicall) = api_call(1); + let broadcast_id = initiate_and_sign_broadcast(&apicall, TxType::Normal); + let broadcast_attempt_id = BroadcastAttemptId { broadcast_id, attempt_count: 0 }; assert_eq!( MockThresholdSigner::::signature_result(0), AsyncResult::Void ); assert!(AwaitingBroadcast::::get(broadcast_attempt_id).is_some()); - assert_eq!( - BroadcastAttemptCount::::get(broadcast_attempt_id.broadcast_id), - 0 - ); + assert_eq!(BroadcastAttemptCount::::get(broadcast_id), 0); - MockTransactionBuilder::::set_invalid_for_rebroadcast(); + MockTransactionBuilder::::set_requires_refresh(); // If invalid on retry then we should re-threshold sign Broadcaster::on_initialize(BROADCAST_EXPIRY_BLOCKS + 1); - threshold_signature_rerequested(broadcast_attempt_id); + // Verify storage has been deleted + assert!(TransactionOutIdToBroadcastId::::get(MOCK_TRANSACTION_OUT_ID) + .is_none()); + // attempt count incremented for the same broadcast_id + assert_eq!(BroadcastAttemptCount::::get(broadcast_id), 1); + // Verify that we have a new signature request in the pipeline + assert_eq!( + MockThresholdSigner::::signature_result(0), + AsyncResult::Pending + ); }); } @@ -506,7 +531,7 @@ fn threshold_sign_and_broadcast_with_callback() { tx_out_id: MOCK_TRANSACTION_OUT_ID, }; - let (broadcast_id, _threshold_request_id) = + let broadcast_id = Broadcaster::threshold_sign_and_broadcast(api_call.clone(), Some(MockCallback), |_| { None }); @@ -573,18 +598,15 @@ fn ensure_retries_are_skipped_during_safe_mode() { fn transaction_succeeded_results_in_refund_refuse_for_signer() { new_test_ext().execute_with(|| { MockEthereumTransactionMetadata::set_validity(false); - let _ = start_mock_broadcast_tx_out_id(MOCK_TRANSACTION_OUT_ID); + + let (tx_out_id, apicall) = api_call(1); + initiate_and_sign_broadcast(&apicall, TxType::Normal); + let nominee = MockNominator::get_last_nominee().unwrap(); assert_eq!(TransactionFeeDeficit::::get(nominee), 0); - assert_ok!(Broadcaster::transaction_succeeded( - RuntimeOrigin::root(), - MOCK_TRANSACTION_OUT_ID, - nominee, - ETH_TX_FEE, - MOCK_TX_METADATA, - )); + witness_broadcast(tx_out_id); assert_eq!( System::events().get(1).expect("an event").event, @@ -603,7 +625,7 @@ fn callback_is_called_upon_broadcast_failure() { sig: Default::default(), tx_out_id: MOCK_TRANSACTION_OUT_ID, }; - let (broadcast_id, _threshold_request_id) = + let broadcast_id = Broadcaster::threshold_sign_and_broadcast(api_call.clone(), None, |_| { Some(MockCallback) }); @@ -691,9 +713,10 @@ fn retry_and_success_in_same_block() { #[test] fn retry_with_threshold_signing_still_allows_late_success_witness_second_attempt() { let mut expected_expiry_block = 0; + const MOCK_TRANSACTION_OUT_ID: [u8; 4] = [0xbc; 4]; new_test_ext() .execute_with(|| { - let broadcast_attempt_id = start_mock_broadcast_tx_out_id(MOCK_TRANSACTION_OUT_ID); + let (broadcast_attempt_id, _) = start_mock_broadcast_tx_out_id(0xbc); let awaiting_broadcast = AwaitingBroadcast::::get(broadcast_attempt_id).unwrap(); @@ -729,7 +752,7 @@ fn retry_with_threshold_signing_still_allows_late_success_witness_second_attempt ), 1 ); - MockTransactionBuilder::::set_invalid_for_rebroadcast(); + MockTransactionBuilder::::set_requires_refresh(); MockCfe::respond(Scenario::Timeout); (nominee, awaiting_broadcast) }) @@ -758,3 +781,232 @@ fn retry_with_threshold_signing_still_allows_late_success_witness_second_attempt ); }); } + +#[test] +fn broadcast_barrier_for_polkadot() { + new_test_ext().execute_with(|| { + MockBroadcastBarriers::set(ChainChoice::Polkadot); + + let (tx_out_id1, api_call1) = api_call(1); + let (tx_out_id2, api_call2) = api_call(2); + let (tx_out_id3, api_call3) = api_call(3); + + // create and sign 3 txs that are then ready for broadcast + + let broadcast_id_1 = initiate_and_sign_broadcast(&api_call1, TxType::Normal); + // tx1 emits broadcast request + assert_transaction_broadcast_request_event(broadcast_id_1, 0, tx_out_id1); + + let broadcast_id_2 = initiate_and_sign_broadcast(&api_call2, TxType::Rotation); + // tx2 emits broadcast request and also pauses any further new broadcast requests + assert_transaction_broadcast_request_event(broadcast_id_2, 0, tx_out_id2); + + let broadcast_id_3 = initiate_and_sign_broadcast(&api_call3, TxType::Normal); + + // tx3 is ready for broadcast but since there is a broadcast pause, broadcast request is + // not issued, the broadcast is rescheduled instead. + System::assert_last_event(RuntimeEvent::Broadcaster( + crate::Event::::BroadcastRetryScheduled { + broadcast_attempt_id: BroadcastAttemptId { + broadcast_id: broadcast_id_3, + attempt_count: 0, + }, + }, + )); + + // report successful broadcast of tx1 + witness_broadcast(tx_out_id1); + + // tx3 should still not be broadcasted because the blocking tx (tx2) has still not + // succeeded. + Broadcaster::on_idle(0, LARGE_EXCESS_WEIGHT); + assert_eq!( + BroadcastRetryQueue::::get()[0] + .broadcast_attempt_id + .broadcast_id, + broadcast_id_3 + ); + + // Now tx2 succeeds which should allow tx3 to be broadcast + witness_broadcast(tx_out_id2); + Broadcaster::on_idle(0, LARGE_EXCESS_WEIGHT); + + // attempt count is 1 because the previous failure to broadcast because of + // broadcast pause is considered an attempt + assert_transaction_broadcast_request_event(broadcast_id_3, 1, tx_out_id3); + + assert!(BroadcastRetryQueue::::get().is_empty()); + + witness_broadcast(tx_out_id3); + }); +} + +#[test] +fn broadcast_barrier_for_bitcoin() { + new_test_ext().execute_with(|| { + MockBroadcastBarriers::set(ChainChoice::Bitcoin); + + let (tx_out_id1, api_call1) = api_call(1); + let (tx_out_id2, api_call2) = api_call(2); + let (tx_out_id3, api_call3) = api_call(3); + + // create and sign 3 txs that are then ready for broadcast + let broadcast_id_1 = initiate_and_sign_broadcast(&api_call1, TxType::Normal); + // tx1 emits broadcast request + assert_transaction_broadcast_request_event(broadcast_id_1, 0, tx_out_id1); + + let broadcast_id_2 = initiate_and_sign_broadcast(&api_call2, TxType::Rotation); + // tx2 emits broadcast request and does not pause future broadcasts in bitcoin + assert_transaction_broadcast_request_event(broadcast_id_2, 0, tx_out_id2); + + let broadcast_id_3 = initiate_and_sign_broadcast(&api_call3, TxType::Normal); + // tx3 emits broadcast request + assert_transaction_broadcast_request_event(broadcast_id_3, 0, tx_out_id3); + + // we successfully witness all txs + witness_broadcast(tx_out_id1); + witness_broadcast(tx_out_id2); + witness_broadcast(tx_out_id3); + }); +} + +#[test] +fn broadcast_barrier_for_ethereum() { + new_test_ext().execute_with(|| { + MockBroadcastBarriers::set(ChainChoice::Ethereum); + + let (tx_out_id1, api_call1) = api_call(1); + let (tx_out_id2, api_call2) = api_call(2); + let (tx_out_id3, api_call3) = api_call(3); + let (tx_out_id4, api_call4) = api_call(4); + + let broadcast_id_1 = initiate_and_sign_broadcast(&api_call1, TxType::Normal); + // tx1 emits broadcast request + assert_transaction_broadcast_request_event(broadcast_id_1, 0, tx_out_id1); + + let broadcast_id_2 = initiate_and_sign_broadcast(&api_call2, TxType::Normal); + // tx2 emits broadcast request + assert_transaction_broadcast_request_event(broadcast_id_2, 0, tx_out_id2); + + // this will put a bbroadcast barrier at tx2 and tx3. tx3 wont be broadcasted yet + let broadcast_id_3 = initiate_and_sign_broadcast(&api_call3, TxType::Rotation); + + // tx3 is ready for broadcast but since there is a broadcast pause, broadcast request is + // not issued, the broadcast is rescheduled instead. + System::assert_last_event(RuntimeEvent::Broadcaster( + crate::Event::::BroadcastRetryScheduled { + broadcast_attempt_id: BroadcastAttemptId { + broadcast_id: broadcast_id_3, + attempt_count: 0, + }, + }, + )); + + // tx4 will be created but not broadcasted yet + let broadcast_id_4 = initiate_and_sign_broadcast(&api_call4, TxType::Normal); + System::assert_last_event(RuntimeEvent::Broadcaster( + crate::Event::::BroadcastRetryScheduled { + broadcast_attempt_id: BroadcastAttemptId { + broadcast_id: broadcast_id_4, + attempt_count: 0, + }, + }, + )); + + // report successful broadcast of tx2 + witness_broadcast(tx_out_id2); + + // tx3 and tx4 should still not be broadcasted because not all txs before and including tx2 + // have been witnessed + Broadcaster::on_idle(0, LARGE_EXCESS_WEIGHT); + assert_eq!( + BroadcastRetryQueue::::get()[0] + .broadcast_attempt_id + .broadcast_id, + broadcast_id_3 + ); + + assert_eq!( + BroadcastRetryQueue::::get()[1] + .broadcast_attempt_id + .broadcast_id, + broadcast_id_4 + ); + + // Now tx1 succeeds which should allow tx3 to be broadcast but not tx4 since there will be + // another barrier at tx3 + witness_broadcast(tx_out_id1); + Broadcaster::on_idle(0, LARGE_EXCESS_WEIGHT); + // attempt count is 1 because the previous failure to broadcast because of + // broadcast pause is considered an attempt + assert_transaction_broadcast_request_event(broadcast_id_3, 1, tx_out_id3); + + // tx4 is still pending + assert_eq!( + BroadcastRetryQueue::::get()[0] + .broadcast_attempt_id + .broadcast_id, + broadcast_id_4 + ); + + // witness tx3 which should allow tx4 to be broadcast + witness_broadcast(tx_out_id3); + Broadcaster::on_idle(0, LARGE_EXCESS_WEIGHT); + + assert_transaction_broadcast_request_event(broadcast_id_4, 1, tx_out_id4); + assert!(BroadcastRetryQueue::::get().is_empty()); + witness_broadcast(tx_out_id4); + }); +} + +fn api_call(i: u8) -> ([u8; 4], MockApiCall) { + let tx_out_id = [i; 4]; + (tx_out_id, MockApiCall { tx_out_id, sig: Default::default(), payload: Default::default() }) +} + +fn assert_transaction_broadcast_request_event( + broadcast_id: BroadcastId, + attempt_count: u32, + tx_out_id: [u8; 4], +) { + System::assert_last_event(RuntimeEvent::Broadcaster( + crate::Event::::TransactionBroadcastRequest { + transaction_out_id: tx_out_id, + broadcast_attempt_id: BroadcastAttemptId { broadcast_id, attempt_count }, + transaction_payload: Default::default(), + nominee: MockNominator::get_last_nominee().unwrap(), + }, + )); +} + +fn initiate_and_sign_broadcast( + apicall: &MockApiCall, + tx_type: TxType, +) -> BroadcastId { + let broadcast_id = match tx_type { + TxType::Normal => >::TargetChain, + >>::threshold_sign_and_broadcast((*apicall).clone()), + TxType::Rotation => >::TargetChain, + >>::threshold_sign_and_broadcast_rotation_tx((*apicall).clone()), + }; + + EthMockThresholdSigner::execute_signature_result_against_last_request(Ok(ETH_DUMMY_SIG)); + + broadcast_id +} + +fn witness_broadcast(tx_out_id: [u8; 4]) { + assert_ok!(Broadcaster::transaction_succeeded( + RuntimeOrigin::root(), + tx_out_id, + Default::default(), + ETH_TX_FEE, + MOCK_TX_METADATA, + )); +} +enum TxType { + Normal, + Rotation, +} diff --git a/state-chain/pallets/cf-emissions/src/mock.rs b/state-chain/pallets/cf-emissions/src/mock.rs index 81389d523a..ae49b056e0 100644 --- a/state-chain/pallets/cf-emissions/src/mock.rs +++ b/state-chain/pallets/cf-emissions/src/mock.rs @@ -173,18 +173,20 @@ impl Broadcaster for MockBroadcast { type ApiCall = MockUpdateFlipSupply; type Callback = MockCallback; - fn threshold_sign_and_broadcast( - api_call: Self::ApiCall, - ) -> (BroadcastId, ThresholdSignatureRequestId) { + fn threshold_sign_and_broadcast(api_call: Self::ApiCall) -> BroadcastId { Self::call(api_call); - (1, 2) + 1 } fn threshold_sign_and_broadcast_with_callback( _api_call: Self::ApiCall, _success_callback: Option, _failed_callback_generator: impl FnOnce(BroadcastId) -> Option, - ) -> (BroadcastId, ThresholdSignatureRequestId) { + ) -> BroadcastId { + unimplemented!() + } + + fn threshold_sign_and_broadcast_rotation_tx(_api_call: Self::ApiCall) -> BroadcastId { unimplemented!() } diff --git a/state-chain/pallets/cf-environment/src/mock.rs b/state-chain/pallets/cf-environment/src/mock.rs index a484a42abd..8d5b5ecb14 100644 --- a/state-chain/pallets/cf-environment/src/mock.rs +++ b/state-chain/pallets/cf-environment/src/mock.rs @@ -103,9 +103,7 @@ impl Broadcaster for MockPolkadotBroadcaster { type ApiCall = MockCreatePolkadotVault; type Callback = MockCallback; - fn threshold_sign_and_broadcast( - _api_call: Self::ApiCall, - ) -> (BroadcastId, ThresholdSignatureRequestId) { + fn threshold_sign_and_broadcast(_api_call: Self::ApiCall) -> BroadcastId { unimplemented!() } @@ -113,7 +111,11 @@ impl Broadcaster for MockPolkadotBroadcaster { _api_call: Self::ApiCall, _success_callback: Option, _failed_callback_generator: impl FnOnce(BroadcastId) -> Option, - ) -> (BroadcastId, ThresholdSignatureRequestId) { + ) -> BroadcastId { + unimplemented!() + } + + fn threshold_sign_and_broadcast_rotation_tx(_api_call: Self::ApiCall) -> BroadcastId { unimplemented!() } diff --git a/state-chain/pallets/cf-funding/src/lib.rs b/state-chain/pallets/cf-funding/src/lib.rs index 5e5b879161..10c6507279 100644 --- a/state-chain/pallets/cf-funding/src/lib.rs +++ b/state-chain/pallets/cf-funding/src/lib.rs @@ -448,7 +448,7 @@ pub mod pallet { Self::deposit_event(Event::RedemptionRequested { account_id, amount: redeem_amount, - broadcast_id: T::Broadcaster::threshold_sign_and_broadcast(call).0, + broadcast_id: T::Broadcaster::threshold_sign_and_broadcast(call), expiry_time: contract_expiry, }); } else { diff --git a/state-chain/pallets/cf-funding/src/mock.rs b/state-chain/pallets/cf-funding/src/mock.rs index 1899e4ce04..28e84d7e1c 100644 --- a/state-chain/pallets/cf-funding/src/mock.rs +++ b/state-chain/pallets/cf-funding/src/mock.rs @@ -154,20 +154,22 @@ impl Broadcaster for MockBroadcaster { type ApiCall = MockRegisterRedemption; type Callback = MockCallback; - fn threshold_sign_and_broadcast( - api_call: Self::ApiCall, - ) -> (BroadcastId, ThresholdSignatureRequestId) { + fn threshold_sign_and_broadcast(api_call: Self::ApiCall) -> BroadcastId { REDEMPTION_BROADCAST_REQUESTS.with(|cell| { cell.borrow_mut().push(api_call.amount); }); - (0, 1) + 0 } fn threshold_sign_and_broadcast_with_callback( _api_call: Self::ApiCall, _success_callback: Option, _failed_callback_generator: impl FnOnce(BroadcastId) -> Option, - ) -> (BroadcastId, ThresholdSignatureRequestId) { + ) -> BroadcastId { + unimplemented!() + } + + fn threshold_sign_and_broadcast_rotation_tx(_api_call: Self::ApiCall) -> BroadcastId { unimplemented!() } diff --git a/state-chain/pallets/cf-ingress-egress/src/lib.rs b/state-chain/pallets/cf-ingress-egress/src/lib.rs index dbcb0fde45..77c7d6afa3 100644 --- a/state-chain/pallets/cf-ingress-egress/src/lib.rs +++ b/state-chain/pallets/cf-ingress-egress/src/lib.rs @@ -835,7 +835,7 @@ impl, I: 'static> Pallet { transfer_params, ) { Ok(egress_transaction) => { - let (broadcast_id, _) = T::Broadcaster::threshold_sign_and_broadcast_with_callback( + let broadcast_id = T::Broadcaster::threshold_sign_and_broadcast_with_callback( egress_transaction, Some(Call::finalise_ingress { addresses }.into()), |_| None, @@ -876,12 +876,11 @@ impl, I: 'static> Pallet { ccm.message.to_vec(), ) { Ok(api_call) => { - let (broadcast_id, _) = - T::Broadcaster::threshold_sign_and_broadcast_with_callback( - api_call, - None, - |broadcast_id| Some(Call::ccm_broadcast_failed { broadcast_id }.into()), - ); + let broadcast_id = T::Broadcaster::threshold_sign_and_broadcast_with_callback( + api_call, + None, + |broadcast_id| Some(Call::ccm_broadcast_failed { broadcast_id }.into()), + ); Self::deposit_event(Event::::CcmBroadcastRequested { broadcast_id, egress_id: ccm.egress_id, diff --git a/state-chain/pallets/cf-threshold-signature/src/benchmarking.rs b/state-chain/pallets/cf-threshold-signature/src/benchmarking.rs index 8cf4bb8f99..2a4255c94a 100644 --- a/state-chain/pallets/cf-threshold-signature/src/benchmarking.rs +++ b/state-chain/pallets/cf-threshold-signature/src/benchmarking.rs @@ -4,7 +4,7 @@ use super::*; use cf_chains::{benchmarking_value::BenchmarkValue, ChainCrypto}; -use cf_traits::{AccountRoleRegistry, Chainflip, ThresholdSigner}; +use cf_traits::{AccountRoleRegistry, Chainflip, CurrentEpochIndex, ThresholdSigner}; use frame_benchmarking::{account, benchmarks_instance_pallet, whitelist_account}; use frame_support::{ assert_ok, @@ -94,12 +94,14 @@ benchmarks_instance_pallet! { let a in 10..150; // r: number of retries let r in 0..50; - T::KeyProvider::set_key(::AggKey::benchmark_value()); + let key = ::AggKey::benchmark_value(); + let current_epoch = CurrentEpochIndex::::get(); + T::KeyProvider::set_key(key, current_epoch); CurrentAuthorities::::put(BTreeSet::<::ValidatorId>::new()); // These attempts will fail because there are no authorities to do the signing. for _ in 0..r { - Pallet::::new_ceremony_attempt(RequestInstruction::new(1, 1, PayloadFor::::benchmark_value(), RequestType::CurrentKey)); + Pallet::::new_ceremony_attempt(RequestInstruction::new(1, 1, PayloadFor::::benchmark_value(), RequestType::SpecificKey(key, current_epoch))); } assert_eq!( diff --git a/state-chain/pallets/cf-threshold-signature/src/lib.rs b/state-chain/pallets/cf-threshold-signature/src/lib.rs index 48bd9ece2a..89868d577e 100644 --- a/state-chain/pallets/cf-threshold-signature/src/lib.rs +++ b/state-chain/pallets/cf-threshold-signature/src/lib.rs @@ -57,9 +57,6 @@ pub enum PalletOffence { #[derive(Clone, RuntimeDebug, PartialEq, Eq, Encode, Decode, TypeInfo)] pub enum RequestType { - /// Will use the current key and current authority set (which might change between retries). - /// This signing request will be retried until success. - CurrentKey, /// Uses the provided key and selects new participants from the provided epoch. /// This signing request will be retried until success. SpecificKey(Key, EpochIndex), @@ -365,11 +362,6 @@ pub mod pallet { request_id: RequestId, attempt_count: AttemptCount, }, - /// We cannot sign because the key is unavailable. - CurrentKeyUnavailable { - request_id: RequestId, - attempt_count: AttemptCount, - }, /// The threshold signature response timeout has been updated ThresholdSignatureResponseTimeoutUpdated { new_timeout: BlockNumberFor, @@ -422,11 +414,7 @@ pub mod pallet { request_id, attempt_count.wrapping_add(1), payload, - if ::sign_with_specific_key() { - RequestType::SpecificKey(key, epoch) - } else { - RequestType::CurrentKey - }, + RequestType::SpecificKey(key, epoch), )); Event::::RetryRequested { request_id, ceremony_id } }, @@ -675,18 +663,6 @@ impl, I: 'static> Pallet { } else { ( match request_instruction.request_type { - RequestType::CurrentKey => T::KeyProvider::active_epoch_key() - .and_then(|EpochKey { key_state, key, epoch_index }| { - if key_state.is_available_for_request(request_id) { - Some((key, epoch_index)) - } else { - None - } - }) - .ok_or(Event::::CurrentKeyUnavailable { - request_id, - attempt_count, - }), RequestType::SpecificKey(key, epoch_index) => Ok((key, epoch_index)), _ => unreachable!("RequestType::KeygenVerification is handled above"), } @@ -816,14 +792,11 @@ where type ValidatorId = T::ValidatorId; fn request_signature(payload: PayloadFor) -> RequestId { - let request_type = if ::sign_with_specific_key() { - T::KeyProvider::active_epoch_key().defensive_map_or_else( - || RequestType::CurrentKey, - |EpochKey { key, epoch_index, .. }| RequestType::SpecificKey(key, epoch_index), - ) - } else { - RequestType::CurrentKey - }; + let request_type = T::KeyProvider::active_epoch_key().defensive_map_or_else( + || RequestType::SpecificKey(Default::default(), Default::default()), + |EpochKey { key, epoch_index, .. }| RequestType::SpecificKey(key, epoch_index), + ); + Self::inner_request_signature(payload, request_type) } diff --git a/state-chain/pallets/cf-threshold-signature/src/tests.rs b/state-chain/pallets/cf-threshold-signature/src/tests.rs index a00d981e97..bfe5caca71 100644 --- a/state-chain/pallets/cf-threshold-signature/src/tests.rs +++ b/state-chain/pallets/cf-threshold-signature/src/tests.rs @@ -4,7 +4,7 @@ use crate::{ self as pallet_cf_threshold_signature, mock::*, AttemptCount, CeremonyContext, CeremonyId, Error, PalletOffence, RequestContext, RequestId, ThresholdSignatureResponseTimeout, }; -use cf_chains::mocks::{MockAggKey, MockEthereumChainCrypto, MockFixedKeySigningRequests}; +use cf_chains::mocks::{MockAggKey, MockEthereumChainCrypto}; use cf_traits::{ mocks::{key_provider::MockKeyProvider, signer_nomination::MockNominator}, AsyncResult, Chainflip, EpochKey, KeyProvider, ThresholdSigner, @@ -433,7 +433,7 @@ fn test_not_enough_signers_for_threshold_schedules_retry() { } #[test] -fn test_retries_for_locked_key() { +fn test_retries() { const NOMINEES: [u64; 4] = [1, 2, 3, 4]; const AUTHORITIES: [u64; 5] = [1, 2, 3, 4, 5]; new_test_ext() @@ -447,13 +447,6 @@ fn test_retries_for_locked_key() { .. } = EthereumThresholdSigner::pending_ceremonies(ceremony_id).unwrap(); - MockKeyProvider::::lock_key(request_id); - - // Key is now locked and should be unavailable for new requests. - >::request_signature(*b"SUP?"); - // Ceremony counter should not have changed. - assert_eq!(ceremony_id, current_ceremony_id()); - // Retry should re-use the same key. let retry_block = frame_system::Pallet::::current_block_number() + ThresholdSignatureResponseTimeout::::get(); @@ -486,8 +479,6 @@ fn test_retries_for_immutable_key() { .. } = EthereumThresholdSigner::pending_ceremonies(ceremony_id).unwrap(); - MockFixedKeySigningRequests::set(true); - // Pretend the key has been updated to the next one. MockKeyProvider::::add_key(MockAggKey(*b"NEXT")); diff --git a/state-chain/pallets/cf-vaults/src/benchmarking.rs b/state-chain/pallets/cf-vaults/src/benchmarking.rs index 0e636901cc..16901bdb53 100644 --- a/state-chain/pallets/cf-vaults/src/benchmarking.rs +++ b/state-chain/pallets/cf-vaults/src/benchmarking.rs @@ -147,20 +147,6 @@ benchmarks_instance_pallet! { if new_public_key == agg_key )) } - vault_key_rotated { - let new_public_key = AggKeyFor::::benchmark_value(); - PendingVaultRotation::::put( - VaultRotationStatus::::AwaitingActivation { new_public_key }, - ); - let call = Call::::vault_key_rotated { - block_number: 5u32.into(), - tx_id: Decode::decode(&mut &TX_HASH[..]).unwrap() - }; - let origin = T::EnsureWitnessedAtCurrentEpoch::try_successful_origin().unwrap(); - } : { call.dispatch_bypass_filter(origin)? } - verify { - assert!(Vaults::::contains_key(T::EpochInfo::epoch_index())); - } vault_key_rotated_externally { let origin = T::EnsureWitnessedAtCurrentEpoch::try_successful_origin().unwrap(); let call = Call::::vault_key_rotated_externally { diff --git a/state-chain/pallets/cf-vaults/src/lib.rs b/state-chain/pallets/cf-vaults/src/lib.rs index 420a423116..ad801e9114 100644 --- a/state-chain/pallets/cf-vaults/src/lib.rs +++ b/state-chain/pallets/cf-vaults/src/lib.rs @@ -9,8 +9,8 @@ use cf_primitives::{ use cf_runtime_utilities::{EnumVariant, StorageDecodeVariant}; use cf_traits::{ offence_reporting::OffenceReporter, AccountRoleRegistry, AsyncResult, Broadcaster, Chainflip, - CurrentEpochIndex, EpochKey, GetBlockHeight, KeyProvider, KeyState, SafeMode, SetSafeMode, - Slashing, ThresholdSigner, VaultKeyWitnessedHandler, VaultRotator, VaultStatus, + CurrentEpochIndex, EpochKey, GetBlockHeight, KeyProvider, SafeMode, SetSafeMode, Slashing, + ThresholdSigner, VaultKeyWitnessedHandler, VaultRotator, VaultStatus, }; use frame_support::{ pallet_prelude::*, @@ -40,7 +40,7 @@ pub mod migrations; mod mock; mod tests; -pub const PALLET_VERSION: StorageVersion = StorageVersion::new(2); +pub const PALLET_VERSION: StorageVersion = StorageVersion::new(3); const KEYGEN_CEREMONY_RESPONSE_TIMEOUT_BLOCKS_DEFAULT: u32 = 90; @@ -153,12 +153,6 @@ pub enum PalletOffence { FailedKeyHandover, } -#[derive(Encode, Decode, TypeInfo)] -pub struct VaultEpochAndState { - pub epoch_index: EpochIndex, - pub key_state: KeyState, -} - #[frame_support::pallet] pub mod pallet { @@ -352,8 +346,7 @@ pub mod pallet { /// The epoch whose authorities control the current vault key. #[pallet::storage] #[pallet::getter(fn current_keyholders_epoch)] - pub type CurrentVaultEpochAndState, I: 'static = ()> = - StorageValue<_, VaultEpochAndState>; + pub type CurrentVaultEpoch, I: 'static = ()> = StorageValue<_, EpochIndex>; /// Vault rotation statuses for the current epoch rotation. #[pallet::storage] @@ -660,33 +653,19 @@ pub mod pallet { ) } - /// A vault rotation event has been witnessed, we update the vault with the new key. - /// - /// ## Events - /// - /// - [VaultRotationCompleted](Event::VaultRotationCompleted) - /// - /// ## Errors - /// - /// - [NoActiveRotation](Error::NoActiveRotation) - /// - [InvalidRotationStatus](Error::InvalidRotationStatus) - /// - /// ## Dependencies - /// - /// - [Epoch Info Trait](EpochInfo) + /// Deprecated! This extrinsic does nothing #[pallet::call_index(3)] - #[pallet::weight(T::WeightInfo::vault_key_rotated())] + #[pallet::weight(Weight::zero())] pub fn vault_key_rotated( origin: OriginFor, - block_number: ChainBlockNumberFor, + _block_number: ChainBlockNumberFor, // This field is primarily required to ensure the witness calls are unique per // transaction (on the external chain) _tx_id: TransactionInIdFor, ) -> DispatchResultWithPostInfo { T::EnsureWitnessedAtCurrentEpoch::ensure_origin(origin)?; - - Self::on_new_key_activated(block_number) + Ok(().into()) } /// The vault's key has been updated externally, outside of the rotation @@ -855,10 +834,7 @@ impl, I: 'static> Pallet { fn set_vault_key_for_epoch(epoch_index: EpochIndex, vault: Vault) { Vaults::::insert(epoch_index, vault); - CurrentVaultEpochAndState::::put(VaultEpochAndState { - epoch_index, - key_state: KeyState::Unlocked, - }); + CurrentVaultEpoch::::put(epoch_index); } // Once we've successfully generated the key, we want to do a signing ceremony to verify that @@ -967,20 +943,19 @@ impl, I: 'static> Pallet { impl, I: 'static> KeyProvider<::ChainCrypto> for Pallet { fn active_epoch_key( ) -> Option::ChainCrypto as ChainCrypto>::AggKey>> { - CurrentVaultEpochAndState::::get().map(|current_vault_epoch_and_state| { + CurrentVaultEpoch::::get().map(|current_vault_epoch| { EpochKey { - key: Vaults::::get(current_vault_epoch_and_state.epoch_index) - .expect("Key must exist if CurrentVaultEpochAndState exists since they get set at the same place: set_next_vault()").public_key, - epoch_index: current_vault_epoch_and_state.epoch_index, - key_state: current_vault_epoch_and_state.key_state, + key: Vaults::::get(current_vault_epoch) + .expect("Key must exist if CurrentVaultEpoch exists since they get set at the same place: set_vault_key_for_epoch()").public_key, + epoch_index: current_vault_epoch, } }) } #[cfg(feature = "runtime-benchmarks")] - fn set_key(key: <::ChainCrypto as ChainCrypto>::AggKey) { + fn set_key(key: <::ChainCrypto as ChainCrypto>::AggKey, epoch: EpochIndex) { Vaults::::insert( - CurrentEpochIndex::::get(), + epoch, Vault { public_key: key, active_from_block: ChainBlockNumberFor::::from(0u32) }, ); } diff --git a/state-chain/pallets/cf-vaults/src/migrations.rs b/state-chain/pallets/cf-vaults/src/migrations.rs index 3eaa166e9b..a5e9e7e5b2 100644 --- a/state-chain/pallets/cf-vaults/src/migrations.rs +++ b/state-chain/pallets/cf-vaults/src/migrations.rs @@ -1,6 +1,9 @@ pub mod v2; +pub mod v3; use cf_runtime_upgrade_utilities::VersionedMigration; -pub type PalletMigration = - (VersionedMigration, v2::Migration, 1, 2>,); +pub type PalletMigration = ( + VersionedMigration, v2::Migration, 1, 2>, + VersionedMigration, v3::Migration, 2, 3>, +); diff --git a/state-chain/pallets/cf-vaults/src/migrations/v3.rs b/state-chain/pallets/cf-vaults/src/migrations/v3.rs new file mode 100644 index 0000000000..927ba73f3e --- /dev/null +++ b/state-chain/pallets/cf-vaults/src/migrations/v3.rs @@ -0,0 +1,64 @@ +use crate::*; + +#[cfg(feature = "try-runtime")] +use frame_support::dispatch::DispatchError; +use frame_support::traits::OnRuntimeUpgrade; +use sp_std::marker::PhantomData; + +/// v2 migrating storage item KeygenSlashRate -> KeygenSlashAmount +/// Percent -> FlipBalance +pub struct Migration, I: 'static>(PhantomData<(T, I)>); + +mod old { + + use super::*; + + #[derive(Debug, TypeInfo, Decode, Encode, Clone, Copy, PartialEq, Eq)] + pub enum KeyState { + Unlocked, + /// Key is only available to sign this request id. + Locked(ThresholdSignatureRequestId), + } + #[derive(Encode, Decode, TypeInfo)] + pub struct VaultEpochAndState { + pub epoch_index: EpochIndex, + pub key_state: KeyState, + } + + #[frame_support::storage_alias] + pub type CurrentVaultEpochAndState, I: 'static> = + StorageValue, VaultEpochAndState>; +} + +impl, I: 'static> OnRuntimeUpgrade for Migration { + fn on_runtime_upgrade() -> frame_support::weights::Weight { + // remove the old storage containing Percent + if let Some(old::VaultEpochAndState { epoch_index, key_state: _ }) = + old::CurrentVaultEpochAndState::::take() + { + // set the new storage using the EpochIndex from the old storage item above. + CurrentVaultEpoch::::put(epoch_index); + } + + Weight::zero() + } + + #[cfg(feature = "try-runtime")] + fn pre_upgrade() -> Result, DispatchError> { + let state = old::CurrentVaultEpochAndState::::get().unwrap(); + + Ok(state.encode()) + } + + #[cfg(feature = "try-runtime")] + fn post_upgrade(state: Vec) -> Result<(), DispatchError> { + let old_state = ::decode(&mut &state[..]) + .map_err(|_| "Failed to decode pre-upgrade state.")?; + + assert!(!old::CurrentVaultEpochAndState::::exists()); + assert!(CurrentVaultEpoch::::exists()); + assert_eq!(old_state.epoch_index, CurrentVaultEpoch::::get().unwrap()); + + Ok(()) + } +} diff --git a/state-chain/pallets/cf-vaults/src/mock.rs b/state-chain/pallets/cf-vaults/src/mock.rs index ffffccc325..d15bd245fd 100644 --- a/state-chain/pallets/cf-vaults/src/mock.rs +++ b/state-chain/pallets/cf-vaults/src/mock.rs @@ -149,21 +149,23 @@ impl Broadcaster for MockBroadcaster { type ApiCall = MockSetAggKeyWithAggKey; type Callback = MockCallback; - fn threshold_sign_and_broadcast( - _api_call: Self::ApiCall, - ) -> (BroadcastId, ThresholdSignatureRequestId) { + fn threshold_sign_and_broadcast(_api_call: Self::ApiCall) -> BroadcastId { Self::send_broadcast(); - (1, 2) + 1 } fn threshold_sign_and_broadcast_with_callback( _api_call: Self::ApiCall, _success_callback: Option, _failed_callback_generator: impl FnOnce(BroadcastId) -> Option, - ) -> (BroadcastId, ThresholdSignatureRequestId) { + ) -> BroadcastId { unimplemented!() } + fn threshold_sign_and_broadcast_rotation_tx(api_call: Self::ApiCall) -> BroadcastId { + Self::threshold_sign_and_broadcast(api_call) + } + fn threshold_resign(_broadcast_id: BroadcastId) -> Option { unimplemented!() } diff --git a/state-chain/pallets/cf-vaults/src/tests.rs b/state-chain/pallets/cf-vaults/src/tests.rs index 7f108be598..0829737c1d 100644 --- a/state-chain/pallets/cf-vaults/src/tests.rs +++ b/state-chain/pallets/cf-vaults/src/tests.rs @@ -7,11 +7,7 @@ use crate::{ KeygenFailureVoters, KeygenOutcomeFor, KeygenResolutionPendingSince, KeygenResponseTimeout, KeygenSuccessVoters, PalletOffence, PendingVaultRotation, Vault, VaultRotationStatus, Vaults, }; -use cf_chains::{ - btc::BitcoinCrypto, - evm::EvmCrypto, - mocks::{MockAggKey, MockOptimisticActivation}, -}; +use cf_chains::{btc::BitcoinCrypto, evm::EvmCrypto, mocks::MockAggKey}; use cf_primitives::{AuthorityCount, GENESIS_EPOCH}; use cf_test_utilities::{last_event, maybe_last_event}; use cf_traits::{ @@ -517,8 +513,6 @@ fn cannot_report_key_handover_outcome_when_awaiting_keygen() { } fn do_full_key_rotation() { - assert!(!MockOptimisticActivation::get(), "Test expects non-optimistic activation"); - let rotation_epoch = ::EpochInfo::epoch_index() + 1; ::keygen( BTreeSet::from_iter(ALL_CANDIDATES.iter().cloned()), @@ -649,19 +643,11 @@ fn do_full_key_rotation() { VaultsPallet::activate(); assert!(!KeygenResolutionPendingSince::::exists()); - assert_eq!(::status(), AsyncResult::Pending); - - assert!(matches!( - PendingVaultRotation::::get().unwrap(), - VaultRotationStatus::::AwaitingActivation { new_public_key: k } if k == NEW_AGG_PUB_KEY_POST_HANDOVER - )); // Voting has been cleared. assert_eq!(KeygenSuccessVoters::::iter_keys().next(), None); assert!(!KeygenFailureVoters::::exists()); - assert_ok!(VaultsPallet::vault_key_rotated(RuntimeOrigin::root(), 1, [0xab; 4],)); - assert_last_event!(crate::Event::VaultRotationCompleted); assert_eq!(PendingVaultRotation::::get(), Some(VaultRotationStatus::Complete)); assert_eq!(VaultsPallet::status(), AsyncResult::Ready(VaultStatus::RotationComplete)); @@ -801,9 +787,6 @@ mod vault_key_rotation { use super::*; - const ACTIVATION_BLOCK_NUMBER: u64 = 42; - const TX_HASH: [u8; 4] = [0xab; 4]; - fn setup(key_handover_outcome: KeygenOutcomeFor) -> TestRunner<()> { let ext = new_test_ext(); ext.execute_with(|| { @@ -812,15 +795,6 @@ mod vault_key_rotation { let rotation_epoch_index = ::EpochInfo::epoch_index() + 1; - assert_noop!( - VaultsPallet::vault_key_rotated( - RuntimeOrigin::root(), - ACTIVATION_BLOCK_NUMBER, - TX_HASH, - ), - Error::::NoActiveRotation - ); - ::keygen(candidates.clone(), GENESIS_EPOCH); let ceremony_id = current_ceremony_id(); VaultsPallet::trigger_keygen_verification( @@ -855,16 +829,6 @@ mod vault_key_rotation { fn final_checks(ext: TestRunner<()>, expected_activation_block: u64) { ext.execute_with(|| { - // Can't repeat. - assert_noop!( - VaultsPallet::vault_key_rotated( - RuntimeOrigin::root(), - expected_activation_block, - TX_HASH, - ), - Error::::InvalidRotationStatus - ); - let current_epoch = ::EpochInfo::epoch_index(); let Vault { public_key, active_from_block } = @@ -899,31 +863,6 @@ mod vault_key_rotation { }); } - #[test] - fn non_optimistic_activation() { - let ext = setup(Ok(NEW_AGG_PUB_KEY_POST_HANDOVER)).execute_with(|| { - BtcMockThresholdSigner::execute_signature_result_against_last_request(Ok(vec![ - BTC_DUMMY_SIG, - ])); - - MockOptimisticActivation::set(false); - VaultsPallet::activate(); - - assert!(matches!( - PendingVaultRotation::::get().unwrap(), - VaultRotationStatus::AwaitingActivation { .. } - )); - - assert_ok!(VaultsPallet::vault_key_rotated( - RuntimeOrigin::root(), - ACTIVATION_BLOCK_NUMBER, - TX_HASH, - )); - }); - - final_checks(ext, ACTIVATION_BLOCK_NUMBER); - } - #[test] fn optimistic_activation() { const HANDOVER_ACTIVATION_BLOCK: u64 = 420; @@ -933,19 +872,8 @@ mod vault_key_rotation { ])); BlockHeightProvider::::set_block_height(HANDOVER_ACTIVATION_BLOCK); - MockOptimisticActivation::set(true); VaultsPallet::activate(); - // No need to call vault_key_rotated. - assert_noop!( - VaultsPallet::vault_key_rotated( - RuntimeOrigin::root(), - ACTIVATION_BLOCK_NUMBER, - TX_HASH, - ), - Error::::InvalidRotationStatus - ); - assert!(matches!( PendingVaultRotation::::get().unwrap(), VaultRotationStatus::Complete, @@ -985,8 +913,6 @@ mod vault_key_rotation { BtcMockThresholdSigner::execute_signature_result_against_last_request(Ok(vec![ BTC_DUMMY_SIG, ])); - - MockOptimisticActivation::set(true); VaultsPallet::activate(); }); diff --git a/state-chain/pallets/cf-vaults/src/vault_rotator.rs b/state-chain/pallets/cf-vaults/src/vault_rotator.rs index 97ab9e1cce..4af31a98b5 100644 --- a/state-chain/pallets/cf-vaults/src/vault_rotator.rs +++ b/state-chain/pallets/cf-vaults/src/vault_rotator.rs @@ -152,38 +152,18 @@ impl, I: 'static> VaultRotator for Pallet { if let Some(VaultRotationStatus::::KeyHandoverComplete { new_public_key }) = PendingVaultRotation::::get() { - if let Some(EpochKey { key, key_state, .. }) = Self::active_epoch_key() { + if let Some(EpochKey { key, .. }) = Self::active_epoch_key() { match >::new_unsigned( Some(key), new_public_key, ) { Ok(activation_call) => { - let (_, threshold_request_id) = - T::Broadcaster::threshold_sign_and_broadcast(activation_call); - if ::ChainCrypto::optimistic_activation() { - // Optimistic activation means we don't need to wait for the activation - // transaction to succeed before using the new key. - Self::activate_new_key( - new_public_key, - T::ChainTracking::get_block_height(), - ); - } else { - debug_assert!( - matches!(key_state, KeyState::Unlocked), - "Current epoch key must be active to activate next key." - ); - // The key needs to be locked until activation is complete. - CurrentVaultEpochAndState::::mutate(|epoch_end_key| { - epoch_end_key - .as_mut() - .expect("Checked above at if let Some") - .key_state - .lock(threshold_request_id) - }); - PendingVaultRotation::::put( - VaultRotationStatus::::AwaitingActivation { new_public_key }, - ); - } + T::Broadcaster::threshold_sign_and_broadcast_rotation_tx(activation_call); + + Self::activate_new_key( + new_public_key, + T::ChainTracking::get_block_height(), + ); }, Err(SetAggKeyWithAggKeyError::NotRequired) => { // This can happen if, for example, on a utxo chain there are no funds that diff --git a/state-chain/pallets/cf-vaults/src/weights.rs b/state-chain/pallets/cf-vaults/src/weights.rs index 982ffdff84..697ad90f0d 100644 --- a/state-chain/pallets/cf-vaults/src/weights.rs +++ b/state-chain/pallets/cf-vaults/src/weights.rs @@ -36,7 +36,6 @@ pub trait WeightInfo { fn on_initialize_success() -> Weight; fn report_keygen_outcome() -> Weight; fn on_keygen_verification_result() -> Weight; - fn vault_key_rotated() -> Weight; fn vault_key_rotated_externally() -> Weight; fn set_keygen_response_timeout() -> Weight; } @@ -132,23 +131,6 @@ impl WeightInfo for PalletWeight { .saturating_add(T::DbWeight::get().reads(1_u64)) .saturating_add(T::DbWeight::get().writes(2_u64)) } - /// Storage: `EthereumVault::PendingVaultRotation` (r:1 w:1) - /// Proof: `EthereumVault::PendingVaultRotation` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) - /// Storage: `Validator::CurrentEpoch` (r:1 w:0) - /// Proof: `Validator::CurrentEpoch` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) - /// Storage: `EthereumVault::Vaults` (r:0 w:1) - /// Proof: `EthereumVault::Vaults` (`max_values`: None, `max_size`: None, mode: `Measured`) - /// Storage: `EthereumVault::CurrentVaultEpochAndState` (r:0 w:1) - /// Proof: `EthereumVault::CurrentVaultEpochAndState` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) - fn vault_key_rotated() -> Weight { - // Proof Size summary in bytes: - // Measured: `493` - // Estimated: `1978` - // Minimum execution time: 23_429_000 picoseconds. - Weight::from_parts(23_875_000, 1978) - .saturating_add(T::DbWeight::get().reads(2_u64)) - .saturating_add(T::DbWeight::get().writes(3_u64)) - } /// Storage: `Validator::CurrentEpoch` (r:1 w:0) /// Proof: `Validator::CurrentEpoch` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) /// Storage: `EthereumVault::Vaults` (r:0 w:1) @@ -271,23 +253,6 @@ impl WeightInfo for () { .saturating_add(RocksDbWeight::get().reads(1_u64)) .saturating_add(RocksDbWeight::get().writes(2_u64)) } - /// Storage: `EthereumVault::PendingVaultRotation` (r:1 w:1) - /// Proof: `EthereumVault::PendingVaultRotation` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) - /// Storage: `Validator::CurrentEpoch` (r:1 w:0) - /// Proof: `Validator::CurrentEpoch` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) - /// Storage: `EthereumVault::Vaults` (r:0 w:1) - /// Proof: `EthereumVault::Vaults` (`max_values`: None, `max_size`: None, mode: `Measured`) - /// Storage: `EthereumVault::CurrentVaultEpochAndState` (r:0 w:1) - /// Proof: `EthereumVault::CurrentVaultEpochAndState` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) - fn vault_key_rotated() -> Weight { - // Proof Size summary in bytes: - // Measured: `493` - // Estimated: `1978` - // Minimum execution time: 23_429_000 picoseconds. - Weight::from_parts(23_875_000, 1978) - .saturating_add(RocksDbWeight::get().reads(2_u64)) - .saturating_add(RocksDbWeight::get().writes(3_u64)) - } /// Storage: `Validator::CurrentEpoch` (r:1 w:0) /// Proof: `Validator::CurrentEpoch` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) /// Storage: `EthereumVault::Vaults` (r:0 w:1) diff --git a/state-chain/runtime/src/chainflip.rs b/state-chain/runtime/src/chainflip.rs index 7e8b199c77..48b3f6029d 100644 --- a/state-chain/runtime/src/chainflip.rs +++ b/state-chain/runtime/src/chainflip.rs @@ -164,18 +164,11 @@ impl TransactionBuilder> for EthTransactio } } - fn is_valid_for_rebroadcast( - call: &EthereumApi, + fn requires_signature_refresh( + _call: &EthereumApi, _payload: &<::ChainCrypto as ChainCrypto>::Payload, - current_key: &<::ChainCrypto as ChainCrypto>::AggKey, - signature: &<::ChainCrypto as ChainCrypto>::ThresholdSignature, ) -> bool { - // Check if signature is valid - <::ChainCrypto as ChainCrypto>::verify_threshold_signature( - current_key, - &call.threshold_signature_payload(), - signature, - ) + false } /// Calculate the gas limit for a Ethereum call, using the current gas price. @@ -217,15 +210,13 @@ impl TransactionBuilder> for DotTransactio // TODO: For now this is a noop until we actually have dot chain tracking } - fn is_valid_for_rebroadcast( + fn requires_signature_refresh( call: &PolkadotApi, payload: &<::ChainCrypto as ChainCrypto>::Payload, - _current_key: &<::ChainCrypto as ChainCrypto>::AggKey, - _signature: &<::ChainCrypto as ChainCrypto>::ThresholdSignature, ) -> bool { // Current key and signature are irrelevant. The only thing that can invalidate a polkadot // transaction is if the payload changes due to a runtime version update. - &call.threshold_signature_payload() == payload + &call.threshold_signature_payload() != payload } } @@ -243,11 +234,9 @@ impl TransactionBuilder> for BtcTransactionB // refreshing btc tx is not trivial. We leave it a no-op for now. } - fn is_valid_for_rebroadcast( + fn requires_signature_refresh( _call: &BitcoinApi, _payload: &<::ChainCrypto as ChainCrypto>::Payload, - _current_key: &<::ChainCrypto as ChainCrypto>::AggKey, - _signature: &<::ChainCrypto as ChainCrypto>::ThresholdSignature, ) -> bool { // The payload for a Bitcoin transaction will never change and so it doesnt need to be // checked here. We also dont need to check for the signature here because even if we are in @@ -255,7 +244,7 @@ impl TransactionBuilder> for BtcTransactionB // since its based on those old input UTXOs. In fact, we never have to resign btc txs and // the btc tx is always valid as long as the input UTXOs are valid. Therefore, we don't have // to check anything here and just rebroadcast. - true + false } } diff --git a/state-chain/runtime/src/chainflip/address_derivation/btc.rs b/state-chain/runtime/src/chainflip/address_derivation/btc.rs index 7dd49340b7..dc5800a8d1 100644 --- a/state-chain/runtime/src/chainflip/address_derivation/btc.rs +++ b/state-chain/runtime/src/chainflip/address_derivation/btc.rs @@ -49,10 +49,9 @@ fn test_address_generation() { use crate::Runtime; use cf_chains::Bitcoin; use cf_primitives::chains::assets::btc; - use cf_traits::KeyState; use cf_utilities::assert_ok; use pallet_cf_validator::CurrentEpoch; - use pallet_cf_vaults::{CurrentVaultEpochAndState, Vault, VaultEpochAndState, Vaults}; + use pallet_cf_vaults::{CurrentVaultEpoch, Vault, Vaults}; frame_support::sp_io::TestExternalities::new_empty().execute_with(|| { CurrentEpoch::::set(1); @@ -68,10 +67,7 @@ fn test_address_generation() { active_from_block: 1, }, ); - CurrentVaultEpochAndState::::put(VaultEpochAndState { - epoch_index: 1, - key_state: KeyState::Unlocked, - }); + CurrentVaultEpoch::::put(1); assert_ok!(>::generate_address( btc::Asset::Btc, 1 diff --git a/state-chain/runtime/src/chainflip/broadcaster.rs b/state-chain/runtime/src/chainflip/broadcaster.rs deleted file mode 100644 index 694c2c86f8..0000000000 --- a/state-chain/runtime/src/chainflip/broadcaster.rs +++ /dev/null @@ -1,48 +0,0 @@ -use core::marker::PhantomData; - -use crate::{EthereumBroadcaster, PolkadotBroadcaster}; -use cf_chains::{ - any::{AnyChainApi, AnyGovKey}, - eth::api::EthereumApi, - AnyChain, ChainCrypto, Ethereum, ForeignChain, Polkadot, SetAggKeyWithAggKey, - SetGovKeyWithAggKey, -}; -use cf_traits::{BroadcastAnyChainGovKey, Broadcaster}; -use sp_std::vec::Vec; - -use super::{DotEnvironment, EthEnvironment}; - -pub enum AnyApi { - Ethereum(EthereumApi), - Polkadot(PolkadotApi), -} - -impl SetGovKeyWithAggKey for AnyApi { - fn new_unsigned(maybe_old_key: Option, new_key: AnyGovKey) -> Result { - match (maybe_old_key, new_key) { - AnyGovKey::Ethereum(key) => - ::new_unsigned(maybe_old_key, new_key), - AnyGovKey::Polkadot(key) => - ::new_unsigned(maybe_old_key, new_key), - } - } -} - -impl Broadcaster for GovKeyBroadcaster { - type ApiCall = AnyApi; - - fn threshold_sign_and_broadcast( - api_call: Self::ApiCall, - ) -> cf_primitives::BroadcastId { - match api_call { - AnyChainApi::Ethereum(eth_api_call) => - >::threshold_sign_and_broadcast( - eth_api_call, - ), - AnyChainApi::Polkadot(dot_api_call) => - >::threshold_sign_and_broadcast( - dot_api_call, - ), - } - } -} diff --git a/state-chain/runtime/src/lib.rs b/state-chain/runtime/src/lib.rs index bf8da944f3..da095f21a9 100644 --- a/state-chain/runtime/src/lib.rs +++ b/state-chain/runtime/src/lib.rs @@ -3,6 +3,7 @@ #![recursion_limit = "256"] pub mod chainflip; pub mod constants; +pub mod migrations; pub mod runtime_apis; pub mod safe_mode; #[cfg(feature = "std")] @@ -674,7 +675,6 @@ impl pallet_cf_broadcast::Config for Runtime { type WeightInfo = pallet_cf_broadcast::weights::PalletWeight; type SafeMode = RuntimeSafeMode; type SafeModeBlockMargin = ConstU32<10>; - type KeyProvider = EthereumVault; type ChainTracking = EthereumChainTracking; } @@ -697,7 +697,6 @@ impl pallet_cf_broadcast::Config for Runtime { type WeightInfo = pallet_cf_broadcast::weights::PalletWeight; type SafeMode = RuntimeSafeMode; type SafeModeBlockMargin = ConstU32<10>; - type KeyProvider = PolkadotVault; type ChainTracking = PolkadotChainTracking; } @@ -720,7 +719,6 @@ impl pallet_cf_broadcast::Config for Runtime { type WeightInfo = pallet_cf_broadcast::weights::PalletWeight; type SafeMode = RuntimeSafeMode; type SafeModeBlockMargin = ConstU32<10>; - type KeyProvider = BitcoinVault; type ChainTracking = BitcoinChainTracking; } @@ -854,6 +852,10 @@ type PalletMigrations = ( pallet_cf_ingress_egress::migrations::PalletMigration, pallet_cf_swapping::migrations::PalletMigration, pallet_cf_lp::migrations::PalletMigration, + migrations::VersionedMigration< + migrations::threshold_signature_callbacks::ThresholdSignatureCallbacks, + 103, + >, pallet_cf_pools::migrations::PalletMigration, ); diff --git a/state-chain/runtime/src/migrations.rs b/state-chain/runtime/src/migrations.rs new file mode 100644 index 0000000000..891b0b68ea --- /dev/null +++ b/state-chain/runtime/src/migrations.rs @@ -0,0 +1,50 @@ +//! Chainflip runtime storage migrations. +use crate::System; +use frame_support::{traits::OnRuntimeUpgrade, weights::Weight}; +use sp_std::marker::PhantomData; + +#[cfg(feature = "try-runtime")] +use sp_std::{vec, vec::Vec}; +pub mod threshold_signature_callbacks; + +#[cfg(feature = "try-runtime")] +use sp_runtime::DispatchError; + +/// A runtime storage migration that will only be applied if the `SPEC_VERSION` matches the +/// post-upgrade runtime's spec version. +pub struct VersionedMigration(PhantomData); + +impl OnRuntimeUpgrade for VersionedMigration +where + U: OnRuntimeUpgrade, +{ + fn on_runtime_upgrade() -> Weight { + if System::runtime_version().spec_version == SPEC_VERSION { + U::on_runtime_upgrade() + } else { + log::info!( + "Skipping storage migration for version {:?} - consider removing this from the runtime.", + SPEC_VERSION + ); + Weight::zero() + } + } + + #[cfg(feature = "try-runtime")] + fn pre_upgrade() -> Result, DispatchError> { + if System::runtime_version().spec_version == SPEC_VERSION { + U::pre_upgrade() + } else { + Ok(vec![]) + } + } + + #[cfg(feature = "try-runtime")] + fn post_upgrade(state: Vec) -> Result<(), DispatchError> { + if System::runtime_version().spec_version == SPEC_VERSION { + U::post_upgrade(state) + } else { + Ok(()) + } + } +} diff --git a/state-chain/runtime/src/migrations/threshold_signature_callbacks.rs b/state-chain/runtime/src/migrations/threshold_signature_callbacks.rs new file mode 100644 index 0000000000..bfddcecbe7 --- /dev/null +++ b/state-chain/runtime/src/migrations/threshold_signature_callbacks.rs @@ -0,0 +1,293 @@ +use crate::{BitcoinInstance, Box, EthereumInstance, PolkadotInstance, Runtime, RuntimeCall}; +use codec::{Decode, Encode}; +use pallet_cf_broadcast::Call; +use scale_info::TypeInfo; +use sp_std::vec::Vec; + +use frame_support::{traits::OnRuntimeUpgrade, weights::Weight}; +use pallet_cf_broadcast::{BroadcastAttemptId, RequestSuccessCallbacks}; + +#[cfg(feature = "try-runtime")] +use sp_runtime::DispatchError; + +pub mod old { + use super::{Decode, Encode, TypeInfo, Vec}; + use cf_chains::{ + btc::{api::BitcoinApi, PreviousOrCurrent, SigningPayload}, + dot::{api::PolkadotApi, EncodedPolkadotPayload}, + eth::api::EthereumApi, + }; + use cf_primitives::{BroadcastId, ThresholdSignatureRequestId}; + use frame_support::Twox64Concat; + use sp_core::H256; + + use crate::chainflip::{BtcEnvironment, DotEnvironment, EthEnvironment}; + + #[derive(Debug, TypeInfo, Decode, Encode, Clone, PartialEq, Eq)] + #[allow(non_camel_case_types)] + pub enum EthereumBroadaster { + #[codec(index = 1u8)] + on_signature_ready { + threshold_request_id: ThresholdSignatureRequestId, + threshold_signature_payload: H256, + api_call: EthereumApi, + broadcast_id: BroadcastId, + initiated_at: u64, + }, + } + + #[derive(Debug, TypeInfo, Decode, Encode, Clone, PartialEq, Eq)] + #[allow(non_camel_case_types)] + pub enum PolkadotBroadaster { + #[codec(index = 1u8)] + on_signature_ready { + threshold_request_id: ThresholdSignatureRequestId, + threshold_signature_payload: EncodedPolkadotPayload, + api_call: PolkadotApi, + broadcast_id: BroadcastId, + initiated_at: u32, + }, + } + + #[derive(Debug, TypeInfo, Decode, Encode, Clone, PartialEq, Eq)] + #[allow(non_camel_case_types)] + pub enum BitcoinBroadaster { + #[codec(index = 1u8)] + on_signature_ready { + threshold_request_id: ThresholdSignatureRequestId, + threshold_signature_payload: Vec<(PreviousOrCurrent, SigningPayload)>, + api_call: BitcoinApi, + broadcast_id: BroadcastId, + initiated_at: u64, + }, + } + #[derive(Debug, Decode, Encode, Clone, PartialEq, Eq, TypeInfo)] + pub enum RuntimeCall { + #[codec(index = 27u8)] + EthereumBroadcaster(EthereumBroadaster), + #[codec(index = 28u8)] + PolkadotBroadaster(PolkadotBroadaster), + #[codec(index = 29u8)] + BitcoinBroadaster(BitcoinBroadaster), + } + + impl RuntimeCall { + pub fn unwrap_eth( + self, + ) -> (ThresholdSignatureRequestId, H256, EthereumApi, BroadcastId, u64) { + match self { + Self::EthereumBroadcaster(b) => match b { + EthereumBroadaster::on_signature_ready { + threshold_request_id, + threshold_signature_payload, + api_call, + broadcast_id, + initiated_at, + } => ( + threshold_request_id, + threshold_signature_payload, + api_call, + broadcast_id, + initiated_at, + ), + }, + _ => unreachable!(), + } + } + + pub fn unwrap_dot( + self, + ) -> ( + ThresholdSignatureRequestId, + EncodedPolkadotPayload, + PolkadotApi, + BroadcastId, + u32, + ) { + match self { + Self::PolkadotBroadaster(b) => match b { + PolkadotBroadaster::on_signature_ready { + threshold_request_id, + threshold_signature_payload, + api_call, + broadcast_id, + initiated_at, + } => ( + threshold_request_id, + threshold_signature_payload, + api_call, + broadcast_id, + initiated_at, + ), + }, + _ => unreachable!(), + } + } + + pub fn unwrap_btc( + self, + ) -> ( + ThresholdSignatureRequestId, + Vec<(PreviousOrCurrent, SigningPayload)>, + BitcoinApi, + BroadcastId, + u64, + ) { + match self { + Self::BitcoinBroadaster(b) => match b { + BitcoinBroadaster::on_signature_ready { + threshold_request_id, + threshold_signature_payload, + api_call, + broadcast_id, + initiated_at, + } => ( + threshold_request_id, + threshold_signature_payload, + api_call, + broadcast_id, + initiated_at, + ), + }, + _ => unreachable!(), + } + } + } + + #[frame_support::storage_alias] + pub type RequestCallbacks, I: 'static> = + StorageMap, Twox64Concat, BroadcastId, RuntimeCall>; +} + +pub struct ThresholdSignatureCallbacks; +impl OnRuntimeUpgrade for ThresholdSignatureCallbacks { + fn on_runtime_upgrade() -> Weight { + use frame_support::storage::StoragePrefixedMap; + frame_support::migration::move_prefix( + old::RequestCallbacks::::storage_prefix(), + RequestSuccessCallbacks::::storage_prefix(), + ); + frame_support::migration::move_prefix( + old::RequestCallbacks::::storage_prefix(), + RequestSuccessCallbacks::::storage_prefix(), + ); + frame_support::migration::move_prefix( + old::RequestCallbacks::::storage_prefix(), + RequestSuccessCallbacks::::storage_prefix(), + ); + + RequestSuccessCallbacks::::translate( + |_k, v: old::RuntimeCall| { + let c = v.unwrap_eth(); + Some(RuntimeCall::EthereumBroadcaster(Call::on_signature_ready { + threshold_request_id: c.0, + threshold_signature_payload: c.1, + api_call: Box::new(c.2), + broadcast_attempt_id: BroadcastAttemptId { + broadcast_id: c.3, + attempt_count: 0, + }, + initiated_at: c.4, + should_broadcast: true, + })) + }, + ); + + RequestSuccessCallbacks::::translate( + |_k, v: old::RuntimeCall| { + let c = v.unwrap_dot(); + Some(RuntimeCall::PolkadotBroadcaster(Call::on_signature_ready { + threshold_request_id: c.0, + threshold_signature_payload: c.1, + api_call: Box::new(c.2), + broadcast_attempt_id: BroadcastAttemptId { + broadcast_id: c.3, + attempt_count: 0, + }, + initiated_at: c.4, + should_broadcast: true, + })) + }, + ); + + RequestSuccessCallbacks::::translate( + |_k, v: old::RuntimeCall| { + let c = v.unwrap_btc(); + Some(RuntimeCall::BitcoinBroadcaster(Call::on_signature_ready { + threshold_request_id: c.0, + threshold_signature_payload: c.1, + api_call: Box::new(c.2), + broadcast_attempt_id: BroadcastAttemptId { + broadcast_id: c.3, + attempt_count: 0, + }, + initiated_at: c.4, + should_broadcast: true, + })) + }, + ); + + Weight::default() + } + + #[cfg(feature = "try-runtime")] + fn pre_upgrade() -> Result, DispatchError> { + let mut eth_broadcastids = old::RequestCallbacks::::iter() + .map(|(k, v)| (k, v.unwrap_eth().3)) + .collect::>(); + let mut dot_broadcastids = old::RequestCallbacks::::iter() + .map(|(k, v): (u32, old::RuntimeCall)| (k, v.unwrap_dot().3)) + .collect::>(); + let mut btc_broadcastids = old::RequestCallbacks::::iter() + .map(|(k, v): (u32, old::RuntimeCall)| (k, v.unwrap_btc().3)) + .collect::>(); + + eth_broadcastids.append(&mut dot_broadcastids); + eth_broadcastids.append(&mut btc_broadcastids); + Ok(eth_broadcastids.encode()) + } + + #[cfg(feature = "try-runtime")] + fn post_upgrade(state: Vec) -> Result<(), DispatchError> { + use pallet_cf_vaults::ensure_variant; + + let old_storage = >::decode(&mut &state[..]).unwrap(); + + let mut eth_broadcastids = RequestSuccessCallbacks::::iter() + .map(|(k, v)| { + let call = ensure_variant!(RuntimeCall::EthereumBroadcaster(call) => call, v, DispatchError::Other("..")); + let broadcast_attempt_id = ensure_variant!(Call::on_signature_ready{broadcast_attempt_id, ..} => broadcast_attempt_id, call, DispatchError::Other("..")); + assert_eq!(broadcast_attempt_id.attempt_count, 0); + Ok((k, broadcast_attempt_id.broadcast_id)) + }) + .collect::, DispatchError>>()?; + + let mut dot_broadcastids = RequestSuccessCallbacks::::iter() + .map(|(k, v)| { + let call = ensure_variant!(RuntimeCall::PolkadotBroadcaster(call) => call, v, DispatchError::Other("..")); + let broadcast_attempt_id = ensure_variant!(Call::on_signature_ready{broadcast_attempt_id, ..} => broadcast_attempt_id, call, DispatchError::Other("..")); + assert_eq!(broadcast_attempt_id.attempt_count, 0); + Ok((k, broadcast_attempt_id.broadcast_id)) + }) + .collect::, DispatchError>>()?; + + let mut btc_broadcastids = RequestSuccessCallbacks::::iter() + .map(|(k, v)| { + let call = ensure_variant!(RuntimeCall::BitcoinBroadcaster(call) => call, v, DispatchError::Other("..")); + let broadcast_attempt_id = ensure_variant!(Call::on_signature_ready{broadcast_attempt_id, ..} => broadcast_attempt_id, call, DispatchError::Other("..")); + assert_eq!(broadcast_attempt_id.attempt_count, 0); + Ok((k, broadcast_attempt_id.broadcast_id)) + }) + .collect::, DispatchError>>()?; + + eth_broadcastids.append(&mut dot_broadcastids); + eth_broadcastids.append(&mut btc_broadcastids); + + for i in 0..eth_broadcastids.len() { + assert_eq!(eth_broadcastids[i].0, old_storage[i].0); + assert_eq!(eth_broadcastids[i].1, old_storage[i].1); + } + + Ok(()) + } +} diff --git a/state-chain/traits/src/lib.rs b/state-chain/traits/src/lib.rs index 682e677315..78587a3e93 100644 --- a/state-chain/traits/src/lib.rs +++ b/state-chain/traits/src/lib.rs @@ -362,41 +362,10 @@ pub trait ThresholdSignerNomination { ) -> Option>; } -#[derive(Debug, TypeInfo, Decode, Encode, Clone, Copy, PartialEq, Eq)] -pub enum KeyState { - Unlocked, - /// Key is only available to sign this request id. - Locked(ThresholdSignatureRequestId), -} - -impl KeyState { - pub fn is_available_for_request(&self, request_id: ThresholdSignatureRequestId) -> bool { - match self { - KeyState::Unlocked => true, - KeyState::Locked(locked_request_id) => request_id == *locked_request_id, - } - } - - pub fn unlock(&mut self) { - *self = KeyState::Unlocked; - } - - pub fn lock(&mut self, request_id: ThresholdSignatureRequestId) { - *self = KeyState::Locked(request_id); - } -} - #[derive(Debug, TypeInfo, Decode, Encode, Clone, Copy, PartialEq, Eq)] pub struct EpochKey { pub key: Key, pub epoch_index: EpochIndex, - pub key_state: KeyState, -} - -impl EpochKey { - pub fn lock_for_request(&mut self, request_id: ThresholdSignatureRequestId) { - self.key_state = KeyState::Locked(request_id); - } } /// Provides the currently valid key for multisig ceremonies. @@ -409,7 +378,7 @@ pub trait KeyProvider { fn active_epoch_key() -> Option>; #[cfg(feature = "runtime-benchmarks")] - fn set_key(_key: C::AggKey) { + fn set_key(_key: C::AggKey, _epoch: EpochIndex) { unimplemented!() } } @@ -487,9 +456,7 @@ pub trait Broadcaster { type Callback: UnfilteredDispatchable; /// Request a threshold signature and then build and broadcast the outbound api call. - fn threshold_sign_and_broadcast( - api_call: Self::ApiCall, - ) -> (BroadcastId, ThresholdSignatureRequestId); + fn threshold_sign_and_broadcast(api_call: Self::ApiCall) -> BroadcastId; /// Like `threshold_sign_and_broadcast` but also registers a callback to be dispatched when the /// signature accepted event has been witnessed. @@ -497,7 +464,11 @@ pub trait Broadcaster { api_call: Self::ApiCall, success_callback: Option, failed_callback_generator: impl FnOnce(BroadcastId) -> Option, - ) -> (BroadcastId, ThresholdSignatureRequestId); + ) -> BroadcastId; + + /// Request a threshold signature and then build and broadcast the outbound api call + /// specifically for a rotation tx.. + fn threshold_sign_and_broadcast_rotation_tx(api_call: Self::ApiCall) -> BroadcastId; /// Resign a call, and update the signature data storage, but do not broadcast. fn threshold_resign(broadcast_id: BroadcastId) -> Option; diff --git a/state-chain/traits/src/mocks/broadcaster.rs b/state-chain/traits/src/mocks/broadcaster.rs index f714049a65..8b54930347 100644 --- a/state-chain/traits/src/mocks/broadcaster.rs +++ b/state-chain/traits/src/mocks/broadcaster.rs @@ -27,36 +27,32 @@ impl< type ApiCall = A; type Callback = C; - fn threshold_sign_and_broadcast( - api_call: Self::ApiCall, - ) -> (cf_primitives::BroadcastId, cf_primitives::ThresholdSignatureRequestId) { + fn threshold_sign_and_broadcast(api_call: Self::ApiCall) -> cf_primitives::BroadcastId { Self::mutate_value(b"API_CALLS", |api_calls: &mut Option>| { let api_calls = api_calls.get_or_insert(Default::default()); api_calls.push(api_call); }); - ( - ::mutate_value(b"BROADCAST_ID", |v: &mut Option| { - let v = v.get_or_insert(0); - *v += 1; - *v - }), - Self::next_threshold_id(), - ) + let _ = Self::next_threshold_id(); + ::mutate_value(b"BROADCAST_ID", |v: &mut Option| { + let v = v.get_or_insert(0); + *v += 1; + *v + }) } fn threshold_sign_and_broadcast_with_callback( api_call: Self::ApiCall, success_callback: Option, failed_callback_generator: impl FnOnce(BroadcastId) -> Option, - ) -> (BroadcastId, ThresholdSignatureRequestId) { - let ids @ (id, _) = >::threshold_sign_and_broadcast(api_call); + ) -> BroadcastId { + let id = >::threshold_sign_and_broadcast(api_call); if let Some(callback) = success_callback { Self::put_storage(b"SUCCESS_CALLBACKS", id, callback); } if let Some(callback) = failed_callback_generator(id) { Self::put_storage(b"FAILED_CALLBACKS", id, callback); } - ids + id } fn threshold_resign(broadcast_id: BroadcastId) -> Option { @@ -66,6 +62,10 @@ impl< /// Clean up storage data related to a broadcast ID. fn clean_up_broadcast_storage(_broadcast_id: BroadcastId) {} + + fn threshold_sign_and_broadcast_rotation_tx(api_call: Self::ApiCall) -> BroadcastId { + >::threshold_sign_and_broadcast(api_call) + } } impl< diff --git a/state-chain/traits/src/mocks/key_provider.rs b/state-chain/traits/src/mocks/key_provider.rs index 4693a21e85..4b013ae0de 100644 --- a/state-chain/traits/src/mocks/key_provider.rs +++ b/state-chain/traits/src/mocks/key_provider.rs @@ -1,8 +1,7 @@ use cf_chains::ChainCrypto; -use cf_primitives::ThresholdSignatureRequestId; use super::{MockPallet, MockPalletStorage}; -use crate::{EpochKey, KeyState}; +use crate::EpochKey; use std::marker::PhantomData; #[derive(Default)] @@ -16,18 +15,7 @@ const EPOCH_KEY: &[u8] = b"EPOCH_KEY"; impl MockKeyProvider { pub fn add_key(key: C::AggKey) { - Self::put_value( - EPOCH_KEY, - EpochKey { key, epoch_index: Default::default(), key_state: KeyState::Unlocked }, - ); - } - - pub fn lock_key(request_id: ThresholdSignatureRequestId) { - Self::mutate_value::, _, _>(EPOCH_KEY, |maybe_key| { - if let Some(key) = maybe_key.as_mut() { - key.lock_for_request(request_id); - } - }); + Self::put_value(EPOCH_KEY, EpochKey { key, epoch_index: Default::default() }); } } diff --git a/state-chain/traits/src/mocks/signer_nomination.rs b/state-chain/traits/src/mocks/signer_nomination.rs index 9c92e7d490..673d789e36 100644 --- a/state-chain/traits/src/mocks/signer_nomination.rs +++ b/state-chain/traits/src/mocks/signer_nomination.rs @@ -45,6 +45,10 @@ impl MockNominator { THRESHOLD_NOMINEES.with(|cell| cell.borrow().clone()) } + pub fn reset_last_nominee() { + LAST_NOMINATED_INDEX.with(|cell| *cell.borrow_mut() = None); + } + pub fn set_nominees(nominees: Option>) { THRESHOLD_NOMINEES.with(|cell| *cell.borrow_mut() = nominees); }