_calc_withdraw_one_coin : incorrect precision handling between Curve and Thorn

[hats-bug-reporter[bot]]

Github username: -- Twitter username: -- Submission hash (on-chain): 0x1924c3ac29209174e34ccc1d7a57b5723428c77c9ac8e6a437272f98c3969d8e Severity: high

Description: Description\

When we cross checking with standard curve implementation on the function _calc_withdraw_one_coin , we found discreency in how the precision is used to get the final output.

The function _calc_withdraw_one_coin is mainly used in remove_liquidity_one_coin which is called by the lp.

Lets see the precison part of the Thorn

_calc_withdraw_one_coin

function _calc_withdraw_one_coin(uint256 _token_amount, uint256 i) internal view returns (uint256, uint256) {
        // First, need to calculate
        // * Get current D
        // * Solve Eqn against y_i for D - _token_amount
        uint256 amp = get_A();
        uint256 _fee = (fee * N_COINS) / (4 * (N_COINS - 1));
        uint256[N_COINS] memory precisions = PRECISION_MUL;
        uint256 total_supply = token.totalSupply();

>>>  uint256[N_COINS] memory xp = _xp();

        uint256 D0 = get_D(xp, amp);
        uint256 D1 = D0 - (_token_amount * D0) / total_supply;
        uint256[N_COINS] memory xp_reduced = xp;

        uint256 new_y = get_y_D(amp, i, xp, D1);
>>>  uint256 dy_0 = (xp[i] - new_y) / precisions[i]; // w/o fees

        for (uint256 k = 0; k < N_COINS; k++) {
            uint256 dx_expected;
            if (k == i) {
                dx_expected = (xp[k] * D1) / D0 - new_y;
            } else {
                dx_expected = xp[k] - (xp[k] * D1) / D0;
            }
            xp_reduced[k] -= (_fee * dx_expected) / FEE_DENOMINATOR;
        }
        uint256 dy = xp_reduced[i] - get_y_D(amp, i, xp_reduced, D1);
>>>  dy = (dy - 1) / precisions[i]; // Withdraw less to account for rounding errors

        return (dy, dy_0 - dy);
    }

_xp

    function _xp() internal view returns (uint256[N_COINS] memory result) {
        result = RATES;
        for (uint256 i = 0; i < N_COINS; i++) {
            result[i] = (result[i] * balances[i]) / PRECISION;
        }
    }

the _xp() returns with precison value of RATES/PRECISION.

But the_calc_withdraw_one_coin just divides the final outut by PRECISION_MUL which is incorrect.

Lets see the curve 2 pool implementation . referred from here https://arbiscan.io/address/0x7f90122BF0700F9E7e1F688fe926940E8839F353#code

def _calc_withdraw_one_coin(_burn_amount: uint256, i: int128) -> uint256[2]:
    # First, need to calculate
    # * Get current D
    # * Solve Eqn against y_i for D - _token_amount
    amp: uint256 = self._A()
    xp: uint256[N_COINS] = self._xp_mem(self.balances)
    D0: uint256 = self.get_D(xp, amp)

    total_supply: uint256 = self.totalSupply
    D1: uint256 = D0 - _burn_amount * D0 / total_supply
    new_y: uint256 = self.get_y_D(amp, i, xp, D1)

    base_fee: uint256 = self.fee * N_COINS / (4 * (N_COINS - 1))
    xp_reduced: uint256[N_COINS] = empty(uint256[N_COINS])

    for j in range(N_COINS):
        dx_expected: uint256 = 0
        xp_j: uint256 = xp[j]
        if j == i:
            dx_expected = xp_j * D1 / D0 - new_y
        else:
            dx_expected = xp_j - xp_j * D1 / D0
        xp_reduced[j] = xp_j - base_fee * dx_expected / FEE_DENOMINATOR

    dy: uint256 = xp_reduced[i] - self.get_y_D(amp, i, xp_reduced, D1)
>> dy_0: uint256 = (xp[i] - new_y) * PRECISION / RATE_MULTIPLIER  # w/o fees
>>    dy = (dy - 1) * PRECISION / RATE_MULTIPLIER  # Withdraw less to account for rounding errors

    return [dy, dy_0 - dy]

@pure
@internal
def _xp_mem(_balances: uint256[N_COINS]) -> uint256[N_COINS]:
    result: uint256[N_COINS] = empty(uint256[N_COINS])
    for i in range(N_COINS):
        result[i] = RATE_MULTIPLIER * _balances[i] / PRECISION
    return result

_xp_mem returns with precision RATE_MULTIPLIER /PRECISION and this has handlned in _calc_withdraw_one_coin as shown in above function.

Impact

Incorrect value computation from the function _calc_withdraw_one_coin

Revised Code File (Optional)

We would suggest to update the code as done in the curve implementation.

[0xpinky]

if needed, we will add more information. we thought its straight forward mistake done.

[AlexAurthur]

no,it works as expected as lets say for 6 decimal token

uint256 dy = xp_reduced[i] - get_y_D(amp, i, xp_reduced, D1);

here dy will have precision of 10^18

dy = (dy - 1) / precisions[i];

and here dy will be 10^18-10^12 = 10^6 for 6 decimals token,which is correct!