Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rate-limit Dex solver solver requests #2068

Closed
fleupold opened this issue Nov 20, 2023 · 0 comments · Fixed by #2071
Closed

Rate-limit Dex solver solver requests #2068

fleupold opened this issue Nov 20, 2023 · 0 comments · Fixed by #2071
Assignees
Labels
E:3.1 Driver Colocation See https://github.com/cowprotocol/pm/issues/14 for details good first issue Good for newcomers

Comments

@fleupold
Copy link
Contributor

The Dex solver (1Inch, Paraswap, 0x) is implemented in a way that it attempts to find a solution to each order in the batch individually (default batch size is 1)

fn solution_stream<'a>(
&'a self,
auction: &'a auction::Auction,
) -> impl stream::Stream<Item = solution::Solution> + 'a {
stream::iter(auction.orders.iter().filter_map(order::UserOrder::new))
.enumerate()
.map(|(i, order)| {
let span = tracing::info_span!("solve", order = %order.get().uid);
self.solve_order(order, &auction.tokens, auction.gas_price)
.map(move |solution| solution.map(|s| s.with_id(solution::Id(i as u64))))
.instrument(span)
})
.buffer_unordered(self.concurrent_requests.get())
.filter_map(future::ready)
}

However, it does not respect or feed-back if the API returns rate-limit errors, which can be seen in the logs here.

In this case, instead of continuing to hammer the API with requests (and causing other consumers to have less API capacity), it should back off and try again after a cool down period.

Suggested Chages

  • Add a new RateLimited variant on solvers::infra::dex::Error
  • Parse each individual dex solver's HTTP sub-error with status 429 into this error type
  • use something like shared::rate_limiter::RateLimiter inside domain::solver::dex when solving the single orders

Given that the new infra should not directly depend on the shared crate, we can either reimplement the rate_limiter logic or pass it through a boundary struct (similar to how e.g. the boundary::baseline has access to the shared::baseline_solver)

@fleupold fleupold added E:3.1 Driver Colocation See https://github.com/cowprotocol/pm/issues/14 for details good first issue Good for newcomers labels Nov 20, 2023
squadgazzz added a commit that referenced this issue Nov 30, 2023
# Description
Properly handles `429 Too Many Requests` response from DEX solvers by
utilizing `shared::RateLimiter` to back off further requests with a
cool-down period.

# Changes

- A new`RateLimited` variant on solvers::infra::dex::Error
- Parse each individual dex solver's HTTP sub-error with status 429 into
this error type
- Used `shared::rate_limiter::RateLimiter` though `solvers::boundary`
- A new config for solvers with `max_retries` for the `solvers` crate

## How to test
TBD: is it possible to simulate it in staging?

## Related Issues
Fixes #2068
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E:3.1 Driver Colocation See https://github.com/cowprotocol/pm/issues/14 for details good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants