Merge remote-tracking branch 'origin/jazzandrock/prefix-detection' in…

…to dvush/wallet-connect

contracts/test/unit_tests/zksync_test.ts

@@ -34,7 +34,7 @@ describe("zkSync signature verification unit tests", function() {
         const pubkeyHash = "sync:fefefefefefefefefefefefefefefefefefefefe";
         const nonce = 0x11223344;
         const accountId = 0xdeadba;
-        const signature = await zksync.utils.signChangePubkeyMessage(randomWallet, pubkeyHash, nonce, accountId);
+        const signature = await randomWallet.signMessage(zksync.utils.getChangePubkeyMessage(pubkeyHash, nonce, accountId));
         const {revertReason, result} = await getCallRevertReason(() =>
             testContract.changePubkeySignatureCheck(signature, pubkeyHash.replace("sync:", "0x"), nonce, randomWallet.address, accountId));
         expect(result).eq(true);
@@ -45,7 +45,7 @@ describe("zkSync signature verification unit tests", function() {
         const pubkeyHash = "sync:fefefefefefefefefefefefefefefefefefefefe";
         const nonce = 0x11223344;
         const accountId = 0xdeadba;
-        const signature = await zksync.utils.signChangePubkeyMessage(randomWallet, pubkeyHash, nonce, accountId);
+        const signature = await randomWallet.signMessage(zksync.utils.getChangePubkeyMessage(pubkeyHash, nonce, accountId));
         const {result} = await getCallRevertReason(() =>
             testContract.changePubkeySignatureCheck(signature, pubkeyHash.replace("sync:", "0x"), incorrectNonce, randomWallet.address, accountId));
         expect(result).eq(false);
@@ -56,7 +56,7 @@ describe("zkSync signature verification unit tests", function() {
         const pubkeyHash = "sync:fefefefefefefefefefefefefefefefefefefefe";
         const nonce = 0x11223344;
         const accountId = 0xdeadba;
-        const signature = await zksync.utils.signChangePubkeyMessage(randomWallet, pubkeyHash, nonce, accountId);
+        const signature = await randomWallet.signMessage(zksync.utils.getChangePubkeyMessage(pubkeyHash, nonce, accountId));
         const {result} = await getCallRevertReason(() =>
             testContract.changePubkeySignatureCheck(signature, incorrectPubkeyHash.replace("sync:", "0x"), nonce, randomWallet.address, accountId));
         expect(result).eq(false);
@@ -67,7 +67,7 @@ describe("zkSync signature verification unit tests", function() {
         const pubkeyHash = "sync:fefefefefefefefefefefefefefefefefefefefe";
         const nonce = 0x11223344;
         const accountId = 0xdeadba;
-        const signature = await zksync.utils.signChangePubkeyMessage(randomWallet, pubkeyHash, nonce, accountId);
+        const signature = await randomWallet.signMessage(zksync.utils.getChangePubkeyMessage(pubkeyHash, nonce, accountId));
         const {result} = await getCallRevertReason(() =>
             testContract.changePubkeySignatureCheck(signature, pubkeyHash.replace("sync:", "0x"), nonce, incorrectSignerAddress, accountId));
         expect(result).eq(false);
@@ -78,7 +78,7 @@ describe("zkSync signature verification unit tests", function() {
         const pubkeyHash = "sync:fefefefefefefefefefefefefefefefefefefefe";
         const nonce = 0x11223344;
         const accountId = 0xdeadba;
-        const signature = await zksync.utils.signChangePubkeyMessage(randomWallet, pubkeyHash, nonce, accountId);
+        const signature = await randomWallet.signMessage(zksync.utils.getChangePubkeyMessage(pubkeyHash, nonce, accountId));
         const {result} = await getCallRevertReason(() =>
             testContract.changePubkeySignatureCheck(signature, pubkeyHash.replace("sync:", "0x"), nonce, randomWallet.address, incorrectAccountId));
         expect(result).eq(false);
@@ -645,7 +645,7 @@ describe("zkSync test process next operation", function() {
         const nonce = 0x1234;
         const pubkeyHash = "sync:fefefefefefefefefefefefefefefefefefefefe";
         const accountId = 0x00ffee12;
-        const ethWitness = await zksync.utils.signChangePubkeyMessage(wallet, pubkeyHash, nonce, accountId);
+        const ethWitness = await wallet.signMessage(zksync.utils.getChangePubkeyMessage(pubkeyHash, nonce, accountId))
 
         const committedPriorityRequestsBefore = await zksyncContract.totalCommittedPriorityRequests();
 

js/zksync.js/abi/IEIP1271.json

@@ -0,0 +1,30 @@
+{
+  "interface": [
+    {
+      "constant": true,
+      "inputs": [
+        {
+          "internalType": "bytes",
+          "name": "_data",
+          "type": "bytes"
+        },
+        {
+          "internalType": "bytes",
+          "name": "_signature",
+          "type": "bytes"
+        }
+      ],
+      "name": "isValidSignature",
+      "outputs": [
+        {
+          "internalType": "bytes4",
+          "name": "",
+          "type": "bytes4"
+        }
+      ],
+      "payable": false,
+      "stateMutability": "view",
+      "type": "function"
+    }
+  ]
+}

js/zksync.js/abi/update-abi.sh

@@ -4,3 +4,4 @@ cd `dirname $0`
 
 cat $ZKSYNC_HOME/contracts/build/ZkSync.json | jq '{ interface: .interface}' > SyncMain.json
 cat $ZKSYNC_HOME/contracts/build/Governance.json | jq '{ interface: .interface}' > SyncGov.json
+cat $ZKSYNC_HOME/contracts/build/IEIP1271.json | jq '{ interface: .interface}' > IEIP1271.json

js/zksync.js/src/signer.ts

@@ -11,7 +11,7 @@ import {
     getEthSignatureType
 } from "./utils";
 import BN = require("bn.js");
-import { Address, CloseAccount, PubKeyHash, Transfer, Withdraw } from "./types";
+import { Address, CloseAccount, PubKeyHash, Transfer, Withdraw, EthSignatureType } from "./types";
 
 const MAX_NUMBER_OF_TOKENS = 4096;
 const MAX_NUMBER_OF_ACCOUNTS = 1 << 24;
@@ -123,15 +123,16 @@ export class Signer {
         ethSigner: ethers.Signer
     ): Promise<{
         signer: Signer;
-        ethSignatureType: "EthereumSignature" | "EIP1271Signature";
+        ethSignatureType: EthSignatureType;
     }> {
         const message =
             "Access zkSync account.\n" +
             "\n" +
             "Only sign this message for a trusted client!";
         const signature = await ethSigner.signMessage(message);
         const address = await ethSigner.getAddress();
-        const ethSignatureType = getEthSignatureType(
+        const ethSignatureType = await getEthSignatureType(
+            ethSigner.provider,
             message,
             signature,
             address

js/zksync.js/src/types.ts

@@ -47,6 +47,11 @@ export interface AccountState {
     };
 }
 
+export type EthSignatureType = {
+    type: "EthereumSignature" | "EIP1271Signature";
+    prefixed: boolean;
+};
+
 export interface TxEthSignature {
     type: "EthereumSignature" | "EIP1271Signature";
     signature: string;

js/zksync.js/src/utils.ts

@@ -5,7 +5,8 @@ import {
     TokenAddress,
     TokenLike,
     Tokens,
-    TokenSymbol
+    TokenSymbol,
+    EthSignatureType
 } from "./types";
 import { serializeAccountId, serializeNonce } from "./signer";
 import { BigNumberish, BigNumber } from "ethers/utils";
@@ -21,6 +22,10 @@ export const SYNC_GOV_CONTRACT_INTERFACE = new utils.Interface(
     require("../abi/SyncGov.json").interface
 );
 
+export const IEIP1271_INTERFACE = new utils.Interface(
+    require("../abi/IEIP1271.json").interface
+);
+
 export const MAX_ERC20_APPROVE_AMOUNT =
     "115792089237316195423570985008687907853269984665640564039457584007913129639935"; // 2^256 - 1
 
@@ -355,12 +360,11 @@ export class TokenSet {
     }
 }
 
-export async function signChangePubkeyMessage(
-    signer: ethers.Signer,
+export function getChangePubkeyMessage(
     pubKeyHash: PubKeyHash,
     nonce: number,
     accountId: number
-): Promise<string> {
+): string {
     const msgNonce = serializeNonce(nonce)
         .toString("hex")
         .toLowerCase();
@@ -374,16 +378,56 @@ export async function signChangePubkeyMessage(
         `nonce: 0x${msgNonce}\n` +
         `account id: 0x${msgAccId}\n\n` +
         `Only sign this message for a trusted client!`;
-    return signer.signMessage(message);
+    return message;
+}
+
+export function hashMessagePrefixless(message: ethers.utils.Arrayish | string): string {
+    return ethers.utils.keccak256(ethers.utils.concat([
+        ((typeof(message) === 'string') ? ethers.utils.toUtf8Bytes(message): message)
+    ]));
 }
 
-export function getEthSignatureType(
+export async function getEthSignatureType(
+    provider: ethers.providers.Provider,
     message: string,
     signature: string,
     address: string
-): "EthereumSignature" | "EIP1271Signature" {
-    const recovered = ethers.utils.verifyMessage(message, signature);
-    return recovered.toLowerCase() === address.toLowerCase()
-        ? "EthereumSignature"
-        : "EIP1271Signature";
+): Promise<EthSignatureType> {
+    const prefixedRecovered = ethers.utils.verifyMessage(message, signature);
+    if (prefixedRecovered.toLowerCase() === address.toLowerCase()) {
+        return {
+            type: "EthereumSignature",
+            prefixed: true,
+        };
+    }
+
+    const recovered = ethers.utils.recoverAddress(hashMessagePrefixless(message), signature);
+    if (recovered.toLowerCase() === address.toLowerCase()) {
+        return {
+            type: "EthereumSignature",
+            prefixed: false,
+        };
+    }
+
+    const eip1271 = new ethers.Contract(address, IEIP1271_INTERFACE, provider);
+    const EIP1271_SUCCESS_VALUE = 0x20c13b0b;
+
+    const eipRetVal = await eip1271.isValidSignature(ethers.utils.toUtf8Bytes(message), signature);
+    if (eipRetVal == EIP1271_SUCCESS_VALUE) {
+        return {
+            type: "EIP1271Signature",
+            prefixed: true,
+        };
+    }
+
+    const prefixedMessage = message = `\x19Ethereum Signed Message:\n${message.length}${message}`;
+    const prefixedEipRetVal = await eip1271.isValidSignature(ethers.utils.toUtf8Bytes(prefixedMessage), signature);
+    if (prefixedEipRetVal == EIP1271_SUCCESS_VALUE) {
+        return {
+            type: "EIP1271Signature",
+            prefixed: false,
+        };
+    }
+
+    throw new Error("Unknown Ethereum signature type!");
 }

js/zksync.js/src/wallet.ts

@@ -10,14 +10,15 @@ import {
     TransactionReceipt,
     PubKeyHash,
     TxEthSignature,
-    ChangePubKey
+    ChangePubKey,
+    EthSignatureType
 } from "./types";
 import {
     ERC20_APPROVE_TRESHOLD,
     IERC20_INTERFACE,
     isTokenETH,
     MAX_ERC20_APPROVE_AMOUNT,
-    signChangePubkeyMessage,
+    getChangePubkeyMessage,
     getEthSignatureType,
     SYNC_MAIN_CONTRACT_INTERFACE
 } from "./utils";
@@ -44,7 +45,7 @@ export class Wallet {
         public cachedAddress: Address,
         public signer?: Signer,
         public accountId?: number,
-        public ethSignatureType?: "EthereumSignature" | "EIP1271Signature"
+        public ethSignatureType?: EthSignatureType
     ) {}
 
     connect(provider: Provider) {
@@ -57,7 +58,7 @@ export class Wallet {
         provider: Provider,
         signer?: Signer,
         accountId?: number,
-        ethSignatureType?: "EthereumSignature" | "EIP1271Signature"
+        ethSignatureType?: EthSignatureType
     ): Promise<Wallet> {
         if (signer == undefined) {
             const signerResult = await Signer.fromETHSignature(ethWallet);
@@ -86,7 +87,7 @@ export class Wallet {
         ethWallet: ethers.Signer,
         provider: Provider,
         accountId?: number,
-        ethSignatureType?: "EthereumSignature" | "EIP1271Signature"
+        ethSignatureType?: EthSignatureType
     ): Promise<Wallet> {
         const wallet = new Wallet(
             ethWallet,
@@ -100,18 +101,17 @@ export class Wallet {
     }
 
     async getEthMessageSignature(message: string): Promise<TxEthSignature> {
-        const signature = await this.ethSigner.signMessage(message);
-
         if (this.ethSignatureType == undefined) {
-            const address = await this.ethSigner.getAddress();
-            this.ethSignatureType = getEthSignatureType(
-                message,
-                signature,
-                address
-            );
+            throw new Error("ethSignatureType is unknown.");
+        }
+
+        if (!this.ethSignatureType.prefixed) {
+            message = `\x19Ethereum Signed Message:\n${message.length}${message}`;
         }
 
-        return { type: this.ethSignatureType, signature };
+        const signature = await this.ethSigner.signMessage(message);
+
+        return { type: this.ethSignatureType.type, signature };
     }
 
     async syncTransfer(transfer: {
@@ -282,14 +282,15 @@ export class Wallet {
         await this.setRequiredAccountIdFromServer("Set Signing Key");
 
         const numNonce = await this.getNonce(nonce);
+
+        const changePubKeyMessage = getChangePubkeyMessage(
+            newPubKeyHash,
+            numNonce,
+            this.accountId,
+        );
         const ethSignature = onchainAuth
             ? null
-            : await signChangePubkeyMessage(
-                  this.ethSigner,
-                  newPubKeyHash,
-                  numNonce,
-                  this.accountId
-              );
+            : (await this.getEthMessageSignature(changePubKeyMessage)).signature;
 
         const txData: ChangePubKey = {
             type: "ChangePubKey",