pool-indexer: support multiple V3 factories per network - #4735
pool-indexer: support multiple V3 factories per network#4735AryanGodara wants to merge 5 commits into
Conversation
|
Claude finished @AryanGodara's task in 2m 43s —— View job PR Review: pool-indexer multi-factory supportReviewed against
Findings1. Looks good
Nits (non-blocking)
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
MartinquaXD
left a comment
MartinquaXD
left a comment
There was a problem hiding this comment.
LGTM. Only nits.
Good test.
… into aryan/be-35-pool-indexer-multi-factory
jmg-duarte
left a comment
jmg-duarte
left a comment
There was a problem hiding this comment.
If I understand correctly this PR changes the semantics of most ops requiring the events from "the latest indexed" to "the latest indexed from the slower pool"
Do you foresee any issues where a pool is too slow and we get stale data because of that?
| /// Builds a URL against the pool-indexer's Uniswap V3 API (all test fixtures | ||
| /// index `mainnet`), so call sites don't repeat the route prefix by hand. | ||
| fn v3_api(path: &str) -> String { | ||
| format!("{POOL_INDEXER_HOST}/api/v1/mainnet/uniswap/v3/{path}") | ||
| } |
There was a problem hiding this comment.
If &str is always &'static str (which seems to be the case) you can use concat! or similar to avoid the String allocation
Since this is test code is not super important, but something to keep in mind for the future
|
|
||
| /// Two factories on one network are served as a single union: each factory's | ||
| /// pools show up in `/pools`, attributed to that factory. | ||
| async fn two_factories(web3: Web3) { |
There was a problem hiding this comment.
There doesn't seem to be a test ensuring the events for each pool are registered correctly, just the initialization
Should at least an event be performed for one of them and asserted? No need for a separate test
| anyhow::ensure!( | ||
| self.factories.len() == 1, | ||
| "network {}: exactly one factory per network is supported in this release, got {}", | ||
| !self.factories.is_empty(), | ||
| "network {}: at least one factory is required", | ||
| self.name, |
There was a problem hiding this comment.
You can extract Tiago's custom non-empty deserializer instead of validating post-hoc
Additionally you could extend it to remove the duplicates
3 participants
Description
Lift the one-factory-per-network cap now that the indexer is subgraph-free, so a network can index several Uniswap V3 factories and serve their pools simultaneously.
Stacked on #4734.
Changes
validate(): allow >1 factory (was exactly one); reject empty + duplicate addresses (checkpoints/pool rows are keyed by factory).factorylabel to the per-factory metrics (chunk/events/indexed_block/lag/errors).How to test