refactor: add conditional to handle ETH transfers

Author: passandscore

contracts/contracts/core/GasTankDepositor.sol

@@ -9,11 +9,10 @@ import {Errors} from "../utils/Errors.sol";
 contract GasTankDepositor {
     address public immutable RPC_SERVICE;
     uint256 public immutable MAXIMUM_DEPOSIT;
+    address public immutable GAS_TANK_ADDRESS;
 
-    event FundsRecovered(address indexed owner, uint256 indexed amount);
     event GasTankFunded(address indexed smartAccount, address indexed caller, uint256 indexed amount);
 
-    error FailedToRecoverFunds(address owner, uint256 amount);
     error FailedToFundGasTank(address rpcProvider, uint256 transferAmount);
     error RPCServiceNotSet(address provider);
     error NotRPCService(address caller);
@@ -38,24 +37,19 @@ contract GasTankDepositor {
         require(_maxDeposit > 0, MaximumDepositNotMet(0, _maxDeposit));
         RPC_SERVICE = rpcService;
         MAXIMUM_DEPOSIT = _maxDeposit;
+        GAS_TANK_ADDRESS = address(this);
     }
 
-    receive() external payable { /* ETH transfers allowed. */ }
+    receive() external payable {
+        if (address(this) == GAS_TANK_ADDRESS) {
+            revert Errors.InvalidReceive();
+        }
+    }
 
     fallback() external payable {
         revert Errors.InvalidFallback();
     }
 
-    /// @notice Recovers funds inadvertently sent to this contract directly.
-    function recoverFunds() external onlyRPCService {
-        uint256 balance = address(this).balance;
-
-        (bool success,) = RPC_SERVICE.call{value: balance}("");
-        require(success, FailedToRecoverFunds(RPC_SERVICE, balance));
-
-        emit FundsRecovered(RPC_SERVICE, balance);
-    }
-
     /// @notice Transfers ETH from the EOA's balance to the Gas RPC Service.
     /// @param _amount The amount to fund the gas tank with.
     /// @dev Only the EOA can call this function.
@@ -69,7 +63,6 @@ contract GasTankDepositor {
         _fundGasTank(MAXIMUM_DEPOSIT);
     }
 
-    /// @dev `fundGasTank` Internal function to fund the gas tank.
     function _fundGasTank(uint256 _amountToTransfer) internal {
         require(address(this).balance >= _amountToTransfer, InsufficientFunds(address(this).balance, _amountToTransfer));
 

contracts/test/core/GasTankDepositorTest.sol

@@ -3,6 +3,7 @@ pragma solidity 0.8.26;
 
 import "forge-std/Test.sol";
 import {GasTankDepositor} from "../../contracts/core/GasTankDepositor.sol";
+import {Errors} from "../../contracts/utils/Errors.sol";
 
 contract GasTankDepositorTest is Test {
     uint256 public constant ALICE_PK = uint256(0xA11CE);
@@ -50,17 +51,11 @@ contract GasTankDepositorTest is Test {
         assertEq(alice.code.length, 0);
     }
 
-    //=======================TESTS FOR IMPROPER CALLS TO THE GAS TANK MANAGER=======================
+    //=======================TESTS FOR RECEIVING FUNDS=======================
 
-    function testFallbackRevert() public {
-        bytes memory badData =
-            abi.encodeWithSelector(GasTankDepositor.recoverFunds.selector, address(0x55555), 1 ether, 1 ether, 1 ether);
-        vm.prank(alice);
-        (bool success,) = address(_gasTankDepositorImpl).call{value: 1 ether}(badData);
-        assertFalse(success);
-    }
+    function testReceivesFunds() public {
+        _delegate();
 
-    function testReceiveNoRevert() public {
         uint256 beforeBalance = alice.balance;
         vm.prank(bob);
         (bool success,) = address(alice).call{value: 1 ether}("");
@@ -69,43 +64,45 @@ contract GasTankDepositorTest is Test {
         assertEq(afterBalance, beforeBalance + 1 ether, "balance not increased");
     }
 
-    function testFundsSentDirectlyToDelegateAddress() public {
-        uint256 beforeBalance = address(_gasTankDepositorImpl).balance;
+    //=======================TESTS FOR RECEIVE AND FALLBACK=======================
 
-        vm.prank(bob);
-        (bool success,) = address(_gasTankDepositorImpl).call{value: 1 ether}("");
-        assertTrue(success);
+    function testFallbackRevert() public {
+        bytes memory badData = abi.encodeWithSelector(bytes4(keccak256("invalidFunction()")));
+        vm.prank(alice);
+        (bool success,) = address(_gasTankDepositorImpl).call{value: 1 ether}(badData);
+        assertFalse(success);
+    }
 
-        uint256 afterBalance = address(_gasTankDepositorImpl).balance;
-        assertEq(afterBalance, beforeBalance + 1 ether, "balance not increased");
+    function testFundsSentDirectlyToDelegateAddress() public {
+        vm.prank(bob);
+        (bool success, bytes memory data) = address(_gasTankDepositorImpl).call{value: 1 ether}("");
+        assertFalse(success);
+        bytes4 selector;
+        assembly {
+            selector := mload(add(data, 0x20))
+        }
+        assertEq(selector, Errors.InvalidReceive.selector);
     }
 
-    function testWithdrawsFundsDirectlyFromDelegateAddress() public {
-        uint256 gasTankBeforeBalance = address(_gasTankDepositorImpl).balance;
-        uint256 depositAmount = 1 ether;
+    function testFundsSentDirectlyToEOAAddressWithDelegation() public {
+        _delegate();
 
+        uint256 beforeBalance = alice.balance;
         vm.prank(bob);
-        (bool success,) = address(_gasTankDepositorImpl).call{value: depositAmount}("");
+        (bool success,) = alice.call{value: 1 ether}("");
         assertTrue(success);
-
-        uint256 gasTankAfterBalance = address(_gasTankDepositorImpl).balance;
-        assertEq(gasTankAfterBalance, gasTankBeforeBalance + depositAmount, "balance not increased");
-
-        vm.prank(rpcService);
-        uint256 rpcServiceBeforeBalance = rpcService.balance;
-        _gasTankDepositorImpl.recoverFunds();
-        uint256 rpcServiceAfterBalance = rpcService.balance;
-
-        assertEq(address(_gasTankDepositorImpl).balance, 0, "funds not drained");
-        assertEq(rpcServiceAfterBalance, rpcServiceBeforeBalance + depositAmount, "balance not recovered");
+        uint256 afterBalance = alice.balance;
+        assertEq(afterBalance, beforeBalance + 1 ether, "balance not increased");
     }
 
-    function testRevertsWhenRecoverFundsIsCalledByUnknownCaller() public {
+    function testFundsSentDirectlyToEOAAddressWithoutDelegation() public {
+        uint256 beforeBalance = alice.balance;
         vm.prank(bob);
-        vm.expectRevert(abi.encodeWithSelector(GasTankDepositor.NotRPCService.selector, bob));
-        _gasTankDepositorImpl.recoverFunds();
+        (bool success,) = address(alice).call{value: 1 ether}("");
+        assertTrue(success);
+        uint256 afterBalance = alice.balance;
+        assertEq(afterBalance, beforeBalance + 1 ether, "balance not increased");
     }
-
     //=======================TESTS FOR FUNDING THE GAS TANK=======================
 
     function testRpcServiceFundsMaximumDeposit() public {