Skip to content
This repository has been archived by the owner on Nov 23, 2023. It is now read-only.

Support domain-only EthereumSignTypedData and other improvements #1033

Merged
merged 4 commits into from
Feb 10, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/karma.yml
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ jobs:
path: build
# xvfb is required to run karma
- run: sudo apt-get install xvfb
- run: xvfb-run --auto-servernum ./tests/run.sh -s 'yarn test:karma:production' -i ethereumGetAddress,ethereumGetPublicKey,ethereumSignMessage,ethereumSignTransaction,ethereumVerifyMessage
- run: xvfb-run --auto-servernum ./tests/run.sh -s 'yarn test:karma:production' -i ethereumGetAddress,ethereumGetPublicKey,ethereumSignMessage,ethereumSignTransaction,ethereumVerifyMessage,ethereumSignTypedData

nem:
needs: check
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ jobs:
with:
path: node_modules
key: ${{ runner.os }}-yarn-${{ hashFiles('**/yarn.lock') }}
- run: ./tests/run.sh -i ethereumGetAddress,ethereumGetPublicKey,ethereumSignMessage,ethereumSignTransaction,ethereumVerifyMessage
- run: ./tests/run.sh -i ethereumGetAddress,ethereumGetPublicKey,ethereumSignMessage,ethereumSignTransaction,ethereumVerifyMessage,ethereumSignTypedData

nem:
needs: check
Expand Down
4 changes: 2 additions & 2 deletions .gitlab-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ publish beta release to npm:
- TESTS_INCLUDED_METHODS: "stellarGetAddress,stellarSignTransaction"
- TESTS_INCLUDED_METHODS: "cardanoGetAddress,cardanoGetNativeScriptHash,cardanoGetPublicKey,cardanoSignTransaction"
- TESTS_INCLUDED_METHODS: "eosGetPublicKey,eosSignTransaction"
- TESTS_INCLUDED_METHODS: "ethereumGetAddress,ethereumGetPublicKey,ethereumSignMessage,ethereumSignTransaction,ethereumVerifyMessage"
- TESTS_INCLUDED_METHODS: "ethereumGetAddress,ethereumGetPublicKey,ethereumSignMessage,ethereumSignTransaction,ethereumVerifyMessage,ethereumSignTypedData"
- TESTS_INCLUDED_METHODS: "nemGetAddress,nemSignTransaction"
- TESTS_INCLUDED_METHODS: "rippleGetAddress,rippleSignTransaction"
- TESTS_INCLUDED_METHODS: "tezosGetAddress,tezosGetPublicKey"
Expand All @@ -171,7 +171,7 @@ publish beta release to npm:
- TESTS_INCLUDED_METHODS: "applySettings,applyFlags,getFeatures"
- TESTS_INCLUDED_METHODS: "signTransaction"
- TESTS_INCLUDED_METHODS: "getAccountInfo,getAddress,getPublicKey,signMessage,verifyMessage,composeTransaction"
- TESTS_INCLUDED_METHODS: "ethereumGetAddress,ethereumGetPublicKey,ethereumSignMessage,ethereumSignTransaction,ethereumVerifyMessage"
- TESTS_INCLUDED_METHODS: "ethereumGetAddress,ethereumGetPublicKey,ethereumSignMessage,ethereumSignTransaction,ethereumVerifyMessage,ethereumSignTypedData"

.test:
extends: .jobs
Expand Down
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,12 @@

### Fixed
- Some methods not throwing `ui-device_firmware_unsupported` when the current device firmware didn't support the method.
- Ethereum: EthereumSignTypedData now supports signing domain-only data, i.e. when `primaryType` is equal to `EIP712Domain`.

### Changed

- @trezor/blockchain-link 2.0.0 use workers as commonjs modules in nodejs and react-native env.
- Ethereum: EthereumSignTypedData must always have at least Trezor T parameters.

# 8.2.6

Expand Down
94 changes: 64 additions & 30 deletions docs/methods/ethereumSignTypedData.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,52 +19,86 @@ TrezorConnect.ethereumSignTypedData(params).then(function(result) {
```

> :warning: **Supported only by Trezor T with Firmware 2.4.3 or higher!**
> :warning: **Blind signing is supported only on Trezor Model 1 with Firmware 1.10.5 or higher!**

### Params

[****Optional common params****](commonParams.md)

###### [flowtype](../../src/js/types/networks/ethereum.js#L102-105)
###### [flowtype](../../src/js/types/networks/ethereum.js#104-116)

> :warning: **Domain-only signing (`data.primaryType` = `"EIP712Domain"`) is supported only on Trezor T with Firmware 2.4.4 or higher!**

* `path` — *required* `string | Array<number>` minimum length is `3`. [read more](path.md)
* `data` - *required* `Object` type of [`EthereumSignTypedDataMessage`](../../src/js/types/networks/ethereum.js#L90)`. A JSON Schema definition can be found in the [EIP-712 spec]([EIP-712](https://eips.ethereum.org/EIPS/eip-712)).
* `metamask_v4_compat` - *required* `boolean` set to `true` for compatibility with [MetaMask's signTypedData_v4](https://docs.metamask.io/guide/signing-data.html#sign-typed-data-v4).

#### Blind signing (optional addition for Trezor Model 1 compatibility)

The Trezor Model 1 firmware currently does not support constructing EIP-712
hashes.

However, it supports signing pre-constructed hashes.

EIP-712 hashes can be constructed with the plugin function at
["trezor-connect/lib/plugins/ethereum/typedData.js"](../../src/js/plugins/ethereum/typedData.js)
(included as a plugin due to a depedency on `@metamask/eth-sig-utils`).

You may also wish to contruct your own hashes using a different library.

###### [flowtype](../../src/js/types/networks/ethereum.js#L114-121)

> :warning: **Domain-only signing (empty `message_hash`) is supported only on Trezor Model 1 with Firmware 1.10.6 or higher!**

* `domain_separator_hash` - *required* `string` hex-encoded 32-byte hash of the EIP-712 domain.
* `message_hash` - *optional* `string` hex-encoded 32-byte hash of the EIP-712 message.
This is optional for the domain-only hashes where `primaryType` is `EIP712Domain`.

### Example

```javascript
const eip712Data = {
types: {
EIP712Domain: [
{
name: 'name',
type: 'string',
},
],
Message: [
{
name: "Best Wallet",
type: "string"
},
{
name: "Number",
type: "uint64"
}
]
},
primaryType: 'Message',
domain: {
name: 'example.trezor.io',
},
message: {
"Best Wallet": "Trezor Model T",
// be careful with JavaScript numbers: MAX_SAFE_INTEGER is quite low
"Number": `${2n ** 55n}`,
},
};

// This functionality is separate from trezor-connect, as it requires @metamask/eth-sig-utils,
// which is a large JavaScript dependency
const transformTypedDataPlugin = require("trezor-connect/lib/plugins/ethereum/typedData.js");
const {domain_separator_hash, message_hash} = transformTypedDataPlugin(eip712Data, true);

TrezorConnect.ethereumSignTypedData({
path: "m/44'/60'/0'",
data: {
types: {
EIP712Domain: [
{
name: 'name',
type: 'string',
},
],
Message: [
{
name: "Best Wallet",
type: "string"
},
{
name: "Number",
type: "uint64"
}
]
},
primaryType: 'Message',
domain: {
name: 'example.trezor.io',
},
message: {
"Best Wallet": "Trezor Model T",
// be careful with JavaScript numbers: MAX_SAFE_INTEGER is quite low
"Number": `${2n ** 55n}`,
},
},
data: eip712Data,
metamask_v4_compat: true,
// These are optional, but required for Trezor Model 1 compatibility
domain_separator_hash,
message_hash,
});
```

Expand Down
5 changes: 5 additions & 0 deletions src/data/config.json
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,11 @@
"methods": ["ethereumSignTypedData"],
"min": ["1.10.5", "2.4.3"],
"comment": ["EIP-712 typed signing support added in 1.10.5/2.4.3"]
},
{
"capabilities": ["eip712-domain-only"],
"min": ["1.10.6", "2.4.4"],
"comment": ["EIP-712 domain-only signing, when primaryType=EIP712Domain"]
}
]
}
72 changes: 55 additions & 17 deletions src/js/core/methods/EthereumSignTypedData.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,18 @@ import type { MessageResponse, EthereumTypedDataStructAck } from '../../types/tr
import { ERRORS } from '../../constants';
import type {
EthereumSignTypedData as EthereumSignTypedDataParams,
EthereumSignTypedHash as EthereumSignTypedHashParams,
EthereumSignTypedHashAndData as EthereumSignTypedHashAndDataParams,
} from '../../types/networks/ethereum';
import { getFieldType, parseArrayType, encodeData } from './helpers/ethereumSignTypedData';
import { messageToHex } from '../../utils/formatUtils';

type Params = {
address_n: number[],
...$Exact<EthereumSignTypedDataParams>,
...$Exact<EthereumSignTypedHashParams>,
};
type Params = $Diff<
{
address_n: number[],
...$Exact<EthereumSignTypedDataParams> | $Exact<EthereumSignTypedHashAndDataParams>,
},
{ path: any }, // removes the "path" variable from this.params
mroz22 marked this conversation as resolved.
Show resolved Hide resolved
>;

export default class EthereumSignTypedData extends AbstractMethod<'ethereumSignTypedData'> {
params: Params;
Expand All @@ -30,10 +33,10 @@ export default class EthereumSignTypedData extends AbstractMethod<'ethereumSignT
// validate incoming parameters
validateParams(payload, [
{ name: 'path', required: true },
{ name: 'metamask_v4_compat', type: 'boolean' },
// model T
{ name: 'data', type: 'object' },
// model One
{ name: 'metamask_v4_compat', type: 'boolean', required: true },
{ name: 'data', type: 'object', required: true },
// model One (optional params)
{ name: 'domain_separator_hash', type: 'string' },
{ name: 'message_hash', type: 'string' },
]);
Expand All @@ -45,13 +48,48 @@ export default class EthereumSignTypedData extends AbstractMethod<'ethereumSignT
this.info = getNetworkLabel('Sign #NETWORK typed data', network);

this.params = {
path,
address_n: path,
metamask_v4_compat: payload.metamask_v4_compat,
domain_separator_hash: payload.domain_separator_hash || '',
message_hash: payload.message_hash || '',
data: payload.data || undefined,
data: payload.data,
};

if (payload.domain_separator_hash) {
this.params = {
...this.params,
// leading `0x` in hash-strings causes issues
domain_separator_hash: messageToHex(payload.domain_separator_hash),
};

if (payload.message_hash) {
this.params = {
...this.params,
// leading `0x` in hash-strings causes issues
message_hash: messageToHex(payload.message_hash),
};
} else if (this.params.data.primaryType !== 'EIP712Domain') {
throw ERRORS.TypedError(
'Method_InvalidParameter',
'message_hash should only be empty when data.primaryType=EIP712Domain',
);
}
}

if (this.params.data.primaryType === 'EIP712Domain') {
// Only newer firmwares support this feature
// Older firmwares will give wrong results / throw errors
this.firmwareRange = getFirmwareRange(
'eip712-domain-only',
network,
this.firmwareRange,
);

if (this.params.message_hash) {
throw ERRORS.TypedError(
'Method_InvalidParameter',
'message_hash should be empty when data.primaryType=EIP712Domain',
);
}
}
}

async run() {
Expand All @@ -61,10 +99,12 @@ export default class EthereumSignTypedData extends AbstractMethod<'ethereumSignT
if (this.device.features.model === '1') {
validateParams(this.params, [
{ name: 'domain_separator_hash', type: 'string', required: true },
{ name: 'message_hash', type: 'string', required: true },
{ name: 'message_hash', type: 'string' },
]);

const { domain_separator_hash, message_hash } = this.params;
const { domain_separator_hash, message_hash } =
// $FlowIssue validateParams() confirms that these hashes exist
(this.params: EthereumSignTypedHashAndDataParams);

// For Model 1 we use EthereumSignTypedHash
const response = await cmd.typedCall(
Expand All @@ -84,9 +124,7 @@ export default class EthereumSignTypedData extends AbstractMethod<'ethereumSignT
};
}

validateParams(this.params, [{ name: 'data', type: 'object', required: true }]);
mroz22 marked this conversation as resolved.
Show resolved Hide resolved
const { data, metamask_v4_compat } = this.params;
// $FlowIssue
const { types, primaryType, domain, message } = data;

// For Model T we use EthereumSignTypedData
Expand Down
27 changes: 21 additions & 6 deletions src/js/plugins/ethereum/typedData.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,17 @@ function sanitizeData(data) {
}
}

/**
* Calculates the domain_separator_hash and message_hash from an EIP-712 Typed Data object.
*
* The Trezor Model 1 does not currently support constructing the hash on the device,
* so this function pre-computes them.
*
* @template {sigUtil.TypedMessage} T
* @param {T} data - The EIP-712 Typed Data object.
* @param {boolean} metamask_v4_compat - Set to `true` for compatibility with Metamask's signTypedData_v4 function.
* @returns {{domain_separator_hash: string, message_hash?: string} & T} The hashes.
*/
mroz22 marked this conversation as resolved.
Show resolved Hide resolved
const transformTypedData = (data, metamask_v4_compat) => {
if (!metamask_v4_compat) {
throw new Error('Trezor: Only version 4 of typed data signing is supported');
Expand All @@ -36,12 +47,16 @@ const transformTypedData = (data, metamask_v4_compat) => {
version,
).toString('hex');

const messageHash = sigUtil.TypedDataUtils.hashStruct(
primaryType,
sanitizeData(message),
types,
version,
).toString('hex');
let messageHash = null;

if (primaryType !== 'EIP712Domain') {
messageHash = sigUtil.TypedDataUtils.hashStruct(
primaryType,
sanitizeData(message),
types,
version,
).toString('hex');
}

return {
domain_separator_hash: domainSeparatorHash,
Expand Down
26 changes: 21 additions & 5 deletions src/js/plugins/ethereum/typedData.test.js
Original file line number Diff line number Diff line change
@@ -1,17 +1,33 @@
const commonFixtures = require('../../../../submodules/trezor-common/tests/fixtures/ethereum/sign_typed_data.json');
const typedData = require('./typedData');

// Adds 0x to a string if it doesn't start with one
// fixtures sometimes start with 0x, sometimes not
function messageToHex(string) {
return string.startsWith('0x') ? string : `0x${string}`;
}

describe('typedData', () => {
commonFixtures.tests
.filter(test => test.parameters.metamask_v4_compat)
.forEach(test => {
it('typedData to message_hash and domain_separator_hash', () => {
const transformed = typedData(test.parameters.data, true);
// todo: fixtures in firmware-repo not unified, probably bug
const { domain_separator_hash /* , message_hash */ } = transformed;
const transformed = typedData(
test.parameters.data,
test.parameters.metamask_v4_compat,
);
const { domain_separator_hash, message_hash } = transformed;

expect(`0x${domain_separator_hash}`).toEqual(test.parameters.domain_separator_hash);
// expect(`0x${message_hash}`).toEqual(test.parameters.message_hash);
expect(messageToHex(domain_separator_hash)).toEqual(
messageToHex(test.parameters.domain_separator_hash),
);
if (message_hash) {
expect(messageToHex(message_hash)).toEqual(
messageToHex(test.parameters.message_hash),
);
} else {
expect(null).toEqual(test.parameters.message_hash);
}
});
});
});
Loading