Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: insert tolerance into redeemShare() checks [SUP-8853] #645

Merged
merged 13 commits into from
Oct 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/interfaces/ISuperformRouterPlus.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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 //
//////////////////////////////////////////////////////////////
Expand Down
7 changes: 7 additions & 0 deletions src/router-plus/SuperformRouterPlus.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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 = 10 wei;

//////////////////////////////////////////////////////////////
// CONSTRUCTOR //
//////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -518,6 +521,10 @@ 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
if (
(balanceDifference != assets)
|| (ENTIRE_SLIPPAGE * assets < ((expectedOutputAmount_ * (ENTIRE_SLIPPAGE - maxSlippage_))))
Expand Down
131 changes: 131 additions & 0 deletions test/unit/router-plus/SuperformRouterPlus.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -553,6 +554,136 @@ 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;
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(1) // Return 1 wei
);

// 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_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);

// 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);

Expand Down