From cf41c472c6ee146db2440b2a2a567aada40184b4 Mon Sep 17 00:00:00 2001 From: domenicodev Date: Wed, 17 Jan 2024 15:24:18 +0100 Subject: [PATCH] fix: Initial code review fixes --- evm/src/integral/IntegralSwapAdapter.sol | 50 ++++++++++++++++-------- evm/test/IntegralSwapAdapter.t.sol | 19 --------- 2 files changed, 34 insertions(+), 35 deletions(-) diff --git a/evm/src/integral/IntegralSwapAdapter.sol b/evm/src/integral/IntegralSwapAdapter.sol index 85e9037..67ab0bb 100644 --- a/evm/src/integral/IntegralSwapAdapter.sol +++ b/evm/src/integral/IntegralSwapAdapter.sol @@ -8,6 +8,7 @@ import {SafeERC20} from "openzeppelin-contracts/contracts/token/ERC20/utils/Safe /// @dev Integral submitted deadline of 3600 seconds (1 hour) to Paraswap, but it is not strictly necessary to be this long /// as the contract allows less durations, we use 1000 seconds (15 minutes) as a deadline uint256 constant SWAP_DEADLINE_SEC = 1000; +uint256 constant STANDARD_TOKEN_DECIMALS = 10**18; /// @title Integral Swap Adapter contract IntegralSwapAdapter is ISwapAdapter { @@ -33,12 +34,43 @@ contract IntegralSwapAdapter is ISwapAdapter { uint256[] memory _specifiedAmounts ) external view override returns (Fraction[] memory _prices) { _prices = new Fraction[](_specifiedAmounts.length); + uint256 price = getPriceAt(address(_sellToken), address(_buyToken)); for (uint256 i = 0; i < _specifiedAmounts.length; i++) { - _prices[i] = getPriceAt(address(_sellToken), address(_buyToken)); + _prices[i] = price; } } + /// @notice Get swap price including fee + /// @param sellToken token to sell + /// @param buyToken token to buy + function getPriceAt(address sellToken, address buyToken) internal view returns(Fraction memory) { + uint256 priceWithoutFee = relayer.getPriceByTokenAddresses(address(sellToken), address(buyToken)); + ITwapFactory factory = ITwapFactory(relayer.factory()); + address pairAddress = factory.getPair(address(sellToken), address(buyToken)); + + // get swapFee formatted; swapFee is a constant + uint256 swapFeeFormatted = (STANDARD_TOKEN_DECIMALS - relayer.swapFee(pairAddress)); + + // get token decimals + uint256 sellTokenDecimals = 10**ERC20(sellToken).decimals(); + uint256 buyTokenDecimals = 10**ERC20(buyToken).decimals(); + + /** + * @dev + * Denominator works as a "standardizer" for the price rather than a reserve value + * as Integral takes prices from oracles and do not operate with reserves; + * it is therefore used to maintain integrity for the Fraction division, + * as numerator and denominator could have different token decimals(es. ETH(18)-USDC(6)). + * Both numerator and denominator are also multiplied by STANDARD_TOKEN_DECIMALS + * to ensure that precision losses are minimized or null. + */ + return Fraction( + priceWithoutFee * STANDARD_TOKEN_DECIMALS, + STANDARD_TOKEN_DECIMALS * sellTokenDecimals * swapFeeFormatted / buyTokenDecimals + ); + } + /// @inheritdoc ISwapAdapter function swap( bytes32, @@ -154,7 +186,7 @@ contract IntegralSwapAdapter is ISwapAdapter { to: msg.sender, submitDeadline: uint32(block.timestamp + SWAP_DEADLINE_SEC), amountIn: amount, - amountOutMin: amountOut + amountOutMin: 0 })); return amountOut; @@ -190,20 +222,6 @@ contract IntegralSwapAdapter is ISwapAdapter { return amountIn; } - - /// @notice Get swap price including fee - /// @param sellToken token to sell - /// @param buyToken token to buy - function getPriceAt(address sellToken, address buyToken) internal view returns(Fraction memory) { - uint256 priceWithoutFee = relayer.getPriceByTokenAddresses(address(sellToken), address(buyToken)); - ITwapFactory factory = ITwapFactory(relayer.factory()); - address pairAddress = factory.getPair(address(sellToken), address(buyToken)); - - return Fraction( - priceWithoutFee * 10**18, - 10**(ERC20(sellToken).decimals()) * 10**18 * (10**18 - relayer.swapFee(pairAddress)) / 10**(ERC20(buyToken).decimals()) - ); - } } interface ITwapRelayer { diff --git a/evm/test/IntegralSwapAdapter.t.sol b/evm/test/IntegralSwapAdapter.t.sol index 65b8fd5..3d41885 100644 --- a/evm/test/IntegralSwapAdapter.t.sol +++ b/evm/test/IntegralSwapAdapter.t.sol @@ -49,25 +49,6 @@ contract IntegralSwapAdapterTest is Test, ISwapAdapterTypes { assertGt(prices[i].denominator, 0); } } - - function testPriceKeepingIntegral() public { - bytes32 pair = bytes32(bytes20(USDC_WETH_PAIR)); - uint256[] memory amounts = new uint256[](TEST_ITERATIONS); - - for (uint256 i = 0; i < TEST_ITERATIONS; i++) { - amounts[i] = 1000 * i * 10 ** 15; - } - - Fraction[] memory prices = adapter.price(pair, WETH, USDC, amounts); - - for (uint256 i = 0; i < TEST_ITERATIONS - 1; i++) { - Fraction memory reducedPrice0 = Fraction(prices[i].numerator / 10**18, prices[i].denominator / 10**18); - Fraction memory reducedPrice1 = Fraction(prices[i + 1].numerator / 10**18, prices[i + 1].denominator / 10**18); - assertEq(reducedPrice0.compareFractions(reducedPrice1), 0); - assertGt(prices[i].denominator, 0); - assertGt(prices[i + 1].denominator, 0); - } - } /// @dev Since TwapRelayer's calculateAmountOut function is internal, and using quoteSell would /// revert the transaction if calculateAmountOut is not enough,