-
Notifications
You must be signed in to change notification settings - Fork 23
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: add EVM indexer #274
Conversation
I'm trying to index another contract on Goerli to test but Also in |
@bonustrack it seems that it's some issue on Infura side that it can never resolve initial request as we are not doing lots of requests and I even lowered the maximum range we are using to 10 blocks. I asked them about it on Discord. |
It’s expected, I only modified the max step internally when handling requests. I will checkout the rest but getLogs call is generally slow and takes 1s for each call. So generally we should do as little requests as possible, but this might increase likelihood of timeouts (however it seems that it can timeout regardless of range). I assume that Anker either doesn’t support getLogs or fails differently to Infura if responses is too big (need to look into it). I guess the issue will still be that we depend on slow getLogs call. Maybe we need better way of handling it - instead of fetching all logs we fetch only those that match tracked contracts - and if new contracts are added to sources on the fly we come back. Might solve the issue. |
Yes, I think this is the only way, if we can target specific contract it will be more efficient we wouldn't need to load each blocks events. It might just be a bit tricky to implement. If we prefetch events and find a new deployment event, we can't continue the prefetch process because we know 1 contract will be missing and will require to reset prefetched events. And the logic to discover new template contract address is part of a writer function, which in theory should run in the right order with the others writer functions. It sound like the solution would be to prefetch events until a new deployment is found then run indexer up to this event then continue the prefetch. But curious to know if you have some others ideas. I think also that we will not find a perfect block range limit for the method |
I think this was considered as a one way of implementing it for Starknet before, but we decided to go other way - I can't find the discussion anymore to see what was the reasoning - ideally we should have consistent logic on all networks - at least to certain degree.
Infura handles that and tells you new range to try and we handle that: checkpoint/src/providers/evm/provider.ts Lines 267 to 271 in 81dd6bf
|
@Sekhmet I just found the discussion, it's here: https://discord.com/channels/1088202060283007137/1088202061176381492/1113515047277318214 Haven't read it yet, but I believe we can do such change on both Starknet and EVM chains.
Ok, but if it's specific to Infura it wouldn't be ideal, we can't assume that everyone including us would use Infura |
I spent some time debugging timeouts and for some reason it very rarely happens when using curl (only been able to reproduce it while running checkpoint in background), but was able to reproduce it using simple script with node-fetch: This also happens when using address filter and topics filter, so I'm unsure if we can do something about it, I provided those PoCs to Infura for debugging. |
If you like I can send you endpoint from others providers to see if this issue is just on Infura side or not. Even if this resolve fast I think syncing all events from a chain is never going to be fast, on the chain like Arbitrum where block time is small it will become even more problematic, we most likely would need to do the change we've discussed previously to sync only relevant contracts. |
To make indexers extractable it was separated into Indexer class which instance consumer initiaties. This allows better separation (no need for pulling deps for all networks) - everything currently is exported under either evm or starknet object, it can be extracted later (we might need to extract some common things first into other package). We also have specific types for writers for each network. This could also be useful in the future if we put multiple APIs in single instance of checkpoint, it could accept indexers instead of just single indexer.
81dd6bf
to
55722a8
Compare
@bonustrack would be great if we can review this, would be nice to have refactor land on master to avoid conflicts. |
src/checkpoint.ts
Outdated
public opts?: CheckpointOptions; | ||
public schema: string; | ||
|
||
private readonly entityController: GqlEntityController; | ||
private readonly log: Logger; | ||
private readonly networkProvider: BaseProvider; | ||
private readonly indexer: EvmIndexer; |
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.
Any reason to have something EVM specific on this file?
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.
Good catch, it should be BaseIndexer
.
What do you mean by "Indexed event parameters probably don't work"? Is it about events coming from any contracts (not defined in config)? |
It seem to works well, when I compared events I can see I have one extra event, 26 instead of 25, not sure why, this is the event that I get, it's the very first event detected by Checkpoint on this contract, not sure why it's missing on Etherscan: {
"id": "0x3fBc546BC7Fcf1e6eC7dAdfe6eBCf3c3ad2713ed",
"implementation": "0xC3031A7d3326E47D49BfF9D374d74f364B29CE4D",
"deployer": "0x556B14CbdA79A36dC33FcD461a04A5BCb5dC2A70",
"tx_hash": "0xa97e863f7089dc5ee3852edaf911d5eb4098c8908ba3b9756c16164f1b5caf4d",
"created_at": 1713349560,
"created_at_block": 5717246
} |
@bonustrack it shows last 25 transactions by default, and it's 26th. There are more transactions we just index them from specific block. |
Ok actually I was looking here and seen only 25 events: https://sepolia.etherscan.io/address/0x4b4f7f64be813ccc66aefc3bfce2baa01188631c#events |
Yes, I didn't want to make it index everything, but we could. I was checking it by looking at all transactions in Etherscan starting at block 5717246 (the one we have configured) and making sure all of those are also present in Checkpoint. |
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.
tACK
Summary
Closes: #188
Closes: https://github.com/snapshot-labs/pitches/issues/29
To make indexers extractable it was separated into Indexer class which instance consumer initiaties. This allows better separation (no need for pulling deps for all networks) - everything currently is exported under either evm or starknet object, it can be extracted later (we might need to extract some common things first into other package). We also have specific types for writers for each network.
Those things are not handled right now:
Test plan