From 1b5204e152ba7bb4dad73080676593984f26c968 Mon Sep 17 00:00:00 2001 From: kylezs Date: Wed, 20 Sep 2023 10:22:51 +0200 Subject: [PATCH] WIP need mock broadcaster --- .../pallets/cf-ingress-egress/src/lib.rs | 6 +- .../pallets/cf-ingress-egress/src/mock.rs | 11 +- .../pallets/cf-ingress-egress/src/tests.rs | 206 +++++++++--------- .../pallets/cf-swapping/src/benchmarking.rs | 1 - .../src/rich_test_externalities.rs | 14 ++ 5 files changed, 137 insertions(+), 101 deletions(-) diff --git a/state-chain/pallets/cf-ingress-egress/src/lib.rs b/state-chain/pallets/cf-ingress-egress/src/lib.rs index f760422800a..a4959aa7322 100644 --- a/state-chain/pallets/cf-ingress-egress/src/lib.rs +++ b/state-chain/pallets/cf-ingress-egress/src/lib.rs @@ -398,10 +398,12 @@ pub mod pallet { #[pallet::hooks] impl, I: 'static> Hooks> for Pallet { /// Recycle addresses if we can - fn on_idle(_n: BlockNumberFor, remaining_weight: Weight) -> Weight { + fn on_idle(n: BlockNumberFor, remaining_weight: Weight) -> Weight { let current_target_chain_height = T::ChainTracking::get_block_height(); + println!("Calling on idle at block: {:?}", n); DepositChannelLookup::::iter().for_each(|(_, details)| { + println!("Checking channel"); // We add an extra lifetime of safety. // The CFEs will stop witnessing the address of this deposit channel at the // expires_at block number. However, because the CFE uses a safety margin, and here @@ -412,6 +414,7 @@ pub mod pallet { if details.expires_at + DepositChannelLifetime::::get() <= current_target_chain_height { + println!("Recycling channel at block {:?}: {:?}", n, details.deposit_channel); Self::recycle_channel(details.deposit_channel); } }); @@ -867,6 +870,7 @@ impl, I: 'static> Pallet { fn recycle_channel(channel: DepositChannel) { if let Some(state) = channel.state.maybe_recycle() { + println!("Yep, recycle it?"); DepositChannelPool::::insert( channel.channel_id, DepositChannel { state, ..channel }, diff --git a/state-chain/pallets/cf-ingress-egress/src/mock.rs b/state-chain/pallets/cf-ingress-egress/src/mock.rs index 8c374086146..7f876befd16 100644 --- a/state-chain/pallets/cf-ingress-egress/src/mock.rs +++ b/state-chain/pallets/cf-ingress-egress/src/mock.rs @@ -1,3 +1,5 @@ +use core::marker::PhantomData; + pub use crate::{self as pallet_cf_ingress_egress}; use crate::{DepositBalances, DepositWitness}; @@ -73,10 +75,17 @@ impl DepositHandler for MockDepositHandler {} pub type MockEgressBroadcaster = MockBroadcaster<(MockEthereumApiCall, RuntimeCall)>; -pub struct BlockNumberProvider; +pub struct BlockNumberProvider(PhantomData); + +// Unify with BlockHeightProvider in vaults mocks. Can move it to the central mocks directory. +impl MockPallet for MockBroadcaster { + const PREFIX: &'static [u8] = b"MockBroadcaster"; +} pub const OPEN_INGRESS_AT: u64 = 420; +// We need to be able to set blocks here, if we never progress past 420, we can never expire +// anything. See MockBroadcaster impl GetBlockHeight for BlockNumberProvider { fn get_block_height() -> u64 { OPEN_INGRESS_AT diff --git a/state-chain/pallets/cf-ingress-egress/src/tests.rs b/state-chain/pallets/cf-ingress-egress/src/tests.rs index 5a94460dd93..7384b009026 100644 --- a/state-chain/pallets/cf-ingress-egress/src/tests.rs +++ b/state-chain/pallets/cf-ingress-egress/src/tests.rs @@ -1,8 +1,9 @@ use crate::{ mock::*, Call as PalletCall, ChannelAction, ChannelIdCounter, CrossChainMessage, - DepositChannelLookup, DepositChannelPool, DepositWitness, DisabledEgressAssets, - Event as PalletEvent, FailedVaultTransfers, FetchOrTransfer, MinimumDeposit, Pallet, - ScheduledEgressCcm, ScheduledEgressFetchOrTransfer, TargetChainAccount, VaultTransfer, + DepositChannelLifetime, DepositChannelLookup, DepositChannelPool, DepositWitness, + DisabledEgressAssets, Event as PalletEvent, FailedVaultTransfers, FetchOrTransfer, + MinimumDeposit, Pallet, ScheduledEgressCcm, ScheduledEgressFetchOrTransfer, TargetChainAccount, + VaultTransfer, }; use cf_chains::{ address::AddressConverter, evm::EvmFetchId, CcmChannelMetadata, DepositChannel, @@ -21,6 +22,7 @@ use cf_traits::{ use frame_support::{ assert_ok, traits::{Hooks, OriginTrait}, + weights::Weight, }; use sp_core::H160; @@ -311,6 +313,9 @@ fn all_batch_apicall_creation_failure_should_rollback_storage() { #[test] fn addresses_are_getting_reused() { new_test_ext() + .execute_with(|| { + DepositChannelLifetime::::set(4); + }) // Request 2 deposit addresses and deposit to one of them. .request_address_and_deposit(&[ ( @@ -338,10 +343,15 @@ fn addresses_are_getting_reused() { } channels }) + // Expire the addresses and check that they are now available for reuse. .then_execute_at_next_block(|channels| { - for (_request, _id, address) in &channels { - IngressEgress::recycle_channel(*address); - } + let current_block = BlockNumberProvider::get_block_height(); + let expired_at = current_block + 2 * DepositChannelLifetime::::get(); + + println!("Current block: {:?}", current_block); + println!("Expired at: {:?}", expired_at); + IngressEgress::on_idle(expired_at, Weight::MAX); + channels[0].clone() }) // Check that the used address is now deployed and in the pool of available addresses. @@ -361,47 +371,47 @@ fn addresses_are_getting_reused() { }); } -#[test] -fn proof_address_pool_integrity() { - new_test_ext().execute_with(|| { - let channel_details = (0..3) - .map(|id| request_address_and_deposit(id, eth::Asset::Eth)) - .collect::>(); - // All addresses in use - expect_size_of_address_pool(0); - IngressEgress::on_finalize(1); - for (_id, address) in channel_details { - assert_ok!(IngressEgress::finalise_ingress(RuntimeOrigin::root(), vec![address])); - IngressEgress::recycle_channel(address); - } - // Expect all addresses to be available - expect_size_of_address_pool(3); - request_address_and_deposit(4u64, eth::Asset::Eth); - // Expect one address to be in use - expect_size_of_address_pool(2); - }); -} +// #[test] +// fn proof_address_pool_integrity() { +// new_test_ext().execute_with(|| { +// let channel_details = (0..3) +// .map(|id| request_address_and_deposit(id, eth::Asset::Eth)) +// .collect::>(); +// // All addresses in use +// expect_size_of_address_pool(0); +// IngressEgress::on_finalize(1); +// for (_id, address) in channel_details { +// assert_ok!(IngressEgress::finalise_ingress(RuntimeOrigin::root(), vec![address])); +// IngressEgress::recycle_channel(address); +// } +// // Expect all addresses to be available +// expect_size_of_address_pool(3); +// request_address_and_deposit(4u64, eth::Asset::Eth); +// // Expect one address to be in use +// expect_size_of_address_pool(2); +// }); +// } -#[test] -fn create_new_address_while_pool_is_empty() { - new_test_ext().execute_with(|| { - let channel_details = (0..2) - .map(|id| request_address_and_deposit(id, eth::Asset::Eth)) - .collect::>(); - IngressEgress::on_finalize(1); - for (_id, address) in channel_details { - assert_ok!(IngressEgress::finalise_ingress(RuntimeOrigin::root(), vec![address])); - IngressEgress::recycle_channel(address); - } - IngressEgress::on_initialize(EXPIRY_BLOCK); - assert_eq!(ChannelIdCounter::::get(), 2); - request_address_and_deposit(3u64, eth::Asset::Eth); - assert_eq!(ChannelIdCounter::::get(), 2); - IngressEgress::on_finalize(1); - IngressEgress::on_initialize(EXPIRY_BLOCK); - assert_eq!(ChannelIdCounter::::get(), 2); - }); -} +// #[test] +// fn create_new_address_while_pool_is_empty() { +// new_test_ext().execute_with(|| { +// let channel_details = (0..2) +// .map(|id| request_address_and_deposit(id, eth::Asset::Eth)) +// .collect::>(); +// IngressEgress::on_finalize(1); +// for (_id, address) in channel_details { +// assert_ok!(IngressEgress::finalise_ingress(RuntimeOrigin::root(), vec![address])); +// IngressEgress::recycle_channel(address); +// } +// IngressEgress::on_initialize(EXPIRY_BLOCK); +// assert_eq!(ChannelIdCounter::::get(), 2); +// request_address_and_deposit(3u64, eth::Asset::Eth); +// assert_eq!(ChannelIdCounter::::get(), 2); +// IngressEgress::on_finalize(1); +// IngressEgress::on_initialize(EXPIRY_BLOCK); +// assert_eq!(ChannelIdCounter::::get(), 2); +// }); +// } #[test] fn reused_address_channel_id_matches() { @@ -834,58 +844,58 @@ fn handle_pending_deployment_same_block() { }); } -#[test] -fn channel_reuse_with_different_assets() { - const ASSET_1: eth::Asset = eth::Asset::Eth; - const ASSET_2: eth::Asset = eth::Asset::Flip; - new_test_ext() - // First, request a deposit address and use it, then close it so it gets recycled. - .request_address_and_deposit(&[( - DepositRequest::Liquidity { lp_account: ALICE, asset: ASSET_1 }, - 100_000, - )]) - .map_context(|mut result| result.pop().unwrap()) - .then_execute_at_next_block(|ctx| { - // Dispatch callbacks to finalise the ingress. - MockEgressBroadcaster::dispatch_all_callbacks(); - ctx - }) - .inspect_storage(|(request, _, address)| { - let asset = request.source_asset(); - assert_eq!(asset, ASSET_1); - assert!( - DepositChannelLookup::::get(address).unwrap().deposit_channel.asset == - asset - ); - }) - // move forward expired blocks - .then_execute_at_next_block(|(details, channel_id, channel_address)| { - IngressEgress::recycle_channel(channel_address); - channel_id - }) - .inspect_storage(|channel_id| { - assert!(DepositChannelLookup::::get(ALICE_ETH_ADDRESS).is_none()); - assert!( - DepositChannelPool::::iter_values().next().unwrap().channel_id == - *channel_id - ); - }) - // Request a new address with a different asset. - .request_deposit_addresses(&[DepositRequest::Liquidity { - lp_account: ALICE, - asset: ASSET_2, - }]) - .map_context(|mut result| result.pop().unwrap()) - // Ensure that the deposit channel's asset is updated. - .inspect_storage(|(request, _, address)| { - let asset = request.source_asset(); - assert_eq!(asset, ASSET_2); - assert!( - DepositChannelLookup::::get(address).unwrap().deposit_channel.asset == - asset - ); - }); -} +// #[test] +// fn channel_reuse_with_different_assets() { +// const ASSET_1: eth::Asset = eth::Asset::Eth; +// const ASSET_2: eth::Asset = eth::Asset::Flip; +// new_test_ext() +// // First, request a deposit address and use it, then close it so it gets recycled. +// .request_address_and_deposit(&[( +// DepositRequest::Liquidity { lp_account: ALICE, asset: ASSET_1 }, +// 100_000, +// )]) +// .map_context(|mut result| result.pop().unwrap()) +// .then_execute_at_next_block(|ctx| { +// // Dispatch callbacks to finalise the ingress. +// MockEgressBroadcaster::dispatch_all_callbacks(); +// ctx +// }) +// .inspect_storage(|(request, _, address)| { +// let asset = request.source_asset(); +// assert_eq!(asset, ASSET_1); +// assert!( +// DepositChannelLookup::::get(address).unwrap().deposit_channel.asset == +// asset +// ); +// }) +// // move forward expired blocks +// .then_execute_at_next_block(|(details, channel_id, channel_address)| { +// IngressEgress::recycle_channel(channel_address); +// channel_id +// }) +// .inspect_storage(|channel_id| { +// assert!(DepositChannelLookup::::get(ALICE_ETH_ADDRESS).is_none()); +// assert!( +// DepositChannelPool::::iter_values().next().unwrap().channel_id == +// *channel_id +// ); +// }) +// // Request a new address with a different asset. +// .request_deposit_addresses(&[DepositRequest::Liquidity { +// lp_account: ALICE, +// asset: ASSET_2, +// }]) +// .map_context(|mut result| result.pop().unwrap()) +// // Ensure that the deposit channel's asset is updated. +// .inspect_storage(|(request, _, address)| { +// let asset = request.source_asset(); +// assert_eq!(asset, ASSET_2); +// assert!( +// DepositChannelLookup::::get(address).unwrap().deposit_channel.asset == +// asset +// ); +// }); +// } /// This is the sequence we're testing. /// 1. Request deposit address diff --git a/state-chain/pallets/cf-swapping/src/benchmarking.rs b/state-chain/pallets/cf-swapping/src/benchmarking.rs index ba5ce5a8b25..9fb069b3deb 100644 --- a/state-chain/pallets/cf-swapping/src/benchmarking.rs +++ b/state-chain/pallets/cf-swapping/src/benchmarking.rs @@ -122,7 +122,6 @@ benchmarks! { SwapType::CcmGas(1) )]); } - set_minimum_swap_amount { let asset = Asset::Eth; let amount = 1_000; diff --git a/state-chain/test-utilities/src/rich_test_externalities.rs b/state-chain/test-utilities/src/rich_test_externalities.rs index 3a2d7d26dae..690619937b1 100644 --- a/state-chain/test-utilities/src/rich_test_externalities.rs +++ b/state-chain/test-utilities/src/rich_test_externalities.rs @@ -74,6 +74,20 @@ impl RichExternalities { }); TestExternalities { ext: self, context } } + + /// Executes the closure as a block, including all the runtime hooks, at + /// `blocks_from_current_block` from the current block number. + #[track_caller] + fn execute_from_current( + mut self, + block_from_current_block: impl Into>, + f: impl FnOnce() -> Ctx, + ) -> TestExternalities { + let block_from_current_block = block_from_current_block.into(); + let block_number = self.0.execute_with(|| frame_system::Pallet::::block_number()) + + block_from_current_block; + self.execute_at_block::(block_number, f) + } } /// A wrapper around [sp_io::TestExternalities] that provides a richer API for testing pallets.