Bug in retireCarbonSpecific

[mahdiGhorbanzadeh]

Hi,there is a bug in retireCarbonSpecific when poolToken and sourceToken is not equal and amountInCarbon is false

    function _prepareRetireSpecific(
        address _sourceToken,
        address _poolToken,
        uint256 _amount,
        bool _amountInCarbon,
        address _retiree
    ) internal returns (uint256, uint256) {
        (
            uint256 sourceAmount,
            uint256 totalCarbon,
            uint256 fee
        ) = _transferSourceTokens(
                _sourceToken,
                _poolToken,
                _amount,
                _amountInCarbon,
                true
            );

        // Get the pool tokens

        if (_sourceToken != _poolToken) {
            // Swap the source to get pool
            if (_amountInCarbon) {
                // Add redemption fee to the total to redeem.
                totalCarbon += _getSpecificCarbonFee(_poolToken, _amount);
                _amount += _getSpecificCarbonFee(_poolToken, _amount);

                // swapTokensForExactTokens
                _swapForExactCarbon(
                    _sourceToken,
                    _poolToken,
                    totalCarbon,
                    sourceAmount,
                    _retiree
                );
            } else {
                // swapExactTokensForTokens
                (_amount, fee) = _swapExactForCarbon(
                    _sourceToken,
                    _poolToken,
                    sourceAmount
                );
            }
        } else {
            _amount = sourceAmount - fee;
        }

        if (!_amountInCarbon) {
            // Calculate the fee and adjust if pool token is provided with false bool
            fee = (_amount * feeAmount) / 1000;
            _amount = totalCarbon - fee;
        }

        return (_amount, fee);
    }

example :

sourceToken : usdc
poolToken : bct
amountInCarbon:false
amount : 10 usdc

we consider 2 usdc equal to 1 bct

in this case _swapExactForCarbon return amount of bct (that is 4.95 bct if feeAmount is 1 %)

then because _amountInCarbon is false the below code also run:

if (!_amountInCarbon) {
    // Calculate the fee and adjust if pool token is provided with false bool
    fee = (_amount * feeAmount) / 1000;
    _amount = totalCarbon - fee;
}

and _amount updated here again with a value greater than the actual value that is 4.95

based on the calculations totalCarbon is 10 and _amount ‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍‍updated to something near to 9.9

in this case the above if must not be executed .

[0xAurelius]

Reported by Discord user aliad010#0292

[cujowolf]

@mahdiGhorbanzadeh thanks for the heads up! I'm not sure I follow where this bug is going though. Let's trace through the function call using your example values.

_sourceToken : usdc
_poolToken : bct
_amountInCarbon:false
_amount : 10 usdc (10000000 raw input)

A quick note on the values supplied to the reitreCarbon function call. If we were using the amountInCarbon as true and wanted to retire 5 tons, we would supply 5 (with appropriate decimals) for the amount field. You don't supply the amount including the fee (as it could change potentially)

            // Swap the source to get pool
            if (_amountInCarbon) {
                // Add redemption fee to the total to redeem.
                totalCarbon += _getSpecificCarbonFee(_poolToken, _amount);
                _amount += _getSpecificCarbonFee(_poolToken, _amount);

                // swapTokensForExactTokens
                _swapForExactCarbon(
                    _sourceToken,
                    _poolToken,
                    totalCarbon,
                    sourceAmount,
                    _retiree
                );
            } else {
                // swapExactTokensForTokens
                (_amount, fee) = _swapExactForCarbon(
                    _sourceToken,
                    _poolToken,
                    sourceAmount
                );
            }
        } else {
            _amount = sourceAmount - fee;
        }```

=== Swapping to the pool tokens ===
- We meet the first condition of this statement as USDC <> BCT.
- As the `amountInCarbon` is false, we execute the `swapExactTokensForTokens` function. This takes the total supplied token amount (10 USDC) and swaps all of it into the pool token. When this is done `_amount` is updated to be the value of the pool tokens received. Taking the value of BCT = 2 USDC, we `_amount` would now equal 5 (with decimals)

=== Adding in the fee for unspecified retirement amounts ===
- At this point we only know how many pool tokens we have been able to acquire, we need to calculate and return a fee.
I believe that the issue might lie within the `_amount = totalCarbon - fee;`, as `totalCarbon` in this case did not get properly updated along the way.

[aliad10]

hi @cujowolf.

we agree that in our case this part execute

 (_amount, fee) = _swapExactForCarbon(
                    _sourceToken,
                    _poolToken,
                    sourceAmount
                );

and gives us 4.95 for _amount and 0.05 for the fee.
but the point is that

if (!_amountInCarbon) {
            // Calculate the fee and adjust if pool token is provided with false bool
            fee = (_amount * feeAmount) / 1000;
            _amount = totalCarbon - fee;
        }

also execute. and wrongly update the fee and _amount.

[aliad10]

when

if (!_amountInCarbon) {
            // Calculate the fee and adjust if pool token is provided with false bool
            fee = (_amount * feeAmount) / 1000;
            _amount = totalCarbon - fee;
        }

executes and wrongly updates _amount (that is more than 4.95)
if we consider this part of code

        (_amount, fee) = _prepareRetireSpecific(
            _sourceToken,
            _poolToken,
            _amount,
            _amountInCarbon,
            _retiree
        );

in the retireToucanSpecific function we got wrong value for _amount and fee and try to retire that wrong value.
because we can retire only 4.95 bct but trying to retire something more than 4.95 bct execution must revert.

[cujowolf]

Ah yeah I see that now. Will get working on the fix here.

[aliad10]

great 👍

[mahdiGhorbanzadeh]

You can do something like this (add return in else) @cujowolf

if (_amountInCarbon) {
                // Add redemption fee to the total to redeem.
                totalCarbon += _getSpecificCarbonFee(_poolToken, _amount);
                _amount += _getSpecificCarbonFee(_poolToken, _amount);

                // swapTokensForExactTokens
                _swapForExactCarbon(
                    _sourceToken,
                    _poolToken,
                    totalCarbon,
                    sourceAmount,
                    _retiree
                );
            } else {
                // swapExactTokensForTokens
                (_amount, fee) = _swapExactForCarbon(
                    _sourceToken,
                    _poolToken,
                    sourceAmount
                );
                return(_amount, fee);
            }

or change check like this

if (!_amountInCarbon && _sourceToken == _poolToken) {
            // Calculate the fee and adjust if pool token is provided with false bool
            fee = (_amount * feeAmount) / 1000;
            _amount = totalCarbon - fee;
        }