Verify message fix (starknet-io#1128)

* fix: verification signature with new braavos account * refactor: change ContractSpecifies name * fix: change to getContractVersion name * fix: change method name in test * fix: implement most of Penovicq comments * fix: handling when cairoversion is known and signatureveriffunctionname is unknown * fix: verifyMessageBraavos PR polution fix * chore: cleanup * chore: cleanup unrelated to the pr but next-version --------- Co-authored-by: PhilippeR26 <philippe.rostan@yahoo.fr>
0xknwn · May 17, 2024 · cfcf93b · cfcf93b
1 parent f1c3b8e
commit cfcf93b
Show file tree

Hide file tree

Showing 3 changed files with 91 additions and 34 deletions.
diff --git a/src/account/default.ts b/src/account/default.ts
@@ -111,7 +111,7 @@ export class Account extends Provider implements AccountInterface {
   }
 
   /**
-   * Retrieves the Cairo version from the network and sets `cairoVersion` if not already set in the constructor
+   * Retrieves the Cairo version from the network and sets `cairoVersion` if not already set in the constructor.
    * @param classHash if provided detects Cairo version from classHash, otherwise from the account address
    */
   public async getCairoVersion(classHash?: string) {
@@ -541,38 +541,90 @@ export class Account extends Provider implements AccountInterface {
     return getMessageHash(typedData, this.address);
   }
 
-  public async verifyMessageHash(hash: BigNumberish, signature: Signature): Promise<boolean> {
-    try {
-      const resp = await this.callContract({
-        contractAddress: this.address,
-        entrypoint: 'isValidSignature',
-        calldata: CallData.compile({
-          hash: toBigInt(hash).toString(),
-          signature: formatSignature(signature),
-        }),
-      });
-      if (BigInt(resp[0]) === 0n) {
-        // OpenZeppelin 0.8.0 invalid signature
-        return false;
-      }
-      // OpenZeppelin 0.8.0, ArgentX 0.3.0 & Braavos Cairo 0 valid signature
-      return true;
-    } catch (err) {
-      if (
-        ['argent/invalid-signature', 'is invalid, with respect to the public key'].some(
-          (errMessage) => (err as Error).message.includes(errMessage)
-        )
-      ) {
-        // ArgentX 0.3.0 invalid signature, Braavos Cairo 0 invalid signature
-        return false;
+  public async verifyMessageHash(
+    hash: BigNumberish,
+    signature: Signature,
+    signatureVerificationFunctionName?: string,
+    signatureVerificationResponse?: { okResponse: string[]; nokResponse: string[]; error: string[] }
+  ): Promise<boolean> {
+    // HOTFIX: Accounts should conform to SNIP-6
+    // (https://github.com/starknet-io/SNIPs/blob/f6998f779ee2157d5e1dea36042b08062093b3c5/SNIPS/snip-6.md?plain=1#L61),
+    // but they don't always conform. Also, the SNIP doesn't standardize the response if the signature isn't valid.
+    const knownSigVerificationFName = signatureVerificationFunctionName
+      ? [signatureVerificationFunctionName]
+      : ['isValidSignature', 'is_valid_signature'];
+    const knownSignatureResponse = signatureVerificationResponse || {
+      okResponse: [
+        // any non-nok response is true
+      ],
+      nokResponse: [
+        '0x0', // Devnet
+        '0x00', // OpenZeppelin 0.7.0 to 0.9.0 invalid signature
+      ],
+      error: [
+        'argent/invalid-signature', // ArgentX 0.3.0 to 0.3.1
+        'is invalid, with respect to the public key', // OpenZeppelin until 0.6.1, Braavos 0.0.11
+        'INVALID_SIG', // Braavos 1.0.0
+      ],
+    };
+    let error: any;
+
+    // eslint-disable-next-line no-restricted-syntax
+    for (const SigVerificationFName of knownSigVerificationFName) {
+      try {
+        // eslint-disable-next-line no-await-in-loop
+        const resp = await this.callContract({
+          contractAddress: this.address,
+          entrypoint: SigVerificationFName,
+          calldata: CallData.compile({
+            hash: toBigInt(hash).toString(),
+            signature: formatSignature(signature),
+          }),
+        });
+        // Response NOK Signature
+        if (knownSignatureResponse.nokResponse.includes(resp[0].toString())) {
+          return false;
+        }
+        // Response OK Signature
+        // Empty okResponse assume all non-nok responses are valid signatures
+        // OpenZeppelin 0.7.0 to 0.9.0, ArgentX 0.3.0 to 0.3.1 & Braavos Cairo 0.0.11 to 1.0.0 valid signature
+        if (
+          knownSignatureResponse.okResponse.length === 0 ||
+          knownSignatureResponse.okResponse.includes(resp[0].toString())
+        ) {
+          return true;
+        }
+        throw Error('signatureVerificationResponse Error: response is not part of known responses');
+      } catch (err) {
+        // Known NOK Errors
+        if (
+          knownSignatureResponse.error.some((errMessage) =>
+            (err as Error).message.includes(errMessage)
+          )
+        ) {
+          return false;
+        }
+        // Unknown Error
+        error = err;
       }
-      throw Error(`Signature verification request is rejected by the network: ${err}`);
     }
+
+    throw Error(`Signature verification Error: ${error}`);
   }
 
-  public async verifyMessage(typedData: TypedData, signature: Signature): Promise<boolean> {
+  public async verifyMessage(
+    typedData: TypedData,
+    signature: Signature,
+    signatureVerificationFunctionName?: string,
+    signatureVerificationResponse?: { okResponse: string[]; nokResponse: string[]; error: string[] }
+  ): Promise<boolean> {
     const hash = await this.hashMessage(typedData);
-    return this.verifyMessageHash(hash, signature);
+    return this.verifyMessageHash(
+      hash,
+      signature,
+      signatureVerificationFunctionName,
+      signatureVerificationResponse
+    );
   }
 
   /*

diff --git a/src/account/interface.ts b/src/account/interface.ts
@@ -368,6 +368,8 @@ export abstract class AccountInterface extends ProviderInterface {
    *
    * @param typedData - JSON object to be verified
    * @param signature - signature of the JSON object
+   * @param signatureVerificationFunctionName - optional account contract verification function name override
+   * @param signatureVerificationResponse - optional response override { okResponse: string[]; nokResponse: string[]; error: string[] }
    * @returns true if the signature is valid, false otherwise
    * @throws {Error} if the JSON object is not a valid JSON or the signature is not a valid signature
    */
@@ -379,6 +381,8 @@ export abstract class AccountInterface extends ProviderInterface {
    *
    * @param hash - hash to be verified
    * @param signature - signature of the hash
+   * @param signatureVerificationFunctionName - optional account contract verification function name override
+   * @param signatureVerificationResponse - optional response override { okResponse: string[]; nokResponse: string[]; error: string[] }
    * @returns true if the signature is valid, false otherwise
    * @throws {Error} if the signature is not a valid signature
    */

diff --git a/src/provider/rpc.ts b/src/provider/rpc.ts
@@ -1,17 +1,17 @@
-import { ProviderInterface } from './interface';
-import { LibraryError } from './errors';
-import { RpcChannel, RPC06, RPC07 } from '../channel';
+import { RPC06, RPC07, RpcChannel } from '../channel';
 import {
   AccountInvocations,
   BigNumberish,
   Block,
   BlockIdentifier,
   BlockTag,
   Call,
+  ContractClassResponse,
   ContractVersion,
   DeclareContractTransaction,
   DeployAccountContractTransaction,
   GetBlockResponse,
+  GetTxReceiptResponseWithoutHelper,
   Invocation,
   InvocationsDetailsWithNonce,
   PendingBlock,
@@ -25,12 +25,13 @@ import {
   getEstimateFeeBulkOptions,
   getSimulateTransactionOptions,
   waitForTransactionOptions,
-  GetTxReceiptResponseWithoutHelper,
 } from '../types';
 import { getAbiContractVersion } from '../utils/calldata/cairo';
 import { isSierra } from '../utils/contract';
 import { RPCResponseParser } from '../utils/responseParser/rpc';
-import { ReceiptTx, GetTransactionReceiptResponse } from '../utils/transactionReceipt';
+import { GetTransactionReceiptResponse, ReceiptTx } from '../utils/transactionReceipt';
+import { LibraryError } from './errors';
+import { ProviderInterface } from './interface';
 
 export class RpcProvider implements ProviderInterface {
   private responseParser: RPCResponseParser;
@@ -248,7 +249,7 @@ export class RpcProvider implements ProviderInterface {
       compiler = true,
     }: getContractVersionOptions = {}
   ): Promise<ContractVersion> {
-    let contractClass;
+    let contractClass: ContractClassResponse;
     if (contractAddress) {
       contractClass = await this.getClassAt(contractAddress, blockIdentifier);
     } else if (classHash) {