AA-466: Require SenderCreator to be called from EntryPoint & AA-470: Make SenderCreator public

.github/workflows/build.yml

@@ -80,7 +80,7 @@ jobs:
       - run: yarn compile
 
       - run: FORCE_COLOR=1 yarn coverage
-      - uses: actions/upload-artifact@v2
+      - uses: actions/upload-artifact@v4
         with:
           name: solidity-coverage
           path: |

contracts/core/EntryPoint.sol

@@ -30,7 +30,7 @@ contract EntryPoint is IEntryPoint, StakeManager, NonceManager, ReentrancyGuard,
 
     SenderCreator private immutable _senderCreator = new SenderCreator();
 
-    function senderCreator() internal view virtual returns (SenderCreator) {
+    function senderCreator() public view virtual returns (ISenderCreator) {
         return _senderCreator;
     }
 

contracts/core/EntryPointSimulations.sol

@@ -24,7 +24,7 @@ contract EntryPointSimulations is EntryPoint, IEntryPointSimulations {
         _senderCreator = SenderCreator(createdObj);
     }
 
-    function senderCreator() internal view virtual override returns (SenderCreator) {
+    function senderCreator() public view virtual override(EntryPoint, IEntryPoint) returns (ISenderCreator) {
         // return the same senderCreator as real EntryPoint.
         // this call is slightly (100) more expensive than EntryPoint's access to immutable member
         return _senderCreator;
@@ -35,7 +35,7 @@ contract EntryPointSimulations is EntryPoint, IEntryPointSimulations {
      * it as entrypoint, since the simulation functions don't check the signatures
      */
     constructor() {
-        require(block.number < 100, "should not be deployed");
+        require(block.number < 1000, "should not be deployed");
     }
 
     /// @inheritdoc IEntryPointSimulations

contracts/core/SenderCreator.sol

@@ -1,11 +1,19 @@
 // SPDX-License-Identifier: GPL-3.0
 pragma solidity ^0.8.23;
 
+import "../interfaces/ISenderCreator.sol";
+
 /**
  * Helper contract for EntryPoint, to call userOp.initCode from a "neutral" address,
  * which is explicitly not the entryPoint itself.
  */
-contract SenderCreator {
+contract SenderCreator is ISenderCreator {
+    address public immutable entryPoint;
+
+    constructor(){
+        entryPoint = msg.sender;
+    }
+
     /**
      * Call the "initCode" factory to create and return the sender account address.
      * @param initCode - The initCode value from a UserOp. contains 20 bytes of factory address,
@@ -15,6 +23,9 @@ contract SenderCreator {
     function createSender(
         bytes calldata initCode
     ) external returns (address sender) {
+        if (msg.sender != entryPoint) {
+            revert("AA97 should call from EntryPoint");
+        }
         address factory = address(bytes20(initCode[0:20]));
         bytes memory initCallData = initCode[20:];
         bool success;

contracts/interfaces/IEntryPoint.sol

@@ -13,6 +13,7 @@ import "./PackedUserOperation.sol";
 import "./IStakeManager.sol";
 import "./IAggregator.sol";
 import "./INonceManager.sol";
+import "./ISenderCreator.sol";
 
 interface IEntryPoint is IStakeManager, INonceManager {
     /***
@@ -220,4 +221,9 @@ interface IEntryPoint is IStakeManager, INonceManager {
      * @param data data to pass to target in a delegatecall
      */
     function delegateAndRevert(address target, bytes calldata data) external;
+
+    /**
+     * @notice Retrieves the immutable SenderCreator contract which is responsible for deployment of sender contracts.
+     */
+    function senderCreator() external view returns (ISenderCreator);
 }

contracts/interfaces/ISenderCreator.sol

@@ -0,0 +1,11 @@
+
+// SPDX-License-Identifier: MIT
+pragma solidity ^0.8.0;
+
+interface ISenderCreator {
+    /**
+     * @dev Creates a new sender contract.
+     * @return sender Address of the newly created sender contract.
+     */
+    function createSender(bytes calldata initCode) external returns (address sender);
+}

contracts/samples/SimpleAccountFactory.sol

@@ -4,6 +4,7 @@ pragma solidity ^0.8.23;
 import "@openzeppelin/contracts/utils/Create2.sol";
 import "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol";
 
+import "../interfaces/ISenderCreator.sol";
 import "./SimpleAccount.sol";
 
 /**
@@ -14,9 +15,11 @@ import "./SimpleAccount.sol";
  */
 contract SimpleAccountFactory {
     SimpleAccount public immutable accountImplementation;
+    ISenderCreator public immutable senderCreator;
 
     constructor(IEntryPoint _entryPoint) {
         accountImplementation = new SimpleAccount(_entryPoint);
+        senderCreator = _entryPoint.senderCreator();
     }
 
     /**
@@ -26,6 +29,7 @@ contract SimpleAccountFactory {
      * This method returns an existing account address so that entryPoint.getSenderAddress() would work even after account creation
      */
     function createAccount(address owner,uint256 salt) public returns (SimpleAccount ret) {
+        require(msg.sender == address(senderCreator), "only callable from SenderCreator");
         address addr = getAddress(owner, salt);
         uint256 codeSize = addr.code.length;
         if (codeSize > 0) {

gascalc/GasChecker.ts

@@ -93,6 +93,7 @@ export class GasChecker {
   accountOwner: Wallet
 
   accountInterface: SimpleAccountInterface
+  private locked: boolean
 
   constructor () {
     this.accountOwner = createAccountOwner()
@@ -139,24 +140,26 @@ export class GasChecker {
       const addr = await fact.getAddress(this.accountOwner.address, salt)
 
       if (!this.createdAccounts.has(addr)) {
-        // explicit call to fillUseROp with no "entryPoint", to make sure we manually fill everything and
-        // not attempt to fill from blockchain.
-        const op = signUserOp(await fillUserOp({
-          sender: addr,
-          nonce: 0,
-          callGasLimit: 30000,
-          verificationGasLimit: 1000000,
-          // paymasterAndData: paymaster,
-          preVerificationGas: 1,
-          maxFeePerGas: 0
-        }), this.accountOwner, this.entryPoint().address, await provider.getNetwork().then(net => net.chainId))
-        creationOps.push(packUserOp(op))
+        const codeSize = await provider.getCode(addr).then(code => code.length)
+        if (codeSize === 2) {
+          // explicit call to fillUseROp with no "entryPoint", to make sure we manually fill everything and
+          // not attempt to fill from blockchain.
+          const op = signUserOp(await fillUserOp({
+            sender: addr,
+            initCode: this.accountInitCode(fact, salt),
+            nonce: 0,
+            callGasLimit: 30000,
+            verificationGasLimit: 1000000,
+            // paymasterAndData: paymaster,
+            preVerificationGas: 1,
+            maxFeePerGas: 0
+          }), this.accountOwner, this.entryPoint().address, await provider.getNetwork().then(net => net.chainId))
+          creationOps.push(packUserOp(op))
+        }
         this.createdAccounts.add(addr)
       }
 
       this.accounts[addr] = this.accountOwner
-      // deploy if not already deployed.
-      await fact.createAccount(this.accountOwner.address, salt)
       const accountBalance = await GasCheckCollector.inst.entryPoint.balanceOf(addr)
       if (accountBalance.lte(minDepositOrBalance)) {
         await GasCheckCollector.inst.entryPoint.depositTo(addr, { value: minDepositOrBalance.mul(5) })
@@ -224,21 +227,30 @@ export class GasChecker {
         }
         // console.debug('== account est=', accountEst.toString())
         accountEst = est.accountEst
-        const op = await fillSignAndPack({
-          sender: account,
-          callData: accountExecFromEntryPoint,
-          maxPriorityFeePerGas: info.gasPrice,
-          maxFeePerGas: info.gasPrice,
-          callGasLimit: accountEst,
-          verificationGasLimit: 1000000,
-          paymaster: paymaster,
-          paymasterVerificationGasLimit: 50000,
-          paymasterPostOpGasLimit: 50000,
-          preVerificationGas: 1
-        }, accountOwner, GasCheckCollector.inst.entryPoint)
-        // const packed = packUserOp(op, false)
-        // console.log('== packed cost=', callDataCost(packed), packed)
-        return op
+        while (this.locked) {
+          await new Promise(resolve => setTimeout(resolve, 1))
+        }
+        try {
+          this.locked = true
+
+          const op = await fillSignAndPack({
+            sender: account,
+            callData: accountExecFromEntryPoint,
+            maxPriorityFeePerGas: info.gasPrice,
+            maxFeePerGas: info.gasPrice,
+            callGasLimit: accountEst,
+            verificationGasLimit: 1000000,
+            paymaster: paymaster,
+            paymasterVerificationGasLimit: 50000,
+            paymasterPostOpGasLimit: 50000,
+            preVerificationGas: 1
+          }, accountOwner, GasCheckCollector.inst.entryPoint)
+          // const packed = packUserOp(op, false)
+          // console.log('== packed cost=', callDataCost(packed), packed)
+          return op
+        } finally {
+          this.locked = false
+        }
       }))
 
     const txdata = GasCheckCollector.inst.entryPoint.interface.encodeFunctionData('handleOps', [userOps, info.beneficiary])
@@ -254,6 +266,13 @@ export class GasChecker {
       throw e
     })
     const ret = await GasCheckCollector.inst.entryPoint.handleOps(userOps, info.beneficiary, { gasLimit: gasEst.mul(3).div(2) })
+    // "ret.wait()" is dead slow without it...
+    for (let count = 0; count < 100; count++) {
+      if (await provider.getTransactionReceipt(ret.hash) != null) {
+        break
+      }
+      await new Promise(resolve => setTimeout(resolve, 10))
+    }
     const rcpt = await ret.wait()
     const gasUsed = rcpt.gasUsed.toNumber()
     const countSuccessOps = rcpt.events?.filter(e => e.event === 'UserOperationEvent' && e.args?.success).length

reports/gas-checker.txt

@@ -12,44 +12,44 @@
 ║                                │       │               │     (delta for │        (compared to ║
 ║                                │       │               │    one UserOp) │     account.exec()) ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple                         │     1 │         79994 │                │                     ║
+║ simple                         │     1 │         80016 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
 ║ simple - diff from previous    │     2 │               │          42192 │               13213 ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple                         │    10 │        459921 │                │                     ║
+║ simple                         │    10 │        459919 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple - diff from previous    │    11 │               │          42223 │               13244 ║
+║ simple - diff from previous    │    11 │               │          42235 │               13256 ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple paymaster               │     1 │         86113 │                │                     ║
+║ simple paymaster               │     1 │         86159 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
 ║ simple paymaster with diff     │     2 │               │          41024 │               12045 ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple paymaster               │    10 │        455444 │                │                     ║
+║ simple paymaster               │    10 │        455670 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ simple paymaster with diff     │    11 │               │          41088 │               12109 ║
+║ simple paymaster with diff     │    11 │               │          41112 │               12133 ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ big tx 5k                      │     1 │        181026 │                │                     ║
+║ big tx 5k                      │     1 │        181060 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ big tx - diff from previous    │     2 │               │         142714 │               17490 ║
+║ big tx - diff from previous    │     2 │               │         142702 │               17478 ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ big tx 5k                      │    10 │       1465443 │                │                     ║
+║ big tx 5k                      │    10 │       1465489 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ big tx - diff from previous    │    11 │               │         142686 │               17462 ║
+║ big tx - diff from previous    │    11 │               │         142710 │               17486 ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ paymaster+postOp               │     1 │         87712 │                │                     ║
+║ paymaster+postOp               │     1 │         87770 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ paymaster+postOp with diff     │     2 │               │          42671 │               13692 ║
+║ paymaster+postOp with diff     │     2 │               │          42659 │               13680 ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ paymaster+postOp               │    10 │        471754 │                │                     ║
+║ paymaster+postOp               │    10 │        471836 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ paymaster+postOp with diff     │    11 │               │          42728 │               13749 ║
+║ paymaster+postOp with diff     │    11 │               │          42692 │               13713 ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ token paymaster                │     1 │        128777 │                │                     ║
+║ token paymaster                │     1 │        128821 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ token paymaster with diff      │     2 │               │          66386 │               37407 ║
+║ token paymaster with diff      │     2 │               │          66408 │               37429 ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ token paymaster                │    10 │        726504 │                │                     ║
+║ token paymaster                │    10 │        726686 │                │                     ║
 ╟────────────────────────────────┼───────┼───────────────┼────────────────┼─────────────────────╢
-║ token paymaster with diff      │    11 │               │          66394 │               37415 ║
+║ token paymaster with diff      │    11 │               │          66524 │               37545 ║
 ╚════════════════════════════════╧═══════╧═══════════════╧════════════════╧═════════════════════╝