Unify to prefix messages for signing

packages/contractkit/src/test-utils/ganache.setup.ts

@@ -3,7 +3,30 @@ import * as ganache from '@celo/ganache-cli'
 import * as path from 'path'
 
 const MNEMONIC = 'concert load couple harbor equip island argue ramp clarify fence smart topic'
-
+export const ACCOUNT_PRIVATE_KEYS = [
+  '0xf2f48ee19680706196e2e339e5da3491186e0c4c5030670656b0e0164837257d',
+  '0x5d862464fe9303452126c8bc94274b8c5f9874cbd219789b3eb2128075a76f72',
+  '0xdf02719c4df8b9b8ac7f551fcb5d9ef48fa27eef7a66453879f4d8fdc6e78fb1',
+  '0xff12e391b79415e941a94de3bf3a9aee577aed0731e297d5cfa0b8a1e02fa1d0',
+  '0x752dd9cf65e68cfaba7d60225cbdbc1f4729dd5e5507def72815ed0d8abc6249',
+  '0xefb595a0178eb79a8df953f87c5148402a224cdf725e88c0146727c6aceadccd',
+  '0x83c6d2cc5ddcf9711a6d59b417dc20eb48afd58d45290099e5987e3d768f328f',
+  '0xbb2d3f7c9583780a7d3904a2f55d792707c345f21de1bacb2d389934d82796b2',
+  '0xb2fd4d29c1390b71b8795ae81196bfd60293adf99f9d32a0aff06288fcdac55f',
+  '0x23cb7121166b9a2f93ae0b7c05bde02eae50d64449b2cbb42bc84e9d38d6cc89',
+]
+export const ACCOUNT_ADDRESSES = [
+  '0x5409ED021D9299bf6814279A6A1411A7e866A631',
+  '0x6Ecbe1DB9EF729CBe972C83Fb886247691Fb6beb',
+  '0xE36Ea790bc9d7AB70C55260C66D52b1eca985f84',
+  '0xE834EC434DABA538cd1b9Fe1582052B880BD7e63',
+  '0x78dc5D2D739606d31509C31d654056A45185ECb6',
+  '0xA8dDa8d7F5310E4A9E24F8eBA77E091Ac264f872',
+  '0x06cEf8E666768cC40Cc78CF93d9611019dDcB628',
+  '0x4404ac8bd8F9618D27Ad2f1485AA1B2cFD82482D',
+  '0x7457d5E02197480Db681D3fdF256c7acA21bDc12',
+  '0x91c987bf62D25945dB517BDAa840A6c661374402',
+]
 export async function startGanache(datadir: string, opts: { verbose?: boolean } = {}) {
   const logFn = opts.verbose
     ? // tslint:disable-next-line: no-console

packages/contractkit/src/utils/signing.test.ts

@@ -0,0 +1,35 @@
+import { LocalSigner, NativeSigner, parseSignature } from '@celo/utils/lib/signatureUtils'
+import { testWithGanache } from '../test-utils/ganache-test'
+import { ACCOUNT_ADDRESSES, ACCOUNT_PRIVATE_KEYS } from '../test-utils/ganache.setup'
+
+// This only really tests signatureUtils in @celo/utils, but is tested here
+// to avoid the web3/ganache setup in @celo/utils
+testWithGanache('Signing', (web3) => {
+  const account = ACCOUNT_ADDRESSES[0]
+  const pKey = ACCOUNT_PRIVATE_KEYS[0]
+
+  const nativeSigner = NativeSigner(web3.eth.sign, account)
+  const localSigner = LocalSigner(pKey)
+
+  it('signs a message the same way via RPC and with an explicit private key', async () => {
+    const message = 'message'
+    const nativeSignature = await nativeSigner.sign(message)
+    const localSignature = await localSigner.sign(message)
+
+    expect(parseSignature(message, nativeSignature, account)).toEqual(
+      parseSignature(message, localSignature, account)
+    )
+  })
+
+  it('signs a message that was hashed the same way via RPC and with an explicit private key', async () => {
+    // This test checks that the prefixing in `signMessage` appropriately considers hex strings
+    // as bytes the same way the native RPC signing would
+    const message = web3.utils.soliditySha3('message')
+    const nativeSignature = await nativeSigner.sign(message)
+    const localSignature = await localSigner.sign(message)
+
+    expect(parseSignature(message, nativeSignature, account)).toEqual(
+      parseSignature(message, localSignature, account)
+    )
+  })
+})

packages/protocol/contracts/common/Signatures.sol

@@ -2,13 +2,13 @@ pragma solidity ^0.5.3;
 
 
 library Signatures {
-/**
-* @notice Given a signed address, returns the signer of the address.
-* @param message The address that was signed.
-* @param v The recovery id of the incoming ECDSA signature.
-* @param r Output value r of the ECDSA signature.
-* @param s Output value s of the ECDSA signature.
-*/
+  /**
+  * @notice Given a signed address, returns the signer of the address.
+  * @param message The address that was signed.
+  * @param v The recovery id of the incoming ECDSA signature.
+  * @param r Output value r of the ECDSA signature.
+  * @param s Output value s of the ECDSA signature.
+  */
   function getSignerOfAddress(
     address message,
     uint8 v,
@@ -20,8 +20,28 @@ library Signatures {
     returns (address)
   {
     bytes32 hash = keccak256(abi.encodePacked(message));
+    return getSignerOfMessageHash(hash, v, r, s);
+  }
+
+  /**
+  * @notice Given a signed address, returns the signer of the address.
+  * @param messageHash The hash of a message.
+  * @param v The recovery id of the incoming ECDSA signature.
+  * @param r Output value r of the ECDSA signature.
+  * @param s Output value s of the ECDSA signature.
+  */
+  function getSignerOfMessageHash(
+    bytes32 messageHash,
+    uint8 v,
+    bytes32 r,
+    bytes32 s
+  )
+    public
+    pure
+    returns (address)
+  {
     bytes memory prefix = "\x19Ethereum Signed Message:\n32";
-    bytes32 prefixedHash = keccak256(abi.encodePacked(prefix, hash));
+    bytes32 prefixedHash = keccak256(abi.encodePacked(prefix, messageHash));
     return ecrecover(prefixedHash, v, r, s);
   }
 }

packages/protocol/contracts/identity/Attestations.sol

@@ -679,7 +679,7 @@ contract Attestations is
     returns (address)
   {
     bytes32 codehash = keccak256(abi.encodePacked(identifier, account));
-    address signer = ecrecover(codehash, v, r, s);
+    address signer = Signatures.getSignerOfMessageHash(codehash, v, r, s);
     address issuer = getAccountFromAttestor(signer);
 
     Attestation storage attestation =

packages/protocol/lib/signing-utils.ts

@@ -1,5 +1,6 @@
 // Originally taken from https://github.com/ethereum/web3.js/blob/1.x/packages/web3-eth-accounts/src/index.js
 
+import { parseSignature } from "@celo/utils/lib/signatureUtils"
 import { bytes, hash, nat, RLP } from 'eth-lib'
 import * as _ from 'underscore'
 import * as helpers from 'web3-core-helpers'
@@ -28,12 +29,8 @@ function makeEven(hex: string) {
 
 export const getParsedSignatureOfAddress = async (web3: Web3, address: string, signer: string) => {
   const addressHash = web3.utils.soliditySha3({ type: 'address', value: address })
-  const signature = (await web3.eth.sign(addressHash, signer)).slice(2)
-  return {
-    r: `0x${signature.slice(0, 64)}`,
-    s: `0x${signature.slice(64, 128)}`,
-    v: web3.utils.hexToNumber(signature.slice(128, 130)) + 27,
-  }
+  const signature = await web3.eth.sign(addressHash, signer)
+  return parseSignature(addressHash, signature, signer)
 }
 
 export async function signTransaction(web3: Web3, txn: any, privateKey: string) {

packages/protocol/test/identity/attestations.ts

@@ -13,18 +13,18 @@ import { getPhoneHash } from '@celo/utils/lib/phoneNumbers'
 import BigNumber from 'bignumber.js'
 import { uniq } from 'lodash'
 import {
-  TestAttestationsContract,
-  TestAttestationsInstance,
+  MockElectionContract,
+  MockElectionInstance,
   MockLockedGoldContract,
   MockLockedGoldInstance,
   MockStableTokenContract,
   MockStableTokenInstance,
-  MockElectionInstance,
-  TestRandomContract,
-  TestRandomInstance,
-  MockElectionContract,
   RegistryContract,
   RegistryInstance,
+  TestAttestationsContract,
+  TestAttestationsInstance,
+  TestRandomContract,
+  TestRandomInstance,
 } from 'types'
 import { getParsedSignatureOfAddress } from '../../lib/signing-utils'
 

packages/protocol/test/identity/escrow.ts

@@ -10,6 +10,7 @@ import {
   RegistryContract,
   RegistryInstance,
 } from 'types'
+import { getParsedSignatureOfAddress } from '../../lib/signing-utils'
 
 // For reference:
 //    accounts[0] = owner
@@ -105,17 +106,6 @@ contract('Escrow', (accounts: string[]) => {
     const receiver: string = accounts[2]
     const sender: string = accounts[1]
 
-    const getParsedSignatureOfAddress = async (address: string, signer: string) => {
-      // @ts-ignore
-      const hash = web3.utils.soliditySha3({ type: 'address', value: address })
-      const signature = (await web3.eth.sign(hash, signer)).slice(2)
-      return {
-        r: `0x${signature.slice(0, 64)}`,
-        s: `0x${signature.slice(64, 128)}`,
-        v: web3.utils.hexToNumber(signature.slice(128, 130)) + 27,
-      }
-    }
-
     // @ts-ignore
     const aPhoneHash: string = web3.utils.soliditySha3({ t: 'string', v: '+18005555555' })
     const withdrawKeyAddress: string = accounts[5]
@@ -255,8 +245,8 @@ contract('Escrow', (accounts: string[]) => {
       let parsedSig2: any
 
       beforeEach(async () => {
-        parsedSig1 = await getParsedSignatureOfAddress(accounts[2], accounts[5])
-        parsedSig2 = await getParsedSignatureOfAddress(accounts[2], accounts[6])
+        parsedSig1 = await getParsedSignatureOfAddress(web3, accounts[2], accounts[5])
+        parsedSig2 = await getParsedSignatureOfAddress(web3, accounts[2], accounts[6])
         await doubleTransfer()
         uniquePaymentIDWithdraw = withdrawKeyAddress
       })
@@ -384,7 +374,7 @@ contract('Escrow', (accounts: string[]) => {
       beforeEach(async () => {
         await doubleTransfer()
         uniquePaymentIDRevoke = withdrawKeyAddress
-        parsedSig1 = await getParsedSignatureOfAddress(accounts[2], accounts[5])
+        parsedSig1 = await getParsedSignatureOfAddress(web3, accounts[2], accounts[5])
       })
 
       it('should allow sender to redeem payment after payment has expired', async () => {

packages/utils/src/signatureUtils.test.ts

@@ -1,12 +1,27 @@
 import * as Web3Utils from 'web3-utils'
 import { privateKeyToAddress } from './address'
-import { parseSignature, serializeSignature, signMessage } from './signatureUtils'
+import {
+  parseSignature,
+  parseSignatureWithoutPrefix,
+  serializeSignature,
+  signMessage,
+  signMessageWithoutPrefix,
+} from './signatureUtils'
 
 describe('signatures', () => {
-  it('should sign appropriately', () => {
+  it('should sign appropriately with a hash of a message', () => {
     const pKey = '0x62633f7c9583780a7d3904a2f55d792707c345f21de1bacb2d389934d82796b2'
     const address = privateKeyToAddress(pKey)
-    const message = Web3Utils.soliditySha3({ type: 'string', value: 'identifier' })
+    const messageHash = Web3Utils.soliditySha3({ type: 'string', value: 'identifier' })
+    const signature = signMessageWithoutPrefix(messageHash, pKey, address)
+    const serializedSig = serializeSignature(signature)
+    parseSignatureWithoutPrefix(messageHash, serializedSig, address)
+  })
+
+  it('should sign appropriately with just the message', () => {
+    const pKey = '0x62633f7c9583780a7d3904a2f55d792707c345f21de1bacb2d389934d82796b2'
+    const address = privateKeyToAddress(pKey)
+    const message = 'mymessage'
     const signature = signMessage(message, pKey, address)
     const serializedSig = serializeSignature(signature)
     parseSignature(message, serializedSig, address)

packages/utils/src/signatureUtils.ts

@@ -1,6 +1,55 @@
 const ethjsutil = require('ethereumjs-util')
 
-export function signMessage(messageHash: string, privateKey: string, address: string) {
+import * as Web3Utils from 'web3-utils'
+import { privateKeyToAddress } from './address'
+
+// If messages is a hex, the length of it should be the number of bytes
+function messageLength(message: string) {
+  if (Web3Utils.isHexStrict(message)) {
+    return (message.length - 2) / 2
+  }
+  return message.length
+}
+// Ethereum has a special signature format that requires a prefix
+// https://github.com/ethereum/wiki/wiki/JSON-RPC#eth_sign
+export function hashMessageWithPrefix(message: string) {
+  const prefix = '\x19Ethereum Signed Message:\n' + messageLength(message)
+  return Web3Utils.soliditySha3(prefix, message)
+}
+
+export function hashMessage(message: string): string {
+  return Web3Utils.soliditySha3({ type: 'string', value: message })
+}
+
+export interface Signer {
+  sign: (message: string) => Promise<string>
+}
+
+// Uses a native function to sign (as signFn), most commonly `web.eth.sign`
+export function NativeSigner(
+  signFn: (message: string, signer: string) => Promise<string>,
+  signer: string
+): Signer {
+  return {
+    sign: async (message: string) => {
+      return signFn(message, signer)
+    },
+  }
+}
+export function LocalSigner(privateKey: string): Signer {
+  return {
+    sign: async (message: string) =>
+      Promise.resolve(
+        serializeSignature(signMessage(message, privateKey, privateKeyToAddress(privateKey)))
+      ),
+  }
+}
+
+export function signMessage(message: string, privateKey: string, address: string) {
+  return signMessageWithoutPrefix(hashMessageWithPrefix(message), privateKey, address)
+}
+
+export function signMessageWithoutPrefix(messageHash: string, privateKey: string, address: string) {
   const publicKey = ethjsutil.privateToPublic(ethjsutil.toBuffer(privateKey))
   const derivedAddress: string = ethjsutil.bufferToHex(ethjsutil.pubToAddress(publicKey))
   if (derivedAddress.toLowerCase() !== address.toLowerCase()) {
@@ -31,7 +80,15 @@ export function serializeSignature(signature: Signature) {
   return '0x' + serializedV + serializedR + serializedS
 }
 
-export function parseSignature(messageHash: string, signature: string, signer: string) {
+export function parseSignature(message: string, signature: string, signer: string) {
+  return parseSignatureWithoutPrefix(hashMessageWithPrefix(message), signature, signer)
+}
+
+export function parseSignatureWithoutPrefix(
+  messageHash: string,
+  signature: string,
+  signer: string
+) {
   let { r, s, v } = parseSignatureAsRsv(signature.slice(2))
   if (isValidSignature(signer, messageHash, v, r, s)) {
     return { v, r, s }
@@ -117,8 +174,12 @@ export function areAddressesEqual(address1: string | null, address2: string | nu
 }
 
 export const SignatureUtils = {
+  NativeSigner,
+  LocalSigner,
   signMessage,
+  signMessageWithoutPrefix,
   parseSignature,
+  parseSignatureWithoutPrefix,
   stripHexLeader,
   ensureHexLeader,
   serializeSignature,