Make metrics based bad order detection order specific

[MartinquaXD]

Description

Currently the bad token detection assumes that we are perfectly able to detect "broken" orders and only orders that trade specific tokens that a particular solver is not able to handle cause problems. However this assumption does not work well with the increasing complexity of new order types that can suddenly start failing for any number of reasons.
The most prominent recent example were flashloan orders where the EIP 1271 signature verified correctly but transferring the tokens into the settlement contract failed because the user's Aave debt position was not healthy enough.
Our current logic caused a lot of collateral damage because such orders could cause many reasonable tokens to be flagged as unsupported although the tokens themselves were perfectly fine and only that particular order was problematic.

Changes

To address this this PR change the metrics based detection mechanism to only flag on an order by order basis instead flagging all orders trading specific tokens. The change itself is relatively simple (collect metrics keyed by Uid instead of token`) but came with a few related changes:

• the name bad_token_detection is now incorrect in most (but not all!) cases so many things were renamed
  • this includes a few config parameters so they must be updated in the infra repo as well!
• caching uids has a lot more potential to bloat the cache so a cache eviction task was introduced, this required 2 new config parameters (max_age, gc_interval)

How to test

• adjusted existing unit to make sure the metrics logic still works correctly with Uid
• added a new unit test for the cache eviction

[MartinquaXD]

github unfortunately marks this whole file as new when it actually was actually just moved and modified slightly. The new parts are:

• added last_seen_at
• added spawn_gc_task (and the associated unit test)

[squadgazzz]

The refactoring/renaming should probably be separated from the logic changes to avoid this.

[squadgazzz]

This diff is much better https://www.diffchecker.com/iMNVcpf7/

[m-sz]

Infra PR https://github.com/cowprotocol/infrastructure/pull/4302

[jmg-duarte]

LGTM

[squadgazzz]

It is unfortunate that bad_orders now relate to tokens.

[m-sz]

I have moved this into common module detector

[squadgazzz]

Do we really need to move the simulation to bad_orders? It still works with tokens, doesn't it?

[m-sz]

The simulation still considers the tokens. I will re-arrange the hierarchy to reflect that and be less confusing.

[m-sz]

Moved bad_token and bad_order detection as submodules to common detector

The public facing api (Quality and the Detector) now reside inside of it. Then its submodules are bad_tokens::simulation and bad_orders::metrics which exactly describe how we rule out the specific offending trades. The rest of the crate uses only the main detector module and detector::Detector struct which makes it less confusing.

Also removed the hand rolled current_unix_timestamp() in favour of now_in_epoch_seconds()

[m-sz]

I am now left wondering how should the configuration struct be called. It's currently named BadOrderDetectionConfig and configures both the bad token detection based on simulation and bad order detection based on metrics. Calling it simply DetectorConfig could be confusing.

[jmg-duarte]

I am now left wondering how should the configuration struct be called. It's currently named BadOrderDetectionConfig and configures both the bad token detection based on simulation and bad order detection based on metrics. Calling it simply DetectorConfig could be confusing.

Suggestions:

1. FaultDetectorConfig
2. Split the configs into token/order or metrics/simulation (or both) and then create a separate one called DetectionConfigurations — its no longer ambiguous because you can open it and see both explicit ones

[squadgazzz]

I am now left wondering how should the configuration struct be called. It's currently named BadOrderDetectionConfig and configures both the bad token detection based on simulation and bad order detection based on metrics. Calling it simply DetectorConfig could be confusing.

We now have 2 detectors that live in different modules. Can we simply split their configs or do they share some config params?