UniswapV2Bridge #2590
UniswapV2Bridge #2590
Conversation
xianny
force-pushed
the
feat/uniswap-v2
branch
2 times, most recently
from
ba6f7d5 to
c375398
Compare
| state.fromTokenBalance = IERC20Token(state.fromTokenAddress).balanceOf(address(this)); | ||
|
|
||
| // Grant the Uniswap router an allowance. | ||
| LibERC20Token.approve( |
There was a problem hiding this comment.
There should be a LibERC20Token .approveIfBelow so we keep it as high as possible and only write the state (expensive) if we have to.
There was a problem hiding this comment.
This is also important for USDT and some others, which will fail after the first approve().
There was a problem hiding this comment.
okiedokie will switch
| block.timestamp | ||
| ); | ||
|
|
||
| state.boughtAmount = amounts[0]; |
There was a problem hiding this comment.
| amounts | uint[] memory | The input token amount and all subsequent output token amounts.|
https://uniswap.org/docs/v2/smart-contracts/router/#swapexacttokensfortokens
does this mean the first element is amountSold?
There was a problem hiding this comment.
Yeah seems like we want the second element here.
There was a problem hiding this comment.
Could also just do state.boughtAmount = router.swapExactTokensForTokens(...)[1].
There was a problem hiding this comment.
Oops my bad. Looks like we want the second element (last element if more than 2 tokens in the path).
| state.fromTokenBalance = IERC20Token(state.fromTokenAddress).balanceOf(address(this)); | ||
|
|
||
| // Grant the Uniswap router an allowance. | ||
| LibERC20Token.approve( |
There was a problem hiding this comment.
This is also important for USDT and some others, which will fail after the first approve().
| block.timestamp | ||
| ); | ||
|
|
||
| state.boughtAmount = amounts[0]; |
There was a problem hiding this comment.
Yeah seems like we want the second element here.
| block.timestamp | ||
| ); | ||
|
|
||
| state.boughtAmount = amounts[0]; |
There was a problem hiding this comment.
Could also just do state.boughtAmount = router.swapExactTokensForTokens(...)[1].
| IWallet, | ||
| DeploymentConstants | ||
| { | ||
| using LibAddressArray for address[]; |
There was a problem hiding this comment.
Think you don't need this.
| address[] memory path = new address[](2); | ||
| path[0] = state.fromTokenAddress; | ||
| path[1] = toTokenAddress; |
There was a problem hiding this comment.
I thought there would be multihop support out of the gate? Should we just encode an address[] path as the bridge data?
There was a problem hiding this comment.
Ya let's do that. I'll change it.
|
|
||
| // Decode the bridge data to get the `fromTokenAddress`. | ||
| // solhint-disable indent | ||
| (state.path) = abi.decode(bridgeData, (address[])); |
There was a problem hiding this comment.
I think you can just do state.path = ... here?
| IWallet, | ||
| DeploymentConstants | ||
| { | ||
|
|
There was a problem hiding this comment.
No need for empty line here (this rule is confusing, I know).
| require(state.path.length >= 2, "PATH_LENGTH_MUST_BE_AT_LEAST_TWO"); | ||
| require(state.path[state.path.length - 1] == toTokenAddress, "LAST_ELEMENT_OF_PATH_MUST_MATCH_OUTPUT_TOKEN"); |
There was a problem hiding this comment.
| require(state.path.length >= 2, "PATH_LENGTH_MUST_BE_AT_LEAST_TWO"); | |
| require(state.path[state.path.length - 1] == toTokenAddress, "LAST_ELEMENT_OF_PATH_MUST_MATCH_OUTPUT_TOKEN"); | |
| require(state.path.length >= 2, "UniswapV2Bridge/PATH_LENGTH_MUST_BE_AT_LEAST_TWO"); | |
| require(state.path[state.path.length - 1] == toTokenAddress, "UniswapV2Bridge/LAST_ELEMENT_OF_PATH_MUST_MATCH_OUTPUT_TOKEN"); |
| struct TransferState { | ||
| address[] path; | ||
| uint256 fromTokenBalance; | ||
| uint256 boughtAmount; |
There was a problem hiding this comment.
No real point to this field since we can just access amounts[amounts.length-1].
| function createTransferFromOpts(opts?: Partial<TransferFromOpts>): TransferFromOpts { | ||
| const amount = getRandomInteger(1, TO_TOKEN_BASE.times(100)); | ||
| return { | ||
| tokenAddressesPath: Array(2).fill(constants.NULL_ADDRESS), |
There was a problem hiding this comment.
Where has this pattern been all my life.
Description
Implement UniswapV2Bridge contract. Allows us to swap tokens through the UniswapV2 Router.
Testing instructions
Please review unit tests and comment if you think any cases should be added.
Types of changes
Checklist:
[WIP]if necessary.