Skip to content

Commit 8cbda68

Browse files
dorothy-zbornakdorothy-zbornak
authored
ZeroExAdapter null recipient fix (#93)
1 parent 20c1b74 commit 8cbda68

File tree

4 files changed

+155
-24
lines changed

4 files changed

+155
-24
lines changed

‎contracts/protocol/integration/exchange/ZeroExApiAdapter.sol‎

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,14 +67,17 @@ contract ZeroExApiAdapter {
6767

6868
// Address of the deployed ZeroEx contract.
6969
address public immutable zeroExAddress;
70+
// Address of the WETH9 contract.
71+
address public immutable wethAddress;
7072

7173
// Returns the address to approve source tokens to for trading. This is the TokenTaker address
7274
address public immutable getSpender;
7375

7476
/* ============ constructor ============ */
7577

76-
constructor(address _zeroExAddress) public {
78+
constructor(address _zeroExAddress, address _wethAddress) public {
7779
zeroExAddress = _zeroExAddress;
80+
wethAddress = _wethAddress;
7881
getSpender = _zeroExAddress;
7982
}
8083

@@ -135,6 +138,9 @@ contract ZeroExApiAdapter {
135138
(inputToken, outputToken, , recipient, inputTokenAmount, minOutputTokenAmount) =
136139
abi.decode(_data[4:], (address, address, address, address, uint256, uint256));
137140
supportsRecipient = true;
141+
if (recipient == address(0)) {
142+
recipient = _destinationAddress;
143+
}
138144
} else if (selector == 0xd9627aa4) {
139145
// sellToUniswap()
140146
address[] memory path;
@@ -166,22 +172,34 @@ contract ZeroExApiAdapter {
166172
(encodedPath, inputTokenAmount, minOutputTokenAmount, recipient) =
167173
abi.decode(_data[4:], (bytes, uint256, uint256, address));
168174
supportsRecipient = true;
175+
if (recipient == address(0)) {
176+
recipient = _destinationAddress;
177+
}
169178
(inputToken, outputToken) = _decodeTokensFromUniswapV3EncodedPath(encodedPath);
170179
} else if (selector == 0x803ba26d) {
171180
// sellTokenForEthToUniswapV3()
172181
bytes memory encodedPath;
173182
(encodedPath, inputTokenAmount, minOutputTokenAmount, recipient) =
174183
abi.decode(_data[4:], (bytes, uint256, uint256, address));
175184
supportsRecipient = true;
185+
if (recipient == address(0)) {
186+
recipient = _destinationAddress;
187+
}
176188
(inputToken, outputToken) = _decodeTokensFromUniswapV3EncodedPath(encodedPath);
189+
require(outputToken == wethAddress, "Last token must be WETH");
190+
outputToken = ETH_ADDRESS;
177191
} else if (selector == 0x3598d8ab) {
178192
// sellEthForTokenToUniswapV3()
179193
inputTokenAmount = _sourceQuantity;
180194
bytes memory encodedPath;
181195
(encodedPath, minOutputTokenAmount, recipient) =
182196
abi.decode(_data[4:], (bytes, uint256, address));
183197
supportsRecipient = true;
184-
(, outputToken) = _decodeTokensFromUniswapV3EncodedPath(encodedPath);
198+
if (recipient == address(0)) {
199+
recipient = _destinationAddress;
200+
}
201+
(inputToken, outputToken) = _decodeTokensFromUniswapV3EncodedPath(encodedPath);
202+
require(inputToken == wethAddress, "First token must be WETH");
185203
inputToken = ETH_ADDRESS;
186204
} else {
187205
revert("Unsupported 0xAPI function selector");

‎test/protocol/integration/exchange/zeroExApiAdapter.spec.ts‎

Lines changed: 132 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ describe("ZeroExApiAdapter", () => {
1515
const sourceToken = "0x6cf5f1d59fddae3a688210953a512b6aee6ea643";
1616
const destToken = "0x5e5d0bea9d4a15db2d0837aff0435faba166190d";
1717
const otherToken = "0xae9902bb655de1a67f334d8661b3ae6a96723d5b";
18+
const wethToken = "0xc02aaa39b223fe8d0a0e5c4f27ead9083c756cc2";
1819
const extraHopToken = "0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48";
1920
const destination = "0x89b3515cad4f23c1deacea79fc12445cc21bd0e1";
2021
const otherDestination = "0xdeb100c55cccfd6e39753f78c8b0c3bcbef86157";
@@ -33,7 +34,7 @@ describe("ZeroExApiAdapter", () => {
3334

3435
// Mock OneInch exchange that allows for only fixed exchange amounts
3536
zeroExMock = await deployer.mocks.deployZeroExMock(ADDRESS_ZERO, ADDRESS_ZERO, ZERO, ZERO);
36-
zeroExApiAdapter = await deployer.adapters.deployZeroExApiAdapter(zeroExMock.address);
37+
zeroExApiAdapter = await deployer.adapters.deployZeroExApiAdapter(zeroExMock.address, wethToken);
3738
});
3839

3940
addSnapshotBeforeRestoreAfterEach();
@@ -305,6 +306,26 @@ describe("ZeroExApiAdapter", () => {
305306
expect(_data).to.deep.eq(data);
306307
});
307308

309+
it("permits any destination address when recipient is null", async () => {
310+
const data = zeroExMock.interface.encodeFunctionData("sellToLiquidityProvider", [
311+
sourceToken,
312+
destToken,
313+
ADDRESS_ZERO,
314+
ADDRESS_ZERO,
315+
sourceQuantity,
316+
minDestinationQuantity,
317+
EMPTY_BYTES,
318+
]);
319+
await zeroExApiAdapter.getTradeCalldata(
320+
sourceToken,
321+
destToken,
322+
destination,
323+
sourceQuantity,
324+
minDestinationQuantity,
325+
data,
326+
);
327+
});
328+
308329
it("rejects wrong input token", async () => {
309330
const data = zeroExMock.interface.encodeFunctionData("sellToLiquidityProvider", [
310331
otherToken,
@@ -659,6 +680,7 @@ describe("ZeroExApiAdapter", () => {
659680

660681
describe("sellTokenForTokenToUniswapV3", () => {
661682
const additionalHops = [otherToken, extraHopToken];
683+
662684
for (let i = 0; i <= additionalHops.length; i++) {
663685
const hops = take(additionalHops, i);
664686
it(`validates data for ${i + 1} hops`, async () => {
@@ -684,6 +706,25 @@ describe("ZeroExApiAdapter", () => {
684706
});
685707
}
686708

709+
it("permits any destination address when recipient is null", async () => {
710+
const path = [sourceToken, destToken];
711+
712+
const data = zeroExMock.interface.encodeFunctionData("sellTokenForTokenToUniswapV3", [
713+
encodePath(path),
714+
sourceQuantity,
715+
minDestinationQuantity,
716+
ADDRESS_ZERO,
717+
]);
718+
await zeroExApiAdapter.getTradeCalldata(
719+
sourceToken,
720+
destToken,
721+
destination,
722+
sourceQuantity,
723+
minDestinationQuantity,
724+
data,
725+
);
726+
});
727+
687728
it("rejects wrong input token", async () => {
688729
const data = zeroExMock.interface.encodeFunctionData("sellTokenForTokenToUniswapV3", [
689730
encodePath([otherToken, destToken]),
@@ -779,12 +820,12 @@ describe("ZeroExApiAdapter", () => {
779820
encodePath([sourceToken, destToken]),
780821
sourceQuantity,
781822
minDestinationQuantity,
782-
ADDRESS_ZERO,
823+
destination,
783824
]);
784825
const tx = zeroExApiAdapter.getTradeCalldata(
785826
sourceToken,
786827
destToken,
787-
destination,
828+
otherDestination,
788829
sourceQuantity,
789830
minDestinationQuantity,
790831
data,
@@ -798,7 +839,7 @@ describe("ZeroExApiAdapter", () => {
798839
for (let i = 0; i <= additionalHops.length; i++) {
799840
const hops = take(additionalHops, i);
800841
it(`validates data for ${i + 1} hops`, async () => {
801-
const path = [sourceToken, ...hops, ETH_ADDRESS];
842+
const path = [sourceToken, ...hops, wethToken];
802843

803844
const data = zeroExMock.interface.encodeFunctionData("sellTokenForEthToUniswapV3", [
804845
encodePath(path),
@@ -820,9 +861,28 @@ describe("ZeroExApiAdapter", () => {
820861
});
821862
}
822863

864+
it("permits any destination address when recipient is null", async () => {
865+
const path = [sourceToken, wethToken];
866+
867+
const data = zeroExMock.interface.encodeFunctionData("sellTokenForEthToUniswapV3", [
868+
encodePath(path),
869+
sourceQuantity,
870+
minDestinationQuantity,
871+
ADDRESS_ZERO,
872+
]);
873+
await zeroExApiAdapter.getTradeCalldata(
874+
sourceToken,
875+
ETH_ADDRESS,
876+
destination,
877+
sourceQuantity,
878+
minDestinationQuantity,
879+
data,
880+
);
881+
});
882+
823883
it("rejects wrong input token", async () => {
824884
const data = zeroExMock.interface.encodeFunctionData("sellTokenForEthToUniswapV3", [
825-
encodePath([otherToken, ETH_ADDRESS]),
885+
encodePath([otherToken, wethToken]),
826886
sourceQuantity,
827887
minDestinationQuantity,
828888
destination,
@@ -839,6 +899,24 @@ describe("ZeroExApiAdapter", () => {
839899
});
840900

841901
it("rejects wrong output token", async () => {
902+
const data = zeroExMock.interface.encodeFunctionData("sellTokenForEthToUniswapV3", [
903+
encodePath([sourceToken, wethToken]),
904+
sourceQuantity,
905+
minDestinationQuantity,
906+
destination,
907+
]);
908+
const tx = zeroExApiAdapter.getTradeCalldata(
909+
sourceToken,
910+
otherToken,
911+
destination,
912+
sourceQuantity,
913+
minDestinationQuantity,
914+
data,
915+
);
916+
await expect(tx).to.be.revertedWith("Mismatched output token");
917+
});
918+
919+
it("rejects non-WETH output token in encoded path", async () => {
842920
const data = zeroExMock.interface.encodeFunctionData("sellTokenForEthToUniswapV3", [
843921
encodePath([sourceToken, otherToken]),
844922
sourceQuantity,
@@ -853,12 +931,12 @@ describe("ZeroExApiAdapter", () => {
853931
minDestinationQuantity,
854932
data,
855933
);
856-
await expect(tx).to.be.revertedWith("Mismatched output token");
934+
await expect(tx).to.be.revertedWith("Last token must be WETH");
857935
});
858936

859937
it("rejects wrong input token quantity", async () => {
860938
const data = zeroExMock.interface.encodeFunctionData("sellTokenForEthToUniswapV3", [
861-
encodePath([sourceToken, ETH_ADDRESS]),
939+
encodePath([sourceToken, wethToken]),
862940
otherQuantity,
863941
minDestinationQuantity,
864942
destination,
@@ -876,7 +954,7 @@ describe("ZeroExApiAdapter", () => {
876954

877955
it("rejects wrong output token quantity", async () => {
878956
const data = zeroExMock.interface.encodeFunctionData("sellTokenForEthToUniswapV3", [
879-
encodePath([sourceToken, ETH_ADDRESS]),
957+
encodePath([sourceToken, wethToken]),
880958
sourceQuantity,
881959
otherQuantity,
882960
destination,
@@ -912,15 +990,15 @@ describe("ZeroExApiAdapter", () => {
912990

913991
it("rejects wrong destination", async () => {
914992
const data = zeroExMock.interface.encodeFunctionData("sellTokenForEthToUniswapV3", [
915-
encodePath([sourceToken, ETH_ADDRESS]),
993+
encodePath([sourceToken, wethToken]),
916994
sourceQuantity,
917995
minDestinationQuantity,
918-
ADDRESS_ZERO,
996+
destination,
919997
]);
920998
const tx = zeroExApiAdapter.getTradeCalldata(
921999
sourceToken,
9221000
ETH_ADDRESS,
923-
destination,
1001+
otherDestination,
9241002
sourceQuantity,
9251003
minDestinationQuantity,
9261004
data,
@@ -934,7 +1012,7 @@ describe("ZeroExApiAdapter", () => {
9341012
for (let i = 0; i <= additionalHops.length; i++) {
9351013
const hops = take(additionalHops, i);
9361014
it(`validates data for ${i + 1} hops`, async () => {
937-
const path = [ETH_ADDRESS, ...hops, destToken];
1015+
const path = [wethToken, ...hops, destToken];
9381016

9391017
const data = zeroExMock.interface.encodeFunctionData("sellEthForTokenToUniswapV3", [
9401018
encodePath(path),
@@ -955,9 +1033,44 @@ describe("ZeroExApiAdapter", () => {
9551033
});
9561034
}
9571035

1036+
it("permits any destination address when recipient is null", async () => {
1037+
const path = [wethToken, destToken];
1038+
1039+
const data = zeroExMock.interface.encodeFunctionData("sellEthForTokenToUniswapV3", [
1040+
encodePath(path),
1041+
minDestinationQuantity,
1042+
ADDRESS_ZERO,
1043+
]);
1044+
await zeroExApiAdapter.getTradeCalldata(
1045+
ETH_ADDRESS,
1046+
destToken,
1047+
destination,
1048+
sourceQuantity,
1049+
minDestinationQuantity,
1050+
data,
1051+
);
1052+
});
1053+
1054+
it("rejects non-WETH input token in encoded path", async () => {
1055+
const data = zeroExMock.interface.encodeFunctionData("sellEthForTokenToUniswapV3", [
1056+
encodePath([otherToken, destToken]),
1057+
minDestinationQuantity,
1058+
destination,
1059+
]);
1060+
const tx = zeroExApiAdapter.getTradeCalldata(
1061+
ETH_ADDRESS,
1062+
destToken,
1063+
destination,
1064+
sourceQuantity,
1065+
minDestinationQuantity,
1066+
data,
1067+
);
1068+
await expect(tx).to.be.revertedWith("First token must be WETH");
1069+
});
1070+
9581071
it("rejects wrong input token", async () => {
9591072
const data = zeroExMock.interface.encodeFunctionData("sellEthForTokenToUniswapV3", [
960-
encodePath([ETH_ADDRESS, destToken]),
1073+
encodePath([wethToken, destToken]),
9611074
minDestinationQuantity,
9621075
destination,
9631076
]);
@@ -974,7 +1087,7 @@ describe("ZeroExApiAdapter", () => {
9741087

9751088
it("rejects wrong output token", async () => {
9761089
const data = zeroExMock.interface.encodeFunctionData("sellEthForTokenToUniswapV3", [
977-
encodePath([ETH_ADDRESS, otherToken]),
1090+
encodePath([wethToken, otherToken]),
9781091
minDestinationQuantity,
9791092
destination,
9801093
]);
@@ -991,7 +1104,7 @@ describe("ZeroExApiAdapter", () => {
9911104

9921105
it("rejects wrong output token quantity", async () => {
9931106
const data = zeroExMock.interface.encodeFunctionData("sellEthForTokenToUniswapV3", [
994-
encodePath([ETH_ADDRESS, destToken]),
1107+
encodePath([wethToken, destToken]),
9951108
otherQuantity,
9961109
destination,
9971110
]);
@@ -1008,7 +1121,7 @@ describe("ZeroExApiAdapter", () => {
10081121

10091122
it("rejects invalid uniswap path", async () => {
10101123
const data = zeroExMock.interface.encodeFunctionData("sellEthForTokenToUniswapV3", [
1011-
encodePath([ETH_ADDRESS]),
1124+
encodePath([wethToken]),
10121125
minDestinationQuantity,
10131126
destination,
10141127
]);
@@ -1025,14 +1138,14 @@ describe("ZeroExApiAdapter", () => {
10251138

10261139
it("rejects wrong destination", async () => {
10271140
const data = zeroExMock.interface.encodeFunctionData("sellEthForTokenToUniswapV3", [
1028-
encodePath([ETH_ADDRESS, destToken]),
1141+
encodePath([wethToken, destToken]),
10291142
minDestinationQuantity,
1030-
ADDRESS_ZERO,
1143+
destination,
10311144
]);
10321145
const tx = zeroExApiAdapter.getTradeCalldata(
10331146
ETH_ADDRESS,
10341147
destToken,
1035-
destination,
1148+
otherDestination,
10361149
sourceQuantity,
10371150
minDestinationQuantity,
10381151
data,

‎test/protocol/modules/tradeModule.spec.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ describe("TradeModule", () => {
132132
BigNumber.from(100000000), // 1 WBTC
133133
wbtcRate, // Trades for 33 WETH
134134
);
135-
zeroExApiAdapter = await deployer.adapters.deployZeroExApiAdapter(zeroExMock.address);
135+
zeroExApiAdapter = await deployer.adapters.deployZeroExApiAdapter(zeroExMock.address, setup.weth.address);
136136

137137

138138
kyberAdapterName = "KYBER";

‎utils/deploys/deployAdapters.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -173,8 +173,8 @@ export default class DeployAdapters {
173173
return await new UniswapPairPriceAdapter__factory(this._deployerSigner).attach(uniswapAdapterAddress);
174174
}
175175

176-
public async deployZeroExApiAdapter(zeroExAddress: Address): Promise<ZeroExApiAdapter> {
177-
return await new ZeroExApiAdapter__factory(this._deployerSigner).deploy(zeroExAddress);
176+
public async deployZeroExApiAdapter(zeroExAddress: Address, wethAddress: Address): Promise<ZeroExApiAdapter> {
177+
return await new ZeroExApiAdapter__factory(this._deployerSigner).deploy(zeroExAddress, wethAddress);
178178
}
179179

180180
public async deploySnapshotGovernanceAdapter(delegateRegistry: Address): Promise<SnapshotGovernanceAdapter> {

0 commit comments

Comments
 (0)