From 0d739333e9ef4947faa22b6cb1d6f776e38df951 Mon Sep 17 00:00:00 2001
From: teddy <teddy@defi.sucks>
Date: Tue, 23 Jul 2024 14:58:51 -0300
Subject: [PATCH] fix: safeERC20 returns different errors with vs without
 return data

---
 test/unit/BPool/BPool.t.sol | 33 +++++++++++++++++++++++++++++----
 test/unit/BPool/BPool.tree  |  8 ++++++--
 2 files changed, 35 insertions(+), 6 deletions(-)

diff --git a/test/unit/BPool/BPool.t.sol b/test/unit/BPool/BPool.t.sol
index 4a0717fb..81f3d133 100644
--- 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
index 95fa1e23..b14cad33 100644
--- 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