-
Notifications
You must be signed in to change notification settings - Fork 4
Add dxswapSwapRelayer #39
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Changes from 2 commits
21f7717
62a5d76
4d046f0
ef70f77
d6e4256
79c0f6c
a220bb7
4f8d2ca
18fcd32
cb28c9a
555cf8a
e0c18c4
b676208
c130ba4
f4dc987
191539a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,296 @@ | ||
| pragma solidity =0.6.6; | ||
| pragma experimental ABIEncoderV2; | ||
|
|
||
| import './OracleCreator.sol'; | ||
| import './../interfaces/IDXswapFactory.sol'; | ||
| import './../interfaces/IDXswapRouter.sol'; | ||
| import './../libraries/TransferHelper.sol'; | ||
| import './../interfaces/IERC20.sol'; | ||
| import './../interfaces/IWETH.sol'; | ||
| import './../libraries/SafeMath.sol'; | ||
| import './../libraries/DXswapLibrary.sol'; | ||
|
|
||
| contract DXswapSwapRelayer { | ||
| using SafeMath for uint256; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When the Solidity version is upgraded, we can remove |
||
|
|
||
| event NewOrder(uint256 indexed _orderIndex); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd remove the underscore here, in all event param names |
||
|
|
||
| event ExecutedOrder(uint256 indexed _orderIndex); | ||
|
|
||
| event WithdrawnExpiredOrder(uint256 indexed _orderIndex); | ||
|
|
||
| event OwnershipTransferred(address indexed previousOwner, address indexed newOwner); | ||
|
|
||
| event Details(uint256 amountIn, uint256 amountOut); | ||
|
|
||
| struct Order { | ||
| address tokenIn; | ||
| address tokenOut; | ||
| uint256 amountIn; | ||
| uint256 priceTolerance; | ||
| uint256 minReserveA; | ||
| uint256 minReserveB; | ||
| address oraclePair; | ||
| uint256 deadline; | ||
| uint256 maxWindowTime; | ||
| uint256 oracleId; | ||
| address factory; | ||
| bool executed; | ||
| } | ||
|
|
||
| uint256 public immutable GAS_ORACLE_UPDATE = 168364; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure about this. The underlying protocol can change and call gas costs might be different in the future... using |
||
| uint256 public immutable PARTS_PER_MILLION = 1000000; | ||
| uint256 public immutable BOUNTY = 0.01 ether; // To be decided | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Calculating the bounty dinamically would probably be best. Let's say as a percentage of the fee used to update the oracle. |
||
| uint8 public immutable PROVISION = 1; | ||
| uint8 public immutable REMOVAL = 2; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Pretty much all of the above state variables are constants, so it's heavily suggested to use |
||
|
|
||
| address public immutable dxSwapFactory; | ||
| address public immutable dxSwapRouter; | ||
| address public immutable uniswapFactory; | ||
| address public immutable uniswapRouter; | ||
| address public immutable WETH; | ||
|
|
||
| OracleCreator oracleCreator; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Visibility specifier missing. Should this be internal? |
||
| uint256 public orderCount; | ||
| mapping(uint256 => Order) orders; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as above. |
||
| address payable public owner; | ||
|
|
||
| constructor( | ||
| address payable _owner, | ||
| address _dxSwapFactory, | ||
| address _dxSwapRouter, | ||
| address _uniswapFactory, | ||
| address _uniswapRouter, | ||
| address _WETH, | ||
| OracleCreator _oracleCreator | ||
| ) public { | ||
| owner = _owner; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are we ok with no validation? |
||
| dxSwapFactory = _dxSwapFactory; | ||
| dxSwapRouter = _dxSwapRouter; | ||
| uniswapFactory = _uniswapFactory; | ||
| uniswapRouter = _uniswapRouter; | ||
| WETH = _WETH; | ||
| oracleCreator = _oracleCreator; | ||
| } | ||
|
|
||
| function createSwapOrder( | ||
| address tokenA, | ||
| address tokenB, | ||
| uint256 amountInTokenA, | ||
| uint256 amountInTokenB, | ||
| uint256 priceTolerance, | ||
| uint256 minReserveA, | ||
| uint256 minReserveB, | ||
| uint256 maxWindowTime, | ||
| uint256 deadline, | ||
| address factory | ||
| ) external payable returns (uint256 orderIndex) { | ||
| require(factory == dxSwapFactory || factory == uniswapFactory, 'DXswapRelayer: INVALID_FACTORY'); | ||
| require(msg.sender == owner, 'DXswapRelayer: CALLER_NOT_OWNER'); | ||
| require(tokenA != tokenB, 'DXswapRelayer: INVALID_PAIR'); | ||
| require(tokenA < tokenB, 'DXswapRelayer: INVALID_TOKEN_ORDER'); | ||
| // only one token amount can be > 0 and second token amount is calculated as output | ||
| require( | ||
| (amountInTokenA > 0 && amountInTokenB == 0) || (amountInTokenA == 0 && amountInTokenB > 0), | ||
| 'DXswapRelayer: INVALID_TOKEN_AMOUNT' | ||
| ); | ||
| require(priceTolerance <= PARTS_PER_MILLION, 'DXswapRelayer: INVALID_TOLERANCE'); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. to consult with John: it may require that priceTolerance is at least 0.3% as this the fee uniswap charges. |
||
| require(block.timestamp <= deadline, 'DXswapRelayer: DEADLINE_REACHED'); | ||
| require(maxWindowTime > 30, 'DXswapRelayer: INVALID_WINDOWTIME'); | ||
|
|
||
| if (tokenA == address(0)) { | ||
| require(address(this).balance >= amountInTokenA, 'DXswapRelayer: INSUFFICIENT_ETH'); | ||
| } else { | ||
| require(IERC20(tokenA).balanceOf(address(this)) >= amountInTokenA, 'DXswapRelayer: INSUFFICIENT_TOKEN_A'); | ||
| } | ||
| require(IERC20(tokenB).balanceOf(address(this)) >= amountInTokenB, 'DXswapRelayer: INSUFFICIENT_TOKEN_B'); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If only one of the 2 tokens can have an amount > 0 (as per one of the requires above), we can shave off some gas by only checking the balance in the non-zero token amount (the other will always be 0). |
||
|
|
||
| address pair = _pair(tokenA, tokenB, factory); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could actually call the factory to get the pair address based on the 2 tokens that are into it. This will also let us know if the pair exists or not. If it doesn't we should abort here. |
||
|
|
||
| require(pair != address(0), 'DXswapRelayer: INVALID_PAIR_ADDRESS'); | ||
|
|
||
| orderIndex = _OrderIndex(); | ||
| orders[orderIndex] = Order({ | ||
| tokenIn: amountInTokenA > 0 ? tokenA : tokenB, | ||
| tokenOut: amountInTokenA > 0 ? tokenB : tokenA, | ||
| amountIn: amountInTokenA > 0 ? amountInTokenA : amountInTokenB, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These checks can all be collapsed into one, saving some gas. |
||
| priceTolerance: priceTolerance, | ||
| minReserveA: minReserveA, | ||
| minReserveB: minReserveB, | ||
| oraclePair: pair, | ||
| deadline: deadline, | ||
| maxWindowTime: maxWindowTime, | ||
| oracleId: 0, | ||
| factory: factory, | ||
| executed: false | ||
| }); | ||
|
|
||
| (uint256 reserveA, uint256 reserveB, ) = IDXswapPair(pair).getReserves(); | ||
|
|
||
| /* Create an oracle to calculate average price before swap */ | ||
| uint256 windowTime = _consultOracleParameters( | ||
| amountInTokenA, | ||
| amountInTokenB, | ||
| reserveA, | ||
| reserveB, | ||
| maxWindowTime | ||
| ); | ||
| orders[orderIndex].oracleId = oracleCreator.createOracle(windowTime, pair); | ||
| emit NewOrder(orderIndex); | ||
| } | ||
|
|
||
| function executeOrder(uint256 orderIndex) external { | ||
| Order storage order = orders[orderIndex]; | ||
| require(orderIndex < orderCount, 'DXswapRelayer: INVALID_ORDER'); | ||
| require(!order.executed, 'DXswapRelayer: ORDER_EXECUTED'); | ||
| require(oracleCreator.isOracleFinalized(order.oracleId), 'DXswapRelayer: OBSERVATION_RUNNING'); | ||
| require(block.timestamp <= order.deadline, 'DXswapRelayer: DEADLINE_REACHED'); | ||
|
|
||
| address tokenIn = order.tokenIn; | ||
| uint256 amountOut; | ||
| amountOut = oracleCreator.consult( | ||
| order.oracleId, | ||
| tokenIn == address(0) ? IDXswapRouter(dxSwapRouter).WETH() : tokenIn, | ||
| order.amountIn | ||
| ); | ||
|
|
||
| uint256 minAmountOut = amountOut.sub(amountOut.mul(order.priceTolerance) / PARTS_PER_MILLION); | ||
|
|
||
| order.executed = true; | ||
|
|
||
| _swap(tokenIn, order.tokenOut, order.amountIn, minAmountOut, order.factory); | ||
|
|
||
| emit ExecutedOrder(orderIndex); | ||
| } | ||
|
|
||
| // Updates a price oracle and sends a bounty to msg.sender | ||
| function updateOracle(uint256 orderIndex) external { | ||
| Order storage order = orders[orderIndex]; | ||
| require(block.timestamp <= order.deadline, 'DXswapRelayer: DEADLINE_REACHED'); | ||
| require(!oracleCreator.isOracleFinalized(order.oracleId), 'DXswapRelayer: OBSERVATION_ENDED'); | ||
| uint256 amountBounty = GAS_ORACLE_UPDATE.mul(tx.gasprice).add(BOUNTY); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd maybe cap the gas price to
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice catch! Agree that would be good to use basefee, but to do that we need to upgrade Solidity, because |
||
|
|
||
| (uint256 reserveA, uint256 reserveB, ) = IDXswapPair(order.oraclePair).getReserves(); | ||
| require(reserveA >= order.minReserveA && reserveB >= order.minReserveB, 'DXswapRelayer: RESERVE_TOO_LOW'); | ||
|
|
||
| oracleCreator.update(order.oracleId); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm wondering what happens here if the reserves have changed and in turn oracle parameters are potentially not safe anymore? I.e. someone removes liquidity without it being brought below minimum levels, but changing the oracle's window time as calculated in
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it would be ok indeed to consult the oracle and check whether window time has changed or not. Let's keep in minds that the same solution is used for DXswapRelayer contract, so if we consider the oracle might be not safe we will have to change also that one, @jpkcambridge? |
||
| if (address(this).balance >= amountBounty) { | ||
| TransferHelper.safeTransferETH(msg.sender, amountBounty); | ||
| } | ||
| } | ||
|
|
||
| function withdrawExpiredOrder(uint256 orderIndex) external { | ||
| Order storage order = orders[orderIndex]; | ||
| require(msg.sender == owner, 'DXswapRelayer: CALLER_NOT_OWNER'); | ||
| require(block.timestamp > order.deadline, 'DXswapRelayer: DEADLINE_NOT_REACHED'); | ||
| require(order.executed == false, 'DXswapRelayer: ORDER_EXECUTED'); | ||
| address tokenIn = order.tokenIn; | ||
| uint256 amountIn = order.amountIn; | ||
| order.executed = true; | ||
|
|
||
| if (tokenIn == address(0)) { | ||
| TransferHelper.safeTransferETH(owner, amountIn); | ||
| } else { | ||
| TransferHelper.safeTransfer(tokenIn, owner, amountIn); | ||
| } | ||
| emit WithdrawnExpiredOrder(orderIndex); | ||
| } | ||
|
|
||
| function _swap( | ||
| address _tokenIn, | ||
| address _tokenOut, | ||
| uint256 _amountIn, | ||
| uint256 _minAmountOut, | ||
| address _factory | ||
| ) internal { | ||
| uint256[] memory amounts; | ||
| address[] memory path = new address[](2); | ||
| address swapRouter; | ||
|
|
||
| if (_tokenIn == address(0)) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Preeeetty sure we can leave this to the router and do a check instead. If |
||
| IWETH(WETH).deposit{value: _amountIn}(); | ||
| _tokenIn = WETH; | ||
| } | ||
|
|
||
| path[0] = _tokenIn; | ||
| path[1] = _tokenOut; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we intend to always do direct swaps without hops? If not, we could manually specify the path when creating the order. It gives us more flexibility.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As I was informed we want to do only direct swaps and looking for better paths is out of scope of this contract. |
||
|
|
||
| if (_factory == dxSwapFactory) { | ||
| swapRouter = dxSwapRouter; | ||
| } else if (_factory == uniswapFactory) { | ||
| swapRouter = uniswapRouter; | ||
| } | ||
|
|
||
| TransferHelper.safeApprove(_tokenIn, swapRouter, _amountIn); | ||
| TransferHelper.safeApprove(_tokenOut, swapRouter, _minAmountOut); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think this approval is necessary? The router only needs to take the |
||
|
|
||
| amounts = IDXswapRouter(swapRouter).swapExactTokensForTokens( | ||
| _amountIn, | ||
| _minAmountOut, | ||
| path, | ||
| address(this), | ||
| block.timestamp | ||
| ); | ||
| TransferHelper.safeApprove(_tokenIn, swapRouter, 0); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This shouldn't be necessary. It depends on the token really, but 90% of them automatically subtract the allowance when "used up". Plus, we're allowing pretty damn safe contracts (Uniswap-like routers only).
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You're right. That was left just "to be sure" about allowance, but can be removed as you explained. |
||
| TransferHelper.safeApprove(_tokenOut, swapRouter, 0); | ||
| } | ||
|
|
||
| // Internal function to calculate the optimal time window for price observation | ||
| function _consultOracleParameters( | ||
| uint256 amountA, | ||
| uint256 amountB, | ||
| uint256 reserveA, | ||
| uint256 reserveB, | ||
| uint256 maxWindowTime | ||
| ) internal view returns (uint256 windowTime) { | ||
| if (reserveA > 0 && reserveB > 0) { | ||
| uint256 poolStake = (amountA.add(amountB)).mul(PARTS_PER_MILLION) / reserveA.add(reserveB); | ||
| // poolStake: 0.1% = 1000; 1=10000; 10% = 100000; | ||
| if (poolStake < 1000) { | ||
| windowTime = 30; | ||
| } else if (poolStake < 2500) { | ||
| windowTime = 60; | ||
| } else if (poolStake < 5000) { | ||
| windowTime = 90; | ||
| } else if (poolStake < 10000) { | ||
| windowTime = 120; | ||
| } else { | ||
| windowTime = 150; | ||
| } | ||
| windowTime = windowTime <= maxWindowTime ? windowTime : maxWindowTime; | ||
| } else { | ||
| windowTime = maxWindowTime; | ||
| } | ||
| } | ||
|
|
||
| // Internal function to return the correct pair address on either DXswap or Uniswap | ||
| function _pair( | ||
| address tokenA, | ||
| address tokenB, | ||
| address factory | ||
| ) internal view returns (address pair) { | ||
| require(factory == dxSwapFactory || factory == uniswapFactory, 'DXswapRelayer: INVALID_FACTORY'); | ||
| if (tokenA == address(0)) tokenA = WETH; | ||
| pair = IDXswapFactory(factory).getPair(tokenA, tokenB); | ||
| } | ||
|
|
||
| // Returns an OrderIndex that is used to reference liquidity orders | ||
| function _OrderIndex() internal returns (uint256 orderIndex) { | ||
| orderIndex = orderCount; | ||
| orderCount++; | ||
| } | ||
|
|
||
| // Returns the data of one specific order | ||
| function GetOrderDetails(uint256 orderIndex) external view returns (Order memory) { | ||
| return orders[orderIndex]; | ||
| } | ||
|
|
||
| function transferOwnership(address payable _newOwner) external { | ||
| require(msg.sender == owner, 'Ownable: caller is not the owner'); | ||
| address _oldOwner = owner; | ||
| owner = _newOwner; | ||
| emit OwnershipTransferred(_oldOwner, _newOwner); | ||
| } | ||
|
|
||
| receive() external payable {} | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Upgrade Solidity to v0.8.16 and remove the ABI encorder v2 pragma.