Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve UX of TypedData usage #1198

Merged
merged 4 commits into from
Aug 26, 2024
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
126 changes: 126 additions & 0 deletions __mocks__/typedData/v1Nested.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
{
"domain": {
"name": "Dappland",
"chainId": "0x534e5f5345504f4c4941",
"version": "1.0.2",
"revision": "1"
},
"message": {
"MessageId": 345,
"From": {
"Name": "Edmund",
"Address": "0x7e00d496e324876bbc8531f2d9a82bf154d1a04a50218ee74cdd372f75a551a"
},
"To": {
"Name": "Alice",
"Address": "0x69b49c2cc8b16e80e86bfc5b0614a59aa8c9b601569c7b80dde04d3f3151b79"
},
"Nft_to_transfer": {
"Collection": "Stupid monkeys",
"Address": "0x69b49c2cc8b16e80e86bfc5b0614a59aa8c9b601569c7b80dde04d3f3151b79",
"Nft_id": 112,
"Negotiated_for": {
"Qty": "18.4569325643",
"Unit": "ETH",
"Token_address": "0x69b49c2cc8b16e80e86bfc5b0614a59aa8c9b601569c7b80dde04d3f3151b79",
"Amount": "0x100243260D270EB00"
}
},
"Comment1": "Monkey with banana, sunglasses,",
"Comment2": "and red hat.",
"Comment3": ""
},
"primaryType": "TransferERC721",
"types": {
"Account1": [
{
"name": "Name",
"type": "string"
},
{
"name": "Address",
"type": "felt"
}
],
"Nft": [
{
"name": "Collection",
"type": "string"
},
{
"name": "Address",
"type": "felt"
},
{
"name": "Nft_id",
"type": "felt"
},
{
"name": "Negotiated_for",
"type": "Transaction"
}
],
"Transaction": [
{
"name": "Qty",
"type": "string"
},
{
"name": "Unit",
"type": "string"
},
{
"name": "Token_address",
"type": "felt"
},
{
"name": "Amount",
"type": "felt"
}
],
"TransferERC721": [
{
"name": "MessageId",
"type": "felt"
},
{
"name": "From",
"type": "Account1"
},
{
"name": "To",
"type": "Account1"
},
{
"name": "Nft_to_transfer",
"type": "Nft"
},
{
"name": "Comment1",
"type": "string"
},
{
"name": "Comment2",
"type": "string"
},
{
"name": "Comment3",
"type": "string"
}
],
"StarknetDomain": [
{
"name": "name",
"type": "string"
},
{
"name": "chainId",
"type": "felt"
},
{
"name": "version",
"type": "string"
}
]
}
}
50 changes: 47 additions & 3 deletions __tests__/rpcProvider.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { getStarkKey, utils } from '@scure/starknet';

import { getStarkKey, Signature, utils } from '@scure/starknet';
import typedDataExample from '../__mocks__/typedData/baseExample.json';
import {
Account,
Block,
Expand All @@ -18,7 +18,7 @@ import {
} from '../src';
import { StarknetChainId } from '../src/constants';
import { felt, uint256 } from '../src/utils/calldata/cairo';
import { toHexString } from '../src/utils/num';
import { toBigInt, toHexString } from '../src/utils/num';
import {
compiledC1v2,
compiledC1v2Casm,
Expand Down Expand Up @@ -493,3 +493,47 @@ describeIfNotDevnet('waitForBlock', () => {
expect(true).toBe(true); // answer without timeout Error (blocks have to be spaced with 16 minutes maximum : 200 retries * 5000ms)
});
});

describe('EIP712 verification', () => {
const rpcProvider = getTestProvider(false);
const account = getTestAccount(rpcProvider);

test('sign and verify message', async () => {
const signature = await account.signMessage(typedDataExample);
const verifMessageResponse: boolean = await rpcProvider.verifyMessageInStarknet(
typedDataExample,
signature,
account.address
);
expect(verifMessageResponse).toBe(true);

const messageHash = await account.hashMessage(typedDataExample);
const verifMessageResponse2: boolean = await rpcProvider.verifyMessageInStarknet(
messageHash,
signature,
account.address
);
expect(verifMessageResponse2).toBe(true);
});

test('sign and verify EIP712 message fail', async () => {
const signature = await account.signMessage(typedDataExample);
const [r, s] = stark.formatSignature(signature);

// change the signature to make it invalid
const r2 = toBigInt(r) + 123n;
const wrongSignature = new Signature(toBigInt(r2.toString()), toBigInt(s));
if (!wrongSignature) return;
const verifMessageResponse: boolean = await rpcProvider.verifyMessageInStarknet(
typedDataExample,
wrongSignature,
account.address
);
expect(verifMessageResponse).toBe(false);

const wrongAccountAddress = '0x123456789';
await expect(
rpcProvider.verifyMessageInStarknet(typedDataExample, signature, wrongAccountAddress)
).rejects.toThrow();
});
});
11 changes: 11 additions & 0 deletions __tests__/utils/num.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -216,3 +216,14 @@ describe('stringToSha256ToArrayBuff4', () => {
expect(buff).toEqual(new Uint8Array([43, 206, 231, 219]));
});
});

describe('isBigNumberish', () => {
test('determine if value is a BigNumberish', () => {
expect(num.isBigNumberish(234)).toBe(true);
expect(num.isBigNumberish(234n)).toBe(true);
expect(num.isBigNumberish('234')).toBe(true);
expect(num.isBigNumberish('0xea')).toBe(true);
expect(num.isBigNumberish('ea')).toBe(false);
expect(num.isBigNumberish('zero')).toBe(false);
});
});
9 changes: 9 additions & 0 deletions __tests__/utils/stark.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,3 +116,12 @@ describe('stark', () => {
expect(stark.v3Details(detailsUndefined)).toEqual(expect.objectContaining(detailsAnything));
});
});

describe('ec full public key', () => {
test('determine if value is a BigNumberish', () => {
const privateKey1 = '0x43b7240d227aa2fb8434350b3321c40ac1b88c7067982549e7609870621b535';
expect(stark.getFullPublicKey(privateKey1)).toBe(
'0x0400b730bd22358612b5a67f8ad52ce80f9e8e893639ade263537e6ef35852e5d3057795f6b090f7c6985ee143f798608a53b3659222c06693c630857a10a92acf'
);
});
});
56 changes: 55 additions & 1 deletion __tests__/utils/typedData.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,17 @@ import exampleEnum from '../../__mocks__/typedData/example_enum.json';
import examplePresetTypes from '../../__mocks__/typedData/example_presetTypes.json';
import typedDataStructArrayExample from '../../__mocks__/typedData/mail_StructArray.json';
import typedDataSessionExample from '../../__mocks__/typedData/session_MerkleTree.json';
import { BigNumberish, StarknetDomain, num } from '../../src';
import v1NestedExample from '../../__mocks__/typedData/v1Nested.json';
import {
Account,
BigNumberish,
StarknetDomain,
num,
stark,
typedData,
type ArraySignatureType,
type Signature,
} from '../../src';
import { PRIME } from '../../src/constants';
import { getSelectorFromName } from '../../src/utils/hash';
import { MerkleTree } from '../../src/utils/merkle';
Expand Down Expand Up @@ -346,4 +356,48 @@ describe('typedData', () => {
expect(() => getMessageHash(baseTypes(type), exampleAddress)).toThrow(RegExp(type));
});
});

describe('verifyMessage', () => {
const addr = '0x64b48806902a367c8598f4f95c305e8c1a1acba5f082d294a43793113115691';
const privK = '0x71d7bb07b9a64f6f78ac4c816aff4da9';
const fullPubK = stark.getFullPublicKey(privK);
const myAccount = new Account({ nodeUrl: 'fake' }, addr, privK);
let signedMessage: Signature;
let hashedMessage: string;
let arraySign: ArraySignatureType;

beforeAll(async () => {
signedMessage = await myAccount.signMessage(v1NestedExample);
hashedMessage = await myAccount.hashMessage(v1NestedExample);
arraySign = stark.formatSignature(signedMessage);
});

test('with TypedMessage', () => {
expect(
typedData.verifyMessage(v1NestedExample, signedMessage, fullPubK, myAccount.address)
).toBe(true);
expect(typedData.verifyMessage(v1NestedExample, arraySign, fullPubK, myAccount.address)).toBe(
true
);
});

test('with messageHash', () => {
expect(typedData.verifyMessage(hashedMessage, signedMessage, fullPubK)).toBe(true);
expect(typedData.verifyMessage(hashedMessage, arraySign, fullPubK)).toBe(true);
});

test('failure cases', () => {
expect(() => typedData.verifyMessage('zero', signedMessage, fullPubK)).toThrow(
'message has a wrong format.'
);

expect(() =>
typedData.verifyMessage(v1NestedExample as any, signedMessage, fullPubK)
).toThrow(/^When providing a TypedData .* the accountAddress parameter has to be provided/);

expect(() =>
typedData.verifyMessage(v1NestedExample, signedMessage, fullPubK, 'wrong')
).toThrow('accountAddress shall be a BigNumberish');
});
});
});
83 changes: 16 additions & 67 deletions src/account/default.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ import { parseContract } from '../utils/provider';
import { isString } from '../utils/shortString';
import {
estimateFeeToBounds,
formatSignature,
reduceV2,
toFeeVersion,
toTransactionVersion,
Expand Down Expand Up @@ -543,87 +542,37 @@ export class Account extends Provider implements AccountInterface {
return getMessageHash(typedData, this.address);
}

/**
* @deprecated To replace by `myRpcProvider.verifyMessageInStarknet()`
*/
public async verifyMessageHash(
hash: BigNumberish,
signature: Signature,
signatureVerificationFunctionName?: string,
signatureVerificationResponse?: { okResponse: string[]; nokResponse: string[]; error: string[] }
): Promise<boolean> {
// HOTFIX: Accounts should conform to SNIP-6
// (https://github.com/starknet-io/SNIPs/blob/f6998f779ee2157d5e1dea36042b08062093b3c5/SNIPS/snip-6.md?plain=1#L61),
// but they don't always conform. Also, the SNIP doesn't standardize the response if the signature isn't valid.
const knownSigVerificationFName = signatureVerificationFunctionName
? [signatureVerificationFunctionName]
: ['isValidSignature', 'is_valid_signature'];
const knownSignatureResponse = signatureVerificationResponse || {
okResponse: [
// any non-nok response is true
],
nokResponse: [
'0x0', // Devnet
'0x00', // OpenZeppelin 0.7.0 to 0.9.0 invalid signature
],
error: [
'argent/invalid-signature', // ArgentX 0.3.0 to 0.3.1
'is invalid, with respect to the public key', // OpenZeppelin until 0.6.1, Braavos 0.0.11
'INVALID_SIG', // Braavos 1.0.0
],
};
let error: any;

// eslint-disable-next-line no-restricted-syntax
for (const SigVerificationFName of knownSigVerificationFName) {
try {
// eslint-disable-next-line no-await-in-loop
const resp = await this.callContract({
contractAddress: this.address,
entrypoint: SigVerificationFName,
calldata: CallData.compile({
hash: toBigInt(hash).toString(),
signature: formatSignature(signature),
}),
});
// Response NOK Signature
if (knownSignatureResponse.nokResponse.includes(resp[0].toString())) {
return false;
}
// Response OK Signature
// Empty okResponse assume all non-nok responses are valid signatures
// OpenZeppelin 0.7.0 to 0.9.0, ArgentX 0.3.0 to 0.3.1 & Braavos Cairo 0.0.11 to 1.0.0 valid signature
if (
knownSignatureResponse.okResponse.length === 0 ||
knownSignatureResponse.okResponse.includes(resp[0].toString())
) {
return true;
}
throw Error('signatureVerificationResponse Error: response is not part of known responses');
} catch (err) {
// Known NOK Errors
if (
knownSignatureResponse.error.some((errMessage) =>
(err as Error).message.includes(errMessage)
)
) {
return false;
}
// Unknown Error
error = err;
}
}

throw Error(`Signature verification Error: ${error}`);
return this.verifyMessageInStarknet(
hash,
signature,
this.address,
signatureVerificationFunctionName,
signatureVerificationResponse
);
}

/**
* @deprecated To replace by `myRpcProvider.verifyMessageInStarknet()`
*/
public async verifyMessage(
typedData: TypedData,
signature: Signature,
signatureVerificationFunctionName?: string,
signatureVerificationResponse?: { okResponse: string[]; nokResponse: string[]; error: string[] }
): Promise<boolean> {
const hash = await this.hashMessage(typedData);
return this.verifyMessageHash(
hash,
return this.verifyMessageInStarknet(
typedData,
signature,
this.address,
signatureVerificationFunctionName,
signatureVerificationResponse
);
Expand Down
Loading