hats-finance / Common--Stableswap-0xd4d9a2772202ce33b24901d3fc94e95a84b37430

Apache License 2.0
0 stars 0 forks source link

`remove_liquidity_by_shares` doesn't take a fee #24

Open hats-bug-reporter[bot] opened 2 months ago

hats-bug-reporter[bot] commented 2 months ago

Github username: @0x3b Twitter username: 0x3b33 Submission hash (on-chain): 0x4e9c05d3f11a4f64bf5cc49cf789d22fcae77a69ad30236befad1c5796f2b349 Severity: medium

Description: Description\ Unlike remove_liquidity_by_amounts which takes a fee on withdraw

            let (shares_to_burn, fee_part) = math::rated_compute_lp_amount_for_withdraw(
                &rates,
                &amounts,
                &self.reserves(),
                self.psp22.total_supply(),
                Some(&self.pool.fees),
                self.amp_coef(),
            )?;

remove_liquidity_by_shares doesn't take any fees.

Because of this users will be able to withdraw without incurring any fees, lowering the yeild for the rest of the LP providers and the system.

Recoomendation Calculate a fee based on the amount of shares withdraws. This can be done by subtracting shares by the fee and using that amount inside compute_amounts_given_lp.

Example pseudo code:

        fn remove_liquidity_by_shares(
            &mut self,
            shares: u128,
            min_amounts: Vec<u128>,
            to: AccountId,
        ) -> Result<Vec<u128>, StablePoolError> {

            let protocol_fee = 0 
            if let Some(fee_to) = self.fee_receiver() {
                protocol_fee = self.pool.fees.protocol_trade_fee(fee_part)?;
                if protocol_fee > 0 {
                    let events = self.psp22.mint(fee_to, protocol_fee)?;
                    self.emit_events(events);
                }
            }

            let amounts = math::compute_amounts_given_lp(
              shares -  protocol_fee,
                &self.reserves(),
                self.psp22.total_supply(),
            )?;
JanKuczma commented 2 months ago

Duplicate of #14