Don't consider inflight orders invalid

[fafk]

Description

When I was working on another feature (the debug endpoint) I noticed that there is a race condition where we sometimes insert "invalid" order event to the DB.

From logs

09:19:13.717  stored order events label=Executing count=1
09:19:13.733  filtered orders reason=insufficient_balance count=1 orders=[0x060a...]
09:19:13.737  stored order events label=Invalid count=1
09:19:13.756  stored order events label=Traded count=1

Order is in executing state, then settles onchain. If the indexing runs now and we set the order to traded all is well. However if the in cache update runs first it's possible the wallet no longer has the necessary balance as it was traded away in the just executed swap and we consider it invalid and we insert the invalid event for an order that went through perfectly well.

Changes

• Don't consider in-flight orders invalid
• Move in-flight filtering logic from run loop to solvable_orders (move upstream)

How to test

I had an e2e test to reproduce this which failed 6/10 times. After the fix it fails 0/10.

[gemini-code-assist]

Code Review

This pull request effectively resolves a race condition where in-flight orders could be incorrectly marked as invalid. The change moves the filtering of in-flight orders to an earlier stage in solvable_orders.rs, ensuring they are removed from consideration before balance and validity checks occur. This correctly prevents orders that are being settled from being marked as invalid due to temporary balance changes. The implementation is sound and no critical issues were found.

_{Note: Security Review did not run due to the size of the PR.}

[squadgazzz]

Probably worth adding a short comment on why it is intentional to filter AMM orders by owners instead of UID.

[fafk]

I have no idea why this is the case, I just kept the original logic from the deleted code. 😅

[MartinquaXD]

Orders rebalancing cow amms revert when the cow amm does not have exactly the state the order was crafted for so having multiple orders in-flight for the same cow amm is an issue.
Additionally an amm can be rebalanced in many different ways which would all result in different order UIDs so filtering based on that is not sufficient. That's way we check if there is any order in-flight for that amm based on the owner of the order (i.e. the cow amm) and then discard that amm altogether for that auction.

[fafk]

I added the above as a comment verbatim. 🌚

[squadgazzz]

Do we need to keep this annotation?

Suggested change

	async fn fetch_in_flight_orders(&self, block: u64) -> HashSet<OrderUid> {
	#[instrument(skip_all)]
	async fn fetch_in_flight_orders(&self, block: u64) -> HashSet<OrderUid> {

[fafk]

We don't, the fn that runs the actual query is annotated. This fn that just does the remapping would just add noise.

[MartinquaXD]

Comments could be cleaned up a bit more but overall LGTM.

[MartinquaXD]

Orders rebalancing cow amms revert when the cow amm does not have exactly the state the order was crafted for so having multiple orders in-flight for the same cow amm is an issue.
Additionally an amm can be rebalanced in many different ways which would all result in different order UIDs so filtering based on that is not sufficient. That's way we check if there is any order in-flight for that amm based on the owner of the order (i.e. the cow amm) and then discard that amm altogether for that auction.