Skip to content
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

feat: eip-1559 fixed gas prices #2398

Merged
merged 1 commit into from
Aug 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
"@safe-global/safe-core-sdk-utils": "^1.7.4",
"@safe-global/safe-deployments": "^1.25.0",
"@safe-global/safe-ethers-lib": "^1.9.4",
"@safe-global/safe-gateway-typescript-sdk": "^3.8.0",
"@safe-global/safe-gateway-typescript-sdk": "^3.9.0",
"@safe-global/safe-modules-deployments": "^1.0.0",
"@safe-global/safe-react-components": "^2.0.6",
"@sentry/react": "^7.28.1",
Expand Down
102 changes: 76 additions & 26 deletions src/hooks/__tests__/useGasPrice.test.ts
Original file line number Diff line number Diff line change
@@ -1,48 +1,47 @@
import { BigNumber } from 'ethers'
import { act, renderHook } from '@/tests/test-utils'
import useGasPrice from '@/hooks/useGasPrice'
import { useCurrentChain } from '../useChains'

// mock useWeb3Readonly
jest.mock('../wallets/web3', () => {
const provider = {
getFeeData: jest.fn(() =>
Promise.resolve({
gasPrice: undefined,
maxFeePerGas: BigNumber.from('0x956e'),
maxPriorityFeePerGas: BigNumber.from('0x136f'),
maxFeePerGas: BigNumber.from('0x956e'), //38254
maxPriorityFeePerGas: BigNumber.from('0x136f'), //4975
}),
),
}
return {
useWeb3ReadOnly: jest.fn(() => provider),
}
})

const currentChain = {
chainId: '4',
gasPrice: [
{
type: 'oracle',
uri: 'https://api.etherscan.io/api?module=gastracker&action=gasoracle',
gasParameter: 'FastGasPrice',
gweiFactor: '1000000000.000000000',
},
{
type: 'oracle',
uri: 'https://ethgasstation.info/json/ethgasAPI.json',
gasParameter: 'fast',
gweiFactor: '200000000.000000000',
},
{
type: 'fixed',
weiValue: '24000000000',
},
],
features: ['EIP1559'],
}
// Mock useCurrentChain
jest.mock('@/hooks/useChains', () => {
const currentChain = {
chainId: '4',
gasPrice: [
{
type: 'ORACLE',
uri: 'https://api.etherscan.io/api?module=gastracker&action=gasoracle',
gasParameter: 'FastGasPrice',
gweiFactor: '1000000000.000000000',
},
{
type: 'ORACLE',
uri: 'https://ethgasstation.info/json/ethgasAPI.json',
gasParameter: 'fast',
gweiFactor: '200000000.000000000',
},
{
type: 'FIXED',
weiValue: '24000000000',
},
],
features: ['EIP1559'],
}

return {
useCurrentChain: jest.fn(() => currentChain),
}
Expand All @@ -52,6 +51,7 @@ describe('useGasPrice', () => {
beforeEach(() => {
jest.useFakeTimers()
jest.clearAllMocks()
;(useCurrentChain as jest.Mock).mockReturnValue(currentChain)
})

it('should return the fetched gas price from the first oracle', async () => {
Expand Down Expand Up @@ -170,6 +170,56 @@ describe('useGasPrice', () => {
expect(result.current[0]?.maxPriorityFeePerGas?.toString()).toEqual('4975')
})

it('should be able to set a fixed EIP 1559 gas price', async () => {
;(useCurrentChain as jest.Mock).mockReturnValue({
chainId: '10',
gasPrice: [
{
type: 'fixed1559',
maxFeePerGas: '100000000',
maxPriorityFeePerGas: '100000',
},
],
features: ['EIP1559'],
})

const { result } = renderHook(() => useGasPrice())

await act(async () => {
await Promise.resolve()
})
// assert the hook is not loading
expect(result.current[2]).toBe(false)

// assert fixed gas price as minimum of 0.1 gwei
expect(result.current[0]?.maxFeePerGas?.toString()).toBe('100000000')

// assert fixed priority fee
expect(result.current[0]?.maxPriorityFeePerGas?.toString()).toBe('100000')
Comment on lines +188 to +198
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: You can also wrap all the expects with an await waitFor() call instead of having an empty Promise.resolve() to achieve the same

})

it("should use the previous block's fee data if there are no oracles", async () => {
;(useCurrentChain as jest.Mock).mockReturnValue({
chainId: '1',
gasPrice: [],
features: ['EIP1559'],
})

const { result } = renderHook(() => useGasPrice())

await act(async () => {
await Promise.resolve()
})
// assert the hook is not loading
expect(result.current[2]).toBe(false)

// assert gas price from provider
expect(result.current[0]?.maxFeePerGas?.toString()).toBe('38254')

// assert priority fee from provider
expect(result.current[0]?.maxPriorityFeePerGas?.toString()).toBe('4975')
})

it('should keep the previous gas price if the hook re-renders', async () => {
// Mock fetch
Object.defineProperty(window, 'fetch', {
Expand Down
4 changes: 2 additions & 2 deletions src/hooks/useDecodeTx.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ const useDecodeTx = (tx?: SafeTransaction): AsyncResult<DecodedDataResponse> =>

const [data = nativeTransfer, error, loading] = useAsync<DecodedDataResponse>(() => {
if (!encodedData || isEmptyData) return
return getDecodedData(chainId, encodedData)
}, [chainId, encodedData, isEmptyData])
return getDecodedData(chainId, encodedData, tx.data.to)
}, [chainId, encodedData, isEmptyData, tx?.data.to])

return [data, error, loading]
}
Expand Down
100 changes: 81 additions & 19 deletions src/hooks/useGasPrice.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
import { BigNumber } from 'ethers'
import type { GasPrice, GasPriceOracle } from '@safe-global/safe-gateway-typescript-sdk'
import type { FeeData } from '@ethersproject/abstract-provider'
import type {
GasPrice,
GasPriceFixed,
GasPriceFixedEIP1559,
GasPriceOracle,
} from '@safe-global/safe-gateway-typescript-sdk'
import { GAS_PRICE_TYPE } from '@safe-global/safe-gateway-typescript-sdk'
import useAsync, { type AsyncResult } from '@/hooks/useAsync'
import { useCurrentChain } from './useChains'
Expand All @@ -9,6 +15,20 @@ import { Errors, logError } from '@/services/exceptions'
import { FEATURES, hasFeature } from '@/utils/chains'
import { asError } from '@/services/exceptions/utils'

type EstimatedGasPrice =
| {
gasPrice: BigNumber
}
| {
maxFeePerGas: BigNumber
maxPriorityFeePerGas: BigNumber
}

type GasFeeParams = {
maxFeePerGas: BigNumber | null | undefined
maxPriorityFeePerGas: BigNumber | null | undefined
}

// Update gas fees every 20 seconds
const REFRESH_DELAY = 20e3

Expand All @@ -27,17 +47,40 @@ const fetchGasOracle = async (gasPriceOracle: GasPriceOracle): Promise<BigNumber
return BigNumber.from(data[gasParameter] * Number(gweiFactor))
}

const getGasPrice = async (gasPriceConfigs: GasPrice): Promise<BigNumber | undefined> => {
let error: Error | undefined
// These typeguards are necessary because the GAS_PRICE_TYPE enum uses uppercase while the config service uses lowercase values
const isGasPriceFixed = (gasPriceConfig: GasPrice[number]): gasPriceConfig is GasPriceFixed => {
return gasPriceConfig.type.toUpperCase() == GAS_PRICE_TYPE.FIXED
}

const isGasPriceFixed1559 = (gasPriceConfig: GasPrice[number]): gasPriceConfig is GasPriceFixedEIP1559 => {
return gasPriceConfig.type.toUpperCase() == GAS_PRICE_TYPE.FIXED_1559
}

const isGasPriceOracle = (gasPriceConfig: GasPrice[number]): gasPriceConfig is GasPriceOracle => {
return gasPriceConfig.type.toUpperCase() == GAS_PRICE_TYPE.ORACLE
}

const getGasPrice = async (gasPriceConfigs: GasPrice): Promise<EstimatedGasPrice | undefined> => {
let error: Error | undefined
for (const config of gasPriceConfigs) {
if (config.type == GAS_PRICE_TYPE.FIXED) {
return BigNumber.from(config.weiValue)
if (isGasPriceFixed(config)) {
return {
gasPrice: BigNumber.from(config.weiValue),
}
}

if (config.type == GAS_PRICE_TYPE.ORACLE) {
if (isGasPriceFixed1559(config)) {
return {
maxFeePerGas: BigNumber.from(config.maxFeePerGas),
maxPriorityFeePerGas: BigNumber.from(config.maxPriorityFeePerGas),
}
}

if (isGasPriceOracle(config)) {
try {
return await fetchGasOracle(config)
return {
gasPrice: await fetchGasOracle(config),
}
} catch (_err) {
error = asError(_err)
logError(Errors._611, error.message)
Expand All @@ -53,10 +96,35 @@ const getGasPrice = async (gasPriceConfigs: GasPrice): Promise<BigNumber | undef
}
}

const useGasPrice = (): AsyncResult<{
maxFeePerGas: BigNumber | undefined
maxPriorityFeePerGas: BigNumber | undefined
}> => {
const getGasParameters = (
estimation: EstimatedGasPrice | undefined,
feeData: FeeData | undefined,
isEIP1559: boolean,
): GasFeeParams => {
if (!estimation) {
return {
maxFeePerGas: isEIP1559 ? feeData?.maxFeePerGas : feeData?.gasPrice,
maxPriorityFeePerGas: isEIP1559 ? feeData?.maxPriorityFeePerGas : undefined,
}
}

if (isEIP1559 && 'maxFeePerGas' in estimation && 'maxPriorityFeePerGas' in estimation) {
return estimation
}

if ('gasPrice' in estimation) {
return {
maxFeePerGas: estimation.gasPrice,
maxPriorityFeePerGas: isEIP1559 ? feeData?.maxPriorityFeePerGas : undefined,
}
}

return {
maxFeePerGas: undefined,
maxPriorityFeePerGas: undefined,
}
}
Comment on lines +99 to +126
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks more readable now but I would suggest to write a few tests to cover some of the edge-cases like if there is no estimation and it is EIP1559 but there is also no feeData etc.

const useGasPrice = (): AsyncResult<GasFeeParams> => {
const chain = useCurrentChain()
const gasPriceConfigs = chain?.gasPrice
const [counter] = useIntervalCounter(REFRESH_DELAY)
Expand All @@ -65,7 +133,7 @@ const useGasPrice = (): AsyncResult<{

const [gasPrice, gasPriceError, gasPriceLoading] = useAsync(
async () => {
const [gasPrice, feeData] = await Promise.all([
const [gasEstimation, feeData] = await Promise.all([
// Fetch gas price from oracles or get a fixed value
gasPriceConfigs ? getGasPrice(gasPriceConfigs) : undefined,

Expand All @@ -74,13 +142,7 @@ const useGasPrice = (): AsyncResult<{
])

// Prepare the return values
const maxFee = gasPrice || (isEIP1559 ? feeData?.maxFeePerGas : feeData?.gasPrice) || undefined
const maxPrioFee = (isEIP1559 && feeData?.maxPriorityFeePerGas) || undefined

return {
maxFeePerGas: maxFee,
maxPriorityFeePerGas: maxPrioFee,
}
return getGasParameters(gasEstimation, feeData, isEIP1559)
},
// eslint-disable-next-line react-hooks/exhaustive-deps
[gasPriceConfigs, provider, counter, isEIP1559],
Expand Down
28 changes: 23 additions & 5 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -3295,7 +3295,18 @@
"@safe-global/safe-gateway-typescript-sdk" "^3.5.3"
viem "^1.0.0"

"@safe-global/safe-core-sdk-types@^1.9.1", "@safe-global/safe-core-sdk-types@^1.9.2":
"@safe-global/safe-core-sdk-types@^1.9.1":
version "1.10.1"
resolved "https://registry.yarnpkg.com/@safe-global/safe-core-sdk-types/-/safe-core-sdk-types-1.10.1.tgz#94331b982671d2f2b8cc23114c58baf63d460c81"
integrity sha512-BKvuYTLOlY16Rq6qCXglmnL6KxInDuXMFqZMaCzwDKiEh+uoHu3xCumG5tVtWOkCgBF4XEZXMqwZUiLcon7IsA==
dependencies:
"@ethersproject/bignumber" "^5.7.0"
"@ethersproject/contracts" "^5.7.0"
"@safe-global/safe-deployments" "^1.20.2"
web3-core "^1.8.1"
web3-utils "^1.8.1"

"@safe-global/safe-core-sdk-types@^1.9.2":
version "1.9.2"
resolved "https://registry.yarnpkg.com/@safe-global/safe-core-sdk-types/-/safe-core-sdk-types-1.9.2.tgz#c8ae3500f5f16a9380f0270ab543f7f0718c9848"
integrity sha512-TVBoCf3bry3y6vmJXACDNOaQnHWTh8Q9G8P3wZCgUBxMc676hP9HEvF1Xrvwe0wMxevMIKyBnEV4FpZUJGSefg==
Expand Down Expand Up @@ -3328,6 +3339,13 @@
semver "^7.3.8"
web3-utils "^1.8.1"

"@safe-global/safe-deployments@^1.20.2":
version "1.26.0"
resolved "https://registry.yarnpkg.com/@safe-global/safe-deployments/-/safe-deployments-1.26.0.tgz#b83615b3b5a66e736e08f8ecf2801ed988e9e007"
integrity sha512-Tw89O4/paT19ieMoiWQbqRApb0Bef/DxweS9rxodXAM5EQModkbyFXGZca+YxXE67sLvWjLr2jJUOxwze8mhGw==
dependencies:
semver "^7.3.7"

"@safe-global/safe-deployments@^1.25.0":
version "1.25.0"
resolved "https://registry.yarnpkg.com/@safe-global/safe-deployments/-/safe-deployments-1.25.0.tgz#882f0703cd4dd86cc19238319d77459ded09ec88"
Expand All @@ -3351,10 +3369,10 @@
dependencies:
cross-fetch "^3.1.5"

"@safe-global/safe-gateway-typescript-sdk@^3.8.0":
version "3.8.0"
resolved "https://registry.yarnpkg.com/@safe-global/safe-gateway-typescript-sdk/-/safe-gateway-typescript-sdk-3.8.0.tgz#6a71eeab0ecd447a585531ef87cf987da30b78a0"
integrity sha512-CiGWIHgIaOdICpDxp05Jw3OPslWTu8AnL0PhrCT1xZgIO86NlMMLzkGbeycJ4FHpTjA999O791Oxp4bZPIjgHA==
"@safe-global/safe-gateway-typescript-sdk@^3.9.0":
version "3.9.0"
resolved "https://registry.yarnpkg.com/@safe-global/safe-gateway-typescript-sdk/-/safe-gateway-typescript-sdk-3.9.0.tgz#5aa36c05b865f6fe754d1d460f83bc9bf3a0145e"
integrity sha512-DxRM/sBBQhv955dPtdo0z2Bf2fXxrzoRUnGyTa3+4Z0RAhcyiqnffRP1Bt3tyuvlyfZnFL0RsvkqDcAIKzq3RQ==
dependencies:
cross-fetch "^3.1.5"

Expand Down
Loading