From 7b7cdc844071d406c22be0124e3114805f404fd6 Mon Sep 17 00:00:00 2001 From: Tamara Ringas <69479754+TamaraRingas@users.noreply.github.com> Date: Wed, 23 Oct 2024 17:22:13 +0200 Subject: [PATCH 01/13] fix: add tolerance check to _redeemShare() --- Makefile | 2 +- src/router-plus/SuperformRouterPlus.sol | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index dfc8845e4..349f59aff 100644 --- a/Makefile +++ b/Makefile @@ -121,7 +121,7 @@ build-sizes: ## Builds the project and shows sizes .PHONY: test-vvv test-vvv: ## Runs tests with verbose output - forge test --match-contract SDMVW0TokenInputNoSlippageAMB1323 --evm-version cancun -vvv + forge test --match-contract SuperformRouterPlusTest --evm-version cancun -vvv .PHONY: ftest ftest: ## Runs tests with cancun evm version diff --git a/src/router-plus/SuperformRouterPlus.sol b/src/router-plus/SuperformRouterPlus.sol index f363d7c8c..742db832a 100644 --- a/src/router-plus/SuperformRouterPlus.sol +++ b/src/router-plus/SuperformRouterPlus.sol @@ -27,6 +27,9 @@ contract SuperformRouterPlus is ISuperformRouterPlus, BaseSuperformRouterPlus { uint256 public ROUTER_PLUS_PAYLOAD_ID; + /// @dev Tolerance constant to account for tokens with rounding issues on transfer + uint256 constant TOLERANCE_CONSTANT = 2 wei; + ////////////////////////////////////////////////////////////// // CONSTRUCTOR // ////////////////////////////////////////////////////////////// @@ -521,6 +524,7 @@ contract SuperformRouterPlus is ISuperformRouterPlus, BaseSuperformRouterPlus { if ( (balanceDifference != assets) || (ENTIRE_SLIPPAGE * assets < ((expectedOutputAmount_ * (ENTIRE_SLIPPAGE - maxSlippage_)))) + || (assets - TOLERANCE_CONSTANT > balanceDifference) ) { revert ASSETS_RECEIVED_OUT_OF_SLIPPAGE(); } From e8a5d18015d045b14340dedf8604f03a05327b0b Mon Sep 17 00:00:00 2001 From: Tamara Ringas <69479754+TamaraRingas@users.noreply.github.com> Date: Wed, 23 Oct 2024 17:22:40 +0200 Subject: [PATCH 02/13] fix: undo change to test-vvv --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 349f59aff..dfc8845e4 100644 --- a/Makefile +++ b/Makefile @@ -121,7 +121,7 @@ build-sizes: ## Builds the project and shows sizes .PHONY: test-vvv test-vvv: ## Runs tests with verbose output - forge test --match-contract SuperformRouterPlusTest --evm-version cancun -vvv + forge test --match-contract SDMVW0TokenInputNoSlippageAMB1323 --evm-version cancun -vvv .PHONY: ftest ftest: ## Runs tests with cancun evm version From 3cab42f1270906ff6c884790c1427b62b018f709 Mon Sep 17 00:00:00 2001 From: Tamara Ringas <69479754+TamaraRingas@users.noreply.github.com> Date: Wed, 23 Oct 2024 18:10:19 +0200 Subject: [PATCH 03/13] chore: add test for deposits with tolerance --- .../router-plus/SuperformRouterPlus.t.sol | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/test/unit/router-plus/SuperformRouterPlus.t.sol b/test/unit/router-plus/SuperformRouterPlus.t.sol index 35dcb6a50..52eae8b1a 100644 --- a/test/unit/router-plus/SuperformRouterPlus.t.sol +++ b/test/unit/router-plus/SuperformRouterPlus.t.sol @@ -553,6 +553,54 @@ contract SuperformRouterPlusTest is ProtocolActions { SuperformRouterPlus(ROUTER_PLUS_SOURCE).deposit4626{ value: 1 ether }(address(mockVault), args); } + function test_deposit4626_withinTolerance() public { + vm.startPrank(deployer); + + // Deploy a mock ERC4626 vault + VaultMock mockVault = new VaultMock(IERC20(getContract(SOURCE_CHAIN, "DAI")), "Mock Vault", "mVLT"); + + // Mint some DAI to the deployer + uint256 daiAmount = 1e18 - 2 wei; + + // Approve and deposit DAI into the mock vault + MockERC20(getContract(SOURCE_CHAIN, "DAI")).approve(address(mockVault), daiAmount); + uint256 vaultTokenAmount = mockVault.deposit(daiAmount, deployer); + + // Prepare deposit4626 args + ISuperformRouterPlus.Deposit4626Args memory args = ISuperformRouterPlus.Deposit4626Args({ + amount: vaultTokenAmount, + expectedOutputAmount: daiAmount, // Assuming 1:1 ratio for simplicity + maxSlippage: 100, // 1% + receiverAddressSP: deployer, + depositCallData: _buildDepositCallData(superformId1, daiAmount) + }); + + // Approve RouterPlus to spend vault tokens + mockVault.approve(ROUTER_PLUS_SOURCE, vaultTokenAmount); + + // Execute deposit4626 + vm.recordLogs(); + SuperformRouterPlus(ROUTER_PLUS_SOURCE).deposit4626{ value: 1 ether }(address(mockVault), args); + + // Verify the results + assertGt( + SuperPositions(SUPER_POSITIONS_SOURCE).balanceOf(deployer, superformId1), + 0, + "Superform balance should be greater than 0" + ); + + // Check that the vault tokens were transferred from the deployer + assertEq(mockVault.balanceOf(deployer), 0, "Deployer's vault token balance should be 0"); + + // Check that the RouterPlus contract doesn't hold any tokens + assertEq(mockVault.balanceOf(ROUTER_PLUS_SOURCE), 0, "RouterPlus should not hold any vault tokens"); + assertEq( + MockERC20(getContract(SOURCE_CHAIN, "DAI")).balanceOf(ROUTER_PLUS_SOURCE), + 0, + "RouterPlus should not hold any DAI" + ); + } + function test_rebalanceSinglePosition_errors() public { vm.startPrank(deployer); From b4af66dd6e73268c873ed86ac7ceefabd8fe3a1a Mon Sep 17 00:00:00 2001 From: Tamara Ringas <69479754+TamaraRingas@users.noreply.github.com> Date: Thu, 24 Oct 2024 10:49:15 +0200 Subject: [PATCH 04/13] fix: add TOLERANCE_EXCEEDED() error --- src/interfaces/ISuperformRouterPlus.sol | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/interfaces/ISuperformRouterPlus.sol b/src/interfaces/ISuperformRouterPlus.sol index b5d356c1a..f3b51aadb 100644 --- a/src/interfaces/ISuperformRouterPlus.sol +++ b/src/interfaces/ISuperformRouterPlus.sol @@ -49,6 +49,9 @@ interface ISuperformRouterPlus is IBaseSuperformRouterPlus { /// @notice thrown if the amount of assets received is lower than the slippage error ASSETS_RECEIVED_OUT_OF_SLIPPAGE(); + /// @notice thrown if the tolerance is exceeded during shares redemption + error TOLERANCE_EXCEEDED(); + ////////////////////////////////////////////////////////////// // EVENTS // ////////////////////////////////////////////////////////////// From b352ecea7af4443e46a27317261eef545c84c4ff Mon Sep 17 00:00:00 2001 From: Tamara Ringas <69479754+TamaraRingas@users.noreply.github.com> Date: Thu, 24 Oct 2024 10:49:40 +0200 Subject: [PATCH 05/13] refactor: throw check tolerance in separate block --- src/router-plus/SuperformRouterPlus.sol | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/router-plus/SuperformRouterPlus.sol b/src/router-plus/SuperformRouterPlus.sol index 742db832a..6756406f5 100644 --- a/src/router-plus/SuperformRouterPlus.sol +++ b/src/router-plus/SuperformRouterPlus.sol @@ -28,7 +28,7 @@ contract SuperformRouterPlus is ISuperformRouterPlus, BaseSuperformRouterPlus { uint256 public ROUTER_PLUS_PAYLOAD_ID; /// @dev Tolerance constant to account for tokens with rounding issues on transfer - uint256 constant TOLERANCE_CONSTANT = 2 wei; + uint256 constant TOLERANCE_CONSTANT = 10 wei; ////////////////////////////////////////////////////////////// // CONSTRUCTOR // @@ -524,10 +524,13 @@ contract SuperformRouterPlus is ISuperformRouterPlus, BaseSuperformRouterPlus { if ( (balanceDifference != assets) || (ENTIRE_SLIPPAGE * assets < ((expectedOutputAmount_ * (ENTIRE_SLIPPAGE - maxSlippage_)))) - || (assets - TOLERANCE_CONSTANT > balanceDifference) ) { revert ASSETS_RECEIVED_OUT_OF_SLIPPAGE(); } + + if ((assets - TOLERANCE_CONSTANT > balanceDifference)) { + revert TOLERANCE_EXCEEDED(); + } } /// @dev helps parse bytes memory selector From 2dad9466b07d761fb8311f017900b567fa11d749 Mon Sep 17 00:00:00 2001 From: Tamara Ringas <69479754+TamaraRingas@users.noreply.github.com> Date: Thu, 24 Oct 2024 11:31:19 +0200 Subject: [PATCH 06/13] chore: add test_deposit4626_toleranceExceeded() --- .../router-plus/SuperformRouterPlus.t.sol | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/test/unit/router-plus/SuperformRouterPlus.t.sol b/test/unit/router-plus/SuperformRouterPlus.t.sol index 52eae8b1a..cc2094981 100644 --- a/test/unit/router-plus/SuperformRouterPlus.t.sol +++ b/test/unit/router-plus/SuperformRouterPlus.t.sol @@ -553,6 +553,39 @@ contract SuperformRouterPlusTest is ProtocolActions { SuperformRouterPlus(ROUTER_PLUS_SOURCE).deposit4626{ value: 1 ether }(address(mockVault), args); } + function test_deposit4626_toleranceExceeded() public { + vm.startPrank(deployer); + + // Deploy a mock ERC4626 vault + VaultMock mockVault = new VaultMock(IERC20(getContract(SOURCE_CHAIN, "DAI")), "Mock Vault", "mVLT"); + + // Mint some DAI to the deployer + uint256 daiAmount = 1e18; + + // Approve and deposit DAI into the mock vault + MockERC20(getContract(SOURCE_CHAIN, "DAI")).approve(address(mockVault), daiAmount); + uint256 vaultTokenAmount = mockVault.deposit(daiAmount, deployer); + + // Prepare deposit4626 args + ISuperformRouterPlus.Deposit4626Args memory args = ISuperformRouterPlus.Deposit4626Args({ + amount: vaultTokenAmount, + expectedOutputAmount: daiAmount - 15 wei, + maxSlippage: 100, // 1% + receiverAddressSP: deployer, + depositCallData: _buildDepositCallData(superformId1, daiAmount) + }); + + // Approve RouterPlus to spend vault tokens + mockVault.approve(ROUTER_PLUS_SOURCE, vaultTokenAmount); + + // Execute deposit4626 + vm.recordLogs(); + vm.expectRevert(ISuperformRouterPlus.TOLERANCE_EXCEEDED.selector); + SuperformRouterPlus(ROUTER_PLUS_SOURCE).deposit4626{ value: 1 ether }(address(mockVault), args); + + vm.stopPrank(); + } + function test_deposit4626_withinTolerance() public { vm.startPrank(deployer); From b5881953791d84eac11280f5b84cc84a48bfe3b1 Mon Sep 17 00:00:00 2001 From: Tamara Ringas <69479754+TamaraRingas@users.noreply.github.com> Date: Thu, 24 Oct 2024 12:32:29 +0200 Subject: [PATCH 07/13] refactor: update tolerance checks in redeemShare() --- src/router-plus/SuperformRouterPlus.sol | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/router-plus/SuperformRouterPlus.sol b/src/router-plus/SuperformRouterPlus.sol index 6756406f5..59e48f14c 100644 --- a/src/router-plus/SuperformRouterPlus.sol +++ b/src/router-plus/SuperformRouterPlus.sol @@ -521,16 +521,15 @@ contract SuperformRouterPlus is ISuperformRouterPlus, BaseSuperformRouterPlus { uint256 assetsBalanceAfter = asset.balanceOf(address(this)); balanceDifference = assetsBalanceAfter - assetsBalanceBefore; + if (assets < TOLERANCE_CONSTANT) revert Error.ZERO_AMOUNT(); + + /// @dev validate the slippage if ( - (balanceDifference != assets) + (balanceDifference < assets - TOLERANCE_CONSTANT) || (ENTIRE_SLIPPAGE * assets < ((expectedOutputAmount_ * (ENTIRE_SLIPPAGE - maxSlippage_)))) ) { revert ASSETS_RECEIVED_OUT_OF_SLIPPAGE(); } - - if ((assets - TOLERANCE_CONSTANT > balanceDifference)) { - revert TOLERANCE_EXCEEDED(); - } } /// @dev helps parse bytes memory selector From baa8aeeda2d95c9100892167206030cc1bf1cfa1 Mon Sep 17 00:00:00 2001 From: Tamara Ringas <69479754+TamaraRingas@users.noreply.github.com> Date: Thu, 24 Oct 2024 13:26:51 +0200 Subject: [PATCH 08/13] fix: update tolerance exceeded test --- test/unit/router-plus/SuperformRouterPlus.t.sol | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/test/unit/router-plus/SuperformRouterPlus.t.sol b/test/unit/router-plus/SuperformRouterPlus.t.sol index cc2094981..b7759e38f 100644 --- a/test/unit/router-plus/SuperformRouterPlus.t.sol +++ b/test/unit/router-plus/SuperformRouterPlus.t.sol @@ -8,6 +8,7 @@ import { ISuperformRouterPlusAsync } from "src/interfaces/ISuperformRouterPlusAs import { IBaseSuperformRouterPlus } from "src/interfaces/IBaseSuperformRouterPlus.sol"; import { IBaseRouter } from "src/interfaces/IBaseRouter.sol"; import { ERC20 } from "openzeppelin-contracts/contracts/token/ERC20/ERC20.sol"; +import { IERC4626 } from "openzeppelin-contracts/contracts/interfaces/IERC4626.sol"; contract RejectEther { // This function will revert when called, simulating a contract that can't receive native tokens @@ -561,15 +562,23 @@ contract SuperformRouterPlusTest is ProtocolActions { // Mint some DAI to the deployer uint256 daiAmount = 1e18; + deal(getContract(SOURCE_CHAIN, "DAI"), deployer, daiAmount); // Approve and deposit DAI into the mock vault MockERC20(getContract(SOURCE_CHAIN, "DAI")).approve(address(mockVault), daiAmount); uint256 vaultTokenAmount = mockVault.deposit(daiAmount, deployer); + // Mock the balanceOf function to return a value less than expected + vm.mockCall( + address(deployer), + abi.encodeWithSelector(IERC4626.maxDeposit.selector, address(mockVault)), + abi.encode(daiAmount - 15 wei) // Return half of the expected amount + ); + // Prepare deposit4626 args ISuperformRouterPlus.Deposit4626Args memory args = ISuperformRouterPlus.Deposit4626Args({ amount: vaultTokenAmount, - expectedOutputAmount: daiAmount - 15 wei, + expectedOutputAmount: daiAmount, maxSlippage: 100, // 1% receiverAddressSP: deployer, depositCallData: _buildDepositCallData(superformId1, daiAmount) From 7fcba9f33db5885661820f3426f72bd38a820b1b Mon Sep 17 00:00:00 2001 From: Tamara Ringas <69479754+TamaraRingas@users.noreply.github.com> Date: Thu, 24 Oct 2024 17:13:03 +0200 Subject: [PATCH 09/13] refactor: update tolerance error in redeemShare() --- src/router-plus/SuperformRouterPlus.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/router-plus/SuperformRouterPlus.sol b/src/router-plus/SuperformRouterPlus.sol index 59e48f14c..95879f8c1 100644 --- a/src/router-plus/SuperformRouterPlus.sol +++ b/src/router-plus/SuperformRouterPlus.sol @@ -521,7 +521,7 @@ contract SuperformRouterPlus is ISuperformRouterPlus, BaseSuperformRouterPlus { uint256 assetsBalanceAfter = asset.balanceOf(address(this)); balanceDifference = assetsBalanceAfter - assetsBalanceBefore; - if (assets < TOLERANCE_CONSTANT) revert Error.ZERO_AMOUNT(); + if (assets < TOLERANCE_CONSTANT) revert TOLERANCE_EXCEEDED(); /// @dev validate the slippage if ( From 8378e43be5442527e0b09a00c57e809d8fb29a19 Mon Sep 17 00:00:00 2001 From: Tamara Ringas <69479754+TamaraRingas@users.noreply.github.com> Date: Thu, 24 Oct 2024 17:14:52 +0200 Subject: [PATCH 10/13] fix: update mock call in test_deposit4626_toleranceExceeded() --- test/unit/router-plus/SuperformRouterPlus.t.sol | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/unit/router-plus/SuperformRouterPlus.t.sol b/test/unit/router-plus/SuperformRouterPlus.t.sol index b7759e38f..85ad865d5 100644 --- a/test/unit/router-plus/SuperformRouterPlus.t.sol +++ b/test/unit/router-plus/SuperformRouterPlus.t.sol @@ -568,11 +568,11 @@ contract SuperformRouterPlusTest is ProtocolActions { MockERC20(getContract(SOURCE_CHAIN, "DAI")).approve(address(mockVault), daiAmount); uint256 vaultTokenAmount = mockVault.deposit(daiAmount, deployer); - // Mock the balanceOf function to return a value less than expected + // Mock the redeem function to return a value less than expected vm.mockCall( - address(deployer), - abi.encodeWithSelector(IERC4626.maxDeposit.selector, address(mockVault)), - abi.encode(daiAmount - 15 wei) // Return half of the expected amount + address(mockVault), + abi.encodeWithSelector(IERC4626.redeem.selector, vaultTokenAmount, ROUTER_PLUS_SOURCE, ROUTER_PLUS_SOURCE), + abi.encode(1) // Return 1 wei ); // Prepare deposit4626 args From c226008c19707d48850c9993e9d0ed7b705c6042 Mon Sep 17 00:00:00 2001 From: Tamara Ringas <69479754+TamaraRingas@users.noreply.github.com> Date: Thu, 24 Oct 2024 17:28:29 +0200 Subject: [PATCH 11/13] chore: add test_deposit4626_toleranceExceeded_noSlippage() --- .../router-plus/SuperformRouterPlus.t.sol | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/test/unit/router-plus/SuperformRouterPlus.t.sol b/test/unit/router-plus/SuperformRouterPlus.t.sol index 85ad865d5..20297c34f 100644 --- a/test/unit/router-plus/SuperformRouterPlus.t.sol +++ b/test/unit/router-plus/SuperformRouterPlus.t.sol @@ -595,6 +595,47 @@ contract SuperformRouterPlusTest is ProtocolActions { vm.stopPrank(); } + function test_deposit4626_toleranceExceeded_noSlippage() public { + vm.startPrank(deployer); + + // Deploy a mock ERC4626 vault + VaultMock mockVault = new VaultMock(IERC20(getContract(SOURCE_CHAIN, "DAI")), "Mock Vault", "mVLT"); + + // Mint some DAI to the deployer + uint256 daiAmount = 1e18; + deal(getContract(SOURCE_CHAIN, "DAI"), deployer, daiAmount); + + // Approve and deposit DAI into the mock vault + MockERC20(getContract(SOURCE_CHAIN, "DAI")).approve(address(mockVault), daiAmount); + uint256 vaultTokenAmount = mockVault.deposit(daiAmount, deployer); + + // Mock the redeem function to return a value less than expected + vm.mockCall( + address(mockVault), + abi.encodeWithSelector(IERC4626.redeem.selector, vaultTokenAmount, ROUTER_PLUS_SOURCE, ROUTER_PLUS_SOURCE), + abi.encode(daiAmount - 15 wei) // Return 15 wei less than expected + ); + + // Prepare deposit4626 args + ISuperformRouterPlus.Deposit4626Args memory args = ISuperformRouterPlus.Deposit4626Args({ + amount: vaultTokenAmount, + expectedOutputAmount: daiAmount, + maxSlippage: 100, // 1% + receiverAddressSP: deployer, + depositCallData: _buildDepositCallData(superformId1, daiAmount) + }); + + // Approve RouterPlus to spend vault tokens + mockVault.approve(ROUTER_PLUS_SOURCE, vaultTokenAmount); + + // Execute deposit4626 + vm.recordLogs(); + vm.expectRevert(ISuperformRouterPlus.TOLERANCE_EXCEEDED.selector); + SuperformRouterPlus(ROUTER_PLUS_SOURCE).deposit4626{ value: 1 ether }(address(mockVault), args); + + vm.stopPrank(); + } + function test_deposit4626_withinTolerance() public { vm.startPrank(deployer); From 87eb8291d2358574ab0b86e582679e79b1f4d21f Mon Sep 17 00:00:00 2001 From: Tamara Ringas <69479754+TamaraRingas@users.noreply.github.com> Date: Thu, 24 Oct 2024 17:30:05 +0200 Subject: [PATCH 12/13] refactor: update tolerance check in redeemShare() --- src/router-plus/SuperformRouterPlus.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/router-plus/SuperformRouterPlus.sol b/src/router-plus/SuperformRouterPlus.sol index 95879f8c1..7001c31d3 100644 --- a/src/router-plus/SuperformRouterPlus.sol +++ b/src/router-plus/SuperformRouterPlus.sol @@ -521,11 +521,11 @@ contract SuperformRouterPlus is ISuperformRouterPlus, BaseSuperformRouterPlus { uint256 assetsBalanceAfter = asset.balanceOf(address(this)); balanceDifference = assetsBalanceAfter - assetsBalanceBefore; - if (assets < TOLERANCE_CONSTANT) revert TOLERANCE_EXCEEDED(); + if (assets < TOLERANCE_CONSTANT || balanceDifference < TOLERANCE_CONSTANT) revert TOLERANCE_EXCEEDED(); /// @dev validate the slippage if ( - (balanceDifference < assets - TOLERANCE_CONSTANT) + (balanceDifference != assets) || (ENTIRE_SLIPPAGE * assets < ((expectedOutputAmount_ * (ENTIRE_SLIPPAGE - maxSlippage_)))) ) { revert ASSETS_RECEIVED_OUT_OF_SLIPPAGE(); From 1ab28aa9ed7c03febf95d3addd7969351b860c27 Mon Sep 17 00:00:00 2001 From: Tamara Ringas <69479754+TamaraRingas@users.noreply.github.com> Date: Thu, 24 Oct 2024 17:32:15 +0200 Subject: [PATCH 13/13] chore: comments --- src/router-plus/SuperformRouterPlus.sol | 1 + 1 file changed, 1 insertion(+) diff --git a/src/router-plus/SuperformRouterPlus.sol b/src/router-plus/SuperformRouterPlus.sol index 7001c31d3..5f596e7cc 100644 --- a/src/router-plus/SuperformRouterPlus.sol +++ b/src/router-plus/SuperformRouterPlus.sol @@ -521,6 +521,7 @@ contract SuperformRouterPlus is ISuperformRouterPlus, BaseSuperformRouterPlus { uint256 assetsBalanceAfter = asset.balanceOf(address(this)); balanceDifference = assetsBalanceAfter - assetsBalanceBefore; + /// @dev validate the tolerance if (assets < TOLERANCE_CONSTANT || balanceDifference < TOLERANCE_CONSTANT) revert TOLERANCE_EXCEEDED(); /// @dev validate the slippage