Skip to content

Commit

Permalink
chore: address review
Browse files Browse the repository at this point in the history
  • Loading branch information
msgmaxim committed Dec 16, 2024
1 parent e341fe3 commit 1bf8d74
Show file tree
Hide file tree
Showing 9 changed files with 109 additions and 50 deletions.
8 changes: 4 additions & 4 deletions state-chain/custom-rpc/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -565,7 +565,7 @@ mod boost_pool_rpc {
available_amounts: Vec<AccountAndAmount>,
deposits_pending_finalization: Vec<PendingBoost>,
pending_withdrawals: Vec<PendingWithdrawal>,
network_fee_deduction_percents: Percent,
network_fee_deduction_percent: Percent,
}

impl BoostPoolDetailsRpc {
Expand Down Expand Up @@ -603,7 +603,7 @@ mod boost_pool_rpc {
pending_deposits,
})
.collect(),
network_fee_deduction_percents: details.network_fee_deduction_percents,
network_fee_deduction_percent: details.network_fee_deduction_percent,
}
}
}
Expand Down Expand Up @@ -2424,7 +2424,7 @@ mod test {
(1, BTreeMap::from([(ID_1.clone(), OwedAmount { total: 1_000, fee: 50 })])),
]),
pending_withdrawals: Default::default(),
network_fee_deduction_percents: Percent::from_percent(40),
network_fee_deduction_percent: Percent::from_percent(40),
}
}

Expand All @@ -2442,7 +2442,7 @@ mod test {
(ID_1.clone(), BTreeSet::from([0])),
(ID_2.clone(), BTreeSet::from([0])),
]),
network_fee_deduction_percents: Percent::from_percent(0),
network_fee_deduction_percent: Percent::from_percent(0),
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ expression: val
}
],
"pending_withdrawals": [],
"network_fee_deduction_percents": 40
"network_fee_deduction_percent": 40
},
{
"fee_tier": 30,
Expand Down Expand Up @@ -74,6 +74,6 @@ expression: val
]
}
],
"network_fee_deduction_percents": 0
"network_fee_deduction_percent": 0
}
]
8 changes: 4 additions & 4 deletions state-chain/pallets/cf-ingress-egress/src/boost_pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ where
AccountId: PartialEq + core::fmt::Debug,
{
pub unlocked_funds: Vec<(AccountId, C::ChainAmount)>,
pub amount_credited: C::ChainAmount,
pub amount_credited_to_boosters: C::ChainAmount,
}

impl<AccountId, C: Chain> BoostPool<AccountId, C>
Expand Down Expand Up @@ -250,8 +250,8 @@ where
(provided_amount, fee)
};

// NOTE: only a portion of the boost fee goes to the boost pool,
// the rest is charged as network fee:
// NOTE: before the boost fee is credited to the boost pool, a portion
// of it is deducted as network fee:
let network_fee = network_fee_deduction * u128::from(fee_amount);
let boost_pool_fee = fee_amount.saturating_sub(ScaledAmount::from(network_fee));

Expand Down Expand Up @@ -393,7 +393,7 @@ where

DepositFinalisationOutcomeForPool {
unlocked_funds,
amount_credited: amount_credited.into_chain_amount(),
amount_credited_to_boosters: amount_credited.into_chain_amount(),
}
}

Expand Down
89 changes: 72 additions & 17 deletions state-chain/pallets/cf-ingress-egress/src/boost_pool/tests.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use super::*;
use cf_chains::Ethereum;
use cf_primitives::{AssetAmount, EthAmount, FLIPPERINOS_PER_FLIP};
use cf_primitives::{AssetAmount, EthAmount, FLIPPERINOS_PER_FLIP, MAX_BASIS_POINTS};

use sp_std::collections::btree_set::BTreeSet;

Expand Down Expand Up @@ -172,25 +172,73 @@ fn boosting_with_fees() {

assert_eq!(
pool.process_deposit_as_finalised(BOOST_1),
DepositFinalisationOutcomeForPool { amount_credited: 1010, unlocked_funds: vec![] }
DepositFinalisationOutcomeForPool {
amount_credited_to_boosters: 1010,
unlocked_funds: vec![]
}
);

check_pool(&pool, [(BOOSTER_1, 1003), (BOOSTER_2, 2006)]);
}

#[test]
fn boosting_with_max_network_fee_deduction() {
const BOOST_FEE_BPS: u16 = 100;
const INIT_BOOSTER_AMOUNT: u128 = 2000;
const NETWORK_FEE_PORTION_PERCENT: u8 = 100;

let mut pool = TestPool::new(BOOST_FEE_BPS);

pool.add_funds(BOOSTER_1, INIT_BOOSTER_AMOUNT);

check_pool(&pool, [(BOOSTER_1, INIT_BOOSTER_AMOUNT)]);

const PROVIDED_AMOUNT: u128 = 1000;
const FULL_BOOST_FEE: u128 =
(PROVIDED_AMOUNT * BOOST_FEE_BPS as u128) / MAX_BASIS_POINTS as u128;

const DEPOSIT_AMOUNT: u128 = PROVIDED_AMOUNT + FULL_BOOST_FEE;

// NOTE: Full 1% boost fee is charged from the deposit
assert_eq!(
pool.provide_funds_for_boosting(
BOOST_1,
DEPOSIT_AMOUNT,
Percent::from_percent(NETWORK_FEE_PORTION_PERCENT)
),
Ok((DEPOSIT_AMOUNT, FULL_BOOST_FEE))
);

// Booster's contribution is recorded, but they earn 0 fees:
check_pending_boosts(&pool, [(BOOST_1, vec![(BOOSTER_1, 1000, 0)])]);

assert_eq!(
pool.process_deposit_as_finalised(BOOST_1),
DepositFinalisationOutcomeForPool {
amount_credited_to_boosters: 0,
unlocked_funds: vec![]
}
);

// No change in the boost pool after deposit is finalised:
check_pool(&pool, [(BOOSTER_1, INIT_BOOSTER_AMOUNT)]);
}

#[test]
fn boosting_with_fees_including_network_fee_portion() {
const NETWORK_FEE_PORTION_PERCENT: u8 = 30;
const BOOST_FEE_BPS: u16 = 100;

let mut pool = TestPool::new(100); // 1%
let mut pool = TestPool::new(BOOST_FEE_BPS);

pool.add_funds(BOOSTER_1, 1000);
pool.add_funds(BOOSTER_2, 2000);

check_pool(&pool, [(BOOSTER_1, 1000), (BOOSTER_2, 2000)]);

const PROVIDED_AMOUNT: u128 = 1000;
const FULL_BOOST_FEE: u128 = 10;
const FULL_BOOST_FEE: u128 =
(PROVIDED_AMOUNT * BOOST_FEE_BPS as u128) / MAX_BASIS_POINTS as u128;

const DEPOSIT_AMOUNT: u128 = PROVIDED_AMOUNT + FULL_BOOST_FEE;

Expand All @@ -211,8 +259,9 @@ fn boosting_with_fees_including_network_fee_portion() {

const NETWORK_FEE_FROM_BOOST: u128 = FULL_BOOST_FEE * NETWORK_FEE_PORTION_PERCENT as u128 / 100;

// NOTE: we subtract 1 to account for a rounding "error" (in real code any remaining fee will be
// used as network fee, so all atomic units will be accounted for)
// Sanity check: network fee and boosters fee should make up the full boost fee.
// Note that we subtract 1 to account for a rounding "error" (in real code any
// remaining fee will be used as network fee, so all atomic units will be accounted for).
assert_eq!(TOTAL_BOOSTERS_FEE, FULL_BOOST_FEE - NETWORK_FEE_FROM_BOOST - 1);

// The recorded amounts include fees
Expand All @@ -230,7 +279,7 @@ fn boosting_with_fees_including_network_fee_portion() {
assert_eq!(
pool.process_deposit_as_finalised(BOOST_1),
DepositFinalisationOutcomeForPool {
amount_credited: PROVIDED_AMOUNT + TOTAL_BOOSTERS_FEE,
amount_credited_to_boosters: PROVIDED_AMOUNT + TOTAL_BOOSTERS_FEE,
unlocked_funds: vec![]
}
);
Expand Down Expand Up @@ -273,7 +322,7 @@ fn adding_funds_during_pending_withdrawal_from_same_booster() {
assert_eq!(
pool.process_deposit_as_finalised(BOOST_1),
DepositFinalisationOutcomeForPool {
amount_credited: DEPOSIT_AMOUNT,
amount_credited_to_boosters: DEPOSIT_AMOUNT,
unlocked_funds: vec![]
}
);
Expand All @@ -296,7 +345,7 @@ fn withdrawing_funds_before_finalisation() {
assert_eq!(
pool.process_deposit_as_finalised(BOOST_1),
DepositFinalisationOutcomeForPool {
amount_credited: 1000,
amount_credited_to_boosters: 1000,
unlocked_funds: vec![(BOOSTER_1, 500)]
}
);
Expand All @@ -323,7 +372,7 @@ fn adding_funds_with_pending_withdrawals() {
assert_eq!(
pool.process_deposit_as_finalised(BOOST_1),
DepositFinalisationOutcomeForPool {
amount_credited: 1000,
amount_credited_to_boosters: 1000,
unlocked_funds: vec![(BOOSTER_1, 500)]
}
);
Expand Down Expand Up @@ -393,7 +442,7 @@ fn partially_losing_pending_withdrawals() {
assert_eq!(
pool.process_deposit_as_finalised(BOOST_1),
DepositFinalisationOutcomeForPool {
amount_credited: 500,
amount_credited_to_boosters: 500,
unlocked_funds: vec![(BOOSTER_1, 250)]
}
);
Expand Down Expand Up @@ -436,7 +485,7 @@ fn booster_joins_then_funds_lost() {
assert_eq!(
pool.process_deposit_as_finalised(BOOST_1),
DepositFinalisationOutcomeForPool {
amount_credited: 500,
amount_credited_to_boosters: 500,
unlocked_funds: vec![(BOOSTER_1, 250)]
}
);
Expand Down Expand Up @@ -484,7 +533,7 @@ fn booster_joins_between_boosts() {
assert_eq!(
pool.process_deposit_as_finalised(BOOST_1),
DepositFinalisationOutcomeForPool {
amount_credited: 500,
amount_credited_to_boosters: 500,
unlocked_funds: vec![(BOOSTER_1, 250)]
},
);
Expand All @@ -506,7 +555,10 @@ fn booster_joins_between_boosts() {
let mut pool = pool.clone();
assert_eq!(
pool.process_deposit_as_finalised(BOOST_2),
DepositFinalisationOutcomeForPool { amount_credited: 1000, unlocked_funds: vec![] }
DepositFinalisationOutcomeForPool {
amount_credited_to_boosters: 1000,
unlocked_funds: vec![]
}
);
check_pool(&pool, [(BOOSTER_2, 1010), (BOOSTER_3, 2014)]);
check_pending_boosts(&pool, []);
Expand All @@ -531,7 +583,7 @@ fn small_rewards_accumulate() {
assert_eq!(
pool.process_deposit_as_finalised(BOOST_1),
DepositFinalisationOutcomeForPool {
amount_credited: SMALL_DEPOSIT,
amount_credited_to_boosters: SMALL_DEPOSIT,
unlocked_funds: vec![]
}
);
Expand All @@ -549,7 +601,7 @@ fn small_rewards_accumulate() {
assert_eq!(
pool.process_deposit_as_finalised(prewitnessed_deposit_id),
DepositFinalisationOutcomeForPool {
amount_credited: SMALL_DEPOSIT,
amount_credited_to_boosters: SMALL_DEPOSIT,
unlocked_funds: vec![]
}
);
Expand Down Expand Up @@ -580,7 +632,10 @@ fn use_max_available_amount() {

assert_eq!(
pool.process_deposit_as_finalised(BOOST_1),
DepositFinalisationOutcomeForPool { amount_credited: 1_010_101, unlocked_funds: vec![] }
DepositFinalisationOutcomeForPool {
amount_credited_to_boosters: 1_010_101,
unlocked_funds: vec![]
}
);

check_pool(&pool, [(BOOSTER_1, 1_010_301)]);
Expand Down
28 changes: 16 additions & 12 deletions state-chain/pallets/cf-ingress-egress/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ pub enum PalletConfigUpdate<T: Config<I>, I: 'static> {
lifetime: TargetChainBlockNumber<T, I>,
},
SetNetworkFeeDeductionFromBoost {
deduction_percents: Percent,
deduction_percent: Percent,
},
}

Expand Down Expand Up @@ -726,7 +726,7 @@ pub mod pallet {

/// The fraction of the network fee that is deducted from the boost fee.
#[pallet::storage]
pub type NetworkFeeDeductionFromBoostPercents<T: Config<I>, I: 'static = ()> =
pub type NetworkFeeDeductionFromBoostPercent<T: Config<I>, I: 'static = ()> =
StorageValue<_, Percent, ValueQuery>;

#[pallet::event]
Expand Down Expand Up @@ -898,7 +898,7 @@ pub mod pallet {
short_affiliate_id: AffiliateShortId,
},
NetworkFeeDeductionFromBoostSet {
deduction_percents: Percent,
deduction_percent: Percent,
},
}

Expand Down Expand Up @@ -1309,11 +1309,11 @@ pub mod pallet {
DepositChannelLifetime::<T, I>::set(lifetime);
Self::deposit_event(Event::<T, I>::DepositChannelLifetimeSet { lifetime });
},
PalletConfigUpdate::SetNetworkFeeDeductionFromBoost { deduction_percents } => {
NetworkFeeDeductionFromBoostPercents::<T, I>::set(deduction_percents);
PalletConfigUpdate::SetNetworkFeeDeductionFromBoost { deduction_percent } => {
NetworkFeeDeductionFromBoostPercent::<T, I>::set(deduction_percent);

Self::deposit_event(Event::<T, I>::NetworkFeeDeductionFromBoostSet {
deduction_percents,
deduction_percent,
});
},
}
Expand Down Expand Up @@ -1781,7 +1781,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
"Boost tiers should be in ascending order"
);

let network_fee_portion = NetworkFeeDeductionFromBoostPercents::<T, I>::get();
let network_fee_portion = NetworkFeeDeductionFromBoostPercent::<T, I>::get();

for boost_tier in sorted_boost_tiers {
if boost_tier > max_boost_fee_bps {
Expand Down Expand Up @@ -2345,16 +2345,19 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
};

if let Some((prewitnessed_deposit_id, used_pools)) = maybe_boost_to_process {
let mut total_amount_credited: TargetChainAmount<T, I> = 0u32.into();
let mut total_amount_credited_to_boosters: TargetChainAmount<T, I> = 0u32.into();
// Note that ingress fee is not payed here, as it has already been payed at the time
// of boosting
for boost_tier in used_pools {
BoostPools::<T, I>::mutate(asset, boost_tier, |maybe_pool| {
if let Some(pool) = maybe_pool {
let DepositFinalisationOutcomeForPool { unlocked_funds, amount_credited } =
pool.process_deposit_as_finalised(prewitnessed_deposit_id);
let DepositFinalisationOutcomeForPool {
unlocked_funds,
amount_credited_to_boosters,
} = pool.process_deposit_as_finalised(prewitnessed_deposit_id);

total_amount_credited += amount_credited;
total_amount_credited_to_boosters
.saturating_accrue(amount_credited_to_boosters);

for (booster_id, finalised_withdrawn_amount) in unlocked_funds {
T::Balance::credit_account(
Expand All @@ -2368,7 +2371,8 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
}

// Any excess amount is charged as network fee:
let network_fee_from_boost = deposit_amount.saturating_sub(total_amount_credited);
let network_fee_from_boost =
deposit_amount.saturating_sub(total_amount_credited_to_boosters);

let network_fee_swap_request_id = if network_fee_from_boost > 0u32.into() {
// NOTE: if asset is FLIP, we shouldn't need to swap, but it should still work, and
Expand Down
Loading

0 comments on commit 1bf8d74

Please sign in to comment.