Skip to content

Commit

Permalink
upgrade eth-sig-util (#1039)
Browse files Browse the repository at this point in the history
Co-authored-by: goosewobbler <goosewobbler@pm.me>
  • Loading branch information
goosewobbler and goosewobbler committed Nov 29, 2022
1 parent c7cf3b7 commit fd343dc
Show file tree
Hide file tree
Showing 20 changed files with 156 additions and 164 deletions.
12 changes: 12 additions & 0 deletions @types/frame/rpc.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,18 @@ interface EVMError {
type RPCRequestPayload = JSONRPCRequestPayload & InternalPayload

declare namespace RPC {

namespace SignTypedData {
interface Request extends Omit<RPCRequestPayload, ['method', 'params']> {
method: 'eth_signTypedData' | 'eth_signTypedData_v1' | 'eth_signTypedData_v3' | 'eth_signTypedData_v4',
params: [string, LegacyTypedData | TypedData | string, ...unknown[]]
}

interface Response extends Omit<RPCResponsePayload, 'result'> {
result?: string
}
}

namespace GetAssets {
interface Balance {
chainId: number,
Expand Down
12 changes: 6 additions & 6 deletions main/accounts/Account/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import log from 'electron-log'
import { isValidAddress, addHexPrefix } from 'ethereumjs-util'
import { Version } from 'eth-sig-util'

import { AccessRequest, AccountRequest, Accounts, RequestMode, TransactionRequest } from '..'
import nebulaApi from '../../nebula'
Expand All @@ -16,6 +15,7 @@ import provider from '../../provider'
import { ApprovalType } from '../../../resources/constants'

import reveal from '../../reveal'
import type { TypedMessage } from '../types'

const nebula = nebulaApi()

Expand Down Expand Up @@ -510,23 +510,23 @@ class FrameAccount {
}
}

signTypedData (version: Version, typedData: any, cb: Callback<string>) {
if (!typedData) return cb(new Error('No data to sign'))
if (typeof (typedData) !== 'object') return cb(new Error('Data to sign has the wrong format'))
signTypedData (typedMessage: TypedMessage, cb: Callback<string>) {
if (!typedMessage.data) return cb(new Error('No data to sign'))
if (typeof (typedMessage.data) !== 'object') return cb(new Error('Data to sign has the wrong format'))
if (this.signer) {
const s = signers.get(this.signer)
if (!s) return cb(new Error(`Cannot find signer for this account`))
const index = s.addresses.map(a => a.toLowerCase()).indexOf(this.address)
if (index === -1) cb(new Error(`Signer cannot sign for this address`))
s.signTypedData(index, version, typedData, cb)
s.signTypedData(index, typedMessage, cb)
} else if (this.smart && this.smart.actor) {
const actingAccount = this.accounts.get(this.smart.actor)
if (!actingAccount) return cb(new Error(`Could not find acting account: ${this.smart.actor}`))
const actingSigner = signers.get(actingAccount.signer)
if (!actingSigner || !actingSigner.verifyAddress) return cb(new Error(`Could not find acting account signer: ${actingAccount.signer}`))
const index = actingSigner.addresses.map(a => a.toLowerCase()).indexOf(actingAccount.address)
if (index === -1) cb(new Error(`Acting signer cannot sign for this address, could not find acting address in signer: ${actingAccount.address}`))
actingSigner.signTypedData(index, version, typedData, cb)
actingSigner.signTypedData(index, typedMessage, cb)
} else {
cb(new Error('No signer found for this account'))
}
Expand Down
7 changes: 3 additions & 4 deletions main/accounts/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import EventEmitter from 'events'
import log from 'electron-log'
import { Notification } from 'electron'
import { addHexPrefix, intToHex} from 'ethereumjs-util'
import { TypedData, Version } from 'eth-sig-util'
import { v5 as uuidv5 } from 'uuid'

import provider from '../provider'
Expand All @@ -20,7 +19,7 @@ import { findUnavailableSigners, getSignerType, isSignerReady } from '../../reso
import {
AccountRequest, AccessRequest,
TransactionRequest, TransactionReceipt,
ReplacementType, RequestStatus, RequestMode
ReplacementType, RequestStatus, RequestMode, TypedMessage
} from './types'

import type { Chain } from '../chains'
Expand Down Expand Up @@ -555,13 +554,13 @@ export class Accounts extends EventEmitter {
currentAccount.signMessage(message, cb)
}

signTypedData (version: Version, address: Address, typedData: TypedData, cb: Callback<string>) {
signTypedData (address: Address, typedMessage: TypedMessage, cb: Callback<string>) {
const currentAccount = this.current()

if (!currentAccount) return cb(new Error('No Account Selected'))
if (address.toLowerCase() !== currentAccount.getSelectedAddress().toLowerCase()) return cb(new Error('signMessage: Wrong Account Selected'))

currentAccount.signTypedData(version, typedData, cb)
currentAccount.signTypedData(typedMessage, cb)
}

signTransaction (rawTx: TransactionData, cb: Callback<string>) {
Expand Down
12 changes: 10 additions & 2 deletions main/accounts/types.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { Version } from 'eth-sig-util'
import type { MessageTypes, SignTypedDataVersion, TypedDataV1, TypedMessage as BaseTypedMessage } from '@metamask/eth-sig-util'
import type { DecodedCallData } from '../contracts'
import type { Chain } from '../chains'
import type { TransactionData } from '../../resources/domain/transaction'
Expand Down Expand Up @@ -77,9 +77,17 @@ export interface TransactionRequest extends Omit<AccountRequest, 'type'> {
recognizedActions: Action<unknown>[]
}

export type TypedData<T extends MessageTypes = MessageTypes> = BaseTypedMessage<T>
export type LegacyTypedData = TypedDataV1

export interface TypedMessage<V extends SignTypedDataVersion = SignTypedDataVersion> {
data: V extends SignTypedDataVersion.V1 ? LegacyTypedData : TypedData
version: V
}

export interface SignTypedDataRequest extends Omit<AccountRequest, 'type'> {
type: 'signTypedData',
version: Version
typedMessage: TypedMessage
}

export interface AccessRequest extends Omit<AccountRequest, 'type'> {
Expand Down
44 changes: 25 additions & 19 deletions main/provider/index.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { v4 as uuid } from 'uuid'
import EventEmitter from 'events'
import log from 'electron-log'
import {isAddress} from '@ethersproject/address'
import { recoverTypedMessage, Version } from 'eth-sig-util'
import { recoverTypedSignature, SignTypedDataVersion } from '@metamask/eth-sig-util'
import { isAddress } from '@ethersproject/address'
import crypto from 'crypto'
import {
addHexPrefix,
Expand Down Expand Up @@ -46,6 +46,7 @@ import {
createOriginChainObserver as OriginChainObserver,
getActiveChains
} from './chains'
import type { LegacyTypedData, TypedData, TypedMessage } from '../accounts/types'


type Subscription = {
Expand Down Expand Up @@ -264,30 +265,27 @@ export class Provider extends EventEmitter {
}

approveSignTypedData (req: SignTypedDataRequest, cb: Callback<string>) {
const res = (data: any) => {
const res = (data: unknown) => {
if (this.handlers[req.handlerId]) this.handlers[req.handlerId](data)
delete this.handlers[req.handlerId]
}

const payload = req.payload
const [address, data] = payload.params

const dataToSign = (Array.isArray(data)) ? [...data] : { ...data }
const { payload, typedMessage } = req
const [address] = payload.params

accounts.signTypedData(req.version, address, dataToSign, (err, signature) => {
accounts.signTypedData(address, typedMessage, (err, signature = '') => {
if (err) {
resError(err.message, payload, res)
cb(err)
} else {
const sig = signature || ''
try {
const recoveredAddress = recoverTypedMessage({ data, sig }, req.version)
const recoveredAddress = recoverTypedSignature({ ...typedMessage, signature })
if (recoveredAddress.toLowerCase() !== address.toLowerCase()) {
throw new Error('TypedData signature verification failed')
}

res({ id: payload.id, jsonrpc: payload.jsonrpc, result: sig })
cb(null, sig)
res({ id: payload.id, jsonrpc: payload.jsonrpc, result: signature })
cb(null, signature)
} catch (e) {
const err = e as Error
resError(err.message, payload, res)
Expand Down Expand Up @@ -565,7 +563,7 @@ export class Provider extends EventEmitter {
accounts.addRequest(req, _res)
}

signTypedData (rawPayload: RPCRequestPayload, version: Version, res: RPCRequestCallback) {
signTypedData (rawPayload: RPC.SignTypedData.Request, version: SignTypedDataVersion, res: RPCCallback<RPC.SignTypedData.Response>) {
// ensure param order is [address, data, ...] regardless of version
const orderedParams = isAddress(rawPayload.params[1]) && !isAddress(rawPayload.params[0])
? [rawPayload.params[1], rawPayload.params[0], ...rawPayload.params.slice(2)]
Expand All @@ -576,7 +574,11 @@ export class Provider extends EventEmitter {
params: orderedParams
}

let [from = '', typedData = {}, ...additionalParams] = payload.params
let [from = '', typedData, ...additionalParams] = payload.params

if (!typedData) {
return resError(`Missing typed data`, payload, res)
}

const targetAccount = accounts.get(from.toLowerCase())

Expand All @@ -587,7 +589,7 @@ export class Provider extends EventEmitter {
// HACK: Standards clearly say, that second param is an object but it seems like in the wild it can be a JSON-string.
if (typeof (typedData) === 'string') {
try {
typedData = JSON.parse(typedData)
typedData = JSON.parse(typedData) as LegacyTypedData | TypedData
payload.params = [from, typedData, ...additionalParams]
} catch (e) {
return resError('Malformed typed data', payload, res)
Expand All @@ -606,14 +608,18 @@ export class Provider extends EventEmitter {
const signerType = getSignerType(targetAccount.lastSignerType)

// check for signers that only support signing a specific version of typed data
if (version !== 'V4' && signerType && [SignerType.Ledger, SignerType.Lattice, SignerType.Trezor].includes(signerType)) {
if (version !== SignTypedDataVersion.V4 && signerType && [SignerType.Ledger, SignerType.Lattice, SignerType.Trezor].includes(signerType)) {
const signerName = capitalize(signerType)
return resError(`${signerName} only supports eth_signTypedData_v4+`, payload, res)
}

const handlerId = this.addRequestHandler(res)
const typedMessage: TypedMessage<typeof version> = {
data: typedData,
version
}

accounts.addRequest({ handlerId, type: 'signTypedData', version, payload, account: targetAccount.address, origin: payload._origin } as SignTypedDataRequest)
accounts.addRequest({ handlerId, type: 'signTypedData', typedMessage, payload, account: targetAccount.address, origin: payload._origin } as SignTypedDataRequest)
}

subscribe (payload: RPC.Subscribe.Request, res: RPCSuccessCallback) {
Expand Down Expand Up @@ -851,8 +857,8 @@ export class Provider extends EventEmitter {

if (['eth_signTypedData', 'eth_signTypedData_v1', 'eth_signTypedData_v3', 'eth_signTypedData_v4'].includes(method)) {
const underscoreIndex = method.lastIndexOf('_')
const version = (underscoreIndex > 3 ? method.substring(underscoreIndex + 1).toUpperCase() : undefined) as Version
return this.signTypedData(payload, version, res)
const version = (underscoreIndex > 3 ? method.substring(underscoreIndex + 1).toUpperCase() : undefined) as SignTypedDataVersion
return this.signTypedData(payload as RPC.SignTypedData.Request, version, res as RPCCallback<RPC.SignTypedData.Response>)
}

if (method === 'wallet_addEthereumChain') return this.addEthereumChain(payload, res)
Expand Down
13 changes: 7 additions & 6 deletions main/provider/typedData.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import { TypedDataV1, TypedMessage, MessageTypes } from '@metamask/eth-sig-util'
import { SignTypedDataVersion } from '@metamask/eth-sig-util'
import type { TypedMessage } from '../accounts/types'

export function getVersionFromTypedData (typedData: TypedDataV1 | TypedMessage<MessageTypes>) {
export function getVersionFromTypedData (typedData: TypedMessage['data']) {
if (Array.isArray(typedData)) {
return 'V1'
return SignTypedDataVersion.V1
}

const hasUndefinedType = () => typedData.types[typedData.primaryType].some(({ name }) => typedData.message[name] === undefined)
Expand All @@ -11,13 +12,13 @@ export function getVersionFromTypedData (typedData: TypedDataV1 | TypedMessage<M
try {
// arrays only supported by v4
if (containsArrays()) {
return 'V4'
return SignTypedDataVersion.V4
}

// no v4-specific features so could use either v3 or v4 - default to v4 unless data contains undefined types (invalid in v4)
return hasUndefinedType() ? 'V3' : 'V4'
return hasUndefinedType() ? SignTypedDataVersion.V3 : SignTypedDataVersion.V4
} catch (e) {
// parsing error - default to v4
return 'V4'
return SignTypedDataVersion.V4
}
}
18 changes: 9 additions & 9 deletions main/signers/Signer/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@ import log from 'electron-log'
import EventEmitter from 'stream'
import { addHexPrefix } from 'ethereumjs-util'

import { TransactionData } from '../../../resources/domain/transaction'
import { deriveHDAccounts } from './derive'
import crypt from '../../crypt'
import { TypedData } from 'eth-sig-util'
import { TransactionData } from '../../../resources/domain/transaction'
import { getSignerDisplayType } from '../../../resources/domain/signer'
import type { TypedMessage } from '../../accounts/types'

export interface SignerSummary {
id: string,
Expand Down Expand Up @@ -72,30 +72,30 @@ export default class Signer extends EventEmitter {
}

open (device?: any) {
console.warn('Signer:' + this.type + ' did not implement a open method')
log.warn(`Signer: ${this.type} did not implement an open method`)
}

close () {
console.warn('Signer:' + this.type + ' did not implement a close method')
log.warn(`Signer: ${this.type} did not implement a close method`)
}

delete () {
console.warn('Signer:' + this.type + ' did not implement a delete method')
log.warn(`Signer: ${this.type} did not implement a delete method`)
}

update (options = {}) {
console.warn('Signer:' + this.type + ' did not implement a update method')
log.warn(`Signer: ${this.type} did not implement an update method`)
}

signMessage (index: number, message: string, cb: Callback<string>) {
console.warn('Signer:' + this.type + ' did not implement a signMessage method')
log.warn(`Signer: ${this.type} did not implement a signMessage method`)
}

signTransaction (index: number, rawTx: TransactionData, cb: Callback<string>) {
console.warn('Signer:' + this.type + ' did not implement a signTransaction method')
log.warn(`Signer: ${this.type} did not implement a signTransaction method`)
}

signTypedData (index: number, version: string, typedData: TypedData, cb: Callback<string>) {
signTypedData (index: number, typedMessage: TypedMessage, cb: Callback<string>) {
return cb(new Error(`Signer: ${this.type} does not support eth_signTypedData`), undefined)
}
}
4 changes: 2 additions & 2 deletions main/signers/hot/HotSigner/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,8 @@ class HotSigner extends Signer {
this._callWorker(payload, cb)
}

signTypedData (index, version, typedData, cb) {
const payload = { method: 'signTypedData', params: { index, typedData, version } }
signTypedData (index, typedMessage, cb) {
const payload = { method: 'signTypedData', params: { index, typedMessage } }
this._callWorker(payload, cb)
}

Expand Down
7 changes: 4 additions & 3 deletions main/signers/hot/HotSigner/worker.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
const crypto = require('crypto')
const ethSigUtil = require('eth-sig-util')
const { signTypedData } = require('@metamask/eth-sig-util')
const { TransactionFactory } = require('@ethereumjs/tx')
const Common = require('@ethereumjs/common').default

Expand Down Expand Up @@ -56,9 +56,10 @@ class HotSignerWorker {
pseudoCallback(null, addHexPrefix(hex))
}

signTypedData (key, params, pseudoCallback) {
signTypedData (key, typedMessage, pseudoCallback) {
try {
const signature = ethSigUtil.signTypedMessage(key, { data: params.typedData }, params.version)
const { data, version } = typedMessage
const signature = signTypedData({ privateKey: key, data, version })
pseudoCallback(null, signature)
} catch (e) {
pseudoCallback(e.message)
Expand Down
6 changes: 3 additions & 3 deletions main/signers/hot/RingSigner/worker.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,13 +52,13 @@ class RingSignerWorker extends HotSignerWorker {
super.signMessage(this.keys[index], message, pseudoCallback)
}

signTypedData (params, pseudoCallback) {
signTypedData ({ index, typedMessage }, pseudoCallback) {
// Make sure signer is unlocked
if (!this.keys) return pseudoCallback('Signer locked')
// Sign Typed Data
super.signTypedData(this.keys[params.index], params, pseudoCallback)
super.signTypedData(this.keys[index], typedMessage, pseudoCallback)
}

signTransaction ({ index, rawTx }, pseudoCallback) {
// Make sure signer is unlocked
if (!this.keys) return pseudoCallback('Signer locked')
Expand Down
6 changes: 3 additions & 3 deletions main/signers/hot/SeedSigner/worker.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,13 @@ class SeedSignerWorker extends HotSignerWorker {
super.signMessage(key, message, pseudoCallback)
}

signTypedData (params, pseudoCallback) {
signTypedData ({ index, typedMessage }, pseudoCallback) {
// Make sure signer is unlocked
if (!this.seed) return pseudoCallback('Signer locked')
// Derive private key
const key = this._derivePrivateKey(params.index)
const key = this._derivePrivateKey(index)
// Sign message
super.signTypedData(key, params, pseudoCallback)
super.signTypedData(key, typedMessage, pseudoCallback)
}

signTransaction ({ index, rawTx }, pseudoCallback) {
Expand Down
Loading

0 comments on commit fd343dc

Please sign in to comment.