Skip to content

Restructure logic that assembles the final auction - #4685

Draft
MartinquaXD wants to merge 9 commits into
mainfrom
solvable-orders-improvements
Draft

Restructure logic that assembles the final auction#4685
MartinquaXD wants to merge 9 commits into
mainfrom
solvable-orders-improvements

Conversation

@MartinquaXD

Copy link
Copy Markdown
Contributor

Description

The current logic to assemble orders tries to be modular which causes a few performance issues. Some issues are:

  • multiple filters iterate over huge orders vector multiple times
  • filters use retain multiple times - because retain preserves order it can be quite slow if you for example filter out the very first item in the vector

Changes

Restructure the main logic.
Before we even collect the first iterator we already apply as many filters as possible as a few don't require any additional data. This already leaves us with a smaller collection at the start (in number of items, not memory footprint).
While we apply the filters we already store the UID + filter reason in separate vectors for later reporting. We also already collect all the data we need for fetching the remaining data for the next filtering step.
With all the data we collected we can now run ALL async tasks that fetch the final set of data concurrently instead of spreading that around.
One we have the final data we can already .into_iter().filter_map().collect() and skip ALL .retain() calls.

After all is said and done we have a single function call to update metrics, log filtered orders and upload order events to the database.

Besides restructuring the loops I also used alloy's FbHasher for maps and sets where the key has a fixed size (Address, OrderUid). Microbenchmarks showed a >10x speed up compared to rust's default hasher. (I imagine in the grand scheme of things this will not do much but the change was dead simple so why not?).

Possible improvements that are out of scope of this PR:

  • log filtered orders in a background task
  • use rayon for parallelizing the loops (now possible in the first place because we don't do retain anymore)
  • fetch cached balances and banned users before the first filtering loop - usually all the data we need is already cached so we should be able to skip the second loop altogether most of the time.

How to test

Existing tests were adjusted but more tests are probably needed.

@MartinquaXD

Copy link
Copy Markdown
Contributor Author

@claude

@claude

claude Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Claude finished @MartinquaXD's task in 4m 34s —— View job


Review

Nice restructure — collapsing the multiple retain passes into one pre-filter loop + concurrent I/O + a single filter_map is a clear improvement, and moving observability into FilteredOrders::report reads well. I focused on correctness of the new filtering logic. Left 4 inline comments; one is a blocking bug.

Findings

  • 🔴 passes_balance partial-fill condition is inverted (balance.is_zero() where the original used balance >= U256::ONE). This drops every partially fillable order that isn't fully funded and lets zero-balance ones through — and it makes the new passes_balance_covers_all_cases test fail. comment
  • 🟡 Orders with a missing balance entry (failed balance fetch) are now silently dropped — no metric, no order event. comment
  • 🟡 auction_market_order_missing_price is now a dead metric — nothing sets it after removing get_orders_with_native_prices. comment
  • 🟡 Native prices are now cache-only (no synchronous fetch / native_price_timeout removed), delaying orders for freshly-seen tokens by a cycle — confirm intended. comment

The FbHasher swaps and the banned/deny-list AddressHashSet conversions look correct.

Todo

  • Read the changed files & understand the restructure
  • Analyze solvable_orders.rs rewrite for correctness
  • Check supporting changes (native_price_cache, banned, bad-tokens)
  • Post inline review comments
  • Post summary
    · branch solvable-orders-improvements

Comment thread crates/autopilot/src/solvable_orders.rs Outdated
Comment thread crates/autopilot/src/solvable_orders.rs Outdated
Comment thread crates/autopilot/src/solvable_orders.rs
Comment thread crates/autopilot/src/solvable_orders.rs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant