-
Notifications
You must be signed in to change notification settings - Fork 2
feat: adding start and end block filters to Deposits api #560
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
base: master
Are you sure you want to change the base?
Conversation
amateima
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you share more details how these query params will be used? How big will be the typical block interval? What's the expected calls frequency?
Also, given the size of the interrogated tables, I think new indexes are necessary in those 3 tables where we want to filter by block number
|
Hey @amateima For the block intervals DefiLlama uses the following: // each chain should have number of blocks per approx. 1.5 or 2 hours. default is 400.
export const maxBlocksToQueryByChain = {
default: 300,
ethereum: 300,
polygon: 1000,
fantom: 800,
arbitrum: 3000,
era: 5000,
linea: 3000,
manta: 800,
blast: 2000,
avax: 3000,
bsc: 2000,
optimism: 3000,
xdai: 400,
aurora: 5400,
celo: 1200,
klaytn: 6000,
sui: 2400, // sui creates a checkpoint about every 3 seconds
solana: 6000,
taiko: 100,
sonic: 10000,
base: 3000,
arbitrum_nova: 3000,
polygon_zkevm: 2000,
scroll: 2000,
mode: 2000,
mantle: 2000,
"b2-mainnet": 2000,
berachain: 3000,
hyperliquid: 3000,
} as { [chain: string]: number };Frequency: Indexes: |
| ]) | ||
| @Index("IX_depositForBurn_finalised", ["finalised"]) | ||
| @Index("IX_depositForBurn_deletedAt", ["deletedAt"]) | ||
| @Index("IX_depositForBurn_chainId_blockNumber", ["chainId", "blockNumber"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will the blockNumber query parameters always be used alongside chainId?
Keep in mind that with this IX, blockNumber index won't be used by postgres if chainId is missing from the request parameters.
Also, I think this index is redundant and doesn't bring any performance improvements because we already have UK_depositForBurn_chain_block_txn_log index. The first 2 columns from there are identical to your columns.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given the conversation we had, we can remove this index
melisaguevara
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good! It will be nice to check Alex's comment regarding the index on DepositForBurn and remove if it's indeed not needed before merging.
| @Index("IX_v3FundsDeposited_originChainId_blockNumber", [ | ||
| "originChainId", | ||
| "blockNumber", | ||
| ]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not 100% sure but I think we can remove this because we have the IX_deposits_block_chain_logIndex index
|
|
||
| export const DepositForBurnFilledRelayFields = [ | ||
| `NULL::varchar as "relayer"`, | ||
| `"mintAndWithdraw"."blockNumber"::integer as "fillBlockNumber"`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible we might get into some trouble with the naming here?
fillBlockNumber is not really a fill, but a mint, no?
Need to add these in order to update our defi llama dashboard data source.
This requests in this file:
https://github.com/DefiLlama/bridges-server/blob/master/src/adapters/across/index.ts
will be replaced with simply reading and transforming the data from this api.
In order to support the DefiLlama adapter we also need to return the FIllBlockNumber so I have added that in this PR as well.