Protocol still get charged a fee

[hats-bug-reporter[bot]]

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

Description: Description\ When users swap, add, or remove LQ, they get charged a fee. This fee is either minted directly as shares to the protocol fee receiver or sent as amounts into mint_protocol_fee to calculate the appropriate share to mint to the fee_receiver.

In the end, no matter what users do, they are charged a fee in the form of shares that are minted to the fee_receiver.

As we can see from mint_protocol_fee, our fee_receiver is not charged a fee on minting his shares (expected and wanted):

                    let (protocol_fee_lp, _) = math::rated_compute_lp_amount_for_deposit(
                        &rates,
                        &protocol_deposit_amounts,
                        &reserves,
                        self.psp22.total_supply(),
                        None, // no fees
                        self.amp_coef(),
                    )?;

The same can be said for adding LQ:

            if let Some(fee_to) = self.fee_receiver() {
                let 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);
                }
            }

However, our system gets minted shares, and to withdraw those shares and turn them into assets, it needs to call remove LQ, which will charges a fee.

This fee might be negligible, but given enough time and volume, it can become quite a big amount.

Recommendation Either send the tokens in their raw form or create an admin/system-only function that withdraws without charging a fee.

[0x3b33]

I forgot to add in into the report, but curve has a way to take out fees without using the withdraw method - docs

@external
def withdraw_admin_fees():
    assert msg.sender == self.owner  # dev: only owner

    for i in range(N_COINS):
        c: address = self.coins[i]
        value: uint256 = ERC20(c).balanceOf(self) - self.balances[i]
        if value > 0:
            assert ERC20(c).transfer(msg.sender, value)

[JanKuczma]

Thank you for your submission.

By users you mean the LP providers? The protocol_fee is part of the trade_fee, so the LP providers never lose.

When the fee_receiver wants to withdraw their fees, indeed they have to call remove_liquidity(...) which would account for some fees if withdrawing in imbalanced amounts, however, they can also withdraw using remove_liquidity_by_shares(...) for which they are not charged any fee.

[0x3b33]

By users you mean the LP providers? Yes

Alright, I went with the assumption that remove_liquidity_by_shares should charge a fee and it's just a bug that it doesn't. However this may be a good alternative, which will disable a pathway for users to withdraw without a fee, in tern generating more fees for the system.

[JanKuczma]

remove_liquidity_by_shares should charge a fee and it's just a bug that it doesn't.

It’s not a bug but an intentional behavior of the contract.