diff --git a/state-chain/pallets/cf-broadcast/src/benchmarking.rs b/state-chain/pallets/cf-broadcast/src/benchmarking.rs index 73f0e42c07..e9564119a3 100644 --- a/state-chain/pallets/cf-broadcast/src/benchmarking.rs +++ b/state-chain/pallets/cf-broadcast/src/benchmarking.rs @@ -34,6 +34,10 @@ fn insert_transaction_broadcast_attempt, I: 'static>( ); } +const INITIATED_AT: u32 = 100; + +pub type AggKeyFor = <<>::TargetChain as cf_chains::Chain>::ChainCrypto as ChainCrypto>::AggKey; + // Generates a new signature ready call. fn generate_on_signature_ready_call, I>() -> pallet::Call { let threshold_request_id = 1; @@ -46,6 +50,7 @@ fn generate_on_signature_ready_call, I>() -> pallet::Call::benchmark_value(), api_call: Box::new(ApiCallFor::::benchmark_value()), broadcast_id: 1, + initiated_at: INITIATED_AT.into(), } } @@ -61,7 +66,7 @@ benchmarks_instance_pallet! { Timeouts::::append(timeout_block, broadcast_attempt_id); ThresholdSignatureData::::insert(i, (ApiCallFor::::benchmark_value(), ThresholdSignatureFor::::benchmark_value())) } - let valid_key = <<>::TargetChain as Chain>::ChainCrypto as ChainCrypto>::AggKey::benchmark_value(); + let valid_key = AggKeyFor::::benchmark_value(); T::KeyProvider::set_key(valid_key); } : { Pallet::::on_initialize(timeout_block); @@ -79,7 +84,7 @@ benchmarks_instance_pallet! { insert_transaction_broadcast_attempt::(caller.clone().into(), broadcast_attempt_id); 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 = <<>::TargetChain as Chain>::ChainCrypto as ChainCrypto>::AggKey::benchmark_value(); + let valid_key = AggKeyFor::::benchmark_value(); T::KeyProvider::set_key(valid_key); }: _(RawOrigin::Signed(caller), broadcast_attempt_id) verify { @@ -94,7 +99,7 @@ benchmarks_instance_pallet! { }; insert_transaction_broadcast_attempt::(whitelisted_caller(), broadcast_attempt_id); let call = generate_on_signature_ready_call::(); - let valid_key = <<>::TargetChain as Chain>::ChainCrypto as ChainCrypto>::AggKey::benchmark_value(); + let valid_key = AggKeyFor::::benchmark_value(); T::KeyProvider::set_key(valid_key); } : { call.dispatch_bypass_filter(T::EnsureThresholdSigned::try_successful_origin().unwrap())? } verify { @@ -110,10 +115,11 @@ benchmarks_instance_pallet! { BenchmarkValue::benchmark_value(), signed_api_call, BenchmarkValue::benchmark_value(), - 1 + 1, + INITIATED_AT.into(), ); - T::KeyProvider::set_key(<<>::TargetChain as Chain>::ChainCrypto as ChainCrypto>::AggKey::benchmark_value()); + T::KeyProvider::set_key(AggKeyFor::::benchmark_value()); let transaction_payload = TransactionFor::::benchmark_value(); } : { @@ -130,7 +136,8 @@ benchmarks_instance_pallet! { transaction_succeeded { let caller: T::AccountId = whitelisted_caller(); let signer_id = SignerIdFor::::benchmark_value(); - TransactionOutIdToBroadcastId::::insert(TransactionOutIdFor::::benchmark_value(), 1); + let initiated_at: ChainBlockNumberFor = INITIATED_AT.into(); + TransactionOutIdToBroadcastId::::insert(TransactionOutIdFor::::benchmark_value(), (1, initiated_at)); let broadcast_attempt_id = BroadcastAttemptId { broadcast_id: 1, @@ -142,7 +149,7 @@ benchmarks_instance_pallet! { signer_id, tx_fee: TransactionFeeFor::::benchmark_value(), }; - let valid_key = <<>::TargetChain as Chain>::ChainCrypto as ChainCrypto>::AggKey::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 { diff --git a/state-chain/pallets/cf-broadcast/src/lib.rs b/state-chain/pallets/cf-broadcast/src/lib.rs index fd3c0c0923..781f1ca971 100644 --- a/state-chain/pallets/cf-broadcast/src/lib.rs +++ b/state-chain/pallets/cf-broadcast/src/lib.rs @@ -6,9 +6,10 @@ mod benchmarking; mod mock; mod tests; +pub mod migrations; pub mod weights; use cf_primitives::{BroadcastId, ThresholdSignatureRequestId}; -use cf_traits::impl_pallet_safe_mode; +use cf_traits::{impl_pallet_safe_mode, GetBlockHeight}; pub use weights::WeightInfo; impl_pallet_safe_mode!(PalletSafeMode; retry_enabled); @@ -23,7 +24,7 @@ use frame_support::{ dispatch::DispatchResultWithPostInfo, pallet_prelude::DispatchResult, sp_runtime::traits::Saturating, - traits::{Get, UnfilteredDispatchable}, + traits::{Get, OnRuntimeUpgrade, StorageVersion, UnfilteredDispatchable}, Twox64Concat, }; @@ -66,6 +67,8 @@ pub enum PalletOffence { FailedToBroadcastTransaction, } +pub const PALLET_VERSION: StorageVersion = StorageVersion::new(1); + #[frame_support::pallet] pub mod pallet { use super::*; @@ -179,6 +182,9 @@ pub mod pallet { type BroadcastReadyProvider: OnBroadcastReady; + /// Get the latest block height of the target chain via Chain Tracking. + type ChainTracking: GetBlockHeight; + /// The timeout duration for the broadcast, measured in number of blocks. #[pallet::constant] type BroadcastTimeout: Get>; @@ -202,6 +208,7 @@ pub mod pallet { pub struct Origin, I: 'static = ()>(pub(super) PhantomData<(T, I)>); #[pallet::pallet] + #[pallet::storage_version(PALLET_VERSION)] #[pallet::without_storage_info] pub struct Pallet(PhantomData<(T, I)>); @@ -237,8 +244,13 @@ pub mod pallet { /// Lookup table between TransactionOutId -> Broadcast. #[pallet::storage] - pub type TransactionOutIdToBroadcastId, I: 'static = ()> = - StorageMap<_, Twox64Concat, TransactionOutIdFor, BroadcastId, OptionQuery>; + pub type TransactionOutIdToBroadcastId, I: 'static = ()> = StorageMap< + _, + Twox64Concat, + TransactionOutIdFor, + (BroadcastId, ChainBlockNumberFor), + OptionQuery, + >; /// The list of failed broadcasts pending retry. #[pallet::storage] @@ -368,6 +380,20 @@ pub mod pallet { Weight::zero() } } + + fn on_runtime_upgrade() -> Weight { + migrations::PalletMigration::::on_runtime_upgrade() + } + + #[cfg(feature = "try-runtime")] + fn pre_upgrade() -> Result, DispatchError> { + migrations::PalletMigration::::pre_upgrade() + } + + #[cfg(feature = "try-runtime")] + fn post_upgrade(state: sp_std::vec::Vec) -> Result<(), DispatchError> { + migrations::PalletMigration::::post_upgrade(state) + } } #[pallet::call] @@ -452,6 +478,7 @@ pub mod pallet { threshold_signature_payload: PayloadFor, api_call: Box<>::ApiCall>, broadcast_id: BroadcastId, + initiated_at: ChainBlockNumberFor, ) -> DispatchResultWithPostInfo { let _ = T::EnsureThresholdSigned::ensure_origin(origin)?; @@ -474,6 +501,7 @@ pub mod pallet { signed_api_call, threshold_signature_payload, broadcast_id, + initiated_at, ); Ok(().into()) } @@ -502,8 +530,9 @@ pub mod pallet { ) -> DispatchResultWithPostInfo { T::EnsureWitnessed::ensure_origin(origin.clone())?; - let broadcast_id = TransactionOutIdToBroadcastId::::take(&tx_out_id) - .ok_or(Error::::InvalidPayload)?; + let (broadcast_id, _initiated_at) = + TransactionOutIdToBroadcastId::::take(&tx_out_id) + .ok_or(Error::::InvalidPayload)?; let to_refund = AwaitingBroadcast::::get(BroadcastAttemptId { broadcast_id, @@ -616,6 +645,13 @@ impl, I: 'static> Pallet { if let Some(callback) = maybe_callback { RequestCallbacks::::insert(broadcast_id, callback); } + + // We must set this here because after the threshold signature is requested, it's + // possible that an authority submits the transaction themselves, not going through the + // standard path. This protects against that, to ensure we always set the earliest possible + // block number we could have broadcast at, so that we can ensure we witness it. + let initiated_at = T::ChainTracking::get_block_height(); + let threshold_signature_payload = api_call.threshold_signature_payload(); let signature_request_id = T::ThresholdSigner::request_signature_with_callback( threshold_signature_payload.clone(), @@ -625,6 +661,7 @@ impl, I: 'static> Pallet { threshold_signature_payload, api_call: Box::new(api_call), broadcast_id, + initiated_at, } .into() }, @@ -643,12 +680,16 @@ impl, I: 'static> Pallet { 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); + TransactionOutIdToBroadcastId::::insert( + &transaction_out_id, + (broadcast_id, initiated_at), + ); ThresholdSignatureData::::insert(broadcast_id, (api_call, signature)); diff --git a/state-chain/pallets/cf-broadcast/src/migrations.rs b/state-chain/pallets/cf-broadcast/src/migrations.rs new file mode 100644 index 0000000000..d7889e06e2 --- /dev/null +++ b/state-chain/pallets/cf-broadcast/src/migrations.rs @@ -0,0 +1,6 @@ +pub mod add_initiated_at; + +use cf_runtime_upgrade_utilities::VersionedMigration; + +pub type PalletMigration = + (VersionedMigration, add_initiated_at::Migration, 0, 1>,); diff --git a/state-chain/pallets/cf-broadcast/src/migrations/add_initiated_at.rs b/state-chain/pallets/cf-broadcast/src/migrations/add_initiated_at.rs new file mode 100644 index 0000000000..af2811d532 --- /dev/null +++ b/state-chain/pallets/cf-broadcast/src/migrations/add_initiated_at.rs @@ -0,0 +1,53 @@ +use crate::*; +#[cfg(feature = "try-runtime")] +use frame_support::dispatch::DispatchError; +use frame_support::{traits::OnRuntimeUpgrade, weights::Weight}; +use sp_std::marker::PhantomData; + +mod old { + use frame_support::pallet_prelude::OptionQuery; + + use super::*; + + #[frame_support::storage_alias] + pub type TransactionOutIdToBroadcastId, I: 'static> = + StorageMap, Twox64Concat, TransactionOutIdFor, BroadcastId, OptionQuery>; +} + +pub struct Migration, I: 'static>(PhantomData<(T, I)>); + +impl, I: 'static> OnRuntimeUpgrade for Migration { + fn on_runtime_upgrade() -> frame_support::weights::Weight { + let chain_height = T::ChainTracking::get_block_height(); + + TransactionOutIdToBroadcastId::::translate::(|_id, old| { + Some((old, chain_height)) + }); + + Weight::zero() + } + + #[cfg(feature = "try-runtime")] + fn pre_upgrade() -> Result, DispatchError> { + use frame_support::ensure; + + let chain_height = T::ChainTracking::get_block_height(); + // If it's at 0 something went wrong with the initialisation. Also since initiated_at is the + // last thing being decoded, this acts as a check that the rest of the decoding worked. + ensure!(chain_height > 0u32.into(), "chain_height is 0"); + Ok(chain_height.encode()) + } + + #[cfg(feature = "try-runtime")] + fn post_upgrade(state: Vec) -> Result<(), DispatchError> { + use frame_support::ensure; + + let pre_upgrade_height = ChainBlockNumberFor::::decode(&mut &state[..]) + .map_err(|_| "Failed to decode pre-upgrade state.")?; + + for (_out_id, (_b_id, initiated_at)) in TransactionOutIdToBroadcastId::::iter() { + ensure!(initiated_at >= pre_upgrade_height, "initiated_at is 0"); + } + Ok(()) + } +} diff --git a/state-chain/pallets/cf-broadcast/src/mock.rs b/state-chain/pallets/cf-broadcast/src/mock.rs index a3a52538c3..a56c03096c 100644 --- a/state-chain/pallets/cf-broadcast/src/mock.rs +++ b/state-chain/pallets/cf-broadcast/src/mock.rs @@ -13,7 +13,10 @@ use cf_chains::{ }; use cf_traits::{ impl_mock_chainflip, impl_mock_runtime_safe_mode, - mocks::{signer_nomination::MockNominator, threshold_signer::MockThresholdSigner}, + mocks::{ + block_height_provider::BlockHeightProvider, signer_nomination::MockNominator, + threshold_signer::MockThresholdSigner, + }, AccountRoleRegistry, EpochKey, KeyState, OnBroadcastReady, }; use codec::{Decode, Encode}; @@ -152,6 +155,7 @@ impl pallet_cf_broadcast::Config for Test { type SafeMode = MockRuntimeSafeMode; type BroadcastReadyProvider = MockBroadcastReadyProvider; type SafeModeBlockMargin = ConstU64<10>; + type ChainTracking = BlockHeightProvider; } impl_mock_chainflip!(Test); diff --git a/state-chain/pallets/cf-broadcast/src/tests.rs b/state-chain/pallets/cf-broadcast/src/tests.rs index 87ea7dd75b..5a1f8772da 100644 --- a/state-chain/pallets/cf-broadcast/src/tests.rs +++ b/state-chain/pallets/cf-broadcast/src/tests.rs @@ -128,6 +128,7 @@ fn start_mock_broadcast_tx_out_id( MockApiCall { tx_out_id, payload: Default::default(), sig: Default::default() }, Default::default(), 1, + 100u64, ) } diff --git a/state-chain/runtime/src/lib.rs b/state-chain/runtime/src/lib.rs index 83ab32f5a6..3d11882869 100644 --- a/state-chain/runtime/src/lib.rs +++ b/state-chain/runtime/src/lib.rs @@ -682,6 +682,7 @@ impl pallet_cf_broadcast::Config for Runtime { type SafeMode = RuntimeSafeMode; type SafeModeBlockMargin = ConstU32<10>; type KeyProvider = EthereumVault; + type ChainTracking = EthereumChainTracking; } impl pallet_cf_broadcast::Config for Runtime { @@ -704,6 +705,7 @@ impl pallet_cf_broadcast::Config for Runtime { type SafeMode = RuntimeSafeMode; type SafeModeBlockMargin = ConstU32<10>; type KeyProvider = PolkadotVault; + type ChainTracking = PolkadotChainTracking; } impl pallet_cf_broadcast::Config for Runtime { @@ -726,6 +728,7 @@ impl pallet_cf_broadcast::Config for Runtime { type SafeMode = RuntimeSafeMode; type SafeModeBlockMargin = ConstU32<10>; type KeyProvider = BitcoinVault; + type ChainTracking = BitcoinChainTracking; } impl pallet_cf_chain_tracking::Config for Runtime {