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

refactor: use getAction internally #2627

Closed
wants to merge 3 commits into from
Closed
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
3 changes: 3 additions & 0 deletions src/actions/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,12 +113,14 @@ export {
type GetBlockParameters,
type GetBlockReturnType,
getBlock,
_getBlock,
} from './public/getBlock.js'
export {
type GetBlockNumberErrorType,
type GetBlockNumberParameters,
type GetBlockNumberReturnType,
getBlockNumber,
_getBlockNumber,
} from './public/getBlockNumber.js'
export {
type GetBlockTransactionCountErrorType,
Expand All @@ -130,6 +132,7 @@ export {
type GetChainIdErrorType,
type GetChainIdReturnType,
getChainId,
_getChainId,
} from './public/getChainId.js'
export {
/** @deprecated Use `GetCodeErrorType` instead */
Expand Down
7 changes: 3 additions & 4 deletions src/actions/public/estimateFeesPerGas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import {
type EstimateMaxPriorityFeePerGasErrorType,
internal_estimateMaxPriorityFeePerGas,
} from './estimateMaxPriorityFeePerGas.js'
import { getBlock } from './getBlock.js'
import { type GetBlockErrorType, getBlock } from './getBlock.js'
import { type GetGasPriceErrorType, getGasPrice } from './getGasPrice.js'

export type EstimateFeesPerGasParameters<
Expand All @@ -52,6 +52,7 @@ export type EstimateFeesPerGasReturnType<

export type EstimateFeesPerGasErrorType =
| BaseFeeScalarErrorType
| GetBlockErrorType
Copy link
Contributor Author

@holic holic Aug 17, 2024

Choose a reason for hiding this comment

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

we call getBlock here, so this error type should also be included here?

| EstimateMaxPriorityFeePerGasErrorType
| GetGasPriceErrorType
| Eip1559FeesNotSupportedErrorType
Expand Down Expand Up @@ -126,9 +127,7 @@ export async function internal_estimateFeesPerGas<
(base * BigInt(Math.ceil(baseFeeMultiplier * denominator))) /
BigInt(denominator)

const block = block_
? block_
: await getAction(client, getBlock, 'getBlock')({})
const block = block_ ? block_ : await getBlock(client)

if (typeof chain?.fees?.estimateFeesPerGas === 'function') {
const fees = (await chain.fees.estimateFeesPerGas({
Expand Down
3 changes: 1 addition & 2 deletions src/actions/public/estimateMaxPriorityFeePerGas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ export type EstimateMaxPriorityFeePerGasErrorType =
| GetBlockErrorType
| HexToBigIntErrorType
| RequestErrorType
| GetBlockErrorType
Copy link
Contributor Author

@holic holic Aug 17, 2024

Choose a reason for hiding this comment

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

this was already in the union a few lines above

| GetGasPriceErrorType
| Eip1559FeesNotSupportedErrorType
| ErrorType
Expand Down Expand Up @@ -87,7 +86,7 @@ export async function internal_estimateMaxPriorityFeePerGas<
): Promise<EstimateMaxPriorityFeePerGasReturnType> {
const { block: block_, chain = client.chain, request } = args || {}
if (typeof chain?.fees?.defaultPriorityFee === 'function') {
const block = block_ || (await getAction(client, getBlock, 'getBlock')({}))
const block = block_ || (await getBlock(client))
return chain.fees.defaultPriorityFee({
block,
client,
Expand Down
13 changes: 13 additions & 0 deletions src/actions/public/getBlock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
type FormattedBlock,
formatBlock,
} from '../../utils/formatters/block.js'
import { getAction } from '../../utils/getAction.js'

export type GetBlockParameters<
includeTransactions extends boolean = false,
Expand Down Expand Up @@ -92,6 +93,18 @@ export async function getBlock<
account extends Account | undefined,
includeTransactions extends boolean = false,
blockTag extends BlockTag = 'latest',
>(
client: Client<Transport, chain, account>,
params: GetBlockParameters<includeTransactions, blockTag> = {},
): Promise<GetBlockReturnType<chain, includeTransactions, blockTag>> {
return getAction(client, _getBlock, 'getBlock')(params)
}
Comment on lines +96 to +101
Copy link
Contributor

@roninjin10 roninjin10 Aug 18, 2024

Choose a reason for hiding this comment

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

Suggested change
>(
client: Client<Transport, chain, account>,
params: GetBlockParameters<includeTransactions, blockTag> = {},
): Promise<GetBlockReturnType<chain, includeTransactions, blockTag>> {
return getAction(client, _getBlock, 'getBlock')(params)
}
>(
client: Client<Transport, chain, account>,
params: GetBlockParameters<includeTransactions, blockTag> = {},
): Promise<GetBlockReturnType<chain, includeTransactions, blockTag>> {
return getAction(client, _getBlock, 'getBlock')(params)
}

Might be able to do this just to save some boilerplate

const withGetAction = <TAction>(action: TAction, name: string): TAction => {
  return ((client, params) => {
     return getAction(client, action, name)(params)
  }) as unknown as TAction
}

export const getBlock = withGetAction(_getBlock, 'getBlock')

Copy link
Contributor

Choose a reason for hiding this comment

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

DOn't do this it's convenient but will make stack traces suck


export async function _getBlock<
chain extends Chain | undefined,
account extends Account | undefined,
includeTransactions extends boolean = false,
blockTag extends BlockTag = 'latest',
>(
client: Client<Transport, chain, account>,
{
Expand Down
8 changes: 8 additions & 0 deletions src/actions/public/getBlockNumber.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import type { Transport } from '../../clients/transports/createTransport.js'
import type { ErrorType } from '../../errors/utils.js'
import type { Chain } from '../../types/chain.js'
import type { RequestErrorType } from '../../utils/buildRequest.js'
import { getAction } from '../../utils/getAction.js'
import {
type GetCacheErrorType,
getCache,
Expand Down Expand Up @@ -52,6 +53,13 @@ export function getBlockNumberCache(id: string) {
* // 69420n
*/
export async function getBlockNumber<chain extends Chain | undefined>(
client: Client<Transport, chain>,
params: GetBlockNumberParameters = {},
): Promise<GetBlockNumberReturnType> {
return getAction(client, _getBlockNumber, 'getBlockNumber')(params)
}

export async function _getBlockNumber<chain extends Chain | undefined>(
Comment on lines +59 to +62
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it actually necessary to export both? Shouldn't getBlockNumber always "just work"?

Copy link
Contributor Author

@holic holic Aug 18, 2024

Choose a reason for hiding this comment

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

My understanding is if I override getBlockNumber with my own action, then inside that override call into viem's getBlockNumber here, it'd end up resolving to my own getBlockNumber, not viem's underlying one, because that's the one attached to the client object?

That points to a potentially deeper issue though - would this result in an infinite loop?

client: Client<Transport, chain>,
{ cacheTime = client.cacheTime }: GetBlockNumberParameters = {},
): Promise<GetBlockNumberReturnType> {
Expand Down
8 changes: 8 additions & 0 deletions src/actions/public/getChainId.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
type HexToNumberErrorType,
hexToNumber,
} from '../../utils/encoding/fromHex.js'
import { getAction } from '../../utils/getAction.js'

export type GetChainIdReturnType = number

Expand Down Expand Up @@ -40,6 +41,13 @@ export type GetChainIdErrorType =
export async function getChainId<
chain extends Chain | undefined,
account extends Account | undefined,
>(client: Client<Transport, chain, account>): Promise<GetChainIdReturnType> {
return getAction(client, _getChainId, 'getChainId')({})
}

export async function _getChainId<
chain extends Chain | undefined,
account extends Account | undefined,
>(client: Client<Transport, chain, account>): Promise<GetChainIdReturnType> {
const chainIdHex = await client.request(
{
Expand Down
2 changes: 1 addition & 1 deletion src/actions/public/getTransactionConfirmations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ export async function getTransactionConfirmations<
{ hash, transactionReceipt }: GetTransactionConfirmationsParameters<chain>,
): Promise<GetTransactionConfirmationsReturnType> {
const [blockNumber, transaction] = await Promise.all([
getAction(client, getBlockNumber, 'getBlockNumber')({}),
getBlockNumber(client),
hash
? getAction(client, getTransaction, 'getTransaction')({ hash })
: undefined,
Expand Down
6 changes: 1 addition & 5 deletions src/actions/public/waitForTransactionReceipt.ts
Original file line number Diff line number Diff line change
Expand Up @@ -259,11 +259,7 @@ export async function waitForTransactionReceipt<
retrying = true
const block = await withRetry(
() =>
getAction(
client,
getBlock,
'getBlock',
)({
getBlock(client, {
blockNumber,
includeTransactions: true,
}),
Expand Down
7 changes: 1 addition & 6 deletions src/actions/public/watchBlockNumber.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import type { ErrorType } from '../../errors/utils.js'
import type { Chain } from '../../types/chain.js'
import type { HasTransportType } from '../../types/transport.js'
import { hexToBigInt } from '../../utils/encoding/fromHex.js'
import { getAction } from '../../utils/getAction.js'
import { observe } from '../../utils/observe.js'
import { type PollErrorType, poll } from '../../utils/poll.js'
import { stringify } from '../../utils/stringify.js'
Expand Down Expand Up @@ -117,11 +116,7 @@ export function watchBlockNumber<
poll(
async () => {
try {
const blockNumber = await getAction(
client,
getBlockNumber,
'getBlockNumber',
)({ cacheTime: 0 })
const blockNumber = await getBlockNumber(client, { cacheTime: 0 })

if (prevBlockNumber) {
// If the current block number is the same as the previous,
Expand Down
6 changes: 1 addition & 5 deletions src/actions/public/watchBlocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -145,11 +145,7 @@ export function watchBlocks<
poll(
async () => {
try {
const block = await getAction(
client,
getBlock,
'getBlock',
)({
const block = await getBlock(client, {
blockTag,
includeTransactions,
})
Expand Down
6 changes: 1 addition & 5 deletions src/actions/public/watchContractEvent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -220,11 +220,7 @@ export function watchContractEvent<
// The fall back exists because some RPC Providers do not support filters.

// Fetch the block number to use for `getLogs`.
const blockNumber = await getAction(
client,
getBlockNumber,
'getBlockNumber',
)({})
const blockNumber = await getBlockNumber(client)

// If the block number has changed, we will need to fetch the logs.
// If the block number doesn't exist, we are yet to reach the first poll interval,
Expand Down
6 changes: 1 addition & 5 deletions src/actions/public/watchEvent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -243,11 +243,7 @@ export function watchEvent<
// The fall back exists because some RPC Providers do not support filters.

// Fetch the block number to use for `getLogs`.
const blockNumber = await getAction(
client,
getBlockNumber,
'getBlockNumber',
)({})
const blockNumber = await getBlockNumber(client)

// If the block number has changed, we will need to fetch the logs.
// If the block number doesn't exist, we are yet to reach the first poll interval,
Expand Down
8 changes: 2 additions & 6 deletions src/actions/wallet/prepareTransactionRequest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -258,11 +258,7 @@ export async function prepareTransactionRequest<
let block: Block | undefined
async function getBlock(): Promise<Block> {
if (block) return block
block = await getAction(
client,
getBlock_,
'getBlock',
)({ blockTag: 'latest' })
block = await getBlock_(client, { blockTag: 'latest' })
return block
}

Expand All @@ -271,7 +267,7 @@ export async function prepareTransactionRequest<
if (chainId) return chainId
if (chain) return chain.id
if (typeof args.chainId !== 'undefined') return args.chainId
const chainId_ = await getAction(client, getChainId_, 'getChainId')({})
const chainId_ = await getChainId_(client)
chainId = chainId_
return chainId
}
Expand Down
Loading