diff --git a/main/provider/index.ts b/main/provider/index.ts index 82274896c..e8d8cf546 100644 --- a/main/provider/index.ts +++ b/main/provider/index.ts @@ -665,32 +665,29 @@ export class Provider extends EventEmitter { this.connection.send(payload, res, targetChain) } - sign (payload: RPCRequestPayload, res: RPCRequestCallback) { - // normalize the payload for downstream rendering, taking the first address and - // making it the first parameter, which is the account that needs to sign - const addressIndex = payload.params.findIndex(utils.isAddress) - - const orderedParams = addressIndex > 0 - ? [ - payload.params[addressIndex], - ...payload.params.slice(0, addressIndex), - ...payload.params.slice(addressIndex + 1) - ] - : payload.params - - const normalizedPayload = { - ...payload, - params: orderedParams + _personalSign (payload: RPCRequestPayload, res: RPCRequestCallback) { + const params = payload.params || [] + + if (utils.isAddress(params[0]) && !utils.isAddress(params[1])) { + // personal_sign requests expect the first parameter to be the message and the second + // parameter to be an address. however some clients send these in the opposite order + // so try to detect that + return this.sign(payload, res) } - const from = orderedParams[0] + // switch the order of params to be consistent with eth_sign + return this.sign({ ...payload, params: [params[1], params[0], ...params.slice(2)] }, res) + } + + sign (payload: RPCRequestPayload, res: RPCRequestCallback) { + const from = (payload.params || [])[0] const currentAccount = accounts.current() if (!this.isCurrentAccount(from, currentAccount)) return this.resError('sign request is not from currently selected account.', payload, res) const handlerId = this.addRequestHandler(res) - const req = { handlerId, type: 'sign', payload: normalizedPayload, account: (currentAccount as FrameAccount).getAccounts()[0], origin: payload._origin } as const + const req = { handlerId, type: 'sign', payload, account: (currentAccount as FrameAccount).getAccounts()[0], origin: payload._origin } as const const _res = (data: any) => { if (this.handlers[req.handlerId]) this.handlers[req.handlerId](data) @@ -982,7 +979,8 @@ export class Provider extends EventEmitter { } if (method === 'eth_unsubscribe' && this.ifSubRemove(payload.params[0])) return res({ id: payload.id, jsonrpc: '2.0', result: true }) // Subscription was ours - if (method === 'eth_sign' || method === 'personal_sign') return this.sign(payload, res) + if (method === 'personal_sign') return this._personalSign(payload, res) + if (method === 'eth_sign') return this.sign(payload, res) if (['eth_signTypedData', 'eth_signTypedData_v1', 'eth_signTypedData_v3', 'eth_signTypedData_v4'].includes(method)) { const underscoreIndex = method.lastIndexOf('_') diff --git a/test/main/provider/index.test.js b/test/main/provider/index.test.js index daa84d524..7f1c17848 100644 --- a/test/main/provider/index.test.js +++ b/test/main/provider/index.test.js @@ -916,23 +916,38 @@ describe('#send', () => { describe('#personal_sign', () => { const message = 'hello, Ethereum!' + const password = 'supersecret' it('submits a request to sign a personal message with the address first', () => { - send({ method: 'personal_sign', params: [message, address] }) + send({ method: 'personal_sign', params: [address, message, password] }) expect(accountRequests).toHaveLength(1) expect(accountRequests[0].handlerId).toBeTruthy() expect(accountRequests[0].payload.params[0]).toBe(address) expect(accountRequests[0].payload.params[1]).toEqual(message) + expect(accountRequests[0].payload.params[2]).toEqual(password) }) it('submits a request to sign a personal message with the message first', () => { - send({ method: 'personal_sign', params: [address, message] }) + send({ method: 'personal_sign', params: [message, address, password] }) expect(accountRequests).toHaveLength(1) expect(accountRequests[0].handlerId).toBeTruthy() expect(accountRequests[0].payload.params[0]).toBe(address) expect(accountRequests[0].payload.params[1]).toEqual(message) + expect(accountRequests[0].payload.params[2]).toEqual(password) + }) + + it('submits a request to sign a personal message with a 20-byte message first', () => { + const hexMessage = '0x6672616d652e7368206973206772656174212121' + + send({ method: 'personal_sign', params: [hexMessage, address, password] }) + + expect(accountRequests).toHaveLength(1) + expect(accountRequests[0].handlerId).toBeTruthy() + expect(accountRequests[0].payload.params[0]).toBe(address) + expect(accountRequests[0].payload.params[1]).toEqual(hexMessage) + expect(accountRequests[0].payload.params[2]).toEqual(password) }) it('does not submit a request from an account other than the current one', done => {