Fix recoverable account (#187)

* fix recoverable account

* try fix test

* try fix test 2

Author: Arvolear

.github/workflows/coverage.yml

@@ -17,6 +17,9 @@ jobs:
   coverage:
     runs-on: ubuntu-latest
     if: ${{ !contains(github.event.head_commit.message, 'chore(release):') }}
+    strategy:
+      matrix:
+        node-version: [22]
     steps:
       - name: Checkout the repository
         uses: actions/checkout@v5

.github/workflows/tests.yml

@@ -20,7 +20,7 @@ jobs:
     if: ${{ github.event_name == 'workflow_dispatch' || !contains(github.event.head_commit.message, 'chore(release):') }}
     strategy:
       matrix:
-        node-version: [24]
+        node-version: [22]
     steps:
       - name: Checkout the repository
         uses: actions/checkout@v5

CHANGELOG.md

@@ -1,5 +1,9 @@
 # Changelog
 
+## [patch]
+
+- Fixed `RecoverableAccount` execution gas estimation revert.
+
 ## [3.3.1]
 
 - Fixed `viaIR` compilation.

contracts/account-abstraction/ARecoverableAccount.sol

@@ -185,6 +185,12 @@ abstract contract ARecoverableAccount is IAccount, Initializable, AAccountRecove
         PackedUserOperation calldata userOp_,
         bytes32 userOpHash_
     ) internal virtual returns (uint256 validationData_) {
+        // For gas estimation bundler sets signature as empty
+        // and we need to avoid reverting in this case
+        if (userOp_.signature.length < 65) {
+            return SIG_VALIDATION_FAILED;
+        }
+
         address recovered_ = ECDSA.recover(userOpHash_, userOp_.signature);
 
         if (recovered_ == address(this) || recovered_ == trustedExecutor()) {

package-lock.json

@@ -64,7 +64,7 @@
     "eslint": "^9.35.0",
     "eslint-plugin-promise": "^7.2.1",
     "ethers": "^6.15.0",
-    "hardhat": "^3.0.15",
+    "hardhat": "3.0.16",
     "husky": "^9.1.7",
     "jiti": "^2.5.1",
     "merkletreejs": "^0.5.2",

package.json

@@ -47,7 +47,7 @@ describe("RecoverableAccount", () => {
     return ethers.Signature.from(signature).serialized;
   }
 
-  function packTwoUint128(a, b) {
+  function packTwoUint128(a: bigint, b: bigint) {
     const maxUint128 = (1n << 128n) - 1n;
 
     if (a > maxUint128 || b > maxUint128) {
@@ -254,7 +254,7 @@ describe("RecoverableAccount", () => {
 
       const transferData = token.interface.encodeFunctionData("transfer", [FIRST.address, transferAmount]);
 
-      calls = [[await token.getAddress(), 0, transferData]];
+      calls = [[await token.getAddress(), 0n, transferData]];
 
       executionData = ethers.AbiCoder.defaultAbiCoder().encode(["tuple(address,uint256,bytes)[]"], [calls]);
 
@@ -515,5 +515,20 @@ describe("RecoverableAccount", () => {
         .to.be.revertedWithCustomError(account, "NotAnEntryPoint")
         .withArgs(DELEGATED.address);
     });
+
+    it("should not revert if signature length is less than 65 bytes", async () => {
+      const userOp = await getUserOp();
+
+      userOp.signature = "0x";
+
+      const userOpHash = await entryPoint.getUserOpHash(userOp);
+
+      await networkHelpers.impersonateAccount(await entryPoint.getAddress());
+      await networkHelpers.setBalance(await entryPoint.getAddress(), wei(1));
+      const entryPointAccount = await ethers.provider.getSigner(await entryPoint.getAddress());
+
+      await expect(account.connect(entryPointAccount).validateUserOp(userOp, userOpHash, 1n)).to.be.eventually
+        .fulfilled;
+    });
   });
 });

test/account-abstraction/RecoverableAccount.test.ts

@@ -31,9 +31,7 @@ describe("ECDSA512", () => {
       "0x43f800fbeaf9238c58af795bcdad04bc49cd850c394d3382953356b023210281757b30e19218a37cbd612086fbc158caa8b4e1acb2ec00837e5d941f342fb3cc";
 
     it("should verify the signature", async () => {
-      // Added very big gasLimit for the coverage to pass
-      expect(await ecdsa512.verifyBrainpoolP512r1WithoutHashing(message, signature, pubKey, { gasLimit: 300000000 })).to
-        .be.true;
+      expect(await ecdsa512.verifyBrainpoolP512r1WithoutHashing(message, signature, pubKey)).to.be.true;
     });
   });
 });

test/libs/crypto/ECDSA512.test.ts

@@ -299,7 +299,7 @@ describe("CartesianMerkleTree", () => {
       const treapleRoot: string = await treaple.getUintRoot();
 
       for (let i = 0; i < keysCount; i++) {
-        const randIndex = Math.floor(Math.random() * (Number(await treaple.getUintNodesCount()) - 1)) + 1;
+        const randIndex = Math.floor(Math.random() * (Number(await treaple.getUintNodesCount()) - 1));
         const proof = await treaple.getUintProof(keys[randIndex], 40);
 
         await verifyCMTProof(proof, treapleRoot, keys[randIndex], true, true);
@@ -624,7 +624,7 @@ describe("CartesianMerkleTree", () => {
       const treapleRoot: string = await treaple.getBytes32Root();
 
       for (let i = 0; i < keysCount; i++) {
-        const randIndex = Math.floor(Math.random() * (Number(await treaple.getBytes32NodesCount()) - 1)) + 1;
+        const randIndex = Math.floor(Math.random() * (Number(await treaple.getBytes32NodesCount()) - 1));
         const proof = await treaple.getBytes32Proof(keys[randIndex], 40);
 
         await verifyCMTProof(proof, treapleRoot, keys[randIndex], true, true);
@@ -733,6 +733,7 @@ describe("CartesianMerkleTree", () => {
             break;
           }
         }
+
         const currentNode = await treaple.getAddressNode(randIndex);
         const currentNodeKey = `0x${currentNode.key.slice(26)}`;
 
@@ -757,7 +758,7 @@ describe("CartesianMerkleTree", () => {
       const treapleRoot: string = await treaple.getAddressRoot();
 
       for (let i = 0; i < keysCount; i++) {
-        const randIndex = Math.floor(Math.random() * (Number(await treaple.getAddressNodesCount()) - 1)) + 1;
+        const randIndex = Math.floor(Math.random() * (Number(await treaple.getAddressNodesCount()) - 1));
         const proof = await treaple.getAddressProof(keys[randIndex], 40);
 
         await verifyCMTProof(proof, treapleRoot, keys[randIndex], true, true);