fix: safeERC20 returns different errors with vs without return data

defi-wonderland · Jul 23, 2024 · 0d73933 · 0d73933
1 parent d9b77d1
commit 0d73933
Show file tree

Hide file tree

Showing 2 changed files with 35 additions and 6 deletions.
diff --git a/test/unit/BPool/BPool.t.sol b/test/unit/BPool/BPool.t.sol
@@ -5,6 +5,7 @@ import {BPoolBase} from './BPoolBase.sol';
 
 import {IERC20} from '@openzeppelin/contracts/token/ERC20/IERC20.sol';
 import {SafeERC20} from '@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol';
+import {Address} from '@openzeppelin/contracts/utils/Address.sol';
 
 import {BMath} from 'contracts/BMath.sol';
 import {IBPool} from 'interfaces/IBPool.sol';
@@ -446,13 +447,25 @@ contract BPool is BPoolBase, BMath {
     bPool.call__pushUnderlying(transferredToken, transferRecipient, transferredAmount);
   }
 
-  function test__pushUnderlyingRevertWhen_UnderlyingTokenReverts(bytes memory errorData) external {
-    // it should revert
+  function test__pushUnderlyingRevertWhen_UnderlyingTokenRevertsWithoutData() external {
+    // it should revert with FailedInnerCall
+    vm.mockCallRevert(
+      transferredToken,
+      abi.encodeWithSelector(IERC20.transfer.selector, transferRecipient, transferredAmount),
+      abi.encode()
+    );
+    vm.expectRevert(Address.FailedInnerCall.selector);
+    bPool.call__pushUnderlying(transferredToken, transferRecipient, transferredAmount);
+  }
+
+  function test__pushUnderlyingRevertWhen_UnderlyingTokenRevertsWithData(bytes memory errorData) external {
+    vm.assume(keccak256(errorData) != keccak256(bytes('')));
     vm.mockCallRevert(
       transferredToken,
       abi.encodeWithSelector(IERC20.transfer.selector, transferRecipient, transferredAmount),
       errorData
     );
+    // it should revert with same error data
     vm.expectRevert(errorData);
     bPool.call__pushUnderlying(transferredToken, transferRecipient, transferredAmount);
   }
@@ -492,13 +505,25 @@ contract BPool is BPoolBase, BMath {
     bPool.call__pullUnderlying(transferredToken, transferFromSpender, transferredAmount);
   }
 
-  function test__pullUnderlyingRevertWhen_UnderlyingTokenReverts(bytes memory errorData) external {
+  function test__pullUnderlyingRevertWhen_UnderlyingTokenRevertsWithoutData() external {
+    vm.mockCallRevert(
+      transferredToken,
+      abi.encodeWithSelector(IERC20.transferFrom.selector, transferFromSpender, address(bPool), transferredAmount),
+      abi.encode()
+    );
+    // it should revert with FailedInnerCall
+    vm.expectRevert(Address.FailedInnerCall.selector);
+    bPool.call__pullUnderlying(transferredToken, transferFromSpender, transferredAmount);
+  }
+
+  function test__pullUnderlyingRevertWhen_UnderlyingTokenRevertsWithData(bytes memory errorData) external {
+    vm.assume(keccak256(errorData) != keccak256(bytes('')));
     vm.mockCallRevert(
       transferredToken,
       abi.encodeWithSelector(IERC20.transferFrom.selector, transferFromSpender, address(bPool), transferredAmount),
       errorData
     );
-    // it should revert
+    // it should revert with same error data
     vm.expectRevert(errorData);
     bPool.call__pullUnderlying(transferredToken, transferFromSpender, transferredAmount);
   }

diff --git a/test/unit/BPool/BPool.tree b/test/unit/BPool/BPool.tree
@@ -147,7 +147,9 @@ BPool::_pushUnderlying
 │   └── it should revert
 ├── when underlying token doesnt return a value
 │   └── it assumes transfer success
-├── when underlying token reverts
+├── when underlying token reverts without data
+│   └── it should revert // with FailedInnerCall
+├── when underlying token reverts with data
 │   └── it should revert // with same error data
 └── when underlying token returns true
     └── it calls underlying transfer
@@ -157,7 +159,9 @@ BPool::_pullUnderlying
 │   └── it should revert
 ├── when underlying token doesnt return a value
 │   └── it assumes transferFrom success
-├── when underlying token reverts
+├── when underlying token reverts without data
+│   └── it should revert // with FailedInnerCall
+├── when underlying token reverts with data
 │   └── it should revert // with same error data
 └── when underlying token returns true
     └── it calls underlying transferFrom