From 069af9ecd417a042b5836d9705ac1ceefb1d8c63 Mon Sep 17 00:00:00 2001 From: miguelmtzinf Date: Tue, 10 Sep 2024 13:10:00 +0200 Subject: [PATCH 1/3] fix: Transfer excess back and reset allowance --- src/contracts/BaseParaSwapBuyAdapter.sol | 12 +++++++----- src/contracts/ParaSwapDebtSwapAdapter.sol | 8 +++++++- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/src/contracts/BaseParaSwapBuyAdapter.sol b/src/contracts/BaseParaSwapBuyAdapter.sol index 52e1089..8e16f45 100644 --- a/src/contracts/BaseParaSwapBuyAdapter.sol +++ b/src/contracts/BaseParaSwapBuyAdapter.sol @@ -38,6 +38,7 @@ abstract contract BaseParaSwapBuyAdapter is BaseParaSwapAdapter { * @param maxAmountToSwap Max amount to be swapped * @param amountToReceive Amount to be received from the swap * @return amountSold The amount sold during the swap + * @return amountBought The amount bought during the swap */ function _buyOnParaSwap( uint256 toAmountOffset, @@ -46,7 +47,7 @@ abstract contract BaseParaSwapBuyAdapter is BaseParaSwapAdapter { IERC20Detailed assetToSwapTo, uint256 maxAmountToSwap, uint256 amountToReceive - ) internal returns (uint256 amountSold) { + ) internal returns (uint256 amountSold, uint256 amountBought) { (bytes memory buyCalldata, IParaSwapAugustus augustus) = abi.decode( paraswapData, (bytes, IParaSwapAugustus) @@ -73,7 +74,6 @@ abstract contract BaseParaSwapBuyAdapter is BaseParaSwapAdapter { uint256 balanceBeforeAssetTo = assetToSwapTo.balanceOf(address(this)); address tokenTransferProxy = augustus.getTokenTransferProxy(); - assetToSwapFrom.safeApprove(tokenTransferProxy, 0); assetToSwapFrom.safeApprove(tokenTransferProxy, maxAmountToSwap); if (toAmountOffset != 0) { @@ -98,13 +98,15 @@ abstract contract BaseParaSwapBuyAdapter is BaseParaSwapAdapter { revert(0, returndatasize()) } } + // Reset allowance + assetToSwapFrom.safeApprove(tokenTransferProxy, 0); uint256 balanceAfterAssetFrom = assetToSwapFrom.balanceOf(address(this)); amountSold = balanceBeforeAssetFrom - balanceAfterAssetFrom; require(amountSold <= maxAmountToSwap, 'WRONG_BALANCE_AFTER_SWAP'); - uint256 amountReceived = assetToSwapTo.balanceOf(address(this)) - balanceBeforeAssetTo; - require(amountReceived >= amountToReceive, 'INSUFFICIENT_AMOUNT_RECEIVED'); + amountBought = assetToSwapTo.balanceOf(address(this)) - balanceBeforeAssetTo; + require(amountBought >= amountToReceive, 'INSUFFICIENT_AMOUNT_RECEIVED'); - emit Bought(address(assetToSwapFrom), address(assetToSwapTo), amountSold, amountReceived); + emit Bought(address(assetToSwapFrom), address(assetToSwapTo), amountSold, amountBought); } } diff --git a/src/contracts/ParaSwapDebtSwapAdapter.sol b/src/contracts/ParaSwapDebtSwapAdapter.sol index 79eadc6..891a4fe 100644 --- a/src/contracts/ParaSwapDebtSwapAdapter.sol +++ b/src/contracts/ParaSwapDebtSwapAdapter.sol @@ -217,7 +217,7 @@ abstract contract ParaSwapDebtSwapAdapter is IERC20Detailed newDebtAsset, uint256 newDebtAmount ) internal returns (uint256) { - uint256 amountSold = _buyOnParaSwap( + (uint256 amountSold, uint256 amountBought) = _buyOnParaSwap( swapParams.offset, swapParams.paraswapData, newDebtAsset, @@ -234,6 +234,12 @@ abstract contract ParaSwapDebtSwapAdapter is swapParams.debtRateMode, swapParams.user ); + + //transfer excess of old debt asset back to the user, if any + uint256 debtAssetExcess = amountBought - swapParams.debtRepayAmount; + if (debtAssetExcess > 0) { + IERC20WithPermit(swapParams.debtAsset).safeTransfer(swapParams.user, debtAssetExcess); + } return amountSold; } From c53ea9ffd5e836a286dfa0f3f8f16fc9498e8a24 Mon Sep 17 00:00:00 2001 From: miguelmtzinf Date: Tue, 10 Sep 2024 13:11:19 +0200 Subject: [PATCH 2/3] fix: Reset allowance after Aave interaction --- src/contracts/ParaSwapDebtSwapAdapter.sol | 32 ++++++------------- src/contracts/ParaSwapDebtSwapAdapterV2.sol | 4 ++- .../ParaSwapDebtSwapAdapterV3GHO.sol | 5 ++- tests/DebtSwapV3.t.sol | 2 +- 4 files changed, 16 insertions(+), 27 deletions(-) diff --git a/src/contracts/ParaSwapDebtSwapAdapter.sol b/src/contracts/ParaSwapDebtSwapAdapter.sol index 891a4fe..e53ad0a 100644 --- a/src/contracts/ParaSwapDebtSwapAdapter.sol +++ b/src/contracts/ParaSwapDebtSwapAdapter.sol @@ -38,16 +38,6 @@ abstract contract ParaSwapDebtSwapAdapter is address owner ) BaseParaSwapBuyAdapter(addressesProvider, pool, augustusRegistry) { transferOwnership(owner); - // set initial approval for all reserves - address[] memory reserves = POOL.getReservesList(); - for (uint256 i = 0; i < reserves.length; i++) { - IERC20WithPermit(reserves[i]).safeApprove(address(POOL), type(uint256).max); - } - } - - function renewAllowance(address reserve) public { - IERC20WithPermit(reserve).safeApprove(address(POOL), 0); - IERC20WithPermit(reserve).safeApprove(address(POOL), type(uint256).max); } /** @@ -131,8 +121,9 @@ abstract contract ParaSwapDebtSwapAdapter is // this might lead to a slight excess decrease uint256 excess = excessAfter > excessBefore ? excessAfter - excessBefore : 0; if (excess > 0) { - _conditionalRenewAllowance(debtSwapParams.newDebtAsset, excess); + IERC20WithPermit(debtSwapParams.newDebtAsset).safeApprove(address(POOL), excess); POOL.repay(debtSwapParams.newDebtAsset, excess, 2, msg.sender); + IERC20WithPermit(debtSwapParams.newDebtAsset).safeApprove(address(POOL), 0); } } @@ -164,6 +155,7 @@ abstract contract ParaSwapDebtSwapAdapter is * enough funds to repay and has approved the Pool to pull the total amount * @param assets The addresses of the flash-borrowed assets * @param amounts The amounts of the flash-borrowed assets + * @param fees The fees of the flash-borrowed assets * @param initiator The address of the flashloan initiator * @param params The byte-encoded params passed when initiating the flashloan * @return True if the execution of the operation succeeds, false otherwise @@ -171,7 +163,7 @@ abstract contract ParaSwapDebtSwapAdapter is function executeOperation( address[] calldata assets, uint256[] calldata amounts, - uint256[] calldata, + uint256[] calldata fees, address initiator, bytes calldata params ) external returns (bool) { @@ -187,7 +179,9 @@ abstract contract ParaSwapDebtSwapAdapter is uint256 collateralAmount = amounts[0]; // Supply + IERC20WithPermit(collateralAsset).safeApprove(address(POOL), collateralAmount); _supply(collateralAsset, collateralAmount, flashParams.user, REFERRER); + IERC20WithPermit(collateralAsset).safeApprove(address(POOL), 0); // Execute the nested flashloan address newAsset = flashParams.nestedFlashloanDebtAsset; @@ -198,7 +192,8 @@ abstract contract ParaSwapDebtSwapAdapter is (, , address aToken) = _getReserveData(collateralAsset); IERC20WithPermit(aToken).safeTransferFrom(flashParams.user, address(this), collateralAmount); // Could be rounding error but it's insignificant POOL.withdraw(collateralAsset, collateralAmount, address(this)); - _conditionalRenewAllowance(collateralAsset, collateralAmount); + + IERC20WithPermit(collateralAsset).safeApprove(address(POOL), collateralAmount + fees[0]); } else { // There is no need for additional collateral, execute the swap. _swapAndRepay(flashParams, IERC20Detailed(assets[0]), amounts[0]); @@ -226,14 +221,14 @@ abstract contract ParaSwapDebtSwapAdapter is swapParams.debtRepayAmount ); - _conditionalRenewAllowance(swapParams.debtAsset, swapParams.debtRepayAmount); - + IERC20WithPermit(swapParams.debtAsset).safeApprove(address(POOL), swapParams.debtRepayAmount); POOL.repay( address(swapParams.debtAsset), swapParams.debtRepayAmount, swapParams.debtRateMode, swapParams.user ); + IERC20WithPermit(swapParams.debtAsset).safeApprove(address(POOL), 0); //transfer excess of old debt asset back to the user, if any uint256 debtAssetExcess = amountBought - swapParams.debtRepayAmount; @@ -242,11 +237,4 @@ abstract contract ParaSwapDebtSwapAdapter is } return amountSold; } - - function _conditionalRenewAllowance(address asset, uint256 minAmount) internal { - uint256 allowance = IERC20(asset).allowance(address(this), address(POOL)); - if (allowance < minAmount) { - renewAllowance(asset); - } - } } diff --git a/src/contracts/ParaSwapDebtSwapAdapterV2.sol b/src/contracts/ParaSwapDebtSwapAdapterV2.sol index 2a6caea..b49ed3f 100644 --- a/src/contracts/ParaSwapDebtSwapAdapterV2.sol +++ b/src/contracts/ParaSwapDebtSwapAdapterV2.sol @@ -19,7 +19,9 @@ contract ParaSwapDebtSwapAdapterV2 is ParaSwapDebtSwapAdapter { address owner ) ParaSwapDebtSwapAdapter(addressesProvider, pool, augustusRegistry, owner) {} - function _getReserveData(address asset) internal view override returns (address, address, address) { + function _getReserveData( + address asset + ) internal view override returns (address, address, address) { DataTypes.ReserveData memory reserveData = ILendingPool(address(POOL)).getReserveData(asset); return ( reserveData.variableDebtTokenAddress, diff --git a/src/contracts/ParaSwapDebtSwapAdapterV3GHO.sol b/src/contracts/ParaSwapDebtSwapAdapterV3GHO.sol index a1da142..431aa6c 100644 --- a/src/contracts/ParaSwapDebtSwapAdapterV3GHO.sol +++ b/src/contracts/ParaSwapDebtSwapAdapterV3GHO.sol @@ -28,9 +28,7 @@ contract ParaSwapDebtSwapAdapterV3GHO is ParaSwapDebtSwapAdapterV3, IERC3156Flas address pool, IParaSwapAugustusRegistry augustusRegistry, address owner - ) ParaSwapDebtSwapAdapterV3(addressesProvider, pool, augustusRegistry, owner) { - IERC20(GHO).approve(address(GHO_FLASH_MINTER), type(uint256).max); - } + ) ParaSwapDebtSwapAdapterV3(addressesProvider, pool, augustusRegistry, owner) {} /// @dev ERC-3156 Flash loan callback (in this case flash mint) function onFlashLoan( @@ -48,6 +46,7 @@ contract ParaSwapDebtSwapAdapterV3GHO is ParaSwapDebtSwapAdapterV3, IERC3156Flas POOL.borrow(GHO, (amountSold + fee), 2, REFERRER, swapParams.user); + IERC20(GHO).approve(address(GHO_FLASH_MINTER), amount + fee); return keccak256('ERC3156FlashBorrower.onFlashLoan'); } diff --git a/tests/DebtSwapV3.t.sol b/tests/DebtSwapV3.t.sol index bc10416..61da742 100644 --- a/tests/DebtSwapV3.t.sol +++ b/tests/DebtSwapV3.t.sol @@ -248,7 +248,7 @@ contract DebtSwapV3Test is BaseTest { address(debtSwapAdapter), address(AaveV3Ethereum.POOL) ), - type(uint256).max + 0 ); vm.record(); From 1b662ab616cc9953dd6b88bca6c4eb24c50dcb45 Mon Sep 17 00:00:00 2001 From: miguelmtzinf Date: Tue, 10 Sep 2024 13:11:19 +0200 Subject: [PATCH 3/3] Revert "fix: Reset allowance after Aave interaction" This reverts commit c53ea9ffd5e836a286dfa0f3f8f16fc9498e8a24. --- src/contracts/ParaSwapDebtSwapAdapter.sol | 32 +++++++++++++------ src/contracts/ParaSwapDebtSwapAdapterV2.sol | 4 +-- .../ParaSwapDebtSwapAdapterV3GHO.sol | 5 +-- tests/DebtSwapV3.t.sol | 2 +- 4 files changed, 27 insertions(+), 16 deletions(-) diff --git a/src/contracts/ParaSwapDebtSwapAdapter.sol b/src/contracts/ParaSwapDebtSwapAdapter.sol index e53ad0a..891a4fe 100644 --- a/src/contracts/ParaSwapDebtSwapAdapter.sol +++ b/src/contracts/ParaSwapDebtSwapAdapter.sol @@ -38,6 +38,16 @@ abstract contract ParaSwapDebtSwapAdapter is address owner ) BaseParaSwapBuyAdapter(addressesProvider, pool, augustusRegistry) { transferOwnership(owner); + // set initial approval for all reserves + address[] memory reserves = POOL.getReservesList(); + for (uint256 i = 0; i < reserves.length; i++) { + IERC20WithPermit(reserves[i]).safeApprove(address(POOL), type(uint256).max); + } + } + + function renewAllowance(address reserve) public { + IERC20WithPermit(reserve).safeApprove(address(POOL), 0); + IERC20WithPermit(reserve).safeApprove(address(POOL), type(uint256).max); } /** @@ -121,9 +131,8 @@ abstract contract ParaSwapDebtSwapAdapter is // this might lead to a slight excess decrease uint256 excess = excessAfter > excessBefore ? excessAfter - excessBefore : 0; if (excess > 0) { - IERC20WithPermit(debtSwapParams.newDebtAsset).safeApprove(address(POOL), excess); + _conditionalRenewAllowance(debtSwapParams.newDebtAsset, excess); POOL.repay(debtSwapParams.newDebtAsset, excess, 2, msg.sender); - IERC20WithPermit(debtSwapParams.newDebtAsset).safeApprove(address(POOL), 0); } } @@ -155,7 +164,6 @@ abstract contract ParaSwapDebtSwapAdapter is * enough funds to repay and has approved the Pool to pull the total amount * @param assets The addresses of the flash-borrowed assets * @param amounts The amounts of the flash-borrowed assets - * @param fees The fees of the flash-borrowed assets * @param initiator The address of the flashloan initiator * @param params The byte-encoded params passed when initiating the flashloan * @return True if the execution of the operation succeeds, false otherwise @@ -163,7 +171,7 @@ abstract contract ParaSwapDebtSwapAdapter is function executeOperation( address[] calldata assets, uint256[] calldata amounts, - uint256[] calldata fees, + uint256[] calldata, address initiator, bytes calldata params ) external returns (bool) { @@ -179,9 +187,7 @@ abstract contract ParaSwapDebtSwapAdapter is uint256 collateralAmount = amounts[0]; // Supply - IERC20WithPermit(collateralAsset).safeApprove(address(POOL), collateralAmount); _supply(collateralAsset, collateralAmount, flashParams.user, REFERRER); - IERC20WithPermit(collateralAsset).safeApprove(address(POOL), 0); // Execute the nested flashloan address newAsset = flashParams.nestedFlashloanDebtAsset; @@ -192,8 +198,7 @@ abstract contract ParaSwapDebtSwapAdapter is (, , address aToken) = _getReserveData(collateralAsset); IERC20WithPermit(aToken).safeTransferFrom(flashParams.user, address(this), collateralAmount); // Could be rounding error but it's insignificant POOL.withdraw(collateralAsset, collateralAmount, address(this)); - - IERC20WithPermit(collateralAsset).safeApprove(address(POOL), collateralAmount + fees[0]); + _conditionalRenewAllowance(collateralAsset, collateralAmount); } else { // There is no need for additional collateral, execute the swap. _swapAndRepay(flashParams, IERC20Detailed(assets[0]), amounts[0]); @@ -221,14 +226,14 @@ abstract contract ParaSwapDebtSwapAdapter is swapParams.debtRepayAmount ); - IERC20WithPermit(swapParams.debtAsset).safeApprove(address(POOL), swapParams.debtRepayAmount); + _conditionalRenewAllowance(swapParams.debtAsset, swapParams.debtRepayAmount); + POOL.repay( address(swapParams.debtAsset), swapParams.debtRepayAmount, swapParams.debtRateMode, swapParams.user ); - IERC20WithPermit(swapParams.debtAsset).safeApprove(address(POOL), 0); //transfer excess of old debt asset back to the user, if any uint256 debtAssetExcess = amountBought - swapParams.debtRepayAmount; @@ -237,4 +242,11 @@ abstract contract ParaSwapDebtSwapAdapter is } return amountSold; } + + function _conditionalRenewAllowance(address asset, uint256 minAmount) internal { + uint256 allowance = IERC20(asset).allowance(address(this), address(POOL)); + if (allowance < minAmount) { + renewAllowance(asset); + } + } } diff --git a/src/contracts/ParaSwapDebtSwapAdapterV2.sol b/src/contracts/ParaSwapDebtSwapAdapterV2.sol index b49ed3f..2a6caea 100644 --- a/src/contracts/ParaSwapDebtSwapAdapterV2.sol +++ b/src/contracts/ParaSwapDebtSwapAdapterV2.sol @@ -19,9 +19,7 @@ contract ParaSwapDebtSwapAdapterV2 is ParaSwapDebtSwapAdapter { address owner ) ParaSwapDebtSwapAdapter(addressesProvider, pool, augustusRegistry, owner) {} - function _getReserveData( - address asset - ) internal view override returns (address, address, address) { + function _getReserveData(address asset) internal view override returns (address, address, address) { DataTypes.ReserveData memory reserveData = ILendingPool(address(POOL)).getReserveData(asset); return ( reserveData.variableDebtTokenAddress, diff --git a/src/contracts/ParaSwapDebtSwapAdapterV3GHO.sol b/src/contracts/ParaSwapDebtSwapAdapterV3GHO.sol index 431aa6c..a1da142 100644 --- a/src/contracts/ParaSwapDebtSwapAdapterV3GHO.sol +++ b/src/contracts/ParaSwapDebtSwapAdapterV3GHO.sol @@ -28,7 +28,9 @@ contract ParaSwapDebtSwapAdapterV3GHO is ParaSwapDebtSwapAdapterV3, IERC3156Flas address pool, IParaSwapAugustusRegistry augustusRegistry, address owner - ) ParaSwapDebtSwapAdapterV3(addressesProvider, pool, augustusRegistry, owner) {} + ) ParaSwapDebtSwapAdapterV3(addressesProvider, pool, augustusRegistry, owner) { + IERC20(GHO).approve(address(GHO_FLASH_MINTER), type(uint256).max); + } /// @dev ERC-3156 Flash loan callback (in this case flash mint) function onFlashLoan( @@ -46,7 +48,6 @@ contract ParaSwapDebtSwapAdapterV3GHO is ParaSwapDebtSwapAdapterV3, IERC3156Flas POOL.borrow(GHO, (amountSold + fee), 2, REFERRER, swapParams.user); - IERC20(GHO).approve(address(GHO_FLASH_MINTER), amount + fee); return keccak256('ERC3156FlashBorrower.onFlashLoan'); } diff --git a/tests/DebtSwapV3.t.sol b/tests/DebtSwapV3.t.sol index 61da742..bc10416 100644 --- a/tests/DebtSwapV3.t.sol +++ b/tests/DebtSwapV3.t.sol @@ -248,7 +248,7 @@ contract DebtSwapV3Test is BaseTest { address(debtSwapAdapter), address(AaveV3Ethereum.POOL) ), - 0 + type(uint256).max ); vm.record();