Contracts cleanup

packages/contracts/src/AAFactory.sol

@@ -1,14 +1,12 @@
 // SPDX-License-Identifier: MIT
 pragma solidity ^0.8.24;
 
-import "@matterlabs/zksync-contracts/l2/system-contracts/Constants.sol";
-import "@matterlabs/zksync-contracts/l2/system-contracts/libraries/SystemContractsCaller.sol";
+import { DEPLOYER_SYSTEM_CONTRACT, IContractDeployer } from "@matterlabs/zksync-contracts/l2/system-contracts/Constants.sol";
+import { SystemContractsCaller } from "@matterlabs/zksync-contracts/l2/system-contracts/libraries/SystemContractsCaller.sol";
 
 import { ISsoAccount } from "./interfaces/ISsoAccount.sol";
 import { UpgradeableBeacon } from "./UpgradeableBeacon.sol";
 
-import "./helpers/Logger.sol";
-
 contract AAFactory is UpgradeableBeacon {
   event AccountCreated(address indexed accountAddress, string uniqueAccountId);
 
@@ -43,9 +41,6 @@ contract AAFactory is UpgradeableBeacon {
 
     (accountAddress) = abi.decode(returnData, (address));
 
-    Logger.logString("accountAddress");
-    Logger.logAddress(accountAddress);
-
     // add session-key/spend-limit module (similar code)
     ISsoAccount(accountAddress).initialize(initialValidators, initialModules, initialK1Owners);
 

packages/contracts/src/AccountProxy.sol

@@ -1,10 +1,19 @@
-// SPDX-License-Identifier: GPL-3.0
-pragma solidity ^0.8.17;
+// SPDX-License-Identifier: MIT
+pragma solidity ^0.8.24;
 
-// TODO: use this to optimize gas?
-// import { EfficientCall } from "@matterlabs/zksync-contracts/l2/system-contracts/libraries/EfficientCall.sol";
+import { Proxy } from "@openzeppelin/contracts/proxy/Proxy.sol";
+import { EfficientProxy } from "./EfficientProxy.sol";
 import { BeaconProxy } from "@openzeppelin/contracts/proxy/beacon/BeaconProxy.sol";
 
-contract AccountProxy is BeaconProxy {
+/// @title AccountProxy
+/// @author Matter Labs
+/// @custom:security-contact [email protected]
+/// @notice This contract is modification of OpenZeppelin `BeaconProxy` with optimisation for
+/// cheap delegate calls on ZKsync.
+contract AccountProxy is BeaconProxy, EfficientProxy {
   constructor(address beacon) BeaconProxy(beacon, bytes("")) {}
+
+  function _delegate(address implementation) internal override(EfficientProxy, Proxy) {
+    EfficientProxy._delegate(implementation);
+  }
 }

packages/contracts/src/EfficientProxy.sol

@@ -0,0 +1,20 @@
+// SPDX-License-Identifier: MIT
+pragma solidity ^0.8.24;
+
+import { EfficientCall } from "@matterlabs/zksync-contracts/l2/system-contracts/libraries/EfficientCall.sol";
+
+/// @title EfficientProxy
+/// @author Matter Labs
+/// @custom:security-contact [email protected]
+/// @notice This contract implement ultra-efficient way for executing delegate calls. It is compatible with
+/// OpenZeppelin proxy implementations.
+abstract contract EfficientProxy {
+  function _delegate(address implementation) internal virtual {
+    // Use the EfficientCall library to forward calldata to the implementation contract,
+    // instead of copying it from calldata to memory.
+    bytes memory data = EfficientCall.delegateCall(gasleft(), implementation, msg.data);
+    assembly {
+      return(add(data, 0x20), mload(data))
+    }
+  }
+}

packages/contracts/src/TransparentProxy.sol

@@ -1,12 +1,20 @@
-// SPDX-License-Identifier: GPL-3.0
-pragma solidity ^0.8.17;
+// SPDX-License-Identifier: MIT
+pragma solidity ^0.8.24;
 
-// This proxy is placed in front of AAFactory and all modules (WebAuthValidator, SessionKeyValidator).
-
-// TODO: use this to optimize gas?
-// import { EfficientCall } from "@matterlabs/zksync-contracts/l2/system-contracts/libraries/EfficientCall.sol";
+import { EfficientProxy } from "./EfficientProxy.sol";
+import { Proxy } from "@openzeppelin/contracts/proxy/Proxy.sol";
 import { TransparentUpgradeableProxy } from "@openzeppelin/contracts/proxy/transparent/TransparentUpgradeableProxy.sol";
 
-contract TransparentProxy is TransparentUpgradeableProxy {
+/// @title TransparentProxy
+/// @author Matter Labs
+/// @custom:security-contact [email protected]
+/// @notice This contract is modification of OpenZeppelin `TransparentUpgradeableProxy` with optimisation for
+/// cheap delegate calls on ZKsync.
+/// @dev This proxy is placed in front of `AAFactory` and all modules (`WebAuthValidator`, `SessionKeyValidator`).
+contract TransparentProxy is TransparentUpgradeableProxy, EfficientProxy {
   constructor(address implementation) TransparentUpgradeableProxy(implementation, msg.sender, bytes("")) {}
+
+  function _delegate(address implementation) internal override(EfficientProxy, Proxy) {
+    EfficientProxy._delegate(implementation);
+  }
 }

packages/contracts/src/batch/BatchCaller.sol

@@ -1,53 +1,62 @@
 // SPDX-License-Identifier: MIT
 pragma solidity ^0.8.24;
 
-import { SystemContractHelper } from "@matterlabs/zksync-contracts/l2/system-contracts/libraries/SystemContractHelper.sol";
+import { SystemContractsCaller } from "@matterlabs/zksync-contracts/l2/system-contracts/libraries/SystemContractsCaller.sol";
 import { EfficientCall } from "@matterlabs/zksync-contracts/l2/system-contracts/libraries/EfficientCall.sol";
+import { DEPLOYER_SYSTEM_CONTRACT } from "@matterlabs/zksync-contracts/l2/system-contracts/Constants.sol";
 import { Errors } from "../libraries/Errors.sol";
 
-// Each call data for batches
+/// @dev Represents an external call data.
+/// @param target The address to which the call will be made.
+/// @param allowFailure Flag that represents whether to revert the whole batch if the call fails.
+/// @param value The amount of Ether (in wei) to be sent along with the call.
+/// @param callData The calldata to be executed on the `target` address.
 struct Call {
-  address target; // Target contract address
-  bool allowFailure; // Whether to revert if the call fails
-  uint256 value; // Amount of ETH to send with call
-  bytes callData; // Calldata to send
+  address target;
+  bool allowFailure;
+  uint256 value;
+  bytes callData;
 }
 
-/// @title BatchCaller
-/// @notice Make multiple calls in a single transaction
-contract BatchCaller {
-  /// @notice Make multiple calls, ensure success if required
-  /// @dev Reverts if not called via delegatecall
-  /// @param calls Call[] calldata - An array of Call structs
-  function batchCall(Call[] calldata calls) external payable {
-    require(msg.sender == address(this), "External calls not allowed");
-
-    bool isDelegateCall = SystemContractHelper.getCodeAddress() != address(this);
-    if (!isDelegateCall) {
-      revert Errors.ONLY_DELEGATECALL();
+/// @title SSO Account
+/// @author Matter Labs
+/// @custom:security-contact [email protected]
+/// @notice Make multiple calls from Account in a single transaction.
+/// @notice The implementation is inspired by Clave wallet.
+abstract contract BatchCaller {
+  /// @notice Make multiple calls, ensure success if required.
+  /// @dev The total Ether sent across all calls must be equal to `msg.value` to maintain the invariant
+  /// that `msg.value` + `tx.fee` is the maximum amount of Ether that can be spent on the transaction.
+  /// @param _calls Array of Call structs, each representing an individual external call to be made.
+  function batchCall(Call[] calldata _calls) external payable {
+    if (msg.sender != address(this)) {
+      revert Errors.NOT_FROM_SELF();
     }
 
-    // Execute each call
-    uint256 len = calls.length;
-    uint256 totalValue = 0;
-    Call calldata calli;
-    for (uint256 i = 0; i < len; ) {
-      calli = calls[i];
-      address target = calli.target;
-      uint256 value = calli.value;
-      bytes calldata callData = calli.callData;
-      totalValue += calli.value;
-
-      bool success = EfficientCall.rawCall(gasleft(), target, value, callData, false);
-      if (!calls[i].allowFailure && !success) {
-        revert Errors.CALL_FAILED();
+    uint256 totalValue;
+    uint256 len = _calls.length;
+    for (uint256 i = 0; i < len; ++i) {
+      totalValue += _calls[i].value;
+      bool success;
+      if (_calls[i].target == address(DEPLOYER_SYSTEM_CONTRACT)) {
+        // Note, that the deployer contract can only be called with a "systemCall" flag.
+        success = SystemContractsCaller.systemCall(
+          uint32(gasleft()),
+          _calls[i].target,
+          _calls[i].value,
+          _calls[i].callData
+        );
+      } else {
+        success = EfficientCall.rawCall(gasleft(), _calls[i].target, _calls[i].value, _calls[i].callData, false);
       }
 
-      unchecked {
-        i++;
+      if (!_calls[i].allowFailure && !success) {
+        revert Errors.CALL_FAILED();
       }
     }
 
-    require(totalValue == msg.value, "Incorrect value for batch call");
+    if (totalValue != msg.value) {
+      revert Errors.MsgValueMismatch(msg.value, totalValue);
+    }
   }
 }

packages/contracts/src/handlers/ValidationHandler.sol

@@ -9,8 +9,6 @@ import { ValidatorManager } from "../managers/ValidatorManager.sol";
 import { IK1Validator, IR1Validator } from "../interfaces/IValidator.sol";
 import { IModuleValidator } from "../interfaces/IModuleValidator.sol";
 
-import "../helpers/Logger.sol";
-
 /**
  * @title ValidationHandler
  * @notice Contract which calls validators for signature validation
@@ -47,7 +45,6 @@ abstract contract ValidationHandler is OwnerManager, ValidatorManager {
         return true;
       }
     } else if (_isModuleValidator(validator)) {
-      Logger.logString("_isModuleValidator");
       return IModuleValidator(validator).handleValidation(signedHash, signature);
     }
 

packages/contracts/src/libraries/Errors.sol

@@ -10,6 +10,7 @@ library Errors {
   error FEE_PAYMENT_FAILED();
   error UNAUTHORIZED_OUTSIDE_TRANSACTION();
   error VALIDATION_HOOK_FAILED();
+  error METHOD_NOT_IMPLEMENTED();
 
   /*//////////////////////////////////////////////////////////////
                                LINKED LIST
@@ -129,8 +130,8 @@ library Errors {
                             BatchCaller
     //////////////////////////////////////////////////////////////*/
 
-  error ONLY_DELEGATECALL();
   error CALL_FAILED();
+  error MsgValueMismatch(uint256 actualValue, uint256 expectedValue);
 
   /*//////////////////////////////////////////////////////////////
                             INITABLE

packages/contracts/src/managers/HookManager.sol

@@ -131,14 +131,15 @@ abstract contract HookManager is IHookManager, Auth {
     mapping(address => address) storage validationHooks = _validationHooksLinkedList();
 
     address cursor = validationHooks[AddressLinkedList.SENTINEL_ADDRESS];
-    uint256 idx = 0;
+    uint256 idx;
     // Iterate through hooks
     while (cursor > AddressLinkedList.SENTINEL_ADDRESS) {
       // Call it with corresponding hookData
       bool success = _call(
         cursor,
-        abi.encodeWithSelector(IValidationHook.validationHook.selector, signedHash, transaction, hookData[idx++])
+        abi.encodeCall(IValidationHook.validationHook, (signedHash, transaction, hookData[idx]))
       );
+      ++idx;
 
       if (!success) {
         return false;

packages/contracts/src/validators/PasskeyValidator.sol

@@ -9,8 +9,6 @@ import { VerifierCaller } from "../helpers/VerifierCaller.sol";
 import { JsmnSolLib } from "../libraries/JsmnSolLib.sol";
 import { Strings } from "@openzeppelin/contracts/utils/Strings.sol";
 
-import "../helpers/Logger.sol";
-
 /**
  * @title validator contract for passkey r1 signatures
  * @author https://getclave.io

packages/contracts/src/validators/WebAuthValidator.sol

@@ -4,8 +4,6 @@ pragma solidity ^0.8.24;
 import { IModuleValidator } from "../interfaces/IModuleValidator.sol";
 import "./PasskeyValidator.sol";
 
-import "../helpers/Logger.sol";
-
 /**
  * @title validator contract for passkey r1 signatures
  * @author https://getclave.io
@@ -29,10 +27,6 @@ contract WebAuthValidator is PasskeyValidator, IModuleValidator {
   }
 
   function handleValidation(bytes32 signedHash, bytes memory signature) external view returns (bool) {
-    // Printing this hash makes capturing this for a replay test easier
-    Logger.logString("signed hash");
-    Logger.logBytes32(signedHash);
-
     return webAuthVerify(signedHash, signature);
   }
 
@@ -42,22 +36,18 @@ contract WebAuthValidator is PasskeyValidator, IModuleValidator {
     );
 
     if (rs[1] > lowSmax) {
-      Logger.logString("malleability check failed");
       return false;
     }
 
     // check if the flags are set
     if (authenticatorData[32] & AUTH_DATA_MASK != AUTH_DATA_MASK) {
-      Logger.logString("auth data mask failed");
       return false;
     }
 
     // parse out the important fields (type, challenge, and origin): https://goo.gl/yabPex
     // TODO: test if the parse fails for more than 10 elements, otherwise can have a malicious header
     (uint returnValue, JsmnSolLib.Token[] memory tokens, uint actualNum) = JsmnSolLib.parse(clientDataJSON, 20);
     if (returnValue != 0) {
-      Logger.logString("failed to parse json");
-      Logger.logUint(returnValue);
       return false;
     }
 
@@ -76,38 +66,30 @@ contract WebAuthValidator is PasskeyValidator, IModuleValidator {
           string memory challengeValue = JsmnSolLib.getBytes(clientDataJSON, nextT.start, nextT.end);
           // this should only be set once, otherwise this is an error
           if (validChallenge) {
-            Logger.logString("duplicate challenge, bad json!");
             return false;
           }
           // this is the key part to ensure the signature is for the provided transaction
           bytes memory challengeDataArray = Base64.decode(challengeValue);
           if (challengeDataArray.length != 32) {
             // wrong hash size
-            Logger.logString("invalid hash data length in json challenge field");
             return false;
           }
           bytes32 challengeData = abi.decode(challengeDataArray, (bytes32));
 
           validChallenge = challengeData == transactionHash;
-          Logger.logString("validChallenge");
-          Logger.logBool(validChallenge);
         } else if (Strings.equal(keyOrValue, "type")) {
           JsmnSolLib.Token memory nextT = tokens[index + 1];
           string memory typeValue = JsmnSolLib.getBytes(clientDataJSON, nextT.start, nextT.end);
           // this should only be set once, otherwise this is an error
           if (validType) {
-            Logger.logString("duplicate type field, bad json");
             return false;
           }
           validType = Strings.equal("webauthn.get", typeValue);
-          Logger.logString("valid type");
-          Logger.logBool(validType);
         } else if (Strings.equal(keyOrValue, "origin")) {
           JsmnSolLib.Token memory nextT = tokens[index + 1];
           string memory originValue = JsmnSolLib.getBytes(clientDataJSON, nextT.start, nextT.end);
           // this should only be set once, otherwise this is an error
           if (validOrigin) {
-            Logger.logString("duplicate origin field, bad json");
             return false;
           }
           pubKey[0] = lowerKeyHalf[originValue][msg.sender];
@@ -121,7 +103,6 @@ contract WebAuthValidator is PasskeyValidator, IModuleValidator {
     }
 
     if (!validChallenge || !validType) {
-      Logger.logString("invalid challenge or type");
       return false;
     }