From bc6847a6fae67ba4f12012933ca7ed11f66ca5b8 Mon Sep 17 00:00:00 2001 From: DC Date: Sun, 15 Sep 2024 15:53:13 -0700 Subject: [PATCH] contract: DFS: no approvals --- packages/contract/src/DaimoAccountV2.sol | 3 +-- packages/contract/src/DaimoFlexSwapper.sol | 22 ++++++++++++++----- .../contract/src/interfaces/IDaimoSwapper.sol | 8 +++---- .../contract/test/dummy/DaimoDummySwapper.sol | 3 +-- .../test/uniswap/DaimoFlexSwapper.t.sol | 8 +------ packages/contract/test/uniswap/Quoter.t.sol | 22 +++++++++---------- packages/daimo-contract/script/gen-feeds.ts | 9 ++++---- .../daimo-contract/src/codegen/contracts.ts | 2 -- 8 files changed, 40 insertions(+), 37 deletions(-) diff --git a/packages/contract/src/DaimoAccountV2.sol b/packages/contract/src/DaimoAccountV2.sol index 423b7ba98..f1a2e2bc2 100644 --- a/packages/contract/src/DaimoAccountV2.sol +++ b/packages/contract/src/DaimoAccountV2.sol @@ -447,11 +447,10 @@ contract DaimoAccountV2 is IAccount, Initializable, IERC1271, ReentrancyGuard { if (address(tokenIn) == address(0)) { value = amountIn; // native token } else { - tokenIn.forceApprove(address(swapper), amountIn); + tokenIn.safeTransfer(address(swapper), amountIn); } amountOut = swapper.swapToCoin{value: value}({ tokenIn: tokenIn, - amountIn: amountIn, tokenOut: tokenOut, extraData: extraData }); diff --git a/packages/contract/src/DaimoFlexSwapper.sol b/packages/contract/src/DaimoFlexSwapper.sol index 6bbd777db..213c6811a 100644 --- a/packages/contract/src/DaimoFlexSwapper.sol +++ b/packages/contract/src/DaimoFlexSwapper.sol @@ -17,6 +17,9 @@ import "./interfaces/IDaimoSwapper.sol"; /// @author The Daimo team /// @custom:security-contact security@daimo.com /// +/// For security, this contract never holds any tokens (except during a swap) +/// and does not require any token approvals. +/// /// Starts by quoting an accurate reference price from any input (token, amount) /// to a list of supported output stablecoins using Uniswap V3 TWAP/TWALs. See /// https://uniswap.org/whitepaper-v3.pdf for more on TWAP and TWAL. @@ -166,10 +169,11 @@ contract DaimoFlexSwapper is // ----- PUBLIC FUNCTIONS ----- /// Swap input to output token at a fair price. Input token 0x0 refers to - /// the native token, eg ETH. Output token cannot be 0x0. + /// the native token, eg ETH. Output token cannot be 0x0. To call this, you + /// must first send the input amount to the contract. This must be done + /// within a single transaction. (Much like the Uniswap UniversalRouter.) function swapToCoin( IERC20 tokenIn, - uint256 amountIn, IERC20 tokenOut, bytes calldata extraData ) public payable returns (uint256 swapAmountOut) { @@ -177,6 +181,16 @@ contract DaimoFlexSwapper is require(tokenIn != tokenOut, "DFS: input token = output token"); require(address(tokenOut) != address(0), "DFS: output token = 0x0"); require(isOutputToken[tokenOut], "DFS: unsupported output token"); + + // Get input amount + uint256 amountIn; + if (address(tokenIn) == address(0)) { + require(msg.value > 0, "DFS: missing msg.value"); + amountIn = msg.value; + } else { + require(msg.value == 0, "DFS: unexpected msg.value"); + amountIn = tokenIn.balanceOf(address(this)); + } require(amountIn < _MAX_UINT128, "DFS: amountIn too large"); DaimoFlexSwapperExtraData memory extra; extra = abi.decode(extraData, (DaimoFlexSwapperExtraData)); @@ -193,11 +207,9 @@ contract DaimoFlexSwapper is bytes memory callData = extra.callData; uint256 callValue = 0; if (address(tokenIn) == address(0)) { - require(msg.value == amountIn, "DFS: incorrect msg.value"); callValue = amountIn; } else { - require(msg.value == 0, "DFS: unexpected msg.value"); - tokenIn.safeTransferFrom(msg.sender, callDest, amountIn); + tokenIn.safeTransfer(callDest, amountIn); } // Execute swap diff --git a/packages/contract/src/interfaces/IDaimoSwapper.sol b/packages/contract/src/interfaces/IDaimoSwapper.sol index a99b16f64..0bc8d5270 100644 --- a/packages/contract/src/interfaces/IDaimoSwapper.sol +++ b/packages/contract/src/interfaces/IDaimoSwapper.sol @@ -4,17 +4,17 @@ pragma solidity ^0.8.12; import "openzeppelin-contracts/contracts/token/ERC20/IERC20.sol"; /// Swaps assets automatically. More precisely, it lets any market maker swap -/// swap tokens for a destination token, ensuring a fair price. The input comes -/// from msg.sender (which must have already approved) and output goes to same. +/// swap tokens for a destination token, ensuring a fair price. The input ETH or +/// ERC-20 tokens must be sent to swapper contract ahead of calling swapToCoin, +/// in the same transaction. Output tokens are sent to msg.sender. IDaimoSwapper +/// is used by other contracts; it can't be used from an EOA. interface IDaimoSwapper { /// Called to swap tokenIn to tokenOut. Ensures fair price or reverts. /// @param tokenIn input ERC-20 token, 0x0 for native token - /// @param amountIn amount to swap. For native token, must match msg.value /// @param tokenOut output ERC-20 token, cannot be 0x0 /// @param extraData swap route or similar, depending on implementation function swapToCoin( IERC20 tokenIn, - uint256 amountIn, IERC20 tokenOut, bytes calldata extraData ) external payable returns (uint256 amountOut); diff --git a/packages/contract/test/dummy/DaimoDummySwapper.sol b/packages/contract/test/dummy/DaimoDummySwapper.sol index c1f2563f5..1ca95edb5 100644 --- a/packages/contract/test/dummy/DaimoDummySwapper.sol +++ b/packages/contract/test/dummy/DaimoDummySwapper.sol @@ -30,7 +30,6 @@ contract DummySwapper is IDaimoSwapper { function swapToCoin( IERC20 tokenIn, - uint256 amountIn, IERC20 tokenOut, bytes calldata extraData ) external payable returns (uint256 amountOut) { @@ -48,7 +47,7 @@ contract DummySwapper is IDaimoSwapper { require(tokenIn == expectedTokenIn, "wrong tokenIn"); require(tokenOut == expectedTokenOut, "wrong tokenOut"); - tokenIn.transferFrom(msg.sender, address(this), amountIn); + uint256 amountIn = tokenIn.balanceOf(address(this)); if (extraData.length > 0) { // Call the reentrant swapAndTip() function diff --git a/packages/contract/test/uniswap/DaimoFlexSwapper.t.sol b/packages/contract/test/uniswap/DaimoFlexSwapper.t.sol index c17848d0b..8dde95a43 100644 --- a/packages/contract/test/uniswap/DaimoFlexSwapper.t.sol +++ b/packages/contract/test/uniswap/DaimoFlexSwapper.t.sol @@ -82,8 +82,6 @@ contract SwapperTest is Test { deal(address(weth), alice, 1e18); deal(address(degen), alice, amountIn); - degen.approve(address(swapper), amountIn); - bytes memory swapCallData = abi.encodeWithSignature( "exactInput((bytes,address,uint256,uint256))", ExactInputParams({ @@ -94,9 +92,9 @@ contract SwapperTest is Test { }) ); + degen.transfer(address(swapper), amountIn); uint256 amountOut = swapper.swapToCoin({ tokenIn: degen, - amountIn: amountIn, tokenOut: usdc, extraData: abi.encode( DaimoFlexSwapper.DaimoFlexSwapperExtraData({ @@ -153,7 +151,6 @@ contract SwapperTest is Test { vm.expectRevert(bytes("DFS: insufficient output")); swapper.swapToCoin{value: 1 ether}({ tokenIn: IERC20(address(0)), - amountIn: 1 ether, tokenOut: weth, extraData: abi.encode( DaimoFlexSwapper.DaimoFlexSwapperExtraData({ @@ -166,7 +163,6 @@ contract SwapperTest is Test { // 1:1 ETH to WETH = allowed uint256 amountOut = swapper.swapToCoin{value: 1 ether}({ tokenIn: IERC20(address(0)), - amountIn: 1 ether, tokenOut: weth, extraData: abi.encode( DaimoFlexSwapper.DaimoFlexSwapperExtraData({ @@ -185,7 +181,6 @@ contract SwapperTest is Test { vm.expectRevert(bytes("DFS: input token = output token")); swapper.swapToCoin({ tokenIn: weth, - amountIn: 1 ether, tokenOut: weth, extraData: abi.encode( DaimoFlexSwapper.DaimoFlexSwapperExtraData({ @@ -221,7 +216,6 @@ contract SwapperTest is Test { amountOut = swapper.swapToCoin{value: amountIn}({ tokenIn: IERC20(address(0)), // ETH - amountIn: amountIn, tokenOut: usdc, extraData: abi.encode( DaimoFlexSwapper.DaimoFlexSwapperExtraData({ diff --git a/packages/contract/test/uniswap/Quoter.t.sol b/packages/contract/test/uniswap/Quoter.t.sol index 9e14d6374..404b0cffa 100644 --- a/packages/contract/test/uniswap/Quoter.t.sol +++ b/packages/contract/test/uniswap/Quoter.t.sol @@ -97,42 +97,42 @@ contract QuoterTest is Test { // $3000.00 = 1 ETH, wrong price = block swap fakeFeedETHUSD.setPrice(300000, 2); vm.expectRevert(bytes("DFS: quote sanity check failed")); - swapper.swapToCoin(eth, 1 ether, usdc, emptySwapData()); + swapper.swapToCoin{value: 1 ether}(eth, usdc, emptySwapData()); // $3450.00 = 1 ETH, ~correct price = OK, attempt swap fakeFeedETHUSD.setPrice(345000, 2); vm.expectRevert(bytes("DFS: swap produced no output")); - swapper.swapToCoin{value: 1 ether}(eth, 1 ether, usdc, emptySwapData()); + swapper.swapToCoin{value: 1 ether}(eth, usdc, emptySwapData()); // Feed returning stale or missing price = block swap fakeFeedETHUSD.setPrice(0, 0); vm.expectRevert(bytes("DFS: CL price <= 0")); - swapper.swapToCoin{value: 1 ether}(eth, 1 ether, usdc, emptySwapData()); + swapper.swapToCoin{value: 1 ether}(eth, usdc, emptySwapData()); // No price feed = OK, attempt swap swapper.setKnownToken(weth, zeroToken); vm.expectRevert(bytes("DFS: swap produced no output")); - swapper.swapToCoin{value: 1 ether}(eth, 1 ether, usdc, emptySwapData()); + swapper.swapToCoin{value: 1 ether}(eth, usdc, emptySwapData()); } function testChainlinkSanityCheckERC20() public { deal(address(degen), address(this), 10e18); - degen.approve(address(swapper), 10e18); + degen.transfer(address(swapper), 1e18); // $0.50 = 1 DEGEN, wrong price = block swap fakeFeedDEGENUSD.setPrice(500, 3); vm.expectRevert(bytes("DFS: quote sanity check failed")); - swapper.swapToCoin(degen, 1e18, usdc, emptySwapData()); + swapper.swapToCoin(degen, usdc, emptySwapData()); // $0.0086 = 1 DEGEN, ~correct price = OK, attempt swap fakeFeedDEGENUSD.setPrice(8600, 6); vm.expectRevert(bytes("DFS: swap produced no output")); - swapper.swapToCoin(degen, 1 ether, usdc, emptySwapData()); + swapper.swapToCoin(degen, usdc, emptySwapData()); // No price = OK, attempt swap swapper.setKnownToken(degen, zeroToken); vm.expectRevert(bytes("DFS: swap produced no output")); - swapper.swapToCoin(degen, 1e18, usdc, emptySwapData()); + swapper.swapToCoin(degen, usdc, emptySwapData()); } function testFlexSwapperQuote() public view { @@ -154,14 +154,14 @@ contract QuoterTest is Test { function testRebasingToken() public { IERC20 usdm = IERC20(0x28eD8909de1b3881400413Ea970ccE377a004ccA); deal(address(usdm), address(this), 123e18); - usdm.approve(address(swapper), 123e18); + usdm.transfer(address(swapper), 123e18); // Protocol lets you unwrap 123 USDM for 122 USDC = within 1% of 1:1 bytes memory callData = fakeSwapData(usdm, usdc, 123e18, 122e6); // Initially, swap fails because USDM is a rebasing token, no Uni price. vm.expectRevert(bytes("DFS: no path found, amountOut 0")); - swapper.swapToCoin(usdm, 123e18, usdc, callData); + swapper.swapToCoin(usdm, usdc, callData); // Give USDM a price feed + skip Uniswap. Swap should succeed. FakeAggregator fakeFeedUSDM = new FakeAggregator(); @@ -174,7 +174,7 @@ contract QuoterTest is Test { skipUniswap: true }) ); - swapper.swapToCoin(usdm, 123e18, usdc, callData); + swapper.swapToCoin(usdm, usdc, callData); assertEq(usdm.balanceOf(address(this)), 0); assertEq(usdc.balanceOf(address(this)), 122e6); diff --git a/packages/daimo-contract/script/gen-feeds.ts b/packages/daimo-contract/script/gen-feeds.ts index 7c67f55f0..cf2a5f3df 100644 --- a/packages/daimo-contract/script/gen-feeds.ts +++ b/packages/daimo-contract/script/gen-feeds.ts @@ -1,4 +1,4 @@ -import { assert, debugJson, optimismWETH } from "@daimo/common"; +import { assert, assertNotNull, debugJson } from "@daimo/common"; import fs from "node:fs/promises"; import { Address, @@ -12,6 +12,7 @@ import { import { arbitrum, base, mainnet, optimism, polygon } from "viem/chains"; import { ChainlinkFeed, PricedToken } from "./scriptModels"; +import { getDAv2Chain } from "../dist"; import { aggregatorV2V3InterfaceABI, arbitrumWETH, @@ -20,7 +21,7 @@ import { erc20ABI, ethereumWETH, ForeignToken, - getAccountChain, + optimismWETH, polygonWETH, polygonWMATIC, } from "../src"; @@ -289,7 +290,7 @@ async function priceTokens( blockTimestamp = Number(block.timestamp); } - const accChain = getAccountChain(token.chainId); + const accChain = getDAv2Chain(token.chainId); const usdcAddr = accChain.bridgeCoin.token; // Get Chainlink price feed for this token, if available @@ -414,7 +415,7 @@ async function quoteToken({ chainId: token.chainId, tokenSymbol: token.symbol, tokenAddress, - tokenName: token.name, + tokenName: assertNotNull(token.name), tokenDecimals: tokenDec, logoURI: token.logoURI, blockNumber, diff --git a/packages/daimo-contract/src/codegen/contracts.ts b/packages/daimo-contract/src/codegen/contracts.ts index 8f31a8532..4dfbcfc17 100644 --- a/packages/daimo-contract/src/codegen/contracts.ts +++ b/packages/daimo-contract/src/codegen/contracts.ts @@ -2034,7 +2034,6 @@ export const daimoFlexSwapperABI = [ type: 'function', inputs: [ { name: 'tokenIn', internalType: 'contract IERC20', type: 'address' }, - { name: 'amountIn', internalType: 'uint256', type: 'uint256' }, { name: 'tokenOut', internalType: 'contract IERC20', type: 'address' }, { name: 'extraData', internalType: 'bytes', type: 'bytes' }, ], @@ -2943,7 +2942,6 @@ export const dummySwapperABI = [ type: 'function', inputs: [ { name: 'tokenIn', internalType: 'contract IERC20', type: 'address' }, - { name: 'amountIn', internalType: 'uint256', type: 'uint256' }, { name: 'tokenOut', internalType: 'contract IERC20', type: 'address' }, { name: 'extraData', internalType: 'bytes', type: 'bytes' }, ],