From 51a92b6a7ff95134a048380cc4728d422a61522e Mon Sep 17 00:00:00 2001 From: Maxim Shishmarev Date: Tue, 19 Nov 2024 17:39:07 +1100 Subject: [PATCH 01/13] feat: boost support for vault swaps --- engine/src/witness/arb.rs | 47 +- engine/src/witness/btc/deposits.rs | 24 +- engine/src/witness/btc/vault_swaps.rs | 42 +- engine/src/witness/eth.rs | 50 +- engine/src/witness/evm/vault.rs | 27 +- .../cf-integration-tests/src/solana.rs | 80 +- .../cf-integration-tests/src/swapping.rs | 81 +- state-chain/chains/src/lib.rs | 8 + .../cf-ingress-egress/src/benchmarking.rs | 37 +- .../pallets/cf-ingress-egress/src/lib.rs | 868 ++++++++++++------ .../pallets/cf-ingress-egress/src/tests.rs | 135 ++- .../cf-ingress-egress/src/tests/boost.rs | 173 +++- .../runtime/src/chainflip/solana_elections.rs | 33 +- state-chain/runtime/src/lib.rs | 26 +- 14 files changed, 1074 insertions(+), 557 deletions(-) diff --git a/engine/src/witness/arb.rs b/engine/src/witness/arb.rs index e1df799ce1..d0a53fb12f 100644 --- a/engine/src/witness/arb.rs +++ b/engine/src/witness/arb.rs @@ -8,11 +8,12 @@ use cf_chains::{ Arbitrum, CcmDepositMetadata, }; use cf_primitives::{ - chains::assets::arb::Asset as ArbAsset, Asset, AssetAmount, Beneficiary, EpochIndex, + chains::assets::arb::Asset as ArbAsset, Asset, AssetAmount, Beneficiary, ChannelId, EpochIndex, }; use cf_utilities::task_scope::Scope; use futures_core::Future; use itertools::Itertools; +use pallet_cf_ingress_egress::VaultDepositWitness; use sp_core::H160; use crate::{ @@ -185,7 +186,10 @@ impl super::evm::vault::IngressCallBuilder for ArbCallBuilder { type Chain = Arbitrum; fn vault_swap_request( + block_height: u64, source_asset: Asset, + deposit_address: cf_chains::eth::Address, + channel_id: ChannelId, deposit_amount: AssetAmount, destination_asset: Asset, destination_address: EncodedAddress, @@ -195,24 +199,29 @@ impl super::evm::vault::IngressCallBuilder for ArbCallBuilder { ) -> state_chain_runtime::RuntimeCall { state_chain_runtime::RuntimeCall::ArbitrumIngressEgress( pallet_cf_ingress_egress::Call::vault_swap_request { - input_asset: source_asset.try_into().expect("invalid asset for chain"), - output_asset: destination_asset, - deposit_amount, - destination_address, - deposit_metadata, - tx_id, - deposit_details: Box::new(DepositDetails { tx_hashes: Some(vec![tx_id]) }), - broker_fee: vault_swap_parameters.broker_fee, - affiliate_fees: vault_swap_parameters - .affiliate_fees - .into_iter() - .map(|entry| Beneficiary { account: entry.affiliate, bps: entry.fee.into() }) - .collect_vec() - .try_into() - .expect("runtime supports at least as many affiliates as we allow in cf_parameters encoding"), - boost_fee: vault_swap_parameters.boost_fee.into(), - dca_params: vault_swap_parameters.dca_params, - refund_params: Box::new(vault_swap_parameters.refund_params), + block_height, + deposits: vec![VaultDepositWitness { + input_asset: source_asset.try_into().expect("invalid asset for chain"), + output_asset: destination_asset, + deposit_amount, + destination_address, + deposit_metadata, + tx_id, + deposit_details: DepositDetails { tx_hashes: Some(vec![tx_id]) }, + broker_fee: vault_swap_parameters.broker_fee, + affiliate_fees: vault_swap_parameters + .affiliate_fees + .into_iter() + .map(|entry| Beneficiary { account: entry.affiliate, bps: entry.fee.into() }) + .collect_vec() + .try_into() + .expect("runtime supports at least as many affiliates as we allow in cf_parameters encoding"), + boost_fee: vault_swap_parameters.boost_fee.into(), + dca_params: vault_swap_parameters.dca_params, + refund_params: vault_swap_parameters.refund_params, + channel_id, + deposit_address, + }], }, ) } diff --git a/engine/src/witness/btc/deposits.rs b/engine/src/witness/btc/deposits.rs index cc3f556003..d5394a3d88 100644 --- a/engine/src/witness/btc/deposits.rs +++ b/engine/src/witness/btc/deposits.rs @@ -6,8 +6,12 @@ use itertools::Itertools; use pallet_cf_ingress_egress::{DepositChannelDetails, DepositWitness}; use state_chain_runtime::BitcoinInstance; -use super::super::common::chunked_chain_source::chunked_by_vault::{ - builder::ChunkedByVaultBuilder, private_deposit_channels::BrokerPrivateChannels, ChunkedByVault, +use super::{ + super::common::chunked_chain_source::chunked_by_vault::{ + builder::ChunkedByVaultBuilder, private_deposit_channels::BrokerPrivateChannels, + ChunkedByVault, + }, + vault_swaps::BtcIngressEgressCall, }; use crate::{ btc::rpc::VerboseTransaction, @@ -71,6 +75,7 @@ impl ChunkedByVaultBuilder { private_channels.clone().into_iter().map(move |(broker_id, channel_id)| { ( broker_id, + channel_id, DepositAddress::new( key, channel_id.try_into().expect("BTC channel id must fit in u32"), @@ -80,14 +85,23 @@ impl ChunkedByVaultBuilder { }) }; - for (broker_id, vault_address) in vault_addresses { + for (broker_id, channel_id, vault_address) in vault_addresses { for tx in &txs { - if let Some(call) = super::vault_swaps::try_extract_vault_swap_call( + if let Some(deposit) = super::vault_swaps::try_extract_vault_swap_witness( tx, &vault_address, + channel_id, &broker_id, ) { - process_call(call.into(), epoch.index).await; + process_call( + BtcIngressEgressCall::vault_swap_request { + block_height: header.index, + deposits: vec![deposit], + } + .into(), + epoch.index, + ) + .await; } } } diff --git a/engine/src/witness/btc/vault_swaps.rs b/engine/src/witness/btc/vault_swaps.rs index 8465f270ed..d51a2f94a7 100644 --- a/engine/src/witness/btc/vault_swaps.rs +++ b/engine/src/witness/btc/vault_swaps.rs @@ -8,7 +8,7 @@ use cf_chains::{ }, ChannelRefundParameters, ForeignChainAddress, }; -use cf_primitives::{AccountId, Beneficiary, DcaParameters}; +use cf_primitives::{AccountId, Beneficiary, ChannelId, DcaParameters}; use cf_utilities::SliceToArray; use codec::Decode; use itertools::Itertools; @@ -77,14 +77,18 @@ fn script_buf_to_script_pubkey(script: &ScriptBuf) -> Option { Some(pubkey) } -type BtcIngressEgressCall = +pub(super) type BtcIngressEgressCall = pallet_cf_ingress_egress::Call; -pub fn try_extract_vault_swap_call( +type VaultDepositWitness = + pallet_cf_ingress_egress::VaultDepositWitness; + +pub fn try_extract_vault_swap_witness( tx: &VerboseTransaction, vault_address: &DepositAddress, + channel_id: ChannelId, broker_id: &AccountId, -) -> Option { +) -> Option { // A correctly constructed transaction carrying CF swap parameters must have at least 3 outputs: let [utxo_to_vault, nulldata_utxo, change_utxo, ..] = &tx.vout[..] else { return None; @@ -130,18 +134,18 @@ pub fn try_extract_vault_swap_call( let tx_id: [u8; 32] = tx.txid.to_byte_array(); - Some(BtcIngressEgressCall::vault_swap_request { + Some(VaultDepositWitness { input_asset: NATIVE_ASSET, output_asset: data.output_asset, deposit_amount, destination_address: data.output_address, tx_id: H256::from(tx_id), - deposit_details: Box::new(Utxo { + deposit_details: Utxo { // we require the deposit to be the first UTXO id: UtxoId { tx_id: tx_id.into(), vout: 0 }, amount: deposit_amount, deposit_address: vault_address.clone(), - }), + }, deposit_metadata: None, // No ccm for BTC (yet?) broker_fee: Beneficiary { account: broker_id.clone(), @@ -155,17 +159,19 @@ pub fn try_extract_vault_swap_call( .collect_vec() .try_into() .expect("runtime supports at least as many affiliates as we allow in UTXO encoding"), - refund_params: Box::new(ChannelRefundParameters { + refund_params: ChannelRefundParameters { retry_duration: data.parameters.retry_duration.into(), refund_address: ForeignChainAddress::Btc(refund_address), min_price, - }), + }, dca_params: Some(DcaParameters { number_of_chunks: data.parameters.number_of_chunks.into(), chunk_interval: data.parameters.chunk_interval.into(), }), // This is only to be checked in the pre-witnessed version boost_fee: data.parameters.boost_fee.into(), + channel_id, + deposit_address: vault_address.script_pubkey(), }) } @@ -288,38 +294,42 @@ mod tests { None, ); + const CHANNEL_ID: ChannelId = 7; + assert_eq!( - try_extract_vault_swap_call(&tx, &vault_deposit_address, &BROKER), - Some(BtcIngressEgressCall::vault_swap_request { + try_extract_vault_swap_witness(&tx, &vault_deposit_address, CHANNEL_ID, &BROKER), + Some(VaultDepositWitness { input_asset: NATIVE_ASSET, output_asset: MOCK_SWAP_PARAMS.output_asset, deposit_amount: DEPOSIT_AMOUNT, destination_address: MOCK_SWAP_PARAMS.output_address.clone(), tx_id: tx.txid.to_byte_array().into(), - deposit_details: Box::new(Utxo { + deposit_details: Utxo { id: UtxoId { tx_id: tx.txid.to_byte_array().into(), vout: 0 }, amount: DEPOSIT_AMOUNT, - deposit_address: vault_deposit_address, - }), + deposit_address: vault_deposit_address.clone(), + }, broker_fee: Beneficiary { account: BROKER, bps: MOCK_SWAP_PARAMS.parameters.broker_fee.into() }, affiliate_fees: bounded_vec![MOCK_SWAP_PARAMS.parameters.affiliates[0].into()], deposit_metadata: None, - refund_params: Box::new(ChannelRefundParameters { + refund_params: ChannelRefundParameters { retry_duration: MOCK_SWAP_PARAMS.parameters.retry_duration.into(), refund_address: ForeignChainAddress::Btc(refund_pubkey), min_price: sqrt_price_to_price(bounded_sqrt_price( MOCK_SWAP_PARAMS.parameters.min_output_amount.into(), DEPOSIT_AMOUNT.into(), )), - }), + }, dca_params: Some(DcaParameters { number_of_chunks: MOCK_SWAP_PARAMS.parameters.number_of_chunks.into(), chunk_interval: MOCK_SWAP_PARAMS.parameters.chunk_interval.into(), }), boost_fee: MOCK_SWAP_PARAMS.parameters.boost_fee.into(), + deposit_address: vault_deposit_address.script_pubkey(), + channel_id: CHANNEL_ID, }) ); } diff --git a/engine/src/witness/eth.rs b/engine/src/witness/eth.rs index ade7ed7e89..0ee789688a 100644 --- a/engine/src/witness/eth.rs +++ b/engine/src/witness/eth.rs @@ -8,10 +8,13 @@ use cf_chains::{ evm::{DepositDetails, H256}, CcmDepositMetadata, Ethereum, }; -use cf_primitives::{chains::assets::eth::Asset as EthAsset, Asset, AssetAmount, EpochIndex}; +use cf_primitives::{ + chains::assets::eth::Asset as EthAsset, Asset, AssetAmount, ChannelId, EpochIndex, +}; use cf_utilities::task_scope::Scope; use futures_core::Future; use itertools::Itertools; +use pallet_cf_ingress_egress::VaultDepositWitness; use sp_core::H160; use crate::{ @@ -232,7 +235,10 @@ impl super::evm::vault::IngressCallBuilder for EthCallBuilder { type Chain = Ethereum; fn vault_swap_request( + block_height: u64, source_asset: Asset, + deposit_address: cf_chains::eth::Address, + channel_id: ChannelId, deposit_amount: AssetAmount, destination_asset: Asset, destination_address: EncodedAddress, @@ -242,24 +248,30 @@ impl super::evm::vault::IngressCallBuilder for EthCallBuilder { ) -> state_chain_runtime::RuntimeCall { state_chain_runtime::RuntimeCall::EthereumIngressEgress( pallet_cf_ingress_egress::Call::vault_swap_request { - input_asset: source_asset.try_into().expect("invalid asset for chain"), - output_asset: destination_asset, - deposit_amount, - destination_address, - deposit_metadata, - tx_id, - deposit_details: Box::new(DepositDetails { tx_hashes: Some(vec![tx_id]) }), - broker_fee: vault_swap_parameters.broker_fee, - affiliate_fees: vault_swap_parameters - .affiliate_fees - .into_iter() - .map(Into::into) - .collect_vec() - .try_into() - .expect("runtime supports at least as many affiliates as we allow in cf_parameters encoding"), - boost_fee: vault_swap_parameters.boost_fee.into(), - dca_params: vault_swap_parameters.dca_params, - refund_params: Box::new(vault_swap_parameters.refund_params), + block_height, + deposits: vec![ VaultDepositWitness { + input_asset: source_asset.try_into().expect("invalid asset for chain"), + output_asset: destination_asset, + deposit_amount, + destination_address, + deposit_metadata, + tx_id, + deposit_details: DepositDetails { tx_hashes: Some(vec![tx_id]) }, + broker_fee: vault_swap_parameters.broker_fee, + affiliate_fees: vault_swap_parameters + .affiliate_fees + .into_iter() + .map(Into::into) + .collect_vec() + .try_into() + .expect("runtime supports at least as many affiliates as we allow in cf_parameters encoding"), + boost_fee: vault_swap_parameters.boost_fee.into(), + dca_params: vault_swap_parameters.dca_params, + refund_params: vault_swap_parameters.refund_params, + channel_id, + deposit_address, + } + ], }, ) } diff --git a/engine/src/witness/evm/vault.rs b/engine/src/witness/evm/vault.rs index de1fb73040..99e4f0dc96 100644 --- a/engine/src/witness/evm/vault.rs +++ b/engine/src/witness/evm/vault.rs @@ -21,16 +21,18 @@ use cf_chains::{ evm::DepositDetails, CcmChannelMetadata, CcmDepositMetadata, Chain, }; -use cf_primitives::{Asset, AssetAmount, EpochIndex, ForeignChain}; +use cf_primitives::{Asset, AssetAmount, ChannelId, EpochIndex, ForeignChain}; use ethers::prelude::*; use state_chain_runtime::{EthereumInstance, Runtime, RuntimeCall}; abigen!(Vault, "$CF_ETH_CONTRACT_ABI_ROOT/$CF_ETH_CONTRACT_ABI_TAG/IVault.json"); pub fn call_from_event< - C: cf_chains::Chain, + C: cf_chains::Chain, CallBuilder: IngressCallBuilder, >( + block_height: u64, + contract_address: EthereumAddress, event: Event, // can be different for different EVM chains native_asset: Asset, @@ -56,6 +58,10 @@ where }) } + // The deposit address and channel id are always the same (unlike BTC vault swaps): + let deposit_address = contract_address; + let channel_id = 0; + Ok(match event.event_parameters { VaultEvents::SwapNativeFilter(SwapNativeFilter { dst_chain, @@ -71,7 +77,10 @@ where }) = VersionedCfParameters::decode(&mut &cf_parameters[..])?; Some(CallBuilder::vault_swap_request( + block_height, native_asset, + deposit_address, + channel_id, try_into_primitive(amount)?, try_into_primitive(dst_token)?, try_into_encoded_address(try_into_primitive(dst_chain)?, dst_address.to_vec())?, @@ -95,9 +104,12 @@ where }) = VersionedCfParameters::decode(&mut &cf_parameters[..])?; Some(CallBuilder::vault_swap_request( + block_height, *(supported_assets .get(&src_token) .ok_or(anyhow!("Source token {src_token:?} not found"))?), + deposit_address, + channel_id, try_into_primitive(amount)?, try_into_primitive(dst_token)?, try_into_encoded_address(try_into_primitive(dst_chain)?, dst_address.to_vec())?, @@ -122,7 +134,10 @@ where }) = VersionedCcmCfParameters::decode(&mut &cf_parameters[..])?; Some(CallBuilder::vault_swap_request( + block_height, native_asset, + deposit_address, + channel_id, try_into_primitive(amount)?, try_into_primitive(dst_token)?, try_into_encoded_address(try_into_primitive(dst_chain)?, dst_address.to_vec())?, @@ -163,9 +178,12 @@ where }) = VersionedCcmCfParameters::decode(&mut &cf_parameters[..])?; Some(CallBuilder::vault_swap_request( + block_height, *(supported_assets .get(&src_token) .ok_or(anyhow!("Source token {src_token:?} not found"))?), + deposit_address, + channel_id, try_into_primitive(amount)?, try_into_primitive(dst_token)?, try_into_encoded_address(try_into_primitive(dst_chain)?, dst_address.to_vec())?, @@ -224,7 +242,10 @@ pub trait IngressCallBuilder { type Chain: cf_chains::Chain; fn vault_swap_request( + block_height: ::ChainBlockNumber, source_asset: Asset, + deposit_address: ::ChainAccount, + channel_id: ChannelId, deposit_amount: cf_primitives::AssetAmount, destination_asset: Asset, destination_address: EncodedAddress, @@ -285,6 +306,8 @@ impl ChunkedByVaultBuilder { .await? { match call_from_event::( + header.index, + contract_address, event, native_asset, source_chain, diff --git a/state-chain/cf-integration-tests/src/solana.rs b/state-chain/cf-integration-tests/src/solana.rs index d04d3bf093..a929e48031 100644 --- a/state-chain/cf-integration-tests/src/solana.rs +++ b/state-chain/cf-integration-tests/src/solana.rs @@ -34,7 +34,7 @@ use pallet_cf_elections::{ vote_storage::{composite::tuple_7_impls::CompositeVote, AuthorityVote}, CompositeAuthorityVoteOf, CompositeElectionIdentifierOf, MAXIMUM_VOTES_PER_EXTRINSIC, }; -use pallet_cf_ingress_egress::{DepositWitness, FetchOrTransfer}; +use pallet_cf_ingress_egress::{DepositWitness, FetchOrTransfer, VaultDepositWitness}; use pallet_cf_validator::RotationPhase; use sp_core::ConstU32; use sp_runtime::BoundedBTreeMap; @@ -437,6 +437,30 @@ fn can_send_solana_ccm() { }); } +fn vault_swap_deposit_witness( + deposit_metadata: Option, +) -> VaultDepositWitness { + VaultDepositWitness { + input_asset: SolAsset::Sol, + output_asset: Asset::SolUsdc, + deposit_amount: 1_000_000_000_000u64, + destination_address: EncodedAddress::Sol([1u8; 32]), + deposit_metadata, + tx_id: Default::default(), + deposit_details: (), + broker_fee: cf_primitives::Beneficiary { + account: sp_runtime::AccountId32::new([0; 32]), + bps: 0, + }, + affiliate_fees: Default::default(), + refund_params: REFUND_PARAMS, + dca_params: None, + boost_fee: 0, + deposit_address: SolAddress([2u8; 32]), + channel_id: 0, + } +} + #[test] fn solana_ccm_fails_with_invalid_input() { const EPOCH_BLOCKS: u32 = 100; @@ -507,21 +531,8 @@ fn solana_ccm_fails_with_invalid_input() { // Contract call fails with invalid CCM assert_ok!(RuntimeCall::SolanaIngressEgress( pallet_cf_ingress_egress::Call::vault_swap_request { - input_asset: SolAsset::Sol, - output_asset: Asset::SolUsdc, - deposit_amount: 1_000_000_000_000u64, - destination_address: EncodedAddress::Sol([1u8; 32]), - deposit_metadata: Some(invalid_ccm), - tx_id: Default::default(), - deposit_details: Box::new(()), - broker_fee: cf_primitives::Beneficiary { - account: sp_runtime::AccountId32::new([0; 32]), - bps: 0, - }, - affiliate_fees: Default::default(), - refund_params: Box::new(REFUND_PARAMS), - dca_params: None, - boost_fee: 0, + block_height: 0, + deposits: vec![vault_swap_deposit_witness(Some(invalid_ccm))], } ) .dispatch_bypass_filter( @@ -565,21 +576,8 @@ fn solana_ccm_fails_with_invalid_input() { witness_call(RuntimeCall::SolanaIngressEgress( pallet_cf_ingress_egress::Call::vault_swap_request { - input_asset: SolAsset::Sol, - output_asset: Asset::SolUsdc, - deposit_amount: 1_000_000_000_000u64, - destination_address: EncodedAddress::Sol([1u8; 32]), - deposit_metadata: Some(ccm), - tx_id: Default::default(), - deposit_details: Box::new(()), - broker_fee: cf_primitives::Beneficiary { - account: sp_runtime::AccountId32::new([0; 32]), - bps: 0, - }, - affiliate_fees: Default::default(), - refund_params: Box::new(REFUND_PARAMS), - dca_params: None, - boost_fee: 0, + block_height: 0, + deposits: vec![vault_swap_deposit_witness(Some(ccm))], }, )); // Setting the current agg key will invalidate the CCM. @@ -793,23 +791,9 @@ fn solana_ccm_execution_error_can_trigger_fallback() { }; witness_call(RuntimeCall::SolanaIngressEgress( pallet_cf_ingress_egress::Call::vault_swap_request { - input_asset: SolAsset::Sol, - output_asset: Asset::SolUsdc, - deposit_amount: 1_000_000_000_000u64, - destination_address: EncodedAddress::Sol([1u8; 32]), - deposit_metadata: Some(ccm), - tx_id: Default::default(), - deposit_details: Box::new(()), - broker_fee: cf_primitives::Beneficiary { - account: sp_runtime::AccountId32::new([0; 32]), - bps: 0, - }, - affiliate_fees: Default::default(), - refund_params: Box::new(REFUND_PARAMS), - dca_params: None, - boost_fee: 0, - - }, + block_height: 0, + deposits: vec![vault_swap_deposit_witness(Some(ccm))], + } )); // Wait for the swaps to complete and call broadcasted. diff --git a/state-chain/cf-integration-tests/src/swapping.rs b/state-chain/cf-integration-tests/src/swapping.rs index d09ea5b8ee..c41794916c 100644 --- a/state-chain/cf-integration-tests/src/swapping.rs +++ b/state-chain/cf-integration-tests/src/swapping.rs @@ -37,10 +37,10 @@ use pallet_cf_broadcast::{ AwaitingBroadcast, BroadcastIdCounter, PendingApiCalls, RequestFailureCallbacks, RequestSuccessCallbacks, }; -use pallet_cf_ingress_egress::{DepositWitness, FailedForeignChainCall}; +use pallet_cf_ingress_egress::{DepositWitness, FailedForeignChainCall, VaultDepositWitness}; use pallet_cf_pools::{HistoricalEarnedFees, OrderId, RangeOrderSize}; use pallet_cf_swapping::{SwapRequestIdCounter, SwapRetryDelay}; -use sp_core::U256; +use sp_core::{H160, U256}; use state_chain_runtime::{ chainflip::{ address_derivation::AddressDerivation, ChainAddressConverter, EthTransactionBuilder, @@ -569,6 +569,31 @@ fn ccm_deposit_metadata_mock() -> CcmDepositMetadata { } } +fn vault_swap_deposit_witness( + deposit_amount: u128, + output_asset: Asset, +) -> VaultDepositWitness { + VaultDepositWitness { + input_asset: EthAsset::Eth, + output_asset, + deposit_amount, + destination_address: EncodedAddress::Eth([0x02; 20]), + deposit_metadata: Some(ccm_deposit_metadata_mock()), + tx_id: Default::default(), + deposit_details: DepositDetails { tx_hashes: None }, + broker_fee: cf_primitives::Beneficiary { + account: sp_runtime::AccountId32::new([0; 32]), + bps: 0, + }, + affiliate_fees: Default::default(), + refund_params: ETH_REFUND_PARAMS, + dca_params: None, + boost_fee: 0, + deposit_address: H160::from([0x03; 20]), + channel_id: 0, + } +} + #[test] fn can_process_ccm_via_direct_deposit() { super::genesis::with_test_defaults().build().execute_with(|| { @@ -578,21 +603,8 @@ fn can_process_ccm_via_direct_deposit() { witness_call(RuntimeCall::EthereumIngressEgress( pallet_cf_ingress_egress::Call::vault_swap_request { - input_asset: EthAsset::Flip, - output_asset: Asset::Usdc, - deposit_amount, - destination_address: EncodedAddress::Eth([0x02; 20]), - deposit_metadata: Some(ccm_deposit_metadata_mock()), - tx_id: Default::default(), - deposit_details: Box::new(DepositDetails { tx_hashes: None }), - broker_fee: cf_primitives::Beneficiary { - account: sp_runtime::AccountId32::new([0; 32]), - bps: 0, - }, - affiliate_fees: Default::default(), - refund_params: Box::new(ETH_REFUND_PARAMS), - dca_params: None, - boost_fee: 0, + block_height: 0, + deposits: vec![vault_swap_deposit_witness(deposit_amount, Asset::Usdc)], }, )); @@ -632,21 +644,8 @@ fn failed_swaps_are_rolled_back() { witness_call(RuntimeCall::EthereumIngressEgress( pallet_cf_ingress_egress::Call::vault_swap_request { - input_asset: EthAsset::Eth, - output_asset: Asset::Flip, - deposit_amount: 10_000 * DECIMALS, - destination_address: EncodedAddress::Eth(Default::default()), - tx_id: Default::default(), - deposit_metadata: None, - deposit_details: Box::new(DepositDetails { tx_hashes: None }), - broker_fee: cf_primitives::Beneficiary { - account: sp_runtime::AccountId32::new([0; 32]), - bps: 0, - }, - affiliate_fees: Default::default(), - refund_params: Box::new(ETH_REFUND_PARAMS), - dca_params: None, - boost_fee: 0, + block_height: 0, + deposits: vec![vault_swap_deposit_witness(10_000 * DECIMALS, Asset::Flip)], }, )); @@ -798,22 +797,8 @@ fn can_resign_failed_ccm() { witness_call(RuntimeCall::EthereumIngressEgress( pallet_cf_ingress_egress::Call::vault_swap_request { - input_asset: EthAsset::Flip, - output_asset: Asset::Usdc, - deposit_amount: 10_000_000_000_000, - destination_address: EncodedAddress::Eth([0x02; 20]), - deposit_metadata: Some(ccm_deposit_metadata_mock()), - tx_id: Default::default(), - deposit_details: Box::new(DepositDetails { tx_hashes: None }), - broker_fee: cf_primitives::Beneficiary { - account: sp_runtime::AccountId32::new([0; 32]), - bps: 0, - }, - affiliate_fees: Default::default(), - - refund_params: Box::new(ETH_REFUND_PARAMS), - dca_params: None, - boost_fee: 0, + block_height: 0, + deposits: vec![vault_swap_deposit_witness(10_000_000_000_000, Asset::Usdc)], }, )); diff --git a/state-chain/chains/src/lib.rs b/state-chain/chains/src/lib.rs index a063e1982b..ed5a855183 100644 --- a/state-chain/chains/src/lib.rs +++ b/state-chain/chains/src/lib.rs @@ -665,6 +665,12 @@ pub enum SwapOrigin { Internal, } +#[derive(Clone, Debug, PartialEq, Eq, Encode, Decode, TypeInfo)] +pub enum DepositOriginType { + DepositChannel, + Vault, +} + pub const MAX_CCM_MSG_LENGTH: u32 = 10_000; pub const MAX_CCM_ADDITIONAL_DATA_LENGTH: u32 = 1_000; @@ -785,6 +791,8 @@ impl
CcmDepositMetadataGeneric
{ let principal_swap_amount = deposit_amount.saturating_sub(gas_budget); + // TODO: we already check ccm support when opening a channel (and we have to). + // If we can also check this in vault swaps, we should be able to remove this here. let destination_chain: ForeignChain = destination_asset.into(); if !destination_chain.ccm_support() { return Err(CcmFailReason::UnsupportedForTargetChain) diff --git a/state-chain/pallets/cf-ingress-egress/src/benchmarking.rs b/state-chain/pallets/cf-ingress-egress/src/benchmarking.rs index 3d37dd5598..891dd2667e 100644 --- a/state-chain/pallets/cf-ingress-egress/src/benchmarking.rs +++ b/state-chain/pallets/cf-ingress-egress/src/benchmarking.rs @@ -307,22 +307,27 @@ mod benchmarks { }, }; let call = Call::::vault_swap_request { - input_asset: BenchmarkValue::benchmark_value(), - deposit_amount: 1_000u32.into(), - output_asset: Asset::Eth, - destination_address: EncodedAddress::benchmark_value(), - deposit_metadata: Some(deposit_metadata), - tx_id: TransactionInIdFor::::benchmark_value(), - deposit_details: Box::new(BenchmarkValue::benchmark_value()), - broker_fee: cf_primitives::Beneficiary { account: account("broker", 0, 0), bps: 0 }, - affiliate_fees: Default::default(), - refund_params: Box::new(ChannelRefundParameters { - retry_duration: Default::default(), - refund_address: ForeignChainAddress::Eth(Default::default()), - min_price: Default::default(), - }), - dca_params: None, - boost_fee: 0, + block_height: 0u32.into(), + deposits: vec![VaultDepositWitness { + input_asset: BenchmarkValue::benchmark_value(), + output_asset: Asset::Eth, + deposit_amount: 1_000u32.into(), + destination_address: EncodedAddress::benchmark_value(), + deposit_metadata: Some(deposit_metadata), + tx_id: TransactionInIdFor::::benchmark_value(), + deposit_details: BenchmarkValue::benchmark_value(), + broker_fee: cf_primitives::Beneficiary { account: account("broker", 0, 0), bps: 0 }, + affiliate_fees: Default::default(), + refund_params: ChannelRefundParameters { + retry_duration: Default::default(), + refund_address: ForeignChainAddress::Eth(Default::default()), + min_price: Default::default(), + }, + dca_params: None, + boost_fee: 0, + channel_id: 0, + deposit_address: BenchmarkValue::benchmark_value(), + }], }; #[block] diff --git a/state-chain/pallets/cf-ingress-egress/src/lib.rs b/state-chain/pallets/cf-ingress-egress/src/lib.rs index 92cb0cc458..e4148c866f 100644 --- a/state-chain/pallets/cf-ingress-egress/src/lib.rs +++ b/state-chain/pallets/cf-ingress-egress/src/lib.rs @@ -30,9 +30,9 @@ use cf_chains::{ ccm_checker::CcmValidityCheck, AllBatch, AllBatchError, CcmAdditionalData, CcmChannelMetadata, CcmDepositMetadata, CcmFailReason, CcmMessage, Chain, ChainCrypto, ChannelLifecycleHooks, ChannelRefundParameters, - ConsolidateCall, DepositChannel, DepositDetailsToTransactionInId, ExecutexSwapAndCall, - FetchAssetParams, ForeignChainAddress, IntoTransactionInIdForAnyChain, RejectCall, SwapOrigin, - TransferAssetParams, + ConsolidateCall, DepositChannel, DepositDetailsToTransactionInId, DepositOriginType, + ExecutexSwapAndCall, FetchAssetParams, ForeignChainAddress, IntoTransactionInIdForAnyChain, + RejectCall, SwapOrigin, TransactionInIdForAnyChain, TransferAssetParams, }; use cf_primitives::{ AccountRole, AffiliateShortId, Affiliates, Asset, AssetAmount, BasisPoints, Beneficiaries, @@ -62,7 +62,6 @@ use scale_info::{ }; use sp_runtime::traits::UniqueSaturatedInto; use sp_std::{ - boxed::Box, collections::{btree_map::BTreeMap, btree_set::BTreeSet}, marker::PhantomData, vec, @@ -140,6 +139,76 @@ pub enum DepositIgnoredReason { TransactionTainted, } +enum FullWitnessDepositOutcome { + BoostFinalised, + DepositActionPerformed, +} + +mod deposit_origin { + + use super::*; + + // TODO: replace SwapOrigin with this? + #[derive(Clone)] + pub(super) enum DepositOrigin { + DepositChannel { + deposit_address: EncodedAddress, + channel_id: ChannelId, + deposit_block_height: u64, + }, + Vault { + tx_id: TransactionInIdForAnyChain, + }, + } + + impl DepositOrigin { + pub(super) fn deposit_channel, I: 'static>( + deposit_address: ::ChainAccount, + channel_id: ChannelId, + deposit_block_height: ::ChainBlockNumber, + ) -> Self { + DepositOrigin::DepositChannel { + deposit_address: T::AddressConverter::to_encoded_address( + ::ChainAccount::into_foreign_chain_address( + deposit_address.clone(), + ), + ), + channel_id, + deposit_block_height: deposit_block_height.into(), + } + } + + pub(super) fn vault, I: 'static>(tx_id: TransactionInIdFor) -> Self { + DepositOrigin::Vault { tx_id: tx_id.into_transaction_in_id_for_any_chain() } + } + } + + impl From for DepositOriginType { + fn from(origin: DepositOrigin) -> Self { + match origin { + DepositOrigin::DepositChannel { .. } => DepositOriginType::DepositChannel, + DepositOrigin::Vault { .. } => DepositOriginType::Vault, + } + } + } + + impl From for SwapOrigin { + fn from(origin: DepositOrigin) -> SwapOrigin { + match origin { + DepositOrigin::Vault { tx_id } => SwapOrigin::Vault { tx_id }, + DepositOrigin::DepositChannel { + deposit_address, + channel_id, + deposit_block_height, + } => + SwapOrigin::DepositChannel { deposit_address, channel_id, deposit_block_height }, + } + } + } +} + +use deposit_origin::DepositOrigin; + /// Holds information about a tainted transaction. #[derive(RuntimeDebug, PartialEq, Eq, Encode, Decode, TypeInfo, CloneNoBound)] #[scale_info(skip_type_params(T, I))] @@ -283,6 +352,27 @@ pub mod pallet { pub deposit_details: C::DepositDetails, } + #[derive( + CloneNoBound, RuntimeDebugNoBound, PartialEqNoBound, EqNoBound, Encode, Decode, TypeInfo, + )] + #[scale_info(skip_type_params(T, I))] + pub struct VaultDepositWitness, I: 'static> { + pub input_asset: TargetChainAsset, + pub deposit_address: TargetChainAccount, + pub channel_id: ChannelId, + pub deposit_amount: ::ChainAmount, + pub deposit_details: ::DepositDetails, + pub output_asset: Asset, + pub destination_address: EncodedAddress, + pub deposit_metadata: Option, + pub tx_id: TransactionInIdFor, + pub broker_fee: Beneficiary, + pub affiliate_fees: Affiliates, + pub refund_params: ChannelRefundParameters, + pub dca_params: Option, + pub boost_fee: BasisPoints, + } + #[derive(CloneNoBound, RuntimeDebug, PartialEq, Eq, Encode, Decode, TypeInfo)] #[scale_info(skip_type_params(T, I))] pub struct DepositChannelDetails, I: 'static> { @@ -598,6 +688,16 @@ pub mod pallet { pub(crate) type FailedRejections, I: 'static = ()> = StorageValue<_, Vec>, ValueQuery>; + /// Stores transaction ids that have been boosted but have not yet been finalised. + #[pallet::storage] + pub(crate) type BoostedVaultTxs, I: 'static = ()> = StorageMap< + _, + Identity, + TransactionInIdFor, + BoostStatus>, + OptionQuery, + >; + #[pallet::event] #[pallet::generate_deposit(pub(super) fn deposit_event)] pub enum Event, I: 'static = ()> { @@ -612,6 +712,7 @@ pub mod pallet { ingress_fee: TargetChainAmount, action: DepositAction, channel_id: ChannelId, + origin_type: DepositOriginType, }, AssetEgressStatusChanged { asset: TargetChainAsset, @@ -699,6 +800,7 @@ pub mod pallet { // Total fee the user paid for their deposit to be boosted. boost_fee: TargetChainAmount, action: DepositAction, + origin_type: DepositOriginType, }, BoostFundsAdded { booster_id: T::AccountId, @@ -1301,37 +1403,19 @@ pub mod pallet { #[pallet::weight(T::WeightInfo::vault_swap_request())] pub fn vault_swap_request( origin: OriginFor, - input_asset: TargetChainAsset, - output_asset: Asset, - deposit_amount: ::ChainAmount, - destination_address: EncodedAddress, - deposit_metadata: Option, - tx_id: TransactionInIdFor, - deposit_details: Box<::DepositDetails>, - broker_fee: Beneficiary, - affiliate_fees: Affiliates, - refund_params: Box, - dca_params: Option, - boost_fee: BasisPoints, + block_height: TargetChainBlockNumber, + deposits: Vec>, ) -> DispatchResult { if T::EnsureWitnessed::ensure_origin(origin.clone()).is_ok() { - Self::process_vault_swap_request( - input_asset, - deposit_amount, - output_asset, - destination_address, - deposit_metadata, - tx_id, - *deposit_details, - broker_fee, - affiliate_fees, - *refund_params, - dca_params, - boost_fee, - ); + for deposit in deposits { + Self::process_vault_swap_request_full_witness(block_height, deposit); + } } else { T::EnsurePrewitnessed::ensure_origin(origin)?; - // Pre-witnessed vault swaps are not supported yet. + + for deposit in deposits { + Self::process_vault_swap_request_prewitness(block_height, deposit); + } } Ok(()) @@ -1727,12 +1811,6 @@ impl, I: 'static> Pallet { ) -> DispatchResult { for DepositWitness { deposit_address, asset, amount, deposit_details } in deposit_witnesses { - if amount < MinimumDeposit::::get(asset) { - // We do not process/record pre-witnessed deposits for amounts smaller - // than MinimumDeposit to match how this is done on finalisation - continue; - } - let DepositChannelDetails { deposit_channel, action, @@ -1743,95 +1821,32 @@ impl, I: 'static> Pallet { } = DepositChannelLookup::::get(&deposit_address) .ok_or(Error::::InvalidDepositAddress)?; - if let Some(tx_id) = deposit_details.deposit_id() { - if TaintedTransactions::::mutate(&owner, &tx_id, |opt| { - match opt.as_mut() { - // Transaction has been reported, mark it as pre-witnessed. - Some(status @ TaintedTransactionStatus::Unseen) => { - *status = TaintedTransactionStatus::Prewitnessed; - true - }, - // Pre-witnessing twice is unlikely but possible. Either way we don't want - // to change the status and we don't want to allow boosting. - Some(TaintedTransactionStatus::Prewitnessed) => true, - // Transaction has not been reported, mark it as boosted to prevent further - // reports. - None => false, - } - }) { - continue; - } - } + let origin = DepositOrigin::deposit_channel::( + deposit_address.clone(), + deposit_channel.channel_id, + block_height, + ); - let prewitnessed_deposit_id = - PrewitnessedDepositIdCounter::::mutate(|id| -> u64 { - *id = id.saturating_add(1); - *id + if let Some(new_boost_status) = Self::process_prewitness_deposit_inner( + amount, + asset, + deposit_details, + deposit_address.clone(), + None, // source address is unknown + action, + &owner, + boost_fee, + boost_status, + deposit_channel.channel_id, + block_height, + origin, + ) { + // Update boost status + DepositChannelLookup::::mutate(&deposit_address, |details| { + if let Some(details) = details { + details.boost_status = new_boost_status; + } }); - - let channel_id = deposit_channel.channel_id; - - // Only boost on non-zero fee and if the channel isn't already boosted: - if T::SafeMode::get().boost_deposits_enabled && - boost_fee > 0 && - !matches!(boost_status, BoostStatus::Boosted { .. }) - { - match Self::try_boosting(asset, amount, boost_fee, prewitnessed_deposit_id) { - Ok(BoostOutput { used_pools, total_fee: boost_fee_amount }) => { - DepositChannelLookup::::mutate(&deposit_address, |details| { - if let Some(details) = details { - details.boost_status = BoostStatus::Boosted { - prewitnessed_deposit_id, - pools: used_pools.keys().cloned().collect(), - amount, - }; - } - }); - - let amount_after_boost_fee = amount.saturating_sub(boost_fee_amount); - - // Note that ingress fee is deducted at the time of boosting rather than the - // time the deposit is finalised (which allows us to perform the channel - // action immediately): - let AmountAndFeesWithheld { amount_after_fees, fees_withheld: ingress_fee } = - Self::withhold_ingress_or_egress_fee( - IngressOrEgress::Ingress, - asset, - amount_after_boost_fee, - ); - - let action = Self::perform_channel_action( - action, - deposit_channel, - amount_after_fees, - block_height, - )?; - - Self::deposit_event(Event::DepositBoosted { - deposit_address: deposit_address.clone(), - asset, - amounts: used_pools, - block_height, - prewitnessed_deposit_id, - channel_id, - deposit_details: deposit_details.clone(), - ingress_fee, - boost_fee: boost_fee_amount, - action, - }); - }, - Err(err) => { - Self::deposit_event(Event::InsufficientBoostLiquidity { - prewitnessed_deposit_id, - asset, - amount_attempted: amount, - channel_id, - }); - log::debug!( - "Deposit (id: {prewitnessed_deposit_id}) of {amount:?} {asset:?} and boost fee {boost_fee} could not be boosted: {err:?}" - ); - }, - } } } Ok(()) @@ -1839,22 +1854,11 @@ impl, I: 'static> Pallet { fn perform_channel_action( action: ChannelAction, - DepositChannel { asset, address: deposit_address, channel_id, .. }: DepositChannel< - T::TargetChain, - >, + asset: TargetChainAsset, + source_address: Option, amount_after_fees: TargetChainAmount, - block_height: TargetChainBlockNumber, + origin: DepositOrigin, ) -> Result, DispatchError> { - let swap_origin = SwapOrigin::DepositChannel { - deposit_address: T::AddressConverter::to_encoded_address( - ::ChainAccount::into_foreign_chain_address( - deposit_address.clone(), - ), - ), - channel_id, - deposit_block_height: block_height.into(), - }; - let action = match action { ChannelAction::LiquidityProvision { lp_account, .. } => { T::Balance::try_credit_account( @@ -1879,7 +1883,7 @@ impl, I: 'static> Pallet { broker_fees, refund_params, dca_params, - swap_origin, + origin.into(), ); DepositAction::Swap { swap_request_id } }, @@ -1894,7 +1898,7 @@ impl, I: 'static> Pallet { let deposit_metadata = CcmDepositMetadata { channel_metadata, source_chain: asset.into(), - source_address: None, + source_address, }; match deposit_metadata.clone().into_swap_metadata( amount_after_fees.into(), @@ -1913,7 +1917,7 @@ impl, I: 'static> Pallet { broker_fees, refund_params, dca_params, - swap_origin, + origin.into(), ); DepositAction::CcmTransfer { swap_request_id } }, @@ -1926,7 +1930,7 @@ impl, I: 'static> Pallet { deposit_metadata: deposit_metadata .clone() .to_encoded::(), - origin: swap_origin.clone(), + origin: origin.into(), }); DepositAction::NoAction }, @@ -1965,6 +1969,284 @@ impl, I: 'static> Pallet { return Err(Error::::InvalidDepositAddress.into()) } + let origin = DepositOrigin::deposit_channel::( + deposit_address.clone(), + channel_id, + block_height, + ); + + if let Ok(FullWitnessDepositOutcome::BoostFinalised) = + Self::process_full_witness_deposit_inner( + deposit_address.clone(), + asset, + deposit_amount, + deposit_details, + None, // source address is unknown + &deposit_channel_details.owner, + deposit_channel_details.boost_status, + channel_id, + deposit_channel_details.action, + block_height, + origin, + ) { + // This allows the channel to be boosted again: + DepositChannelLookup::::mutate(&deposit_address, |details| { + if let Some(details) = details { + details.boost_status = BoostStatus::NotBoosted; + } + }); + } + + Ok(()) + } + + fn assemble_broker_fees( + broker_fee: Beneficiary, + affiliate_fees: Affiliates, + ) -> Beneficiaries { + let primary_broker = broker_fee.account.clone(); + + if T::AccountRoleRegistry::has_account_role(&primary_broker, AccountRole::Broker) { + [broker_fee] + .into_iter() + .chain(affiliate_fees.into_iter().filter_map( + |Beneficiary { account: short_affiliate_id, bps }| { + if let Some(affiliate_id) = T::AffiliateRegistry::get_account_id( + &primary_broker, + short_affiliate_id, + ) { + Some(Beneficiary { account: affiliate_id, bps }) + } else { + // In case the entry not found, we ignore the entry, but process the + // swap (to avoid having to refund it). + Self::deposit_event(Event::::UnknownAffiliate { + broker_id: primary_broker.clone(), + short_affiliate_id, + }); + + None + } + }, + )) + .collect::>() + .try_into() + .expect( + "must fit since affiliates are limited to 1 fewer element than beneficiaries", + ) + } else { + Self::deposit_event(Event::::UnknownBroker { broker_id: primary_broker }); + Default::default() + } + } + + fn process_prewitness_deposit_inner( + amount: TargetChainAmount, + asset: TargetChainAsset, + deposit_details: ::DepositDetails, + deposit_address: TargetChainAccount, + source_address: Option, + action: ChannelAction, + broker: &T::AccountId, + boost_fee: u16, + boost_status: BoostStatus>, + channel_id: u64, + block_height: TargetChainBlockNumber, + origin: DepositOrigin, + ) -> Option>> { + if amount < MinimumDeposit::::get(asset) { + // We do not process/record pre-witnessed deposits for amounts smaller + // than MinimumDeposit to match how this is done on finalisation + return None; + } + + if let Some(tx_id) = deposit_details.deposit_id() { + if TaintedTransactions::::mutate(broker, &tx_id, |opt| { + match opt.as_mut() { + // Transaction has been reported, mark it as pre-witnessed. + Some(status @ TaintedTransactionStatus::Unseen) => { + *status = TaintedTransactionStatus::Prewitnessed; + true + }, + // Pre-witnessing twice is unlikely but possible. Either way we don't want + // to change the status and we don't want to allow boosting. + Some(TaintedTransactionStatus::Prewitnessed) => true, + // Transaction has not been reported, mark it as boosted to prevent further + // reports. + None => false, + } + }) { + return None; + } + } + + let prewitnessed_deposit_id = PrewitnessedDepositIdCounter::::mutate(|id| -> u64 { + *id = id.saturating_add(1); + *id + }); + + // Only boost on non-zero fee and if the channel isn't already boosted: + if T::SafeMode::get().boost_deposits_enabled && + boost_fee > 0 && + !matches!(boost_status, BoostStatus::Boosted { .. }) + { + match Self::try_boosting(asset, amount, boost_fee, prewitnessed_deposit_id) { + Ok(BoostOutput { used_pools, total_fee: boost_fee_amount }) => { + let amount_after_boost_fee = amount.saturating_sub(boost_fee_amount); + + // Note that ingress fee is deducted at the time of boosting rather than the + // time the deposit is finalised (which allows us to perform the channel + // action immediately): + let AmountAndFeesWithheld { amount_after_fees, fees_withheld: ingress_fee } = + Self::conditionally_withhold_ingress_fee( + asset, + amount_after_boost_fee, + &origin, + ); + + let used_pool_tiers = used_pools.keys().cloned().collect(); + + if let Ok(action) = Self::perform_channel_action( + action, + asset, + source_address, + amount_after_fees, + origin.clone(), + ) { + Self::deposit_event(Event::DepositBoosted { + deposit_address, + asset, + amounts: used_pools, + block_height, + prewitnessed_deposit_id, + channel_id, + deposit_details: deposit_details.clone(), + ingress_fee, + boost_fee: boost_fee_amount, + action, + origin_type: origin.into(), + }); + } else { + // TODO: emit error? + } + + return Some(BoostStatus::Boosted { + prewitnessed_deposit_id, + pools: used_pool_tiers, + amount, + }); + }, + Err(err) => { + Self::deposit_event(Event::InsufficientBoostLiquidity { + prewitnessed_deposit_id, + asset, + amount_attempted: amount, + channel_id, + }); + log::debug!( + "Deposit (id: {prewitnessed_deposit_id}) of {amount:?} {asset:?} and boost fee {boost_fee} could not be boosted: {err:?}" + ); + }, + } + } + + None + } + + fn process_vault_swap_request_prewitness( + block_height: TargetChainBlockNumber, + VaultDepositWitness { + input_asset: asset, + deposit_address, + channel_id, + deposit_amount: amount, + deposit_details, + output_asset, + destination_address, + deposit_metadata, + tx_id, + broker_fee, + affiliate_fees, + refund_params, + dca_params, + boost_fee, + }: VaultDepositWitness, + ) { + let destination_address_internal = + match T::AddressConverter::decode_and_validate_address_for_asset( + destination_address.clone(), + output_asset, + ) { + Ok(address) => address, + Err(err) => { + log::warn!("Failed to process vault swap due to invalid destination address. Tx hash: {tx_id:?}. Error: {err:?}"); + return; + }, + }; + + let broker = broker_fee.account.clone(); + + let broker_fees = Self::assemble_broker_fees(broker_fee, affiliate_fees); + + let (action, source_address) = if let Some(deposit_metadata) = deposit_metadata { + ( + ChannelAction::CcmTransfer { + destination_asset: output_asset, + destination_address: destination_address_internal, + broker_fees, + refund_params: Some(refund_params), + dca_params, + channel_metadata: deposit_metadata.channel_metadata, + }, + deposit_metadata.source_address, + ) + } else { + ( + ChannelAction::Swap { + destination_asset: output_asset, + destination_address: destination_address_internal, + broker_fees, + refund_params: Some(refund_params), + dca_params, + }, + None, + ) + }; + + let boost_status = BoostedVaultTxs::::get(&tx_id).unwrap_or(BoostStatus::NotBoosted); + + let origin = DepositOrigin::vault::(tx_id.clone()); + + if let Some(new_boost_status) = Self::process_prewitness_deposit_inner( + amount, + asset, + deposit_details, + deposit_address, + source_address, + action, + &broker, + boost_fee, + boost_status, + channel_id, + block_height, + origin, + ) { + BoostedVaultTxs::::insert(&tx_id, new_boost_status); + } + } + + fn process_full_witness_deposit_inner( + deposit_address: TargetChainAccount, + asset: TargetChainAsset, + deposit_amount: TargetChainAmount, + deposit_details: ::DepositDetails, + source_address: Option, + broker: &T::AccountId, + boost_status: BoostStatus>, + channel_id: u64, + action: ChannelAction, + block_height: TargetChainBlockNumber, + origin: DepositOrigin, + ) -> Result { // TODO: only apply this check if the deposit hasn't been boosted // already (in case MinimumDeposit increases after some small deposit // is boosted)? @@ -1979,16 +2261,14 @@ impl, I: 'static> Pallet { deposit_details, reason: DepositIgnoredReason::BelowMinimumDeposit, }); - return Ok(()) + return Err(()); } - let channel_owner = deposit_channel_details.owner.clone(); - if let Some(tx_id) = deposit_details.deposit_id() { - if TaintedTransactions::::take(&channel_owner, &tx_id).is_some() && - !matches!(deposit_channel_details.boost_status, BoostStatus::Boosted { .. }) + if TaintedTransactions::::take(broker, &tx_id).is_some() && + !matches!(boost_status, BoostStatus::Boosted { .. }) { - let refund_address = match deposit_channel_details.action.clone() { + let refund_address = match action.clone() { ChannelAction::Swap { refund_params, .. } => refund_params .as_ref() .map(|refund_params| refund_params.refund_address.clone()), @@ -2013,17 +2293,22 @@ impl, I: 'static> Pallet { deposit_details, reason: DepositIgnoredReason::TransactionTainted, }); - return Ok(()) + return Err(()); } } - ScheduledEgressFetchOrTransfer::::append(FetchOrTransfer::::Fetch { - asset, - deposit_address: deposit_address.clone(), - deposit_fetch_id: None, - amount: deposit_amount, - }); - Self::deposit_event(Event::::DepositFetchesScheduled { channel_id, asset }); + // Vault deposits don't need to be fetched: + if !matches!(origin, DepositOrigin::Vault { .. }) { + ScheduledEgressFetchOrTransfer::::append( + FetchOrTransfer::::Fetch { + asset, + deposit_address: deposit_address.clone(), + deposit_fetch_id: None, + amount: deposit_amount, + }, + ); + Self::deposit_event(Event::::DepositFetchesScheduled { channel_id, asset }); + } // Add the deposit to the balance. T::DepositHandler::on_deposit_made(deposit_details.clone()); @@ -2032,7 +2317,7 @@ impl, I: 'static> Pallet { // (i.e. awaiting finalisation), *and* the boosted amount matches the amount // in this deposit, finalise the boost by crediting boost pools with the deposit. // Process as non-boosted deposit otherwise: - let maybe_boost_to_process = match deposit_channel_details.boost_status { + let maybe_boost_to_process = match boost_status { BoostStatus::Boosted { prewitnessed_deposit_id, pools, amount } if amount == deposit_amount => Some((prewitnessed_deposit_id, pools)), @@ -2063,30 +2348,23 @@ impl, I: 'static> Pallet { }); } - // This allows the channel to be boosted again: - DepositChannelLookup::::mutate(&deposit_address, |details| { - if let Some(details) = details { - details.boost_status = BoostStatus::NotBoosted; - } - }); - Self::deposit_event(Event::DepositFinalised { deposit_address, asset, amount: deposit_amount, block_height, deposit_details, + // no ingrees fee as it was already charged at the time of boosting ingress_fee: 0u32.into(), action: DepositAction::BoostersCredited { prewitnessed_deposit_id }, channel_id, + origin_type: origin.into(), }); + + Ok(FullWitnessDepositOutcome::BoostFinalised) } else { let AmountAndFeesWithheld { amount_after_fees, fees_withheld } = - Self::withhold_ingress_or_egress_fee( - IngressOrEgress::Ingress, - deposit_channel_details.deposit_channel.asset, - deposit_amount, - ); + Self::conditionally_withhold_ingress_fee(asset, deposit_amount, &origin); if amount_after_fees.is_zero() { Self::deposit_event(Event::::DepositIgnored { @@ -2096,59 +2374,58 @@ impl, I: 'static> Pallet { deposit_details, reason: DepositIgnoredReason::NotEnoughToPayFees, }); + Err(()) } else { - let deposit_action = Self::perform_channel_action( - deposit_channel_details.action, - deposit_channel_details.deposit_channel, - amount_after_fees, - block_height, - )?; + // Processing as a non-boosted deposit: - Self::deposit_event(Event::DepositFinalised { - deposit_address, + if let Ok(action) = Self::perform_channel_action( + action, asset, - amount: deposit_amount, - block_height, - deposit_details, - ingress_fee: fees_withheld, - action: deposit_action, - channel_id, - }); + source_address, + amount_after_fees, + origin.clone(), + ) { + // TODO: this needs to include deposit type (vault/channel) + Self::deposit_event(Event::DepositFinalised { + deposit_address, + asset, + amount: deposit_amount, + block_height, + deposit_details, + ingress_fee: fees_withheld, + action, + channel_id, + origin_type: origin.into(), + }); + Ok(FullWitnessDepositOutcome::DepositActionPerformed) + } else { + Err(()) + } } } - - Ok(()) } - pub fn process_vault_swap_request( - source_asset: TargetChainAsset, - deposit_amount: ::ChainAmount, - destination_asset: Asset, - destination_address: EncodedAddress, - deposit_metadata: Option, - tx_id: TransactionInIdFor, - deposit_details: ::DepositDetails, - broker_fee: Beneficiary, - affiliate_fees: Affiliates, - refund_params: ChannelRefundParameters, - dca_params: Option, - // This is only to be checked in the pre-witnessed version (not implemented yet) - _boost_fee: BasisPoints, + pub fn process_vault_swap_request_full_witness( + block_height: TargetChainBlockNumber, + VaultDepositWitness { + input_asset: source_asset, + deposit_address, + channel_id, + deposit_amount, + deposit_details, + output_asset: destination_asset, + destination_address, + deposit_metadata, + tx_id, + broker_fee, + affiliate_fees, + refund_params, + dca_params, + // Boost fee is only relevant for prewitnessing + boost_fee: _, + }: VaultDepositWitness, ) { - if deposit_amount < MinimumDeposit::::get(source_asset) { - // If the deposit amount is below the minimum allowed, the deposit is ignored. - // TODO: track these funds somewhere, for example add them to the withheld fees. - Self::deposit_event(Event::::DepositIgnored { - deposit_address: None, - asset: source_asset, - amount: deposit_amount, - deposit_details, - reason: DepositIgnoredReason::BelowMinimumDeposit, - }); - return; - } - - T::DepositHandler::on_deposit_made(deposit_details); + let boost_status = BoostedVaultTxs::::get(&tx_id).unwrap_or(BoostStatus::NotBoosted); let destination_address_internal = match T::AddressConverter::decode_and_validate_address_for_asset( @@ -2162,116 +2439,96 @@ impl, I: 'static> Pallet { }, }; - let swap_origin = - SwapOrigin::Vault { tx_id: tx_id.clone().into_transaction_in_id_for_any_chain() }; - - let request_type = if let Some(deposit_metadata) = deposit_metadata { - let ccm_failed = |reason| { - log::warn!("Failed to process CCM. Tx id: {:?}, Reason: {:?}", tx_id, reason); - - Self::deposit_event(Event::::CcmFailed { - reason, - destination_address: destination_address.clone(), - deposit_metadata: deposit_metadata.clone().to_encoded::(), - origin: swap_origin.clone(), - }); - }; + let deposit_origin = DepositOrigin::vault::(tx_id.clone()); + if let Some(deposit_metadata) = &deposit_metadata { if T::CcmValidityChecker::check_and_decode( &deposit_metadata.channel_metadata, destination_asset, ) .is_err() { - ccm_failed(CcmFailReason::InvalidMetadata); + log::warn!( + "Failed to process CCM. Tx id: {:?}, Reason: {:?}", + tx_id, + CcmFailReason::InvalidMetadata + ); + + Self::deposit_event(Event::::CcmFailed { + reason: CcmFailReason::InvalidMetadata, + destination_address: destination_address.clone(), + deposit_metadata: deposit_metadata.clone().to_encoded::(), + origin: deposit_origin.clone().into(), + }); + return; }; - - match deposit_metadata.clone().into_swap_metadata( - deposit_amount.into(), - source_asset.into(), - destination_asset, - ) { - Ok(ccm_swap_metadata) => SwapRequestType::Ccm { - ccm_swap_metadata, - output_address: destination_address_internal.clone(), - }, - Err(reason) => { - ccm_failed(reason); - return; - }, - } - } else { - SwapRequestType::Regular { output_address: destination_address_internal.clone() } - }; + } if let Err(err) = T::SwapLimitsProvider::validate_refund_params(refund_params.retry_duration) { - log::warn!( - "Failed to process vault swap due to invalid refund params. Tx id: {tx_id:?}. Error: {err:?}", - ); + log::warn!("Failed to process vault swap due to invalid refund params. Tx id: {tx_id:?}. Error: {err:?}"); return; } if let Some(params) = &dca_params { if let Err(err) = T::SwapLimitsProvider::validate_dca_params(params) { - log::warn!( - "Failed to process vault swap due to invalid dca params. Tx id: {tx_id:?}. Error: {err:?}", - ); + log::warn!("Failed to process vault swap due to invalid dca params. Tx id: {tx_id:?}. Error: {err:?}"); return; } } - let primary_broker = broker_fee.account.clone(); - - let broker_fees = - if T::AccountRoleRegistry::has_account_role(&primary_broker, AccountRole::Broker) { - [broker_fee] - .into_iter() - .chain(affiliate_fees.into_iter().filter_map( - |Beneficiary { account: short_affiliate_id, bps }| { - if let Some(affiliate_id) = - T::AffiliateRegistry::get_account_id(&primary_broker, short_affiliate_id) - { - Some(Beneficiary { account: affiliate_id, bps }) - } else { - // In case the entry not found, we ignore the entry, but process the - // swap (to avoid having to refund it). - Self::deposit_event(Event::::UnknownAffiliate { - broker_id: primary_broker.clone(), - short_affiliate_id, - }); - - None - } - }, - )) - .collect::>() - .try_into() - .expect("must fit since affiliates are limited to 1 fewer element than beneficiaries") - } else { - Self::deposit_event(Event::::UnknownBroker { broker_id: primary_broker }); - Default::default() - }; + let broker = broker_fee.account.clone(); + let broker_fees = Self::assemble_broker_fees(broker_fee, affiliate_fees); if let Err(err) = T::SwapLimitsProvider::validate_broker_fees(&broker_fees) { - log::warn!( - "Failed to process vault swap due to invalid broker fees. Tx id: {tx_id:?}. Error: {err:?}", - ); + log::warn!("Failed to process vault swap due to invalid broker fees. Tx id: {tx_id:?}. Error: {err:?}"); return; } - T::SwapRequestHandler::init_swap_request( - source_asset.into(), - deposit_amount.into(), - destination_asset, - request_type, - broker_fees, - Some(refund_params), - dca_params, - swap_origin, - ); + let (action, source_address) = if let Some(deposit_metadata) = deposit_metadata { + ( + ChannelAction::CcmTransfer { + destination_asset, + destination_address: destination_address_internal, + broker_fees, + refund_params: Some(refund_params), + dca_params, + channel_metadata: deposit_metadata.channel_metadata, + }, + deposit_metadata.source_address, + ) + } else { + ( + ChannelAction::Swap { + destination_asset, + destination_address: destination_address_internal, + broker_fees, + refund_params: Some(refund_params), + dca_params, + }, + None, + ) + }; + + if let Ok(FullWitnessDepositOutcome::BoostFinalised) = + Self::process_full_witness_deposit_inner( + deposit_address.clone(), + source_asset, + deposit_amount, + deposit_details, + source_address, + &broker, + boost_status, + channel_id, + action, + block_height, + deposit_origin, + ) { + // This allows the channel to be boosted again: + BoostedVaultTxs::::remove(&tx_id); + } } fn expiry_and_recycle_block_height( @@ -2396,6 +2653,23 @@ impl, I: 'static> Pallet { .cloned() } + // Withholds ingress fee, but only after checking the origin + fn conditionally_withhold_ingress_fee( + asset: TargetChainAsset, + available_amount: TargetChainAmount, + origin: &DepositOrigin, + ) -> AmountAndFeesWithheld { + if matches!(origin, &DepositOrigin::DepositChannel { .. }) { + Self::withhold_ingress_or_egress_fee(IngressOrEgress::Ingress, asset, available_amount) + } else { + // No ingress fee for vault swaps. + AmountAndFeesWithheld { + amount_after_fees: available_amount, + fees_withheld: 0u32.into(), + } + } + } + /// Withholds the fee for a given amount. /// /// Returns the remaining amount after the fee has been withheld, and the fee itself, both diff --git a/state-chain/pallets/cf-ingress-egress/src/tests.rs b/state-chain/pallets/cf-ingress-egress/src/tests.rs index 132461e325..e25c2e82c9 100644 --- a/state-chain/pallets/cf-ingress-egress/src/tests.rs +++ b/state-chain/pallets/cf-ingress-egress/src/tests.rs @@ -8,7 +8,7 @@ use crate::{ DisabledEgressAssets, EgressDustLimit, Event as PalletEvent, FailedForeignChainCall, FailedForeignChainCalls, FetchOrTransfer, MinimumDeposit, Pallet, PalletConfigUpdate, PalletSafeMode, PrewitnessedDepositIdCounter, ScheduledEgressCcm, - ScheduledEgressFetchOrTransfer, + ScheduledEgressFetchOrTransfer, VaultDepositWitness, }; use cf_chains::{ address::{AddressConverter, EncodedAddress}, @@ -16,11 +16,12 @@ use cf_chains::{ btc::{BitcoinNetwork, ScriptPubkey}, evm::{DepositDetails, EvmFetchId, H256}, mocks::MockEthereum, - CcmChannelMetadata, CcmFailReason, ChannelRefundParameters, DepositChannel, + CcmChannelMetadata, CcmFailReason, ChannelRefundParameters, DepositChannel, DepositOriginType, ExecutexSwapAndCall, SwapOrigin, TransactionInIdForAnyChain, TransferAssetParams, }; use cf_primitives::{ - AffiliateShortId, AssetAmount, BasisPoints, Beneficiary, ChannelId, ForeignChain, + AffiliateShortId, Affiliates, AssetAmount, BasisPoints, Beneficiary, ChannelId, DcaParameters, + ForeignChain, }; use cf_test_utilities::{assert_events_eq, assert_has_event, assert_has_matching_event}; use cf_traits::{ @@ -47,7 +48,7 @@ use frame_support::{ weights::Weight, }; use sp_core::{bounded_vec, H160}; -use sp_runtime::DispatchError; +use sp_runtime::{DispatchError, DispatchResult}; const ALICE_ETH_ADDRESS: EthereumAddress = H160([100u8; 20]); const BOB_ETH_ADDRESS: EthereumAddress = H160([101u8; 20]); @@ -855,6 +856,7 @@ fn deposits_below_minimum_are_rejected() { ingress_fee: 0, action: DepositAction::LiquidityProvision { lp_account: LP_ACCOUNT }, channel_id, + origin_type: DepositOriginType::DepositChannel, }, )); }); @@ -1790,6 +1792,43 @@ fn do_not_process_more_ccm_swaps_than_allowed_by_limit() { }); } +fn submit_vault_swap_request( + input_asset: Asset, + output_asset: Asset, + deposit_amount: AssetAmount, + deposit_address: H160, + destination_address: EncodedAddress, + deposit_metadata: Option, + tx_id: H256, + deposit_details: DepositDetails, + broker_fee: Beneficiary, + affiliate_fees: Affiliates, + refund_params: ChannelRefundParameters, + dca_params: Option, + boost_fee: BasisPoints, +) -> DispatchResult { + IngressEgress::vault_swap_request( + RuntimeOrigin::root(), + 0, + vec![VaultDepositWitness { + input_asset: input_asset.try_into().unwrap(), + deposit_address, + channel_id: 0, + deposit_amount, + deposit_details, + output_asset, + destination_address, + deposit_metadata, + tx_id, + broker_fee, + affiliate_fees, + refund_params, + dca_params, + boost_fee, + }], + ) +} + #[test] fn can_request_swap_via_extrinsic() { const INPUT_ASSET: Asset = Asset::Eth; @@ -1799,20 +1838,20 @@ fn can_request_swap_via_extrinsic() { let output_address = ForeignChainAddress::Eth([1; 20].into()); new_test_ext().execute_with(|| { - assert_ok!(IngressEgress::vault_swap_request( - RuntimeOrigin::root(), - INPUT_ASSET.try_into().unwrap(), + assert_ok!(submit_vault_swap_request( + INPUT_ASSET, OUTPUT_ASSET, INPUT_AMOUNT, + Default::default(), MockAddressConverter::to_encoded_address(output_address.clone()), None, Default::default(), - Box::new(DepositDetails { tx_hashes: None }), + DepositDetails { tx_hashes: None }, Beneficiary { account: BROKER, bps: 0 }, Default::default(), - Box::new(ETH_REFUND_PARAMS), + ETH_REFUND_PARAMS, None, - 0, + 0 )); assert_eq!( @@ -1855,21 +1894,21 @@ fn vault_swaps_support_affiliate_fees() { // have no effect on the test: MockAffiliateRegistry::register_affiliate(BROKER + 1, AFFILIATE_2, AFFILIATE_SHORT_1); - assert_ok!(IngressEgress::vault_swap_request( - RuntimeOrigin::root(), - INPUT_ASSET.try_into().unwrap(), + assert_ok!(submit_vault_swap_request( + INPUT_ASSET, OUTPUT_ASSET, INPUT_AMOUNT, + Default::default(), MockAddressConverter::to_encoded_address(output_address.clone()), None, Default::default(), - Box::new(DepositDetails { tx_hashes: None }), + DepositDetails { tx_hashes: None }, Beneficiary { account: BROKER, bps: BROKER_FEE }, bounded_vec![ Beneficiary { account: AFFILIATE_SHORT_1, bps: AFFILIATE_FEE }, Beneficiary { account: AFFILIATE_SHORT_2, bps: AFFILIATE_FEE } ], - Box::new(ETH_REFUND_PARAMS), + ETH_REFUND_PARAMS, None, 0 )); @@ -1913,18 +1952,18 @@ fn charge_no_broker_fees_on_unknown_primary_broker() { let output_address = ForeignChainAddress::Eth([1; 20].into()); - assert_ok!(IngressEgress::vault_swap_request( - RuntimeOrigin::root(), - INPUT_ASSET.try_into().unwrap(), + assert_ok!(submit_vault_swap_request( + INPUT_ASSET, OUTPUT_ASSET, INPUT_AMOUNT, + Default::default(), MockAddressConverter::to_encoded_address(output_address.clone()), None, Default::default(), - Box::new(DepositDetails { tx_hashes: None }), + DepositDetails { tx_hashes: None }, Beneficiary { account: NOT_A_BROKER, bps: BROKER_FEE }, Default::default(), - Box::new(ETH_REFUND_PARAMS), + ETH_REFUND_PARAMS, None, 0 )); @@ -1959,7 +1998,7 @@ fn can_request_ccm_swap_via_extrinsic() { let ccm_deposit_metadata = CcmDepositMetadata { source_chain: ForeignChain::Ethereum, - source_address: Some(ForeignChainAddress::Eth([0xcf; 20].into())), + source_address: None, channel_metadata: CcmChannelMetadata { message: vec![0x01].try_into().unwrap(), gas_budget: 1_000, @@ -1970,18 +2009,18 @@ fn can_request_ccm_swap_via_extrinsic() { let output_address = ForeignChainAddress::Eth([1; 20].into()); new_test_ext().execute_with(|| { - assert_ok!(IngressEgress::vault_swap_request( - RuntimeOrigin::root(), - INPUT_ASSET.try_into().unwrap(), + assert_ok!(submit_vault_swap_request( + INPUT_ASSET, OUTPUT_ASSET, 10_000, + Default::default(), MockAddressConverter::to_encoded_address(output_address.clone()), Some(ccm_deposit_metadata.clone()), Default::default(), - Box::new(DepositDetails { tx_hashes: None }), + DepositDetails { tx_hashes: None }, Beneficiary { account: BROKER, bps: 0 }, Default::default(), - Box::new(ETH_REFUND_PARAMS), + ETH_REFUND_PARAMS, None, 0 )); @@ -2020,41 +2059,41 @@ fn rejects_invalid_swap_by_witnesser() { MockAddressConverter::to_encoded_address(ForeignChainAddress::Btc(script_pubkey)); // Is valid Bitcoin address, but asset is Dot, so not compatible - assert_ok!(IngressEgress::vault_swap_request( - RuntimeOrigin::root(), - EthAsset::Eth, + assert_ok!(submit_vault_swap_request( + Asset::Eth, Asset::Dot, 10000, + Default::default(), btc_encoded_address, None, Default::default(), - Box::new(DepositDetails { tx_hashes: None }), + DepositDetails { tx_hashes: None }, Beneficiary { account: 0, bps: 0 }, Default::default(), - Box::new(ETH_REFUND_PARAMS), + ETH_REFUND_PARAMS, None, 0 - ),); + )); // No swap request created -> the call was ignored assert!(MockSwapRequestHandler::::get_swap_requests().is_empty()); // Invalid BTC address: - assert_ok!(IngressEgress::vault_swap_request( - RuntimeOrigin::root(), - EthAsset::Eth, + assert_ok!(submit_vault_swap_request( + Asset::Eth, Asset::Btc, 10000, + Default::default(), EncodedAddress::Btc(vec![0x41, 0x80, 0x41]), None, Default::default(), - Box::new(DepositDetails { tx_hashes: None }), + DepositDetails { tx_hashes: None }, Beneficiary { account: 0, bps: 0 }, Default::default(), - Box::new(ETH_REFUND_PARAMS), + ETH_REFUND_PARAMS, None, 0 - ),); + )); assert!(MockSwapRequestHandler::::get_swap_requests().is_empty()); }); @@ -2076,18 +2115,18 @@ fn failed_ccm_deposit_can_deposit_event() { new_test_ext().execute_with(|| { // CCM is not supported for Dot: - assert_ok!(IngressEgress::vault_swap_request( - RuntimeOrigin::root(), - EthAsset::Flip, + assert_ok!(submit_vault_swap_request( + Asset::Flip, Asset::Dot, 10_000, + Default::default(), EncodedAddress::Dot(Default::default()), Some(ccm_deposit_metadata.clone()), Default::default(), - Box::new(DepositDetails { tx_hashes: None }), + DepositDetails { tx_hashes: None }, Beneficiary { account: 0, bps: 0 }, Default::default(), - Box::new(ETH_REFUND_PARAMS), + ETH_REFUND_PARAMS, None, 0 )); @@ -2103,18 +2142,18 @@ fn failed_ccm_deposit_can_deposit_event() { System::reset_events(); // Insufficient deposit amount: - assert_ok!(IngressEgress::vault_swap_request( - RuntimeOrigin::root(), - EthAsset::Flip, + assert_ok!(submit_vault_swap_request( + Asset::Flip, Asset::Eth, GAS_BUDGET - 1, + Default::default(), EncodedAddress::Eth(Default::default()), Some(ccm_deposit_metadata), Default::default(), - Box::new(DepositDetails { tx_hashes: None }), + DepositDetails { tx_hashes: None }, Beneficiary { account: 0, bps: 0 }, Default::default(), - Box::new(ETH_REFUND_PARAMS), + ETH_REFUND_PARAMS, None, 0 )); 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 46b32a561f..5da283efe5 100644 --- a/state-chain/pallets/cf-ingress-egress/src/tests/boost.rs +++ b/state-chain/pallets/cf-ingress-egress/src/tests/boost.rs @@ -1,6 +1,6 @@ use super::*; -use cf_chains::FeeEstimationApi; +use cf_chains::{DepositOriginType, FeeEstimationApi}; use cf_primitives::{AssetAmount, BasisPoints, PrewitnessedDepositId}; use cf_test_utilities::assert_event_sequence; use cf_traits::{ @@ -227,6 +227,7 @@ fn basic_passive_boosting() { ingress_fee: INGRESS_FEE, boost_fee: POOL_1_FEE + POOL_2_FEE, action: DepositAction::LiquidityProvision { lp_account: LP_ACCOUNT }, + origin_type: DepositOriginType::DepositChannel, })); assert_boosted(deposit_address, prewitnessed_deposit_id, [TIER_5_BPS, TIER_10_BPS]); @@ -254,6 +255,7 @@ fn basic_passive_boosting() { ingress_fee: 0, action: DepositAction::BoostersCredited { prewitnessed_deposit_id }, channel_id, + origin_type: DepositOriginType::DepositChannel, })); assert_eq!(get_available_amount(ASSET, TIER_5_BPS), BOOSTER_AMOUNT_1 + POOL_1_FEE); @@ -989,3 +991,172 @@ fn test_create_boost_pools() { ); }); } + +mod vault_swaps { + + use crate::BoostedVaultTxs; + + use super::*; + + #[test] + fn vault_swap_boosting() { + new_test_ext().execute_with(|| { + let output_address = ForeignChainAddress::Eth([1; 20].into()); + + let block_height = 10; + let deposit_address = [1; 20].into(); + + const BOOSTER_AMOUNT: AssetAmount = 500_000_000; + const DEPOSIT_AMOUNT: AssetAmount = 100_000_000; + const INPUT_ASSET: Asset = Asset::Eth; + const OUTPUT_ASSET: Asset = Asset::Flip; + + const BOOST_FEE: AssetAmount = DEPOSIT_AMOUNT * TIER_5_BPS as u128 / 10_000; + const PREWITNESS_DEPOSIT_ID: PrewitnessedDepositId = 1; + const CHANNEL_ID: ChannelId = 1; + + setup(); + + assert_ok!(IngressEgress::add_boost_funds( + RuntimeOrigin::signed(BOOSTER_1), + EthAsset::Eth, + BOOSTER_AMOUNT, + TIER_5_BPS + )); + + let tx_id = [9u8; 32].into(); + + // Initially tx is not recorded as boosted + assert!(!BoostedVaultTxs::::contains_key(tx_id)); + + let deposit = VaultDepositWitness { + input_asset: INPUT_ASSET.try_into().unwrap(), + deposit_address, + channel_id: CHANNEL_ID, + deposit_amount: DEPOSIT_AMOUNT, + deposit_details: Default::default(), + output_asset: OUTPUT_ASSET, + destination_address: MockAddressConverter::to_encoded_address( + output_address.clone(), + ), + deposit_metadata: None, + tx_id, + broker_fee: Beneficiary { account: BROKER, bps: 5 }, + affiliate_fees: Default::default(), + refund_params: ChannelRefundParameters { + retry_duration: 2, + refund_address: ForeignChainAddress::Eth([2; 20].into()), + min_price: Default::default(), + }, + dca_params: None, + boost_fee: 5, + }; + + // Prewitnessing a deposit for the first time should result in a boost: + { + IngressEgress::process_vault_swap_request_prewitness(block_height, deposit.clone()); + assert_eq!(PrewitnessedDepositIdCounter::::get(), PREWITNESS_DEPOSIT_ID); + + assert_eq!( + BoostPools::::get(EthAsset::Eth, TIER_5_BPS) + .unwrap() + .get_pending_boost_ids() + .len(), + 1 + ); + + assert_eq!( + MockSwapRequestHandler::::get_swap_requests(), + vec![MockSwapRequest { + input_asset: INPUT_ASSET, + output_asset: OUTPUT_ASSET, + // Note that ingress fee is not charged: + input_amount: DEPOSIT_AMOUNT - BOOST_FEE, + swap_type: SwapRequestType::Regular { output_address }, + broker_fees: bounded_vec![Beneficiary { account: BROKER, bps: 5 }], + origin: SwapOrigin::Vault { tx_id: TransactionInIdForAnyChain::Evm(tx_id) }, + },] + ); + + assert_has_matching_event!( + Test, + RuntimeEvent::IngressEgress(Event::DepositBoosted { + prewitnessed_deposit_id: PREWITNESS_DEPOSIT_ID, + channel_id: CHANNEL_ID, + action: DepositAction::Swap { .. }, + .. + }) + ); + + // Now the tx is recorded as boosted + assert!(BoostedVaultTxs::::contains_key(tx_id)); + } + + // Prewitnessing the same deposit (e.g. due to a reorg) should not result in a second + // boost: + { + IngressEgress::process_vault_swap_request_prewitness(block_height, deposit.clone()); + + assert_eq!( + BoostPools::::get(EthAsset::Eth, TIER_5_BPS) + .unwrap() + .get_pending_boost_ids() + .len(), + 1 + ); + + assert_eq!(MockSwapRequestHandler::::get_swap_requests().len(), 1); + } + + // Prewitnessing a different deposit *should* result in a second boost: + { + let other_deposit = + VaultDepositWitness { tx_id: [10u8; 32].into(), ..deposit.clone() }; + IngressEgress::process_vault_swap_request_prewitness(block_height, other_deposit); + + assert_eq!( + BoostPools::::get(EthAsset::Eth, TIER_5_BPS) + .unwrap() + .get_pending_boost_ids() + .len(), + 2 + ); + + assert_eq!(MockSwapRequestHandler::::get_swap_requests().len(), 2); + } + + // Fully witnessing a boosted deposit should finalise boost: + { + IngressEgress::process_vault_swap_request_full_witness( + block_height, + deposit.clone(), + ); + + // No new swap is initiated: + assert_eq!(MockSwapRequestHandler::::get_swap_requests().len(), 2); + + assert_eq!( + BoostPools::::get(EthAsset::Eth, TIER_5_BPS) + .unwrap() + .get_pending_boost_ids() + .len(), + 1 + ); + + assert_has_matching_event!( + Test, + RuntimeEvent::IngressEgress(Event::DepositFinalised { + channel_id: CHANNEL_ID, + action: DepositAction::BoostersCredited { + prewitnessed_deposit_id: PREWITNESS_DEPOSIT_ID + }, + .. + }) + ); + + // Boost record for tx is removed: + assert!(!BoostedVaultTxs::::contains_key(tx_id)); + } + }); + } +} diff --git a/state-chain/runtime/src/chainflip/solana_elections.rs b/state-chain/runtime/src/chainflip/solana_elections.rs index 2db0e38335..2c40c969d9 100644 --- a/state-chain/runtime/src/chainflip/solana_elections.rs +++ b/state-chain/runtime/src/chainflip/solana_elections.rs @@ -37,6 +37,7 @@ use pallet_cf_elections::{ }, CorruptStorageError, ElectionIdentifier, InitialState, InitialStateOf, RunnerStorageAccess, }; +use pallet_cf_ingress_egress::VaultDepositWitness; use scale_info::TypeInfo; use serde::{Deserialize, Serialize}; use sp_runtime::{DispatchResult, FixedPointNumber, FixedU128}; @@ -559,19 +560,25 @@ impl > for SolanaVaultSwapsHandler { fn initiate_vault_swap(swap_details: SolanaVaultSwapDetails) { - SolanaIngressEgress::process_vault_swap_request( - swap_details.from, - swap_details.deposit_amount, - swap_details.to, - swap_details.destination_address, - swap_details.deposit_metadata, - (swap_details.swap_account, swap_details.creation_slot), - (), - swap_details.broker_fee, - swap_details.affiliate_fees, - swap_details.refund_params, - swap_details.dca_params, - swap_details.boost_fee.into(), + let block_height = swap_details.creation_slot; + SolanaIngressEgress::process_vault_swap_request_full_witness( + block_height, + VaultDepositWitness { + input_asset: swap_details.from, + deposit_address: todo!(), + channel_id: todo!(), + deposit_amount: swap_details.deposit_amount, + deposit_details: (), + output_asset: swap_details.to, + destination_address: swap_details.destination_address, + deposit_metadata: swap_details.deposit_metadata, + tx_id: (swap_details.swap_account, swap_details.creation_slot), + broker_fee: swap_details.broker_fee, + affiliate_fees: swap_details.affiliate_fees, + dca_params: swap_details.dca_params, + refund_params: swap_details.refund_params, + boost_fee: swap_details.boost_fee.into(), + }, ); } diff --git a/state-chain/runtime/src/lib.rs b/state-chain/runtime/src/lib.rs index 49da0c11b4..5595749a82 100644 --- a/state-chain/runtime/src/lib.rs +++ b/state-chain/runtime/src/lib.rs @@ -1924,39 +1924,15 @@ impl_runtime_apis! { let current_block_events = System::read_events_no_consensus(); for event in current_block_events { + #[allow(clippy::collapsible_match)] match *event { frame_system::EventRecord:: { event: RuntimeEvent::Witnesser(pallet_cf_witnesser::Event::Prewitnessed { call }), ..} => { match call { - RuntimeCall::EthereumIngressEgress(pallet_cf_ingress_egress::Call::vault_swap_request { - input_asset: swap_from, output_asset: swap_to, deposit_amount, .. - }) if from == swap_from.into() && to == swap_to => { - all_prewitnessed_swaps.push(deposit_amount); - } - RuntimeCall::ArbitrumIngressEgress(pallet_cf_ingress_egress::Call::vault_swap_request { - input_asset: swap_from, output_asset: swap_to, deposit_amount, .. - }) if from == swap_from.into() && to == swap_to => { - all_prewitnessed_swaps.push(deposit_amount); - } - RuntimeCall::EthereumIngressEgress(pallet_cf_ingress_egress::Call::process_deposits::<_, EthereumInstance> { - deposit_witnesses, .. - }) => { - all_prewitnessed_swaps.extend(filter_deposit_swaps::(from, to, deposit_witnesses)); - }, - RuntimeCall::ArbitrumIngressEgress(pallet_cf_ingress_egress::Call::process_deposits::<_, ArbitrumInstance> { - deposit_witnesses, .. - }) => { - all_prewitnessed_swaps.extend(filter_deposit_swaps::(from, to, deposit_witnesses)); - }, RuntimeCall::BitcoinIngressEgress(pallet_cf_ingress_egress::Call::process_deposits { deposit_witnesses, .. }) => { all_prewitnessed_swaps.extend(filter_deposit_swaps::(from, to, deposit_witnesses)); }, - RuntimeCall::PolkadotIngressEgress(pallet_cf_ingress_egress::Call::process_deposits { - deposit_witnesses, .. - }) => { - all_prewitnessed_swaps.extend(filter_deposit_swaps::(from, to, deposit_witnesses)); - }, _ => { // ignore, we only care about calls that trigger swaps. }, From acaebcfd79300596fe32416db4b5aca040f01065 Mon Sep 17 00:00:00 2001 From: Maxim Shishmarev Date: Tue, 3 Dec 2024 13:00:53 +1100 Subject: [PATCH 02/13] feat: make deposit_address and channel_id optional in events --- engine/src/witness/arb.rs | 8 +- engine/src/witness/btc/vault_swaps.rs | 8 +- engine/src/witness/eth.rs | 10 +- engine/src/witness/evm/vault.rs | 18 +-- .../cf-integration-tests/src/solana.rs | 4 +- .../cf-integration-tests/src/swapping.rs | 4 +- .../pallets/cf-ingress-egress/src/lib.rs | 139 +++++++++--------- .../pallets/cf-ingress-egress/src/tests.rs | 10 +- .../cf-ingress-egress/src/tests/boost.rs | 18 +-- .../runtime/src/chainflip/solana_elections.rs | 4 +- 10 files changed, 102 insertions(+), 121 deletions(-) diff --git a/engine/src/witness/arb.rs b/engine/src/witness/arb.rs index d0a53fb12f..eb6ddaf4f1 100644 --- a/engine/src/witness/arb.rs +++ b/engine/src/witness/arb.rs @@ -8,7 +8,7 @@ use cf_chains::{ Arbitrum, CcmDepositMetadata, }; use cf_primitives::{ - chains::assets::arb::Asset as ArbAsset, Asset, AssetAmount, Beneficiary, ChannelId, EpochIndex, + chains::assets::arb::Asset as ArbAsset, Asset, AssetAmount, Beneficiary, EpochIndex, }; use cf_utilities::task_scope::Scope; use futures_core::Future; @@ -188,8 +188,6 @@ impl super::evm::vault::IngressCallBuilder for ArbCallBuilder { fn vault_swap_request( block_height: u64, source_asset: Asset, - deposit_address: cf_chains::eth::Address, - channel_id: ChannelId, deposit_amount: AssetAmount, destination_asset: Asset, destination_address: EncodedAddress, @@ -219,8 +217,8 @@ impl super::evm::vault::IngressCallBuilder for ArbCallBuilder { boost_fee: vault_swap_parameters.boost_fee.into(), dca_params: vault_swap_parameters.dca_params, refund_params: vault_swap_parameters.refund_params, - channel_id, - deposit_address, + channel_id: None, + deposit_address: None, }], }, ) diff --git a/engine/src/witness/btc/vault_swaps.rs b/engine/src/witness/btc/vault_swaps.rs index d51a2f94a7..b0c920ffe7 100644 --- a/engine/src/witness/btc/vault_swaps.rs +++ b/engine/src/witness/btc/vault_swaps.rs @@ -170,8 +170,8 @@ pub fn try_extract_vault_swap_witness( }), // This is only to be checked in the pre-witnessed version boost_fee: data.parameters.boost_fee.into(), - channel_id, - deposit_address: vault_address.script_pubkey(), + channel_id: Some(channel_id), + deposit_address: Some(vault_address.script_pubkey()), }) } @@ -328,8 +328,8 @@ mod tests { chunk_interval: MOCK_SWAP_PARAMS.parameters.chunk_interval.into(), }), boost_fee: MOCK_SWAP_PARAMS.parameters.boost_fee.into(), - deposit_address: vault_deposit_address.script_pubkey(), - channel_id: CHANNEL_ID, + deposit_address: Some(vault_deposit_address.script_pubkey()), + channel_id: Some(CHANNEL_ID), }) ); } diff --git a/engine/src/witness/eth.rs b/engine/src/witness/eth.rs index 0ee789688a..be068cb74a 100644 --- a/engine/src/witness/eth.rs +++ b/engine/src/witness/eth.rs @@ -8,9 +8,7 @@ use cf_chains::{ evm::{DepositDetails, H256}, CcmDepositMetadata, Ethereum, }; -use cf_primitives::{ - chains::assets::eth::Asset as EthAsset, Asset, AssetAmount, ChannelId, EpochIndex, -}; +use cf_primitives::{chains::assets::eth::Asset as EthAsset, Asset, AssetAmount, EpochIndex}; use cf_utilities::task_scope::Scope; use futures_core::Future; use itertools::Itertools; @@ -237,8 +235,6 @@ impl super::evm::vault::IngressCallBuilder for EthCallBuilder { fn vault_swap_request( block_height: u64, source_asset: Asset, - deposit_address: cf_chains::eth::Address, - channel_id: ChannelId, deposit_amount: AssetAmount, destination_asset: Asset, destination_address: EncodedAddress, @@ -268,8 +264,8 @@ impl super::evm::vault::IngressCallBuilder for EthCallBuilder { boost_fee: vault_swap_parameters.boost_fee.into(), dca_params: vault_swap_parameters.dca_params, refund_params: vault_swap_parameters.refund_params, - channel_id, - deposit_address, + channel_id: None, + deposit_address: None, } ], }, diff --git a/engine/src/witness/evm/vault.rs b/engine/src/witness/evm/vault.rs index 99e4f0dc96..e1b793e2da 100644 --- a/engine/src/witness/evm/vault.rs +++ b/engine/src/witness/evm/vault.rs @@ -21,7 +21,7 @@ use cf_chains::{ evm::DepositDetails, CcmChannelMetadata, CcmDepositMetadata, Chain, }; -use cf_primitives::{Asset, AssetAmount, ChannelId, EpochIndex, ForeignChain}; +use cf_primitives::{Asset, AssetAmount, EpochIndex, ForeignChain}; use ethers::prelude::*; use state_chain_runtime::{EthereumInstance, Runtime, RuntimeCall}; @@ -32,7 +32,6 @@ pub fn call_from_event< CallBuilder: IngressCallBuilder, >( block_height: u64, - contract_address: EthereumAddress, event: Event, // can be different for different EVM chains native_asset: Asset, @@ -58,10 +57,6 @@ where }) } - // The deposit address and channel id are always the same (unlike BTC vault swaps): - let deposit_address = contract_address; - let channel_id = 0; - Ok(match event.event_parameters { VaultEvents::SwapNativeFilter(SwapNativeFilter { dst_chain, @@ -79,8 +74,6 @@ where Some(CallBuilder::vault_swap_request( block_height, native_asset, - deposit_address, - channel_id, try_into_primitive(amount)?, try_into_primitive(dst_token)?, try_into_encoded_address(try_into_primitive(dst_chain)?, dst_address.to_vec())?, @@ -108,8 +101,6 @@ where *(supported_assets .get(&src_token) .ok_or(anyhow!("Source token {src_token:?} not found"))?), - deposit_address, - channel_id, try_into_primitive(amount)?, try_into_primitive(dst_token)?, try_into_encoded_address(try_into_primitive(dst_chain)?, dst_address.to_vec())?, @@ -136,8 +127,6 @@ where Some(CallBuilder::vault_swap_request( block_height, native_asset, - deposit_address, - channel_id, try_into_primitive(amount)?, try_into_primitive(dst_token)?, try_into_encoded_address(try_into_primitive(dst_chain)?, dst_address.to_vec())?, @@ -182,8 +171,6 @@ where *(supported_assets .get(&src_token) .ok_or(anyhow!("Source token {src_token:?} not found"))?), - deposit_address, - channel_id, try_into_primitive(amount)?, try_into_primitive(dst_token)?, try_into_encoded_address(try_into_primitive(dst_chain)?, dst_address.to_vec())?, @@ -244,8 +231,6 @@ pub trait IngressCallBuilder { fn vault_swap_request( block_height: ::ChainBlockNumber, source_asset: Asset, - deposit_address: ::ChainAccount, - channel_id: ChannelId, deposit_amount: cf_primitives::AssetAmount, destination_asset: Asset, destination_address: EncodedAddress, @@ -307,7 +292,6 @@ impl ChunkedByVaultBuilder { { match call_from_event::( header.index, - contract_address, event, native_asset, source_chain, diff --git a/state-chain/cf-integration-tests/src/solana.rs b/state-chain/cf-integration-tests/src/solana.rs index a929e48031..9a8e2d0892 100644 --- a/state-chain/cf-integration-tests/src/solana.rs +++ b/state-chain/cf-integration-tests/src/solana.rs @@ -456,8 +456,8 @@ fn vault_swap_deposit_witness( refund_params: REFUND_PARAMS, dca_params: None, boost_fee: 0, - deposit_address: SolAddress([2u8; 32]), - channel_id: 0, + deposit_address: Some(SolAddress([2u8; 32])), + channel_id: Some(0), } } diff --git a/state-chain/cf-integration-tests/src/swapping.rs b/state-chain/cf-integration-tests/src/swapping.rs index c41794916c..fda81939eb 100644 --- a/state-chain/cf-integration-tests/src/swapping.rs +++ b/state-chain/cf-integration-tests/src/swapping.rs @@ -589,8 +589,8 @@ fn vault_swap_deposit_witness( refund_params: ETH_REFUND_PARAMS, dca_params: None, boost_fee: 0, - deposit_address: H160::from([0x03; 20]), - channel_id: 0, + deposit_address: Some(H160::from([0x03; 20])), + channel_id: Some(0), } } diff --git a/state-chain/pallets/cf-ingress-egress/src/lib.rs b/state-chain/pallets/cf-ingress-egress/src/lib.rs index e4148c866f..5f3cce3c0c 100644 --- a/state-chain/pallets/cf-ingress-egress/src/lib.rs +++ b/state-chain/pallets/cf-ingress-egress/src/lib.rs @@ -23,8 +23,7 @@ pub use boost_pool::OwedAmount; use cf_chains::{ address::{ - AddressConverter, AddressDerivationApi, AddressDerivationError, EncodedAddress, - IntoForeignChainAddress, + AddressConverter, AddressDerivationApi, AddressDerivationError, IntoForeignChainAddress, }, assets::any::GetChainAssetMap, ccm_checker::CcmValidityCheck, @@ -148,11 +147,10 @@ mod deposit_origin { use super::*; - // TODO: replace SwapOrigin with this? - #[derive(Clone)] - pub(super) enum DepositOrigin { + #[derive(CloneNoBound)] + pub(super) enum DepositOrigin, I: 'static> { DepositChannel { - deposit_address: EncodedAddress, + deposit_address: ::ChainAccount, channel_id: ChannelId, deposit_block_height: u64, }, @@ -161,30 +159,26 @@ mod deposit_origin { }, } - impl DepositOrigin { - pub(super) fn deposit_channel, I: 'static>( + impl, I: 'static> DepositOrigin { + pub(super) fn deposit_channel( deposit_address: ::ChainAccount, channel_id: ChannelId, deposit_block_height: ::ChainBlockNumber, ) -> Self { DepositOrigin::DepositChannel { - deposit_address: T::AddressConverter::to_encoded_address( - ::ChainAccount::into_foreign_chain_address( - deposit_address.clone(), - ), - ), + deposit_address, channel_id, deposit_block_height: deposit_block_height.into(), } } - pub(super) fn vault, I: 'static>(tx_id: TransactionInIdFor) -> Self { + pub(super) fn vault(tx_id: TransactionInIdFor) -> Self { DepositOrigin::Vault { tx_id: tx_id.into_transaction_in_id_for_any_chain() } } } - impl From for DepositOriginType { - fn from(origin: DepositOrigin) -> Self { + impl, I: 'static> From> for DepositOriginType { + fn from(origin: DepositOrigin) -> Self { match origin { DepositOrigin::DepositChannel { .. } => DepositOriginType::DepositChannel, DepositOrigin::Vault { .. } => DepositOriginType::Vault, @@ -192,16 +186,23 @@ mod deposit_origin { } } - impl From for SwapOrigin { - fn from(origin: DepositOrigin) -> SwapOrigin { + impl, I: 'static> From> for SwapOrigin { + fn from(origin: DepositOrigin) -> SwapOrigin { match origin { DepositOrigin::Vault { tx_id } => SwapOrigin::Vault { tx_id }, DepositOrigin::DepositChannel { deposit_address, channel_id, deposit_block_height, - } => - SwapOrigin::DepositChannel { deposit_address, channel_id, deposit_block_height }, + } => SwapOrigin::DepositChannel { + deposit_address: T::AddressConverter::to_encoded_address( + ::ChainAccount::into_foreign_chain_address( + deposit_address.clone(), + ), + ), + channel_id, + deposit_block_height, + }, } } } @@ -358,8 +359,8 @@ pub mod pallet { #[scale_info(skip_type_params(T, I))] pub struct VaultDepositWitness, I: 'static> { pub input_asset: TargetChainAsset, - pub deposit_address: TargetChainAccount, - pub channel_id: ChannelId, + pub deposit_address: Option>, + pub channel_id: Option, pub deposit_amount: ::ChainAmount, pub deposit_details: ::DepositDetails, pub output_asset: Asset, @@ -702,7 +703,7 @@ pub mod pallet { #[pallet::generate_deposit(pub(super) fn deposit_event)] pub enum Event, I: 'static = ()> { DepositFinalised { - deposit_address: TargetChainAccount, + deposit_address: Option>, asset: TargetChainAsset, amount: TargetChainAmount, block_height: TargetChainBlockNumber, @@ -711,7 +712,7 @@ pub mod pallet { // a non-gas asset. ingress_fee: TargetChainAmount, action: DepositAction, - channel_id: ChannelId, + channel_id: Option, origin_type: DepositOriginType, }, AssetEgressStatusChanged { @@ -787,12 +788,12 @@ pub mod pallet { fee: T::Amount, }, DepositBoosted { - deposit_address: TargetChainAccount, + deposit_address: Option>, asset: TargetChainAsset, amounts: BTreeMap>, deposit_details: ::DepositDetails, prewitnessed_deposit_id: PrewitnessedDepositId, - channel_id: ChannelId, + channel_id: Option, block_height: TargetChainBlockNumber, // Ingress fee in the deposit asset. i.e. *NOT* the gas asset, if the deposit asset is // a non-gas asset. The ingress fee is taken *after* the boost fee. @@ -821,7 +822,7 @@ pub mod pallet { prewitnessed_deposit_id: PrewitnessedDepositId, asset: TargetChainAsset, amount_attempted: TargetChainAmount, - channel_id: ChannelId, + channel_id: Option, }, BoostPoolCreated { boost_pool: BoostPoolId, @@ -1821,25 +1822,23 @@ impl, I: 'static> Pallet { } = DepositChannelLookup::::get(&deposit_address) .ok_or(Error::::InvalidDepositAddress)?; - let origin = DepositOrigin::deposit_channel::( - deposit_address.clone(), - deposit_channel.channel_id, - block_height, - ); - if let Some(new_boost_status) = Self::process_prewitness_deposit_inner( amount, asset, deposit_details, - deposit_address.clone(), + Some(deposit_address.clone()), None, // source address is unknown action, &owner, boost_fee, boost_status, - deposit_channel.channel_id, + Some(deposit_channel.channel_id), block_height, - origin, + DepositOrigin::deposit_channel( + deposit_address.clone(), + deposit_channel.channel_id, + block_height, + ), ) { // Update boost status DepositChannelLookup::::mutate(&deposit_address, |details| { @@ -1857,7 +1856,7 @@ impl, I: 'static> Pallet { asset: TargetChainAsset, source_address: Option, amount_after_fees: TargetChainAmount, - origin: DepositOrigin, + origin: DepositOrigin, ) -> Result, DispatchError> { let action = match action { ChannelAction::LiquidityProvision { lp_account, .. } => { @@ -1969,25 +1968,19 @@ impl, I: 'static> Pallet { return Err(Error::::InvalidDepositAddress.into()) } - let origin = DepositOrigin::deposit_channel::( - deposit_address.clone(), - channel_id, - block_height, - ); - if let Ok(FullWitnessDepositOutcome::BoostFinalised) = Self::process_full_witness_deposit_inner( - deposit_address.clone(), + Some(deposit_address.clone()), asset, deposit_amount, deposit_details, None, // source address is unknown &deposit_channel_details.owner, deposit_channel_details.boost_status, - channel_id, + Some(channel_id), deposit_channel_details.action, block_height, - origin, + DepositOrigin::deposit_channel(deposit_address.clone(), channel_id, block_height), ) { // This allows the channel to be boosted again: DepositChannelLookup::::mutate(&deposit_address, |details| { @@ -2043,15 +2036,15 @@ impl, I: 'static> Pallet { amount: TargetChainAmount, asset: TargetChainAsset, deposit_details: ::DepositDetails, - deposit_address: TargetChainAccount, + deposit_address: Option>, source_address: Option, action: ChannelAction, broker: &T::AccountId, boost_fee: u16, boost_status: BoostStatus>, - channel_id: u64, + channel_id: Option, block_height: TargetChainBlockNumber, - origin: DepositOrigin, + origin: DepositOrigin, ) -> Option>> { if amount < MinimumDeposit::::get(asset) { // We do not process/record pre-witnessed deposits for amounts smaller @@ -2214,7 +2207,7 @@ impl, I: 'static> Pallet { let boost_status = BoostedVaultTxs::::get(&tx_id).unwrap_or(BoostStatus::NotBoosted); - let origin = DepositOrigin::vault::(tx_id.clone()); + let origin = DepositOrigin::vault(tx_id.clone()); if let Some(new_boost_status) = Self::process_prewitness_deposit_inner( amount, @@ -2235,17 +2228,17 @@ impl, I: 'static> Pallet { } fn process_full_witness_deposit_inner( - deposit_address: TargetChainAccount, + deposit_address: Option>, asset: TargetChainAsset, deposit_amount: TargetChainAmount, deposit_details: ::DepositDetails, source_address: Option, broker: &T::AccountId, boost_status: BoostStatus>, - channel_id: u64, + channel_id: Option, action: ChannelAction, block_height: TargetChainBlockNumber, - origin: DepositOrigin, + origin: DepositOrigin, ) -> Result { // TODO: only apply this check if the deposit hasn't been boosted // already (in case MinimumDeposit increases after some small deposit @@ -2255,7 +2248,7 @@ impl, I: 'static> Pallet { // If the deposit amount is below the minimum allowed, the deposit is ignored. // TODO: track these funds somewhere, for example add them to the withheld fees. Self::deposit_event(Event::::DepositIgnored { - deposit_address: Some(deposit_address), + deposit_address, asset, amount: deposit_amount, deposit_details, @@ -2287,7 +2280,7 @@ impl, I: 'static> Pallet { ScheduledTxForReject::::append(tainted_transaction_details); Self::deposit_event(Event::::DepositIgnored { - deposit_address: Some(deposit_address), + deposit_address, asset, amount: deposit_amount, deposit_details, @@ -2298,16 +2291,26 @@ impl, I: 'static> Pallet { } // Vault deposits don't need to be fetched: - if !matches!(origin, DepositOrigin::Vault { .. }) { - ScheduledEgressFetchOrTransfer::::append( - FetchOrTransfer::::Fetch { + if !matches!(origin, DepositOrigin::Vault { .. }) {} + + match &origin { + DepositOrigin::DepositChannel { deposit_address, channel_id, .. } => { + ScheduledEgressFetchOrTransfer::::append( + FetchOrTransfer::::Fetch { + asset, + deposit_address: deposit_address.clone(), + deposit_fetch_id: None, + amount: deposit_amount, + }, + ); + Self::deposit_event(Event::::DepositFetchesScheduled { + channel_id: *channel_id, asset, - deposit_address: deposit_address.clone(), - deposit_fetch_id: None, - amount: deposit_amount, - }, - ); - Self::deposit_event(Event::::DepositFetchesScheduled { channel_id, asset }); + }); + }, + DepositOrigin::Vault { .. } => { + // Vault deposits don't need to be fetched + }, } // Add the deposit to the balance. @@ -2354,7 +2357,7 @@ impl, I: 'static> Pallet { amount: deposit_amount, block_height, deposit_details, - // no ingrees fee as it was already charged at the time of boosting + // no ingress fee as it was already charged at the time of boosting ingress_fee: 0u32.into(), action: DepositAction::BoostersCredited { prewitnessed_deposit_id }, channel_id, @@ -2368,7 +2371,7 @@ impl, I: 'static> Pallet { if amount_after_fees.is_zero() { Self::deposit_event(Event::::DepositIgnored { - deposit_address: Some(deposit_address), + deposit_address, asset, amount: deposit_amount, deposit_details, @@ -2439,7 +2442,7 @@ impl, I: 'static> Pallet { }, }; - let deposit_origin = DepositOrigin::vault::(tx_id.clone()); + let deposit_origin = DepositOrigin::vault(tx_id.clone()); if let Some(deposit_metadata) = &deposit_metadata { if T::CcmValidityChecker::check_and_decode( @@ -2657,7 +2660,7 @@ impl, I: 'static> Pallet { fn conditionally_withhold_ingress_fee( asset: TargetChainAsset, available_amount: TargetChainAmount, - origin: &DepositOrigin, + origin: &DepositOrigin, ) -> AmountAndFeesWithheld { if matches!(origin, &DepositOrigin::DepositChannel { .. }) { Self::withhold_ingress_or_egress_fee(IngressOrEgress::Ingress, asset, available_amount) diff --git a/state-chain/pallets/cf-ingress-egress/src/tests.rs b/state-chain/pallets/cf-ingress-egress/src/tests.rs index e25c2e82c9..b106f853e8 100644 --- a/state-chain/pallets/cf-ingress-egress/src/tests.rs +++ b/state-chain/pallets/cf-ingress-egress/src/tests.rs @@ -618,7 +618,7 @@ fn multi_deposit_includes_deposit_beyond_recycle_height() { crate::Event::DepositFinalised { deposit_address, .. - }) if deposit_address == expected_accepted_address + }) if deposit_address.as_ref() == Some(expected_accepted_address) )),); }); } @@ -848,14 +848,14 @@ fn deposits_below_minimum_are_rejected() { let (channel_id, deposit_address) = request_address_and_deposit(LP_ACCOUNT, flip); System::assert_last_event(RuntimeEvent::IngressEgress( crate::Event::::DepositFinalised { - deposit_address, + deposit_address: Some(deposit_address), asset: flip, amount: default_deposit_amount, block_height: Default::default(), deposit_details: Default::default(), ingress_fee: 0, action: DepositAction::LiquidityProvision { lp_account: LP_ACCOUNT }, - channel_id, + channel_id: Some(channel_id), origin_type: DepositOriginType::DepositChannel, }, )); @@ -1812,8 +1812,8 @@ fn submit_vault_swap_request( 0, vec![VaultDepositWitness { input_asset: input_asset.try_into().unwrap(), - deposit_address, - channel_id: 0, + deposit_address: Some(deposit_address), + channel_id: Some(0), deposit_amount, deposit_details, output_asset, 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 5da283efe5..77eae93b7a 100644 --- a/state-chain/pallets/cf-ingress-egress/src/tests/boost.rs +++ b/state-chain/pallets/cf-ingress-egress/src/tests/boost.rs @@ -214,14 +214,14 @@ fn basic_passive_boosting() { const POOL_2_CONTRIBUTION: AssetAmount = DEPOSIT_AMOUNT - POOL_1_CONTRIBUTION; System::assert_last_event(RuntimeEvent::IngressEgress(Event::DepositBoosted { - deposit_address, + deposit_address: Some(deposit_address), asset: ASSET, amounts: BTreeMap::from_iter(vec![ (TIER_5_BPS, POOL_1_CONTRIBUTION), (TIER_10_BPS, POOL_2_CONTRIBUTION), ]), block_height: Default::default(), - channel_id, + channel_id: Some(channel_id), prewitnessed_deposit_id, deposit_details: Default::default(), ingress_fee: INGRESS_FEE, @@ -247,14 +247,14 @@ fn basic_passive_boosting() { witness_deposit(deposit_address, ASSET, DEPOSIT_AMOUNT); System::assert_last_event(RuntimeEvent::IngressEgress(Event::DepositFinalised { - deposit_address, + deposit_address: Some(deposit_address), asset: ASSET, amount: DEPOSIT_AMOUNT, block_height: Default::default(), deposit_details: Default::default(), ingress_fee: 0, action: DepositAction::BoostersCredited { prewitnessed_deposit_id }, - channel_id, + channel_id: Some(channel_id), origin_type: DepositOriginType::DepositChannel, })); @@ -672,7 +672,7 @@ fn insufficient_funds_for_boost() { prewitnessed_deposit_id: deposit_id, asset: EthAsset::Eth, amount_attempted: DEPOSIT_AMOUNT, - channel_id, + channel_id: Some(channel_id), })); // When the deposit is finalised, it is processed as normal: @@ -1031,8 +1031,8 @@ mod vault_swaps { let deposit = VaultDepositWitness { input_asset: INPUT_ASSET.try_into().unwrap(), - deposit_address, - channel_id: CHANNEL_ID, + deposit_address: Some(deposit_address), + channel_id: Some(CHANNEL_ID), deposit_amount: DEPOSIT_AMOUNT, deposit_details: Default::default(), output_asset: OUTPUT_ASSET, @@ -1082,7 +1082,7 @@ mod vault_swaps { Test, RuntimeEvent::IngressEgress(Event::DepositBoosted { prewitnessed_deposit_id: PREWITNESS_DEPOSIT_ID, - channel_id: CHANNEL_ID, + channel_id: Some(CHANNEL_ID), action: DepositAction::Swap { .. }, .. }) @@ -1146,7 +1146,7 @@ mod vault_swaps { assert_has_matching_event!( Test, RuntimeEvent::IngressEgress(Event::DepositFinalised { - channel_id: CHANNEL_ID, + channel_id: Some(CHANNEL_ID), action: DepositAction::BoostersCredited { prewitnessed_deposit_id: PREWITNESS_DEPOSIT_ID }, diff --git a/state-chain/runtime/src/chainflip/solana_elections.rs b/state-chain/runtime/src/chainflip/solana_elections.rs index 2c40c969d9..0454116678 100644 --- a/state-chain/runtime/src/chainflip/solana_elections.rs +++ b/state-chain/runtime/src/chainflip/solana_elections.rs @@ -565,8 +565,8 @@ impl block_height, VaultDepositWitness { input_asset: swap_details.from, - deposit_address: todo!(), - channel_id: todo!(), + deposit_address: None, + channel_id: None, deposit_amount: swap_details.deposit_amount, deposit_details: (), output_asset: swap_details.to, From 8c273d2645a713c49f8c56362355eb3df9dd5a63 Mon Sep 17 00:00:00 2001 From: Maxim Shishmarev Date: Tue, 3 Dec 2024 14:48:15 +1100 Subject: [PATCH 03/13] refactor: vault_deposit_witness macro and other small changes --- engine/src/witness/arb.rs | 35 ++++++----------- engine/src/witness/eth.rs | 38 +++++++------------ engine/src/witness/evm/vault.rs | 30 +++++++++++++++ .../pallets/cf-ingress-egress/src/lib.rs | 21 ++++++---- .../cf-ingress-egress/src/tests/boost.rs | 8 ++-- 5 files changed, 73 insertions(+), 59 deletions(-) diff --git a/engine/src/witness/arb.rs b/engine/src/witness/arb.rs index eb6ddaf4f1..fa9f4ee038 100644 --- a/engine/src/witness/arb.rs +++ b/engine/src/witness/arb.rs @@ -31,7 +31,7 @@ use crate::{ use super::{ common::{chain_source::extension::ChainSourceExt, epoch_source::EpochSourceBuilder}, - evm::source::EvmSource, + evm::{source::EvmSource, vault::vault_deposit_witness}, }; use anyhow::{Context, Result}; @@ -195,31 +195,20 @@ impl super::evm::vault::IngressCallBuilder for ArbCallBuilder { tx_id: H256, vault_swap_parameters: VaultSwapParameters, ) -> state_chain_runtime::RuntimeCall { + let deposit = vault_deposit_witness!( + source_asset, + deposit_amount, + destination_asset, + destination_address, + deposit_metadata, + tx_id, + vault_swap_parameters + ); + state_chain_runtime::RuntimeCall::ArbitrumIngressEgress( pallet_cf_ingress_egress::Call::vault_swap_request { block_height, - deposits: vec![VaultDepositWitness { - input_asset: source_asset.try_into().expect("invalid asset for chain"), - output_asset: destination_asset, - deposit_amount, - destination_address, - deposit_metadata, - tx_id, - deposit_details: DepositDetails { tx_hashes: Some(vec![tx_id]) }, - broker_fee: vault_swap_parameters.broker_fee, - affiliate_fees: vault_swap_parameters - .affiliate_fees - .into_iter() - .map(|entry| Beneficiary { account: entry.affiliate, bps: entry.fee.into() }) - .collect_vec() - .try_into() - .expect("runtime supports at least as many affiliates as we allow in cf_parameters encoding"), - boost_fee: vault_swap_parameters.boost_fee.into(), - dca_params: vault_swap_parameters.dca_params, - refund_params: vault_swap_parameters.refund_params, - channel_id: None, - deposit_address: None, - }], + deposits: vec![deposit], }, ) } diff --git a/engine/src/witness/eth.rs b/engine/src/witness/eth.rs index be068cb74a..60ca0457b5 100644 --- a/engine/src/witness/eth.rs +++ b/engine/src/witness/eth.rs @@ -31,7 +31,10 @@ use crate::{ }, }; -use super::{common::epoch_source::EpochSourceBuilder, evm::source::EvmSource}; +use super::{ + common::epoch_source::EpochSourceBuilder, + evm::{source::EvmSource, vault::vault_deposit_witness}, +}; use crate::witness::common::chain_source::extension::ChainSourceExt; use anyhow::{Context, Result}; @@ -242,32 +245,19 @@ impl super::evm::vault::IngressCallBuilder for EthCallBuilder { tx_id: H256, vault_swap_parameters: VaultSwapParameters, ) -> state_chain_runtime::RuntimeCall { + let deposit = vault_deposit_witness!( + source_asset, + deposit_amount, + destination_asset, + destination_address, + deposit_metadata, + tx_id, + vault_swap_parameters + ); state_chain_runtime::RuntimeCall::EthereumIngressEgress( pallet_cf_ingress_egress::Call::vault_swap_request { block_height, - deposits: vec![ VaultDepositWitness { - input_asset: source_asset.try_into().expect("invalid asset for chain"), - output_asset: destination_asset, - deposit_amount, - destination_address, - deposit_metadata, - tx_id, - deposit_details: DepositDetails { tx_hashes: Some(vec![tx_id]) }, - broker_fee: vault_swap_parameters.broker_fee, - affiliate_fees: vault_swap_parameters - .affiliate_fees - .into_iter() - .map(Into::into) - .collect_vec() - .try_into() - .expect("runtime supports at least as many affiliates as we allow in cf_parameters encoding"), - boost_fee: vault_swap_parameters.boost_fee.into(), - dca_params: vault_swap_parameters.dca_params, - refund_params: vault_swap_parameters.refund_params, - channel_id: None, - deposit_address: None, - } - ], + deposits: vec![deposit], }, ) } diff --git a/engine/src/witness/evm/vault.rs b/engine/src/witness/evm/vault.rs index e1b793e2da..b3910540b4 100644 --- a/engine/src/witness/evm/vault.rs +++ b/engine/src/witness/evm/vault.rs @@ -1,6 +1,7 @@ use crate::evm::retry_rpc::EvmRetryRpcApi; use codec::Decode; use ethers::types::Bloom; +use pallet_cf_ingress_egress::VaultDepositWitness; use sp_core::H256; use std::collections::HashMap; @@ -225,6 +226,35 @@ where }) } +macro_rules! vault_deposit_witness { + ($source_asset: expr, $deposit_amount: expr, $dest_asset: expr, $dest_address: expr, $metadata: expr, $tx_id: expr, $params: expr) => { + VaultDepositWitness { + input_asset: $source_asset.try_into().expect("invalid asset for chain"), + output_asset: $dest_asset, + deposit_amount: $deposit_amount, + destination_address: $dest_address, + deposit_metadata: $metadata, + tx_id: $tx_id, + deposit_details: DepositDetails { tx_hashes: Some(vec![$tx_id]) }, + broker_fee: $params.broker_fee, + affiliate_fees: $params + .affiliate_fees + .into_iter() + .map(Into::into) + .collect_vec() + .try_into() + .expect("runtime supports at least as many affiliates as we allow in cf_parameters encoding"), + boost_fee: $params.boost_fee.into(), + dca_params: $params.dca_params, + refund_params: $params.refund_params, + channel_id: None, + deposit_address: None, + } + } +} + +pub(crate) use vault_deposit_witness; + pub trait IngressCallBuilder { type Chain: cf_chains::Chain; diff --git a/state-chain/pallets/cf-ingress-egress/src/lib.rs b/state-chain/pallets/cf-ingress-egress/src/lib.rs index 5f3cce3c0c..66443e527c 100644 --- a/state-chain/pallets/cf-ingress-egress/src/lib.rs +++ b/state-chain/pallets/cf-ingress-egress/src/lib.rs @@ -155,7 +155,7 @@ mod deposit_origin { deposit_block_height: u64, }, Vault { - tx_id: TransactionInIdForAnyChain, + tx_id: TransactionInIdFor, }, } @@ -173,7 +173,7 @@ mod deposit_origin { } pub(super) fn vault(tx_id: TransactionInIdFor) -> Self { - DepositOrigin::Vault { tx_id: tx_id.into_transaction_in_id_for_any_chain() } + DepositOrigin::Vault { tx_id } } } @@ -189,7 +189,8 @@ mod deposit_origin { impl, I: 'static> From> for SwapOrigin { fn from(origin: DepositOrigin) -> SwapOrigin { match origin { - DepositOrigin::Vault { tx_id } => SwapOrigin::Vault { tx_id }, + DepositOrigin::Vault { tx_id } => + SwapOrigin::Vault { tx_id: tx_id.into_transaction_in_id_for_any_chain() }, DepositOrigin::DepositChannel { deposit_address, channel_id, @@ -691,7 +692,7 @@ pub mod pallet { /// Stores transaction ids that have been boosted but have not yet been finalised. #[pallet::storage] - pub(crate) type BoostedVaultTxs, I: 'static = ()> = StorageMap< + pub(crate) type BoostedVaultTransactions, I: 'static = ()> = StorageMap< _, Identity, TransactionInIdFor, @@ -2205,7 +2206,8 @@ impl, I: 'static> Pallet { ) }; - let boost_status = BoostedVaultTxs::::get(&tx_id).unwrap_or(BoostStatus::NotBoosted); + let boost_status = + BoostedVaultTransactions::::get(&tx_id).unwrap_or(BoostStatus::NotBoosted); let origin = DepositOrigin::vault(tx_id.clone()); @@ -2223,7 +2225,7 @@ impl, I: 'static> Pallet { block_height, origin, ) { - BoostedVaultTxs::::insert(&tx_id, new_boost_status); + BoostedVaultTransactions::::insert(&tx_id, new_boost_status); } } @@ -2258,6 +2260,8 @@ impl, I: 'static> Pallet { } if let Some(tx_id) = deposit_details.deposit_id() { + // Only check if the transaction is tainted if we haven't already boosted it, + // since by boosting the protocol is committing to accept the deposit. if TaintedTransactions::::take(broker, &tx_id).is_some() && !matches!(boost_status, BoostStatus::Boosted { .. }) { @@ -2428,7 +2432,8 @@ impl, I: 'static> Pallet { boost_fee: _, }: VaultDepositWitness, ) { - let boost_status = BoostedVaultTxs::::get(&tx_id).unwrap_or(BoostStatus::NotBoosted); + let boost_status = + BoostedVaultTransactions::::get(&tx_id).unwrap_or(BoostStatus::NotBoosted); let destination_address_internal = match T::AddressConverter::decode_and_validate_address_for_asset( @@ -2530,7 +2535,7 @@ impl, I: 'static> Pallet { deposit_origin, ) { // This allows the channel to be boosted again: - BoostedVaultTxs::::remove(&tx_id); + BoostedVaultTransactions::::remove(&tx_id); } } 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 77eae93b7a..4e28a00d6d 100644 --- a/state-chain/pallets/cf-ingress-egress/src/tests/boost.rs +++ b/state-chain/pallets/cf-ingress-egress/src/tests/boost.rs @@ -994,7 +994,7 @@ fn test_create_boost_pools() { mod vault_swaps { - use crate::BoostedVaultTxs; + use crate::BoostedVaultTransactions; use super::*; @@ -1027,7 +1027,7 @@ mod vault_swaps { let tx_id = [9u8; 32].into(); // Initially tx is not recorded as boosted - assert!(!BoostedVaultTxs::::contains_key(tx_id)); + assert!(!BoostedVaultTransactions::::contains_key(tx_id)); let deposit = VaultDepositWitness { input_asset: INPUT_ASSET.try_into().unwrap(), @@ -1089,7 +1089,7 @@ mod vault_swaps { ); // Now the tx is recorded as boosted - assert!(BoostedVaultTxs::::contains_key(tx_id)); + assert!(BoostedVaultTransactions::::contains_key(tx_id)); } // Prewitnessing the same deposit (e.g. due to a reorg) should not result in a second @@ -1155,7 +1155,7 @@ mod vault_swaps { ); // Boost record for tx is removed: - assert!(!BoostedVaultTxs::::contains_key(tx_id)); + assert!(!BoostedVaultTransactions::::contains_key(tx_id)); } }); } From a323d4f15a4f619a95ff3bf3e5c45fab0d4268f6 Mon Sep 17 00:00:00 2001 From: Maxim Shishmarev Date: Tue, 3 Dec 2024 17:04:20 +1100 Subject: [PATCH 04/13] refactor: inline process_deposit_witnesses --- .../cf-ingress-egress/src/benchmarking.rs | 8 +- .../pallets/cf-ingress-egress/src/lib.rs | 82 +++------ .../cf-ingress-egress/src/mocks/mock_eth.rs | 8 +- .../pallets/cf-ingress-egress/src/tests.rs | 171 +++++++++--------- .../cf-ingress-egress/src/tests/boost.rs | 10 +- .../cf-ingress-egress/src/tests/screening.rs | 56 +++--- .../pallets/cf-ingress-egress/src/weights.rs | 6 +- 7 files changed, 165 insertions(+), 176 deletions(-) diff --git a/state-chain/pallets/cf-ingress-egress/src/benchmarking.rs b/state-chain/pallets/cf-ingress-egress/src/benchmarking.rs index 891dd2667e..dc74db8f67 100644 --- a/state-chain/pallets/cf-ingress-egress/src/benchmarking.rs +++ b/state-chain/pallets/cf-ingress-egress/src/benchmarking.rs @@ -49,7 +49,7 @@ mod benchmarks { } #[benchmark] - fn process_single_deposit() { + fn process_channel_deposit_full_witness() { const CHANNEL_ID: u64 = 1; let deposit_address: <>::TargetChain as Chain>::ChainAccount = @@ -82,7 +82,7 @@ mod benchmarks { #[block] { - assert_ok!(Pallet::::process_single_deposit( + assert_ok!(Pallet::::process_channel_deposit_full_witness( deposit_address, source_asset, deposit_amount, @@ -478,7 +478,7 @@ mod benchmarks { #[block] { - assert_ok!(Pallet::::process_single_deposit( + assert_ok!(Pallet::::process_channel_deposit_full_witness( deposit_address, asset, 1_000u32.into(), @@ -544,7 +544,7 @@ mod benchmarks { _finalise_ingress::(100, true); }); new_test_ext().execute_with(|| { - _process_single_deposit::(true); + _process_channel_deposit_full_witness::(true); }); new_test_ext().execute_with(|| { _disable_asset_egress::(true); diff --git a/state-chain/pallets/cf-ingress-egress/src/lib.rs b/state-chain/pallets/cf-ingress-egress/src/lib.rs index 66443e527c..594900552a 100644 --- a/state-chain/pallets/cf-ingress-egress/src/lib.rs +++ b/state-chain/pallets/cf-ingress-egress/src/lib.rs @@ -31,7 +31,7 @@ use cf_chains::{ CcmFailReason, CcmMessage, Chain, ChainCrypto, ChannelLifecycleHooks, ChannelRefundParameters, ConsolidateCall, DepositChannel, DepositDetailsToTransactionInId, DepositOriginType, ExecutexSwapAndCall, FetchAssetParams, ForeignChainAddress, IntoTransactionInIdForAnyChain, - RejectCall, SwapOrigin, TransactionInIdForAnyChain, TransferAssetParams, + RejectCall, SwapOrigin, TransferAssetParams, }; use cf_primitives::{ AccountRole, AffiliateShortId, Affiliates, Asset, AssetAmount, BasisPoints, Beneficiaries, @@ -1156,7 +1156,16 @@ pub mod pallet { Self::add_prewitnessed_deposits(deposit_witnesses, block_height)?; } else { T::EnsureWitnessed::ensure_origin(origin)?; - Self::process_deposit_witnesses(deposit_witnesses, block_height)?; + + for deposit_witness in deposit_witnesses { + Self::process_channel_deposit_full_witness(&deposit_witness, block_height) + .unwrap_or_else(|e| { + Self::deposit_event(Event::::DepositWitnessRejected { + reason: e, + deposit_witness, + }); + }) + } } Ok(()) } @@ -1439,18 +1448,16 @@ impl, I: 'static> IngressSink for Pallet { block_number: Self::BlockNumber, details: Self::DepositDetails, ) { - Self::process_single_deposit(channel.clone(), asset, amount, details.clone(), block_number) - .unwrap_or_else(|e| { + let deposit_witness = + DepositWitness { deposit_address: channel, asset, amount, deposit_details: details }; + Self::process_channel_deposit_full_witness(&deposit_witness, block_number).unwrap_or_else( + |e| { Self::deposit_event(Event::::DepositWitnessRejected { reason: e, - deposit_witness: DepositWitness { - deposit_address: channel, - asset, - amount, - deposit_details: details, - }, + deposit_witness, }); - }); + }, + ); } fn on_ingress_reverted(_channel: Self::Account, _asset: Self::Asset, _amount: Self::Amount) {} @@ -1716,34 +1723,6 @@ impl, I: 'static> Pallet { } } - fn process_deposit_witnesses( - deposit_witnesses: Vec>, - block_height: TargetChainBlockNumber, - ) -> DispatchResult { - for ref deposit_witness @ DepositWitness { - ref deposit_address, - asset, - amount, - ref deposit_details, - } in deposit_witnesses - { - Self::process_single_deposit( - deposit_address.clone(), - asset, - amount, - deposit_details.clone(), - block_height, - ) - .unwrap_or_else(|e| { - Self::deposit_event(Event::::DepositWitnessRejected { - reason: e, - deposit_witness: deposit_witness.clone(), - }); - }) - } - Ok(()) - } - /// Returns a list of contributions from the used pools and the total boost fee. #[transactional] fn try_boosting( @@ -1943,18 +1922,17 @@ impl, I: 'static> Pallet { /// Completes a single deposit request. #[transactional] - fn process_single_deposit( - deposit_address: TargetChainAccount, - asset: TargetChainAsset, - deposit_amount: TargetChainAmount, - deposit_details: ::DepositDetails, + fn process_channel_deposit_full_witness( + DepositWitness { deposit_address, asset, amount, deposit_details }: &DepositWitness< + T::TargetChain, + >, block_height: TargetChainBlockNumber, ) -> DispatchResult { let deposit_channel_details = DepositChannelLookup::::get(&deposit_address) .ok_or(Error::::InvalidDepositAddress)?; ensure!( - deposit_channel_details.deposit_channel.asset == asset, + deposit_channel_details.deposit_channel.asset == *asset, Error::::AssetMismatch ); @@ -1972,9 +1950,9 @@ impl, I: 'static> Pallet { if let Ok(FullWitnessDepositOutcome::BoostFinalised) = Self::process_full_witness_deposit_inner( Some(deposit_address.clone()), - asset, - deposit_amount, - deposit_details, + *asset, + *amount, + deposit_details.clone(), None, // source address is unknown &deposit_channel_details.owner, deposit_channel_details.boost_status, @@ -2129,16 +2107,13 @@ impl, I: 'static> Pallet { amount, }); }, - Err(err) => { + Err(_) => { Self::deposit_event(Event::InsufficientBoostLiquidity { prewitnessed_deposit_id, asset, amount_attempted: amount, channel_id, }); - log::debug!( - "Deposit (id: {prewitnessed_deposit_id}) of {amount:?} {asset:?} and boost fee {boost_fee} could not be boosted: {err:?}" - ); }, } } @@ -2392,7 +2367,6 @@ impl, I: 'static> Pallet { amount_after_fees, origin.clone(), ) { - // TODO: this needs to include deposit type (vault/channel) Self::deposit_event(Event::DepositFinalised { deposit_address, asset, @@ -2534,7 +2508,7 @@ impl, I: 'static> Pallet { block_height, deposit_origin, ) { - // This allows the channel to be boosted again: + // Clean up a record that's no longer needed: BoostedVaultTransactions::::remove(&tx_id); } } diff --git a/state-chain/pallets/cf-ingress-egress/src/mocks/mock_eth.rs b/state-chain/pallets/cf-ingress-egress/src/mocks/mock_eth.rs index 008cb86310..04d2f7d4fc 100644 --- a/state-chain/pallets/cf-ingress-egress/src/mocks/mock_eth.rs +++ b/state-chain/pallets/cf-ingress-egress/src/mocks/mock_eth.rs @@ -194,16 +194,16 @@ impl RequestAddressAndDeposit for TestRunner { .zip(amounts) .map(|((request, channel_id, deposit_address), amount)| { if !amount.is_zero() { - IngressEgress::process_deposit_witnesses( - vec![DepositWitness { + IngressEgress::process_channel_deposit_full_witness( + &DepositWitness { deposit_address, asset: request.source_asset(), amount, deposit_details: Default::default(), - }], + }, Default::default(), ) - .unwrap(); + .unwrap() } (request, channel_id, deposit_address) }) diff --git a/state-chain/pallets/cf-ingress-egress/src/tests.rs b/state-chain/pallets/cf-ingress-egress/src/tests.rs index b106f853e8..d7f91cb028 100644 --- a/state-chain/pallets/cf-ingress-egress/src/tests.rs +++ b/state-chain/pallets/cf-ingress-egress/src/tests.rs @@ -243,11 +243,13 @@ fn request_address_and_deposit( ) .unwrap(); let address: ::ChainAccount = address.try_into().unwrap(); - assert_ok!(IngressEgress::process_single_deposit( - address, - asset, - DEFAULT_DEPOSIT_AMOUNT, - Default::default(), + assert_ok!(IngressEgress::process_channel_deposit_full_witness( + &DepositWitness { + deposit_address: address.clone(), + asset, + amount: DEFAULT_DEPOSIT_AMOUNT, + deposit_details: Default::default() + }, Default::default() )); (id, address) @@ -576,25 +578,27 @@ fn multi_deposit_includes_deposit_beyond_recycle_height() { (address, address2) }) .then_execute_at_next_block(|(address, address2)| { - IngressEgress::process_deposit_witnesses( - vec![ - DepositWitness { - deposit_address: address, - asset: ETH, - amount: 1, - deposit_details: Default::default(), - }, - DepositWitness { - deposit_address: address2, - asset: ETH, - amount: 1, - deposit_details: Default::default(), - }, - ], - // block height is purely informative. - BlockHeightProvider::::get_block_height(), - ) - .unwrap(); + // block height is purely informative. + let block_height = BlockHeightProvider::::get_block_height(); + assert_ok!(IngressEgress::process_channel_deposit_full_witness( + &DepositWitness { + deposit_address: address, + asset: ETH, + amount: 1, + deposit_details: Default::default(), + }, + block_height, + )); + + assert_ok!(IngressEgress::process_channel_deposit_full_witness( + &DepositWitness { + deposit_address: address2, + asset: ETH, + amount: 1, + deposit_details: Default::default(), + }, + block_height, + )); (address, address2) }) .then_process_events(|_, event| match event { @@ -630,25 +634,26 @@ fn multi_use_deposit_address_different_blocks() { new_test_ext() .then_execute_at_next_block(|_| request_address_and_deposit(ALICE, ETH)) .then_execute_at_next_block(|(_, deposit_address)| { - IngressEgress::process_deposit_witnesses( - vec![DepositWitness { + assert_ok!(IngressEgress::process_channel_deposit_full_witness( + &DepositWitness { deposit_address, asset: ETH, amount: 1, deposit_details: Default::default(), - }], + }, // block height is purely informative. BlockHeightProvider::::get_block_height(), - ) - .unwrap(); + )); deposit_address }) .then_execute_at_next_block(|deposit_address| { - assert_ok!(Pallet::::process_single_deposit( - deposit_address, - ETH, - 1, - Default::default(), + assert_ok!(IngressEgress::process_channel_deposit_full_witness( + &DepositWitness { + deposit_address, + asset: ETH, + amount: 1, + deposit_details: Default::default() + }, Default::default() )); assert!( @@ -662,17 +667,16 @@ fn multi_use_deposit_address_different_blocks() { }) // The channel should be closed at the next block. .then_execute_at_next_block(|deposit_address| { - IngressEgress::process_deposit_witnesses( - vec![DepositWitness { + assert_ok!(IngressEgress::process_channel_deposit_full_witness( + &DepositWitness { deposit_address, asset: ETH, amount: 1, deposit_details: Default::default(), - }], + }, // block height is purely informative. BlockHeightProvider::::get_block_height(), - ) - .unwrap(); + )); deposit_address }) .then_process_events(|_, event| match event { @@ -708,24 +712,22 @@ fn multi_use_deposit_same_block() { }) .then_execute_at_next_block(|(request, channel_id, deposit_address)| { let asset = request.source_asset(); - IngressEgress::process_deposit_witnesses( - vec![ - DepositWitness { - deposit_address, - asset, - amount: MinimumDeposit::::get(asset) + DEPOSIT_AMOUNT, - deposit_details: Default::default(), - }, - DepositWitness { - deposit_address, - asset, - amount: MinimumDeposit::::get(asset) + DEPOSIT_AMOUNT, - deposit_details: Default::default(), - }, - ], + let deposit_witness = DepositWitness { + deposit_address, + asset, + amount: MinimumDeposit::::get(asset) + DEPOSIT_AMOUNT, + deposit_details: Default::default(), + }; + assert_ok!(IngressEgress::process_channel_deposit_full_witness( + &deposit_witness, Default::default(), - ) - .unwrap(); + )); + + assert_ok!(IngressEgress::process_channel_deposit_full_witness( + &deposit_witness, + Default::default(), + )); + (request, channel_id, deposit_address) }) .then_execute_with_keep_context(|(_, channel_id, deposit_address)| { @@ -883,15 +885,16 @@ fn deposits_ingress_fee_exceeding_deposit_amount_rejected() { let deposit_address = address.try_into().unwrap(); // Swap a low enough amount such that it gets swallowed by fees - let deposit_detail: DepositWitness = DepositWitness:: { + let deposit = DepositWitness:: { deposit_address, asset: ASSET, amount: DEPOSIT_AMOUNT, deposit_details: Default::default(), }; - assert_ok!(IngressEgress::process_deposit_witnesses( - vec![deposit_detail.clone()], - Default::default(), + + assert_ok!(IngressEgress::process_channel_deposit_full_witness( + &deposit, + Default::default() )); // Observe the DepositIgnored Event assert!( @@ -912,9 +915,9 @@ fn deposits_ingress_fee_exceeding_deposit_amount_rejected() { // Set fees to less than the deposit amount and retry. ChainTracker::::set_fee(LOW_FEE); - assert_ok!(IngressEgress::process_deposit_witnesses( - vec![deposit_detail], - Default::default(), + assert_ok!(IngressEgress::process_channel_deposit_full_witness( + &deposit, + Default::default() )); // Observe the DepositReceived Event assert!( @@ -946,14 +949,15 @@ fn handle_pending_deployment() { IngressEgress::on_finalize(1); assert_eq!(ScheduledEgressFetchOrTransfer::::decode_len().unwrap_or_default(), 0); // Process deposit again the same address. - Pallet::::process_single_deposit( - deposit_address, - ETH, - 1, - Default::default(), + assert_ok!(IngressEgress::process_channel_deposit_full_witness( + &DepositWitness { + deposit_address, + asset: ETH, + amount: 1, + deposit_details: Default::default(), + }, Default::default(), - ) - .unwrap(); + )); assert!(MockBalance::get_balance(&ALICE, ETH.into()) > 0, "LP account hasn't earned fees!"); // None-pending requests can still be sent request_address_and_deposit(1u64, EthAsset::Eth); @@ -976,14 +980,15 @@ fn handle_pending_deployment_same_block() { new_test_ext().execute_with(|| { // Initial request. let (_, deposit_address) = request_address_and_deposit(ALICE, EthAsset::Eth); - Pallet::::process_single_deposit( - deposit_address, - EthAsset::Eth, - 1, - Default::default(), + assert_ok!(IngressEgress::process_channel_deposit_full_witness( + &DepositWitness { + deposit_address, + asset: EthAsset::Eth, + amount: 1, + deposit_details: Default::default(), + }, Default::default(), - ) - .unwrap(); + )); assert!( MockBalance::get_balance(&ALICE, EthAsset::Eth.into()) > 0, "LP account hasn't earned fees!" @@ -1711,11 +1716,13 @@ fn do_not_batch_more_fetches_than_the_limit_allows() { .unwrap(); let address: ::ChainAccount = address.try_into().unwrap(); - assert_ok!(IngressEgress::process_single_deposit( - address, - ASSET, - DEFAULT_DEPOSIT_AMOUNT, - Default::default(), + assert_ok!(IngressEgress::process_channel_deposit_full_witness( + &DepositWitness { + deposit_address: address, + asset: ASSET, + amount: DEFAULT_DEPOSIT_AMOUNT, + deposit_details: Default::default(), + }, Default::default() )); } 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 4e28a00d6d..dc3749d9f9 100644 --- a/state-chain/pallets/cf-ingress-egress/src/tests/boost.rs +++ b/state-chain/pallets/cf-ingress-egress/src/tests/boost.rs @@ -78,14 +78,14 @@ fn prewitness_deposit(deposit_address: H160, asset: EthAsset, amount: AssetAmoun #[track_caller] fn witness_deposit(deposit_address: H160, asset: EthAsset, amount: AssetAmount) { - assert_ok!(Pallet::::process_deposit_witnesses( - vec![DepositWitness:: { + assert_ok!(Pallet::::process_channel_deposit_full_witness( + &DepositWitness:: { deposit_address, asset, amount, - deposit_details: Default::default() - }], - Default::default() + deposit_details: Default::default(), + }, + Default::default(), )); } diff --git a/state-chain/pallets/cf-ingress-egress/src/tests/screening.rs b/state-chain/pallets/cf-ingress-egress/src/tests/screening.rs index 123389d33b..24051e584f 100644 --- a/state-chain/pallets/cf-ingress-egress/src/tests/screening.rs +++ b/state-chain/pallets/cf-ingress-egress/src/tests/screening.rs @@ -55,11 +55,13 @@ mod helpers { ) .unwrap(); let address: ::ChainAccount = address.try_into().unwrap(); - assert_ok!(IngressEgress::process_single_deposit( - address.clone(), - asset, - DEFAULT_DEPOSIT_AMOUNT, - deposit_details, + assert_ok!(IngressEgress::process_channel_deposit_full_witness( + &DepositWitness { + deposit_address: address.clone(), + asset, + amount: DEFAULT_DEPOSIT_AMOUNT, + deposit_details + }, Default::default() )); (id, address) @@ -122,11 +124,13 @@ fn process_tainted_transaction_and_expect_refund() { tx_in_id, )); - assert_ok!(IngressEgress::process_single_deposit( - address, - btc::Asset::Btc, - DEFAULT_DEPOSIT_AMOUNT, - deposit_details, + assert_ok!(IngressEgress::process_channel_deposit_full_witness( + &DepositWitness { + deposit_address: address.clone(), + asset: btc::Asset::Btc, + amount: DEFAULT_DEPOSIT_AMOUNT, + deposit_details + }, Default::default() )); @@ -171,11 +175,13 @@ fn finalize_boosted_tx_if_tainted_after_prewitness() { // It's possible to report the tx, but reporting will have no effect. assert_ok!(IngressEgress::mark_transaction_as_tainted(OriginTrait::signed(BROKER), tx_id,),); - assert_ok!(IngressEgress::process_single_deposit( - address, - btc::Asset::Btc, - DEFAULT_DEPOSIT_AMOUNT, - deposit_details, + assert_ok!(IngressEgress::process_channel_deposit_full_witness( + &DepositWitness { + deposit_address: address.clone(), + asset: btc::Asset::Btc, + amount: DEFAULT_DEPOSIT_AMOUNT, + deposit_details + }, Default::default() )); @@ -215,11 +221,13 @@ fn reject_tx_if_tainted_before_prewitness() { 10, ); - assert_ok!(IngressEgress::process_single_deposit( - address, - btc::Asset::Btc, - DEFAULT_DEPOSIT_AMOUNT, - deposit_details, + assert_ok!(IngressEgress::process_channel_deposit_full_witness( + &DepositWitness { + deposit_address: address.clone(), + asset: btc::Asset::Btc, + amount: DEFAULT_DEPOSIT_AMOUNT, + deposit_details + }, Default::default() )); @@ -389,16 +397,16 @@ fn can_report_between_prewitness_and_witness_if_tx_was_not_boosted() { _ => unreachable!(), }; - let deposit_witnesses = vec![DepositWitness { + let deposit_witness = DepositWitness { deposit_address, asset: btc::Asset::Btc, amount: DEFAULT_DEPOSIT_AMOUNT, deposit_details, - }]; + }; - assert_ok!(IngressEgress::add_prewitnessed_deposits(deposit_witnesses.clone(), 10,)); + assert_ok!(IngressEgress::add_prewitnessed_deposits(vec![deposit_witness.clone()], 10,)); assert_ok!(IngressEgress::mark_transaction_as_tainted(OriginTrait::signed(BROKER), tx_id,)); - assert_ok!(IngressEgress::process_deposit_witnesses(deposit_witnesses, 10,)); + assert_ok!(IngressEgress::process_channel_deposit_full_witness(&deposit_witness, 10)); assert_has_matching_event!( Test, diff --git a/state-chain/pallets/cf-ingress-egress/src/weights.rs b/state-chain/pallets/cf-ingress-egress/src/weights.rs index 869141392a..719921a9f4 100644 --- a/state-chain/pallets/cf-ingress-egress/src/weights.rs +++ b/state-chain/pallets/cf-ingress-egress/src/weights.rs @@ -33,7 +33,7 @@ use core::marker::PhantomData; /// Weight functions needed for pallet_cf_ingress_egress. pub trait WeightInfo { fn disable_asset_egress() -> Weight; - fn process_single_deposit() -> Weight; + fn process_channel_deposit_full_witness() -> Weight; fn finalise_ingress(a: u32, ) -> Weight; fn vault_transfer_failed() -> Weight; fn ccm_broadcast_failed() -> Weight; @@ -75,7 +75,7 @@ impl WeightInfo for PalletWeight { /// Proof: `EthereumChainTracking::CurrentChainState` (`max_values`: Some(1), `max_size`: Some(40), added: 535, mode: `MaxEncodedLen`) /// Storage: `AssetBalances::WithheldAssets` (r:1 w:1) /// Proof: `AssetBalances::WithheldAssets` (`max_values`: None, `max_size`: None, mode: `Measured`) - fn process_single_deposit() -> Weight { + fn process_channel_deposit_full_witness() -> Weight { // Proof Size summary in bytes: // Measured: `624` // Estimated: `4089` @@ -325,7 +325,7 @@ impl WeightInfo for () { /// Proof: `EthereumChainTracking::CurrentChainState` (`max_values`: Some(1), `max_size`: Some(40), added: 535, mode: `MaxEncodedLen`) /// Storage: `AssetBalances::WithheldAssets` (r:1 w:1) /// Proof: `AssetBalances::WithheldAssets` (`max_values`: None, `max_size`: None, mode: `Measured`) - fn process_single_deposit() -> Weight { + fn process_channel_deposit_full_witness() -> Weight { // Proof Size summary in bytes: // Measured: `624` // Estimated: `4089` From 27b74a7a3c3a2b34de862cca2faac7186800c1d8 Mon Sep 17 00:00:00 2001 From: Maxim Shishmarev Date: Tue, 3 Dec 2024 17:18:01 +1100 Subject: [PATCH 05/13] refactor: rename add_prewitnessed_deposits --- .../pallets/cf-ingress-egress/src/lib.rs | 73 +++++++++---------- .../cf-ingress-egress/src/tests/boost.rs | 6 +- .../cf-ingress-egress/src/tests/screening.rs | 16 ++-- 3 files changed, 46 insertions(+), 49 deletions(-) diff --git a/state-chain/pallets/cf-ingress-egress/src/lib.rs b/state-chain/pallets/cf-ingress-egress/src/lib.rs index 594900552a..47b0d3c4ec 100644 --- a/state-chain/pallets/cf-ingress-egress/src/lib.rs +++ b/state-chain/pallets/cf-ingress-egress/src/lib.rs @@ -1153,7 +1153,10 @@ pub mod pallet { block_height: TargetChainBlockNumber, ) -> DispatchResult { if T::EnsurePrewitnessed::ensure_origin(origin.clone()).is_ok() { - Self::add_prewitnessed_deposits(deposit_witnesses, block_height)?; + for deposit_witness in deposit_witnesses { + // TODO: emit event on error? + let _ = Self::process_channel_deposit_prewitness(deposit_witness, block_height); + } } else { T::EnsureWitnessed::ensure_origin(origin)?; @@ -1786,47 +1789,41 @@ impl, I: 'static> Pallet { Err("Insufficient boost funds".into()) } - fn add_prewitnessed_deposits( - deposit_witnesses: Vec>, + fn process_channel_deposit_prewitness( + DepositWitness { deposit_address, asset, amount, deposit_details }: DepositWitness< + T::TargetChain, + >, block_height: TargetChainBlockNumber, ) -> DispatchResult { - for DepositWitness { deposit_address, asset, amount, deposit_details } in deposit_witnesses - { - let DepositChannelDetails { - deposit_channel, - action, - boost_fee, - boost_status, - owner, - .. - } = DepositChannelLookup::::get(&deposit_address) - .ok_or(Error::::InvalidDepositAddress)?; + let DepositChannelDetails { + deposit_channel, action, boost_fee, boost_status, owner, .. + } = DepositChannelLookup::::get(&deposit_address) + .ok_or(Error::::InvalidDepositAddress)?; - if let Some(new_boost_status) = Self::process_prewitness_deposit_inner( - amount, - asset, - deposit_details, - Some(deposit_address.clone()), - None, // source address is unknown - action, - &owner, - boost_fee, - boost_status, - Some(deposit_channel.channel_id), + if let Some(new_boost_status) = Self::process_prewitness_deposit_inner( + amount, + asset, + deposit_details, + Some(deposit_address.clone()), + None, // source address is unknown + action, + &owner, + boost_fee, + boost_status, + Some(deposit_channel.channel_id), + block_height, + DepositOrigin::deposit_channel( + deposit_address.clone(), + deposit_channel.channel_id, block_height, - DepositOrigin::deposit_channel( - deposit_address.clone(), - deposit_channel.channel_id, - block_height, - ), - ) { - // Update boost status - DepositChannelLookup::::mutate(&deposit_address, |details| { - if let Some(details) = details { - details.boost_status = new_boost_status; - } - }); - } + ), + ) { + // Update boost status + DepositChannelLookup::::mutate(&deposit_address, |details| { + if let Some(details) = details { + details.boost_status = new_boost_status; + } + }); } Ok(()) } 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 dc3749d9f9..4e1a65d544 100644 --- a/state-chain/pallets/cf-ingress-egress/src/tests/boost.rs +++ b/state-chain/pallets/cf-ingress-egress/src/tests/boost.rs @@ -63,13 +63,13 @@ fn request_deposit_address_eth(account_id: u64, max_boost_fee: BasisPoints) -> ( #[track_caller] fn prewitness_deposit(deposit_address: H160, asset: EthAsset, amount: AssetAmount) -> u64 { - assert_ok!(Pallet::::add_prewitnessed_deposits( - vec![DepositWitness:: { + assert_ok!(IngressEgress::process_channel_deposit_prewitness( + DepositWitness:: { deposit_address, asset, amount, deposit_details: Default::default() - }], + }, 0 ),); diff --git a/state-chain/pallets/cf-ingress-egress/src/tests/screening.rs b/state-chain/pallets/cf-ingress-egress/src/tests/screening.rs index 24051e584f..6e90588d2a 100644 --- a/state-chain/pallets/cf-ingress-egress/src/tests/screening.rs +++ b/state-chain/pallets/cf-ingress-egress/src/tests/screening.rs @@ -162,13 +162,13 @@ fn finalize_boosted_tx_if_tainted_after_prewitness() { let address: ::ChainAccount = helpers::setup_boost_swap().try_into().unwrap(); - let _ = IngressEgress::add_prewitnessed_deposits( - vec![DepositWitness { + let _ = IngressEgress::process_channel_deposit_prewitness( + DepositWitness { deposit_address: address.clone(), asset: btc::Asset::Btc, amount: DEFAULT_DEPOSIT_AMOUNT, deposit_details: deposit_details.clone(), - }], + }, 10, ); @@ -211,15 +211,15 @@ fn reject_tx_if_tainted_before_prewitness() { assert_ok!(IngressEgress::mark_transaction_as_tainted(OriginTrait::signed(BROKER), tx_id,)); - let _ = IngressEgress::add_prewitnessed_deposits( - vec![DepositWitness { + assert_ok!(IngressEgress::process_channel_deposit_prewitness( + DepositWitness { deposit_address: address.clone(), asset: btc::Asset::Btc, amount: DEFAULT_DEPOSIT_AMOUNT, deposit_details: deposit_details.clone(), - }], + }, 10, - ); + )); assert_ok!(IngressEgress::process_channel_deposit_full_witness( &DepositWitness { @@ -404,7 +404,7 @@ fn can_report_between_prewitness_and_witness_if_tx_was_not_boosted() { deposit_details, }; - assert_ok!(IngressEgress::add_prewitnessed_deposits(vec![deposit_witness.clone()], 10,)); + assert_ok!(IngressEgress::process_channel_deposit_prewitness(deposit_witness.clone(), 10,)); assert_ok!(IngressEgress::mark_transaction_as_tainted(OriginTrait::signed(BROKER), tx_id,)); assert_ok!(IngressEgress::process_channel_deposit_full_witness(&deposit_witness, 10)); From 716d599f870ca93144ef94d6b079b6283ecd1db8 Mon Sep 17 00:00:00 2001 From: Maxim Shishmarev Date: Tue, 3 Dec 2024 17:57:07 +1100 Subject: [PATCH 06/13] test: assembling_broker_fees --- .../pallets/cf-ingress-egress/src/tests.rs | 41 +++++++++++++++++-- 1 file changed, 38 insertions(+), 3 deletions(-) diff --git a/state-chain/pallets/cf-ingress-egress/src/tests.rs b/state-chain/pallets/cf-ingress-egress/src/tests.rs index d7f91cb028..3ad6b6ce49 100644 --- a/state-chain/pallets/cf-ingress-egress/src/tests.rs +++ b/state-chain/pallets/cf-ingress-egress/src/tests.rs @@ -20,8 +20,8 @@ use cf_chains::{ ExecutexSwapAndCall, SwapOrigin, TransactionInIdForAnyChain, TransferAssetParams, }; use cf_primitives::{ - AffiliateShortId, Affiliates, AssetAmount, BasisPoints, Beneficiary, ChannelId, DcaParameters, - ForeignChain, + AffiliateShortId, Affiliates, AssetAmount, BasisPoints, Beneficiaries, Beneficiary, ChannelId, + DcaParameters, ForeignChain, MAX_AFFILIATES, }; use cf_test_utilities::{assert_events_eq, assert_has_event, assert_has_matching_event}; use cf_traits::{ @@ -47,7 +47,7 @@ use frame_support::{ traits::{Hooks, OriginTrait}, weights::Weight, }; -use sp_core::{bounded_vec, H160}; +use sp_core::{bounded_vec, H160, U256}; use sp_runtime::{DispatchError, DispatchResult}; const ALICE_ETH_ADDRESS: EthereumAddress = H160([100u8; 20]); @@ -2207,3 +2207,38 @@ fn private_and_regular_channel_ids_do_not_overlap() { open_regular_channel_expecting_id(REGULAR_CHANNEL_ID_2); }); } + +#[test] +fn assembling_broker_fees() { + new_test_ext().execute_with(|| { + let broker_fee = Beneficiary { account: BROKER, bps: 0 }; + + const AFFILIATE_IDS: [u64; 5] = [10, 20, 30, 40, 50]; + const AFFILIATE_SHORT_IDS: [u8; 5] = [1, 2, 3, 4, 5]; + + assert_eq!(AFFILIATE_IDS.len(), MAX_AFFILIATES as usize); + + for (i, id) in AFFILIATE_IDS.into_iter().enumerate() { + let short_id = AFFILIATE_SHORT_IDS[i]; + MockAffiliateRegistry::register_affiliate(BROKER, id, short_id.into()); + } + + let affiliate_fees: Vec> = AFFILIATE_SHORT_IDS + .into_iter() + .map(|short_id| Beneficiary { account: short_id.into(), bps: short_id.into() }) + .collect(); + + let affiliate_fees: Affiliates = affiliate_fees.try_into().unwrap(); + + let expected: Beneficiaries = bounded_vec![ + Beneficiary { account: BROKER, bps: 0 }, + Beneficiary { account: 10, bps: 1 }, + Beneficiary { account: 20, bps: 2 }, + Beneficiary { account: 30, bps: 3 }, + Beneficiary { account: 40, bps: 4 }, + Beneficiary { account: 50, bps: 5 }, + ]; + + assert_eq!(IngressEgress::assemble_broker_fees(broker_fee, affiliate_fees), expected); + }); +} From 6aa3c606cb154b2ad0f28df275f5faad37c7a448 Mon Sep 17 00:00:00 2001 From: Maxim Shishmarev Date: Tue, 3 Dec 2024 18:14:25 +1100 Subject: [PATCH 07/13] fix: Vec -> Box (addressing bouncer decoding issue) --- engine/src/witness/arb.rs | 6 ++---- engine/src/witness/btc/deposits.rs | 2 +- engine/src/witness/eth.rs | 2 +- engine/src/witness/evm/vault.rs | 1 - state-chain/cf-integration-tests/src/solana.rs | 6 +++--- state-chain/cf-integration-tests/src/swapping.rs | 6 +++--- state-chain/pallets/cf-ingress-egress/src/lib.rs | 11 ++++------- state-chain/pallets/cf-ingress-egress/src/tests.rs | 6 +++--- 8 files changed, 17 insertions(+), 23 deletions(-) diff --git a/engine/src/witness/arb.rs b/engine/src/witness/arb.rs index fa9f4ee038..da98b9f300 100644 --- a/engine/src/witness/arb.rs +++ b/engine/src/witness/arb.rs @@ -7,9 +7,7 @@ use cf_chains::{ evm::{DepositDetails, H256}, Arbitrum, CcmDepositMetadata, }; -use cf_primitives::{ - chains::assets::arb::Asset as ArbAsset, Asset, AssetAmount, Beneficiary, EpochIndex, -}; +use cf_primitives::{chains::assets::arb::Asset as ArbAsset, Asset, AssetAmount, EpochIndex}; use cf_utilities::task_scope::Scope; use futures_core::Future; use itertools::Itertools; @@ -208,7 +206,7 @@ impl super::evm::vault::IngressCallBuilder for ArbCallBuilder { state_chain_runtime::RuntimeCall::ArbitrumIngressEgress( pallet_cf_ingress_egress::Call::vault_swap_request { block_height, - deposits: vec![deposit], + deposit: Box::new(deposit), }, ) } diff --git a/engine/src/witness/btc/deposits.rs b/engine/src/witness/btc/deposits.rs index d5394a3d88..a49f8038e6 100644 --- a/engine/src/witness/btc/deposits.rs +++ b/engine/src/witness/btc/deposits.rs @@ -96,7 +96,7 @@ impl ChunkedByVaultBuilder { process_call( BtcIngressEgressCall::vault_swap_request { block_height: header.index, - deposits: vec![deposit], + deposit: Box::new(deposit), } .into(), epoch.index, diff --git a/engine/src/witness/eth.rs b/engine/src/witness/eth.rs index 60ca0457b5..3a4d4fb428 100644 --- a/engine/src/witness/eth.rs +++ b/engine/src/witness/eth.rs @@ -257,7 +257,7 @@ impl super::evm::vault::IngressCallBuilder for EthCallBuilder { state_chain_runtime::RuntimeCall::EthereumIngressEgress( pallet_cf_ingress_egress::Call::vault_swap_request { block_height, - deposits: vec![deposit], + deposit: Box::new(deposit), }, ) } diff --git a/engine/src/witness/evm/vault.rs b/engine/src/witness/evm/vault.rs index b3910540b4..016c742ccf 100644 --- a/engine/src/witness/evm/vault.rs +++ b/engine/src/witness/evm/vault.rs @@ -1,7 +1,6 @@ use crate::evm::retry_rpc::EvmRetryRpcApi; use codec::Decode; use ethers::types::Bloom; -use pallet_cf_ingress_egress::VaultDepositWitness; use sp_core::H256; use std::collections::HashMap; diff --git a/state-chain/cf-integration-tests/src/solana.rs b/state-chain/cf-integration-tests/src/solana.rs index 9a8e2d0892..0b930a4b55 100644 --- a/state-chain/cf-integration-tests/src/solana.rs +++ b/state-chain/cf-integration-tests/src/solana.rs @@ -532,7 +532,7 @@ fn solana_ccm_fails_with_invalid_input() { assert_ok!(RuntimeCall::SolanaIngressEgress( pallet_cf_ingress_egress::Call::vault_swap_request { block_height: 0, - deposits: vec![vault_swap_deposit_witness(Some(invalid_ccm))], + deposit: Box::new(vault_swap_deposit_witness(Some(invalid_ccm))), } ) .dispatch_bypass_filter( @@ -577,7 +577,7 @@ fn solana_ccm_fails_with_invalid_input() { witness_call(RuntimeCall::SolanaIngressEgress( pallet_cf_ingress_egress::Call::vault_swap_request { block_height: 0, - deposits: vec![vault_swap_deposit_witness(Some(ccm))], + deposit: Box::new(vault_swap_deposit_witness(Some(ccm))), }, )); // Setting the current agg key will invalidate the CCM. @@ -792,7 +792,7 @@ fn solana_ccm_execution_error_can_trigger_fallback() { witness_call(RuntimeCall::SolanaIngressEgress( pallet_cf_ingress_egress::Call::vault_swap_request { block_height: 0, - deposits: vec![vault_swap_deposit_witness(Some(ccm))], + deposit: Box::new(vault_swap_deposit_witness(Some(ccm))), } )); diff --git a/state-chain/cf-integration-tests/src/swapping.rs b/state-chain/cf-integration-tests/src/swapping.rs index fda81939eb..53d4269a15 100644 --- a/state-chain/cf-integration-tests/src/swapping.rs +++ b/state-chain/cf-integration-tests/src/swapping.rs @@ -604,7 +604,7 @@ fn can_process_ccm_via_direct_deposit() { witness_call(RuntimeCall::EthereumIngressEgress( pallet_cf_ingress_egress::Call::vault_swap_request { block_height: 0, - deposits: vec![vault_swap_deposit_witness(deposit_amount, Asset::Usdc)], + deposit: Box::new(vault_swap_deposit_witness(deposit_amount, Asset::Usdc)), }, )); @@ -645,7 +645,7 @@ fn failed_swaps_are_rolled_back() { witness_call(RuntimeCall::EthereumIngressEgress( pallet_cf_ingress_egress::Call::vault_swap_request { block_height: 0, - deposits: vec![vault_swap_deposit_witness(10_000 * DECIMALS, Asset::Flip)], + deposit: Box::new(vault_swap_deposit_witness(10_000 * DECIMALS, Asset::Flip)), }, )); @@ -798,7 +798,7 @@ fn can_resign_failed_ccm() { witness_call(RuntimeCall::EthereumIngressEgress( pallet_cf_ingress_egress::Call::vault_swap_request { block_height: 0, - deposits: vec![vault_swap_deposit_witness(10_000_000_000_000, Asset::Usdc)], + deposit: Box::new(vault_swap_deposit_witness(10_000_000_000_000, Asset::Usdc)), }, )); diff --git a/state-chain/pallets/cf-ingress-egress/src/lib.rs b/state-chain/pallets/cf-ingress-egress/src/lib.rs index 47b0d3c4ec..3c9c668cf6 100644 --- a/state-chain/pallets/cf-ingress-egress/src/lib.rs +++ b/state-chain/pallets/cf-ingress-egress/src/lib.rs @@ -61,6 +61,7 @@ use scale_info::{ }; use sp_runtime::traits::UniqueSaturatedInto; use sp_std::{ + boxed::Box, collections::{btree_map::BTreeMap, btree_set::BTreeSet}, marker::PhantomData, vec, @@ -1418,18 +1419,14 @@ pub mod pallet { pub fn vault_swap_request( origin: OriginFor, block_height: TargetChainBlockNumber, - deposits: Vec>, + deposit: Box>, ) -> DispatchResult { if T::EnsureWitnessed::ensure_origin(origin.clone()).is_ok() { - for deposit in deposits { - Self::process_vault_swap_request_full_witness(block_height, deposit); - } + Self::process_vault_swap_request_full_witness(block_height, *deposit); } else { T::EnsurePrewitnessed::ensure_origin(origin)?; - for deposit in deposits { - Self::process_vault_swap_request_prewitness(block_height, deposit); - } + Self::process_vault_swap_request_prewitness(block_height, *deposit); } Ok(()) diff --git a/state-chain/pallets/cf-ingress-egress/src/tests.rs b/state-chain/pallets/cf-ingress-egress/src/tests.rs index 3ad6b6ce49..34023e53f0 100644 --- a/state-chain/pallets/cf-ingress-egress/src/tests.rs +++ b/state-chain/pallets/cf-ingress-egress/src/tests.rs @@ -47,7 +47,7 @@ use frame_support::{ traits::{Hooks, OriginTrait}, weights::Weight, }; -use sp_core::{bounded_vec, H160, U256}; +use sp_core::{bounded_vec, H160}; use sp_runtime::{DispatchError, DispatchResult}; const ALICE_ETH_ADDRESS: EthereumAddress = H160([100u8; 20]); @@ -1817,7 +1817,7 @@ fn submit_vault_swap_request( IngressEgress::vault_swap_request( RuntimeOrigin::root(), 0, - vec![VaultDepositWitness { + Box::new(VaultDepositWitness { input_asset: input_asset.try_into().unwrap(), deposit_address: Some(deposit_address), channel_id: Some(0), @@ -1832,7 +1832,7 @@ fn submit_vault_swap_request( refund_params, dca_params, boost_fee, - }], + }), ) } From 5b056ed21a684addcb190ad8a4bbd95445c48802 Mon Sep 17 00:00:00 2001 From: Maxim Shishmarev Date: Wed, 4 Dec 2024 09:30:24 +1100 Subject: [PATCH 08/13] feat: infallible credit_account --- .../cf-integration-tests/src/swapping.rs | 2 +- .../pallets/cf-asset-balances/src/lib.rs | 30 ++++-- .../pallets/cf-asset-balances/src/tests.rs | 6 +- .../cf-ingress-egress/src/benchmarking.rs | 8 +- .../pallets/cf-ingress-egress/src/lib.rs | 93 ++++++++----------- .../cf-ingress-egress/src/tests/boost.rs | 8 +- .../cf-ingress-egress/src/tests/screening.rs | 3 +- state-chain/pallets/cf-lp/src/benchmarking.rs | 2 +- state-chain/pallets/cf-lp/src/lib.rs | 2 +- state-chain/pallets/cf-lp/src/mock.rs | 16 ++-- state-chain/pallets/cf-lp/src/tests.rs | 2 +- .../pallets/cf-pools/src/benchmarking.rs | 28 +++--- state-chain/pallets/cf-pools/src/lib.rs | 18 ++-- state-chain/pallets/cf-pools/src/mock.rs | 14 ++- .../pallets/cf-swapping/src/benchmarking.rs | 2 +- state-chain/pallets/cf-swapping/src/lib.rs | 10 +- state-chain/pallets/cf-swapping/src/tests.rs | 6 +- .../pallets/cf-swapping/src/tests/config.rs | 2 +- .../pallets/cf-swapping/src/tests/fees.rs | 3 +- state-chain/traits/src/lib.rs | 2 + state-chain/traits/src/mocks/balance_api.rs | 14 ++- 21 files changed, 131 insertions(+), 140 deletions(-) diff --git a/state-chain/cf-integration-tests/src/swapping.rs b/state-chain/cf-integration-tests/src/swapping.rs index 53d4269a15..14e09ce3c0 100644 --- a/state-chain/cf-integration-tests/src/swapping.rs +++ b/state-chain/cf-integration-tests/src/swapping.rs @@ -81,7 +81,7 @@ fn new_pool(unstable_asset: Asset, fee_hundredth_pips: u32, initial_price: Price fn credit_account(account_id: &AccountId, asset: Asset, amount: AssetAmount) { let original_amount = pallet_cf_asset_balances::FreeBalances::::get(account_id, asset); - assert_ok!(AssetBalances::try_credit_account(account_id, asset, amount)); + AssetBalances::credit_account(account_id, asset, amount); assert_eq!( pallet_cf_asset_balances::FreeBalances::::get(account_id, asset), original_amount + amount diff --git a/state-chain/pallets/cf-asset-balances/src/lib.rs b/state-chain/pallets/cf-asset-balances/src/lib.rs index 9c1bc4b052..8436d0aa24 100644 --- a/state-chain/pallets/cf-asset-balances/src/lib.rs +++ b/state-chain/pallets/cf-asset-balances/src/lib.rs @@ -370,19 +370,15 @@ where { type AccountId = T::AccountId; - fn try_credit_account( - account_id: &Self::AccountId, - asset: Asset, - amount: AssetAmount, - ) -> DispatchResult { + fn credit_account(account_id: &Self::AccountId, asset: Asset, amount: AssetAmount) { if amount == 0 { - return Ok(()) + return; } - let new_balance = FreeBalances::::try_mutate(account_id, asset, |balance| { - *balance = balance.checked_add(amount).ok_or(Error::::BalanceOverflow)?; - Ok::<_, Error>(*balance) - })?; + let new_balance = FreeBalances::::mutate(account_id, asset, |balance| { + *balance = balance.saturating_add(amount); + *balance + }); Self::deposit_event(Event::AccountCredited { account_id: account_id.clone(), @@ -390,6 +386,20 @@ where amount_credited: amount, new_balance, }); + } + + fn try_credit_account( + account_id: &Self::AccountId, + asset: Asset, + amount: AssetAmount, + ) -> DispatchResult { + // Check if the result would overflow: + FreeBalances::::get(account_id, asset) + .checked_add(amount) + .ok_or(Error::::BalanceOverflow)?; + + Self::credit_account(account_id, asset, amount); + Ok(()) } diff --git a/state-chain/pallets/cf-asset-balances/src/tests.rs b/state-chain/pallets/cf-asset-balances/src/tests.rs index 42e72f9302..ce1521b187 100644 --- a/state-chain/pallets/cf-asset-balances/src/tests.rs +++ b/state-chain/pallets/cf-asset-balances/src/tests.rs @@ -288,11 +288,7 @@ pub mod balance_api { let alice = AccountId::from([1; 32]); const AMOUNT: u128 = 100; const DELTA: u128 = 10; - assert_ok!(Pallet::::try_credit_account( - &alice, - ForeignChain::Ethereum.gas_asset(), - AMOUNT - )); + Pallet::::credit_account(&alice, ForeignChain::Ethereum.gas_asset(), AMOUNT); assert_has_event::( crate::Event::AccountCredited { account_id: alice.clone(), diff --git a/state-chain/pallets/cf-ingress-egress/src/benchmarking.rs b/state-chain/pallets/cf-ingress-egress/src/benchmarking.rs index dc74db8f67..10418f1aa2 100644 --- a/state-chain/pallets/cf-ingress-egress/src/benchmarking.rs +++ b/state-chain/pallets/cf-ingress-egress/src/benchmarking.rs @@ -183,16 +183,12 @@ mod benchmarks { } ::OnNewAccount::on_new_account(&caller); assert_ok!(::AccountRoleRegistry::register_as_liquidity_provider(&caller)); - assert_ok!(T::Balance::try_credit_account(&caller, asset.into(), 1_000_000,)); + T::Balance::credit_account(&caller, asset.into(), 1_000_000); // A non-zero balance is required to pay for the channel opening fee. T::FeePayment::mint_to_account(&caller, u32::MAX.into()); - assert_ok!(T::Balance::try_credit_account( - &caller, - asset.into(), - 5_000_000_000_000_000_000u128 - )); + T::Balance::credit_account(&caller, asset.into(), 5_000_000_000_000_000_000u128); caller } diff --git a/state-chain/pallets/cf-ingress-egress/src/lib.rs b/state-chain/pallets/cf-ingress-egress/src/lib.rs index 3c9c668cf6..bfa5e84f6c 100644 --- a/state-chain/pallets/cf-ingress-egress/src/lib.rs +++ b/state-chain/pallets/cf-ingress-egress/src/lib.rs @@ -1336,7 +1336,7 @@ pub mod pallet { pool.stop_boosting(booster.clone()) })?; - T::Balance::try_credit_account(&booster, asset.into(), unlocked_amount.into())?; + T::Balance::credit_account(&booster, asset.into(), unlocked_amount.into()); Self::deposit_event(Event::StoppedBoosting { booster_id: booster, @@ -1831,14 +1831,10 @@ impl, I: 'static> Pallet { source_address: Option, amount_after_fees: TargetChainAmount, origin: DepositOrigin, - ) -> Result, DispatchError> { - let action = match action { + ) -> DepositAction { + match action { ChannelAction::LiquidityProvision { lp_account, .. } => { - T::Balance::try_credit_account( - &lp_account, - asset.into(), - amount_after_fees.into(), - )?; + T::Balance::credit_account(&lp_account, asset.into(), amount_after_fees.into()); DepositAction::LiquidityProvision { lp_account } }, ChannelAction::Swap { @@ -1909,9 +1905,7 @@ impl, I: 'static> Pallet { }, } }, - }; - - Ok(action) + } } /// Completes a single deposit request. @@ -2071,29 +2065,27 @@ impl, I: 'static> Pallet { let used_pool_tiers = used_pools.keys().cloned().collect(); - if let Ok(action) = Self::perform_channel_action( + let action = Self::perform_channel_action( action, asset, source_address, amount_after_fees, origin.clone(), - ) { - Self::deposit_event(Event::DepositBoosted { - deposit_address, - asset, - amounts: used_pools, - block_height, - prewitnessed_deposit_id, - channel_id, - deposit_details: deposit_details.clone(), - ingress_fee, - boost_fee: boost_fee_amount, - action, - origin_type: origin.into(), - }); - } else { - // TODO: emit error? - } + ); + + Self::deposit_event(Event::DepositBoosted { + deposit_address, + asset, + amounts: used_pools, + block_height, + prewitnessed_deposit_id, + channel_id, + deposit_details: deposit_details.clone(), + ingress_fee, + boost_fee: boost_fee_amount, + action, + origin_type: origin.into(), + }); return Some(BoostStatus::Boosted { prewitnessed_deposit_id, @@ -2309,16 +2301,11 @@ impl, I: 'static> Pallet { for (booster_id, finalised_withdrawn_amount) in pool.process_deposit_as_finalised(prewitnessed_deposit_id) { - if let Err(err) = T::Balance::try_credit_account( + T::Balance::credit_account( &booster_id, asset.into(), finalised_withdrawn_amount.into(), - ) { - log_or_panic!( - "Failed to credit booster account {:?} after unlock of {finalised_withdrawn_amount:?} {asset:?}: {:?}", - booster_id, err - ); - } + ); } } }); @@ -2353,29 +2340,27 @@ impl, I: 'static> Pallet { Err(()) } else { // Processing as a non-boosted deposit: - - if let Ok(action) = Self::perform_channel_action( + let action = Self::perform_channel_action( action, asset, source_address, amount_after_fees, origin.clone(), - ) { - Self::deposit_event(Event::DepositFinalised { - deposit_address, - asset, - amount: deposit_amount, - block_height, - deposit_details, - ingress_fee: fees_withheld, - action, - channel_id, - origin_type: origin.into(), - }); - Ok(FullWitnessDepositOutcome::DepositActionPerformed) - } else { - Err(()) - } + ); + + Self::deposit_event(Event::DepositFinalised { + deposit_address, + asset, + amount: deposit_amount, + block_height, + deposit_details, + ingress_fee: fees_withheld, + action, + channel_id, + origin_type: origin.into(), + }); + + Ok(FullWitnessDepositOutcome::DepositActionPerformed) } } } 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 4e1a65d544..ee42e8af3f 100644 --- a/state-chain/pallets/cf-ingress-egress/src/tests/boost.rs +++ b/state-chain/pallets/cf-ingress-egress/src/tests/boost.rs @@ -121,17 +121,17 @@ fn setup() { ); for asset in EthAsset::all() { - assert_ok!(::Balance::try_credit_account( + ::Balance::credit_account( &BOOSTER_1, asset.into(), INIT_BOOSTER_ETH_BALANCE, - )); + ); - assert_ok!(::Balance::try_credit_account( + ::Balance::credit_account( &BOOSTER_2, asset.into(), INIT_BOOSTER_ETH_BALANCE, - )); + ); } assert_eq!(get_lp_eth_balance(&BOOSTER_1), INIT_BOOSTER_ETH_BALANCE); diff --git a/state-chain/pallets/cf-ingress-egress/src/tests/screening.rs b/state-chain/pallets/cf-ingress-egress/src/tests/screening.rs index 6e90588d2a..bbc85d2c0c 100644 --- a/state-chain/pallets/cf-ingress-egress/src/tests/screening.rs +++ b/state-chain/pallets/cf-ingress-egress/src/tests/screening.rs @@ -79,8 +79,7 @@ mod helpers { vec![BoostPoolId { asset: btc::Asset::Btc, tier: 10 }], )); - ::Balance::try_credit_account(&ALICE, btc::Asset::Btc.into(), 1000) - .unwrap(); + ::Balance::credit_account(&ALICE, btc::Asset::Btc.into(), 1000); let (_, address, _, _) = IngressEgress::request_swap_deposit_address( btc::Asset::Btc, diff --git a/state-chain/pallets/cf-lp/src/benchmarking.rs b/state-chain/pallets/cf-lp/src/benchmarking.rs index c5c3de9211..93eeebb8cd 100644 --- a/state-chain/pallets/cf-lp/src/benchmarking.rs +++ b/state-chain/pallets/cf-lp/src/benchmarking.rs @@ -38,7 +38,7 @@ mod benchmarks { AccountRole::LiquidityProvider, ) .unwrap(); - assert_ok!(T::BalanceApi::try_credit_account(&caller, Asset::Eth, 1_000_000,)); + T::BalanceApi::credit_account(&caller, Asset::Eth, 1_000_000); #[extrinsic_call] withdraw_asset( diff --git a/state-chain/pallets/cf-lp/src/lib.rs b/state-chain/pallets/cf-lp/src/lib.rs index a62d9f2a1e..83bfe2c24c 100644 --- a/state-chain/pallets/cf-lp/src/lib.rs +++ b/state-chain/pallets/cf-lp/src/lib.rs @@ -364,7 +364,7 @@ impl Pallet { T::BalanceApi::try_debit_account(&account_id, asset, amount)?; // Credit the asset to the destination account. - T::BalanceApi::try_credit_account(&destination_account, asset, amount)?; + T::BalanceApi::credit_account(&destination_account, asset, amount); Self::deposit_event(Event::AssetTransferred { from: account_id, diff --git a/state-chain/pallets/cf-lp/src/mock.rs b/state-chain/pallets/cf-lp/src/mock.rs index 4c4faa0747..2223cc68b2 100644 --- a/state-chain/pallets/cf-lp/src/mock.rs +++ b/state-chain/pallets/cf-lp/src/mock.rs @@ -78,16 +78,20 @@ pub struct MockBalanceApi; impl BalanceApi for MockBalanceApi { type AccountId = AccountId; + fn credit_account(who: &Self::AccountId, _asset: Asset, amount: AssetAmount) { + BALANCE_MAP.with(|balance_map| { + let mut balance_map = balance_map.borrow_mut(); + *balance_map.entry(who.to_owned()).or_default() += amount; + }); + } + fn try_credit_account( who: &Self::AccountId, - _asset: cf_primitives::Asset, + asset: cf_primitives::Asset, amount: cf_primitives::AssetAmount, ) -> frame_support::dispatch::DispatchResult { - BALANCE_MAP.with(|balance_map| { - let mut balance_map = balance_map.borrow_mut(); - *balance_map.entry(who.to_owned()).or_default() += amount; - Ok(()) - }) + Self::credit_account(who, asset, amount); + Ok(()) } fn try_debit_account( diff --git a/state-chain/pallets/cf-lp/src/tests.rs b/state-chain/pallets/cf-lp/src/tests.rs index b2cd5d4ba7..965e944c2b 100644 --- a/state-chain/pallets/cf-lp/src/tests.rs +++ b/state-chain/pallets/cf-lp/src/tests.rs @@ -320,7 +320,7 @@ fn account_registration_and_deregistration() { EncodedAddress::Eth([0x01; 20]) )); - MockBalanceApi::try_credit_account(&LP_ACCOUNT_ID, Asset::Eth, DEPOSIT_AMOUNT).unwrap(); + MockBalanceApi::credit_account(&LP_ACCOUNT_ID, Asset::Eth, DEPOSIT_AMOUNT); assert_noop!( LiquidityProvider::deregister_lp_account(OriginTrait::signed(LP_ACCOUNT_ID)), diff --git a/state-chain/pallets/cf-pools/src/benchmarking.rs b/state-chain/pallets/cf-pools/src/benchmarking.rs index 9f65be6035..aa8e3b3e32 100644 --- a/state-chain/pallets/cf-pools/src/benchmarking.rs +++ b/state-chain/pallets/cf-pools/src/benchmarking.rs @@ -61,8 +61,8 @@ mod benchmarks { 0, price_at_tick(0).unwrap() )); - assert_ok!(T::LpBalance::try_credit_account(&caller, Asset::Eth, 1_000_000,)); - assert_ok!(T::LpBalance::try_credit_account(&caller, Asset::Usdc, 1_000_000,)); + assert_ok!(T::LpBalance::credit_account(&caller, Asset::Eth, 1_000_000)); + assert_ok!(T::LpBalance::credit_account(&caller, Asset::Usdc, 1_000_000)); #[extrinsic_call] update_range_order( @@ -88,8 +88,8 @@ mod benchmarks { 0, price_at_tick(0).unwrap() )); - assert_ok!(T::LpBalance::try_credit_account(&caller, Asset::Eth, 1_000_000,)); - assert_ok!(T::LpBalance::try_credit_account(&caller, Asset::Usdc, 1_000_000,)); + T::LpBalance::credit_account(&caller, Asset::Eth, 1_000_000); + T::LpBalance::credit_account(&caller, Asset::Usdc, 1_000_000); #[extrinsic_call] set_range_order( @@ -115,8 +115,8 @@ mod benchmarks { 0, price_at_tick(0).unwrap() )); - assert_ok!(T::LpBalance::try_credit_account(&caller, Asset::Eth, 1_000_000,)); - assert_ok!(T::LpBalance::try_credit_account(&caller, Asset::Usdc, 1_000_000,)); + T::LpBalance::credit_account(&caller, Asset::Eth, 1_000_000); + T::LpBalance::credit_account(&caller, Asset::Usdc, 1_000_000); #[extrinsic_call] update_limit_order( @@ -140,8 +140,8 @@ mod benchmarks { 0, price_at_tick(0).unwrap() )); - assert_ok!(T::LpBalance::try_credit_account(&caller, Asset::Eth, 1_000_000,)); - assert_ok!(T::LpBalance::try_credit_account(&caller, Asset::Usdc, 1_000_000,)); + T::LpBalance::credit_account(&caller, Asset::Eth, 1_000_000); + T::LpBalance::credit_account(&caller, Asset::Usdc, 1_000_000); #[extrinsic_call] set_limit_order( @@ -165,8 +165,8 @@ mod benchmarks { 0, price_at_tick(0).unwrap() )); - assert_ok!(T::LpBalance::try_credit_account(&caller, Asset::Eth, 1_000_000,)); - assert_ok!(T::LpBalance::try_credit_account(&caller, Asset::Usdc, 1_000_000,)); + T::LpBalance::credit_account(&caller, Asset::Eth, 1_000_000); + T::LpBalance::credit_account(&caller, Asset::Usdc, 1_000_000); assert_ok!(Pallet::::set_limit_order( RawOrigin::Signed(caller.clone()).into(), Asset::Eth, @@ -219,8 +219,8 @@ mod benchmarks { 0, price_at_tick(0).unwrap() )); - assert_ok!(T::LpBalance::try_credit_account(&caller, Asset::Eth, 1_000_000,)); - assert_ok!(T::LpBalance::try_credit_account(&caller, Asset::Usdc, 1_000_000,)); + T::LpBalance::credit_account(&caller, Asset::Eth, 1_000_000); + T::LpBalance::credit_account(&caller, Asset::Usdc, 1_000_000); #[extrinsic_call] schedule_limit_order_update( RawOrigin::Signed(caller.clone()), @@ -278,8 +278,8 @@ mod benchmarks { 0, price_at_tick(0).unwrap() )); - assert_ok!(T::LpBalance::try_credit_account(&caller, Asset::Eth, 1_000_000_000,)); - assert_ok!(T::LpBalance::try_credit_account(&caller, Asset::Usdc, 1_000_000_000,)); + T::LpBalance::credit_account(&caller, Asset::Eth, 1_000_000_000); + T::LpBalance::credit_account(&caller, Asset::Usdc, 1_000_000_000); let mut orders_to_delete: BoundedVec> = vec![].try_into().unwrap(); for i in 1..101i32 { diff --git a/state-chain/pallets/cf-pools/src/lib.rs b/state-chain/pallets/cf-pools/src/lib.rs index 37c6724672..d826b6cf1d 100644 --- a/state-chain/pallets/cf-pools/src/lib.rs +++ b/state-chain/pallets/cf-pools/src/lib.rs @@ -1472,11 +1472,11 @@ impl Pallet { }?; let withdrawn_amount: AssetAmount = sold_amount.try_into()?; - T::LpBalance::try_credit_account( + T::LpBalance::credit_account( lp, asset_pair.assets()[side.to_sold_pair()], withdrawn_amount, - )?; + ); (IncreaseOrDecrease::Decrease(withdrawn_amount), position_info, collected) }, @@ -1584,12 +1584,14 @@ impl Pallet { let assets_withdrawn = asset_pair.assets().zip(assets_withdrawn).try_map( |(asset, amount_withdrawn)| { - AssetAmount::try_from(amount_withdrawn).map_err(Into::into).and_then( - |amount_withdrawn| { - T::LpBalance::try_credit_account(lp, asset, amount_withdrawn) - .map(|()| amount_withdrawn) - }, - ) + AssetAmount::try_from(amount_withdrawn) + .map_err(Into::::into) + // Use map? + .and_then(|amount_withdrawn| { + T::LpBalance::credit_account(lp, asset, amount_withdrawn); + + Ok(amount_withdrawn) + }) }, )?; diff --git a/state-chain/pallets/cf-pools/src/mock.rs b/state-chain/pallets/cf-pools/src/mock.rs index b5f4b810be..40c38f96da 100644 --- a/state-chain/pallets/cf-pools/src/mock.rs +++ b/state-chain/pallets/cf-pools/src/mock.rs @@ -50,11 +50,7 @@ pub struct MockBalance; impl BalanceApi for MockBalance { type AccountId = AccountId; - fn try_credit_account( - who: &Self::AccountId, - asset: cf_primitives::Asset, - amount: cf_primitives::AssetAmount, - ) -> DispatchResult { + fn credit_account(who: &Self::AccountId, asset: Asset, amount: AssetAmount) { match (*who, asset) { (ALICE, Asset::Eth) => AliceCollectedEth::set(AliceCollectedEth::get() + amount), (ALICE, Asset::Usdc) => AliceCollectedUsdc::set(AliceCollectedUsdc::get() + amount), @@ -62,6 +58,14 @@ impl BalanceApi for MockBalance { (BOB, Asset::Usdc) => BobCollectedUsdc::set(BobCollectedUsdc::get() + amount), _ => (), } + } + + fn try_credit_account( + who: &Self::AccountId, + asset: cf_primitives::Asset, + amount: cf_primitives::AssetAmount, + ) -> DispatchResult { + Self::credit_account(who, asset, amount); Ok(()) } diff --git a/state-chain/pallets/cf-swapping/src/benchmarking.rs b/state-chain/pallets/cf-swapping/src/benchmarking.rs index e90a23c6dd..fcbb67ee6e 100644 --- a/state-chain/pallets/cf-swapping/src/benchmarking.rs +++ b/state-chain/pallets/cf-swapping/src/benchmarking.rs @@ -94,7 +94,7 @@ mod benchmarks { ) .unwrap(); - T::BalanceApi::try_credit_account(&caller, Asset::Eth, 200).unwrap(); + T::BalanceApi::credit_account(&caller, Asset::Eth, 200); #[extrinsic_call] withdraw(RawOrigin::Signed(caller.clone()), Asset::Eth, EncodedAddress::benchmark_value()); diff --git a/state-chain/pallets/cf-swapping/src/lib.rs b/state-chain/pallets/cf-swapping/src/lib.rs index 86d59a064e..595f1f5de1 100644 --- a/state-chain/pallets/cf-swapping/src/lib.rs +++ b/state-chain/pallets/cf-swapping/src/lib.rs @@ -1308,15 +1308,7 @@ pub mod pallet { let fee = Permill::from_parts(*bps as u32 * BASIS_POINTS_PER_MILLION) * stable_amount; - if let Err(err) = - T::BalanceApi::try_credit_account(account, STABLE_ASSET, fee) - { - log_or_panic!( - "Failed to credit broker fee to account {:?} with error: {:?}", - account, - err - ); - } + T::BalanceApi::credit_account(account, STABLE_ASSET, fee); fee_accumulator.saturating_add(fee) }, diff --git a/state-chain/pallets/cf-swapping/src/tests.rs b/state-chain/pallets/cf-swapping/src/tests.rs index 596d378f62..821b06408e 100644 --- a/state-chain/pallets/cf-swapping/src/tests.rs +++ b/state-chain/pallets/cf-swapping/src/tests.rs @@ -235,10 +235,6 @@ fn get_broker_balance(who: &T::AccountId, asset: Asset) -> AssetAmoun T::BalanceApi::get_balance(who, asset) } -fn credit_broker_account(who: &T::AccountId, asset: Asset, amount: AssetAmount) { - assert_ok!(T::BalanceApi::try_credit_account(who, asset, amount)); -} - #[track_caller] fn assert_swaps_queue_is_empty() { assert_eq!(SwapQueue::::iter_keys().count(), 0); @@ -1228,7 +1224,7 @@ fn broker_deregistration_checks_earned_fees() { .expect("BROKER was registered in test setup."); // Earn some fees. - credit_broker_account::(&BROKER, Asset::Eth, 100); + ::BalanceApi::credit_account(&BROKER, Asset::Eth, 100); assert_noop!( Swapping::deregister_as_broker(OriginTrait::signed(BROKER)), diff --git a/state-chain/pallets/cf-swapping/src/tests/config.rs b/state-chain/pallets/cf-swapping/src/tests/config.rs index 2856a5178d..5e75386711 100644 --- a/state-chain/pallets/cf-swapping/src/tests/config.rs +++ b/state-chain/pallets/cf-swapping/src/tests/config.rs @@ -274,7 +274,7 @@ fn cannot_swap_in_safe_mode() { #[test] fn cannot_withdraw_in_safe_mode() { new_test_ext().execute_with(|| { - credit_broker_account::(&BROKER, Asset::Eth, 200); + ::BalanceApi::credit_account(&BROKER, Asset::Eth, 200); // Activate code red >::set_code_red(); diff --git a/state-chain/pallets/cf-swapping/src/tests/fees.rs b/state-chain/pallets/cf-swapping/src/tests/fees.rs index 86f6286377..43eeeb31a5 100644 --- a/state-chain/pallets/cf-swapping/src/tests/fees.rs +++ b/state-chain/pallets/cf-swapping/src/tests/fees.rs @@ -374,7 +374,8 @@ fn withdraw_broker_fees() { ), >::NoFundsAvailable ); - credit_broker_account::(&BROKER, Asset::Eth, 200); + + ::BalanceApi::credit_account(&BROKER, Asset::Eth, 200); assert_ok!(Swapping::withdraw( RuntimeOrigin::signed(BROKER), Asset::Eth, diff --git a/state-chain/traits/src/lib.rs b/state-chain/traits/src/lib.rs index 29c3396c26..e86d29b251 100644 --- a/state-chain/traits/src/lib.rs +++ b/state-chain/traits/src/lib.rs @@ -1004,6 +1004,8 @@ pub trait SwapLimitsProvider { pub trait BalanceApi { type AccountId; + fn credit_account(who: &Self::AccountId, asset: Asset, amount: AssetAmount); + /// Attempt to credit the account with the given asset and amount. fn try_credit_account( who: &Self::AccountId, diff --git a/state-chain/traits/src/mocks/balance_api.rs b/state-chain/traits/src/mocks/balance_api.rs index 6cdbce2685..20ab8e5cc6 100644 --- a/state-chain/traits/src/mocks/balance_api.rs +++ b/state-chain/traits/src/mocks/balance_api.rs @@ -33,11 +33,7 @@ const FREE_BALANCES: &[u8] = b"FREE_BALANCES"; impl BalanceApi for MockBalance { type AccountId = u64; - fn try_credit_account( - who: &Self::AccountId, - asset: Asset, - amount_to_credit: AssetAmount, - ) -> DispatchResult { + fn credit_account(who: &Self::AccountId, asset: Asset, amount_to_credit: AssetAmount) { Self::mutate_storage::<(u64, cf_primitives::Asset), _, _, _, _>( FREE_BALANCES, &(*who, asset), @@ -46,6 +42,14 @@ impl BalanceApi for MockBalance { *amount = amount.saturating_add(amount_to_credit); }, ); + } + + fn try_credit_account( + who: &Self::AccountId, + asset: Asset, + amount_to_credit: AssetAmount, + ) -> DispatchResult { + Self::credit_account(who, asset, amount_to_credit); Ok(()) } From ac509660cfe7409fc862e2c5d0c7856313348dd0 Mon Sep 17 00:00:00 2001 From: Maxim Shishmarev Date: Wed, 4 Dec 2024 10:52:30 +1100 Subject: [PATCH 09/13] fix: clippy --- .../cf-ingress-egress/src/benchmarking.rs | 42 ++++++++++--------- .../pallets/cf-ingress-egress/src/lib.rs | 7 +--- .../pallets/cf-ingress-egress/src/tests.rs | 2 +- .../pallets/cf-pools/src/benchmarking.rs | 4 +- state-chain/pallets/cf-pools/src/lib.rs | 5 +-- 5 files changed, 29 insertions(+), 31 deletions(-) diff --git a/state-chain/pallets/cf-ingress-egress/src/benchmarking.rs b/state-chain/pallets/cf-ingress-egress/src/benchmarking.rs index 10418f1aa2..43e60cb55e 100644 --- a/state-chain/pallets/cf-ingress-egress/src/benchmarking.rs +++ b/state-chain/pallets/cf-ingress-egress/src/benchmarking.rs @@ -83,10 +83,12 @@ mod benchmarks { #[block] { assert_ok!(Pallet::::process_channel_deposit_full_witness( - deposit_address, - source_asset, - deposit_amount, - BenchmarkValue::benchmark_value(), + &DepositWitness { + deposit_address, + asset: source_asset, + amount: deposit_amount, + deposit_details: BenchmarkValue::benchmark_value(), + }, BenchmarkValue::benchmark_value() )); } @@ -236,15 +238,15 @@ mod benchmarks { ) .unwrap(); - assert_ok!(Pallet::::add_prewitnessed_deposits( - vec![DepositWitness:: { + assert_ok!(Pallet::::process_channel_deposit_prewitness( + DepositWitness:: { deposit_address: deposit_address.clone(), asset, amount: TargetChainAmount::::from(1000u32), deposit_details: BenchmarkValue::benchmark_value() - }], + }, BenchmarkValue::benchmark_value() - ),); + )); deposit_address } @@ -304,7 +306,7 @@ mod benchmarks { }; let call = Call::::vault_swap_request { block_height: 0u32.into(), - deposits: vec![VaultDepositWitness { + deposit: Box::new(VaultDepositWitness { input_asset: BenchmarkValue::benchmark_value(), output_asset: Asset::Eth, deposit_amount: 1_000u32.into(), @@ -321,9 +323,9 @@ mod benchmarks { }, dca_params: None, boost_fee: 0, - channel_id: 0, - deposit_address: BenchmarkValue::benchmark_value(), - }], + channel_id: None, + deposit_address: None, + }), }; #[block] @@ -410,13 +412,13 @@ mod benchmarks { #[block] { - assert_ok!(Pallet::::add_prewitnessed_deposits( - vec![DepositWitness:: { + assert_ok!(Pallet::::process_channel_deposit_prewitness( + DepositWitness:: { deposit_address, asset, amount: TargetChainAmount::::from(1000u32), deposit_details: BenchmarkValue::benchmark_value() - }], + }, BenchmarkValue::benchmark_value() ),); } @@ -475,10 +477,12 @@ mod benchmarks { #[block] { assert_ok!(Pallet::::process_channel_deposit_full_witness( - deposit_address, - asset, - 1_000u32.into(), - BenchmarkValue::benchmark_value(), + &DepositWitness { + deposit_address, + asset, + amount: 1_000u32.into(), + deposit_details: BenchmarkValue::benchmark_value(), + }, BenchmarkValue::benchmark_value() )); } diff --git a/state-chain/pallets/cf-ingress-egress/src/lib.rs b/state-chain/pallets/cf-ingress-egress/src/lib.rs index bfa5e84f6c..1033e01fc9 100644 --- a/state-chain/pallets/cf-ingress-egress/src/lib.rs +++ b/state-chain/pallets/cf-ingress-egress/src/lib.rs @@ -1916,7 +1916,7 @@ impl, I: 'static> Pallet { >, block_height: TargetChainBlockNumber, ) -> DispatchResult { - let deposit_channel_details = DepositChannelLookup::::get(&deposit_address) + let deposit_channel_details = DepositChannelLookup::::get(deposit_address) .ok_or(Error::::InvalidDepositAddress)?; ensure!( @@ -1950,7 +1950,7 @@ impl, I: 'static> Pallet { DepositOrigin::deposit_channel(deposit_address.clone(), channel_id, block_height), ) { // This allows the channel to be boosted again: - DepositChannelLookup::::mutate(&deposit_address, |details| { + DepositChannelLookup::::mutate(deposit_address, |details| { if let Some(details) = details { details.boost_status = BoostStatus::NotBoosted; } @@ -2255,9 +2255,6 @@ impl, I: 'static> Pallet { } } - // Vault deposits don't need to be fetched: - if !matches!(origin, DepositOrigin::Vault { .. }) {} - match &origin { DepositOrigin::DepositChannel { deposit_address, channel_id, .. } => { ScheduledEgressFetchOrTransfer::::append( diff --git a/state-chain/pallets/cf-ingress-egress/src/tests.rs b/state-chain/pallets/cf-ingress-egress/src/tests.rs index 34023e53f0..4c7b225f3a 100644 --- a/state-chain/pallets/cf-ingress-egress/src/tests.rs +++ b/state-chain/pallets/cf-ingress-egress/src/tests.rs @@ -245,7 +245,7 @@ fn request_address_and_deposit( let address: ::ChainAccount = address.try_into().unwrap(); assert_ok!(IngressEgress::process_channel_deposit_full_witness( &DepositWitness { - deposit_address: address.clone(), + deposit_address: address, asset, amount: DEFAULT_DEPOSIT_AMOUNT, deposit_details: Default::default() diff --git a/state-chain/pallets/cf-pools/src/benchmarking.rs b/state-chain/pallets/cf-pools/src/benchmarking.rs index aa8e3b3e32..42484764ed 100644 --- a/state-chain/pallets/cf-pools/src/benchmarking.rs +++ b/state-chain/pallets/cf-pools/src/benchmarking.rs @@ -61,8 +61,8 @@ mod benchmarks { 0, price_at_tick(0).unwrap() )); - assert_ok!(T::LpBalance::credit_account(&caller, Asset::Eth, 1_000_000)); - assert_ok!(T::LpBalance::credit_account(&caller, Asset::Usdc, 1_000_000)); + T::LpBalance::credit_account(&caller, Asset::Eth, 1_000_000); + T::LpBalance::credit_account(&caller, Asset::Usdc, 1_000_000); #[extrinsic_call] update_range_order( diff --git a/state-chain/pallets/cf-pools/src/lib.rs b/state-chain/pallets/cf-pools/src/lib.rs index d826b6cf1d..2edbf28961 100644 --- a/state-chain/pallets/cf-pools/src/lib.rs +++ b/state-chain/pallets/cf-pools/src/lib.rs @@ -1586,11 +1586,8 @@ impl Pallet { |(asset, amount_withdrawn)| { AssetAmount::try_from(amount_withdrawn) .map_err(Into::::into) - // Use map? - .and_then(|amount_withdrawn| { + .inspect(|&amount_withdrawn| { T::LpBalance::credit_account(lp, asset, amount_withdrawn); - - Ok(amount_withdrawn) }) }, )?; From 7ffa472fcba3529d4b6a2d92c328f108e06cdfc5 Mon Sep 17 00:00:00 2001 From: Maxim Shishmarev Date: Wed, 4 Dec 2024 11:19:28 +1100 Subject: [PATCH 10/13] feat: include origin type in InsufficientBoostLiquidity event --- state-chain/pallets/cf-ingress-egress/src/lib.rs | 2 ++ state-chain/pallets/cf-ingress-egress/src/tests/boost.rs | 1 + 2 files changed, 3 insertions(+) diff --git a/state-chain/pallets/cf-ingress-egress/src/lib.rs b/state-chain/pallets/cf-ingress-egress/src/lib.rs index 2c48281620..a550e764a0 100644 --- a/state-chain/pallets/cf-ingress-egress/src/lib.rs +++ b/state-chain/pallets/cf-ingress-egress/src/lib.rs @@ -826,6 +826,7 @@ pub mod pallet { asset: TargetChainAsset, amount_attempted: TargetChainAmount, channel_id: Option, + origin_type: DepositOriginType, }, BoostPoolCreated { boost_pool: BoostPoolId, @@ -2103,6 +2104,7 @@ impl, I: 'static> Pallet { asset, amount_attempted: amount, channel_id, + origin_type: origin.into(), }); }, } 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 ee42e8af3f..8a4d13de4b 100644 --- a/state-chain/pallets/cf-ingress-egress/src/tests/boost.rs +++ b/state-chain/pallets/cf-ingress-egress/src/tests/boost.rs @@ -673,6 +673,7 @@ fn insufficient_funds_for_boost() { asset: EthAsset::Eth, amount_attempted: DEPOSIT_AMOUNT, channel_id: Some(channel_id), + origin_type: DepositOriginType::DepositChannel, })); // When the deposit is finalised, it is processed as normal: From b8fa075dbecf351bfdb2bf50453c57d7bace139a Mon Sep 17 00:00:00 2001 From: Maxim Shishmarev Date: Wed, 4 Dec 2024 12:08:27 +1100 Subject: [PATCH 11/13] chore: fix tests with a process_channel_deposit_full_witness wrapper --- .../cf-ingress-egress/src/benchmarking.rs | 4 +- .../pallets/cf-ingress-egress/src/lib.rs | 37 ++++++++++--------- .../cf-ingress-egress/src/mocks/mock_eth.rs | 2 +- .../pallets/cf-ingress-egress/src/tests.rs | 34 ++++++++--------- .../cf-ingress-egress/src/tests/boost.rs | 2 +- .../cf-ingress-egress/src/tests/screening.rs | 10 ++--- 6 files changed, 46 insertions(+), 43 deletions(-) diff --git a/state-chain/pallets/cf-ingress-egress/src/benchmarking.rs b/state-chain/pallets/cf-ingress-egress/src/benchmarking.rs index 3866b26743..75817d98f6 100644 --- a/state-chain/pallets/cf-ingress-egress/src/benchmarking.rs +++ b/state-chain/pallets/cf-ingress-egress/src/benchmarking.rs @@ -82,7 +82,7 @@ mod benchmarks { #[block] { - assert_ok!(Pallet::::process_channel_deposit_full_witness( + assert_ok!(Pallet::::process_channel_deposit_full_witness_inner( &DepositWitness { deposit_address, asset: source_asset, @@ -476,7 +476,7 @@ mod benchmarks { #[block] { - assert_ok!(Pallet::::process_channel_deposit_full_witness( + assert_ok!(Pallet::::process_channel_deposit_full_witness_inner( &DepositWitness { deposit_address, asset, diff --git a/state-chain/pallets/cf-ingress-egress/src/lib.rs b/state-chain/pallets/cf-ingress-egress/src/lib.rs index a550e764a0..89c7cc7873 100644 --- a/state-chain/pallets/cf-ingress-egress/src/lib.rs +++ b/state-chain/pallets/cf-ingress-egress/src/lib.rs @@ -1169,13 +1169,7 @@ pub mod pallet { T::EnsureWitnessed::ensure_origin(origin)?; for deposit_witness in deposit_witnesses { - Self::process_channel_deposit_full_witness(&deposit_witness, block_height) - .unwrap_or_else(|e| { - Self::deposit_event(Event::::DepositWitnessRejected { - reason: e, - deposit_witness, - }); - }) + Self::process_channel_deposit_full_witness(deposit_witness, block_height); } } Ok(()) @@ -1455,15 +1449,9 @@ impl, I: 'static> IngressSink for Pallet { block_number: Self::BlockNumber, details: Self::DepositDetails, ) { - let deposit_witness = - DepositWitness { deposit_address: channel, asset, amount, deposit_details: details }; - Self::process_channel_deposit_full_witness(&deposit_witness, block_number).unwrap_or_else( - |e| { - Self::deposit_event(Event::::DepositWitnessRejected { - reason: e, - deposit_witness, - }); - }, + Self::process_channel_deposit_full_witness( + DepositWitness { deposit_address: channel, asset, amount, deposit_details: details }, + block_number, ); } @@ -1913,9 +1901,24 @@ impl, I: 'static> Pallet { } } + // A wrapper around `process_channel_deposit_full_witness_inner` that catches any + // error and emits a rejection event + fn process_channel_deposit_full_witness( + deposit_witness: DepositWitness, + block_height: TargetChainBlockNumber, + ) { + Self::process_channel_deposit_full_witness_inner(&deposit_witness, block_height) + .unwrap_or_else(|e| { + Self::deposit_event(Event::::DepositWitnessRejected { + reason: e, + deposit_witness, + }); + }) + } + /// Completes a single deposit request. #[transactional] - fn process_channel_deposit_full_witness( + fn process_channel_deposit_full_witness_inner( DepositWitness { deposit_address, asset, amount, deposit_details }: &DepositWitness< T::TargetChain, >, diff --git a/state-chain/pallets/cf-ingress-egress/src/mocks/mock_eth.rs b/state-chain/pallets/cf-ingress-egress/src/mocks/mock_eth.rs index 04d2f7d4fc..935dc03e84 100644 --- a/state-chain/pallets/cf-ingress-egress/src/mocks/mock_eth.rs +++ b/state-chain/pallets/cf-ingress-egress/src/mocks/mock_eth.rs @@ -194,7 +194,7 @@ impl RequestAddressAndDeposit for TestRunner { .zip(amounts) .map(|((request, channel_id, deposit_address), amount)| { if !amount.is_zero() { - IngressEgress::process_channel_deposit_full_witness( + IngressEgress::process_channel_deposit_full_witness_inner( &DepositWitness { deposit_address, asset: request.source_asset(), diff --git a/state-chain/pallets/cf-ingress-egress/src/tests.rs b/state-chain/pallets/cf-ingress-egress/src/tests.rs index 4c7b225f3a..e14eb3627e 100644 --- a/state-chain/pallets/cf-ingress-egress/src/tests.rs +++ b/state-chain/pallets/cf-ingress-egress/src/tests.rs @@ -243,7 +243,7 @@ fn request_address_and_deposit( ) .unwrap(); let address: ::ChainAccount = address.try_into().unwrap(); - assert_ok!(IngressEgress::process_channel_deposit_full_witness( + assert_ok!(IngressEgress::process_channel_deposit_full_witness_inner( &DepositWitness { deposit_address: address, asset, @@ -580,25 +580,25 @@ fn multi_deposit_includes_deposit_beyond_recycle_height() { .then_execute_at_next_block(|(address, address2)| { // block height is purely informative. let block_height = BlockHeightProvider::::get_block_height(); - assert_ok!(IngressEgress::process_channel_deposit_full_witness( - &DepositWitness { + IngressEgress::process_channel_deposit_full_witness( + DepositWitness { deposit_address: address, asset: ETH, amount: 1, deposit_details: Default::default(), }, block_height, - )); + ); - assert_ok!(IngressEgress::process_channel_deposit_full_witness( - &DepositWitness { + IngressEgress::process_channel_deposit_full_witness( + DepositWitness { deposit_address: address2, asset: ETH, amount: 1, deposit_details: Default::default(), }, block_height, - )); + ); (address, address2) }) .then_process_events(|_, event| match event { @@ -634,7 +634,7 @@ fn multi_use_deposit_address_different_blocks() { new_test_ext() .then_execute_at_next_block(|_| request_address_and_deposit(ALICE, ETH)) .then_execute_at_next_block(|(_, deposit_address)| { - assert_ok!(IngressEgress::process_channel_deposit_full_witness( + assert_ok!(IngressEgress::process_channel_deposit_full_witness_inner( &DepositWitness { deposit_address, asset: ETH, @@ -647,7 +647,7 @@ fn multi_use_deposit_address_different_blocks() { deposit_address }) .then_execute_at_next_block(|deposit_address| { - assert_ok!(IngressEgress::process_channel_deposit_full_witness( + assert_ok!(IngressEgress::process_channel_deposit_full_witness_inner( &DepositWitness { deposit_address, asset: ETH, @@ -667,7 +667,7 @@ fn multi_use_deposit_address_different_blocks() { }) // The channel should be closed at the next block. .then_execute_at_next_block(|deposit_address| { - assert_ok!(IngressEgress::process_channel_deposit_full_witness( + assert_ok!(IngressEgress::process_channel_deposit_full_witness_inner( &DepositWitness { deposit_address, asset: ETH, @@ -718,12 +718,12 @@ fn multi_use_deposit_same_block() { amount: MinimumDeposit::::get(asset) + DEPOSIT_AMOUNT, deposit_details: Default::default(), }; - assert_ok!(IngressEgress::process_channel_deposit_full_witness( + assert_ok!(IngressEgress::process_channel_deposit_full_witness_inner( &deposit_witness, Default::default(), )); - assert_ok!(IngressEgress::process_channel_deposit_full_witness( + assert_ok!(IngressEgress::process_channel_deposit_full_witness_inner( &deposit_witness, Default::default(), )); @@ -892,7 +892,7 @@ fn deposits_ingress_fee_exceeding_deposit_amount_rejected() { deposit_details: Default::default(), }; - assert_ok!(IngressEgress::process_channel_deposit_full_witness( + assert_ok!(IngressEgress::process_channel_deposit_full_witness_inner( &deposit, Default::default() )); @@ -915,7 +915,7 @@ fn deposits_ingress_fee_exceeding_deposit_amount_rejected() { // Set fees to less than the deposit amount and retry. ChainTracker::::set_fee(LOW_FEE); - assert_ok!(IngressEgress::process_channel_deposit_full_witness( + assert_ok!(IngressEgress::process_channel_deposit_full_witness_inner( &deposit, Default::default() )); @@ -949,7 +949,7 @@ fn handle_pending_deployment() { IngressEgress::on_finalize(1); assert_eq!(ScheduledEgressFetchOrTransfer::::decode_len().unwrap_or_default(), 0); // Process deposit again the same address. - assert_ok!(IngressEgress::process_channel_deposit_full_witness( + assert_ok!(IngressEgress::process_channel_deposit_full_witness_inner( &DepositWitness { deposit_address, asset: ETH, @@ -980,7 +980,7 @@ fn handle_pending_deployment_same_block() { new_test_ext().execute_with(|| { // Initial request. let (_, deposit_address) = request_address_and_deposit(ALICE, EthAsset::Eth); - assert_ok!(IngressEgress::process_channel_deposit_full_witness( + assert_ok!(IngressEgress::process_channel_deposit_full_witness_inner( &DepositWitness { deposit_address, asset: EthAsset::Eth, @@ -1716,7 +1716,7 @@ fn do_not_batch_more_fetches_than_the_limit_allows() { .unwrap(); let address: ::ChainAccount = address.try_into().unwrap(); - assert_ok!(IngressEgress::process_channel_deposit_full_witness( + assert_ok!(IngressEgress::process_channel_deposit_full_witness_inner( &DepositWitness { deposit_address: address, asset: ASSET, 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 8a4d13de4b..67ae718006 100644 --- a/state-chain/pallets/cf-ingress-egress/src/tests/boost.rs +++ b/state-chain/pallets/cf-ingress-egress/src/tests/boost.rs @@ -78,7 +78,7 @@ fn prewitness_deposit(deposit_address: H160, asset: EthAsset, amount: AssetAmoun #[track_caller] fn witness_deposit(deposit_address: H160, asset: EthAsset, amount: AssetAmount) { - assert_ok!(Pallet::::process_channel_deposit_full_witness( + assert_ok!(Pallet::::process_channel_deposit_full_witness_inner( &DepositWitness:: { deposit_address, asset, diff --git a/state-chain/pallets/cf-ingress-egress/src/tests/screening.rs b/state-chain/pallets/cf-ingress-egress/src/tests/screening.rs index 58661ee289..557821af44 100644 --- a/state-chain/pallets/cf-ingress-egress/src/tests/screening.rs +++ b/state-chain/pallets/cf-ingress-egress/src/tests/screening.rs @@ -55,7 +55,7 @@ mod helpers { ) .unwrap(); let address: ::ChainAccount = address.try_into().unwrap(); - assert_ok!(IngressEgress::process_channel_deposit_full_witness( + assert_ok!(IngressEgress::process_channel_deposit_full_witness_inner( &DepositWitness { deposit_address: address.clone(), asset, @@ -123,7 +123,7 @@ fn process_marked_transaction_and_expect_refund() { tx_in_id, )); - assert_ok!(IngressEgress::process_channel_deposit_full_witness( + assert_ok!(IngressEgress::process_channel_deposit_full_witness_inner( &DepositWitness { deposit_address: address.clone(), asset: btc::Asset::Btc, @@ -177,7 +177,7 @@ fn finalize_boosted_tx_if_marked_after_prewitness() { tx_id, ),); - assert_ok!(IngressEgress::process_channel_deposit_full_witness( + assert_ok!(IngressEgress::process_channel_deposit_full_witness_inner( &DepositWitness { deposit_address: address.clone(), asset: btc::Asset::Btc, @@ -226,7 +226,7 @@ fn reject_tx_if_marked_before_prewitness() { 10, )); - assert_ok!(IngressEgress::process_channel_deposit_full_witness( + assert_ok!(IngressEgress::process_channel_deposit_full_witness_inner( &DepositWitness { deposit_address: address.clone(), asset: btc::Asset::Btc, @@ -424,7 +424,7 @@ fn can_report_between_prewitness_and_witness_if_tx_was_not_boosted() { OriginTrait::signed(BROKER), tx_id )); - assert_ok!(IngressEgress::process_channel_deposit_full_witness(&deposit_witness, 10)); + assert_ok!(IngressEgress::process_channel_deposit_full_witness_inner(&deposit_witness, 10)); assert_has_matching_event!( Test, From 665b00292c107fe93bd8a54ace8e7a8676500b05 Mon Sep 17 00:00:00 2001 From: Maxim Shishmarev Date: Wed, 4 Dec 2024 13:48:02 +1100 Subject: [PATCH 12/13] chore: fix another unit test --- state-chain/pallets/cf-ingress-egress/src/tests.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/state-chain/pallets/cf-ingress-egress/src/tests.rs b/state-chain/pallets/cf-ingress-egress/src/tests.rs index e14eb3627e..2b99f6ba67 100644 --- a/state-chain/pallets/cf-ingress-egress/src/tests.rs +++ b/state-chain/pallets/cf-ingress-egress/src/tests.rs @@ -667,8 +667,8 @@ fn multi_use_deposit_address_different_blocks() { }) // The channel should be closed at the next block. .then_execute_at_next_block(|deposit_address| { - assert_ok!(IngressEgress::process_channel_deposit_full_witness_inner( - &DepositWitness { + IngressEgress::process_channel_deposit_full_witness( + DepositWitness { deposit_address, asset: ETH, amount: 1, @@ -676,7 +676,7 @@ fn multi_use_deposit_address_different_blocks() { }, // block height is purely informative. BlockHeightProvider::::get_block_height(), - )); + ); deposit_address }) .then_process_events(|_, event| match event { From 573982acf58caa0f84a76a4b349314d973e87332 Mon Sep 17 00:00:00 2001 From: Maxim Shishmarev Date: Wed, 4 Dec 2024 17:08:08 +1100 Subject: [PATCH 13/13] test: add extra test --- .../pallets/cf-ingress-egress/src/lib.rs | 3 +- .../cf-ingress-egress/src/tests/boost.rs | 47 +++++++++++++++++++ 2 files changed, 48 insertions(+), 2 deletions(-) diff --git a/state-chain/pallets/cf-ingress-egress/src/lib.rs b/state-chain/pallets/cf-ingress-egress/src/lib.rs index 89c7cc7873..49963d7115 100644 --- a/state-chain/pallets/cf-ingress-egress/src/lib.rs +++ b/state-chain/pallets/cf-ingress-egress/src/lib.rs @@ -2038,8 +2038,7 @@ impl, I: 'static> Pallet { // Pre-witnessing twice is unlikely but possible. Either way we don't want // to change the status and we don't want to allow boosting. Some(TransactionPrewitnessedStatus::Prewitnessed) => true, - // Transaction has not been reported, mark it as boosted to prevent further - // reports. + // Transaction has not been reported None => false, } }) { 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 67ae718006..ddd5e3d2e3 100644 --- a/state-chain/pallets/cf-ingress-egress/src/tests/boost.rs +++ b/state-chain/pallets/cf-ingress-egress/src/tests/boost.rs @@ -993,6 +993,53 @@ fn test_create_boost_pools() { }); } +#[test] +fn failed_prewitness_does_not_discard_remaining_deposits_in_a_batch() { + new_test_ext().execute_with(|| { + setup(); + + assert_ok!(IngressEgress::add_boost_funds( + RuntimeOrigin::signed(BOOSTER_1), + EthAsset::Eth, + DEFAULT_DEPOSIT_AMOUNT, + TIER_5_BPS + )); + + let (_, address, _, _) = IngressEgress::open_channel( + &ALICE, + EthAsset::Eth, + ChannelAction::LiquidityProvision { lp_account: 0, refund_address: None }, + TIER_5_BPS, + ) + .unwrap(); + + assert_ok!(IngressEgress::process_deposits( + RuntimeOrigin::root(), + vec![ + // The deposit into an unkown address should fail + DepositWitness { + deposit_address: [0; 20].into(), + asset: EthAsset::Eth, + amount: DEFAULT_DEPOSIT_AMOUNT, + deposit_details: Default::default(), + // This deposit should succeed: + }, DepositWitness { + deposit_address: address, + asset: EthAsset::Eth, + amount: DEFAULT_DEPOSIT_AMOUNT, + deposit_details: Default::default(), + } + ], + 0 + )); + + assert_has_matching_event!( + Test, + RuntimeEvent::IngressEgress(Event::DepositBoosted { deposit_address, .. }) if deposit_address == &Some(address) + ); + }); +} + mod vault_swaps { use crate::BoostedVaultTransactions;