Updated the sweep function and wrote some comprehensive tests (#720)

* Updated the sweep function and wrote some comprehensive tests * Fixed the rust tests
delvtech · Jan 10, 2024 · c2e0f6c · c2e0f6c
1 parent aadaa99
commit c2e0f6c
Show file tree

Hide file tree

Showing 16 changed files with 282 additions and 188 deletions.
diff --git a/contracts/src/instances/ERC4626Base.sol b/contracts/src/instances/ERC4626Base.sol
@@ -22,10 +22,6 @@ abstract contract ERC4626Base is HyperdriveBase {
     /// @dev The yield source contract for this hyperdrive.
     IERC4626 internal immutable _pool;
 
-    /// @dev A mapping from addresses to their status as a sweep target. This
-    ///      mapping does not change after construction.
-    mapping(address target => bool canSweep) internal _isSweepable;
-
     /// @notice Instantiates the ERC4626 Hyperdrive base contract.
     /// @param __pool The ERC4626 compatible yield source.
     constructor(IERC4626 __pool) {

diff --git a/contracts/src/instances/ERC4626Hyperdrive.sol b/contracts/src/instances/ERC4626Hyperdrive.sol
@@ -28,18 +28,13 @@ contract ERC4626Hyperdrive is Hyperdrive, ERC4626Base {
     /// @param _target2 The target2 address.
     /// @param _target3 The target3 address.
     /// @param __pool The ERC4626 compatible yield source.
-    /// @param _targets The addresses that can be swept by governance. This
-    ///        allows governance to collect rewards derived from incentive
-    ///        programs while also preventing edge cases where `sweep` is used
-    ///        to access the pool or base tokens.
     constructor(
         IHyperdrive.PoolConfig memory _config,
         address _target0,
         address _target1,
         address _target2,
         address _target3,
-        IERC4626 __pool,
-        address[] memory _targets
+        IERC4626 __pool
     )
         Hyperdrive(_config, _target0, _target1, _target2, _target3)
         ERC4626Base(__pool)
@@ -61,27 +56,11 @@ contract ERC4626Hyperdrive is Hyperdrive, ERC4626Base {
         // Approve the base token with 1 wei. This ensures that all of the
         // subsequent approvals will be writing to a dirty storage slot.
         ERC20(address(_config.baseToken)).safeApprove(address(_pool), 1);
-
-        // Set the sweep targets. The base and pool tokens can't be set as sweep
-        // targets to prevent governance from rugging the pool.
-        for (uint256 i = 0; i < _targets.length; i++) {
-            address target = _targets[i];
-            if (
-                address(target) == address(_pool) ||
-                address(target) == address(_baseToken)
-            ) {
-                revert IHyperdrive.UnsupportedToken();
-            }
-            _isSweepable[target] = true;
-        }
     }
 
     /// @notice Some yield sources [eg Morpho] pay rewards directly to this
     ///         contract but we can't handle distributing them internally so we
     ///         sweep to the fee collector address to then redistribute to users.
-    /// @dev WARN: The entire balance of any of the sweep targets can be swept
-    ///      by governance. If these sweep targets provide access to the base or
-    ///      pool token, then governance has the ability to rug the pool.
     /// @dev WARN: It is unlikely but possible that there is a selector overlap
     ///      with 'transferFrom'. Any integrating contracts should be checked
     ///      for that, as it may result in an unexpected call from this address.

diff --git a/contracts/src/instances/ERC4626HyperdriveCoreDeployer.sol b/contracts/src/instances/ERC4626HyperdriveCoreDeployer.sol
@@ -17,12 +17,12 @@ import { ERC4626Hyperdrive } from "../instances/ERC4626Hyperdrive.sol";
 contract ERC4626HyperdriveCoreDeployer is IERC4626HyperdriveDeployer {
     /// @notice Deploys a Hyperdrive instance with the given parameters.
     /// @param _config The configuration of the Hyperdrive pool.
-    /// @param _extraData The extra data that contains the sweep targets.
+    /// @param _extraData The extra data that contains the ERC4626 vault.
     /// @param target0 The target0 address.
     /// @param target1 The target1 address.
     /// @param target2 The target2 address.
     /// @param target3 The target3 address.
-    /// @return The address of the newly deployed ERC4626Hyperdrive Instance.
+    /// @return The address of the newly deployed ERC4626Hyperdrive instance.
     function deploy(
         IHyperdrive.PoolConfig memory _config,
         bytes memory _extraData,
@@ -31,10 +31,7 @@ contract ERC4626HyperdriveCoreDeployer is IERC4626HyperdriveDeployer {
         address target2,
         address target3
     ) external override returns (address) {
-        (address pool, address[] memory sweepTargets) = abi.decode(
-            _extraData,
-            (address, address[])
-        );
+        address pool = abi.decode(_extraData, (address));
 
         // Deploy the ERC4626Hyperdrive instance.
         return (
@@ -45,8 +42,7 @@ contract ERC4626HyperdriveCoreDeployer is IERC4626HyperdriveDeployer {
                     target1,
                     target2,
                     target3,
-                    IERC4626(pool),
-                    sweepTargets
+                    IERC4626(pool)
                 )
             )
         );

diff --git a/contracts/src/instances/ERC4626Target0.sol b/contracts/src/instances/ERC4626Target0.sol
@@ -33,9 +33,6 @@ contract ERC4626Target0 is HyperdriveTarget0, ERC4626Base {
     /// @notice Some yield sources [eg Morpho] pay rewards directly to this
     ///         contract but we can't handle distributing them internally so we
     ///         sweep to the fee collector address to then redistribute to users.
-    /// @dev WARN: The entire balance of any of the sweep targets can be swept
-    ///      by governance. If these sweep targets provide access to the base or
-    ///      pool token, then governance has the ability to rug the pool.
     /// @dev WARN: It is unlikely but possible that there is a selector overlap
     ///      with 'transferFrom'. Any integrating contracts should be checked
     ///      for that, as it may result in an unexpected call from this address.
@@ -46,14 +43,30 @@ contract ERC4626Target0 is HyperdriveTarget0, ERC4626Base {
             revert IHyperdrive.Unauthorized();
         }
 
-        // Ensure that thet target can be swept by governance.
-        if (!_isSweepable[address(_target)]) {
+        // Ensure that thet target isn't the base or vault token
+        if (
+            address(_target) == address(_baseToken) ||
+            address(_target) == address(_pool)
+        ) {
             revert IHyperdrive.UnsupportedToken();
         }
 
+        // Get Hyperdrive's balance of the base and pool tokens prior to
+        // sweeping.
+        uint256 baseBalance = _baseToken.balanceOf(address(this));
+        uint256 poolBalance = _pool.balanceOf(address(this));
+
         // Transfer the entire balance of the sweep target to the fee collector.
         uint256 balance = _target.balanceOf(address(this));
         ERC20(address(_target)).safeTransfer(_feeCollector, balance);
+
+        // Ensure that the base and pool balances haven't changed.
+        if (
+            _baseToken.balanceOf(address(this)) != baseBalance ||
+            _pool.balanceOf(address(this)) != poolBalance
+        ) {
+            revert IHyperdrive.SweepFailed();
+        }
     }
 
     /// Getters ///
@@ -63,10 +76,4 @@ contract ERC4626Target0 is HyperdriveTarget0, ERC4626Base {
     function pool() external view returns (IERC4626) {
         _revert(abi.encode(_pool));
     }
-
-    /// @notice Gets the sweepable status of a target.
-    /// @param _target The target address.
-    function isSweepable(address _target) external view returns (bool) {
-        _revert(abi.encode(_isSweepable[_target]));
-    }
 }
diff --git a/contracts/src/interfaces/IERC4626HyperdriveRead.sol b/contracts/src/interfaces/IERC4626HyperdriveRead.sol
@@ -6,6 +6,4 @@ import { IHyperdriveRead } from "./IHyperdriveRead.sol";
 
 interface IERC4626HyperdriveRead is IHyperdriveRead {
     function pool() external view returns (IERC4626);
-
-    function isSweepable(address _target) external view returns (bool);
 }
diff --git a/contracts/src/interfaces/IHyperdrive.sol b/contracts/src/interfaces/IHyperdrive.sol
@@ -256,18 +256,23 @@ interface IHyperdrive is IHyperdriveRead, IHyperdriveCore, IMultiToken {
     error ApprovalFailed();
     error BelowMinimumContribution();
     error BelowMinimumShareReserves();
+    error EndIndexTooLarge();
     error InvalidApr();
     error InvalidBaseToken();
     error InvalidCheckpointTime();
     error InvalidCheckpointDuration();
+    error InvalidDeployer();
+    error InvalidFeeAmounts();
+    error InvalidFeeDestination();
+    error InvalidIndexes();
     error InvalidInitialSharePrice();
     error InvalidMaturityTime();
     error InvalidMinimumShareReserves();
     error InvalidPositionDuration();
     error InvalidShareReserves();
-    error InvalidFeeAmounts();
-    error InvalidFeeDestination();
     error InsufficientLiquidity();
+    error MinimumSharePrice();
+    error MinimumTransactionAmount();
     error NegativeInterest();
     error NegativePresentValue();
     error NoAssetsToWithdraw();
@@ -276,16 +281,12 @@ interface IHyperdrive is IHyperdriveRead, IHyperdriveCore, IMultiToken {
     error Paused();
     error PoolAlreadyInitialized();
     error ShareReservesDeltaExceedsBondReservesDelta();
+    error SweepFailed();
     error TransferFailed();
     error UnexpectedAssetId();
     error UnexpectedSender();
     error UnsupportedToken();
-    error MinimumSharePrice();
-    error MinimumTransactionAmount();
     error ZeroLpTotalSupply();
-    error InvalidIndexes();
-    error EndIndexTooLarge();
-    error InvalidDeployer();
 
     /// ############
     /// ### TWAP ###

diff --git a/contracts/test/MockERC4626Hyperdrive.sol b/contracts/test/MockERC4626Hyperdrive.sol
@@ -12,17 +12,15 @@ contract MockERC4626Hyperdrive is ERC4626Hyperdrive {
         address _target1,
         address _target2,
         address _target3,
-        IERC4626 _pool,
-        address[] memory _sweepTargets
+        IERC4626 _pool
     )
         ERC4626Hyperdrive(
             _config,
             _target0,
             _target1,
             _target2,
             _target3,
-            _pool,
-            _sweepTargets
+            _pool
         )
     {}
 

diff --git a/crates/test-utils/src/chain/test_chain.rs b/crates/test-utils/src/chain/test_chain.rs
@@ -306,7 +306,6 @@ impl TestChain {
                 target2.address(),
                 target3.address(),
                 pool.address(),
-                Vec::<U256>::new(),
             ),
         )?
         .gas_price(DEFAULT_GAS_PRICE)

diff --git a/test/integrations/AaveV3ERC4626.t.sol → test/instances/erc4626/AaveV3ERC4626.t.sol b/test/integrations/AaveV3ERC4626.t.sol → test/instances/erc4626/AaveV3ERC4626.t.sol
diff --git a/test/integrations/ERC4626Hyperdrive.t.sol → ...instances/erc4626/ERC4626Hyperdrive.t.sol b/test/integrations/ERC4626Hyperdrive.t.sol → ...instances/erc4626/ERC4626Hyperdrive.t.sol
@@ -121,8 +121,7 @@ contract ERC4626HyperdriveTest is HyperdriveTest {
             target1,
             target2,
             target3,
-            pool,
-            new address[](0)
+            pool
         );
 
         vm.stopPrank();
@@ -344,131 +343,4 @@ contract ERC4626HyperdriveTest is HyperdriveTest {
             (pool.totalAssets()).divDown(pool.totalSupply())
         );
     }
-
-    function test_erc4626_sweep() public {
-        // Ensure that deployment will fail if the pool or base token is
-        // specified as a sweep target.
-        vm.startPrank(alice);
-        address[] memory sweepTargets = new address[](1);
-        sweepTargets[0] = address(dai);
-        bytes memory extraData = abi.encode(address(pool), sweepTargets);
-        IHyperdrive.PoolDeployConfig memory config = IHyperdrive
-            .PoolDeployConfig({
-                baseToken: dai,
-                linkerFactory: address(0),
-                linkerCodeHash: bytes32(0),
-                minimumShareReserves: ONE,
-                minimumTransactionAmount: 0.001e18,
-                positionDuration: 365 days,
-                checkpointDuration: 1 days,
-                timeStretch: HyperdriveUtils.calculateTimeStretch(
-                    0.01e18,
-                    365 days
-                ),
-                governance: alice,
-                feeCollector: bob,
-                fees: IHyperdrive.Fees(0, 0, 0, 0)
-            });
-        vm.expectRevert(IHyperdrive.UnsupportedToken.selector);
-        factory.deployAndInitialize(
-            hyperdriveDeployer,
-            config,
-            extraData,
-            1_000e18,
-            0.05e18,
-            new bytes(0)
-        );
-        assert(
-            !IERC4626Hyperdrive(address(mockHyperdrive)).isSweepable(
-                address(dai)
-            )
-        );
-        sweepTargets[0] = address(pool);
-        extraData = abi.encode(address(pool), sweepTargets);
-        vm.expectRevert(IHyperdrive.UnsupportedToken.selector);
-        factory.deployAndInitialize(
-            hyperdriveDeployer,
-            config,
-            extraData,
-            1_000e18,
-            0.05e18,
-            new bytes(0)
-        );
-        assert(
-            !IERC4626Hyperdrive(address(mockHyperdrive)).isSweepable(
-                address(pool)
-            )
-        );
-        vm.stopPrank();
-
-        // Ensure that the base token and the pool cannot be swept.
-        vm.startPrank(bob);
-        vm.expectRevert(IHyperdrive.UnsupportedToken.selector);
-        IERC4626Hyperdrive(address(mockHyperdrive)).sweep(dai);
-        vm.expectRevert(IHyperdrive.UnsupportedToken.selector);
-        IERC4626Hyperdrive(address(mockHyperdrive)).sweep(
-            IERC20(address(pool))
-        );
-        vm.stopPrank();
-
-        // Ensure that a sweep target that isn't the base token or the pool
-        // can be initialized and that the target can be swept successfully.
-        vm.startPrank(alice);
-        ERC20Mintable otherToken = new ERC20Mintable(
-            "Other",
-            "OTHER",
-            18,
-            address(0),
-            false
-        );
-        sweepTargets[0] = address(otherToken);
-        extraData = abi.encode(address(pool), sweepTargets);
-        mockHyperdrive = MockERC4626Hyperdrive(
-            address(
-                factory.deployAndInitialize(
-                    hyperdriveDeployer,
-                    config,
-                    extraData,
-                    1_000e18,
-                    0.05e18,
-                    new bytes(0)
-                )
-            )
-        );
-        assert(
-            IERC4626Hyperdrive(address(mockHyperdrive)).isSweepable(
-                address(otherToken)
-            )
-        );
-        vm.stopPrank();
-        vm.startPrank(bob);
-        otherToken.mint(address(mockHyperdrive), 1e18);
-        IERC4626Hyperdrive(address(mockHyperdrive)).sweep(
-            IERC20(address(otherToken))
-        );
-        assertEq(otherToken.balanceOf(bob), 1e18);
-        vm.stopPrank();
-
-        // Alice should not be able to sweep the target since she isn't a pauser.
-        vm.startPrank(alice);
-        vm.expectRevert(IHyperdrive.Unauthorized.selector);
-        IERC4626Hyperdrive(address(mockHyperdrive)).sweep(
-            IERC20(address(otherToken))
-        );
-        vm.stopPrank();
-
-        // Bob adds Alice as a pauser.
-        vm.startPrank(bob);
-        IERC4626Hyperdrive(address(mockHyperdrive)).setPauser(alice, true);
-        vm.stopPrank();
-
-        // Alice should be able to sweep the target successfully.
-        vm.startPrank(alice);
-        otherToken.mint(address(mockHyperdrive), 1e18);
-        IERC4626Hyperdrive(address(mockHyperdrive)).sweep(
-            IERC20(address(otherToken))
-        );
-        assertEq(otherToken.balanceOf(bob), 2e18);
-        vm.stopPrank();
-    }
 }
diff --git a/test/integrations/ERC4626Validation.t.sol → ...instances/erc4626/ERC4626Validation.t.sol b/test/integrations/ERC4626Validation.t.sol → ...instances/erc4626/ERC4626Validation.t.sol
diff --git a/test/integrations/StethERC4626.t.sol → test/instances/erc4626/StethERC4626.t.sol b/test/integrations/StethERC4626.t.sol → test/instances/erc4626/StethERC4626.t.sol