fixes max fee per gas calculation (#350)

### Description we were missing the fact that base fee or gas price should be multipled by some buffer and that priority fee should be added to these when setting max fee per gas. ### Other changes setup tests with realistic values obtained by making rpc call with cast ### Tested see the tests ### Related issues - Fixes #227 ### Backwards compatibility yes ### Documentation n/a  --- ## PR-Codex overview This PR focuses on fixing the calculation of `maxFeePerGas` in the `Connection` class, ensuring it includes a buffer based on the `baseFeePerGas`. It also updates tests to reflect the new logic. ### Detailed summary - Updated `maxFeePerGas` calculation to include a buffer. - Introduced `addBufferToBaseFee` function to apply the buffer. - Modified logic to determine `baseFee` based on `feeCurrency`. - Adjusted tests for `setFeeMarketGas` to validate new calculations. > ✨ Ask PR-Codex anything about this PR by commenting with `/codex {your question}`
celo-org · Sep 30, 2024 · 433b70e · 433b70e
1 parent 76d09b7
commit 433b70e
Show file tree

Hide file tree

Showing 4 changed files with 49 additions and 34 deletions.
diff --git a/.changeset/nice-apricots-confess.md b/.changeset/nice-apricots-confess.md
@@ -0,0 +1,5 @@
+---
+'@celo/connect': patch
+---
+
+Fix calculation of maxFeePerGas. Multiple base Fee by constant for buffer
diff --git a/docs/sdk/connect/classes/connection.Connection.md b/docs/sdk/connect/classes/connection.Connection.md
@@ -266,7 +266,7 @@ ___
 
 #### Defined in
 
-[packages/sdk/connect/src/connection.ts:407](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/connect/src/connection.ts#L407)
+[packages/sdk/connect/src/connection.ts:405](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/connect/src/connection.ts#L405)
 
 ___
 
@@ -280,7 +280,7 @@ ___
 
 #### Defined in
 
-[packages/sdk/connect/src/connection.ts:429](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/connect/src/connection.ts#L429)
+[packages/sdk/connect/src/connection.ts:427](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/connect/src/connection.ts#L427)
 
 ___
 
@@ -302,7 +302,7 @@ ___
 
 #### Defined in
 
-[packages/sdk/connect/src/connection.ts:359](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/connect/src/connection.ts#L359)
+[packages/sdk/connect/src/connection.ts:357](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/connect/src/connection.ts#L357)
 
 ___
 
@@ -324,7 +324,7 @@ ___
 
 #### Defined in
 
-[packages/sdk/connect/src/connection.ts:390](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/connect/src/connection.ts#L390)
+[packages/sdk/connect/src/connection.ts:388](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/connect/src/connection.ts#L388)
 
 ___
 
@@ -344,7 +344,7 @@ ___
 
 #### Defined in
 
-[packages/sdk/connect/src/connection.ts:435](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/connect/src/connection.ts#L435)
+[packages/sdk/connect/src/connection.ts:433](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/connect/src/connection.ts#L433)
 
 ___
 
@@ -358,7 +358,7 @@ ___
 
 #### Defined in
 
-[packages/sdk/connect/src/connection.ts:386](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/connect/src/connection.ts#L386)
+[packages/sdk/connect/src/connection.ts:384](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/connect/src/connection.ts#L384)
 
 ___
 
@@ -393,7 +393,7 @@ ___
 
 #### Defined in
 
-[packages/sdk/connect/src/connection.ts:485](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/connect/src/connection.ts#L485)
+[packages/sdk/connect/src/connection.ts:483](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/connect/src/connection.ts#L483)
 
 ___
 
@@ -414,7 +414,7 @@ ___
 
 #### Defined in
 
-[packages/sdk/connect/src/connection.ts:460](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/connect/src/connection.ts#L460)
+[packages/sdk/connect/src/connection.ts:458](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/connect/src/connection.ts#L458)
 
 ___
 
@@ -434,7 +434,7 @@ ___
 
 #### Defined in
 
-[packages/sdk/connect/src/connection.ts:473](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/connect/src/connection.ts#L473)
+[packages/sdk/connect/src/connection.ts:471](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/connect/src/connection.ts#L471)
 
 ___
 
@@ -448,7 +448,7 @@ ___
 
 #### Defined in
 
-[packages/sdk/connect/src/connection.ts:451](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/connect/src/connection.ts#L451)
+[packages/sdk/connect/src/connection.ts:449](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/connect/src/connection.ts#L449)
 
 ___
 
@@ -482,7 +482,7 @@ ___
 
 #### Defined in
 
-[packages/sdk/connect/src/connection.ts:444](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/connect/src/connection.ts#L444)
+[packages/sdk/connect/src/connection.ts:442](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/connect/src/connection.ts#L442)
 
 ___
 
@@ -516,7 +516,7 @@ ___
 
 #### Defined in
 
-[packages/sdk/connect/src/connection.ts:494](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/connect/src/connection.ts#L494)
+[packages/sdk/connect/src/connection.ts:492](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/connect/src/connection.ts#L492)
 
 ___
 
@@ -536,7 +536,7 @@ ___
 
 #### Defined in
 
-[packages/sdk/connect/src/connection.ts:418](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/connect/src/connection.ts#L418)
+[packages/sdk/connect/src/connection.ts:416](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/connect/src/connection.ts#L416)
 
 ___
 
@@ -556,7 +556,7 @@ ___
 
 #### Defined in
 
-[packages/sdk/connect/src/connection.ts:502](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/connect/src/connection.ts#L502)
+[packages/sdk/connect/src/connection.ts:500](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/connect/src/connection.ts#L500)
 
 ___
 
@@ -664,7 +664,7 @@ ___
 
 #### Defined in
 
-[packages/sdk/connect/src/connection.ts:425](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/connect/src/connection.ts#L425)
+[packages/sdk/connect/src/connection.ts:423](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/connect/src/connection.ts#L423)
 
 ___
 
@@ -849,4 +849,4 @@ ___
 
 #### Defined in
 
-[packages/sdk/connect/src/connection.ts:527](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/connect/src/connection.ts#L527)
+[packages/sdk/connect/src/connection.ts:525](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/connect/src/connection.ts#L525)
diff --git a/packages/sdk/connect/src/connection.test.ts b/packages/sdk/connect/src/connection.test.ts
@@ -1,3 +1,4 @@
+import { ensureLeading0x } from '@celo/base'
 import Web3 from 'web3'
 import { Connection } from './connection'
 
@@ -37,25 +38,29 @@ describe('Connection', () => {
     })
 
     describe('when fee market gas is not set', () => {
+      const ETH_GAS_PRICE = 25001000000
+      const BASE_FEE_PER = 25000000000
+      const PRIORITYFEE = 200000
+      const multiple = BigInt(120)
       beforeEach(() => {
         connection.rpcCaller.call = jest.fn(async (method) => {
           if (method === 'eth_gasPrice') {
             return {
-              result: '300000',
+              result: ensureLeading0x(ETH_GAS_PRICE.toString(16)),
               id: 22,
               jsonrpc: '2.0',
             }
           }
           if (method === 'eth_maxPriorityFeePerGas') {
             return {
-              result: '200000',
+              result: ensureLeading0x(PRIORITYFEE.toString(16)),
               id: 23,
               jsonrpc: '2.0',
             }
           }
           if (method === 'eth_getBlockByNumber') {
             return {
-              result: { gasLimit: 0, baseFeePerGas: 42 },
+              result: { gasLimit: 30000000, baseFeePerGas: BASE_FEE_PER },
               id: 24,
               jsonrpc: '2.0',
             }
@@ -67,24 +72,29 @@ describe('Connection', () => {
           }
         })
       })
-      it('asked the rpc what they should be w/ feeCurrency', async () => {
+      it('asked the rpc what they should be with feeCurrency', async () => {
         const result = await connection.setFeeMarketGas({ feeCurrency: '0x000001' })
         expect(connection.rpcCaller.call).toHaveBeenCalledWith('eth_maxPriorityFeePerGas', [
           '0x000001',
         ])
         expect(connection.rpcCaller.call).toHaveBeenCalledWith('eth_gasPrice', ['0x000001'])
-        expect(result.maxFeePerGas).toEqual('300000')
-        expect(result.maxPriorityFeePerGas).toEqual('200000')
+
+        expect(BigInt(result.maxPriorityFeePerGas as string)).toEqual(BigInt(PRIORITYFEE))
+        expect(BigInt(result.maxFeePerGas as string)).toEqual(
+          (BigInt(ETH_GAS_PRICE) * multiple) / BigInt(100) + BigInt(PRIORITYFEE)
+        )
       })
-      it('asked the rpc what they should be w/o feeCurrency', async () => {
+      it('asked the rpc what they should be without feeCurrency', async () => {
         const result = await connection.setFeeMarketGas({})
         expect(connection.rpcCaller.call).toHaveBeenCalledWith('eth_maxPriorityFeePerGas', [])
         expect(connection.rpcCaller.call).toHaveBeenCalledWith('eth_getBlockByNumber', [
           'latest',
           true,
         ])
-        expect(result.maxFeePerGas).toEqual(`0x${BigInt('200042').toString(16)}`)
-        expect(result.maxPriorityFeePerGas).toEqual('200000')
+        expect(BigInt(result.maxPriorityFeePerGas as string)).toEqual(BigInt(PRIORITYFEE))
+        expect(BigInt(result.maxFeePerGas as string)).toEqual(
+          (BigInt(BASE_FEE_PER) * multiple) / BigInt(100) + BigInt(PRIORITYFEE)
+        )
       })
     })
   })

diff --git a/packages/sdk/connect/src/connection.ts b/packages/sdk/connect/src/connection.ts
@@ -339,15 +339,13 @@ export class Connection {
     }
 
     if (isEmpty(tx.maxFeePerGas)) {
-      if (isEmpty(tx.feeCurrency)) {
-        const baseFee = await this.getBlock('latest').then((block: any) => block.baseFeePerGas)
-        tx.maxFeePerGas = `0x${(
-          BigInt(baseFee as string) + BigInt(tx.maxPriorityFeePerGas as string)
-        ).toString(16)}`
-      } else {
-        // NOTE: fallback to original estimation
-        tx.maxFeePerGas = await this.gasPrice(tx.feeCurrency)
-      }
+      const baseFee = isEmpty(tx.feeCurrency)
+        ? await this.getBlock('latest').then((block) => block.baseFeePerGas)
+        : await this.gasPrice(tx.feeCurrency)
+      const withBuffer = addBufferToBaseFee(BigInt(baseFee!))
+      const maxFeePerGas =
+        withBuffer + BigInt(ensureLeading0x(tx.maxPriorityFeePerGas.toString(16)))
+      tx.maxFeePerGas = ensureLeading0x(maxFeePerGas.toString(16))
     }
 
     return {
@@ -530,6 +528,8 @@ export class Connection {
   }
 }
 
+const addBufferToBaseFee = (gasPrice: bigint) => (gasPrice * BigInt(120)) / BigInt(100)
+
 function isEmpty(value: string | undefined | number | BN | bigint): value is undefined {
   return (
     value === 0 ||