Bound RecentBlockCache by age#4587
Merged
Merged
Conversation
Contributor
|
Claude encountered an error after 2m 41s —— View job PR Review: Bound RecentBlockCache by age
|
Contributor
|
Warning Gemini encountered an error creating the review. You can try again by commenting |
jmg-duarte
approved these changes
Jul 3, 2026
jmg-duarte
left a comment
jmg-duarte
left a comment
Contributor
There was a problem hiding this comment.
LGTM, I'm looking forward to the memory savings given all the recent effort we've put in lowering the consumption
Additionally, we could consider changing the cached with moka for an additional speed boost and less unwraps in the code
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Especially on BNB we are using an insane amount of RPC calls for fetching uniswap v2 liquidity which already starts high after a restart and then grows over time until it flattens out.
As it turns out the graph flattens out when the
recently_usedset of theRecentBlockCachehits its size limit of 1000 - meaning at this point we try to keep 1000 uni v2 pools up-to-date via a background task. However, in a usual auction we actually only deliver ~500 uni v2 pools to the solvers for building their solutions. So effectively we are wasting half of those RPC requests keeping pools updated that can only be used during quoting.The reason the cache starts huge and grows even bigger is that we have a lot of long standing limit order with many different tokens (initial size) and over time market orders increase the set of used tokens even further (growth) until we hit the limit.
Changes
To keep all the relevant pools updated without suffering linear growth over time the
recently_usedset is now also bounded to 5 minutes. That way an order requiring new liquidity will have plenty of time to get settled and soon after the now irrelevant AMM stops getting updated.Additionally I reduced the number of blocks a cache snapshot is kept alive to 2 (from 10). This should effectively reduce the cache size by 80%. This should also not cause any issues with liquidity that gets flushed out too soon.
This long lookback was primarily intended to have liquidity quickly available for quoting when the liquidity caches were fragmented across different orderbook instances but since they now live in the driver and the driver keeps a ton of highly relevant liquidity up-to-date for the auctions we'll still get a high cache hit rate with shorter lookback.
How to test
test run in prod to verify constant RPC calls and reduced memory usage.
I also updated the quoting with stale liquidity e2e test since we now have to unbalance the pool with a single block to uphold the original assertion with a recent block cache depth of 2 blocks.