Chore/pick 1.1 fixes (#4390)

* fix: keep utxos even if swallowed (PRO-1068) (#4360)

* fix: network fee (#4364)

Co-authored-by: Daniel <daniel@chainflip.io>

* feat: more accurate btc fees (#4367)

* fix test

---------

Co-authored-by: dandanlen <3168260+dandanlen@users.noreply.github.com>
Co-authored-by: Alastair Holmes <42404303+AlastairHolmes@users.noreply.github.com>
Co-authored-by: Daniel <daniel@chainflip.io>

state-chain/chains/src/btc.rs

@@ -21,6 +21,7 @@ use cf_utilities::SliceToArray;
 use codec::{Decode, Encode, MaxEncodedLen};
 use frame_support::{
 	sp_io::hashing::sha2_256,
+	sp_runtime::{FixedPointNumber, FixedU128},
 	traits::{ConstBool, ConstU32},
 	BoundedVec, RuntimeDebug,
 };
@@ -116,21 +117,30 @@ impl Default for BitcoinTrackedData {
 	}
 }
 
+/// A constant multiplier applied to the fees.
+///
+/// TODO: Allow this value to adjust based on the current fee deficit/surplus.
+const BTC_FEE_MULTIPLIER: FixedU128 = FixedU128::from_rational(3, 2);
+
 impl FeeEstimationApi<Bitcoin> for BitcoinTrackedData {
 	fn estimate_ingress_fee(
 		&self,
 		_asset: <Bitcoin as Chain>::ChainAsset,
 	) -> <Bitcoin as Chain>::ChainAmount {
-		// Include the min fee so we over-estimate the cost.
-		self.btc_fee_info.min_fee_required_per_tx + self.btc_fee_info.fee_per_input_utxo
+		BTC_FEE_MULTIPLIER
+			.checked_mul_int(self.btc_fee_info.fee_per_input_utxo)
+			.expect("fee is a u64, multiplier is 1.5, so this should never overflow")
 	}
 
 	fn estimate_egress_fee(
 		&self,
 		_asset: <Bitcoin as Chain>::ChainAsset,
 	) -> <Bitcoin as Chain>::ChainAmount {
-		// Include the min fee so we over-estimate the cost.
-		self.btc_fee_info.min_fee_required_per_tx + self.btc_fee_info.fee_per_output_utxo
+		BTC_FEE_MULTIPLIER
+			.checked_mul_int(
+				self.btc_fee_info.min_fee_required_per_tx + self.btc_fee_info.fee_per_output_utxo,
+			)
+			.expect("fee is a u64, multiplier is 1.5, so this should never overflow")
 	}
 }
 
@@ -173,15 +183,18 @@ impl BitcoinFeeInfo {
 	/// We ensure that a minimum of 1 sat per vByte is set for each of the fees.
 	pub fn new(sats_per_kilo_byte: BtcAmount) -> Self {
 		Self {
-			// Our input utxos are approximately 178 bytes each in the Btc transaction
+			// Our input utxos are approximately INPUT_UTXO_SIZE_IN_BYTES vbytes each in the Btc
+			// transaction
 			fee_per_input_utxo: max(sats_per_kilo_byte, BYTES_PER_KILOBYTE)
 				.saturating_mul(INPUT_UTXO_SIZE_IN_BYTES) /
 				BYTES_PER_KILOBYTE,
-			// Our output utxos are approximately 34 bytes each in the Btc transaction
+			// Our output utxos are approximately OUTPUT_UTXO_SIZE_IN_BYTES vbytes each in the Btc
+			// transaction
 			fee_per_output_utxo: max(BYTES_PER_KILOBYTE, sats_per_kilo_byte)
 				.saturating_mul(OUTPUT_UTXO_SIZE_IN_BYTES) /
 				BYTES_PER_KILOBYTE,
-			// Minimum size of tx that does not scale with input and output utxos is 12 bytes
+			// Minimum size of tx that does not scale with input and output utxos is
+			// MINIMUM_BTC_TX_SIZE_IN_BYTES bytes
 			min_fee_required_per_tx: max(BYTES_PER_KILOBYTE, sats_per_kilo_byte)
 				.saturating_mul(MINIMUM_BTC_TX_SIZE_IN_BYTES) /
 				BYTES_PER_KILOBYTE,

state-chain/pallets/cf-environment/src/tests.rs

@@ -53,23 +53,24 @@ fn test_btc_utxo_selection() {
 		add_utxo_amount(dust_amount);
 
 		// select some utxos for a tx
+		const EXPECTED_CHANGE_AMOUNT: crate::BtcAmount = 24770;
 		assert_eq!(
 			Environment::select_and_take_bitcoin_utxos(UtxoSelectionType::Some {
 				output_amount: 12000,
 				number_of_outputs: 2
 			})
 			.unwrap(),
-			(vec![utxo(5000), utxo(10000), utxo(25000), utxo(100000)], 120080)
+			(vec![utxo(5000), utxo(10000), utxo(25000)], EXPECTED_CHANGE_AMOUNT)
 		);
 
 		// add the change utxo back to the available utxo list
-		add_utxo_amount(120080);
+		add_utxo_amount(EXPECTED_CHANGE_AMOUNT);
 
 		// select all remaining utxos
 		assert_eq!(
 			Environment::select_and_take_bitcoin_utxos(UtxoSelectionType::SelectAllForRotation)
 				.unwrap(),
-			(vec![utxo(5000000), utxo(120080),], 5116060)
+			(vec![utxo(5000000), utxo(100000), utxo(EXPECTED_CHANGE_AMOUNT),], 5121970)
 		);
 
 		// add some more utxos to the list
@@ -87,7 +88,7 @@ fn test_btc_utxo_selection() {
 		assert_eq!(
 			Environment::select_and_take_bitcoin_utxos(UtxoSelectionType::SelectAllForRotation)
 				.unwrap(),
-			(vec![utxo(5000), utxo(15000),], 15980)
+			(vec![utxo(5000), utxo(15000),], 17950)
 		);
 	});
 }
@@ -144,7 +145,7 @@ fn test_btc_utxo_consolidation() {
 		// Should select two UTXOs, with all funds (minus fees) going back to us as change
 		assert_eq!(
 			Environment::select_and_take_bitcoin_utxos(UtxoSelectionType::SelectForConsolidation),
-			Some((vec![utxo(10000), utxo(20000)], 25980))
+			Some((vec![utxo(10000), utxo(20000)], 27950))
 		);
 
 		// Any utxo that didn't get consolidated should still be available:

state-chain/pallets/cf-ingress-egress/src/lib.rs

@@ -997,6 +997,16 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 			deposit_amount,
 		);
 
+		// Add the deposit to the balance.
+		T::DepositHandler::on_deposit_made(
+			deposit_details.clone(),
+			deposit_amount,
+			deposit_channel_details.deposit_channel,
+		);
+		DepositBalances::<T, I>::mutate(asset, |deposits| {
+			deposits.register_deposit(net_deposit_amount)
+		});
+
 		if net_deposit_amount.is_zero() {
 			Self::deposit_event(Event::<T, I>::DepositIgnored {
 				deposit_address,
@@ -1005,74 +1015,64 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 				deposit_details,
 				reason: DepositIgnoredReason::NotEnoughToPayFees,
 			});
-			return Ok(())
-		}
-
-		match deposit_channel_details.action {
-			ChannelAction::LiquidityProvision { lp_account, .. } =>
-				T::LpBalance::try_credit_account(
-					&lp_account,
+		} else {
+			match deposit_channel_details.action {
+				ChannelAction::LiquidityProvision { lp_account, .. } =>
+					T::LpBalance::try_credit_account(
+						&lp_account,
+						asset.into(),
+						net_deposit_amount.into(),
+					)?,
+				ChannelAction::Swap {
+					destination_address,
+					destination_asset,
+					broker_id,
+					broker_commission_bps,
+					..
+				} => T::SwapDepositHandler::schedule_swap_from_channel(
+					deposit_address.clone().into(),
+					block_height.into(),
 					asset.into(),
+					destination_asset,
 					net_deposit_amount.into(),
-				)?,
-			ChannelAction::Swap {
-				destination_address,
-				destination_asset,
-				broker_id,
-				broker_commission_bps,
-				..
-			} => T::SwapDepositHandler::schedule_swap_from_channel(
-				deposit_address.clone().into(),
-				block_height.into(),
-				asset.into(),
-				destination_asset,
-				net_deposit_amount.into(),
-				destination_address,
-				broker_id,
-				broker_commission_bps,
-				channel_id,
-			),
-			ChannelAction::CcmTransfer {
-				destination_asset,
-				destination_address,
-				channel_metadata,
-				..
-			} => T::CcmHandler::on_ccm_deposit(
-				asset.into(),
-				net_deposit_amount.into(),
-				destination_asset,
-				destination_address,
-				CcmDepositMetadata {
-					source_chain: asset.into(),
-					source_address: None,
-					channel_metadata,
-				},
-				SwapOrigin::DepositChannel {
-					deposit_address: T::AddressConverter::to_encoded_address(
-						deposit_address.clone().into(),
-					),
+					destination_address,
+					broker_id,
+					broker_commission_bps,
 					channel_id,
-					deposit_block_height: block_height.into(),
-				},
-			),
-		};
+				),
+				ChannelAction::CcmTransfer {
+					destination_asset,
+					destination_address,
+					channel_metadata,
+					..
+				} => T::CcmHandler::on_ccm_deposit(
+					asset.into(),
+					net_deposit_amount.into(),
+					destination_asset,
+					destination_address,
+					CcmDepositMetadata {
+						source_chain: asset.into(),
+						source_address: None,
+						channel_metadata,
+					},
+					SwapOrigin::DepositChannel {
+						deposit_address: T::AddressConverter::to_encoded_address(
+							deposit_address.clone().into(),
+						),
+						channel_id,
+						deposit_block_height: block_height.into(),
+					},
+				),
+			};
 
-		// Add the deposit to the balance.
-		T::DepositHandler::on_deposit_made(
-			deposit_details.clone(),
-			deposit_amount,
-			deposit_channel_details.deposit_channel,
-		);
-		DepositBalances::<T, I>::mutate(asset, |deposits| {
-			deposits.register_deposit(deposit_amount)
-		});
+			Self::deposit_event(Event::DepositReceived {
+				deposit_address,
+				asset,
+				amount: deposit_amount,
+				deposit_details,
+			});
+		}
 
-		Self::deposit_event(Event::DepositReceived {
-			deposit_address,
-			asset,
-			amount: deposit_amount,
-			deposit_details,
-		});
 		Ok(())
 	}
 

state-chain/pallets/cf-pools/src/lib.rs

@@ -242,7 +242,7 @@ impl<T> From<AssetsMap<T>> for SideMap<T> {
 	}
 }
 
-pub const PALLET_VERSION: StorageVersion = StorageVersion::new(1);
+pub const PALLET_VERSION: StorageVersion = StorageVersion::new(2);
 
 #[frame_support::pallet]
 pub mod pallet {
@@ -1559,10 +1559,14 @@ impl<T: Config> Pallet<T> {
 		input_amount: AssetAmount,
 	) -> Result<SwapOutput, DispatchError> {
 		Ok(match (from, to) {
-			(_, STABLE_ASSET) | (STABLE_ASSET, _) => {
+			(_, STABLE_ASSET) => {
 				let output = Self::take_network_fee(Self::swap_single_leg(from, to, input_amount)?);
 				SwapOutput { intermediary: None, output }
 			},
+			(STABLE_ASSET, _) => {
+				let output = Self::swap_single_leg(from, to, Self::take_network_fee(input_amount))?;
+				SwapOutput { intermediary: None, output }
+			},
 			_ => {
 				let intermediary = Self::swap_single_leg(from, STABLE_ASSET, input_amount)?;
 				let output =

state-chain/pallets/cf-pools/src/migrations.rs

@@ -1,5 +1,9 @@
 pub mod v1;
+pub mod v2;
 
 use cf_runtime_upgrade_utilities::VersionedMigration;
 
-pub type PalletMigration<T> = (VersionedMigration<crate::Pallet<T>, v1::Migration<T>, 0, 1>,);
+pub type PalletMigration<T> = (
+	VersionedMigration<crate::Pallet<T>, v1::Migration<T>, 0, 1>,
+	VersionedMigration<crate::Pallet<T>, v2::Migration<T>, 1, 2>,
+);

state-chain/pallets/cf-pools/src/migrations/v2.rs

@@ -0,0 +1,20 @@
+use crate::*;
+use frame_support::traits::OnRuntimeUpgrade;
+use sp_std::marker::PhantomData;
+
+/// Resets the CollectedFees.
+pub struct Migration<T: Config>(PhantomData<T>);
+
+impl<T: Config> OnRuntimeUpgrade for Migration<T> {
+	fn on_runtime_upgrade() -> frame_support::weights::Weight {
+		log::info!("Deleting CollectedNetworkFee");
+		CollectedNetworkFee::<T>::kill();
+		Zero::zero()
+	}
+
+	#[cfg(feature = "try-runtime")]
+	fn post_upgrade(_state: Vec<u8>) -> Result<(), frame_support::sp_runtime::TryRuntimeError> {
+		ensure!(CollectedNetworkFee::<T>::get() == 0, "CollectedNetworkFee should be zero");
+		Ok(())
+	}
+}