Cleanup Post-Review

modules/4337/contracts/Safe4337Module.sol

@@ -215,8 +215,8 @@ contract Safe4337Module is IAccount, HandlerContext, CompatibilityFallbackHandle
      * @dev Checks if the signatures length is correct and does not contain additional bytes. The function does not
      * check the integrity of the signature encoding, as this is expected to be checked by the {Safe} implementation
      * of {checkSignatures}.
-     * @param signatures signatures data
-     * @param threshold Indicates the number of iterations to perform in the loop.
+     * @param signatures Signatures data.
+     * @param threshold Signer threshold for the Safe account.
      * @return isValid True if length check passes, false otherwise.
      */
     function _checkSignaturesLength(bytes calldata signatures, uint256 threshold) internal pure returns (bool isValid) {
@@ -271,15 +271,16 @@ contract Safe4337Module is IAccount, HandlerContext, CompatibilityFallbackHandle
         (bytes memory operationData, uint48 validAfter, uint48 validUntil, bytes calldata signatures) = _getSafeOp(userOp);
 
         // The `checkSignatures` function in the Safe contract does not force a fixed size on signature length.
-        // A malicious bundler can pad the Safe operation `signatures` with additional bytes, causing the account to pay more gas than needed for user operation validation (capped by `verificationGasLimit`).
+        // A malicious bundler can pad the Safe operation `signatures` with additional bytes, causing the account to pay
+        // more gas than needed for user operation validation (capped by `verificationGasLimit`).
         // `_checkSignaturesLength` ensures that there are no additional bytes in the `signature` than are required.
         bool validSignature = _checkSignaturesLength(signatures, ISafe(payable(userOp.sender)).getThreshold());
 
         try ISafe(payable(userOp.sender)).checkSignatures(keccak256(operationData), operationData, signatures) {} catch {
             validSignature = false;
         }
 
-        // The timestamps are validated by the entry point, therefore we will not check them again
+        // The timestamps are validated by the entry point, therefore we will not check them again.
         validationData = _packValidationData(!validSignature, validUntil, validAfter);
     }
 

modules/4337/test/erc4337/ERC4337ModuleNew.spec.ts

@@ -208,7 +208,7 @@ describe('Safe4337Module - Newly deployed safe', () => {
         .withArgs(0, 'AA24 signature error')
     })
 
-    it('should revert when signature pointer points to invalid part of signature data - Smart contract signature', async () => {
+    it('should revert when signature offset points to invalid part of signature data - Smart contract signature', async () => {
       const { user1, relayer, safe: parentSafe, validator, entryPoint, safeGlobalConfig } = await setupTests()
 
       await parentSafe.deploy(user1)

modules/4337/test/erc4337/ReferenceEntryPoint.spec.ts

@@ -301,7 +301,7 @@ describe('Safe4337Module - Reference EntryPoint', () => {
       .withArgs(0, 'AA24 signature error')
   })
 
-  it('should revert when signature pointer points to invalid part of signature data - Smart contract signature', async () => {
+  it('should revert when signature offset points to invalid part of signature data - Smart contract signature', async () => {
     const { user, relayer, safe: parentSafe, validator, entryPoint, safeGlobalConfig } = await setupTests()
 
     await parentSafe.deploy(user)

modules/4337/test/erc4337/Safe4337Module.spec.ts

@@ -359,7 +359,11 @@ describe('Safe4337Module', () => {
 
       const userOp = buildPackedUserOperationFromSafeUserOperation({
         safeOp,
-        signature: ethers.hexlify(ethers.randomBytes(32)) + '00'.padStart(64, '0') + '00' + ethers.hexlify(ethers.randomBytes(65)).slice(2),
+        signature: ethers.concat([
+          ethers.randomBytes(32),
+          ethers.toBeHex(0, 32), // point to start of the signatures bytes
+          '0x00', // contract signature type
+        ]),
       })
       const packedValidationData = packValidationData(1, validUntil, validAfter)
       const entryPointImpersonator = await ethers.getSigner(await entryPoint.getAddress())