Skip to content

Commit

Permalink
feat: ensure amountOut's are rounded down and amountIn's are roundedUp (
Browse files Browse the repository at this point in the history 
#562)
  • Loading branch information
@philbow61
philbow61 authored Dec 4, 2024
1 parent 06b02cc commit 20fc515
Show file tree
Hide file tree
Showing 2 changed files with 287 additions and 2 deletions.
14 changes: 12 additions & 2 deletions contracts/goodDollar/BancorExchangeProvider.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ contract BancorExchangeProvider is IExchangeProvider, IBancorExchangeProvider, B
require(scaledAmountIn < exchange.tokenSupply, "amountIn is greater than tokenSupply");
}

amountIn = scaledAmountIn / tokenPrecisionMultipliers[tokenIn];
amountIn = divAndRoundUp(scaledAmountIn, tokenPrecisionMultipliers[tokenIn]);
return amountIn;
}

Expand Down Expand Up @@ -261,7 +261,7 @@ contract BancorExchangeProvider is IExchangeProvider, IBancorExchangeProvider, B
_accountExitContribution(exchangeId, exitContribution);
}

amountIn = scaledAmountInWithExitContribution / tokenPrecisionMultipliers[tokenIn];
amountIn = divAndRoundUp(scaledAmountInWithExitContribution, tokenPrecisionMultipliers[tokenIn]);
return amountIn;
}

Expand Down Expand Up @@ -359,6 +359,16 @@ contract BancorExchangeProvider is IExchangeProvider, IBancorExchangeProvider, B
exchanges[exchangeId].tokenSupply -= exitContribution;
}

/**
* @notice Division and rounding up if there is a remainder
* @param a The dividend
* @param b The divisor
* @return The result of the division rounded up
*/
function divAndRoundUp(uint256 a, uint256 b) internal pure returns (uint256) {
return (a / b) + (a % b > 0 ? 1 : 0);
}

/**
* @notice Calculate the scaledAmountIn of tokenIn for a given scaledAmountOut of tokenOut
* @param exchange The pool exchange to operate on
Expand Down
275 changes: 275 additions & 0 deletions test/unit/goodDollar/BancorExchangeProvider.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -968,6 +968,88 @@ contract BancorExchangeProviderTest_getAmountIn is BancorExchangeProviderTest {
// we allow up to 1% difference due to precision loss
assertApproxEqRel(reversedAmountOut, amountOut, 1e18 * 0.01);
}

function test_getAmountIn_whenTokenInIsTokenWith6TokenDecimals_shouldRoundUpInFavorOfReserve() public {
bytes32 exchangeId6Decimals = bancorExchangeProvider.createExchange(poolExchange4);

uint256 amountOut = 55e18;
uint256 amountIn6Decimals = bancorExchangeProvider.getAmountIn({
exchangeId: exchangeId6Decimals,
tokenIn: address(tokenWith6Decimals),
tokenOut: address(reserveToken),
amountOut: amountOut
});

bancorExchangeProvider.destroyExchange(exchangeId6Decimals, 0);
bytes32 exchangeId18Decimals = bancorExchangeProvider.createExchange(poolExchange1);

uint256 amountIn18Decimals = bancorExchangeProvider.getAmountIn({
exchangeId: exchangeId18Decimals,
tokenIn: address(token),
tokenOut: address(reserveToken),
amountOut: amountOut
});

assertTrue(amountIn18Decimals <= amountIn6Decimals * 1e12);
assertEq(amountIn6Decimals, (amountIn18Decimals / 1e12) + 1);
}

function test_getAmountIn_whenTokenInIsReserveAssetWith6TokenDecimals_shouldRoundUpInFavorOfReserve() public {
bytes32 exchangeId6Decimals = bancorExchangeProvider.createExchange(poolExchange3);

uint256 amountOut = 55e18;
uint256 amountIn6Decimals = bancorExchangeProvider.getAmountIn({
exchangeId: exchangeId6Decimals,
tokenIn: address(reserveTokenWith6Decimals),
tokenOut: address(token),
amountOut: amountOut
});

bancorExchangeProvider.destroyExchange(exchangeId6Decimals, 0);
bytes32 exchangeId18Decimals = bancorExchangeProvider.createExchange(poolExchange1);

uint256 amountIn18Decimals = bancorExchangeProvider.getAmountIn({
exchangeId: exchangeId18Decimals,
tokenIn: address(reserveToken),
tokenOut: address(token),
amountOut: amountOut
});

assertTrue(amountIn18Decimals <= amountIn6Decimals * 1e12);
assertEq(amountIn6Decimals, (amountIn18Decimals / 1e12) + 1);
}

function test_getAmountIn_whenTokenInHas6DecimalsButNoRoundingNeeded_shouldNotRoundUp() public {
bytes32 exchangeId = bancorExchangeProvider.createExchange(poolExchange1);

uint256 amountOut = bancorExchangeProvider.getAmountOut({
exchangeId: exchangeId,
tokenIn: address(token),
tokenOut: address(reserveToken),
amountIn: 15e17
});

uint256 amountIn18Decimals = bancorExchangeProvider.getAmountIn({
exchangeId: exchangeId,
tokenIn: address(token),
tokenOut: address(reserveToken),
amountOut: amountOut
});

assertEq(amountIn18Decimals, 15e17);

bancorExchangeProvider.destroyExchange(exchangeId, 0);
bytes32 exchangeId6Decimals = bancorExchangeProvider.createExchange(poolExchange4);

uint256 amountIn6Decimals = bancorExchangeProvider.getAmountIn({
exchangeId: exchangeId6Decimals,
tokenIn: address(tokenWith6Decimals),
tokenOut: address(reserveToken),
amountOut: amountOut
});

assertEq(amountIn6Decimals, amountIn18Decimals / 1e12);
}
}

contract BancorExchangeProviderTest_getAmountOut is BancorExchangeProviderTest {
Expand Down Expand Up @@ -1455,6 +1537,56 @@ contract BancorExchangeProviderTest_getAmountOut is BancorExchangeProviderTest {
// we allow up to 1% difference due to precision loss
assertApproxEqRel(reversedAmountIn, amountIn, 1e18 * 0.01);
}

function test_getAmountOut_whenTokenOutIsTokenWith6TokenDecimals_shouldRoundDownInFavorOfReserve() public {
bytes32 exchangeId6Decimals = bancorExchangeProvider.createExchange(poolExchange4);

uint256 amountIn = 55e18;
uint256 amountOut6Decimals = bancorExchangeProvider.getAmountOut({
exchangeId: exchangeId6Decimals,
tokenIn: address(reserveToken),
tokenOut: address(tokenWith6Decimals),
amountIn: amountIn
});

bancorExchangeProvider.destroyExchange(exchangeId6Decimals, 0);
bytes32 exchangeId18Decimals = bancorExchangeProvider.createExchange(poolExchange1);

uint256 amountOut18Decimals = bancorExchangeProvider.getAmountOut({
exchangeId: exchangeId18Decimals,
tokenIn: address(reserveToken),
tokenOut: address(token),
amountIn: amountIn
});

assertTrue(amountOut6Decimals * 1e12 < amountOut18Decimals);
assertEq(amountOut6Decimals, (amountOut18Decimals / 1e12));
}

function test_getAmountOut_whenTokenOutIsReserveAssetWith6TokenDecimals_shouldRoundDownInFavorOfReserve() public {
bytes32 exchangeId6Decimals = bancorExchangeProvider.createExchange(poolExchange3);

uint256 amountIn = 55e18;
uint256 amountOut6Decimals = bancorExchangeProvider.getAmountOut({
exchangeId: exchangeId6Decimals,
tokenIn: address(token),
tokenOut: address(reserveTokenWith6Decimals),
amountIn: amountIn
});

bancorExchangeProvider.destroyExchange(exchangeId6Decimals, 0);
bytes32 exchangeId18Decimals = bancorExchangeProvider.createExchange(poolExchange1);

uint256 amountOut18Decimals = bancorExchangeProvider.getAmountOut({
exchangeId: exchangeId18Decimals,
tokenIn: address(token),
tokenOut: address(reserveToken),
amountIn: amountIn
});

assertTrue(amountOut6Decimals * 1e12 < amountOut18Decimals);
assertEq(amountOut6Decimals, (amountOut18Decimals / 1e12));
}
}

contract BancorExchangeProviderTest_currentPrice is BancorExchangeProviderTest {
Expand Down Expand Up @@ -1697,6 +1829,60 @@ contract BancorExchangeProviderTest_swapIn is BancorExchangeProviderTest {
// we allow up to 0.1% difference due to precision loss on exitContribution accounting
assertApproxEqRel(amountOutInOneSell, amountOutInMultipleSells, 1e18 * 0.001);
}

function test_swapIn_whenTokenOutIsTokenWith6Decimals_shouldRoundDownInFavorOfReserve() public {
BancorExchangeProvider bancorExchangeProvider = initializeBancorExchangeProvider();
uint256 amountIn = 55e18;

bytes32 exchangeId6Decimals = bancorExchangeProvider.createExchange(poolExchange4);
vm.prank(brokerAddress);
uint256 amountOut6Decimals = bancorExchangeProvider.swapIn(
exchangeId6Decimals,
address(reserveToken),
address(tokenWith6Decimals),
amountIn
);

bancorExchangeProvider.destroyExchange(exchangeId6Decimals, 0);
bytes32 exchangeId18Decimals = bancorExchangeProvider.createExchange(poolExchange1);
vm.prank(brokerAddress);
uint256 amountOut18Decimals = bancorExchangeProvider.swapIn(
exchangeId18Decimals,
address(reserveToken),
address(token),
amountIn
);

assertTrue(amountOut6Decimals * 1e12 < amountOut18Decimals);
assertEq(amountOut6Decimals, (amountOut18Decimals / 1e12));
}

function test_swapIn_whenTokenOutIsReserveAssetWith6Decimals_shouldRoundDownInFavorOfReserve() public {
BancorExchangeProvider bancorExchangeProvider = initializeBancorExchangeProvider();
uint256 amountIn = 55e18;

bytes32 exchangeId6Decimals = bancorExchangeProvider.createExchange(poolExchange3);
vm.prank(brokerAddress);
uint256 amountOut6Decimals = bancorExchangeProvider.swapIn(
exchangeId6Decimals,
address(token),
address(reserveTokenWith6Decimals),
amountIn
);

bancorExchangeProvider.destroyExchange(exchangeId6Decimals, 0);
bytes32 exchangeId18Decimals = bancorExchangeProvider.createExchange(poolExchange1);
vm.prank(brokerAddress);
uint256 amountOut18Decimals = bancorExchangeProvider.swapIn(
exchangeId18Decimals,
address(token),
address(reserveToken),
amountIn
);

assertTrue(amountOut6Decimals * 1e12 < amountOut18Decimals);
assertEq(amountOut6Decimals, (amountOut18Decimals / 1e12));
}
}

contract BancorExchangeProviderTest_swapOut is BancorExchangeProviderTest {
Expand Down Expand Up @@ -1875,4 +2061,93 @@ contract BancorExchangeProviderTest_swapOut is BancorExchangeProviderTest {
// we allow up to 0.1% difference due to precision loss on exitContribution accounting
assertApproxEqRel(amountInInOneBuy, amountInInMultipleBuys, 1e18 * 0.001);
}

function test_swapOut_whenTokenInIsTokenWith6Decimals_shouldRoundUpInFavorOfReserve() public {
BancorExchangeProvider bancorExchangeProvider = initializeBancorExchangeProvider();
uint256 amountOut = 55e18;

bytes32 exchangeId6Decimals = bancorExchangeProvider.createExchange(poolExchange4);
vm.prank(brokerAddress);
uint256 amountIn6Decimals = bancorExchangeProvider.swapOut(
exchangeId6Decimals,
address(tokenWith6Decimals),
address(reserveToken),
amountOut
);

bancorExchangeProvider.destroyExchange(exchangeId6Decimals, 0);
bytes32 exchangeId18Decimals = bancorExchangeProvider.createExchange(poolExchange1);
vm.prank(brokerAddress);
uint256 amountIn18Decimals = bancorExchangeProvider.swapOut(
exchangeId18Decimals,
address(token),
address(reserveToken),
amountOut
);

assertTrue(amountIn18Decimals < amountIn6Decimals * 1e12);
assertEq(amountIn6Decimals, (amountIn18Decimals / 1e12) + 1);
}

function test_swapOut_whenTokenInIsReserveAssetWith6Decimals_shouldRoundUpInFavorOfReserve() public {
BancorExchangeProvider bancorExchangeProvider = initializeBancorExchangeProvider();
uint256 amountOut = 55e18;

bytes32 exchangeId6Decimals = bancorExchangeProvider.createExchange(poolExchange3);
vm.prank(brokerAddress);
uint256 amountIn6Decimals = bancorExchangeProvider.swapOut(
exchangeId6Decimals,
address(reserveTokenWith6Decimals),
address(token),
amountOut
);

bancorExchangeProvider.destroyExchange(exchangeId6Decimals, 0);
bytes32 exchangeId18Decimals = bancorExchangeProvider.createExchange(poolExchange1);
vm.prank(brokerAddress);
uint256 amountIn18Decimals = bancorExchangeProvider.swapOut(
exchangeId18Decimals,
address(reserveToken),
address(token),
amountOut
);

assertTrue(amountIn18Decimals < amountIn6Decimals * 1e12);
assertEq(amountIn6Decimals, (amountIn18Decimals / 1e12) + 1);
}

function test_swapOut_whenTokenInHas6DecimalsButNoRoundingNeeded_shouldNotRoundUp() public {
BancorExchangeProvider bancorExchangeProvider = initializeBancorExchangeProvider();
bytes32 exchangeId = bancorExchangeProvider.createExchange(poolExchange1);

uint256 amountOut = bancorExchangeProvider.getAmountOut({
exchangeId: exchangeId,
tokenIn: address(token),
tokenOut: address(reserveToken),
amountIn: 15e17
});

vm.prank(brokerAddress);
uint256 amountIn18Decimals = bancorExchangeProvider.swapOut(
exchangeId,
address(token),
address(reserveToken),
amountOut
);

assertEq(amountIn18Decimals, 15e17);

bancorExchangeProvider.destroyExchange(exchangeId, 0);
exchangeId = bancorExchangeProvider.createExchange(poolExchange4);

vm.prank(brokerAddress);
uint256 amountIn6Decimals = bancorExchangeProvider.swapOut(
exchangeId,
address(tokenWith6Decimals),
address(reserveToken),
amountOut
);

assertEq(amountIn6Decimals, amountIn18Decimals / 1e12);
}
}

0 comments on commit 20fc515

Please sign in to comment.