From dc8bf3e6e043696a3dca41a33ed2bfc8bf7632fe Mon Sep 17 00:00:00 2001 From: Janislav Date: Thu, 24 Oct 2024 14:11:00 +0200 Subject: [PATCH] feat: tainted transaction reporting (#5310) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat: Handling of tainted transactions - Extended DepositChannelDetails with owner field - Added extrinsic to mark transaction as tainted - Handling deposit and save details for a refund if tx is tainted * tests: Added tests - Added tests to verify that tainted transactions can get detected for all possible swap types - Added tests to check that txs marked by other brokers are getting ignored * chore: Extended LpRegistration trait + Added to Ingress/Egress config * feat: Getting LP refund address * feature: ensure by lp and broker + added more tests * refactor: Don't error if tx is tainted * refactor: Using DoubleMap instead of Map * refactor: Using BadOrigin + Added unit test * refactor: Inline code + Add Deposit Witness to struct * refactor: Extended lp deposit with refund address - Taking refund address if we open a deposit channel as lp - Extended ChannelAction to take an optional refund address - Removed all dependencies regarding the LpRegistration trait (added last commit) - Refactored tests, benchmarks, etc * feature: Added benchmark * chore: Changes to benchmark * chore: generated mock weights * chore: only allow mark transactions for BTC * feature: expire tainted transaction * test: refactored tests * chore: Removed unused events * chore: Ensure only by broker * chore: removed broker from tainted tx struct * chore: Only clone owner * chore: Moved tx tainted check * feature: Added migration for DepositChannelLookup * refactor: Changed data structure + fixed migrations * chore: Handle LP refund address as requirement * chore: Made clippy happy ๐Ÿ™‚ * chore: don't manipulate storage in place in iteration ๐Ÿ™…โ€โ™‚๏ธ * test: Added migration test ๐Ÿงช * chore: changed pallet storage version ๐Ÿ“€ * chore: bumped pallet storage version (again) * refactor: Changed accounting of expired transactions * refactor: using translate for migration * refactor: using append, refactored test * feature: Added handling of boost channels * feature: Marking txs when prewitness and reject when we process the depo * feat: pre-witnessed rejection handling * chore: Fixed logic + added tests * tests: Refactor/Rearranged tests * chore: Using SECONDS_PER_BLOCK instead of static block seconds * chore: Addressed comments * chore: Fixed clippy in CI * chore: update comments * fix: don't allow report overwrite * chore: Renamed event * feat: improvements: - mark boosted transactions as boosted instead of using channel status - allow pallet config instead of relying on chain - add event for tx reports - only allow reporting of unseen transactions - add doc comments - renaming of types/events - remove unused error * fix: migration * fix: made clippy happy --------- Co-authored-by: Daniel Co-authored-by: Maxim Shishmarev --- engine/src/witness/btc/deposits.rs | 3 + .../cf-ingress-egress/src/benchmarking.rs | 38 +- .../pallets/cf-ingress-egress/src/lib.rs | 218 +++++++++- .../cf-ingress-egress/src/migrations.rs | 8 +- .../add_owner_to_channel_details.rs | 183 +++++++++ .../pallets/cf-ingress-egress/src/mock_btc.rs | 2 + .../pallets/cf-ingress-egress/src/mock_eth.rs | 18 +- .../pallets/cf-ingress-egress/src/tests.rs | 376 ++++++++++++++++-- .../cf-ingress-egress/src/tests/boost.rs | 9 +- .../pallets/cf-ingress-egress/src/weights.rs | 19 + state-chain/pallets/cf-lp/src/lib.rs | 40 +- state-chain/runtime/src/chainflip.rs | 6 +- state-chain/runtime/src/lib.rs | 5 + state-chain/traits/src/lib.rs | 1 + state-chain/traits/src/mocks/balance_api.rs | 5 +- .../traits/src/mocks/deposit_handler.rs | 1 + 16 files changed, 857 insertions(+), 75 deletions(-) create mode 100644 state-chain/pallets/cf-ingress-egress/src/migrations/add_owner_to_channel_details.rs diff --git a/engine/src/witness/btc/deposits.rs b/engine/src/witness/btc/deposits.rs index ba5734914e..881928b29c 100644 --- a/engine/src/witness/btc/deposits.rs +++ b/engine/src/witness/btc/deposits.rs @@ -193,7 +193,9 @@ pub mod tests { fn fake_details( deposit_address: DepositAddress, ) -> DepositChannelDetails { + use cf_chains::{btc::ScriptPubkey, ForeignChainAddress}; DepositChannelDetails::<_, BitcoinInstance> { + owner: AccountId32::new([0xab; 32]), opened_at: 1, expires_at: 10, deposit_channel: DepositChannel { @@ -204,6 +206,7 @@ pub mod tests { }, action: ChannelAction::::LiquidityProvision { lp_account: AccountId32::new([0xab; 32]), + refund_address: Some(ForeignChainAddress::Btc(ScriptPubkey::P2PKH([0; 20]))), }, boost_fee: 0, boost_status: BoostStatus::NotBoosted, diff --git a/state-chain/pallets/cf-ingress-egress/src/benchmarking.rs b/state-chain/pallets/cf-ingress-egress/src/benchmarking.rs index 9af3401b1f..e1804dab57 100644 --- a/state-chain/pallets/cf-ingress-egress/src/benchmarking.rs +++ b/state-chain/pallets/cf-ingress-egress/src/benchmarking.rs @@ -7,6 +7,7 @@ use cf_chains::{ benchmarking_value::{BenchmarkValue, BenchmarkValueExtended}, DepositChannel, }; +use cf_primitives::AccountRole; use cf_traits::AccountRoleRegistry; use frame_benchmarking::v2::*; use frame_support::{ @@ -61,6 +62,7 @@ mod benchmarks { DepositChannelLookup::::insert( &deposit_address, DepositChannelDetails { + owner: account("doogle", 0, 0), opened_at: block_number, expires_at: block_number, deposit_channel: @@ -71,6 +73,7 @@ mod benchmarks { .unwrap(), action: ChannelAction::::LiquidityProvision { lp_account: account("doogle", 0, 0), + refund_address: None, }, boost_fee: 0, boost_status: BoostStatus::NotBoosted, @@ -102,6 +105,7 @@ mod benchmarks { let block_number = TargetChainBlockNumber::::benchmark_value(); let mut channel = DepositChannelDetails:: { + owner: account("doogle", 0, 0), opened_at: block_number, expires_at: block_number, deposit_channel: DepositChannel::generate_new::< @@ -110,6 +114,7 @@ mod benchmarks { .unwrap(), action: ChannelAction::::LiquidityProvision { lp_account: account("doogle", 0, 0), + refund_address: None, }, boost_fee: 0, boost_status: BoostStatus::NotBoosted, @@ -227,7 +232,10 @@ mod benchmarks { let (_channel_id, deposit_address, ..) = Pallet::::open_channel( lp_account, asset, - ChannelAction::LiquidityProvision { lp_account: lp_account.clone() }, + ChannelAction::LiquidityProvision { + lp_account: lp_account.clone(), + refund_address: None, + }, fee_tier, ) .unwrap(); @@ -401,7 +409,10 @@ mod benchmarks { let (_channel_id, deposit_address, ..) = Pallet::::open_channel( &boosters[0], asset, - ChannelAction::LiquidityProvision { lp_account: boosters[0].clone() }, + ChannelAction::LiquidityProvision { + lp_account: boosters[0].clone(), + refund_address: None, + }, TIER_5_BPS, ) .unwrap(); @@ -506,6 +517,26 @@ mod benchmarks { assert_eq!(BoostPools::::iter().count(), 1); } + #[benchmark] + fn mark_transaction_as_tainted() { + let caller = + T::AccountRoleRegistry::whitelisted_caller_with_role(AccountRole::Broker).unwrap(); + let tx_id = <>::TargetChain as Chain>::DepositDetails::benchmark_value(); + + #[block] + { + assert_ok!(Pallet::::mark_transaction_as_tainted_inner( + caller.clone(), + tx_id.clone(), + )); + } + + assert!( + TaintedTransactions::::get(caller, tx_id).is_some(), + "No tainted transactions found" + ); + } + #[cfg(test)] use crate::mock_eth::*; @@ -550,5 +581,8 @@ mod benchmarks { new_test_ext().execute_with(|| { _contract_ccm_swap_request::(true); }); + new_test_ext().execute_with(|| { + _mark_transaction_as_tainted::(true); + }); } } diff --git a/state-chain/pallets/cf-ingress-egress/src/lib.rs b/state-chain/pallets/cf-ingress-egress/src/lib.rs index 2f32808296..4b49230dd6 100644 --- a/state-chain/pallets/cf-ingress-egress/src/lib.rs +++ b/state-chain/pallets/cf-ingress-egress/src/lib.rs @@ -34,7 +34,7 @@ use cf_chains::{ use cf_primitives::{ Asset, AssetAmount, BasisPoints, Beneficiaries, BoostPoolTier, BroadcastId, ChannelId, DcaParameters, EgressCounter, EgressId, EpochIndex, ForeignChain, PrewitnessedDepositId, - SwapRequestId, ThresholdSignatureRequestId, TransactionHash, + SwapRequestId, ThresholdSignatureRequestId, TransactionHash, SECONDS_PER_BLOCK, }; use cf_runtime_utilities::log_or_panic; use cf_traits::{ @@ -65,6 +65,8 @@ use sp_std::{ }; pub use weights::WeightInfo; +const TAINTED_TX_EXPIRATION_BLOCKS: u32 = 3600 / SECONDS_PER_BLOCK as u32; + #[derive(Clone, Debug, PartialEq, Eq, Encode, Decode, TypeInfo)] pub enum BoostStatus { // If a (pre-witnessed) deposit on a channel has been boosted, we record @@ -77,6 +79,17 @@ pub enum BoostStatus { NotBoosted, } +#[derive(Clone, Debug, PartialEq, Eq, Encode, Decode, TypeInfo, Default)] +pub enum TaintedTransactionStatus { + /// Transaction was boosted, can't be rejected. + Boosted, + /// Transaction was prewitnessed but not boosted due to being reported. + Prewitnessed, + /// Transaction has been reported but not neither prewitnessed nor boosted. + #[default] + Unseen, +} + #[derive(Clone, Debug, PartialEq, Eq, Encode, Decode, TypeInfo)] pub struct BoostPoolId { asset: C::ChainAsset, @@ -121,6 +134,17 @@ pub enum DepositIgnoredReason { /// The deposit was ignored because the amount provided was not high enough to pay for the fees /// required to process the requisite transactions. NotEnoughToPayFees, + TransactionTainted, +} + +/// Holds information about a tainted transaction. +#[derive(RuntimeDebug, PartialEq, Eq, Encode, Decode, TypeInfo, CloneNoBound)] +#[scale_info(skip_type_params(T, I))] +pub struct TaintedTransactionDetails, I: 'static> { + pub refund_address: Option, + pub asset: TargetChainAsset, + pub amount: TargetChainAmount, + pub tx_id: ::DepositDetails, } /// Cross-chain messaging requests. @@ -145,7 +169,7 @@ impl CrossChainMessage { } } -pub const PALLET_VERSION: StorageVersion = StorageVersion::new(15); +pub const PALLET_VERSION: StorageVersion = StorageVersion::new(16); impl_pallet_safe_mode! { PalletSafeMode; @@ -253,6 +277,8 @@ pub mod pallet { #[derive(CloneNoBound, RuntimeDebug, PartialEq, Eq, Encode, Decode, TypeInfo)] #[scale_info(skip_type_params(T, I))] pub struct DepositChannelDetails, I: 'static> { + /// The owner of the deposit channel. + pub owner: T::AccountId, pub deposit_channel: DepositChannel, /// The block number at which the deposit channel was opened, expressed as a block number /// on the external Chain. @@ -260,7 +286,6 @@ pub mod pallet { /// The last block on the target chain that the witnessing will witness it in. If funds are /// sent after this block, they will not be witnessed. pub expires_at: TargetChainBlockNumber, - /// The action to be taken when the DepositChannel is deposited to. pub action: ChannelAction, /// The boost fee @@ -291,6 +316,7 @@ pub mod pallet { }, LiquidityProvision { lp_account: AccountId, + refund_address: Option, }, CcmTransfer { destination_asset: Asset, @@ -421,6 +447,9 @@ pub mod pallet { /// For checking if the CCM message passed in is valid. type CcmValidityChecker: CcmValidityCheck; + + #[pallet::constant] + type AllowTransactionReports: Get; } /// Lookup table for addresses to corresponding deposit channels. @@ -525,6 +554,33 @@ pub mod pallet { pub type PrewitnessedDepositIdCounter, I: 'static = ()> = StorageValue<_, PrewitnessedDepositId, ValueQuery>; + /// Stores the reporter and the tx_id against the BlockNumber when the report expires. + #[pallet::storage] + pub(crate) type TaintedTransactions, I: 'static = ()> = StorageDoubleMap< + _, + Identity, + T::AccountId, + Blake2_128Concat, + ::DepositDetails, + TaintedTransactionStatus, + OptionQuery, + >; + + /// Stores the block number when the report expires to gather with the reporter and the tx_id. + #[pallet::storage] + pub(crate) type ReportExpiresAt, I: 'static = ()> = StorageMap< + _, + Twox64Concat, + BlockNumberFor, + Vec<(T::AccountId, ::DepositDetails)>, + ValueQuery, + >; + + /// Stores the details of the tainted transactions that are scheduled for rejecting. + #[pallet::storage] + pub(crate) type ScheduledTxForReject, I: 'static = ()> = + StorageValue<_, Vec>, ValueQuery>; + #[pallet::event] #[pallet::generate_deposit(pub(super) fn deposit_event)] pub enum Event, I: 'static = ()> { @@ -658,6 +714,15 @@ pub mod pallet { deposit_metadata: CcmDepositMetadataEncoded, origin: SwapOrigin, }, + TaintedTransactionReportReceived { + account_id: T::AccountId, + tx_id: ::DepositDetails, + expires_at: BlockNumberFor, + }, + TaintedTransactionReportExpired { + account_id: T::AccountId, + tx_id: ::DepositDetails, + }, CcmFallbackScheduled { broadcast_id: BroadcastId, egress_details: ScheduledEgressDetails, @@ -701,12 +766,16 @@ pub mod pallet { BoostPoolDoesNotExist, /// CCM parameters from a contract swap failed validity check. InvalidCcm, + /// Unsupported chain + UnsupportedChain, + /// Transaction cannot be reported after being pre-witnessed or boosted. + TransactionAlreadyPreWitnessed, } #[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(now: BlockNumberFor, remaining_weight: Weight) -> Weight { let mut used_weight = Weight::zero(); // Approximate weight calculation: r/w DepositChannelLookup + w DepositChannelPool @@ -748,6 +817,29 @@ pub mod pallet { Self::recycle_channel(&mut used_weight, address); } } + + // A report gets cleaned up after approx 1 hour and needs to be re-reported by the + // broker if necessary. This is needed as some kind of garbage collection mechanism + // for tainted deposits. + for (account_id, tx_id) in ReportExpiresAt::::take(now) { + let _ = TaintedTransactions::::try_mutate(&account_id, &tx_id, |status| { + match status.take() { + Some(TaintedTransactionStatus::Unseen) => { + Self::deposit_event(Event::::TaintedTransactionReportExpired { + account_id: account_id.clone(), + tx_id: tx_id.clone(), + }); + Ok(()) + }, + _ => { + // Don't apply the mutation. We expect the pre-witnessed/boosted + // transaction to eventually be fully witnessed. + Err(()) + }, + } + }); + } + used_weight } @@ -1221,6 +1313,18 @@ pub mod pallet { Ok(()) } + + #[pallet::call_index(12)] + #[pallet::weight(T::WeightInfo::mark_transaction_as_tainted())] + pub fn mark_transaction_as_tainted( + origin: OriginFor, + tx_id: ::DepositDetails, + ) -> DispatchResult { + let account_id = T::AccountRoleRegistry::ensure_broker(origin)?; + ensure!(T::AllowTransactionReports::get(), Error::::UnsupportedChain); + Self::mark_transaction_as_tainted_inner(account_id, tx_id)?; + Ok(()) + } } } @@ -1260,6 +1364,28 @@ impl, I: 'static> IngressSink for Pallet { } impl, I: 'static> Pallet { + fn mark_transaction_as_tainted_inner( + account_id: T::AccountId, + tx_id: ::DepositDetails, + ) -> DispatchResult { + TaintedTransactions::::try_mutate(&account_id, &tx_id, |opt| { + const UNSEEN: TaintedTransactionStatus = TaintedTransactionStatus::Unseen; + ensure!( + opt.replace(UNSEEN).unwrap_or_default() == UNSEEN, + Error::::TransactionAlreadyPreWitnessed + ); + Ok::<_, DispatchError>(()) + })?; + let expires_at = >::block_number() + .saturating_add(BlockNumberFor::::from(TAINTED_TX_EXPIRATION_BLOCKS)); + ReportExpiresAt::::append(expires_at, (&account_id, &tx_id)); + Self::deposit_event(Event::::TaintedTransactionReportReceived { + account_id, + tx_id, + expires_at, + }); + Ok(()) + } fn recycle_channel(used_weight: &mut Weight, address: ::ChainAccount) { if let Some(DepositChannelDetails { deposit_channel, boost_status, .. }) = DepositChannelLookup::::take(address) @@ -1596,16 +1722,44 @@ impl, I: 'static> Pallet { continue; } + let DepositChannelDetails { + deposit_channel, + action, + boost_fee, + boost_status, + owner, + .. + } = DepositChannelLookup::::get(&deposit_address) + .ok_or(Error::::InvalidDepositAddress)?; + + if TaintedTransactions::::mutate(&owner, &deposit_details, |opt| { + match opt.as_mut() { + // Transaction has been reported, mark it as pre-witnessed. + Some(status @ TaintedTransactionStatus::Unseen) => { + *status = TaintedTransactionStatus::Prewitnessed; + true + }, + // Transaction has not been reported, mark it as boosted to prevent further + // reports. + None => { + let _ = opt.insert(TaintedTransactionStatus::Boosted); + false + }, + // Pre-witnessing twice or pre-witnessing after boosting is unlikely but + // possible. Either way we don't want to change the status. + Some(TaintedTransactionStatus::Prewitnessed) | + Some(TaintedTransactionStatus::Boosted) => true, + } + }) { + continue; + } + let prewitnessed_deposit_id = PrewitnessedDepositIdCounter::::mutate(|id| -> u64 { *id = id.saturating_add(1); *id }); - let DepositChannelDetails { deposit_channel, action, boost_fee, boost_status, .. } = - DepositChannelLookup::::get(&deposit_address) - .ok_or(Error::::InvalidDepositAddress)?; - let channel_id = deposit_channel.channel_id; // Only boost on non-zero fee and if the channel isn't already boosted: @@ -1786,6 +1940,11 @@ impl, I: 'static> Pallet { let deposit_channel_details = DepositChannelLookup::::get(&deposit_address) .ok_or(Error::::InvalidDepositAddress)?; + ensure!( + deposit_channel_details.deposit_channel.asset == asset, + Error::::AssetMismatch + ); + let channel_id = deposit_channel_details.deposit_channel.channel_id; if DepositChannelPool::::get(channel_id).is_some() { @@ -1797,11 +1956,6 @@ impl, I: 'static> Pallet { return Err(Error::::InvalidDepositAddress.into()) } - ensure!( - deposit_channel_details.deposit_channel.asset == asset, - Error::::AssetMismatch - ); - // TODO: only apply this check if the deposit hasn't been boosted // already (in case MinimumDeposit increases after some small deposit // is boosted)? @@ -1819,6 +1973,37 @@ impl, I: 'static> Pallet { return Ok(()) } + let channel_owner = deposit_channel_details.owner.clone(); + + if matches!(TaintedTransactions::::take(&channel_owner, &deposit_details), + Some(status) if status != TaintedTransactionStatus::Boosted) + { + let refund_address = match deposit_channel_details.action.clone() { + ChannelAction::Swap { refund_params, .. } => + refund_params.as_ref().map(|refund_params| refund_params.refund_address.clone()), + ChannelAction::CcmTransfer { refund_params, .. } => + refund_params.as_ref().map(|refund_params| refund_params.refund_address.clone()), + ChannelAction::LiquidityProvision { refund_address, .. } => refund_address, + }; + + let tainted_transaction_details = TaintedTransactionDetails { + refund_address, + amount: deposit_amount, + asset, + tx_id: deposit_details.clone(), + }; + ScheduledTxForReject::::append(tainted_transaction_details); + + Self::deposit_event(Event::::DepositIgnored { + deposit_address, + asset, + amount: deposit_amount, + deposit_details, + reason: DepositIgnoredReason::TransactionTainted, + }); + return Ok(()) + } + ScheduledEgressFetchOrTransfer::::append(FetchOrTransfer::::Fetch { asset, deposit_address: deposit_address.clone(), @@ -2022,6 +2207,7 @@ impl, I: 'static> Pallet { DepositChannelLookup::::insert( &deposit_address, DepositChannelDetails { + owner: requester.clone(), deposit_channel, opened_at: current_height, expires_at: expiry_height, @@ -2209,6 +2395,7 @@ impl, I: 'static> DepositApi for Pallet { lp_account: T::AccountId, source_asset: TargetChainAsset, boost_fee: BasisPoints, + refund_address: ForeignChainAddress, ) -> Result< (ChannelId, ForeignChainAddress, ::ChainBlockNumber, Self::Amount), DispatchError, @@ -2216,7 +2403,10 @@ impl, I: 'static> DepositApi for Pallet { let (channel_id, deposit_address, expiry_block, channel_opening_fee) = Self::open_channel( &lp_account, source_asset, - ChannelAction::LiquidityProvision { lp_account: lp_account.clone() }, + ChannelAction::LiquidityProvision { + lp_account: lp_account.clone(), + refund_address: Some(refund_address), + }, boost_fee, )?; diff --git a/state-chain/pallets/cf-ingress-egress/src/migrations.rs b/state-chain/pallets/cf-ingress-egress/src/migrations.rs index 73f726554c..c924a60d83 100644 --- a/state-chain/pallets/cf-ingress-egress/src/migrations.rs +++ b/state-chain/pallets/cf-ingress-egress/src/migrations.rs @@ -1,4 +1,8 @@ use crate::Pallet; -use cf_runtime_upgrade_utilities::PlaceholderMigration; +use cf_runtime_upgrade_utilities::{PlaceholderMigration, VersionedMigration}; -pub type PalletMigration = PlaceholderMigration, 15>; +mod add_owner_to_channel_details; +pub type PalletMigration = ( + VersionedMigration, add_owner_to_channel_details::Migration, 15, 16>, + PlaceholderMigration, 16>, +); diff --git a/state-chain/pallets/cf-ingress-egress/src/migrations/add_owner_to_channel_details.rs b/state-chain/pallets/cf-ingress-egress/src/migrations/add_owner_to_channel_details.rs new file mode 100644 index 0000000000..48e9562703 --- /dev/null +++ b/state-chain/pallets/cf-ingress-egress/src/migrations/add_owner_to_channel_details.rs @@ -0,0 +1,183 @@ +use frame_support::{traits::OnRuntimeUpgrade, weights::Weight}; + +use crate::*; +mod old { + use super::*; + + #[derive(Clone, RuntimeDebug, PartialEq, Eq, Encode, Decode, TypeInfo)] + pub enum ChannelAction { + Swap { + destination_asset: Asset, + destination_address: ForeignChainAddress, + broker_fees: Beneficiaries, + refund_params: Option, + dca_params: Option, + }, + LiquidityProvision { + lp_account: AccountId, + }, + CcmTransfer { + destination_asset: Asset, + destination_address: ForeignChainAddress, + broker_fees: Beneficiaries, + channel_metadata: CcmChannelMetadata, + refund_params: Option, + dca_params: Option, + }, + } + + #[derive(CloneNoBound, RuntimeDebug, PartialEq, Eq, Encode, Decode, TypeInfo)] + #[scale_info(skip_type_params(T, I))] + pub struct DepositChannelDetails, I: 'static> { + pub deposit_channel: DepositChannel, + pub opened_at: TargetChainBlockNumber, + pub expires_at: TargetChainBlockNumber, + pub action: ChannelAction, + pub boost_fee: BasisPoints, + pub boost_status: BoostStatus>, + } + + #[frame_support::storage_alias] + pub type DepositChannelLookup, I: 'static> = StorageMap< + Pallet, + Twox64Concat, + TargetChainAccount, + DepositChannelDetails, + OptionQuery, + >; +} + +pub struct Migration, I: 'static>(PhantomData<(T, I)>); + +impl, I: 'static> OnRuntimeUpgrade for Migration { + fn on_runtime_upgrade() -> Weight { + DepositChannelLookup::::translate( + |_account, channel_details: old::DepositChannelDetails| { + let dummy_account = T::AccountId::decode(&mut &[0u8; 32][..]).unwrap(); + let channel_action = match channel_details.action { + old::ChannelAction::LiquidityProvision { lp_account, .. } => + ChannelAction::LiquidityProvision { lp_account, refund_address: None }, + old::ChannelAction::Swap { + destination_asset, + destination_address, + broker_fees, + refund_params, + dca_params, + } => ChannelAction::Swap { + destination_asset, + destination_address, + broker_fees, + refund_params, + dca_params, + }, + old::ChannelAction::CcmTransfer { + destination_asset, + destination_address, + broker_fees, + channel_metadata, + refund_params, + dca_params, + } => ChannelAction::CcmTransfer { + destination_asset, + destination_address, + broker_fees, + channel_metadata, + refund_params, + dca_params, + }, + }; + let new_channel_details = DepositChannelDetails { + owner: dummy_account, + deposit_channel: channel_details.deposit_channel, + opened_at: channel_details.opened_at, + expires_at: channel_details.expires_at, + action: channel_action, + boost_fee: channel_details.boost_fee, + boost_status: channel_details.boost_status, + }; + Some(new_channel_details) + }, + ); + Weight::zero() + } + + #[cfg(feature = "try-runtime")] + fn pre_upgrade() -> Result, DispatchError> { + let number_of_old_channels: u32 = + old::DepositChannelLookup::::iter().collect::>().len() as u32; + Ok(number_of_old_channels.encode()) + } + + #[cfg(feature = "try-runtime")] + fn post_upgrade(state: Vec) -> Result<(), DispatchError> { + let number_of_old_channels = u32::decode(&mut &state[..]).unwrap(); + let number_of_new_channels = + DepositChannelLookup::::iter().collect::>().len() as u32; + assert_eq!(number_of_old_channels, number_of_new_channels); + Ok(()) + } +} + +#[cfg(test)] +mod migration_tests { + use super::*; + use crate::mock_eth::*; + + #[test] + fn test_migration() { + use cf_chains::evm::DeploymentStatus; + new_test_ext().execute_with(|| { + let channel_id = 1u64; + let address = sp_core::H160([1u8; 20]); + let asset = cf_chains::assets::eth::Asset::Eth; + let deployment_state = DeploymentStatus::Deployed; + let lp_account = 5u64; + let opened_at = 1u64; + let expires_at = 2u64; + let action = ChannelAction::LiquidityProvision { lp_account, refund_address: None }; + let boost_fee = 1; + let boost_status = BoostStatus::NotBoosted; + + old::DepositChannelLookup::::insert( + address, + old::DepositChannelDetails { + deposit_channel: DepositChannel { + asset, + channel_id, + address, + state: deployment_state, + }, + opened_at, + expires_at, + action: old::ChannelAction::LiquidityProvision { lp_account }, + boost_fee, + boost_status, + }, + ); + assert_eq!(old::DepositChannelLookup::::iter().count(), 1); + + #[cfg(feature = "try-runtime")] + let state = super::Migration::::pre_upgrade().unwrap(); + super::Migration::::on_runtime_upgrade(); + + #[cfg(feature = "try-runtime")] + super::Migration::::post_upgrade(state).unwrap(); + + assert_eq!(DepositChannelLookup::::iter().count(), 1); + + let migrated_deposit_channel = DepositChannelLookup::::get(address) + .expect("to have a channel in storage"); + + assert_eq!(migrated_deposit_channel.owner, 0); + assert_eq!(old::DepositChannelLookup::::iter().count(), 0); + + assert_eq!(migrated_deposit_channel.deposit_channel.asset, asset); + assert_eq!(migrated_deposit_channel.deposit_channel.channel_id, channel_id); + assert_eq!(migrated_deposit_channel.deposit_channel.address, address); + assert_eq!(migrated_deposit_channel.deposit_channel.state, deployment_state); + assert_eq!(migrated_deposit_channel.opened_at, opened_at); + assert_eq!(migrated_deposit_channel.expires_at, expires_at); + assert_eq!(migrated_deposit_channel.action, action); + }); + } +} diff --git a/state-chain/pallets/cf-ingress-egress/src/mock_btc.rs b/state-chain/pallets/cf-ingress-egress/src/mock_btc.rs index f08150dbd8..7d4a57d4b7 100644 --- a/state-chain/pallets/cf-ingress-egress/src/mock_btc.rs +++ b/state-chain/pallets/cf-ingress-egress/src/mock_btc.rs @@ -26,6 +26,7 @@ use cf_traits::{ DummyIngressSource, NetworkEnvironmentProvider, OnDeposit, }; use frame_support::derive_impl; +use sp_core::ConstBool; type Block = frame_system::mocking::MockBlock; @@ -105,6 +106,7 @@ impl pallet_cf_ingress_egress::Config for Test { type SafeMode = MockRuntimeSafeMode; type SwapLimitsProvider = MockSwapLimitsProvider; type CcmValidityChecker = cf_chains::ccm_checker::CcmValidityChecker; + type AllowTransactionReports = ConstBool; } impl_test_helpers! { diff --git a/state-chain/pallets/cf-ingress-egress/src/mock_eth.rs b/state-chain/pallets/cf-ingress-egress/src/mock_eth.rs index 1a41fc08f5..5673212d4c 100644 --- a/state-chain/pallets/cf-ingress-egress/src/mock_eth.rs +++ b/state-chain/pallets/cf-ingress-egress/src/mock_eth.rs @@ -32,7 +32,7 @@ use cf_traits::{ }; use frame_support::derive_impl; use frame_system as system; -use sp_core::H256; +use sp_core::{ConstBool, H256}; use sp_runtime::traits::{BlakeTwo256, IdentityLookup, Zero}; type AccountId = u64; @@ -136,6 +136,7 @@ impl crate::Config for Test { type SafeMode = MockRuntimeSafeMode; type SwapLimitsProvider = MockSwapLimitsProvider; type CcmValidityChecker = cf_chains::ccm_checker::CcmValidityChecker; + type AllowTransactionReports = ConstBool; } pub const ALICE: ::AccountId = 123u64; @@ -248,11 +249,16 @@ impl RequestAddress for TestExternalities { .cloned() .map(|request| match request { DepositRequest::Liquidity { lp_account, asset } => - IngressEgress::request_liquidity_deposit_address(lp_account, asset, 0) - .map(|(id, addr, ..)| { - (request, id, TestChainAccount::try_from(addr).unwrap()) - }) - .unwrap(), + IngressEgress::request_liquidity_deposit_address( + lp_account, + asset, + 0, + ForeignChainAddress::Eth(Default::default()), + ) + .map(|(id, addr, ..)| { + (request, id, TestChainAccount::try_from(addr).unwrap()) + }) + .unwrap(), DepositRequest::SimpleSwap { source_asset, destination_asset, diff --git a/state-chain/pallets/cf-ingress-egress/src/tests.rs b/state-chain/pallets/cf-ingress-egress/src/tests.rs index 9d393f7186..7e4585c9c2 100644 --- a/state-chain/pallets/cf-ingress-egress/src/tests.rs +++ b/state-chain/pallets/cf-ingress-egress/src/tests.rs @@ -1,26 +1,28 @@ mod boost; use crate::{ - mock_eth::*, BoostStatus, Call as PalletCall, ChannelAction, ChannelIdCounter, + mock_eth::*, BoostPoolId, BoostStatus, Call as PalletCall, ChannelAction, ChannelIdCounter, ChannelOpeningFee, CrossChainMessage, DepositAction, DepositChannelLookup, DepositChannelPool, DepositIgnoredReason, DepositWitness, DisabledEgressAssets, EgressDustLimit, Event as PalletEvent, FailedForeignChainCall, FailedForeignChainCalls, FetchOrTransfer, MinimumDeposit, Pallet, PalletConfigUpdate, PalletSafeMode, PrewitnessedDepositIdCounter, - ScheduledEgressCcm, ScheduledEgressFetchOrTransfer, + ReportExpiresAt, ScheduledEgressCcm, ScheduledEgressFetchOrTransfer, ScheduledTxForReject, + TaintedTransactionStatus, TaintedTransactions, TAINTED_TX_EXPIRATION_BLOCKS, }; use cf_chains::{ address::{AddressConverter, EncodedAddress}, btc::{BitcoinNetwork, ScriptPubkey}, - evm::{DepositDetails, EvmFetchId}, + evm::{self, DepositDetails, EvmFetchId}, mocks::MockEthereum, CcmChannelMetadata, CcmFailReason, DepositChannel, ExecutexSwapAndCall, SwapOrigin, TransferAssetParams, }; -use cf_primitives::{chains::assets::eth, AssetAmount, ChannelId, ForeignChain}; +use cf_primitives::{chains::assets::eth, AssetAmount, Beneficiaries, ChannelId, ForeignChain}; use cf_test_utilities::{assert_events_eq, assert_has_event, assert_has_matching_event}; use cf_traits::{ mocks::{ self, + account_role_registry::MockAccountRoleRegistry, address_converter::MockAddressConverter, api_call::{MockEthAllBatch, MockEthereumApiCall, MockEvmEnvironment}, asset_converter::MockAssetConverter, @@ -32,16 +34,17 @@ use cf_traits::{ funding_info::MockFundingInfo, swap_request_api::{MockSwapRequest, MockSwapRequestHandler}, }, - BalanceApi, DepositApi, EgressApi, EpochInfo, FetchesTransfersLimitProvider, FundingInfo, - GetBlockHeight, SafeMode, ScheduledEgressDetails, SwapRequestType, + AccountRoleRegistry, BalanceApi, DepositApi, EgressApi, EpochInfo, + FetchesTransfersLimitProvider, FundingInfo, GetBlockHeight, SafeMode, ScheduledEgressDetails, + SwapRequestType, }; use frame_support::{ assert_err, assert_noop, assert_ok, traits::{Hooks, OriginTrait}, weights::Weight, }; -use sp_core::H160; -use sp_runtime::DispatchError; +use sp_core::{H160, H256}; +use sp_runtime::{DispatchError, DispatchError::BadOrigin}; const ALICE_ETH_ADDRESS: EthereumAddress = H160([100u8; 20]); const BOB_ETH_ADDRESS: EthereumAddress = H160([101u8; 20]); @@ -223,8 +226,13 @@ fn request_address_and_deposit( who: ChannelId, asset: eth::Asset, ) -> (ChannelId, ::ChainAccount) { - let (id, address, ..) = - IngressEgress::request_liquidity_deposit_address(who, asset, 0).unwrap(); + let (id, address, ..) = IngressEgress::request_liquidity_deposit_address( + who, + asset, + 0, + ForeignChainAddress::Eth(Default::default()), + ) + .unwrap(); let address: ::ChainAccount = address.try_into().unwrap(); assert_ok!(IngressEgress::process_single_deposit( address, @@ -456,7 +464,10 @@ fn reused_address_channel_id_matches() { let (reused_channel_id, reused_address, ..) = IngressEgress::open_channel( &ALICE, eth::Asset::Eth, - ChannelAction::LiquidityProvision { lp_account: 0 }, + ChannelAction::LiquidityProvision { + lp_account: 0, + refund_address: Some(ForeignChainAddress::Eth([0u8; 20].into())), + }, 0, ) .unwrap(); @@ -532,8 +543,13 @@ fn multi_deposit_includes_deposit_beyond_recycle_height() { const ETH: eth::Asset = eth::Asset::Eth; new_test_ext() .then_execute_at_next_block(|_| { - let (_, address, ..) = - IngressEgress::request_liquidity_deposit_address(ALICE, ETH, 0).unwrap(); + let (_, address, ..) = IngressEgress::request_liquidity_deposit_address( + ALICE, + ETH, + 0, + ForeignChainAddress::Eth(Default::default()), + ) + .unwrap(); let address: ::ChainAccount = address.try_into().unwrap(); let recycles_at = IngressEgress::expiry_and_recycle_block_height().2; (address, recycles_at) @@ -543,8 +559,13 @@ fn multi_deposit_includes_deposit_beyond_recycle_height() { address }) .then_execute_at_next_block(|address| { - let (_, address2, ..) = - IngressEgress::request_liquidity_deposit_address(ALICE, ETH, 0).unwrap(); + let (_, address2, ..) = IngressEgress::request_liquidity_deposit_address( + ALICE, + ETH, + 0, + ForeignChainAddress::Eth(Default::default()), + ) + .unwrap(); let address2: ::ChainAccount = address2.try_into().unwrap(); (address, address2) }) @@ -845,8 +866,13 @@ fn deposits_ingress_fee_exceeding_deposit_amount_rejected() { // Set fee to be higher than the deposit value. ChainTracker::::set_fee(HIGH_FEE); - let (_id, address, ..) = - IngressEgress::request_liquidity_deposit_address(ALICE, ASSET, 0).unwrap(); + let (_id, address, ..) = IngressEgress::request_liquidity_deposit_address( + ALICE, + ASSET, + 0, + ForeignChainAddress::Eth(Default::default()), + ) + .unwrap(); let deposit_address = address.try_into().unwrap(); // Swap a low enough amount such that it gets swallowed by fees @@ -1395,7 +1421,10 @@ fn broker_pays_a_fee_for_each_deposit_address() { assert_ok!(IngressEgress::open_channel( &CHANNEL_REQUESTER, eth::Asset::Eth, - ChannelAction::LiquidityProvision { lp_account: CHANNEL_REQUESTER }, + ChannelAction::LiquidityProvision { + lp_account: CHANNEL_REQUESTER, + refund_address: Some(ForeignChainAddress::Eth(Default::default())), + }, 0 )); assert_eq!(MockFundingInfo::::total_balance_of(&CHANNEL_REQUESTER), 0); @@ -1409,7 +1438,10 @@ fn broker_pays_a_fee_for_each_deposit_address() { IngressEgress::open_channel( &CHANNEL_REQUESTER, eth::Asset::Eth, - ChannelAction::LiquidityProvision { lp_account: CHANNEL_REQUESTER }, + ChannelAction::LiquidityProvision { + lp_account: CHANNEL_REQUESTER, + refund_address: Some(ForeignChainAddress::Eth(Default::default())), + }, 0 ), mocks::fee_payment::ERROR_INSUFFICIENT_LIQUIDITY @@ -1567,7 +1599,10 @@ fn safe_mode_prevents_deposit_channel_creation() { assert_ok!(IngressEgress::open_channel( &ALICE, eth::Asset::Eth, - ChannelAction::LiquidityProvision { lp_account: 0 }, + ChannelAction::LiquidityProvision { + lp_account: 0, + refund_address: Some(ForeignChainAddress::Eth(Default::default())) + }, 0, )); @@ -1584,7 +1619,10 @@ fn safe_mode_prevents_deposit_channel_creation() { IngressEgress::open_channel( &ALICE, eth::Asset::Eth, - ChannelAction::LiquidityProvision { lp_account: 0 }, + ChannelAction::LiquidityProvision { + lp_account: 0, + refund_address: Some(ForeignChainAddress::Eth(Default::default())) + }, 0, ), crate::Error::::DepositChannelCreationDisabled @@ -1647,9 +1685,13 @@ fn do_not_batch_more_fetches_than_the_limit_allows() { let fetch_limits = MockFetchesTransfersLimitProvider::maybe_fetches_limit().unwrap(); for i in 1..=fetch_limits + EXCESS_FETCHES { - let (_, address, ..) = - IngressEgress::request_liquidity_deposit_address(i.try_into().unwrap(), ASSET, 0) - .unwrap(); + let (_, address, ..) = IngressEgress::request_liquidity_deposit_address( + i.try_into().unwrap(), + ASSET, + 0, + ForeignChainAddress::Eth(Default::default()), + ) + .unwrap(); let address: ::ChainAccount = address.try_into().unwrap(); assert_ok!(IngressEgress::process_single_deposit( @@ -1922,3 +1964,289 @@ fn failed_ccm_deposit_can_deposit_event() { ); }); } + +#[test] +fn process_tainted_transaction_and_expect_refund() { + new_test_ext().execute_with(|| { + let (_, address) = request_address_and_deposit(BROKER, eth::Asset::Eth); + let _ = DepositChannelLookup::::get(address).unwrap(); + + let deposit_details: cf_chains::evm::DepositDetails = Default::default(); + + assert_ok!(>::register_as_broker( + &BROKER, + )); + + assert_ok!(IngressEgress::mark_transaction_as_tainted( + OriginTrait::signed(BROKER), + Default::default(), + )); + + assert_ok!(IngressEgress::process_single_deposit( + address, + eth::Asset::Eth, + DEFAULT_DEPOSIT_AMOUNT, + deposit_details, + Default::default() + )); + + assert_has_matching_event!( + Test, + RuntimeEvent::IngressEgress(crate::Event::::DepositIgnored { + deposit_address: _address, + asset: eth::Asset::Eth, + amount: DEFAULT_DEPOSIT_AMOUNT, + deposit_details: _, + reason: DepositIgnoredReason::TransactionTainted, + }) + ); + + assert_eq!(ScheduledTxForReject::::decode_len(), Some(1)); + }); +} + +#[test] +fn only_broker_can_mark_transaction_as_tainted() { + new_test_ext().execute_with(|| { + assert_noop!( + IngressEgress::mark_transaction_as_tainted( + OriginTrait::signed(ALICE), + Default::default(), + ), + BadOrigin + ); + + assert_ok!(>::register_as_broker( + &BROKER, + )); + + assert_ok!(IngressEgress::mark_transaction_as_tainted( + OriginTrait::signed(BROKER), + Default::default(), + )); + }); +} + +#[test] +fn tainted_transactions_expire_if_not_witnessed() { + new_test_ext().execute_with(|| { + let tx_id = DepositDetails::default(); + let expiry_at = System::block_number() + TAINTED_TX_EXPIRATION_BLOCKS as u64; + + let (_, address) = request_address_and_deposit(BROKER, eth::Asset::Eth); + let _ = DepositChannelLookup::::get(address).unwrap(); + + assert_ok!(>::register_as_broker( + &BROKER, + )); + + assert_ok!(IngressEgress::mark_transaction_as_tainted( + OriginTrait::signed(BROKER), + Default::default(), + )); + + System::set_block_number(expiry_at); + + IngressEgress::on_idle(expiry_at, Weight::MAX); + + assert!(!TaintedTransactions::::contains_key(BROKER, tx_id)); + + assert_has_event::(RuntimeEvent::IngressEgress( + crate::Event::TaintedTransactionReportExpired { + account_id: BROKER, + tx_id: Default::default(), + }, + )); + }); +} + +fn setup_boost_swap() -> ForeignChainAddress { + assert_ok!( + >::register_as_liquidity_provider( + &ALICE, + ) + ); + + assert_ok!(IngressEgress::create_boost_pools( + RuntimeOrigin::root(), + vec![BoostPoolId { asset: eth::Asset::Eth, tier: 10 }], + )); + + ::Balance::try_credit_account(&ALICE, eth::Asset::Eth.into(), 1000) + .unwrap(); + + let (_, address, _, _) = IngressEgress::request_swap_deposit_address( + eth::Asset::Eth, + eth::Asset::Eth.into(), + ForeignChainAddress::Eth(Default::default()), + Beneficiaries::new(), + BROKER, + None, + 10, + None, + None, + ) + .unwrap(); + + assert_ok!(IngressEgress::add_boost_funds( + RuntimeOrigin::signed(ALICE), + eth::Asset::Eth, + 1000, + 10 + )); + + address +} + +#[test] +fn finalize_boosted_tx_if_tainted_after_prewitness() { + new_test_ext().execute_with(|| { + let tx_id = DepositDetails::default(); + + assert_ok!(>::register_as_broker( + &BROKER, + )); + + let address: ::ChainAccount = setup_boost_swap().try_into().unwrap(); + + let _ = IngressEgress::add_prewitnessed_deposits( + vec![DepositWitness { + deposit_address: address, + asset: eth::Asset::Eth, + amount: DEFAULT_DEPOSIT_AMOUNT, + deposit_details: tx_id.clone(), + }], + 10, + ); + + assert_noop!( + IngressEgress::mark_transaction_as_tainted(OriginTrait::signed(BROKER), tx_id.clone(),), + crate::Error::::TransactionAlreadyPreWitnessed + ); + + assert_ok!(IngressEgress::process_single_deposit( + address, + eth::Asset::Eth, + DEFAULT_DEPOSIT_AMOUNT, + tx_id, + Default::default() + )); + + assert_has_matching_event!( + Test, + RuntimeEvent::IngressEgress(crate::Event::DepositFinalised { + deposit_address: _, + asset: eth::Asset::Eth, + .. + }) + ); + }); +} + +#[test] +fn reject_tx_if_tainted_before_prewitness() { + new_test_ext().execute_with(|| { + let tx_id = DepositDetails::default(); + + assert_ok!(>::register_as_broker( + &BROKER, + )); + + let address: ::ChainAccount = setup_boost_swap().try_into().unwrap(); + + assert_ok!(IngressEgress::mark_transaction_as_tainted( + OriginTrait::signed(BROKER), + tx_id.clone(), + )); + + let _ = IngressEgress::add_prewitnessed_deposits( + vec![DepositWitness { + deposit_address: address, + asset: eth::Asset::Eth, + amount: DEFAULT_DEPOSIT_AMOUNT, + deposit_details: tx_id.clone(), + }], + 10, + ); + + assert_ok!(IngressEgress::process_single_deposit( + address, + eth::Asset::Eth, + DEFAULT_DEPOSIT_AMOUNT, + tx_id, + Default::default() + )); + + assert_has_matching_event!( + Test, + RuntimeEvent::IngressEgress(crate::Event::DepositIgnored { + deposit_address: _, + asset: eth::Asset::Eth, + amount: DEFAULT_DEPOSIT_AMOUNT, + deposit_details: _, + reason: DepositIgnoredReason::TransactionTainted, + }) + ); + }); +} + +#[test] +fn do_not_expire_tainted_transactions_if_prewitnessed() { + new_test_ext().execute_with(|| { + let tx_id = DepositDetails::default(); + let expiry_at = System::block_number() + TAINTED_TX_EXPIRATION_BLOCKS as u64; + + TaintedTransactions::::insert( + BROKER, + &tx_id, + TaintedTransactionStatus::Prewitnessed, + ); + + ReportExpiresAt::::insert(expiry_at, vec![(BROKER, tx_id.clone())]); + + IngressEgress::on_idle(expiry_at, Weight::MAX); + + assert!(TaintedTransactions::::contains_key(BROKER, tx_id)); + }); +} + +#[test] +fn can_not_report_transaction_after_witnessing() { + new_test_ext().execute_with(|| { + assert_ok!(>::register_as_broker( + &BROKER, + )); + let unreported = evm::DepositDetails { tx_hashes: Some(vec![H256::random()]) }; + let unseen = evm::DepositDetails { tx_hashes: Some(vec![H256::random()]) }; + let prewitnessed = evm::DepositDetails { tx_hashes: Some(vec![H256::random()]) }; + let boosted = evm::DepositDetails { tx_hashes: Some(vec![H256::random()]) }; + + TaintedTransactions::::insert(BROKER, &unseen, TaintedTransactionStatus::Unseen); + TaintedTransactions::::insert( + BROKER, + &prewitnessed, + TaintedTransactionStatus::Prewitnessed, + ); + TaintedTransactions::::insert( + BROKER, + &boosted, + TaintedTransactionStatus::Boosted, + ); + + assert_ok!(IngressEgress::mark_transaction_as_tainted( + OriginTrait::signed(BROKER), + unreported, + )); + assert_ok!( + IngressEgress::mark_transaction_as_tainted(OriginTrait::signed(BROKER), unseen,) + ); + assert_noop!( + IngressEgress::mark_transaction_as_tainted(OriginTrait::signed(BROKER), prewitnessed,), + crate::Error::::TransactionAlreadyPreWitnessed + ); + assert_noop!( + IngressEgress::mark_transaction_as_tainted(OriginTrait::signed(BROKER), boosted,), + crate::Error::::TransactionAlreadyPreWitnessed + ); + }); +} diff --git a/state-chain/pallets/cf-ingress-egress/src/tests/boost.rs b/state-chain/pallets/cf-ingress-egress/src/tests/boost.rs index ccdea830de..74af24fe9a 100644 --- a/state-chain/pallets/cf-ingress-egress/src/tests/boost.rs +++ b/state-chain/pallets/cf-ingress-egress/src/tests/boost.rs @@ -46,8 +46,13 @@ fn request_deposit_address( asset: eth::Asset, max_boost_fee: BasisPoints, ) -> (u64, H160) { - let (channel_id, deposit_address, ..) = - IngressEgress::request_liquidity_deposit_address(account_id, asset, max_boost_fee).unwrap(); + let (channel_id, deposit_address, ..) = IngressEgress::request_liquidity_deposit_address( + account_id, + asset, + max_boost_fee, + ForeignChainAddress::Eth(Default::default()), + ) + .unwrap(); (channel_id, deposit_address.try_into().unwrap()) } diff --git a/state-chain/pallets/cf-ingress-egress/src/weights.rs b/state-chain/pallets/cf-ingress-egress/src/weights.rs index fbbbd28452..afa3a67a47 100644 --- a/state-chain/pallets/cf-ingress-egress/src/weights.rs +++ b/state-chain/pallets/cf-ingress-egress/src/weights.rs @@ -45,6 +45,7 @@ pub trait WeightInfo { fn deposit_boosted() -> Weight; fn boost_finalised() -> Weight; fn create_boost_pools() -> Weight; + fn mark_transaction_as_tainted() -> Weight; } /// Weights for pallet_cf_ingress_egress using the Substrate node and recommended hardware. @@ -306,6 +307,15 @@ impl WeightInfo for PalletWeight { .saturating_add(T::DbWeight::get().reads(1_u64)) .saturating_add(T::DbWeight::get().writes(1_u64)) } + + /// TODO: This needs to get generated during the reals benchmarking. + fn mark_transaction_as_tainted() -> Weight { + // Proof Size summary in bytes: + // Measured: `0` + // Estimated: `0` + // Minimum execution time: 0 picoseconds. + Weight::from_parts(0, 0) + } } // For backwards compatibility and tests @@ -566,4 +576,13 @@ impl WeightInfo for () { .saturating_add(RocksDbWeight::get().reads(1_u64)) .saturating_add(RocksDbWeight::get().writes(1_u64)) } + + /// TODO: This needs to get generated during the reals benchmarking. + fn mark_transaction_as_tainted() -> Weight { + // Proof Size summary in bytes: + // Measured: `0` + // Estimated: `0` + // Minimum execution time: 0 picoseconds. + Weight::from_parts(0, 0) + } } diff --git a/state-chain/pallets/cf-lp/src/lib.rs b/state-chain/pallets/cf-lp/src/lib.rs index 6a5e1546bc..a62d9f2a1e 100644 --- a/state-chain/pallets/cf-lp/src/lib.rs +++ b/state-chain/pallets/cf-lp/src/lib.rs @@ -194,29 +194,31 @@ pub mod pallet { let account_id = T::AccountRoleRegistry::ensure_liquidity_provider(origin)?; - ensure!( - LiquidityRefundAddress::::contains_key(&account_id, ForeignChain::from(asset)), - Error::::NoLiquidityRefundAddressRegistered - ); + if let Some(refund_address) = + LiquidityRefundAddress::::get(&account_id, ForeignChain::from(asset)) + { + let (channel_id, deposit_address, expiry_block, channel_opening_fee) = + T::DepositHandler::request_liquidity_deposit_address( + account_id.clone(), + asset, + boost_fee, + refund_address, + )?; - let (channel_id, deposit_address, expiry_block, channel_opening_fee) = - T::DepositHandler::request_liquidity_deposit_address( - account_id.clone(), + Self::deposit_event(Event::LiquidityDepositAddressReady { + channel_id, asset, + deposit_address: T::AddressConverter::to_encoded_address(deposit_address), + account_id, + deposit_chain_expiry_block: expiry_block, boost_fee, - )?; - - Self::deposit_event(Event::LiquidityDepositAddressReady { - channel_id, - asset, - deposit_address: T::AddressConverter::to_encoded_address(deposit_address), - account_id, - deposit_chain_expiry_block: expiry_block, - boost_fee, - channel_opening_fee, - }); + channel_opening_fee, + }); - Ok(()) + Ok(()) + } else { + Err(Error::::NoLiquidityRefundAddressRegistered.into()) + } } /// Withdraw some amount of an asset from the free balance to an external address. diff --git a/state-chain/runtime/src/chainflip.rs b/state-chain/runtime/src/chainflip.rs index 729d8a2128..373cef9289 100644 --- a/state-chain/runtime/src/chainflip.rs +++ b/state-chain/runtime/src/chainflip.rs @@ -667,7 +667,8 @@ macro_rules! impl_deposit_api_for_anychain { fn request_liquidity_deposit_address( lp_account: Self::AccountId, source_asset: Asset, - boost_fee: BasisPoints + boost_fee: BasisPoints, + refund_address: ForeignChainAddress, ) -> Result<(ChannelId, ForeignChainAddress, ::ChainBlockNumber, FlipBalance), DispatchError> { match source_asset.into() { $( @@ -675,7 +676,8 @@ macro_rules! impl_deposit_api_for_anychain { $pallet::request_liquidity_deposit_address( lp_account, source_asset, - boost_fee + boost_fee, + refund_address, ).map(|(channel, address, block_number, channel_opening_fee)| (channel, address, block_number.into(), channel_opening_fee)), )+ } diff --git a/state-chain/runtime/src/lib.rs b/state-chain/runtime/src/lib.rs index 9cfb04501f..38a1833d3b 100644 --- a/state-chain/runtime/src/lib.rs +++ b/state-chain/runtime/src/lib.rs @@ -381,6 +381,7 @@ impl pallet_cf_ingress_egress::Config for Runtime { type SafeMode = RuntimeSafeMode; type SwapLimitsProvider = Swapping; type CcmValidityChecker = cf_chains::ccm_checker::CcmValidityChecker; + type AllowTransactionReports = ConstBool; } impl pallet_cf_ingress_egress::Config for Runtime { @@ -407,6 +408,7 @@ impl pallet_cf_ingress_egress::Config for Runtime { type SafeMode = RuntimeSafeMode; type SwapLimitsProvider = Swapping; type CcmValidityChecker = cf_chains::ccm_checker::CcmValidityChecker; + type AllowTransactionReports = ConstBool; } impl pallet_cf_ingress_egress::Config for Runtime { @@ -433,6 +435,7 @@ impl pallet_cf_ingress_egress::Config for Runtime { type SafeMode = RuntimeSafeMode; type SwapLimitsProvider = Swapping; type CcmValidityChecker = cf_chains::ccm_checker::CcmValidityChecker; + type AllowTransactionReports = ConstBool; } impl pallet_cf_ingress_egress::Config for Runtime { @@ -459,6 +462,7 @@ impl pallet_cf_ingress_egress::Config for Runtime { type SafeMode = RuntimeSafeMode; type SwapLimitsProvider = Swapping; type CcmValidityChecker = cf_chains::ccm_checker::CcmValidityChecker; + type AllowTransactionReports = ConstBool; } impl pallet_cf_ingress_egress::Config for Runtime { @@ -485,6 +489,7 @@ impl pallet_cf_ingress_egress::Config for Runtime { type SafeMode = RuntimeSafeMode; type SwapLimitsProvider = Swapping; type CcmValidityChecker = cf_chains::ccm_checker::CcmValidityChecker; + type AllowTransactionReports = ConstBool; } impl pallet_cf_pools::Config for Runtime { diff --git a/state-chain/traits/src/lib.rs b/state-chain/traits/src/lib.rs index 67480a536b..0ce94920ed 100644 --- a/state-chain/traits/src/lib.rs +++ b/state-chain/traits/src/lib.rs @@ -730,6 +730,7 @@ pub trait DepositApi { lp_account: Self::AccountId, source_asset: C::ChainAsset, boost_fee: BasisPoints, + refund_address: ForeignChainAddress, ) -> Result<(ChannelId, ForeignChainAddress, C::ChainBlockNumber, Self::Amount), DispatchError>; /// Issues a channel id and deposit address for a new swap. diff --git a/state-chain/traits/src/mocks/balance_api.rs b/state-chain/traits/src/mocks/balance_api.rs index e08610dae4..6cdbce2685 100644 --- a/state-chain/traits/src/mocks/balance_api.rs +++ b/state-chain/traits/src/mocks/balance_api.rs @@ -6,9 +6,6 @@ use frame_support::sp_runtime::{ DispatchError, DispatchResult, }; -#[cfg(feature = "runtime-benchmarks")] -use cf_chains::ForeignChainAddress; - use super::{MockPallet, MockPalletStorage}; use crate::LpRegistration; @@ -88,7 +85,7 @@ impl LpRegistration for MockLpRegistration { type AccountId = u64; #[cfg(feature = "runtime-benchmarks")] - fn register_liquidity_refund_address(_: &Self::AccountId, _: ForeignChainAddress) {} + fn register_liquidity_refund_address(_: &Self::AccountId, _: cf_chains::ForeignChainAddress) {} fn ensure_has_refund_address_for_pair( _who: &Self::AccountId, diff --git a/state-chain/traits/src/mocks/deposit_handler.rs b/state-chain/traits/src/mocks/deposit_handler.rs index 232beeb28e..390a0fe2c1 100644 --- a/state-chain/traits/src/mocks/deposit_handler.rs +++ b/state-chain/traits/src/mocks/deposit_handler.rs @@ -89,6 +89,7 @@ impl DepositApi for MockDepositHandler { lp_account: Self::AccountId, source_asset: ::ChainAsset, boost_fee: BasisPoints, + _refund_address: ForeignChainAddress, ) -> Result< ( cf_primitives::ChannelId,