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

Fetch gas price using low-level RPC method #321

Merged
merged 5 commits into from
May 28, 2024
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
15 changes: 7 additions & 8 deletions local-test-configuration/scripts/initialize-chain.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,10 @@ import {
Api3ServerV1__factory as Api3ServerV1Factory,
} from '@api3/contracts';
import dotenv from 'dotenv';
import { NonceManager, ethers } from 'ethers';
import { type JsonRpcProvider, NonceManager, ethers } from 'ethers';
import { zip } from 'lodash';

import { getGasPrice } from '../../src/gas-price';
import { deriveSponsorWallet, encodeDapiName } from '../../src/utils';

interface RawBeaconData {
Expand Down Expand Up @@ -66,7 +67,7 @@ function encodeUpdateParameters() {

// NOTE: This function is not used by the initialization script, but you can use it after finishing Airseeker test on a
// public testnet to refund test ETH from sponsor wallets to the funder wallet.
export const refundFunder = async (funderWallet: ethers.NonceManager) => {
export const refundFunder = async (funderWallet: ethers.NonceManager, provider: JsonRpcProvider) => {
const configPath = join(__dirname, `/../airseeker`);
const rawConfig = loadConfig(join(configPath, 'airseeker.json'));
const airseekerSecrets = dotenv.parse(readFileSync(join(configPath, 'secrets.env'), 'utf8'));
Expand All @@ -78,8 +79,6 @@ export const refundFunder = async (funderWallet: ethers.NonceManager) => {
const dapiName = encodeDapiName(beaconSetName);
const updateParameters = encodeUpdateParameters();

const provider = funderWallet.provider!;

const sponsorWallet = deriveSponsorWallet(airseekerWalletMnemonic, {
...rawConfig.walletDerivationScheme,
dapiNameOrDataFeedId: dapiName,
Expand All @@ -88,11 +87,11 @@ export const refundFunder = async (funderWallet: ethers.NonceManager) => {
const sponsorWalletBalance = await provider.getBalance(sponsorWallet);
console.info('Sponsor wallet balance:', sponsorWallet.address, ethers.formatEther(sponsorWalletBalance.toString()));

const feeData = await provider.getFeeData();
const { gasPrice } = feeData;
const gasPrice = await getGasPrice(provider);

// We assume the legacy gas price will always exist. See:
// https://api3workspace.slack.com/archives/C05TQPT7PNJ/p1699098552350519
const gasFee = gasPrice! * BigInt(21_000);
const gasFee = gasPrice * BigInt(21_000);
if (sponsorWalletBalance < gasFee) {
console.info('Sponsor wallet balance is too low, skipping refund');
continue;
Expand Down Expand Up @@ -281,7 +280,7 @@ async function main() {
});
const funderWallet = new NonceManager(ethers.Wallet.fromPhrase(process.env.FUNDER_MNEMONIC, provider));

await refundFunder(funderWallet);
await refundFunder(funderWallet, provider);
const balance = await provider.getBalance(funderWallet);
console.info('Funder balance:', ethers.formatEther(balance.toString()));
console.info();
Expand Down
34 changes: 20 additions & 14 deletions src/gas-price/gas-price.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { Hex } from '@api3/commons';
import { ethers } from 'ethers';
import { ethers, toBeHex } from 'ethers';
import { range } from 'lodash';

import { generateTestConfig, initializeState } from '../../test/fixtures/mock-config';
Expand All @@ -15,6 +15,7 @@ import {
calculateScalingMultiplier,
getPercentile,
fetchAndStoreGasPrice,
getGasPrice,
} from './gas-price';

const chainId = '31337';
Expand Down Expand Up @@ -373,10 +374,26 @@ describe(getRecommendedGasPrice.name, () => {
});
});

describe(getGasPrice.name, () => {
it('returns gas price as bigint when internal rpc call returns a valid hex string', async () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you add one more test when the provider fails? I checked Ethereum RPC spec (and some other sources) and I think we can rely that when the API returns something it will be a hex string...

jest.spyOn(provider, 'send').mockResolvedValueOnce('0x1a13b8600');
const gasPrice = await getGasPrice(provider);

expect(provider.send).toHaveBeenCalledWith('eth_gasPrice', []);
expect(gasPrice).toBe(BigInt('0x1a13b8600'));
});

it('throws when internal rpc call fails', async () => {
jest.spyOn(provider, 'send').mockRejectedValueOnce(new Error('Provider error'));

await expect(getGasPrice(provider)).rejects.toThrow('Provider error');
});
});

describe(fetchAndStoreGasPrice.name, () => {
it('fetches and stores the gas price from RPC provider', async () => {
jest.spyOn(Date, 'now').mockReturnValue(dateNowMock);
jest.spyOn(provider, 'getFeeData').mockResolvedValueOnce({ gasPrice: ethers.parseUnits('10', 'gwei') } as any);
jest.spyOn(provider, 'send').mockResolvedValueOnce(toBeHex(ethers.parseUnits('10', 'gwei')));

const gasPrice = await fetchAndStoreGasPrice(chainId, providerName, provider);

Expand All @@ -387,7 +404,7 @@ describe(fetchAndStoreGasPrice.name, () => {
});

it('logs an error when fetching gas price from RPC provider fails', async () => {
jest.spyOn(provider, 'getFeeData').mockRejectedValueOnce(new Error('Provider error'));
jest.spyOn(provider, 'send').mockRejectedValueOnce(new Error('Provider error'));
jest.spyOn(logger, 'error');

const gasPrice = await fetchAndStoreGasPrice(chainId, providerName, provider);
Expand All @@ -400,15 +417,4 @@ describe(fetchAndStoreGasPrice.name, () => {
new Error('Provider error')
);
});

it('logs an error when RPC provider does not return the gas price', async () => {
jest.spyOn(provider, 'getFeeData').mockResolvedValueOnce({} as any);
jest.spyOn(logger, 'error');

const gasPrice = await fetchAndStoreGasPrice(chainId, providerName, provider);

expect(gasPrice).toBeNull();
expect(logger.error).toHaveBeenCalledTimes(1);
expect(logger.error).toHaveBeenNthCalledWith(1, 'No gas price returned from RPC provider.');
});
});
21 changes: 9 additions & 12 deletions src/gas-price/gas-price.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import type { Address, Hex } from '@api3/commons';
import { go } from '@api3/promise-utils';
import type { ethers } from 'ethers';
import { getBigInt, type ethers } from 'ethers';
import { maxBy, minBy, remove } from 'lodash';

import { logger } from '../logger';
Expand Down Expand Up @@ -49,28 +49,25 @@ export const calculateScalingMultiplier = (
maxScalingMultiplier
);

export const getGasPrice = async (provider: ethers.JsonRpcProvider) => {
const gasPriceInHex = await provider.send('eth_gasPrice', []);
return getBigInt(gasPriceInHex);
};

export const fetchAndStoreGasPrice = async (
chainId: string,
providerName: string,
provider: ethers.JsonRpcProvider
) => {
// Get the provider recommended gas price and save it to the state
logger.debug('Fetching gas price and saving it to the state.');
const goGasPrice = await go(async () => {
const feeData = await provider.getFeeData();
// We assume the legacy gas price will always exist. See:
// https://api3workspace.slack.com/archives/C05TQPT7PNJ/p1699098552350519
return feeData.gasPrice;
});
const gasPrice = goGasPrice.data;
const goGasPrice = await go(async () => getGasPrice(provider));

if (!goGasPrice.success) {
logger.error('Failed to fetch gas price from RPC provider.', sanitizeEthersError(goGasPrice.error));
return null;
}
if (!gasPrice) {
logger.error('No gas price returned from RPC provider.');
return null;
}
const gasPrice = goGasPrice.data;

const state = getState();
const {
Expand Down
2 changes: 0 additions & 2 deletions src/update-feeds-loops/contracts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,6 @@ export const verifyMulticallResponse = (

export const decodeActiveDataFeedCountResponse = Number;

export const decodeGetBlockNumberResponse = Number;

export const decodeGetChainIdResponse = Number;

export interface Beacon {
Expand Down
99 changes: 84 additions & 15 deletions src/update-feeds-loops/update-feeds-loops.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import type { Hex } from '@api3/commons';
import type { AirseekerRegistry } from '@api3/contracts';
import { ethers } from 'ethers';
import { ethers, toBeHex } from 'ethers';
import { omit } from 'lodash';

import { generateTestConfig } from '../../test/fixtures/mock-config';
Expand Down Expand Up @@ -128,8 +128,12 @@ describe(updateFeedsLoopsModule.startUpdateFeedsLoops.name, () => {

describe(updateFeedsLoopsModule.runUpdateFeeds.name, () => {
it('aborts when fetching first data feed batch fails', async () => {
const getBlockNumberSpy = jest.fn().mockResolvedValue(123n);
jest
.spyOn(contractsModule, 'createProvider')
.mockResolvedValue({ getBlockNumber: getBlockNumberSpy } as any as ethers.JsonRpcProvider);

const airseekerRegistry = generateMockAirseekerRegistry();
jest.spyOn(contractsModule, 'createProvider').mockResolvedValue(123 as any as ethers.JsonRpcProvider);
jest
.spyOn(contractsModule, 'getAirseekerRegistry')
.mockReturnValue(airseekerRegistry as unknown as AirseekerRegistry);
Expand Down Expand Up @@ -157,6 +161,66 @@ describe(updateFeedsLoopsModule.runUpdateFeeds.name, () => {
);
});

it('handles error when fetching block number call fails', async () => {
// Prepare the mocked contract so it returns two batch (of size 2) of data feeds.
const firstDataFeed = generateActiveDataFeedResponse();
const secondDataFeed = generateActiveDataFeedResponse();
const getBlockNumberSpy = jest.fn();
// Prepare the first batch to be fetched successfully and the second batch to fail.
getBlockNumberSpy.mockResolvedValueOnce(123n);
getBlockNumberSpy.mockRejectedValueOnce(new Error('provider-error-get-block-number'));
jest
.spyOn(contractsModule, 'createProvider')
.mockResolvedValue({ getBlockNumber: getBlockNumberSpy } as any as ethers.JsonRpcProvider);

const airseekerRegistry = generateMockAirseekerRegistry();
jest
.spyOn(contractsModule, 'getAirseekerRegistry')
.mockReturnValue(airseekerRegistry as unknown as AirseekerRegistry);
airseekerRegistry.interface.decodeFunctionResult.mockImplementation((_fn, value) => value);
const chainId = BigInt(31_337);
airseekerRegistry.tryMulticall.staticCall.mockResolvedValueOnce({
successes: [true, true, true],
returndata: [2n, chainId, firstDataFeed],
});
airseekerRegistry.tryMulticall.staticCall.mockResolvedValueOnce({
successes: [true, true],
returndata: [chainId, secondDataFeed],
});

jest.spyOn(logger, 'error');
jest.spyOn(stateModule, 'updateState').mockImplementation();
jest.spyOn(updateFeedsLoopsModule, 'processBatch').mockResolvedValueOnce({
signedApiUrlsFromConfig: [],
signedApiUrlsFromContract: [],
beaconIds: [],
successCount: 1,
errorCount: 0,
});

await updateFeedsLoopsModule.runUpdateFeeds(
'provider-name',
allowPartial<Chain>({
dataFeedBatchSize: 1,
dataFeedUpdateInterval: 0.3, // 300ms update interval to make the test run quicker.
providers: { ['provider-name']: { url: 'provider-url' } },
contracts: {
AirseekerRegistry: '0x9fE46736679d2D9a65F0992F2272dE9f3c7fa6e0',
},
}),
'31337'
);

// Expect the logs to be called with the correct context.
expect(logger.error).toHaveBeenCalledTimes(1);
expect(logger.error).toHaveBeenCalledWith(
'Failed to get active data feeds batch.',
new Error('provider-error-get-block-number')
);
// Expect the processBatch to be called only once for the first batch.
expect(updateFeedsLoopsModule.processBatch).toHaveBeenCalledTimes(1);
});

it('fetches and processes other batches in a staggered way and logs errors', async () => {
// Prepare the mocked contract so it returns three batches (of size 1) of data feeds and the second batch fails to load.
const firstDataFeed = generateActiveDataFeedResponse();
Expand All @@ -182,27 +246,27 @@ describe(updateFeedsLoopsModule.runUpdateFeeds.name, () => {
),
} as contractsModule.DecodedActiveDataFeedResponse;
const airseekerRegistry = generateMockAirseekerRegistry();
const getFeeDataSpy = jest.fn().mockResolvedValue({ gasPrice: ethers.parseUnits('5', 'gwei') });
const getBlockNumberSpy = jest.fn().mockResolvedValue(123n);
const getGasPriceSpy = jest.fn().mockResolvedValue(toBeHex(ethers.parseUnits('5', 'gwei')));
jest
.spyOn(contractsModule, 'createProvider')
.mockResolvedValue({ getFeeData: getFeeDataSpy } as any as ethers.JsonRpcProvider);
.mockResolvedValue({ send: getGasPriceSpy, getBlockNumber: getBlockNumberSpy } as any as ethers.JsonRpcProvider);
jest
.spyOn(contractsModule, 'getAirseekerRegistry')
.mockReturnValue(airseekerRegistry as unknown as AirseekerRegistry);
airseekerRegistry.interface.decodeFunctionResult.mockImplementation((_fn, value) => value);
const blockNumber = 123n;
const chainId = BigInt(31_337);
airseekerRegistry.tryMulticall.staticCall.mockResolvedValueOnce({
successes: [true, true, true, true],
returndata: [3n, blockNumber, chainId, firstDataFeed],
successes: [true, true, true],
returndata: [3n, chainId, firstDataFeed],
});
airseekerRegistry.tryMulticall.staticCall.mockResolvedValueOnce({
successes: [true, true, false],
returndata: [blockNumber, chainId, '0x'],
successes: [true, false],
returndata: [chainId, '0x'],
});
airseekerRegistry.tryMulticall.staticCall.mockResolvedValueOnce({
successes: [true, true, true],
returndata: [blockNumber, chainId, thirdDataFeed],
successes: [true, true],
returndata: [chainId, thirdDataFeed],
});
const sleepCalls = [] as number[];
const originalSleep = utilsModule.sleep;
Expand Down Expand Up @@ -317,16 +381,18 @@ describe(updateFeedsLoopsModule.runUpdateFeeds.name, () => {
it('catches unhandled error', async () => {
const dataFeed = generateActiveDataFeedResponse();
const airseekerRegistry = generateMockAirseekerRegistry();
jest.spyOn(contractsModule, 'createProvider').mockResolvedValue(123 as any as ethers.JsonRpcProvider);
const getBlockNumberSpy = jest.fn().mockResolvedValue(123n);
jest
.spyOn(contractsModule, 'createProvider')
.mockResolvedValue({ getBlockNumber: getBlockNumberSpy } as any as ethers.JsonRpcProvider);
jest
.spyOn(contractsModule, 'getAirseekerRegistry')
.mockReturnValue(airseekerRegistry as unknown as AirseekerRegistry);
airseekerRegistry.interface.decodeFunctionResult.mockImplementation((_fn, value) => value);
const blockNumber = 123n;
const chainId = BigInt(31_337);
airseekerRegistry.tryMulticall.staticCall.mockResolvedValueOnce({
successes: [true, true, true, true],
returndata: [1n, blockNumber, chainId, dataFeed],
successes: [true, true, true],
returndata: [1n, chainId, dataFeed],
});
const testConfig = generateTestConfig();
jest.spyOn(stateModule, 'getState').mockReturnValue(
Expand Down Expand Up @@ -443,6 +509,7 @@ describe(updateFeedsLoopsModule.processBatch.name, () => {
jest.spyOn(logger, 'info');

// Skip actions other than generating signed api urls.
jest.spyOn(gasPriceModule, 'fetchAndStoreGasPrice').mockImplementation();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Mhh, why do these need to be mocked now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes it looks redundant but I wanted to add them because processBatch includes fetchAndStoreGasPrice. Should I remove them?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Nah, leave them. I was just surprised why the test was passing, but I guess either Ethers made a noop call or silently did nothing 🤷

Copy link
Contributor Author

@bdrhn9 bdrhn9 May 27, 2024

Choose a reason for hiding this comment

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

Test was passing probably because fetchAndStoreGasPrice handles error by returning null when it fails. However, this spy on fetchAndStoreGasPrice is good to have because I encountered this error when I am playing with tests even though I couldn't produce it now.

jest.spyOn(getUpdatableFeedsModule, 'getUpdatableFeeds').mockReturnValue([]);
jest.spyOn(submitTransactionModule, 'getDerivedSponsorWallet').mockReturnValue(ethers.Wallet.createRandom());

Expand Down Expand Up @@ -485,6 +552,7 @@ describe(updateFeedsLoopsModule.processBatch.name, () => {
jest.spyOn(logger, 'info');

// Skip actions other than generating signed api urls.
jest.spyOn(gasPriceModule, 'fetchAndStoreGasPrice').mockImplementation();
jest.spyOn(getUpdatableFeedsModule, 'getUpdatableFeeds').mockReturnValue([]);
jest.spyOn(submitTransactionModule, 'getDerivedSponsorWallet').mockReturnValue(ethers.Wallet.createRandom());

Expand Down Expand Up @@ -532,6 +600,7 @@ describe(updateFeedsLoopsModule.processBatch.name, () => {
jest.spyOn(provider, 'getTransactionCount').mockResolvedValue(123);

// Skip actions other than generating signed api urls.
jest.spyOn(gasPriceModule, 'fetchAndStoreGasPrice').mockImplementation();
jest.spyOn(getUpdatableFeedsModule, 'getUpdatableFeeds').mockReturnValue([
allowPartial<getUpdatableFeedsModule.UpdatableDataFeed>({
dataFeedInfo: {
Expand Down
17 changes: 6 additions & 11 deletions src/update-feeds-loops/update-feeds-loops.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import {
createProvider,
decodeActiveDataFeedCountResponse,
decodeActiveDataFeedResponse,
decodeGetBlockNumberResponse,
decodeGetChainIdResponse,
getAirseekerRegistry,
getApi3ServerV1,
Expand Down Expand Up @@ -90,20 +89,22 @@ export const readActiveDataFeedBatch = async (
const calldatas: string[] = [];
if (fromIndex === 0) calldatas.push(airseekerRegistry.interface.encodeFunctionData('activeDataFeedCount'));
calldatas.push(
airseekerRegistry.interface.encodeFunctionData('getBlockNumber'),
airseekerRegistry.interface.encodeFunctionData('getChainId'),
...range(fromIndex, toIndex).map((dataFeedIndex) =>
airseekerRegistry.interface.encodeFunctionData('activeDataFeed', [dataFeedIndex])
)
);

let returndatas = verifyMulticallResponse(await airseekerRegistry.tryMulticall.staticCall(calldatas));
const [blockNumber, multicallResponse] = await Promise.all([
provider.getBlockNumber(),
airseekerRegistry.tryMulticall.staticCall(calldatas),
]);
let returndatas = verifyMulticallResponse(multicallResponse);
let activeDataFeedCountReturndata: string | undefined;
if (fromIndex === 0) {
activeDataFeedCountReturndata = returndatas[0]!;
returndatas = returndatas.slice(1);
}
const [getBlockNumberReturndata, getChainIdReturndata, ...activeDataFeedReturndatas] = returndatas;
const [getChainIdReturndata, ...activeDataFeedReturndatas] = returndatas;

// Check that the chain ID is correct and log a warning if it's not because it's possible that providers switch chain
// ID at runtime by mistake. In case the chain ID is wrong, we want to skip all data feeds in the batch (or all of
Expand Down Expand Up @@ -131,12 +132,6 @@ export const readActiveDataFeedBatch = async (
return isRegistered;
});

// NOTE: https://api3workspace.slack.com/archives/C05TQPT7PNJ/p1713441156074839?thread_ts=1713438669.278119&cid=C05TQPT7PNJ
const blockNumber =
chainId === '42161' || chainId === '421614'
? await provider.getBlockNumber()
: decodeGetBlockNumberResponse(getBlockNumberReturndata!);

return {
batch,
blockNumber,
Expand Down