diff --git a/src/ForeignController.sol b/src/ForeignController.sol index 2a33a2a..d4cd139 100644 --- a/src/ForeignController.sol +++ b/src/ForeignController.sol @@ -371,7 +371,26 @@ contract ForeignController is AccessControl { /*** Internal helper functions ***/ /**********************************************************************************************/ + // NOTE: This logic was inspired by OpenZeppelin's forceApprove in SafeERC20 library function _approve(address token, address spender, uint256 amount) internal { + bytes memory approveData = abi.encodeCall(IERC20.approve, (spender, amount)); + + // Call doCall on proxy to approve the token + ( bool success, bytes memory data ) + = address(proxy).call(abi.encodeCall(IALMProxy.doCall, (token, approveData))); + + // Decode the first 32 bytes of the data, ALMProxy returns 96 bytes + bytes32 result; + assembly { result := mload(add(data, 32)) } + + // Decode the result to check if the approval was successful + bool decodedSuccess = (data.length == 0) || result != bytes32(0); + + // If call succeeded with expected calldata, return + if (success && decodedSuccess) return; + + // If call reverted, set to zero and try again + proxy.doCall(token, abi.encodeCall(IERC20.approve, (spender, 0))); proxy.doCall(token, abi.encodeCall(IERC20.approve, (spender, amount))); } diff --git a/src/MainnetController.sol b/src/MainnetController.sol index 6552efe..75b2f28 100644 --- a/src/MainnetController.sol +++ b/src/MainnetController.sol @@ -799,7 +799,26 @@ contract MainnetController is AccessControl { /*** Relayer helper functions ***/ /**********************************************************************************************/ + // NOTE: This logic was inspired by OpenZeppelin's forceApprove in SafeERC20 library function _approve(address token, address spender, uint256 amount) internal { + bytes memory approveData = abi.encodeCall(IERC20.approve, (spender, amount)); + + // Call doCall on proxy to approve the token + ( bool success, bytes memory data ) + = address(proxy).call(abi.encodeCall(IALMProxy.doCall, (token, approveData))); + + // Decode the first 32 bytes of the data, ALMProxy returns 96 bytes + bytes32 result; + assembly { result := mload(add(data, 32)) } + + // Decode the result to check if the approval was successful + bool decodedSuccess = (data.length == 0) || result != bytes32(0); + + // If call succeeded with expected calldata, return + if (success && decodedSuccess) return; + + // If call reverted, set to zero and try again + proxy.doCall(token, abi.encodeCall(IERC20.approve, (spender, 0))); proxy.doCall(token, abi.encodeCall(IERC20.approve, (spender, amount))); } diff --git a/src/libraries/CCTPLib.sol b/src/libraries/CCTPLib.sol index 2de8aab..7a027dc 100644 --- a/src/libraries/CCTPLib.sol +++ b/src/libraries/CCTPLib.sol @@ -91,6 +91,7 @@ library CCTPLib { /*** Relayer helper functions ***/ /**********************************************************************************************/ + // NOTE: As USDC is the only asset transferred using CCTP, _forceApprove logic is unnecessary. function _approve( IALMProxy proxy, address token, @@ -134,7 +135,7 @@ library CCTPLib { /**********************************************************************************************/ /*** Rate Limit helper functions ***/ /**********************************************************************************************/ - + function _rateLimited(IRateLimits rateLimits, bytes32 key, uint256 amount) internal { rateLimits.triggerRateLimitDecrease(key, amount); } diff --git a/src/libraries/CurveLib.sol b/src/libraries/CurveLib.sol index 22856af..de12cbd 100644 --- a/src/libraries/CurveLib.sol +++ b/src/libraries/CurveLib.sol @@ -277,6 +277,24 @@ library CurveLib { ) internal { + bytes memory approveData = abi.encodeCall(IERC20.approve, (spender, amount)); + + // Call doCall on proxy to approve the token + ( bool success, bytes memory data ) + = address(proxy).call(abi.encodeCall(IALMProxy.doCall, (token, approveData))); + + // Decode the first 32 bytes of the data, ALMProxy returns 96 bytes + bytes32 result; + assembly { result := mload(add(data, 32)) } + + // Decode the result to check if the approval was successful + bool decodedSuccess = (data.length == 0) || result != bytes32(0); + + // If call succeeded with expected calldata, return + if (success && decodedSuccess) return; + + // If call reverted, set to zero and try again + proxy.doCall(token, abi.encodeCall(IERC20.approve, (spender, 0))); proxy.doCall(token, abi.encodeCall(IERC20.approve, (spender, amount))); } diff --git a/src/libraries/PSMLib.sol b/src/libraries/PSMLib.sol index 4b771a2..f361b9f 100644 --- a/src/libraries/PSMLib.sol +++ b/src/libraries/PSMLib.sol @@ -128,6 +128,8 @@ library PSMLib { /*** Helper functions ***/ /**********************************************************************************************/ + // NOTE: As swaps are only done between USDC and USDS and vice versa, using `_forceApprove` + // is unnecessary. function _approve( IALMProxy proxy, address token, diff --git a/test/mainnet-fork/Approve.t.sol b/test/mainnet-fork/Approve.t.sol new file mode 100644 index 0000000..5a4f7c0 --- /dev/null +++ b/test/mainnet-fork/Approve.t.sol @@ -0,0 +1,218 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later +pragma solidity >=0.8.0; + +import "./ForkTestBase.t.sol"; + +import { ForeignController } from "../../src/ForeignController.sol"; +import { MainnetController } from "../../src/MainnetController.sol"; + +import { CurveLib } from "../../src/libraries/CurveLib.sol"; + +import { IALMProxy } from "../../src/interfaces/IALMProxy.sol"; + +interface IHarness { + function approve(address token, address spender, uint256 amount) external; + function approveCurve(address proxy, address token, address spender, uint256 amount) external; +} + +contract MainnetControllerHarness is MainnetController { + + using CurveLib for IALMProxy; + + constructor( + address admin_, + address proxy_, + address rateLimits_, + address vault_, + address psm_, + address daiUsds_, + address cctp_ + ) MainnetController(admin_, proxy_, rateLimits_, vault_, psm_, daiUsds_, cctp_) {} + + function approve(address token, address spender, uint256 amount) external { + _approve(token, spender, amount); + } + + function approveCurve(address proxy, address token, address spender, uint256 amount) external { + IALMProxy(proxy)._approve(token, spender, amount); + } + +} + +contract ForeignControllerHarness is ForeignController { + + constructor( + address admin_, + address proxy_, + address rateLimits_, + address psm_, + address usdc_, + address cctp_ + ) ForeignController(admin_, proxy_, rateLimits_, psm_, usdc_, cctp_) {} + + function approve(address token, address spender, uint256 amount) external { + _approve(token, spender, amount); + } + +} + +contract ApproveTestBase is ForkTestBase { + + function _approveTest(address token, address harness) internal { + address spender = makeAddr("spender"); + + assertEq(IERC20(token).allowance(harness, spender), 0); + + IHarness(harness).approve(token, spender, 100); + + assertEq(IERC20(token).allowance(address(almProxy), spender), 100); + + IHarness(harness).approve(token, spender, 200); // Would revert without setting to zero + + assertEq(IERC20(token).allowance(address(almProxy), spender), 200); + } + + function _approveCurveTest(address token, address harness) internal { + address spender = makeAddr("spender"); + + assertEq(IERC20(token).allowance(harness, spender), 0); + + IHarness(harness).approveCurve(address(almProxy), token, spender, 100); + + assertEq(IERC20(token).allowance(address(almProxy), spender), 100); + + IHarness(harness).approveCurve(address(almProxy), token, spender, 200); // Would revert without setting to zero + + assertEq(IERC20(token).allowance(address(almProxy), spender), 200); + } + +} + +contract MainnetControllerApproveSuccessTests is ApproveTestBase { + + address harness; + + function setUp() public virtual override { + super.setUp(); + + MainnetControllerHarness harnessCode = new MainnetControllerHarness( + SPARK_PROXY, + address(mainnetController.proxy()), + address(mainnetController.rateLimits()), + address(mainnetController.vault()), + address(mainnetController.psm()), + address(mainnetController.daiUsds()), + address(mainnetController.cctp()) + ); + + vm.etch(address(mainnetController), address(harnessCode).code); + + harness = address(MainnetControllerHarness(address(mainnetController))); + } + + function test_approveTokens() public { + _approveTest(Ethereum.CBBTC, harness); + _approveTest(Ethereum.DAI, harness); + _approveTest(Ethereum.GNO, harness); + _approveTest(Ethereum.MKR, harness); + _approveTest(Ethereum.RETH, harness); + _approveTest(Ethereum.SDAI, harness); + _approveTest(Ethereum.SUSDE, harness); + _approveTest(Ethereum.SUSDS, harness); + _approveTest(Ethereum.USDC, harness); + _approveTest(Ethereum.USDE, harness); + _approveTest(Ethereum.USDS, harness); + _approveTest(Ethereum.USCC, harness); + _approveTest(Ethereum.USDT, harness); + _approveTest(Ethereum.USTB, harness); + _approveTest(Ethereum.WBTC, harness); + _approveTest(Ethereum.WEETH, harness); + _approveTest(Ethereum.WETH, harness); + _approveTest(Ethereum.WSTETH, harness); + } + + function test_approveCurveTokens() public { + _approveCurveTest(Ethereum.CBBTC, harness); + _approveCurveTest(Ethereum.DAI, harness); + _approveCurveTest(Ethereum.GNO, harness); + _approveCurveTest(Ethereum.MKR, harness); + _approveCurveTest(Ethereum.RETH, harness); + _approveCurveTest(Ethereum.SDAI, harness); + _approveCurveTest(Ethereum.SUSDE, harness); + _approveCurveTest(Ethereum.SUSDS, harness); + _approveCurveTest(Ethereum.USDC, harness); + _approveCurveTest(Ethereum.USDE, harness); + _approveCurveTest(Ethereum.USDS, harness); + _approveCurveTest(Ethereum.USCC, harness); + _approveCurveTest(Ethereum.USDT, harness); + _approveCurveTest(Ethereum.USTB, harness); + _approveCurveTest(Ethereum.WBTC, harness); + _approveCurveTest(Ethereum.WEETH, harness); + _approveCurveTest(Ethereum.WETH, harness); + _approveCurveTest(Ethereum.WSTETH, harness); + } + +} + +// NOTE: This code is running against mainnet, but is used to demonstrate equivalent approve behaviour +// for USDT-type contracts. Because of this, the foreignController has to be onboarded in the same +// way as the mainnetController. +contract ForeignControllerApproveSuccessTests is ApproveTestBase { + + address harness; + + function setUp() public virtual override { + super.setUp(); + + // NOTE: This etching setup is necessary to get coverage to work + + ForeignController foreignController = new ForeignController( + SPARK_PROXY, + address(almProxy), + makeAddr("rateLimits"), + makeAddr("psm"), + makeAddr("usdc"), + makeAddr("cctp") + ); + + ForeignControllerHarness harnessCode = new ForeignControllerHarness( + SPARK_PROXY, + address(almProxy), + makeAddr("rateLimits"), + makeAddr("psm"), + makeAddr("usdc"), + makeAddr("cctp") + ); + + // Allow the foreign controller to call the ALMProxy + vm.startPrank(SPARK_PROXY); + almProxy.grantRole(almProxy.CONTROLLER(), address(foreignController)); + vm.stopPrank(); + + vm.etch(address(foreignController), address(harnessCode).code); + + harness = address(ForeignControllerHarness(address(foreignController))); + } + + function test_approveTokens() public { + _approveTest(Ethereum.CBBTC, harness); + _approveTest(Ethereum.DAI, harness); + _approveTest(Ethereum.GNO, harness); + _approveTest(Ethereum.MKR, harness); + _approveTest(Ethereum.RETH, harness); + _approveTest(Ethereum.SDAI, harness); + _approveTest(Ethereum.SUSDE, harness); + _approveTest(Ethereum.SUSDS, harness); + _approveTest(Ethereum.USDC, harness); + _approveTest(Ethereum.USDE, harness); + _approveTest(Ethereum.USDS, harness); + _approveTest(Ethereum.USCC, harness); + _approveTest(Ethereum.USDT, harness); + _approveTest(Ethereum.USTB, harness); + _approveTest(Ethereum.WBTC, harness); + _approveTest(Ethereum.WEETH, harness); + _approveTest(Ethereum.WETH, harness); + _approveTest(Ethereum.WSTETH, harness); + } + +}