-
Notifications
You must be signed in to change notification settings - Fork 1
feat(eip712): populate signature requests with additional data (WALLET-1318) #42
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
base: master
Are you sure you want to change the base?
Changes from all commits
724da02
1136c06
4ece675
03faa9f
66d829c
80cebd6
09297e4
cfc31d6
11c3da8
3d45004
feae98e
e547e9a
09f11a3
c116e81
58a72ac
6fbac2f
2abed30
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,112 @@ | ||
| import { EIP712SignatureRequestDto } from './EIP712SignatureRequestDto'; | ||
|
|
||
| const OWNER = 'a'.repeat(64); | ||
| const PKG_HASH = '0x' + '01'.repeat(32); | ||
| const SIGNING_PK = '0106956df3aba7115e28271d053205ec7f33cab259f8e2da2f38150f0ece65a2a8'; | ||
|
|
||
| const typedData = { | ||
| domain: { chain_name: 'casper', contract_package_hash: PKG_HASH }, | ||
| types: { | ||
| EIP712Domain: [ | ||
| { name: 'chain_name', type: 'string' }, | ||
| { name: 'contract_package_hash', type: 'bytes32' }, | ||
| ], | ||
| Permit: [ | ||
| { name: 'owner', type: 'address' }, | ||
| { name: 'value', type: 'uint256' }, | ||
| ], | ||
| }, | ||
| primaryType: 'Permit', | ||
| message: { owner: OWNER, value: '1000' }, | ||
| }; | ||
|
|
||
| const ownerAccountInfo = { | ||
| id: 'a', | ||
| publicKey: '', | ||
| accountHash: OWNER, | ||
| name: 'Alice', | ||
| brandingLogo: null, | ||
| csprName: null, | ||
| explorerLink: null, | ||
| }; | ||
|
|
||
| const contractPackage = { | ||
| id: 'c', | ||
| latestVersionContractTypeId: 1, | ||
| contractPackageHash: PKG_HASH, | ||
| name: 'MyToken', | ||
| iconUrl: null, | ||
| symbol: 'MTK', | ||
| decimals: 9, | ||
| }; | ||
|
|
||
| describe('EIP712SignatureRequestDto', () => { | ||
| const dto = new EIP712SignatureRequestDto({ | ||
| typedData, | ||
| signingPublicKeyHex: SIGNING_PK, | ||
| network: 'mainnet', | ||
| digest: '0xdigest', | ||
| accountInfoMap: { [OWNER]: ownerAccountInfo }, | ||
| contractPackage, | ||
| }); | ||
|
|
||
| it('enriches address rows with account info', () => { | ||
| expect(dto.messageRows.find(r => r.label === 'Owner')!.accountInfo).toEqual(ownerAccountInfo); | ||
| }); | ||
|
|
||
| it('enriches the contract_package_hash row with the contract package', () => { | ||
| expect(dto.domainRows.find(r => r.label === 'Package Hash')!.contractPackage).toEqual( | ||
| contractPackage, | ||
| ); | ||
| }); | ||
|
|
||
| it('carries scalar fields', () => { | ||
| expect(dto.network).toBe('mainnet'); | ||
| expect(dto.chainName).toBe('casper'); | ||
| expect(dto.primaryType).toBe('Permit'); | ||
| expect(dto.digest).toBe('0xdigest'); | ||
| expect(dto.id).toBe('0xdigest'); | ||
| expect(JSON.parse(dto.rawJson)).toEqual(typedData); | ||
| expect(dto.hashArtifacts).toBeUndefined(); | ||
| }); | ||
|
|
||
| it('forwards hashArtifacts when provided', () => { | ||
| const artifacts = { | ||
| domainTypeString: 'EIP712Domain(...)', | ||
| domain: typedData.domain, | ||
| domainSeparator: '0xsep', | ||
| structHash: '0xstruct', | ||
| canonicalTypeString: 'Permit(...)', | ||
| typeHash: '0xtype', | ||
| }; | ||
| const withArtifacts = new EIP712SignatureRequestDto({ | ||
| typedData, | ||
| signingPublicKeyHex: SIGNING_PK, | ||
| network: 'mainnet', | ||
| digest: '0xdigest', | ||
| hashArtifacts: artifacts, | ||
| accountInfoMap: {}, | ||
| contractPackage: null, | ||
| }); | ||
| expect(withArtifacts.hashArtifacts).toEqual(artifacts); | ||
| }); | ||
|
|
||
| it('falls back to the raw signing key when no account info is found', () => { | ||
| expect(dto.signingKey).toBe(SIGNING_PK); | ||
| expect(dto.signingKeyType).toBe('publicKey'); | ||
| expect(dto.signingAccountInfo).toBeNull(); | ||
| }); | ||
|
|
||
| it('serializes bigint message values in rawJson', () => { | ||
| const bigintData = { ...typedData, message: { owner: OWNER, value: 1000n } }; | ||
| const d = new EIP712SignatureRequestDto({ | ||
| typedData: bigintData, | ||
| signingPublicKeyHex: SIGNING_PK, | ||
| network: 'mainnet', | ||
| digest: '0xd', | ||
| accountInfoMap: {}, | ||
| contractPackage: null, | ||
| }); | ||
| expect(JSON.parse(d.rawJson).message.value).toBe('1000'); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,80 @@ | ||
| import { Maybe } from '../../../typings'; | ||
| import { | ||
| AccountKeyType, | ||
| CasperNetwork, | ||
| IAccountInfo, | ||
| IContractPackage, | ||
| IEIP712DisplayRow, | ||
| IEIP712HashArtifacts, | ||
| IEIP712SignatureRequest, | ||
| IEIP712TypedData, | ||
| } from '../../../domain'; | ||
| import { buildTypedDataDisplayModel } from '../../../utils'; | ||
| import { deriveKeyType, getAccountInfoFromMap } from '../common'; | ||
| import { resolveEip712AddressToAccountHash } from './common'; | ||
|
|
||
| export interface IEIP712SignatureRequestDtoProps { | ||
| typedData: IEIP712TypedData; | ||
| signingPublicKeyHex: string; | ||
| network: Maybe<CasperNetwork>; | ||
| digest: string; | ||
| hashArtifacts?: IEIP712HashArtifacts; | ||
| accountInfoMap: Record<string, IAccountInfo>; | ||
| contractPackage: Maybe<IContractPackage>; | ||
| } | ||
|
|
||
| export class EIP712SignatureRequestDto implements IEIP712SignatureRequest { | ||
| readonly id: string; | ||
| readonly signingKey: string; | ||
| readonly signingKeyType: AccountKeyType; | ||
| readonly signingAccountInfo: Maybe<IAccountInfo>; | ||
| readonly network: Maybe<CasperNetwork>; | ||
| readonly chainName: string; | ||
| readonly primaryType: string; | ||
| readonly domainRows: IEIP712DisplayRow[]; | ||
| readonly messageRows: IEIP712DisplayRow[]; | ||
| readonly digest: string; | ||
| readonly hashArtifacts?: IEIP712HashArtifacts; | ||
| readonly rawJson: string; | ||
|
|
||
| constructor({ | ||
| typedData, | ||
| signingPublicKeyHex, | ||
| network, | ||
| digest, | ||
| hashArtifacts, | ||
| accountInfoMap, | ||
| contractPackage, | ||
| }: IEIP712SignatureRequestDtoProps) { | ||
| const { domainRows, messageRows } = buildTypedDataDisplayModel(typedData, { | ||
| resolveAccountInfo: value => { | ||
| // address values may be a Casper public key or account hash — resolve to account hash first, | ||
| // then look up by it so the key matches what getAccountHashesFromTypedData fetched. | ||
| const accountHash = resolveEip712AddressToAccountHash(value); | ||
| return accountHash | ||
| ? (getAccountInfoFromMap(accountInfoMap, accountHash, 'accountHash') ?? null) | ||
| : null; | ||
| }, | ||
| contractPackage, | ||
| }); | ||
|
|
||
| const signingKeyType = deriveKeyType(signingPublicKeyHex); | ||
| // getAccountInfoFromMap yields runtime `undefined` for a missing key; normalize to null for Maybe<>. | ||
| this.signingAccountInfo = | ||
| getAccountInfoFromMap(accountInfoMap, signingPublicKeyHex, signingKeyType) ?? null; | ||
| this.signingKey = this.signingAccountInfo?.publicKey || signingPublicKeyHex; | ||
| this.signingKeyType = this.signingAccountInfo?.publicKey ? 'publicKey' : signingKeyType; | ||
|
|
||
| this.network = network; | ||
| this.chainName = String(typedData.domain.chain_name ?? ''); | ||
| this.primaryType = typedData.primaryType; | ||
| this.domainRows = domainRows; | ||
| this.messageRows = messageRows; | ||
| this.digest = digest; | ||
| this.hashArtifacts = hashArtifacts; | ||
| this.rawJson = JSON.stringify(typedData, (_key, value) => | ||
| typeof value === 'bigint' ? value.toString() : value, | ||
| ); | ||
| this.id = digest; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,83 @@ | ||
| import { | ||
| getAccountHashesFromTypedData, | ||
| resolveEip712AddressToAccountHash, | ||
| stripHexPrefix, | ||
| } from './common'; | ||
| import { getAccountHashFromPublicKey } from '../../../utils'; | ||
|
|
||
| const OWNER = 'a'.repeat(64); | ||
| const SPENDER = 'b'.repeat(64); | ||
| const SIGNING_PK = '0106956df3aba7115e28271d053205ec7f33cab259f8e2da2f38150f0ece65a2a8'; | ||
|
|
||
| const typedData = { | ||
| domain: { chain_name: 'casper', contract_package_hash: '0x' + '01'.repeat(32) }, | ||
| types: { | ||
| EIP712Domain: [ | ||
| { name: 'chain_name', type: 'string' }, | ||
| { name: 'contract_package_hash', type: 'bytes32' }, | ||
| ], | ||
| Permit: [ | ||
| { name: 'owner', type: 'address' }, | ||
| { name: 'spender', type: 'address' }, | ||
| { name: 'value', type: 'uint256' }, | ||
| ], | ||
| }, | ||
| primaryType: 'Permit', | ||
| message: { owner: OWNER, spender: SPENDER, value: '1000' }, | ||
| }; | ||
|
|
||
| describe('getAccountHashesFromTypedData', () => { | ||
| it('collects address-typed values plus the signing-key account hash, deduped', () => { | ||
| const hashes = getAccountHashesFromTypedData(typedData, SIGNING_PK); | ||
|
|
||
| expect(hashes).toContain(OWNER); | ||
| expect(hashes).toContain(SPENDER); | ||
| expect(hashes).toContain(getAccountHashFromPublicKey(SIGNING_PK)); | ||
| // non-address fields are ignored: value/contract_package_hash/chain_name absent | ||
| expect(hashes).not.toContain('1000'); | ||
| // deduped | ||
| expect(new Set(hashes).size).toBe(hashes.length); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This assertion is vacuous: OWNER, SPENDER, and the signing-key hash are all distinct in the fixture, so |
||
| }); | ||
|
|
||
| it('returns only the signing-key hash when there are no address fields', () => { | ||
| const noAddr = { | ||
| ...typedData, | ||
| types: { ...typedData.types, Permit: [{ name: 'value', type: 'uint256' }] }, | ||
| message: { value: '1' }, | ||
| }; | ||
| const hashes = getAccountHashesFromTypedData(noAddr, SIGNING_PK); | ||
| expect(hashes).toEqual([getAccountHashFromPublicKey(SIGNING_PK)]); | ||
| }); | ||
|
|
||
| it('ignores an address field whose value is missing/non-string', () => { | ||
| const missingValue = { | ||
| ...typedData, | ||
| types: { ...typedData.types, Permit: [{ name: 'owner', type: 'address' }] }, | ||
| message: {}, // owner declared as address but absent | ||
| }; | ||
| const hashes = getAccountHashesFromTypedData(missingValue, SIGNING_PK); | ||
| expect(hashes).toEqual([getAccountHashFromPublicKey(SIGNING_PK)]); | ||
| expect(hashes).not.toContain('undefined'); | ||
| }); | ||
| }); | ||
|
|
||
| describe('stripHexPrefix', () => { | ||
| it('removes a leading 0x only', () => { | ||
| expect(stripHexPrefix('0xabc')).toBe('abc'); | ||
| expect(stripHexPrefix('abc')).toBe('abc'); | ||
| }); | ||
| }); | ||
|
|
||
| describe('resolveEip712AddressToAccountHash', () => { | ||
| it('strips 0x and resolves a public key to its account hash', () => { | ||
| const expected = getAccountHashFromPublicKey(SIGNING_PK); | ||
| expect(resolveEip712AddressToAccountHash(SIGNING_PK)).toBe(expected); | ||
| expect(resolveEip712AddressToAccountHash('0x' + SIGNING_PK)).toBe(expected); | ||
| }); | ||
|
|
||
| it('returns a 64-hex account-hash value unchanged (minus 0x)', () => { | ||
| const acct = 'c'.repeat(64); | ||
| expect(resolveEip712AddressToAccountHash(acct)).toBe(acct); | ||
| expect(resolveEip712AddressToAccountHash('0x' + acct)).toBe(acct); | ||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| import { IEIP712Field, IEIP712TypedData } from '../../../domain'; | ||
| import { Maybe } from '../../../typings'; | ||
| import { getHashByType } from '../common'; | ||
|
|
||
| /** Strip an optional `0x` prefix. */ | ||
| export const stripHexPrefix = (value: string): string => | ||
| value.startsWith('0x') ? value.slice(2) : value; | ||
|
|
||
| /** | ||
| * Resolve an EIP-712 `address` field value to a Casper account hash. The value may be a Casper public | ||
| * key (01/02-prefixed, 66/68 hex) or an account hash (64 hex), with an optional `0x` prefix. Returns | ||
| * null when it cannot be resolved. | ||
| */ | ||
| export const resolveEip712AddressToAccountHash = (rawValue: string): Maybe<string> => { | ||
| const value = stripHexPrefix(String(rawValue)); | ||
| const isPublicKey = | ||
| (value.startsWith('01') && value.length === 66) || | ||
| (value.startsWith('02') && value.length === 68); | ||
| return getHashByType(value, isPublicKey ? 'publicKey' : 'accountHash'); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The docstring says "Returns null when it cannot be resolved", but this branch can't return null for garbage: So an ETH-style 20-byte Suggest validating so a bad value degrades per-row instead of per-request ( const ACCOUNT_HASH_REGEX = /^[\da-fA-F]{64}$/;
...
if (isPublicKey) {
return getHashByType(value, 'publicKey');
}
return ACCOUNT_HASH_REGEX.test(value) ? value : null;Plus tests: 40-hex ETH address → null, non-hex string → null, and the secp256k1 ( |
||
| }; | ||
|
|
||
| /** | ||
| * Account hashes to resolve for a typed-data request: every `address`-typed field value across the | ||
| * domain and the primary-type message (resolved to account hashes), plus the signing key. | ||
| * Deduplicated; null/empty results dropped. | ||
| */ | ||
| export const getAccountHashesFromTypedData = ( | ||
| typedData: IEIP712TypedData, | ||
| signingPublicKeyHex: string, | ||
| ): string[] => { | ||
| const hashes: Array<Maybe<string>> = []; | ||
|
|
||
| const collect = (fields: IEIP712Field[] | undefined, source: Record<string, unknown>) => { | ||
| (fields ?? []).forEach(({ name, type }) => { | ||
| const value = source[name]; | ||
| if (type === 'address' && typeof value === 'string' && value) { | ||
| hashes.push(resolveEip712AddressToAccountHash(value)); | ||
| } | ||
| }); | ||
| }; | ||
|
|
||
| collect(typedData.types.EIP712Domain, typedData.domain); | ||
| collect(typedData.types[typedData.primaryType], typedData.message); | ||
| hashes.push(getHashByType(signingPublicKeyHex, 'publicKey')); | ||
|
|
||
| return Array.from(new Set(hashes.filter((h): h is string => Boolean(h)))); | ||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| export * from './common'; | ||
| export * from './EIP712SignatureRequestDto'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DTO construction runs outside any error wrapping in
prepareSignatureRequest, andJSON.stringifythrows a rawTypeErroron circular references — which can survivecomputeDigestwhen they live in parts of the payloadhashTypedDatanever visits (extra keys, unused entries intypes). That leaks an unclassified error out of a method whose documented contract is "throwsEIP712Error", and callers discriminate withisEIP712Error.Suggest wrapping the DTO construction (or the stringify) so it's surfaced as
EIP712Errorlike the digest path.