From 0fef4649cad55ab8f18c46d43fffdc841c220f13 Mon Sep 17 00:00:00 2001
From: Michael Standen <screaminghawk@gmail.com>
Date: Mon, 11 Sep 2023 11:12:20 +1200
Subject: [PATCH] Nicer errors

---
 contracts/hooks/EIP4337Hook.sol             | 13 +++++++-----
 contracts/hooks/interfaces/IEIP4337Hook.sol |  9 ++++++++-
 foundry_test/hooks/EIP4337Hook.t.sol        | 22 +++++++++++++++++++--
 3 files changed, 36 insertions(+), 8 deletions(-)

diff --git a/contracts/hooks/EIP4337Hook.sol b/contracts/hooks/EIP4337Hook.sol
index 51a24873..b308c0bd 100644
--- a/contracts/hooks/EIP4337Hook.sol
+++ b/contracts/hooks/EIP4337Hook.sol
@@ -20,7 +20,9 @@ contract EIP4337Hook is IEIP4337Hook, ModuleNonce {
   }
 
   modifier onlyEntrypoint() {
-    require(msg.sender == entrypoint, 'EIP4337Hook: only 4337 or self'); //FIXME error obj
+    if (msg.sender != entrypoint) {
+      revert InvalidCaller();
+    }
     _;
   }
 
@@ -28,14 +30,14 @@ contract EIP4337Hook is IEIP4337Hook, ModuleNonce {
    * Allow the EIP-4337 entrypoint to execute a transaction on the wallet.
    * @dev This function does not validate as the entrypoint is trusted to have called validateUserOp.
    * @dev Failure handling done by ModuleCalls.
+   * @notice This functions is only callable by the Entrypoint.
    */
   function eip4337SelfExecute(IModuleCalls.Transaction[] calldata txs) external payable onlyEntrypoint {
     // Self execute
     (bool success, ) = payable(address(this)).call{value: msg.value}(
       abi.encodeWithSelector(IModuleCalls.selfExecute.selector, txs)
     );
-    //FIXME Required? selfexecute should revert if it fails
-    require(success, 'call failed'); // FIXME Better error?
+    (success);
   }
 
   /**
@@ -48,8 +50,9 @@ contract EIP4337Hook is IEIP4337Hook, ModuleNonce {
     bytes32 userOpHash,
     uint256 missingAccountFunds
   ) external onlyEntrypoint returns (uint256 validationData) {
-    // Check nonce
-    _validateNonce(userOp.nonce); //FIXME Sequence space encoding is diff to EIP-4337 encoding
+    // Check nonce.
+    // Note Sequence space encoding is diff to EIP-4337 encoding.
+    _validateNonce(userOp.nonce);
 
     // Check signature
     bytes32 ethHash = keccak256(abi.encodePacked("\x19Ethereum Signed Message:\n32", userOpHash));
diff --git a/contracts/hooks/interfaces/IEIP4337Hook.sol b/contracts/hooks/interfaces/IEIP4337Hook.sol
index cd40b450..a2f7cd44 100644
--- a/contracts/hooks/interfaces/IEIP4337Hook.sol
+++ b/contracts/hooks/interfaces/IEIP4337Hook.sol
@@ -4,10 +4,17 @@ pragma solidity 0.8.18;
 import "../../modules/commons/interfaces/IModuleCalls.sol";
 import "./IAccount.sol";
 
+interface IEIP4337HookErrors {
+
+  // Thrown when not called by the entrypoint.
+  error InvalidCaller();
+
+}
+
 /**
  * An extension to EIP-4337 that includes a self execute function.
  */
-interface IEIP4337Hook is IAccount {
+interface IEIP4337Hook is IAccount, IEIP4337HookErrors {
 
   /**
    * @notice Allow wallet owner to execute an action.
diff --git a/foundry_test/hooks/EIP4337Hook.t.sol b/foundry_test/hooks/EIP4337Hook.t.sol
index 062f89b2..b52e6eb1 100644
--- a/foundry_test/hooks/EIP4337Hook.t.sol
+++ b/foundry_test/hooks/EIP4337Hook.t.sol
@@ -2,6 +2,7 @@
 pragma solidity 0.8.18;
 
 import 'contracts/hooks/EIP4337Hook.sol';
+import 'contracts/hooks/interfaces/IEIP4337Hook.sol';
 import 'contracts/modules/commons/ModuleHooks.sol';
 import 'contracts/modules/MainModule.sol';
 import 'contracts/modules/MainModuleUpgradable.sol';
@@ -9,7 +10,7 @@ import 'contracts/Factory.sol';
 
 import 'foundry_test/base/AdvTest.sol';
 
-contract EIP4337HookTest is AdvTest {
+contract EIP4337HookTest is AdvTest, IEIP4337HookErrors {
   MainModule private template;
   EIP4337Hook private wallet;
 
@@ -40,7 +41,24 @@ contract EIP4337HookTest is AdvTest {
     uint256 value;
   }
 
-  function test_execute_sendEth(ToVal memory sendTx) external {
+  function test_4337execute_invalidCaller(ToVal memory sendTx, address sender) external {
+    vm.assume(sender != ENTRYPOINT);
+    IModuleCalls.Transaction[] memory txs = new IModuleCalls.Transaction[](1);
+    txs[0] = IModuleCalls.Transaction({
+      delegateCall: false,
+      revertOnError: false,
+      gasLimit: 0,
+      target: sendTx.target,
+      value: sendTx.value,
+      data: ''
+    });
+
+    vm.expectRevert(InvalidCaller.selector);
+    vm.prank(sender);
+    wallet.eip4337SelfExecute(txs);
+  }
+
+  function test_4337execute_sendEth(ToVal memory sendTx) external {
     vm.assume(sendTx.target.code.length == 0); // Non contract
     uint256 walletBal = address(wallet).balance;
     vm.assume(sendTx.value <= walletBal);