From 917461c601e2814c94a3770093af7adce96df5c3 Mon Sep 17 00:00:00 2001 From: Piers Powlesland Date: Mon, 14 Oct 2024 15:55:56 +0100 Subject: [PATCH 1/5] Fix the calculation for estimated gas price The gas price returned by the rpc is base_fee + max_priority_fee. So we shouldn't add the max_priority_fee onto that value since it's already included. Also the logic used by viem for applying the multiplier is to only apply the multiplier to the base fee and not the priority fee, so we match that approach here. --- src/celo/fees.test.ts | 79 +++++++++++++++++++++++++++++--- src/celo/fees.ts | 36 +++++++++++++-- src/celo/sendTransaction.test.ts | 2 +- 3 files changed, 105 insertions(+), 12 deletions(-) diff --git a/src/celo/fees.test.ts b/src/celo/fees.test.ts index 2a4f8b0b5e..c186142b3f 100644 --- a/src/celo/fees.test.ts +++ b/src/celo/fees.test.ts @@ -26,17 +26,29 @@ describe('celo/fees', () => { expect(requestMock).not.toHaveBeenCalled() }) - test('calls the client when feeCurrency is provided', async () => { + test('calls the client when feeCurrency is provided celo L1', async () => { const requestMock = vi.spyOn(client, 'request') + + const baseFee = 15057755162n + const priorityFee = 602286n + + expect(celo.fees.estimateFeesPerGas).toBeTypeOf('function') + + // The check to determine if the chain is L1 or L2 is done by checking to + // see if there is code at the proxy admin address (by calling + // eth_getCode), if there is code then the chain is considered to be L2. A + // response of '0x' as used here is what is returned when there is no code + // at the address. + // @ts-ignore requestMock.mockImplementation((request) => { - if (request.method === 'eth_gasPrice') return '15057755162' - if (request.method === 'eth_maxPriorityFeePerGas') return '602286' + if (request.method === 'eth_gasPrice') return baseFee.toString() + if (request.method === 'eth_maxPriorityFeePerGas') + return priorityFee.toString() + if (request.method === 'eth_getCode') return '0x' return }) - expect(celo.fees.estimateFeesPerGas).toBeTypeOf('function') - const fees = await celoestimateFeesPerGasFn({ client, multiply: (value: bigint) => (value * 150n) / 100n, @@ -45,10 +57,63 @@ describe('celo/fees', () => { }, } as any) + // For celo L1 the fees maxFeePerGas is calculated as `baseFee + maxPriorityFeePerGas`, the multiply method is not used. expect(fees).toMatchInlineSnapshot(` { - "maxFeePerGas": 22587235029n, - "maxPriorityFeePerGas": 602286n, + "maxFeePerGas": ${baseFee + priorityFee}n, + "maxPriorityFeePerGas": ${priorityFee}n, + } + `) + expect(requestMock).toHaveBeenCalledWith({ + method: 'eth_maxPriorityFeePerGas', + params: ['0xfee'], + }) + expect(requestMock).toHaveBeenCalledWith({ + method: 'eth_gasPrice', + params: ['0xfee'], + }) + }) + + test('calls the client when feeCurrency is provided celo L2', async () => { + const requestMock = vi.spyOn(client, 'request') + + const baseFee = 15057755162n + const priorityFee = 602286n + + expect(celo.fees.estimateFeesPerGas).toBeTypeOf('function') + + // The check to determine if the chain is L1 or L2 is done by checking to + // see if there is code at the proxy admin address (by calling + // eth_getCode), if there is code then the chain is considered to be L2. A + // response longer than '0x' as used here is what is returned when there is + // code at the address. + + // @ts-ignore + requestMock.mockImplementation((request) => { + if (request.method === 'eth_gasPrice') + return (baseFee + priorityFee).toString() + if (request.method === 'eth_maxPriorityFeePerGas') + return priorityFee.toString() + if (request.method === 'eth_getCode') return '0x00400400404040404040404' + return + }) + + const multiply = (value: bigint) => (value * 150n) / 100n + const feesCeloL1 = await celoestimateFeesPerGasFn({ + client, + multiply: multiply, + request: { + feeCurrency: '0xfee', + }, + } as any) + + // For Celo L2 the fees maxFeePerGas is calculated as the following where + // multiply is the method passed to celoestimateFeesPerGasFn: + // `multiply(baseFeePerGas - maxPriorityFeePerGas) + maxPriorityFeePerGas`. + expect(feesCeloL1).toMatchInlineSnapshot(` + { + "maxFeePerGas": ${multiply(baseFee) + priorityFee}n, + "maxPriorityFeePerGas": ${priorityFee}n, } `) expect(requestMock).toHaveBeenCalledWith({ diff --git a/src/celo/fees.ts b/src/celo/fees.ts index e6334247c4..1cd8dc09f2 100644 --- a/src/celo/fees.ts +++ b/src/celo/fees.ts @@ -7,6 +7,27 @@ import type { } from '../index.js' import type { formatters } from './formatters.js' +type RequestGetCodeParams = { + Method: 'eth_getCode' + Parameters: [Address, 'latest'] + ReturnType: Hex +} +/* + * This checks if we're in L2 context, it's a port of the technique used in + * https://github.com/celo-org/celo-monorepo/blob/da9b4955c1fdc8631980dc4adf9b05e0524fc228/packages/protocol/contracts-0.8/common/IsL2Check.sol#L17 + */ +const isCel2 = async (client: Client) => { + const proxyAdminAddress = '0x4200000000000000000000000000000000000018' + const code = await client.request({ + method: 'eth_getCode', + params: [proxyAdminAddress, 'latest'], + }) + if (typeof code === 'string') { + return code !== '0x' && code.length > 2 + } + return false +} + export const fees: ChainFees = { /* * Estimates the fees per gas for a transaction. @@ -22,7 +43,7 @@ export const fees: ChainFees = { ) => { if (!params.request?.feeCurrency) return null - const [maxFeePerGas, maxPriorityFeePerGas] = await Promise.all([ + const [gasPrice, maxPriorityFeePerGas] = await Promise.all([ estimateFeePerGasInFeeCurrency(params.client, params.request.feeCurrency), estimateMaxPriorityFeePerGasInFeeCurrency( params.client, @@ -30,11 +51,18 @@ export const fees: ChainFees = { ), ]) - const suggestedMaxFeePerGas = - params.multiply(maxFeePerGas) + maxPriorityFeePerGas + let maxFeePerGas: bigint; + if (await isCel2(params.client)) { + // eth_gasPrice for cel2 returns baseFeePerGas + maxPriorityFeePerGas + maxFeePerGas = + params.multiply(gasPrice - maxPriorityFeePerGas) + maxPriorityFeePerGas + } else { + // eth_gasPrice for Celo L1 returns (baseFeePerGas * multiplier), where the multiplier is 2 by default. + maxFeePerGas = gasPrice + maxPriorityFeePerGas + } return { - maxFeePerGas: suggestedMaxFeePerGas, + maxFeePerGas: maxFeePerGas, maxPriorityFeePerGas, } }, diff --git a/src/celo/sendTransaction.test.ts b/src/celo/sendTransaction.test.ts index 1d6a9cac59..90a34d7822 100644 --- a/src/celo/sendTransaction.test.ts +++ b/src/celo/sendTransaction.test.ts @@ -99,7 +99,7 @@ describe('sendTransaction()', () => { expect(transportRequestMock).toHaveBeenLastCalledWith({ method: 'eth_sendRawTransaction', params: [ - '0x7bf87f82a4ec80830930ae8504350cec000194f39fd6e51aad88f6f4ce6ab8827279cfffb922660180c0940000000000000000000000000000000000000fee80a0b61a83b7fe73e24f223f563447cdb69f6dedc7f7f7b2acc4e41f2e57143ccd57a03ce0bcc81c026ff0eb17274940748e23250fe988f71370eba1e59e43557835b3', + '0x7bf87f82a4ec80830930ae8503818c4cc80194f39fd6e51aad88f6f4ce6ab8827279cfffb922660180c0940000000000000000000000000000000000000fee01a07dbc07e3ed73e889b085f29f2b4a1e794d578307199b5a677071a204328837a0a02da0ba218d0ab8164010f3c0efa7f84f4df6d879a242dc5110248e7014df75dc', ], }) }) From 47d32825c96040d6bfded4db2534134c6e47b6c7 Mon Sep 17 00:00:00 2001 From: Piers Powlesland Date: Thu, 12 Dec 2024 12:56:50 +0000 Subject: [PATCH 2/5] Add changeset --- .changeset/serious-yaks-deny.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/serious-yaks-deny.md diff --git a/.changeset/serious-yaks-deny.md b/.changeset/serious-yaks-deny.md new file mode 100644 index 0000000000..0423383e55 --- /dev/null +++ b/.changeset/serious-yaks-deny.md @@ -0,0 +1,5 @@ +--- +"viem": minor +--- + +CELO: Support gas price estimation for both Celo L1 and Celo L2 From c1a3901bef5cf759a96dc12cc893b787f95fdf27 Mon Sep 17 00:00:00 2001 From: jxom <7336481+jxom@users.noreply.github.com> Date: Thu, 9 Jan 2025 07:50:18 +1100 Subject: [PATCH 3/5] chore: up lock --- pnpm-lock.yaml | 33 +++++++++++++++++++++++++++++---- 1 file changed, 29 insertions(+), 4 deletions(-) diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 2f58ee6358..77ce53250c 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -56,7 +56,7 @@ importers: version: 20.16.12 '@vitest/coverage-v8': specifier: ^1.0.4 - version: 1.0.4(vitest@1.0.4(@types/node@20.16.12)(@vitest/ui@1.0.4)(terser@5.36.0)) + version: 1.0.4(vitest@1.0.4) '@vitest/ui': specifier: ^1.0.4 version: 1.0.4(vitest@1.0.4) @@ -2642,7 +2642,6 @@ packages: bun@1.1.30: resolution: {integrity: sha512-ysRL1pq10Xba0jqVLPrKU3YIv0ohfp3cTajCPtpjCyppbn3lfiAVNpGoHfyaxS17OlPmWmR67UZRPw/EueQuug==} - cpu: [arm64, x64] os: [darwin, linux, win32] hasBin: true @@ -5434,6 +5433,14 @@ packages: typescript: optional: true + viem@2.22.4: + resolution: {integrity: sha512-35/T2ySpUM0kUVEy2SkaWIAIabSRzF/IZxDdnLxuaI1pwURC92ZiHa9J9nL1rWGl0HTyAqUxcycgJEpOvHrcQA==} + peerDependencies: + typescript: '>=5.0.4' + peerDependenciesMeta: + typescript: + optional: true + viem@file:src: resolution: {directory: src, type: directory} peerDependencies: @@ -6690,7 +6697,7 @@ snapshots: pino-http: 8.6.1 pino-pretty: 10.3.1 prom-client: 14.2.0 - viem: 2.22.3(typescript@5.6.2)(zod@3.23.8) + viem: 2.22.4(typescript@5.6.2)(zod@3.23.8) yargs: 17.7.2 zod: 3.23.8 zod-validation-error: 1.5.0(zod@3.23.8) @@ -7573,7 +7580,7 @@ snapshots: transitivePeerDependencies: - supports-color - '@vitest/coverage-v8@1.0.4(vitest@1.0.4(@types/node@20.16.12)(@vitest/ui@1.0.4)(terser@5.36.0))': + '@vitest/coverage-v8@1.0.4(vitest@1.0.4)': dependencies: '@ampproject/remapping': 2.3.0 '@bcoe/v8-coverage': 0.2.3 @@ -11295,6 +11302,24 @@ snapshots: - utf-8-validate - zod + viem@2.22.4(typescript@5.6.2)(zod@3.23.8): + dependencies: + '@noble/curves': 1.7.0 + '@noble/hashes': 1.6.1 + '@scure/bip32': 1.6.0 + '@scure/bip39': 1.5.0 + abitype: 1.0.7(typescript@5.6.2)(zod@3.23.8) + isows: 1.0.6(ws@8.18.0) + ox: 0.6.0(typescript@5.6.2)(zod@3.23.8) + webauthn-p256: 0.0.10 + ws: 8.18.0 + optionalDependencies: + typescript: 5.6.2 + transitivePeerDependencies: + - bufferutil + - utf-8-validate + - zod + viem@file:src(typescript@5.6.2)(zod@3.23.8): dependencies: '@noble/curves': 1.7.0 From b7163c28ba9d34a86b532cd055928e60f7a105a2 Mon Sep 17 00:00:00 2001 From: jxom <7336481+jxom@users.noreply.github.com> Date: Thu, 9 Jan 2025 07:57:25 +1100 Subject: [PATCH 4/5] chore: clean --- src/celo/fees.ts | 45 ++++++++++++++------------------------------- 1 file changed, 14 insertions(+), 31 deletions(-) diff --git a/src/celo/fees.ts b/src/celo/fees.ts index 1cd8dc09f2..282892d49b 100644 --- a/src/celo/fees.ts +++ b/src/celo/fees.ts @@ -1,3 +1,4 @@ +import { getCode } from '../actions/public/getCode.js' import type { Client } from '../clients/createClient.js' import type { Address, @@ -7,27 +8,6 @@ import type { } from '../index.js' import type { formatters } from './formatters.js' -type RequestGetCodeParams = { - Method: 'eth_getCode' - Parameters: [Address, 'latest'] - ReturnType: Hex -} -/* - * This checks if we're in L2 context, it's a port of the technique used in - * https://github.com/celo-org/celo-monorepo/blob/da9b4955c1fdc8631980dc4adf9b05e0524fc228/packages/protocol/contracts-0.8/common/IsL2Check.sol#L17 - */ -const isCel2 = async (client: Client) => { - const proxyAdminAddress = '0x4200000000000000000000000000000000000018' - const code = await client.request({ - method: 'eth_getCode', - params: [proxyAdminAddress, 'latest'], - }) - if (typeof code === 'string') { - return code !== '0x' && code.length > 2 - } - return false -} - export const fees: ChainFees = { /* * Estimates the fees per gas for a transaction. @@ -43,26 +23,23 @@ export const fees: ChainFees = { ) => { if (!params.request?.feeCurrency) return null - const [gasPrice, maxPriorityFeePerGas] = await Promise.all([ + const [gasPrice, maxPriorityFeePerGas, cel2] = await Promise.all([ estimateFeePerGasInFeeCurrency(params.client, params.request.feeCurrency), estimateMaxPriorityFeePerGasInFeeCurrency( params.client, params.request.feeCurrency, ), + isCel2(params.client), ]) - let maxFeePerGas: bigint; - if (await isCel2(params.client)) { - // eth_gasPrice for cel2 returns baseFeePerGas + maxPriorityFeePerGas - maxFeePerGas = + const maxFeePerGas = cel2 + ? // eth_gasPrice for cel2 returns baseFeePerGas + maxPriorityFeePerGas params.multiply(gasPrice - maxPriorityFeePerGas) + maxPriorityFeePerGas - } else { - // eth_gasPrice for Celo L1 returns (baseFeePerGas * multiplier), where the multiplier is 2 by default. - maxFeePerGas = gasPrice + maxPriorityFeePerGas - } + : // eth_gasPrice for Celo L1 returns (baseFeePerGas * multiplier), where the multiplier is 2 by default. + gasPrice + maxPriorityFeePerGas return { - maxFeePerGas: maxFeePerGas, + maxFeePerGas, maxPriorityFeePerGas, } }, @@ -120,3 +97,9 @@ async function estimateMaxPriorityFeePerGasInFeeCurrency( }) return BigInt(feesPerGas) } + +async function isCel2(client: Client) { + const proxyAdminAddress = '0x4200000000000000000000000000000000000018' + const code = await getCode(client, { address: proxyAdminAddress }) + return Boolean(code) +} From 7249a73c5756339a3d71e2f29f1abdac325ebee0 Mon Sep 17 00:00:00 2001 From: jxom <7336481+jxom@users.noreply.github.com> Date: Thu, 9 Jan 2025 07:58:17 +1100 Subject: [PATCH 5/5] Update serious-yaks-deny.md --- .changeset/serious-yaks-deny.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.changeset/serious-yaks-deny.md b/.changeset/serious-yaks-deny.md index 0423383e55..e31a336d2f 100644 --- a/.changeset/serious-yaks-deny.md +++ b/.changeset/serious-yaks-deny.md @@ -1,5 +1,5 @@ --- -"viem": minor +"viem": patch --- -CELO: Support gas price estimation for both Celo L1 and Celo L2 +**Celo:** Added support for gas price estimation on both Celo L1 and Celo L2.