fix: more fixes

matter-labs · Dec 11, 2024 · 98e975b · 98e975b
1 parent d9f320d
commit 98e975b
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 15 deletions.
diff --git a/src/SsoAccount.sol b/src/SsoAccount.sol
@@ -165,20 +165,23 @@ contract SsoAccount is Initializable, HookManager, ERC1271Handler, TokenCallback
   /// @param _transaction The transaction data.
   /// @return The magic value if the validation was successful and bytes4(0) otherwise.
   function _validateTransaction(bytes32 _signedHash, Transaction calldata _transaction) internal returns (bytes4) {
-    if (_transaction.signature.length == 65) {
-      (address signer, ) = ECDSA.tryRecover(_signedHash, _transaction.signature);
-      return _k1IsOwner(signer) ? ACCOUNT_VALIDATION_SUCCESS_MAGIC : bytes4(0);
-    }
-
-    // Extract the signature, validator address and hook data from the _transaction.signature
-    (bytes memory signature, address validator, ) = SignatureDecoder.decodeSignature(_transaction.signature);
-
     // Run validation hooks
     bool hookSuccess = runValidationHooks(_signedHash, _transaction);
     if (!hookSuccess) {
       return bytes4(0);
     }
 
+    if (_transaction.signature.length == 65) {
+      (address signer, ECDSA.RecoverError error) = ECDSA.tryRecover(_signedHash, _transaction.signature);
+      return
+        signer == address(0) || error != ECDSA.RecoverError.NoError || !_k1IsOwner(signer)
+          ? bytes4(0)
+          : ACCOUNT_VALIDATION_SUCCESS_MAGIC;
+    }
+
+    // Extract the signature, validator address and hook data from the _transaction.signature
+    (bytes memory signature, address validator, ) = SignatureDecoder.decodeSignature(_transaction.signature);
+
     bool validationSuccess = _handleValidation(validator, _signedHash, signature, _transaction);
     return validationSuccess ? ACCOUNT_VALIDATION_SUCCESS_MAGIC : bytes4(0);
   }

diff --git a/test/SessionKeyTest.ts b/test/SessionKeyTest.ts
@@ -250,7 +250,7 @@ class SessionTester {
 
   async sendTxSuccess(txRequest: ethers.TransactionLike = {}) {
     this.aaTransaction = {
-      ...await this.aaTxTemplate(),
+      ...await this.aaTxTemplate(true),
       ...txRequest,
     };
     const estimatedGas = await provider.estimateGas(this.aaTransaction);
@@ -265,7 +265,7 @@ class SessionTester {
 
   async sendTxFail(tx: ethers.TransactionLike = {}) {
     this.aaTransaction = {
-      ...await this.aaTxTemplate(),
+      ...await this.aaTxTemplate(true),
       gasLimit: 100_000_000n,
       ...tx,
     };
@@ -320,7 +320,7 @@ class SessionTester {
     };
   }
 
-  async aaTxTemplate() {
+  async aaTxTemplate(forSession: boolean = false) {
     const numberOfConstraints = this.session.callPolicies.map((policy) => policy.constraints.length);
     return {
       type: 113,
@@ -332,7 +332,7 @@ class SessionTester {
       gasPrice: await provider.getGasPrice(),
       customData: {
         gasPerPubdata: utils.DEFAULT_GAS_PER_PUBDATA_LIMIT,
-        customSignature: abiCoder.encode(
+        customSignature: forSession ? abiCoder.encode(
           ["bytes", "address", "bytes"],
           [
             ethers.zeroPadValue("0x1b", 65),
@@ -342,7 +342,7 @@ class SessionTester {
               [this.session, new Array(2 + Math.max(0, ...numberOfConstraints)).fill(0)],
             ),
           ],
-        ),
+        ) : undefined,
       },
       gasLimit: 0n,
     };
@@ -601,7 +601,5 @@ describe("SessionKeyModule tests", function () {
   });
 
   // TODO: module uninstall tests
-  // TODO: session expiresAt tests
   // TODO: session fee limit tests
-  // TODO: allowance tests
 });