From 44d0b5fbfbc93d0c465dd789868aecace52cb607 Mon Sep 17 00:00:00 2001 From: Leszek Stachowski Date: Thu, 9 Nov 2023 17:22:20 +0100 Subject: [PATCH 1/7] fix: CIP-64 are sent as EIP-1559 transactions --- src/chains/celo/serializers.test.ts | 30 ++++++++++++++--------------- src/chains/celo/utils.ts | 8 ++++++-- 2 files changed, 21 insertions(+), 17 deletions(-) diff --git a/src/chains/celo/serializers.test.ts b/src/chains/celo/serializers.test.ts index 26dfb2d494..e55c39c3d4 100644 --- a/src/chains/celo/serializers.test.ts +++ b/src/chains/celo/serializers.test.ts @@ -130,21 +130,6 @@ describe('cip42', () => { ) }) - test('args: gatewayFee (absent)', () => { - const transaction: TransactionSerializableCIP42 = { - ...baseCip42, - gatewayFeeRecipient: undefined, - gatewayFee: undefined, - type: 'cip42', - } - expect(parseTransactionCelo(serializeTransactionCelo(transaction))).toEqual( - { - ...transaction, - type: 'cip42', - }, - ) - }) - test('args: maxFeePerGas (absent)', () => { const transaction: TransactionSerializableCIP42 = { ...baseCip42, @@ -475,6 +460,21 @@ describe('cip64', () => { expect(parseTransactionCelo(tx1)).toEqual(parseTransactionCelo(tx2)) expect(parseTransactionCelo(tx1)).toEqual({ ...baseCip64, type: 'cip64' }) }) + + test('CIP-42 transaction that has all CIP-64 fields and CIP-64 takes precedence', () => { + const transaction: TransactionSerializableCIP42 = { + ...baseCip42, + gatewayFeeRecipient: undefined, + gatewayFee: undefined, + type: 'cip42', + } + expect(parseTransactionCelo(serializeTransactionCelo(transaction))).toEqual( + { + ...transaction, + type: 'cip64', + }, + ) + }) }) describe('invalid params specific to CIP-42', () => { diff --git a/src/chains/celo/utils.ts b/src/chains/celo/utils.ts index 874de53272..97d82e6aa4 100644 --- a/src/chains/celo/utils.ts +++ b/src/chains/celo/utils.ts @@ -43,7 +43,9 @@ export function isCIP42( transaction: CeloTransactionSerializable | CeloTransactionRequest, ): transaction is TransactionSerializableCIP42 { // Enable end-user to force the tx to be considered as a cip42 - if (transaction.type) return transaction.type === 'cip42' + if (transaction.type === 'cip42') { + return true + } return ( isEIP1559(transaction) && @@ -57,7 +59,9 @@ export function isCIP64( transaction: CeloTransactionSerializable | CeloTransactionRequest, ): transaction is TransactionSerializableCIP64 { // Enable end-user to force the tx to be considered as a cip64 - if (transaction.type) return transaction.type === 'cip64' + if (transaction.type === 'cip64') { + return true + } return ( isEIP1559(transaction) && From 3efaec578b1175cff121aa428b0fd57ef012653c Mon Sep 17 00:00:00 2001 From: Leszek Stachowski Date: Fri, 10 Nov 2023 08:11:41 +0100 Subject: [PATCH 2/7] fix: EIP-1159 -> EIP-1559 typo --- src/chains/celo/utils.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/chains/celo/utils.test.ts b/src/chains/celo/utils.test.ts index b32bc0cc47..64618d91a3 100644 --- a/src/chains/celo/utils.test.ts +++ b/src/chains/celo/utils.test.ts @@ -74,7 +74,7 @@ describe('isPresent', () => { }) describe('isEIP1559', () => { - test('it checks if a transaction is EIP-1159', () => { + test('it checks if a transaction is EIP-1559', () => { expect(isEIP1559({})).toBe(false) expect( @@ -118,7 +118,7 @@ describe('isEIP1559', () => { }) describe('isCIP42', () => { - test('it allows forcing the type even if transaction is not EIP-1159', () => { + test('it allows forcing the type even if transaction is not EIP-1559', () => { expect( isCIP42({ type: 'cip42', @@ -203,7 +203,7 @@ describe('isCIP42', () => { }) describe('isCIP64', () => { - test('it allows forcing the type even if transaction is not EIP-1159', () => { + test('it allows forcing the type even if transaction is not EIP-1559', () => { expect( isCIP64({ type: 'cip64', From 2d9482cb26993baee5ffe87617082cce546debf8 Mon Sep 17 00:00:00 2001 From: Leszek Stachowski Date: Fri, 10 Nov 2023 08:59:58 +0100 Subject: [PATCH 3/7] chore: add test case for CIP-64 recognition when eip1559 type is provided --- src/chains/celo/utils.test.ts | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/chains/celo/utils.test.ts b/src/chains/celo/utils.test.ts index 64618d91a3..858cd6bc02 100644 --- a/src/chains/celo/utils.test.ts +++ b/src/chains/celo/utils.test.ts @@ -236,6 +236,30 @@ describe('isCIP64', () => { ).toBe(true) }) + test('it recognizes valid CIP-64 with "eip1559" type provided', () => { + expect( + isCIP64({ + type: 'eip1559', + feeCurrency: mockAddress, + gatewayFeeRecipient: '0x', + gatewayFee: 0n, + maxFeePerGas: 123n, + maxPriorityFeePerGas: 456n, + from: mockAddress, + }), + ).toBe(true) + + expect( + isCIP64({ + type: 'eip1559', + feeCurrency: mockAddress, + maxFeePerGas: 123n, + maxPriorityFeePerGas: 456n, + from: mockAddress, + }), + ).toBe(true) + }) + test('it does not recognize valid CIP-64', () => { expect(isCIP64({})).toBe(false) From a5f7206332692e6a6c733cd5afe512f76ee8dd3e Mon Sep 17 00:00:00 2001 From: Leszek Stachowski Date: Fri, 10 Nov 2023 16:18:42 +0100 Subject: [PATCH 4/7] chore: introduce a sendTransaction Celo test to verify CIP-64 being sent instead of EIP-1559 --- src/chains/celo/sendTransaction.test.ts | 82 +++++++++++++++++++++++++ src/chains/celo/utils.ts | 5 ++ 2 files changed, 87 insertions(+) create mode 100644 src/chains/celo/sendTransaction.test.ts diff --git a/src/chains/celo/sendTransaction.test.ts b/src/chains/celo/sendTransaction.test.ts new file mode 100644 index 0000000000..47f669869c --- /dev/null +++ b/src/chains/celo/sendTransaction.test.ts @@ -0,0 +1,82 @@ +import { describe, expect, test, vi } from 'vitest' +import { + createTransport, + type EIP1193RequestFn, + createWalletClient, + type WalletRpcSchema, + type PublicRpcSchema, +} from '../../index.js' +import type { CeloTransactionSerialized } from './types.js' +import { celo } from '../index.js' +import { generateRandomAddress } from './utils.js' +import { privateKeyToAccount } from '~viem/accounts/privateKeyToAccount.js' +import { accounts } from '~test/src/constants.js' +import { parseTransactionCelo } from './parsers.js' + +describe('sendTransaction', () => { + test('provides valid transaction params to sign for eth_sendRawTransaction (local account)', async () => { + let rawTransaction: CeloTransactionSerialized | undefined + + const feeCurrencyAddress = generateRandomAddress() + const transactionHash = generateRandomAddress() + const toAddress = generateRandomAddress() + + const mockTransport = () => + createTransport({ + key: 'mock', + name: 'Mock Transport', + request: vi.fn(async (request) => { + if (request.method === 'eth_chainId') { + return celo.id + } + + if (request.method === 'eth_getBlockByNumber') { + // We just need baseFeePerGas for gas estimation + return { + baseFeePerGas: '0x12a05f200', + } + } + + if (request.method === 'eth_estimateGas') { + return 1n + } + + if (request.method === 'eth_getTransactionCount') { + return 0 + } + + if (request.method === 'eth_sendRawTransaction') { + rawTransaction = ( + request.params as Array + )[0] + + return transactionHash + } + + return null + }) as EIP1193RequestFn, + type: 'mock', + }) + const client = createWalletClient({ + transport: mockTransport, + chain: celo, + // We need a local account + account: privateKeyToAccount(accounts[0].privateKey), + }) + + const hash = await client.sendTransaction({ + value: 1n, + to: toAddress, + feeCurrency: feeCurrencyAddress, + maxFeePerGas: 123n, + maxPriorityFeePerGas: 123n, + }) + + expect(hash).toEqual(transactionHash) + expect(rawTransaction).not.toBeUndefined() + + if (rawTransaction) { + expect(parseTransactionCelo(rawTransaction).type).toEqual('cip64') + } + }) +}) diff --git a/src/chains/celo/utils.ts b/src/chains/celo/utils.ts index 97d82e6aa4..295a7269b9 100644 --- a/src/chains/celo/utils.ts +++ b/src/chains/celo/utils.ts @@ -1,4 +1,5 @@ import type { Address } from 'abitype' +import { randomBytes } from 'crypto' import { trim } from '../../utils/data/trim.js' import type { CeloTransactionRequest, @@ -70,3 +71,7 @@ export function isCIP64( isEmpty(transaction.gatewayFeeRecipient) ) } + +export function generateRandomAddress(): Address { + return `0x${randomBytes(20).toString('hex')}` as Address +} From 976956df55f07b8f35177236ab023f57c1b9fbad Mon Sep 17 00:00:00 2001 From: Leszek Stachowski Date: Mon, 13 Nov 2023 16:21:07 +0100 Subject: [PATCH 5/7] chore: refactor and simplify sendTransaction test --- src/chains/celo/sendTransaction.test.ts | 163 ++++++++++++++++-------- src/chains/celo/serializers.ts | 2 +- src/chains/celo/utils.ts | 5 - 3 files changed, 114 insertions(+), 56 deletions(-) diff --git a/src/chains/celo/sendTransaction.test.ts b/src/chains/celo/sendTransaction.test.ts index 47f669869c..d1102ce615 100644 --- a/src/chains/celo/sendTransaction.test.ts +++ b/src/chains/celo/sendTransaction.test.ts @@ -1,67 +1,61 @@ import { describe, expect, test, vi } from 'vitest' +import { accounts } from '~test/src/constants.js' +import { privateKeyToAccount } from '~viem/accounts/privateKeyToAccount.js' import { - createTransport, type EIP1193RequestFn, - createWalletClient, - type WalletRpcSchema, type PublicRpcSchema, + type WalletRpcSchema, + createTransport, + createWalletClient, } from '../../index.js' -import type { CeloTransactionSerialized } from './types.js' import { celo } from '../index.js' -import { generateRandomAddress } from './utils.js' -import { privateKeyToAccount } from '~viem/accounts/privateKeyToAccount.js' -import { accounts } from '~test/src/constants.js' -import { parseTransactionCelo } from './parsers.js' -describe('sendTransaction', () => { - test('provides valid transaction params to sign for eth_sendRawTransaction (local account)', async () => { - let rawTransaction: CeloTransactionSerialized | undefined +describe('sendTransaction()', () => { + test('provides valid transaction params to sign for eth_sendRawTransaction (local account) for CIP-64', async () => { + // We need a local account + const account = privateKeyToAccount(accounts[0].privateKey) + const feeCurrencyAddress = '0x0000000000000000000000000000000000000fee' + const transactionHash = '0xtransaction-hash' + const toAddress = account.address + const transportRequestMock = vi.fn(async (request) => { + if (request.method === 'eth_chainId') { + return celo.id + } + + if (request.method === 'eth_getBlockByNumber') { + // We just need baseFeePerGas for gas estimation + return { + baseFeePerGas: '0x12a05f200', + } + } + + if (request.method === 'eth_estimateGas') { + return 1n + } - const feeCurrencyAddress = generateRandomAddress() - const transactionHash = generateRandomAddress() - const toAddress = generateRandomAddress() + if (request.method === 'eth_getTransactionCount') { + return 0 + } + + if (request.method === 'eth_sendRawTransaction') { + return transactionHash + } + + return null + }) as EIP1193RequestFn const mockTransport = () => createTransport({ key: 'mock', name: 'Mock Transport', - request: vi.fn(async (request) => { - if (request.method === 'eth_chainId') { - return celo.id - } - - if (request.method === 'eth_getBlockByNumber') { - // We just need baseFeePerGas for gas estimation - return { - baseFeePerGas: '0x12a05f200', - } - } - - if (request.method === 'eth_estimateGas') { - return 1n - } - - if (request.method === 'eth_getTransactionCount') { - return 0 - } - - if (request.method === 'eth_sendRawTransaction') { - rawTransaction = ( - request.params as Array - )[0] - - return transactionHash - } - - return null - }) as EIP1193RequestFn, + request: transportRequestMock, type: 'mock', }) + const client = createWalletClient({ transport: mockTransport, chain: celo, - // We need a local account - account: privateKeyToAccount(accounts[0].privateKey), + account, }) const hash = await client.sendTransaction({ @@ -73,10 +67,79 @@ describe('sendTransaction', () => { }) expect(hash).toEqual(transactionHash) - expect(rawTransaction).not.toBeUndefined() + expect(transportRequestMock).toHaveBeenLastCalledWith({ + method: 'eth_sendRawTransaction', + params: [ + '0x7bf87782a4ec807b7b0194f39fd6e51aad88f6f4ce6ab8827279cfffb922660180c0940000000000000000000000000000000000000fee01a038c5dfc128d40b147544b13572dbb0462b9389a8a687d0fe32973e435d7de23aa03c01d6bff1279e94f53a1244302de288bd335bc3a1e61da73fd6215f6d67ccf2', + ], + }) + }) + + test('provides valid transaction params to sign for eth_sendRawTransaction (local account) for CIP-42', async () => { + const account = privateKeyToAccount(accounts[0].privateKey) + const feeCurrencyAddress = '0x0000000000000000000000000000000000000fee' + const transactionHash = '0xtransaction-hash' + const toAddress = account.address + const transportRequestMock = vi.fn(async (request) => { + if (request.method === 'eth_chainId') { + return celo.id + } + + if (request.method === 'eth_getBlockByNumber') { + // We just need baseFeePerGas for gas estimation + return { + baseFeePerGas: '0x12a05f200', + } + } + + if (request.method === 'eth_getTransactionCount') { + return 0 + } + + if (request.method === 'eth_maxPriorityFeePerGas') { + return 1n + } + + if (request.method === 'eth_estimateGas') { + return 1n + } - if (rawTransaction) { - expect(parseTransactionCelo(rawTransaction).type).toEqual('cip64') - } + if (request.method === 'eth_sendRawTransaction') { + return transactionHash + } + + return null + }) as EIP1193RequestFn + + const mockTransport = () => + createTransport({ + key: 'mock', + name: 'Mock Transport', + request: transportRequestMock, + type: 'mock', + }) + + const client = createWalletClient({ + transport: mockTransport, + chain: celo, + // We need a local account + account, + }) + + const hash = await client.sendTransaction({ + value: 1n, + to: toAddress, + feeCurrency: feeCurrencyAddress, + gatewayFee: 123n, + gatewayFeeRecipient: '0x0000000000000000000000000000000000000001', + }) + + expect(hash).toEqual(transactionHash) + expect(transportRequestMock).toHaveBeenLastCalledWith({ + method: 'eth_sendRawTransaction', + params: [ + '0x7cf89282a4ec8001850165a0bc0101940000000000000000000000000000000000000fee9400000000000000000000000000000000000000017b94f39fd6e51aad88f6f4ce6ab8827279cfffb922660180c080a004389976320970e0227b20df6f79f2f35a2832d18b9732cb017d15db9f80fb44a0735b9abf965b7f38d1c659527cc93a9fc37b3a3b7bd5910d0c7db4b740be860f', + ], + }) }) }) diff --git a/src/chains/celo/serializers.ts b/src/chains/celo/serializers.ts index 2cc385e350..7e9c7506a8 100644 --- a/src/chains/celo/serializers.ts +++ b/src/chains/celo/serializers.ts @@ -231,7 +231,7 @@ export function assertTransactionCIP64( ) throw new TipAboveFeeCapError({ maxFeePerGas, maxPriorityFeePerGas }) - if (isPresent(feeCurrency) && !feeCurrency?.startsWith('0x')) { + if (isPresent(feeCurrency) && !isAddress(feeCurrency)) { throw new BaseError( '`feeCurrency` MUST be a token address for CIP-64 transactions.', ) diff --git a/src/chains/celo/utils.ts b/src/chains/celo/utils.ts index 295a7269b9..97d82e6aa4 100644 --- a/src/chains/celo/utils.ts +++ b/src/chains/celo/utils.ts @@ -1,5 +1,4 @@ import type { Address } from 'abitype' -import { randomBytes } from 'crypto' import { trim } from '../../utils/data/trim.js' import type { CeloTransactionRequest, @@ -71,7 +70,3 @@ export function isCIP64( isEmpty(transaction.gatewayFeeRecipient) ) } - -export function generateRandomAddress(): Address { - return `0x${randomBytes(20).toString('hex')}` as Address -} From 439fbf71fa5b9230ab0eba954bcfb8da50f310b0 Mon Sep 17 00:00:00 2001 From: Leszek Stachowski Date: Mon, 13 Nov 2023 16:43:44 +0100 Subject: [PATCH 6/7] chore: DRY --- src/chains/celo/sendTransaction.test.ts | 130 ++++++++---------------- 1 file changed, 42 insertions(+), 88 deletions(-) diff --git a/src/chains/celo/sendTransaction.test.ts b/src/chains/celo/sendTransaction.test.ts index d1102ce615..877051403b 100644 --- a/src/chains/celo/sendTransaction.test.ts +++ b/src/chains/celo/sendTransaction.test.ts @@ -11,53 +11,57 @@ import { import { celo } from '../index.js' describe('sendTransaction()', () => { - test('provides valid transaction params to sign for eth_sendRawTransaction (local account) for CIP-64', async () => { - // We need a local account - const account = privateKeyToAccount(accounts[0].privateKey) - const feeCurrencyAddress = '0x0000000000000000000000000000000000000fee' - const transactionHash = '0xtransaction-hash' - const toAddress = account.address - const transportRequestMock = vi.fn(async (request) => { - if (request.method === 'eth_chainId') { - return celo.id + // We need a local account + const account = privateKeyToAccount(accounts[0].privateKey) + const toAddress = account.address + const transactionHash = '0xtransaction-hash' + const feeCurrencyAddress = '0x0000000000000000000000000000000000000fee' + const transportRequestMock = vi.fn(async (request) => { + if (request.method === 'eth_chainId') { + return celo.id + } + + if (request.method === 'eth_getBlockByNumber') { + // We just need baseFeePerGas for gas estimation + return { + baseFeePerGas: '0x12a05f200', } + } - if (request.method === 'eth_getBlockByNumber') { - // We just need baseFeePerGas for gas estimation - return { - baseFeePerGas: '0x12a05f200', - } - } + if (request.method === 'eth_maxPriorityFeePerGas') { + return 1n + } - if (request.method === 'eth_estimateGas') { - return 1n - } + if (request.method === 'eth_estimateGas') { + return 1n + } - if (request.method === 'eth_getTransactionCount') { - return 0 - } + if (request.method === 'eth_getTransactionCount') { + return 0 + } - if (request.method === 'eth_sendRawTransaction') { - return transactionHash - } - - return null - }) as EIP1193RequestFn + if (request.method === 'eth_sendRawTransaction') { + return transactionHash + } - const mockTransport = () => - createTransport({ - key: 'mock', - name: 'Mock Transport', - request: transportRequestMock, - type: 'mock', - }) + return null + }) as EIP1193RequestFn - const client = createWalletClient({ - transport: mockTransport, - chain: celo, - account, + const mockTransport = () => + createTransport({ + key: 'mock', + name: 'Mock Transport', + request: transportRequestMock, + type: 'mock', }) + const client = createWalletClient({ + transport: mockTransport, + chain: celo, + account, + }) + + test('provides valid transaction params to sign for eth_sendRawTransaction (local account) for CIP-64', async () => { const hash = await client.sendTransaction({ value: 1n, to: toAddress, @@ -76,56 +80,6 @@ describe('sendTransaction()', () => { }) test('provides valid transaction params to sign for eth_sendRawTransaction (local account) for CIP-42', async () => { - const account = privateKeyToAccount(accounts[0].privateKey) - const feeCurrencyAddress = '0x0000000000000000000000000000000000000fee' - const transactionHash = '0xtransaction-hash' - const toAddress = account.address - const transportRequestMock = vi.fn(async (request) => { - if (request.method === 'eth_chainId') { - return celo.id - } - - if (request.method === 'eth_getBlockByNumber') { - // We just need baseFeePerGas for gas estimation - return { - baseFeePerGas: '0x12a05f200', - } - } - - if (request.method === 'eth_getTransactionCount') { - return 0 - } - - if (request.method === 'eth_maxPriorityFeePerGas') { - return 1n - } - - if (request.method === 'eth_estimateGas') { - return 1n - } - - if (request.method === 'eth_sendRawTransaction') { - return transactionHash - } - - return null - }) as EIP1193RequestFn - - const mockTransport = () => - createTransport({ - key: 'mock', - name: 'Mock Transport', - request: transportRequestMock, - type: 'mock', - }) - - const client = createWalletClient({ - transport: mockTransport, - chain: celo, - // We need a local account - account, - }) - const hash = await client.sendTransaction({ value: 1n, to: toAddress, From ee23a1d98d1009920dcb36d4f30dd3980397eed9 Mon Sep 17 00:00:00 2001 From: Leszek Stachowski Date: Wed, 15 Nov 2023 13:27:58 +0100 Subject: [PATCH 7/7] chore: add a comment for CIP-64 check --- src/chains/celo/utils.ts | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/chains/celo/utils.ts b/src/chains/celo/utils.ts index 97d82e6aa4..bd6358e811 100644 --- a/src/chains/celo/utils.ts +++ b/src/chains/celo/utils.ts @@ -58,7 +58,16 @@ export function isCIP42( export function isCIP64( transaction: CeloTransactionSerializable | CeloTransactionRequest, ): transaction is TransactionSerializableCIP64 { - // Enable end-user to force the tx to be considered as a cip64 + /* + * Enable end user to force the tx to be considered as a CIP-64. + * + * The preliminary type will be determined as "eip1559" by src/utils/transaction/getTransactionType.ts + * and so we need the logic below to check for the specific value instead of checking if just any + * transaction type is provided. If that's anything else than "cip64" then we need to reevaluate the + * type based on the transaction fields. + * + * Modify with caution and according to https://github.com/celo-org/celo-proposals/blob/master/CIPs/cip-0064.md + */ if (transaction.type === 'cip64') { return true }