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: fetch token deposits from event logs #2168

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 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
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,26 @@ import {
} from './fetchDepositsTestHelpers'
import { fetchDepositsFromSubgraph } from '../fetchDepositsFromSubgraph'

const defaults = {
sender: '0x5d64a0fd6af0d76a7ed189d4061ffa6823fbf97e',
pageSize: 100
}

describe('fetchDepositsFromSubgraph', () => {
it('fetches no deposits from subgraph pre-nitro', async () => {
const result = await fetchDepositsFromSubgraph(
getQueryCoveringClassicOnlyWithoutResults()
)
const result = await fetchDepositsFromSubgraph({
...defaults,
...getQueryCoveringClassicOnlyWithoutResults()
})

expect(result).toHaveLength(0)
})

it('fetches some deposits from subgraph pre-nitro', async () => {
const result = await fetchDepositsFromSubgraph(
getQueryCoveringClassicOnlyWithResults()
)
const result = await fetchDepositsFromSubgraph({
...defaults,
...getQueryCoveringClassicOnlyWithResults()
})

expect(result).toHaveLength(3)
expect(result).toEqual(
Expand All @@ -39,9 +46,10 @@ describe('fetchDepositsFromSubgraph', () => {
})

it('fetches some deposits from subgraph pre-nitro and post-nitro', async () => {
const result = await fetchDepositsFromSubgraph(
getQueryCoveringClassicAndNitroWithResults()
)
const result = await fetchDepositsFromSubgraph({
...defaults,
...getQueryCoveringClassicAndNitroWithResults()
})

expect(result).toHaveLength(4)
expect(result).toEqual(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,14 @@
const sender = '0x5d64a0fd6af0d76a7ed189d4061ffa6823fbf97e'
import { getArbitrumNetwork } from '@arbitrum/sdk'

const baseQuery = {
sender,
l2ChainId: 42161,
pageSize: 100
childChainId: 42161,
parentChainId: 1,
parentGatewayAddresses: [
getArbitrumNetwork(42161).tokenBridge!.parentErc20Gateway,
getArbitrumNetwork(42161).tokenBridge!.parentCustomGateway,
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: we could have getArbitrumNetwork(42161) as a variable

getArbitrumNetwork(42161).tokenBridge!.parentWethGateway
]
}

export function getQueryCoveringClassicOnlyWithoutResults() {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
import { fetchTokenDepositsFromEventLogs } from '../fetchTokenDepositsFromEventLogs'
import {
getQueryCoveringClassicAndNitroWithResults,
getQueryCoveringClassicOnlyWithoutResults,
getQueryCoveringClassicOnlyWithResults
} from './fetchDepositsTestHelpers'

const TIMEOUT = 30_000

describe('fetchTokenDepositsFromEventLogs', () => {
it(
'fetches no token deposits from event logs pre-nitro',
async () => {
const result = await fetchTokenDepositsFromEventLogs(
getQueryCoveringClassicOnlyWithoutResults()
)

expect(result).toHaveLength(0)
},
TIMEOUT
)

it(
'fetches some token deposits from event logs pre-nitro',
async () => {
const result = await fetchTokenDepositsFromEventLogs({
...getQueryCoveringClassicOnlyWithResults(),
sender: '0x981EA2202d2d33D26583E96f6e6449C1F9b6Bbb1'
})

expect(result).toHaveLength(1)
expect(result).toEqual(
expect.arrayContaining([
Copy link
Contributor

Choose a reason for hiding this comment

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

How are the results significantly different from fetchDepositsFromSubgraph test? I'd assume fetching deposits through event logs, with an additional sender condition, would be a subset of overall deposits being fetched via subgraph, but there seems to be no intersection. 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

fetchDepositsFromSubgraph only tests for ETH deposits so this token deposits tests will be all different

in fact, we need to add tests for token deposits fetches to the subgraph test

// 14387620
expect.objectContaining({
txHash:
'0xc112c54c261dabbd919132149226fc8a4c0196948e18dda947aa06e8b8b69064'
})
])
)
},
TIMEOUT
)

it(
'fetches some token deposits from event logs pre-nitro and post-nitro',
async () => {
const result1 = await fetchTokenDepositsFromEventLogs({
...getQueryCoveringClassicAndNitroWithResults(),
sender: '0x5049FEB67800e5BA82626c9f78FF9C458E0Eb66f'
})

expect(result1).toHaveLength(1)
expect(result1).toEqual(
expect.arrayContaining([
// 15417417
expect.objectContaining({
txHash:
'0xd9e72eba4548811425fd57def8aad238fcfdb0ced5a2af1d6b99f48ef51d52f0'
})
])
)

const result2 = await fetchTokenDepositsFromEventLogs({
...getQueryCoveringClassicAndNitroWithResults(),
sender: '0x8594D8e9483473626908648A5539D9d65Ca2fe8d'
})

expect(result2).toHaveLength(149)
},
TIMEOUT
)
})

it(
'fetches some token deposits from event logs post-nitro',
async () => {
const result = await fetchTokenDepositsFromEventLogs({
...getQueryCoveringClassicOnlyWithResults(),
sender: '0x07aE8551Be970cB1cCa11Dd7a11F47Ae82e70E67',
fromBlock: 21502346,
toBlock: 21502471
})

expect(result).toHaveLength(2)
expect(result).toEqual(
expect.arrayContaining([
// 14310032
expect.objectContaining({
txHash:
'0x583bea616664fa32f68d25822bb64f18c8186221c35919a567b3de5eb9c1ae7e'
}),
// 14309826
expect.objectContaining({
txHash:
'0x012ef30f19eb89951590790f6e2882fe6a5dc7671e8cbddb2b09b6efd7ad892a'
})
])
)
},
TIMEOUT
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
import { EventArgs, EventFetcher } from '@arbitrum/sdk'
import { L1ERC20Gateway__factory } from '@arbitrum/sdk/dist/lib/abi/factories/L1ERC20Gateway__factory'
import { DepositInitiatedEvent } from '@arbitrum/sdk/dist/lib/abi/L1ERC20Gateway'
import { BlockTag } from '@ethersproject/providers'

import { getProviderForChainId } from '@/token-bridge-sdk/utils'
import { dedupeEvents } from './helpers'

/**
* Get the parent network events created by a deposit
* @param parentChainId
* @param gatewayAddress
* @param filter
* @param parentTokenAddress
* @param fromAddress
* @param toAddress
* @returns
*/
export async function getDepositEvents(
parentChainId: number,
gatewayAddress: string,
filter: { fromBlock: BlockTag; toBlock: BlockTag },
parentTokenAddress?: string,
fromAddress?: string,
toAddress?: string
): Promise<(EventArgs<DepositInitiatedEvent> & { txHash: string })[]> {
const parentProvider = getProviderForChainId(parentChainId)

const eventFetcher = new EventFetcher(parentProvider)
const events = (
await eventFetcher.getEvents(
L1ERC20Gateway__factory,
contract =>
contract.filters.DepositInitiated(
null, // parentToken
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't we pass parentTokenAddress here?

Copy link
Member Author

Choose a reason for hiding this comment

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

fairly sure it's a typechain typing issue
we are doing the same with withdrawals

image

fromAddress || null, // _from
toAddress || null // _to
),
{ ...filter, address: gatewayAddress }
)
).map(a => ({ txHash: a.transactionHash, ...a.event }))

return parentTokenAddress
? events.filter(
log =>
log.l1Token.toLocaleLowerCase() ===
parentTokenAddress.toLocaleLowerCase()
)
: events
}

export type FetchTokenDepositsFromEventLogsParams = {
sender?: string
receiver?: string
fromBlock: BlockTag
toBlock: BlockTag
parentChainId: number
parentGatewayAddresses?: string[]
}

/**
* Fetches initiated token deposits from event logs in range of [fromBlock, toBlock].
*
* @param query Query params
* @param query.sender Address that initiated the withdrawal
* @param query.receiver Address that received the funds
* @param query.fromBlock Start at this block number (including)
* @param query.toBlock Stop at this block number (including)
* @param query.childChainId child chain id
* @param query.parentGatewayAddresses L2 gateway addresses to use
*/
export async function fetchTokenDepositsFromEventLogs({
sender,
Copy link
Contributor

Choose a reason for hiding this comment

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

We must decide between sender vs fromAddress, and receiver vs toAddress in this file and stick to one.

Copy link
Member Author

Choose a reason for hiding this comment

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

getDepositEvents is following the format of getWithdrawalEvents in arbitrum sdk's erc20Bridger
we'll move this function to arbitrum sdk later so i'll keep fromAddress and toAddress for it

receiver,
fromBlock,
toBlock,
parentChainId,
parentGatewayAddresses = []
}: FetchTokenDepositsFromEventLogsParams) {
const promises: ReturnType<typeof getDepositEvents>[] = []

parentGatewayAddresses.forEach(gatewayAddress => {
// funds sent by this address
if (sender) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we okay with returning empty data if no sender or receiver is passed?
getDepositEvents supports sender and receiver being null.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is how we're handling it at the moment with fetchTokenWithdrawalsFromEventLogs so following that format and i think that's fine

promises.push(
getDepositEvents(
parentChainId,
gatewayAddress,
{ fromBlock, toBlock },
undefined,
sender,
undefined
)
)
}

// funds received by this address
if (receiver) {
promises.push(
getDepositEvents(
parentChainId,
gatewayAddress,
{ fromBlock, toBlock },
undefined,
undefined,
receiver
)
)
}
})

// when getting funds received by this address we will also get duplicate txs returned in 'funds sent by this address'
return dedupeEvents<DepositInitiatedEvent>(
(await Promise.all(promises)).flat()
)
}
11 changes: 10 additions & 1 deletion packages/arb-token-bridge-ui/src/util/deposits/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ import {
ParentToChildMessageReader,
ParentToChildMessageReaderClassic,
EthL1L3DepositStatus,
Erc20L1L3DepositStatus
Erc20L1L3DepositStatus,
EventArgs
} from '@arbitrum/sdk'
import { utils } from 'ethers'

Expand Down Expand Up @@ -675,3 +676,11 @@ export const getParentToChildMessageDataFromParentTxHash = async ({
// post-nitro deposit - both eth + token
return getNitroDepositMessage()
}

export function dedupeEvents<T>(
events: (EventArgs<T> & {
txHash: string
})[]
) {
return [...new Map(events.map(item => [item.txHash, item])).values()]
}
Original file line number Diff line number Diff line change
@@ -1,14 +1,7 @@
import { Provider, BlockTag } from '@ethersproject/providers'
import { Erc20Bridger, EventArgs } from '@arbitrum/sdk'
import { Erc20Bridger } from '@arbitrum/sdk'
import { WithdrawalInitiatedEvent } from '@arbitrum/sdk/dist/lib/abi/L2ArbitrumGateway'

function dedupeEvents(
events: (EventArgs<WithdrawalInitiatedEvent> & {
txHash: string
})[]
) {
return [...new Map(events.map(item => [item.txHash, item])).values()]
}
import { dedupeEvents } from '../deposits/helpers'
Copy link
Contributor

Choose a reason for hiding this comment

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

Withdrawals shouldn't really import from deposits helpers, maybe we can move it to /TransactionHistory/helpers instead?


export type FetchTokenWithdrawalsFromEventLogsParams = {
sender?: string
Expand Down Expand Up @@ -72,5 +65,7 @@ export async function fetchTokenWithdrawalsFromEventLogs({
})

// when getting funds received by this address we will also get duplicate txs returned in 'funds sent by this address'
return dedupeEvents((await Promise.all(promises)).flat())
return dedupeEvents<WithdrawalInitiatedEvent>(
(await Promise.all(promises)).flat()
)
}
Loading