Use RequestSharing in Native price estimator

[fleupold]

Description

I noticed in the logs that we sometimes send the same native price request within extremely short intervals to our solvers (cf. logs). This is due to us not sharing inflight native price requests.

This leads to us being more annoying that necessary to solver that already have very limited incentive and interest in responding to these requests.

This PR adds our RequestSharing abstraction to this component so we remove unnecessary requests.

Changes

• Use RequestSharing for native price requsts

How to test

Added a unit test

[claude]

Claude finished @fleupold's task in 3m 32s —— View job

---

PR Review: Use RequestSharing in Native price estimator

• Read changed file and surrounding context
• Understand RequestSharing semantics
• Review for correctness & concurrency issues
• Post inline comments & summary

Verdict: LGTM ✅ — a clean, well-scoped change that correctly coalesces concurrent native-price requests.

Things I checked that hold up well:

• Sharing scope is correct. Only truly in-flight requests are de-duplicated; once a result lands in the Cache, the early cache-check (line 334) short-circuits before ever touching RequestSharing, so sequential callers are unaffected. Existing accumulative-error / no-caching tests remain valid because they run sequentially (no sharing).
• Lifetimes/'static. self.clone() inside the factory correctly produces an owned Arc so the boxed future is 'static; the leader owns the estimator clone and followers just await the Shared.
• Cancellation is safe. futures::Shared keeps the underlying request alive as long as any follower is awaiting it, so one caller dropping out doesn't cancel the shared work.
• CacheEntry: Clone satisfies the Shared output bound.
• New test is deterministic, not timing-flaky: join_all polls the first future into leader position before the others share, so expect(...).times(1) is guaranteed.

One minor, non-blocking observation posted inline: followers inherit the leader's request_timeout; harmless on the fetch_prices path (outer time::timeout) but the single-token estimate_native_price path has no outer timeout. Only matters if timeouts ever diverge per call site.

Note: I couldn't run cargo check/tests in this environment (command approval denied), so compilation relies on CI.

[claude]

Minor / non-blocking: with sharing, request_timeout (and the approximation lookup) is now captured from whichever caller starts the request (the leader). Followers that join an in-flight request inherit the leader's request_timeout rather than their own.

On the multi-token fetch_prices path this is harmless because the whole stream is wrapped in a per-caller time::timeout(timeout, ...). But the single-token estimate_native_price path (line ~438) has no such outer wrapper, so a follower's effective timeout is the leader's, not its own. In practice all callers seem to pass the same configured timeout, so this is likely a non-issue — just flagging in case timeouts ever diverge per call site.

[MartinquaXD]

Valid point. This could be worked around by using a (Address, Timeout) key to store the inflight request with. Given that in practice people use the default timeout this should not increase the number of duplicated requests much but would prevent requests with super short deadlines from messing with honest citizens using reasonable timeouts.

[jmg-duarte]

LGTM

[MartinquaXD]

Fine to merge as is as the (Address, Timeout) suggestion will likely not have a huge impact either way.