Summary
The LibUnripe library contains a function unripeToUnderlying
that converts unripe tokens to their underlying value. This function is used in the BDVFacet contract to calculate the Bean Denominated Value (BDV) of unripe LP tokens and unripe Beans. Due to Solidity's integer division, this conversion introduces a rounding error that can lead to inconsistent BDV calculations and potential value loss for users.
https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/8c8710df547f7d7c5dd82c5381eb6b34532e4484/protocol/contracts/libraries/LibUnripe.sol#L85C3-L94C6
https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/8c8710df547f7d7c5dd82c5381eb6b34532e4484/protocol/contracts/beanstalk/silo/BDVFacet.sol#L30C3-L38C6
In LibUnripe.sol
function unripeToUnderlying(
address unripeToken,
uint256 unripe,
uint256 supply
) internal view returns (uint256 underlying) {
AppStorage storage s = LibAppStorage.diamondStorage();
underlying = s.sys.silo.unripeSettings[unripeToken].balanceOfUnderlying.mul(unripe).div(
supply
);
}
In BDVFacet.sol
function unripeLPToBDV(uint256 amount) public view returns (uint256) {
amount = LibUnripe.unripeToUnderlying(
C.UNRIPE_LP,
amount,
IBean(C.UNRIPE_LP).totalSupply()
);
amount = LibWellBdv.bdv(LibBarnRaise.getBarnRaiseWell(), amount);
return amount;
}
Proof of Concept
Consider the following scenario:
Calculation:
1000 * 3 = 3000
3000 / 7 = 428 (rounded down from 428.571428...)
The function returns 428 underlying tokens instead of the mathematically correct 428.571428..., resulting in a loss of 0.571428... tokens (approximately 0.13% of the correct value).
Coded PoC
pragma solidity ^0.8.20;
import "forge-std/Test.sol";
import "../contracts/libraries/LibUnripe.sol";
import "../contracts/BDVFacet.sol";
contract UnripeConversionTest is Test {
LibUnripe libUnripe;
BDVFacet bdvFacet;
function setUp() public {
libUnripe = new LibUnripe();
bdvFacet = new BDVFacet();
}
function testUnripeToUnderlyingRoundingError() public {
uint256 balanceOfUnderlying = 1000;
uint256 unripe = 3;
uint256 supply = 7;
uint256 result = libUnripe.unripeToUnderlying(address(0), unripe, supply);
uint256 expectedExact = (balanceOfUnderlying * unripe * 1e18) / supply;
uint256 actualScaled = result * 1e18;
assertLt(actualScaled, expectedExact, "Result should be less than mathematically exact value");
assertGt(expectedExact - actualScaled, 0.1e18, "Error should be greater than 0.1");
assertLt(expectedExact - actualScaled, 0.6e18, "Error should be less than 0.6");
}
function testBDVCalculationDiscrepancy() public {
uint256 amount = 1000000;
uint256 bdv1 = bdvFacet.unripeLPToBDV(amount);
uint256 bdv2 = bdvFacet.unripeLPToBDV(amount / 2) * 2;
assertNotEq(bdv1, bdv2, "BDV calculations should not be exactly equal due to rounding");
assertLt(absDifference(bdv1, bdv2), amount / 1000, "Difference should be less than 0.1%");
}
function absDifference(uint256 a, uint256 b) internal pure returns (uint256) {
return a > b ? a - b : b - a;
}
}
These tests demonstrate the rounding error in unripeToUnderlying
and its impact on BDV calculations. The first test shows that the conversion result is less than the mathematically exact value, with an error between 0.1 and 0.6 tokens per 1000 underlying. The second test reveals that calculating BDV for a large amount all at once versus in two halves can lead to discrepancies, albeit small ones.
Impact
1: The rounding error can lead to slightly different BDV values for the same amount of tokens in different transactions.
2: Users may receive slightly less value than expected when converting unripe tokens.
Tools Used
Manual Review
Recommendations
1: Consider using a fixed-point arithmetic library like PRBMath for more precise calculations.
2: Alternatively, increase precision by scaling up values before division and scaling down afterward:
function unripeToUnderlying(
address unripeToken,
uint256 unripe,
uint256 supply
) internal view returns (uint256 underlying) {
AppStorage storage s = LibAppStorage.diamondStorage();
underlying = s.sys.silo.unripeSettings[unripeToken].balanceOfUnderlying.mul(unripe).mul(1e18).div(supply);
underlying = underlying.div(1e18);
}