Skip to content

Commit

Permalink
fix: CIP-64 are sent as EIP-1559 transactions (wevm#1499)
Browse files Browse the repository at this point in the history
* fix: CIP-64 are sent as EIP-1559 transactions (#9)

* fix: CIP-64 are sent as EIP-1559 transactions
* fix: EIP-1159 -> EIP-1559 typo
* chore: add test case for CIP-64 recognition when eip1559 type is provided
* chore: introduce a sendTransaction Celo test to verify CIP-64 being sent instead of EIP-1559
* chore: refactor and simplify sendTransaction test
* chore: DRY
* chore: add a comment for CIP-64 check

* chore: add changeset
  • Loading branch information
shazarre authored Nov 20, 2023
1 parent 9871061 commit 115d579
Show file tree
Hide file tree
Showing 6 changed files with 163 additions and 22 deletions.
5 changes: 5 additions & 0 deletions .changeset/friendly-numbers-rest.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"viem": patch
---

Fixes an issue where CIP-64 are sent as EIP-1559 transactions
99 changes: 99 additions & 0 deletions src/chains/celo/sendTransaction.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
import { describe, expect, test, vi } from 'vitest'
import { accounts } from '~test/src/constants.js'
import { privateKeyToAccount } from '~viem/accounts/privateKeyToAccount.js'
import {
type EIP1193RequestFn,
type PublicRpcSchema,
type WalletRpcSchema,
createTransport,
createWalletClient,
} from '../../index.js'
import { celo } from '../index.js'

describe('sendTransaction()', () => {
// 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_maxPriorityFeePerGas') {
return 1n
}

if (request.method === 'eth_estimateGas') {
return 1n
}

if (request.method === 'eth_getTransactionCount') {
return 0
}

if (request.method === 'eth_sendRawTransaction') {
return transactionHash
}

return null
}) as EIP1193RequestFn<WalletRpcSchema & PublicRpcSchema>

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,
feeCurrency: feeCurrencyAddress,
maxFeePerGas: 123n,
maxPriorityFeePerGas: 123n,
})

expect(hash).toEqual(transactionHash)
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 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',
],
})
})
})
30 changes: 15 additions & 15 deletions src/chains/celo/serializers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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', () => {
Expand Down
2 changes: 1 addition & 1 deletion src/chains/celo/serializers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.',
)
Expand Down
30 changes: 27 additions & 3 deletions src/chains/celo/utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -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)

Expand Down
19 changes: 16 additions & 3 deletions src/chains/celo/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) &&
Expand All @@ -56,8 +58,19 @@ 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
if (transaction.type) return transaction.type === '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
}

return (
isEIP1559(transaction) &&
Expand Down

0 comments on commit 115d579

Please sign in to comment.