Merge pull request #176 from morpho-labs/refactor/errors

refactor(errors): add vault & instance checks
morpho-org · Sep 18, 2023 · 25799fe · 25799fe
2 parents dd103eb + b35535e
commit 25799fe
Show file tree

Hide file tree

Showing 4 changed files with 43 additions and 1 deletion.
diff --git a/contracts/ERC4626Bundler.sol b/contracts/ERC4626Bundler.sol
@@ -21,6 +21,7 @@ abstract contract ERC4626Bundler is BaseBundler, Permit2Bundler {
 
     /// @notice Mints the given amount of `shares` on the given ERC4626 `vault`, on behalf of `owner`.
     function erc4626Mint(address vault, uint256 shares, address owner) external payable {
+        require(vault != address(0), ErrorsLib.ZERO_ADDRESS);
         require(owner != address(0), ErrorsLib.ZERO_ADDRESS);
 
         address asset = IERC4626(vault).asset();
@@ -39,6 +40,7 @@ abstract contract ERC4626Bundler is BaseBundler, Permit2Bundler {
 
     /// @notice Deposits the given amount of `assets` on the given ERC4626 `vault`, on behalf of `owner`.
     function erc4626Deposit(address vault, uint256 assets, address owner) external payable {
+        require(vault != address(0), ErrorsLib.ZERO_ADDRESS);
         require(owner != address(0), ErrorsLib.ZERO_ADDRESS);
 
         address asset = IERC4626(vault).asset();
@@ -58,6 +60,7 @@ abstract contract ERC4626Bundler is BaseBundler, Permit2Bundler {
     /// `receiver`.
     /// @notice Warning: should only be called via the bundler's `multicall` function.
     function erc4626Withdraw(address vault, uint256 assets, address receiver) external payable {
+        require(vault != address(0), ErrorsLib.ZERO_ADDRESS);
         require(receiver != address(0), ErrorsLib.ZERO_ADDRESS);
 
         address initiator = _initiator;
@@ -71,6 +74,7 @@ abstract contract ERC4626Bundler is BaseBundler, Permit2Bundler {
     /// @notice Redeems the given amount of `shares` from the given ERC4626 `vault`, transferring assets to `receiver`.
     /// @notice Warning: should only be called via the bundler's `multicall` function.
     function erc4626Redeem(address vault, uint256 shares, address receiver) external payable {
+        require(vault != address(0), ErrorsLib.ZERO_ADDRESS);
         require(receiver != address(0), ErrorsLib.ZERO_ADDRESS);
 
         address initiator = _initiator;

diff --git a/contracts/URDBundler.sol b/contracts/URDBundler.sol
@@ -24,8 +24,8 @@ contract URDBundler is BaseBundler {
         external
         payable
     {
-        require(account != address(this), ErrorsLib.BUNDLER_ADDRESS);
         require(account != address(0), ErrorsLib.ZERO_ADDRESS);
+        require(account != address(this), ErrorsLib.BUNDLER_ADDRESS);
 
         URD.claim(distributionId, account, reward, claimable, proof);
     }

diff --git a/contracts/migration/CompoundV3MigrationBundler.sol b/contracts/migration/CompoundV3MigrationBundler.sol
@@ -3,6 +3,8 @@ pragma solidity 0.8.21;
 
 import {ICompoundV3} from "./interfaces/ICompoundV3.sol";
 
+import {ErrorsLib} from "../libraries/ErrorsLib.sol";
+
 import {Permit2Bundler} from "../Permit2Bundler.sol";
 import {MigrationBundler} from "./MigrationBundler.sol";
 
@@ -20,6 +22,8 @@ contract CompoundV3MigrationBundler is Permit2Bundler, MigrationBundler {
     /// @notice Repays `amount` of `asset` on the CompoundV3 `instance`, on behalf of the initiator.
     /// @notice Warning: should only be called via the bundler's `multicall` function.
     function compoundV3Repay(address instance, address asset, uint256 amount) external payable {
+        require(instance != address(0), ErrorsLib.ZERO_ADDRESS);
+
         _approveMaxTo(asset, instance);
 
         // Compound V3 uses signed accounting: supplying to a negative balance actually repays the borrow position.
@@ -29,13 +33,17 @@ contract CompoundV3MigrationBundler is Permit2Bundler, MigrationBundler {
     /// @notice Withdraws `amount` of `asset` on the CompoundV3 `instance`.
     /// @dev Initiator must have previously transferred their CompoundV3 position to the bundler.
     function compoundV3Withdraw(address instance, address asset, uint256 amount) external payable {
+        require(instance != address(0), ErrorsLib.ZERO_ADDRESS);
+
         ICompoundV3(instance).withdraw(asset, amount);
     }
 
     /// @notice Withdraws `amount` of `asset` from the CompoundV3 `instance`, on behalf of the initiator.
     /// @notice Warning: should only be called via the bundler's `multicall` function.
     /// @dev Initiator must have previously approved the bundler to manage their CompoundV3 position.
     function compoundV3WithdrawFrom(address instance, address asset, uint256 amount) external payable {
+        require(instance != address(0), ErrorsLib.ZERO_ADDRESS);
+
         ICompoundV3(instance).withdrawFrom(_initiator, address(this), asset, amount);
     }
 
@@ -51,6 +59,8 @@ contract CompoundV3MigrationBundler is Permit2Bundler, MigrationBundler {
         bytes32 r,
         bytes32 s
     ) external payable {
+        require(instance != address(0), ErrorsLib.ZERO_ADDRESS);
+
         ICompoundV3(instance).allowBySig(_initiator, address(this), isAllowed, nonce, expiry, v, r, s);
     }
 }
diff --git a/test/forge/ERC4626BundlerLocalTest.sol b/test/forge/ERC4626BundlerLocalTest.sol
@@ -21,27 +21,55 @@ contract ERC4626BundlerLocalTest is LocalTest {
         bundler = new ERC4626BundlerMock();
     }
 
+    function testErc4626MintZeroAdressTarget(uint256 shares) public {
+        bundle.push(abi.encodeCall(ERC4626Bundler.erc4626Mint, (address(0), shares, RECEIVER)));
+
+        vm.expectRevert(bytes(ErrorsLib.ZERO_ADDRESS));
+        bundler.multicall(block.timestamp, bundle);
+    }
+
     function testErc4626MintZeroAdress(uint256 shares) public {
         bundle.push(abi.encodeCall(ERC4626Bundler.erc4626Mint, (address(vault), shares, address(0))));
 
         vm.expectRevert(bytes(ErrorsLib.ZERO_ADDRESS));
         bundler.multicall(block.timestamp, bundle);
     }
 
+    function testErc4626DepositZeroAdressTarget(uint256 assets) public {
+        bundle.push(abi.encodeCall(ERC4626Bundler.erc4626Deposit, (address(0), assets, RECEIVER)));
+
+        vm.expectRevert(bytes(ErrorsLib.ZERO_ADDRESS));
+        bundler.multicall(block.timestamp, bundle);
+    }
+
     function testErc4626DepositZeroAdress(uint256 assets) public {
         bundle.push(abi.encodeCall(ERC4626Bundler.erc4626Deposit, (address(vault), assets, address(0))));
 
         vm.expectRevert(bytes(ErrorsLib.ZERO_ADDRESS));
         bundler.multicall(block.timestamp, bundle);
     }
 
+    function testErc4626WithdrawZeroAdressTarget(uint256 assets) public {
+        bundle.push(abi.encodeCall(ERC4626Bundler.erc4626Withdraw, (address(0), assets, RECEIVER)));
+
+        vm.expectRevert(bytes(ErrorsLib.ZERO_ADDRESS));
+        bundler.multicall(block.timestamp, bundle);
+    }
+
     function testErc4626WithdrawZeroAdress(uint256 assets) public {
         bundle.push(abi.encodeCall(ERC4626Bundler.erc4626Withdraw, (address(vault), assets, address(0))));
 
         vm.expectRevert(bytes(ErrorsLib.ZERO_ADDRESS));
         bundler.multicall(block.timestamp, bundle);
     }
 
+    function testErc4626RedeemZeroAdressTarget(uint256 assets) public {
+        bundle.push(abi.encodeCall(ERC4626Bundler.erc4626Redeem, (address(0), assets, RECEIVER)));
+
+        vm.expectRevert(bytes(ErrorsLib.ZERO_ADDRESS));
+        bundler.multicall(block.timestamp, bundle);
+    }
+
     function testErc4626RedeemZeroAdress(uint256 shares) public {
         bundle.push(abi.encodeCall(ERC4626Bundler.erc4626Redeem, (address(vault), shares, address(0))));