CoW AMM indexer DB storage #3684
Conversation
|
Reminder: Please update the DB Readme and comment whether migrations are reversible (include rollback scripts if applicable). Caused by: |
# Conflicts: # crates/driver/src/domain/competition/pre_processing.rs # crates/driver/src/infra/api/routes/solve/dto/solve_request.rs # crates/e2e/src/setup/deploy.rs
# Conflicts: # crates/driver/src/infra/blockchain/mod.rs # crates/driver/src/run.rs # crates/driver/src/tests/setup/solver.rs
|
Reminder: Please update the DB Readme and comment whether migrations are reversible (include rollback scripts if applicable).
Caused by: |
MartinquaXD
left a comment
MartinquaXD
left a comment
There was a problem hiding this comment.
Looks already good over all. Just a couple small comments.
database/README.md
Outdated
| limit | Long lived order that may receive surplus. Users sign a static fee of 0 upfront and either the backend or the solvers compute a dynamic fee that gets taken from the surplus (while still respecting the user's limit price!). | ||
|
|
||
|
|
||
| ### cow\_amms |
There was a problem hiding this comment.
cow_amms is a new table but shows up in the enum section and doesn't follow the same format as other tables.
There was a problem hiding this comment.
Addressed all the comments.
| let mut ex = self.0.db.begin().await?; | ||
| database::cow_amms::upsert_batched(&mut ex, &db_amms).await?; |
There was a problem hiding this comment.
Good that you do the DB stuff first to avoid getting the in-memory cache into a weird state when inserting there works but the DB query fails. 👍
# Conflicts: # crates/driver/src/infra/blockchain/contracts.rs # crates/e2e/src/setup/deploy.rs
MartinquaXD
left a comment
MartinquaXD
left a comment
There was a problem hiding this comment.
Approved assuming the one new comment gets addressed.
# Conflicts: # crates/e2e/src/setup/deploy.rs
crates/cow-amm/src/cache.rs
Outdated
| database::last_indexed_blocks::update( | ||
| &mut ex, | ||
| &self.0.factory_address.to_string(), | ||
| i64::try_from(latest_block).context("last block is not u64")?, |
There was a problem hiding this comment.
This error is confusing, the integer may be a u64 but it's bigger than 1 << 32 - 1 (the positive side of the integer)
There was a problem hiding this comment.
We have it all around the code base. If the function signature changes, the error message won't be valid anymore with your suggestion.
crates/cow-amm/src/cache.rs
Outdated
| cache.entry(block_number).or_default().push(amm); | ||
| } | ||
| tracing::info!( | ||
| count, |
There was a problem hiding this comment.
This would allow you to remove the count variable above
| count, | |
| count = %processed_amms.len(), |
There was a problem hiding this comment.
It won't work since the processed_amms is being moved above.
| let mut ex = self.0.db.acquire().await?; | ||
| database::last_indexed_blocks::fetch(&mut ex, &self.0.factory_address.to_string()) | ||
| .await? | ||
| .map(|block| block.try_into().context("last block is not u64")) |
# Conflicts: # Cargo.lock
Description
Currently, on each service restart, CoW AMM indexing starts from the AMM Factory SC's deployment block, since all the indexed events are stored in in-memory cache. This has become problematic on some chains that actively use AMMs, since on each restart, the service has to re-index all the events. As a result, this consumes a lot of RPC resources, and the overall protocol performance drops drastically. On Base, for example, it might take 2-3h.
Changes
This PR introduces a persistence layer for the CoW AMM indexing, which effectively stores all the indexed AMMs and the last indexed block, so on the next restart, it can continue from the last indexed block.
last_indexed_blockswill now contain information per CoW AMM factory, which is required for initialization of each indexer. If no data is found, use the configured deployment block.How to test
Existing tests. Updated postgres tests. Staging.