From a62ec3b18e286795602e8fd6cce54e97a28b9866 Mon Sep 17 00:00:00 2001 From: 0xSachinK Date: Wed, 30 Mar 2022 21:32:24 +0530 Subject: [PATCH 1/7] Add uniswapV3ExchangeAdapterV2 contract --- .../exchange/UniswapV3ExchangeAdapterV2.sol | 168 ++++++++++++++++++ 1 file changed, 168 insertions(+) create mode 100644 contracts/protocol/integration/exchange/UniswapV3ExchangeAdapterV2.sol diff --git a/contracts/protocol/integration/exchange/UniswapV3ExchangeAdapterV2.sol b/contracts/protocol/integration/exchange/UniswapV3ExchangeAdapterV2.sol new file mode 100644 index 000000000..a0d1049a2 --- /dev/null +++ b/contracts/protocol/integration/exchange/UniswapV3ExchangeAdapterV2.sol @@ -0,0 +1,168 @@ +/* + Copyright 2022 Set Labs Inc. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + + SPDX-License-Identifier: Apache License, Version 2.0 +*/ + +pragma solidity 0.6.10; +pragma experimental "ABIEncoderV2"; + +import { BytesLib } from "../../../../external/contracts/uniswap/v3/lib/BytesLib.sol"; +import { ISwapRouter } from "../../../interfaces/external/ISwapRouter.sol"; + +/** + * @title UniswapV3ExchangeAdapterV2 + * @author Set Protocol + * + * Exchange adapter for Uniswap V3 SwapRouter that encodes trade data. + * + * CHANGE LOG: + * - Generalized ability to choose whether to swap an exact amount of source token for a min amount of + * receive token or swap a max amount of source token for an exact amount of receive token. + */ +contract UniswapV3ExchangeAdapterV2 { + + using BytesLib for bytes; + + /* ============= Constants ================= */ + + // signature of SwapRouter#exactInput + string internal constant EXACT_INPUT = "exactInput((bytes,address,uint256,uint256,uint256))"; + + // signature of SwapRouter#exactOutput + string internal constant EXACT_OUTPUT = "exactOutput((bytes,address,uint256,uint256,uint256))"; + + /* ============ State Variables ============ */ + + // Address of Uniswap V3 SwapRouter contract + address public immutable swapRouter; + + /* ============ Constructor ============ */ + + /** + * Set state variables + * + * @param _swapRouter Address of Uniswap V3 SwapRouter + */ + constructor(address _swapRouter) public { + swapRouter = _swapRouter; + } + + /* ============ External Getter Functions ============ */ + + /** + * Return calldata for Uniswap V3 SwapRouter + * + * @param _sourceToken Address of source token to be sold + * @param _destinationToken Address of destination token to buy + * @param _destinationAddress Address that assets should be transferred to + * @param _sourceQuantity Fixed/Max amount of source token to sell + * @param _destinationQuantity Min/Fixed amount of destination token to buy + * @param _data Arbitrary bytes containing trade path and bool to determine function string. + * Equals the output of the generateDataParam function + * + * @return address Target contract address + * @return uint256 Call value + * @return bytes Trade calldata + */ + function getTradeCalldata( + address _sourceToken, + address _destinationToken, + address _destinationAddress, + uint256 _sourceQuantity, + uint256 _destinationQuantity, + bytes calldata _data + ) + external + view + returns (address, uint256, bytes memory) + { + + address sourceFromPath = _data.toAddress(0); + require(_sourceToken == sourceFromPath, "UniswapV3ExchangeAdapter: source token path mismatch"); + + address destinationFromPath = _data.toAddress(23); // 20 bytes (sourceFromPath) + 3 bytes (fees) + require(_destinationToken == destinationFromPath, "UniswapV3ExchangeAdapter: destination token path mismatch"); + + bool fixInput = _toBool(_data, _data.length - 1); // `fixInput` bool is stored at last byte + + bytes memory pathData = _data.slice(0, _data.length - 1); + + ISwapRouter.ExactInputParams memory params = fixInput + ? ISwapRouter.ExactInputParams( + pathData, + _destinationAddress, + block.timestamp, + _sourceQuantity, + _destinationQuantity + ) + : ISwapRouter.ExactOutputParams( + pathData, + _destinationAddress, + block.timestamp, + _destinationQuantity, // swapped vs exactInputParams + _sourceQuantity + ); + + bytes memory callData = abi.encodeWithSignature(EXACT_INPUT, params); + return (swapRouter, 0, callData); + } + + /** + * Returns the address to approve source tokens to for trading. This is the Uniswap SwapRouter address + * + * @return address Address of the contract to approve tokens to + */ + function getSpender() external view returns (address) { + return swapRouter; + } + + /** + * Returns the appropriate _data argument for getTradeCalldata. Equal to the encodePacked path with the + * fee of each hop between it, e.g [token1, fee1, token2, fee2, token3]. Note: _fees.length == _path.length - 1 + * + * @param _path array of addresses to use as the path for the trade + * @param _fees array of uint24 representing the pool fee to use for each hop + */ + function generateDataParam(address[] calldata _path, uint24[] calldata _fees, bool _fixIn) external pure returns (bytes memory) { + bytes memory data = ""; + for (uint256 i = 0; i < _path.length - 1; i++) { + data = abi.encodePacked(data, _path[i], _fees[i]); + } + + // Last encode has no fee associated with it since _fees.length == _path.length - 1 + data = abi.encodePacked(data, _path[_path.length - 1]); + + // Encode fixIn + return abi.encode(data, _fixIn); + } + + /** + * Helper function to decode bytes to boolean. Similar to functions found in BytesLib. + */ + function _toBool(bytes memory _bytes, uint256 _start) internal pure returns (bool) { + require(_start + 1 >= _start, "toBool_overflow"); + require(_bytes.length >= _start + 1, "toBool_outOfBounds"); + uint8 tempUint; + + assembly { + tempUint := mload(add(add(_bytes, 0x1), _start)) + } + + require(tempUint <= 1, "Invalid data"); // Should be either 0 or 1 + + return (tempUint == 0) ? false : true; + } +} \ No newline at end of file From f626f13d35b44758187726e10c6007cd2c8137a2 Mon Sep 17 00:00:00 2001 From: 0xSachinK Date: Wed, 30 Mar 2022 21:41:36 +0530 Subject: [PATCH 2/7] Fix compilation bug --- .../exchange/UniswapV3ExchangeAdapterV2.sol | 48 ++++++++++++------- 1 file changed, 30 insertions(+), 18 deletions(-) diff --git a/contracts/protocol/integration/exchange/UniswapV3ExchangeAdapterV2.sol b/contracts/protocol/integration/exchange/UniswapV3ExchangeAdapterV2.sol index a0d1049a2..e1ff37124 100644 --- a/contracts/protocol/integration/exchange/UniswapV3ExchangeAdapterV2.sol +++ b/contracts/protocol/integration/exchange/UniswapV3ExchangeAdapterV2.sol @@ -70,7 +70,7 @@ contract UniswapV3ExchangeAdapterV2 { * @param _destinationAddress Address that assets should be transferred to * @param _sourceQuantity Fixed/Max amount of source token to sell * @param _destinationQuantity Min/Fixed amount of destination token to buy - * @param _data Arbitrary bytes containing trade path and bool to determine function string. + * @param _data Bytes containing trade path and bool to determine function string. * Equals the output of the generateDataParam function * * @return address Target contract address @@ -91,32 +91,37 @@ contract UniswapV3ExchangeAdapterV2 { { address sourceFromPath = _data.toAddress(0); - require(_sourceToken == sourceFromPath, "UniswapV3ExchangeAdapter: source token path mismatch"); + require(_sourceToken == sourceFromPath, "UniswapV3ExchangeAdapterV2: source token path mismatch"); address destinationFromPath = _data.toAddress(23); // 20 bytes (sourceFromPath) + 3 bytes (fees) - require(_destinationToken == destinationFromPath, "UniswapV3ExchangeAdapter: destination token path mismatch"); + require(_destinationToken == destinationFromPath, "UniswapV3ExchangeAdapterV2: destination token path mismatch"); bool fixInput = _toBool(_data, _data.length - 1); // `fixInput` bool is stored at last byte bytes memory pathData = _data.slice(0, _data.length - 1); - ISwapRouter.ExactInputParams memory params = fixInput - ? ISwapRouter.ExactInputParams( - pathData, - _destinationAddress, - block.timestamp, - _sourceQuantity, - _destinationQuantity + bytes memory callData = fixInput + ? abi.encodeWithSignature( + EXACT_INPUT, + ISwapRouter.ExactInputParams( + pathData, + _destinationAddress, + block.timestamp, + _sourceQuantity, + _destinationQuantity + ) ) - : ISwapRouter.ExactOutputParams( - pathData, - _destinationAddress, - block.timestamp, - _destinationQuantity, // swapped vs exactInputParams - _sourceQuantity + : abi.encodeWithSignature( + EXACT_OUTPUT, + ISwapRouter.ExactOutputParams( + pathData, + _destinationAddress, + block.timestamp, + _destinationQuantity, // swapped vs exactInputParams + _sourceQuantity + ) ); - bytes memory callData = abi.encodeWithSignature(EXACT_INPUT, params); return (swapRouter, 0, callData); } @@ -135,8 +140,15 @@ contract UniswapV3ExchangeAdapterV2 { * * @param _path array of addresses to use as the path for the trade * @param _fees array of uint24 representing the pool fee to use for each hop + * @param _fixIn Boolean indicating if input amount is fixed + * + * @return bytes Bytes containing trade path and bool to determine function string. */ - function generateDataParam(address[] calldata _path, uint24[] calldata _fees, bool _fixIn) external pure returns (bytes memory) { + function generateDataParam( + address[] calldata _path, + uint24[] calldata _fees, + bool _fixIn + ) external pure returns (bytes memory) { bytes memory data = ""; for (uint256 i = 0; i < _path.length - 1; i++) { data = abi.encodePacked(data, _path[i], _fees[i]); From 483b1468b871d735c712c3ebfad9217d319312d4 Mon Sep 17 00:00:00 2001 From: 0xSachinK Date: Thu, 31 Mar 2022 16:49:23 +0530 Subject: [PATCH 3/7] Add unit tests --- .../exchange/UniswapV3ExchangeAdapterV2.sol | 36 ++- .../uniswapV3ExchangeAdapterV2.spec.ts | 268 ++++++++++++++++++ utils/contracts/index.ts | 1 + utils/deploys/deployAdapters.ts | 6 + 4 files changed, 291 insertions(+), 20 deletions(-) create mode 100644 test/protocol/integration/exchange/uniswapV3ExchangeAdapterV2.spec.ts diff --git a/contracts/protocol/integration/exchange/UniswapV3ExchangeAdapterV2.sol b/contracts/protocol/integration/exchange/UniswapV3ExchangeAdapterV2.sol index e1ff37124..75f9344d6 100644 --- a/contracts/protocol/integration/exchange/UniswapV3ExchangeAdapterV2.sol +++ b/contracts/protocol/integration/exchange/UniswapV3ExchangeAdapterV2.sol @@ -36,14 +36,6 @@ contract UniswapV3ExchangeAdapterV2 { using BytesLib for bytes; - /* ============= Constants ================= */ - - // signature of SwapRouter#exactInput - string internal constant EXACT_INPUT = "exactInput((bytes,address,uint256,uint256,uint256))"; - - // signature of SwapRouter#exactOutput - string internal constant EXACT_OUTPUT = "exactOutput((bytes,address,uint256,uint256,uint256))"; - /* ============ State Variables ============ */ // Address of Uniswap V3 SwapRouter contract @@ -89,20 +81,23 @@ contract UniswapV3ExchangeAdapterV2 { view returns (address, uint256, bytes memory) { + // 20 bytes (_sourceToken address) + 3 bytes (UniV3 fees tier) + // + 20 bytes (_destinationToken address) + 1 byte (bool to determine fixed input/output) + require(_data.length == 44, "Invalid data"); address sourceFromPath = _data.toAddress(0); - require(_sourceToken == sourceFromPath, "UniswapV3ExchangeAdapterV2: source token path mismatch"); + require(_sourceToken == sourceFromPath, "Source token path mismatch"); address destinationFromPath = _data.toAddress(23); // 20 bytes (sourceFromPath) + 3 bytes (fees) - require(_destinationToken == destinationFromPath, "UniswapV3ExchangeAdapterV2: destination token path mismatch"); + require(_destinationToken == destinationFromPath, "Destination token path mismatch"); - bool fixInput = _toBool(_data, _data.length - 1); // `fixInput` bool is stored at last byte + bytes memory pathData = _data.slice(0, _data.length - 1); // Extract path data from `_data` - bytes memory pathData = _data.slice(0, _data.length - 1); + bool fixInput = _toBool(_data, _data.length - 1); // `fixInput` bool is stored at last byte bytes memory callData = fixInput - ? abi.encodeWithSignature( - EXACT_INPUT, + ? abi.encodeWithSelector( + ISwapRouter.exactInput.selector, ISwapRouter.ExactInputParams( pathData, _destinationAddress, @@ -111,8 +106,8 @@ contract UniswapV3ExchangeAdapterV2 { _destinationQuantity ) ) - : abi.encodeWithSignature( - EXACT_OUTPUT, + : abi.encodeWithSelector( + ISwapRouter.exactOutput.selector, ISwapRouter.ExactOutputParams( pathData, _destinationAddress, @@ -158,22 +153,23 @@ contract UniswapV3ExchangeAdapterV2 { data = abi.encodePacked(data, _path[_path.length - 1]); // Encode fixIn - return abi.encode(data, _fixIn); + return abi.encodePacked(data, _fixIn); } /** * Helper function to decode bytes to boolean. Similar to functions found in BytesLib. */ function _toBool(bytes memory _bytes, uint256 _start) internal pure returns (bool) { - require(_start + 1 >= _start, "toBool_overflow"); - require(_bytes.length >= _start + 1, "toBool_outOfBounds"); + // Don't need these checks, because we have assured `_bytes` is of length 44 in the calling function + // require(_start + 1 >= _start, "toBool_overflow"); + // require(_bytes.length >= _start + 1, "toBool_outOfBounds"); uint8 tempUint; assembly { tempUint := mload(add(add(_bytes, 0x1), _start)) } - require(tempUint <= 1, "Invalid data"); // Should be either 0 or 1 + require(tempUint <= 1, "Invalid bool data"); // Should be either 0 or 1 return (tempUint == 0) ? false : true; } diff --git a/test/protocol/integration/exchange/uniswapV3ExchangeAdapterV2.spec.ts b/test/protocol/integration/exchange/uniswapV3ExchangeAdapterV2.spec.ts new file mode 100644 index 000000000..d8c7f9ffb --- /dev/null +++ b/test/protocol/integration/exchange/uniswapV3ExchangeAdapterV2.spec.ts @@ -0,0 +1,268 @@ +import "module-alias/register"; +import { BigNumber, BigNumberish } from "ethers"; +import { solidityPack } from "ethers/lib/utils"; + +import { Address, Bytes } from "@utils/types"; +import { Account } from "@utils/test/types"; +import { ZERO } from "@utils/constants"; +import { UniswapV3ExchangeAdapterV2 } from "@utils/contracts"; +import DeployHelper from "@utils/deploys"; +import { ether } from "@utils/index"; +import { + addSnapshotBeforeRestoreAfterEach, + getAccounts, + getSystemFixture, + getWaffleExpect, + getLastBlockTimestamp, + getUniswapV3Fixture, + getRandomAddress +} from "@utils/test/index"; +import { SystemFixture, UniswapV3Fixture } from "@utils/fixtures"; +const expect = getWaffleExpect(); + + +describe("UniswapV3ExchangeAdapterV2", () => { + let owner: Account; + let mockSetToken: Account; + let deployer: DeployHelper; + let setup: SystemFixture; + let uniswapV3Fixture: UniswapV3Fixture; + + let uniswapV3ExchangeAdapter: UniswapV3ExchangeAdapterV2; + + before(async () => { + [ + owner, + mockSetToken, + ] = await getAccounts(); + + deployer = new DeployHelper(owner.wallet); + setup = getSystemFixture(owner.address); + await setup.initialize(); + + uniswapV3Fixture = getUniswapV3Fixture(owner.address); + await uniswapV3Fixture.initialize( + owner, + setup.weth, + 2500, + setup.wbtc, + 35000, + setup.dai + ); + + uniswapV3ExchangeAdapter = await deployer.adapters.deployUniswapV3ExchangeAdapterV2(uniswapV3Fixture.swapRouter.address); + }); + + addSnapshotBeforeRestoreAfterEach(); + + describe("#constructor", async () => { + let subjectSwapRouter: Address; + + beforeEach(async () => { + subjectSwapRouter = uniswapV3Fixture.swapRouter.address; + }); + + async function subject(): Promise { + return await deployer.adapters.deployUniswapV3ExchangeAdapterV2(subjectSwapRouter); + } + + it("should have the correct SwapRouter address", async () => { + const deployedUniswapV3ExchangeAdapterV2 = await subject(); + + const actualRouterAddress = await deployedUniswapV3ExchangeAdapterV2.swapRouter(); + expect(actualRouterAddress).to.eq(uniswapV3Fixture.swapRouter.address); + }); + }); + + describe("#getSpender", async () => { + async function subject(): Promise { + return await uniswapV3ExchangeAdapter.getSpender(); + } + + it("should return the correct spender address", async () => { + const spender = await subject(); + + expect(spender).to.eq(uniswapV3Fixture.swapRouter.address); + }); + }); + + describe("#getTradeCalldata", async () => { + let fixIn: boolean = true; + + let subjectMockSetToken: Address; + let subjectSourceToken: Address; + let subjectDestinationToken: Address; + let subjectSourceQuantity: BigNumber; + let subjectMinDestinationQuantity: BigNumber; + let subjectPath: Bytes; + + beforeEach(async () => { + subjectSourceToken = setup.wbtc.address; + subjectSourceQuantity = BigNumber.from(100000000); + subjectDestinationToken = setup.weth.address; + subjectMinDestinationQuantity = ether(25); + subjectMockSetToken = mockSetToken.address; + subjectPath = solidityPack( + ["address", "uint24", "address", "bool"], + [subjectSourceToken, BigNumber.from(3000), subjectDestinationToken, fixIn] + ); + }); + + async function subject(): Promise { + return await uniswapV3ExchangeAdapter.getTradeCalldata( + subjectSourceToken, + subjectDestinationToken, + subjectMockSetToken, + subjectSourceQuantity, + subjectMinDestinationQuantity, + subjectPath, + ); + } + + it("should return the correct trade calldata", async () => { + const calldata = await subject(); + const callTimestamp = await getLastBlockTimestamp(); + const encodedPathWithoutBool = solidityPack( + ["address", "uint24", "address"], + [subjectSourceToken, BigNumber.from(3000), subjectDestinationToken] + ); + + const expectedCallData = uniswapV3Fixture.swapRouter.interface.encodeFunctionData("exactInput", [{ + path: encodedPathWithoutBool, + recipient: mockSetToken.address, + deadline: callTimestamp, + amountIn: subjectSourceQuantity, + amountOutMinimum: subjectMinDestinationQuantity, + }]); + + expect(JSON.stringify(calldata)).to.eq(JSON.stringify([uniswapV3Fixture.swapRouter.address, ZERO, expectedCallData])); + }); + + describe("when fixIn is false", async () => { + beforeEach(async () => { + fixIn = false; + + subjectPath = solidityPack( + ["address", "uint24", "address", "bool"], + [subjectSourceToken, BigNumber.from(3000), subjectDestinationToken, fixIn] + ); + }); + + it("should return the correct trade calldata", async () => { + const calldata = await subject(); + const callTimestamp = await getLastBlockTimestamp(); + const encodedPathWithoutBool = solidityPack( + ["address", "uint24", "address"], + [subjectSourceToken, BigNumber.from(3000), subjectDestinationToken] + ); + + const expectedCallData = uniswapV3Fixture.swapRouter.interface.encodeFunctionData("exactOutput", [{ + path: encodedPathWithoutBool, + recipient: mockSetToken.address, + deadline: callTimestamp, + amountOut: subjectMinDestinationQuantity, + amountInMaximum: subjectSourceQuantity, + }]); + + expect(JSON.stringify(calldata)).to.eq(JSON.stringify([uniswapV3Fixture.swapRouter.address, ZERO, expectedCallData])); + }); + }); + + context("when data is of invalid length", async () => { + beforeEach(() => { + // Skip encoding `fixIn` bool + subjectPath = solidityPack( + ["address", "uint24", "address"], + [subjectSourceToken, BigNumber.from(3000), subjectDestinationToken] + ); + }); + + it("should revert", async () => { + await expect(subject()).to.be.revertedWith("Invalid data"); + }); + }); + + context("when source token does not match path", async () => { + beforeEach(async () => { + subjectSourceToken = await getRandomAddress(); + }); + + it("should revert", async () => { + await expect(subject()).to.be.revertedWith("Source token path mismatch"); + }); + }); + + context("when destination token does not match path", async () => { + beforeEach(async () => { + subjectDestinationToken = await getRandomAddress(); + }); + + it("should revert", async () => { + await expect(subject()).to.be.revertedWith("Destination token path mismatch"); + }); + }); + + context("when fixIn boolean is invalid number", async () => { + beforeEach(async () => { + subjectPath = solidityPack( + ["address", "uint24", "address", "uint8"], + [subjectSourceToken, BigNumber.from(3000), subjectDestinationToken, BigNumber.from(2)] + ); + }); + + it("should revert", async () => { + await expect(subject()).to.be.revertedWith("Invalid bool data"); + }); + }); + }); + + describe("#generateDataParam", async () => { + let subjectToken1: Address; + let subjectFee1: BigNumberish; + let subjectToken2: Address; + let subjectFee2: BigNumberish; + let subjectToken3: Address; + let subjectFixIn: boolean; + + beforeEach(async () => { + subjectToken1 = setup.wbtc.address; + subjectFee1 = 3000; + subjectToken2 = setup.weth.address; + subjectFee2 = 500; + subjectToken3 = setup.weth.address; + subjectFixIn = true; + }); + + async function subject(): Promise { + return await uniswapV3ExchangeAdapter.generateDataParam([subjectToken1, subjectToken2, subjectToken3], [subjectFee1, subjectFee2], subjectFixIn); + } + + it("should create the correct path data", async () => { + const data = await subject(); + + const expectedData = solidityPack( + ["address", "uint24", "address", "uint24", "address", "bool"], + [subjectToken1, subjectFee1, subjectToken2, subjectFee2, subjectToken3, subjectFixIn] + ); + + expect(data).to.eq(expectedData); + }); + + describe("when fixIn is false", async () => { + beforeEach(async () => { + subjectFixIn = false; + }); + + it("should create the correct path data", async () => { + const data = await subject(); + + const expectedData = solidityPack( + ["address", "uint24", "address", "uint24", "address", "bool"], + [subjectToken1, subjectFee1, subjectToken2, subjectFee2, subjectToken3, subjectFixIn] + ); + + expect(data).to.eq(expectedData); + }); + }); + }); +}); \ No newline at end of file diff --git a/utils/contracts/index.ts b/utils/contracts/index.ts index f015d6aec..181846adb 100644 --- a/utils/contracts/index.ts +++ b/utils/contracts/index.ts @@ -110,6 +110,7 @@ export { AMMSplitter } from "../../typechain/AMMSplitter"; export { UniswapV2Pair } from "../../typechain/UniswapV2Pair"; export { UniswapV2Router02 } from "../../typechain/UniswapV2Router02"; export { UniswapV3ExchangeAdapter } from "../../typechain/UniswapV3ExchangeAdapter"; +export { UniswapV3ExchangeAdapterV2 } from "../../typechain/UniswapV3ExchangeAdapterV2"; export { UniswapV3MathMock } from "../../typechain/UniswapV3MathMock"; export { UnitConversionUtilsMock } from "../../typechain/UnitConversionUtilsMock"; export { WETH9 } from "../../typechain/WETH9"; diff --git a/utils/deploys/deployAdapters.ts b/utils/deploys/deployAdapters.ts index 885ce1aaf..6621f03db 100644 --- a/utils/deploys/deployAdapters.ts +++ b/utils/deploys/deployAdapters.ts @@ -19,6 +19,7 @@ import { UniswapV3IndexExchangeAdapter, UniswapV2TransferFeeExchangeAdapter, UniswapV3ExchangeAdapter, + UniswapV3ExchangeAdapterV2, ZeroExApiAdapter, SnapshotGovernanceAdapter, SynthetixExchangeAdapter, @@ -46,6 +47,7 @@ import { UniswapV2ExchangeAdapterV2__factory } from "../../typechain/factories/U import { UniswapV2IndexExchangeAdapter__factory } from "../../typechain/factories/UniswapV2IndexExchangeAdapter__factory"; import { UniswapV3IndexExchangeAdapter__factory } from "../../typechain/factories/UniswapV3IndexExchangeAdapter__factory"; import { UniswapV3ExchangeAdapter__factory } from "../../typechain/factories/UniswapV3ExchangeAdapter__factory"; +import { UniswapV3ExchangeAdapterV2__factory } from "../../typechain/factories/UniswapV3ExchangeAdapterV2__factory"; import { SnapshotGovernanceAdapter__factory } from "../../typechain/factories/SnapshotGovernanceAdapter__factory"; import { SynthetixExchangeAdapter__factory } from "../../typechain/factories/SynthetixExchangeAdapter__factory"; import { CompoundBravoGovernanceAdapter__factory } from "../../typechain/factories/CompoundBravoGovernanceAdapter__factory"; @@ -155,6 +157,10 @@ export default class DeployAdapters { return await new UniswapV3ExchangeAdapter__factory(this._deployerSigner).deploy(swapRouter); } + public async deployUniswapV3ExchangeAdapterV2(swapRouter: Address): Promise { + return await new UniswapV3ExchangeAdapterV2__factory(this._deployerSigner).deploy(swapRouter); + } + public async deployKyberV3IndexExchangeAdapter(dmmRouter: Address, dmmFactory: Address): Promise { return await new KyberV3IndexExchangeAdapter__factory(this._deployerSigner).deploy(dmmRouter, dmmFactory); } From 7e5ad81286c45cde6df8207151c256d6401c2d33 Mon Sep 17 00:00:00 2001 From: 0xSachinK Date: Thu, 31 Mar 2022 19:21:12 +0530 Subject: [PATCH 4/7] Fix exactOutput bug; Make toBool public and add unit tests for it --- .../exchange/UniswapV3ExchangeAdapterV2.sol | 35 +++++--- .../uniswapV3ExchangeAdapterV2.spec.ts | 80 ++++++++++++++++++- 2 files changed, 98 insertions(+), 17 deletions(-) diff --git a/contracts/protocol/integration/exchange/UniswapV3ExchangeAdapterV2.sol b/contracts/protocol/integration/exchange/UniswapV3ExchangeAdapterV2.sol index 75f9344d6..06c8dcaa1 100644 --- a/contracts/protocol/integration/exchange/UniswapV3ExchangeAdapterV2.sol +++ b/contracts/protocol/integration/exchange/UniswapV3ExchangeAdapterV2.sol @@ -26,7 +26,7 @@ import { ISwapRouter } from "../../../interfaces/external/ISwapRouter.sol"; * @title UniswapV3ExchangeAdapterV2 * @author Set Protocol * - * Exchange adapter for Uniswap V3 SwapRouter that encodes trade data. + * Exchange adapter for Uniswap V3 SwapRouter that encodes trade data. Supports multi-hop trades. * * CHANGE LOG: * - Generalized ability to choose whether to swap an exact amount of source token for a min amount of @@ -64,6 +64,7 @@ contract UniswapV3ExchangeAdapterV2 { * @param _destinationQuantity Min/Fixed amount of destination token to buy * @param _data Bytes containing trade path and bool to determine function string. * Equals the output of the generateDataParam function + * NOTE: Path for `exactOutput` swaps are reversed * * @return address Target contract address * @return uint256 Call value @@ -81,20 +82,29 @@ contract UniswapV3ExchangeAdapterV2 { view returns (address, uint256, bytes memory) { - // 20 bytes (_sourceToken address) + 3 bytes (UniV3 fees tier) - // + 20 bytes (_destinationToken address) + 1 byte (bool to determine fixed input/output) - require(_data.length == 44, "Invalid data"); + // `_data.length` should be atleast 44. For a single hop trade, `_data.length` is 44. + // 20 source/destination token address + 3 fees + 20 source/destination tokne address + 1 fixInput bool. + require(_data.length >= 44, "Invalid data"); + + bool fixInput = toBool(_data, _data.length - 1); // `fixInput` bool is stored at last byte + + address sourceFromPath; + address destinationFromPath; + + if (fixInput) { + sourceFromPath = _data.toAddress(0); + destinationFromPath = _data.toAddress(_data.length - 21); + } else { + // Path for exactOutput swaps are reversed + sourceFromPath = _data.toAddress(_data.length - 21); + destinationFromPath = _data.toAddress(0); + } - address sourceFromPath = _data.toAddress(0); require(_sourceToken == sourceFromPath, "Source token path mismatch"); - - address destinationFromPath = _data.toAddress(23); // 20 bytes (sourceFromPath) + 3 bytes (fees) require(_destinationToken == destinationFromPath, "Destination token path mismatch"); bytes memory pathData = _data.slice(0, _data.length - 1); // Extract path data from `_data` - bool fixInput = _toBool(_data, _data.length - 1); // `fixInput` bool is stored at last byte - bytes memory callData = fixInput ? abi.encodeWithSelector( ISwapRouter.exactInput.selector, @@ -159,10 +169,9 @@ contract UniswapV3ExchangeAdapterV2 { /** * Helper function to decode bytes to boolean. Similar to functions found in BytesLib. */ - function _toBool(bytes memory _bytes, uint256 _start) internal pure returns (bool) { - // Don't need these checks, because we have assured `_bytes` is of length 44 in the calling function - // require(_start + 1 >= _start, "toBool_overflow"); - // require(_bytes.length >= _start + 1, "toBool_outOfBounds"); + function toBool(bytes memory _bytes, uint256 _start) public pure returns (bool) { + require(_start + 1 >= _start, "toBool_overflow"); + require(_bytes.length >= _start + 1, "toBool_outOfBounds"); uint8 tempUint; assembly { diff --git a/test/protocol/integration/exchange/uniswapV3ExchangeAdapterV2.spec.ts b/test/protocol/integration/exchange/uniswapV3ExchangeAdapterV2.spec.ts index d8c7f9ffb..baee557b7 100644 --- a/test/protocol/integration/exchange/uniswapV3ExchangeAdapterV2.spec.ts +++ b/test/protocol/integration/exchange/uniswapV3ExchangeAdapterV2.spec.ts @@ -4,7 +4,7 @@ import { solidityPack } from "ethers/lib/utils"; import { Address, Bytes } from "@utils/types"; import { Account } from "@utils/test/types"; -import { ZERO } from "@utils/constants"; +import { MAX_INT_256, MAX_UINT_256, ZERO } from "@utils/constants"; import { UniswapV3ExchangeAdapterV2 } from "@utils/contracts"; import DeployHelper from "@utils/deploys"; import { ether } from "@utils/index"; @@ -87,7 +87,7 @@ describe("UniswapV3ExchangeAdapterV2", () => { }); describe("#getTradeCalldata", async () => { - let fixIn: boolean = true; + let fixIn: boolean; let subjectMockSetToken: Address; let subjectSourceToken: Address; @@ -97,6 +97,8 @@ describe("UniswapV3ExchangeAdapterV2", () => { let subjectPath: Bytes; beforeEach(async () => { + fixIn = true; + subjectSourceToken = setup.wbtc.address; subjectSourceQuantity = BigNumber.from(100000000); subjectDestinationToken = setup.weth.address; @@ -144,7 +146,7 @@ describe("UniswapV3ExchangeAdapterV2", () => { subjectPath = solidityPack( ["address", "uint24", "address", "bool"], - [subjectSourceToken, BigNumber.from(3000), subjectDestinationToken, fixIn] + [subjectDestinationToken, BigNumber.from(3000), subjectSourceToken, fixIn] ); }); @@ -153,7 +155,7 @@ describe("UniswapV3ExchangeAdapterV2", () => { const callTimestamp = await getLastBlockTimestamp(); const encodedPathWithoutBool = solidityPack( ["address", "uint24", "address"], - [subjectSourceToken, BigNumber.from(3000), subjectDestinationToken] + [subjectDestinationToken, BigNumber.from(3000), subjectSourceToken] ); const expectedCallData = uniswapV3Fixture.swapRouter.interface.encodeFunctionData("exactOutput", [{ @@ -265,4 +267,74 @@ describe("UniswapV3ExchangeAdapterV2", () => { }); }); }); + + describe.only("#toBool", async () => { + let bool: boolean; + let randomAddress: Address; + + let subjectBytes: Bytes; + let subjectStart: BigNumber; + + before(async () => { + randomAddress = await getRandomAddress(); + }); + + beforeEach(async() => { + bool = true; + + subjectBytes = solidityPack( + ["address", "bool"], + [randomAddress, bool] + ); + subjectStart = BigNumber.from(20); // Address is 20 bytes long + }); + + async function subject(): Promise { + return await uniswapV3ExchangeAdapter.toBool(subjectBytes, subjectStart); + } + + it("should return correct bool", async () => { + const actualBool = await subject(); + + expect(actualBool).to.eq(bool); + }); + + describe("when bool is false", async () => { + beforeEach(async() => { + bool = false; + + subjectBytes = solidityPack( + ["address", "bool"], + [randomAddress, bool] + ); + }); + + it("should return correct bool", async () => { + const actualBool = await subject(); + + expect(actualBool).to.eq(bool); + }); + }); + + describe("when start is max uint 256", async () => { + beforeEach(() => { + subjectStart = MAX_UINT_256; + }); + + it("should revert", async () => { + await expect(subject()).to.be.revertedWith("toBool_overflow"); + }); + }); + + + describe("when start is out of bounds", async () => { + beforeEach(() => { + subjectStart = BigNumber.from(subjectBytes.length); + }); + + it("should revert", async () => { + await expect(subject()).to.be.revertedWith("toBool_outOfBounds"); + }); + }); + }); }); \ No newline at end of file From aec83894f8dfb98636d3c80ca7931eb7bbd0279f Mon Sep 17 00:00:00 2001 From: 0xSachinK Date: Thu, 31 Mar 2022 19:36:36 +0530 Subject: [PATCH 5/7] Add integration tests with TradeModule --- test/protocol/modules/v1/tradeModule.spec.ts | 280 ++++++++++++++++++- 1 file changed, 277 insertions(+), 3 deletions(-) diff --git a/test/protocol/modules/v1/tradeModule.spec.ts b/test/protocol/modules/v1/tradeModule.spec.ts index d36116534..00475768c 100644 --- a/test/protocol/modules/v1/tradeModule.spec.ts +++ b/test/protocol/modules/v1/tradeModule.spec.ts @@ -19,10 +19,11 @@ import { UniswapV2ExchangeAdapter, UniswapV2TransferFeeExchangeAdapter, UniswapV2ExchangeAdapterV2, + UniswapV3ExchangeAdapter, + UniswapV3ExchangeAdapterV2, WETH9, ZeroExApiAdapter, ZeroExMock, - UniswapV3ExchangeAdapter, } from "@utils/contracts"; import { ADDRESS_ZERO, EMPTY_BYTES, MAX_UINT_256, ZERO } from "@utils/constants"; import DeployHelper from "@utils/deploys"; @@ -68,6 +69,8 @@ describe("TradeModule", () => { let uniswapAdapterV2Name: string; let uniswapV3ExchangeAdapter: UniswapV3ExchangeAdapter; let uniswapV3AdapterName: string; + let uniswapV3ExchangeAdapterV2: UniswapV3ExchangeAdapterV2; + let uniswapV3AdapterV2Name: string; let zeroExMock: ZeroExMock; let zeroExApiAdapter: ZeroExApiAdapter; @@ -141,6 +144,7 @@ describe("TradeModule", () => { uniswapTransferFeeExchangeAdapter = await deployer.adapters.deployUniswapV2TransferFeeExchangeAdapter(uniswapSetup.router.address); uniswapExchangeAdapterV2 = await deployer.adapters.deployUniswapV2ExchangeAdapterV2(uniswapSetup.router.address); uniswapV3ExchangeAdapter = await deployer.adapters.deployUniswapV3ExchangeAdapter(uniswapV3Setup.swapRouter.address); + uniswapV3ExchangeAdapterV2 = await deployer.adapters.deployUniswapV3ExchangeAdapterV2(uniswapV3Setup.swapRouter.address); zeroExMock = await deployer.mocks.deployZeroExMock( setup.wbtc.address, @@ -158,6 +162,7 @@ describe("TradeModule", () => { uniswapAdapterV2Name = "UNISWAPV2"; zeroExApiAdapterName = "ZERO_EX"; uniswapV3AdapterName = "UNISWAPV3"; + uniswapV3AdapterV2Name = "UNISWAPV3_V2" tradeModule = await deployer.modules.deployTradeModule(setup.controller.address); await setup.controller.addModule(tradeModule.address); @@ -171,6 +176,7 @@ describe("TradeModule", () => { tradeModule.address, tradeModule.address, tradeModule.address, + tradeModule.address ], [ kyberAdapterName, @@ -180,6 +186,7 @@ describe("TradeModule", () => { uniswapAdapterV2Name, zeroExApiAdapterName, uniswapV3AdapterName, + uniswapV3AdapterV2Name ], [ kyberExchangeAdapter.address, @@ -189,6 +196,7 @@ describe("TradeModule", () => { uniswapExchangeAdapterV2.address, zeroExApiAdapter.address, uniswapV3ExchangeAdapter.address, + uniswapV3ExchangeAdapterV2.address ] ); }); @@ -210,7 +218,7 @@ describe("TradeModule", () => { context("when there is a deployed SetToken with enabled TradeModule", async () => { let sourceToken: StandardTokenMock; let wbtcUnits: BigNumber; - let destinationToken: WETH9; + let destinationToken: WETH9 | StandardTokenMock; let setToken: SetToken; let issueQuantity: BigNumber; let mockPreIssuanceHook: ManagerIssuanceHookMock; @@ -1675,7 +1683,7 @@ describe("TradeModule", () => { }); }); - context("when trading a Default component on Uniswap V3", async () => { + context("when trading a Default component on Uniswap V3 using UniswapV3ExchangeAdapter", async () => { cacheBeforeEach(async () => { await setup.weth.connect(owner.wallet).approve(uniswapV3Setup.nftPositionManager.address, ether(350)); await setup.wbtc.connect(owner.wallet).approve(uniswapV3Setup.nftPositionManager.address, bitcoin(25)); @@ -1823,6 +1831,272 @@ describe("TradeModule", () => { }); }); }); + + context.only("when trading a Default component on Uniswap V3 using UniswapV3ExchangeAdapterV2", async () => { + let fixIn: boolean = true; + + cacheBeforeEach(async () => { + await setup.weth.connect(owner.wallet).approve(uniswapV3Setup.nftPositionManager.address, ether(350)); + await setup.wbtc.connect(owner.wallet).approve(uniswapV3Setup.nftPositionManager.address, bitcoin(25)); + + await uniswapV3Setup.addLiquidityWide( + setup.weth, + setup.wbtc, + 3000, + ether(350), + bitcoin(25), + owner.address + ); + + tradeModule = tradeModule.connect(manager.wallet); + await tradeModule.initialize(setToken.address); + + sourceTokenQuantity = wbtcUnits; + + // Transfer sourceToken from owner to manager for issuance + sourceToken = sourceToken.connect(owner.wallet); + await sourceToken.transfer(manager.address, wbtcUnits.mul(100)); + + // Deploy mock issuance hook and initialize issuance module + setup.issuanceModule = setup.issuanceModule.connect(manager.wallet); + mockPreIssuanceHook = await deployer.mocks.deployManagerIssuanceHookMock(); + await setup.issuanceModule.initialize(setToken.address, mockPreIssuanceHook.address); + + // Approve tokens to issuance module and call issue + sourceToken = sourceToken.connect(manager.wallet); + await sourceToken.approve(setup.issuanceModule.address, ethers.constants.MaxUint256); + issueQuantity = ether(1); + await setup.issuanceModule.issue(setToken.address, issueQuantity, owner.address); + }); + + beforeEach(async () => { + subjectSourceToken = sourceToken.address; + subjectDestinationToken = destinationToken.address; + subjectSourceQuantity = sourceTokenQuantity; + subjectSetToken = setToken.address; + subjectAdapterName = uniswapV3AdapterV2Name; + subjectData = await uniswapV3ExchangeAdapterV2.generateDataParam([setup.wbtc.address, setup.weth.address], [3000], fixIn); + subjectMinDestinationQuantity = BigNumber.from(0); + subjectCaller = manager; + }); + + async function subject(): Promise { + tradeModule = tradeModule.connect(subjectCaller.wallet); + return tradeModule.trade( + subjectSetToken, + subjectAdapterName, + subjectSourceToken, + subjectSourceQuantity, + subjectDestinationToken, + subjectMinDestinationQuantity, + subjectData + ); + } + + it("should transfer the correct components to the SetToken", async () => { + const oldDestinationTokenBalance = await destinationToken.balanceOf(setToken.address); + const expectedReceiveQuantity = await uniswapV3Setup.quoter.callStatic.quoteExactInputSingle( + subjectSourceToken, + subjectDestinationToken, + 3000, + subjectSourceQuantity, + 0 + ); + + await subject(); + + const expectedDestinationTokenBalance = oldDestinationTokenBalance.add(expectedReceiveQuantity); + const newDestinationTokenBalance = await destinationToken.balanceOf(setToken.address); + expect(newDestinationTokenBalance).to.eq(expectedDestinationTokenBalance); + }); + + it("should transfer the correct components from the SetToken", async () => { + const oldSourceTokenBalance = await sourceToken.balanceOf(setToken.address); + + await subject(); + + const totalSourceQuantity = issueQuantity.mul(sourceTokenQuantity).div(ether(1)); + const expectedSourceTokenBalance = oldSourceTokenBalance.sub(totalSourceQuantity); + const newSourceTokenBalance = await sourceToken.balanceOf(setToken.address); + expect(newSourceTokenBalance).to.eq(expectedSourceTokenBalance); + }); + + it("should update the positions on the SetToken correctly", async () => { + const initialPositions = await setToken.getPositions(); + const expectedReceiveQuantity = await uniswapV3Setup.quoter.callStatic.quoteExactInputSingle( + subjectSourceToken, + subjectDestinationToken, + 3000, + subjectSourceQuantity, + 0 + ); + + await subject(); + + // All WBTC is sold for WETH + const currentPositions = await setToken.getPositions(); + const newFirstPosition = (await setToken.getPositions())[0]; + + expect(initialPositions.length).to.eq(1); + expect(currentPositions.length).to.eq(1); + expect(newFirstPosition.component).to.eq(destinationToken.address); + expect(newFirstPosition.unit).to.eq(expectedReceiveQuantity); + expect(newFirstPosition.module).to.eq(ADDRESS_ZERO); + }); + + describe("when fixedIn is false", async () => { + beforeEach(async () => { + fixIn = false; + + subjectSourceQuantity = sourceTokenQuantity; // Max send + subjectMinDestinationQuantity = ether(10); // Fixed out + // Exact output swaps path is reveresed + subjectData = await uniswapV3ExchangeAdapterV2.generateDataParam([setup.weth.address, setup.wbtc.address], [3000], fixIn); + }); + + it("should transfer the correct components to the SetToken", async () => { + const oldDestinationTokenBalance = await destinationToken.balanceOf(setToken.address); + + await subject(); + + const expectedDestinationTokenBalance = oldDestinationTokenBalance.add(subjectMinDestinationQuantity); + const newDestinationTokenBalance = await destinationToken.balanceOf(setToken.address); + expect(newDestinationTokenBalance).to.eq(expectedDestinationTokenBalance); + }); + + it("should transfer the correct components from the SetToken", async () => { + const oldSourceTokenBalance = await sourceToken.balanceOf(setToken.address); + const expectedSendQuantity = await uniswapV3Setup.quoter.callStatic.quoteExactOutputSingle( + subjectSourceToken, + subjectDestinationToken, + 3000, + subjectMinDestinationQuantity, + 0 + ); + + await subject(); + + const expectedSourceTokenBalance = oldSourceTokenBalance.sub(expectedSendQuantity); + const newSourceTokenBalance = await sourceToken.balanceOf(setToken.address); + expect(newSourceTokenBalance).to.eq(expectedSourceTokenBalance); + }); + + it("should update the positions on the SetToken correctly", async () => { + const initialPositions = await setToken.getPositions(); + const oldSourceTokenBalance = await sourceToken.balanceOf(setToken.address); + const expectedSendQuantity = await uniswapV3Setup.quoter.callStatic.quoteExactOutputSingle( + subjectSourceToken, + subjectDestinationToken, + 3000, + subjectMinDestinationQuantity, + 0 + ); + + await subject(); + + const currentPositions = await setToken.getPositions(); + + expect(initialPositions.length).to.eq(1); + expect(currentPositions.length).to.eq(2); + // wbtc position + expect(currentPositions[0].component).to.eq(sourceToken.address); + expect(currentPositions[0].unit).to.eq(oldSourceTokenBalance.sub(expectedSendQuantity)); + expect(currentPositions[0].module).to.eq(ADDRESS_ZERO); + // eth position + expect(currentPositions[1].component).to.eq(destinationToken.address); + expect(currentPositions[1].unit).to.eq(subjectMinDestinationQuantity); + expect(currentPositions[1].module).to.eq(ADDRESS_ZERO); + }); + }); + + describe("when path is through multiple trading pairs", async () => { + beforeEach(async () => { + await setup.weth.connect(owner.wallet).approve(uniswapV3Setup.nftPositionManager.address, ether(1000)); + await setup.dai.connect(owner.wallet).approve(uniswapV3Setup.nftPositionManager.address, ether(1000000)); + + await uniswapV3Setup.addLiquidityWide( + setup.weth, + setup.dai, + 3000, + ether(1000), + ether(1000000), + owner.address + ); + + destinationToken = setup.dai; + subjectDestinationToken = setup.dai.address; + + const tradePath = [subjectSourceToken, setup.weth.address, subjectDestinationToken]; + const fees = [3000, 3000]; + fixIn = true; + + subjectData = await uniswapV3ExchangeAdapterV2.generateDataParam(tradePath, fees, fixIn); + }); + + it("should transfer the correct components from the SetToken", async () => { + const oldSourceTokenBalance = await sourceToken.balanceOf(setToken.address); + + await subject(); + + const totalSourceQuantity = issueQuantity.mul(sourceTokenQuantity).div(ether(1)); + const expectedSourceTokenBalance = oldSourceTokenBalance.sub(totalSourceQuantity); + const newSourceTokenBalance = await sourceToken.balanceOf(setToken.address); + expect(newSourceTokenBalance).to.eq(expectedSourceTokenBalance); + }); + + it("should transfer the correct components to the SetToken", async () => { + const oldDestinationTokenBalance = await setup.dai.balanceOf(setToken.address); + const expectedReceiveQuantity = await uniswapV3Setup.quoter.callStatic.quoteExactInput( + subjectData, + subjectSourceQuantity + ); + + await subject(); + + const expectedDestinationTokenBalance = oldDestinationTokenBalance.add(expectedReceiveQuantity); + const newDestinationTokenBalance = await setup.dai.balanceOf(setToken.address); + expect(newDestinationTokenBalance).to.eq(expectedDestinationTokenBalance); + }); + + describe("when fixIn is false", async () => { + beforeEach(async () => { + subjectSourceQuantity = sourceTokenQuantity; // Max send + subjectMinDestinationQuantity = ether(10000); // Fixed out + + // Exact output swaps path is reveresed + const tradePath = [subjectDestinationToken, setup.weth.address, subjectSourceToken]; + const fees = [3000, 3000]; + fixIn = false; + + subjectData = await uniswapV3ExchangeAdapterV2.generateDataParam(tradePath, fees, fixIn); + }); + + it("should transfer the correct components from the SetToken", async () => { + const oldSourceTokenBalance = await sourceToken.balanceOf(setToken.address); + const expectedSendQuantity = await uniswapV3Setup.quoter.callStatic.quoteExactOutput( + subjectData, + subjectMinDestinationQuantity + ); + + await subject(); + + const expectedSourceTokenBalance = oldSourceTokenBalance.sub(expectedSendQuantity); + const newSourceTokenBalance = await sourceToken.balanceOf(setToken.address); + expect(newSourceTokenBalance).to.eq(expectedSourceTokenBalance); + }); + + it("should transfer the correct components to the SetToken", async () => { + const oldDestinationTokenBalance = await destinationToken.balanceOf(setToken.address); + + await subject(); + + const expectedDestinationTokenBalance = oldDestinationTokenBalance.add(subjectMinDestinationQuantity); + const newDestinationTokenBalance = await destinationToken.balanceOf(setToken.address); + expect(newDestinationTokenBalance).to.eq(expectedDestinationTokenBalance); + }); + }); + }); + }); }); describe("#removeModule", async () => { From f4df15c37127a7f8162e3300595be1d95ae5de53 Mon Sep 17 00:00:00 2001 From: 0xSachinK Date: Thu, 31 Mar 2022 21:51:04 +0530 Subject: [PATCH 6/7] Fix coverage: Remove .only from tests --- .../exchange/uniswapV3ExchangeAdapterV2.spec.ts | 10 +++++++--- test/protocol/modules/v1/tradeModule.spec.ts | 4 ++-- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/test/protocol/integration/exchange/uniswapV3ExchangeAdapterV2.spec.ts b/test/protocol/integration/exchange/uniswapV3ExchangeAdapterV2.spec.ts index baee557b7..876546535 100644 --- a/test/protocol/integration/exchange/uniswapV3ExchangeAdapterV2.spec.ts +++ b/test/protocol/integration/exchange/uniswapV3ExchangeAdapterV2.spec.ts @@ -4,7 +4,7 @@ import { solidityPack } from "ethers/lib/utils"; import { Address, Bytes } from "@utils/types"; import { Account } from "@utils/test/types"; -import { MAX_INT_256, MAX_UINT_256, ZERO } from "@utils/constants"; +import { MAX_UINT_256, ZERO } from "@utils/constants"; import { UniswapV3ExchangeAdapterV2 } from "@utils/contracts"; import DeployHelper from "@utils/deploys"; import { ether } from "@utils/index"; @@ -236,7 +236,11 @@ describe("UniswapV3ExchangeAdapterV2", () => { }); async function subject(): Promise { - return await uniswapV3ExchangeAdapter.generateDataParam([subjectToken1, subjectToken2, subjectToken3], [subjectFee1, subjectFee2], subjectFixIn); + return await uniswapV3ExchangeAdapter.generateDataParam( + [subjectToken1, subjectToken2, subjectToken3], + [subjectFee1, subjectFee2], + subjectFixIn + ); } it("should create the correct path data", async () => { @@ -268,7 +272,7 @@ describe("UniswapV3ExchangeAdapterV2", () => { }); }); - describe.only("#toBool", async () => { + describe("#toBool", async () => { let bool: boolean; let randomAddress: Address; diff --git a/test/protocol/modules/v1/tradeModule.spec.ts b/test/protocol/modules/v1/tradeModule.spec.ts index 00475768c..523cf7095 100644 --- a/test/protocol/modules/v1/tradeModule.spec.ts +++ b/test/protocol/modules/v1/tradeModule.spec.ts @@ -162,7 +162,7 @@ describe("TradeModule", () => { uniswapAdapterV2Name = "UNISWAPV2"; zeroExApiAdapterName = "ZERO_EX"; uniswapV3AdapterName = "UNISWAPV3"; - uniswapV3AdapterV2Name = "UNISWAPV3_V2" + uniswapV3AdapterV2Name = "UNISWAPV3_V2"; tradeModule = await deployer.modules.deployTradeModule(setup.controller.address); await setup.controller.addModule(tradeModule.address); @@ -1832,7 +1832,7 @@ describe("TradeModule", () => { }); }); - context.only("when trading a Default component on Uniswap V3 using UniswapV3ExchangeAdapterV2", async () => { + context("when trading a Default component on Uniswap V3 using UniswapV3ExchangeAdapterV2", async () => { let fixIn: boolean = true; cacheBeforeEach(async () => { From 75bb334646637fa31d354b3323ccc732694f7b83 Mon Sep 17 00:00:00 2001 From: 0xSachinK Date: Fri, 1 Apr 2022 11:11:55 +0530 Subject: [PATCH 7/7] Improve javadocs and comments; Add suggested changes --- .../integration/exchange/UniswapV3ExchangeAdapterV2.sol | 9 ++++++--- .../exchange/uniswapV3ExchangeAdapterV2.spec.ts | 2 +- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/contracts/protocol/integration/exchange/UniswapV3ExchangeAdapterV2.sol b/contracts/protocol/integration/exchange/UniswapV3ExchangeAdapterV2.sol index 06c8dcaa1..96b73c244 100644 --- a/contracts/protocol/integration/exchange/UniswapV3ExchangeAdapterV2.sol +++ b/contracts/protocol/integration/exchange/UniswapV3ExchangeAdapterV2.sol @@ -82,8 +82,9 @@ contract UniswapV3ExchangeAdapterV2 { view returns (address, uint256, bytes memory) { - // `_data.length` should be atleast 44. For a single hop trade, `_data.length` is 44. - // 20 source/destination token address + 3 fees + 20 source/destination tokne address + 1 fixInput bool. + // For a single hop trade, `_data.length` is 44. 20 source/destination token address + 3 fees + + // 20 source/destination token address + 1 fixInput bool. + // For multi-hop trades, `_data.length` is greater than 44. require(_data.length >= 44, "Invalid data"); bool fixInput = toBool(_data, _data.length - 1); // `fixInput` bool is stored at last byte @@ -141,7 +142,8 @@ contract UniswapV3ExchangeAdapterV2 { /** * Returns the appropriate _data argument for getTradeCalldata. Equal to the encodePacked path with the - * fee of each hop between it, e.g [token1, fee1, token2, fee2, token3]. Note: _fees.length == _path.length - 1 + * fee of each hop between it and fixInput bool at the very end., e.g [token1, fee1, token2, fee2, token3, fixIn]. + * Note: _fees.length == _path.length - 1 * * @param _path array of addresses to use as the path for the trade * @param _fees array of uint24 representing the pool fee to use for each hop @@ -168,6 +170,7 @@ contract UniswapV3ExchangeAdapterV2 { /** * Helper function to decode bytes to boolean. Similar to functions found in BytesLib. + * Note: Access modifier is set to public to enable complete testing. */ function toBool(bytes memory _bytes, uint256 _start) public pure returns (bool) { require(_start + 1 >= _start, "toBool_overflow"); diff --git a/test/protocol/integration/exchange/uniswapV3ExchangeAdapterV2.spec.ts b/test/protocol/integration/exchange/uniswapV3ExchangeAdapterV2.spec.ts index 876546535..dbd1e897f 100644 --- a/test/protocol/integration/exchange/uniswapV3ExchangeAdapterV2.spec.ts +++ b/test/protocol/integration/exchange/uniswapV3ExchangeAdapterV2.spec.ts @@ -229,7 +229,7 @@ describe("UniswapV3ExchangeAdapterV2", () => { beforeEach(async () => { subjectToken1 = setup.wbtc.address; subjectFee1 = 3000; - subjectToken2 = setup.weth.address; + subjectToken2 = setup.dai.address; subjectFee2 = 500; subjectToken3 = setup.weth.address; subjectFixIn = true;