Open
Conversation
…ost context calculation - Added method to for determining flow keys based on custody decisions. - Updated to utilize flow keys for cost calculations. - Refactored cost estimation logic to support multiple flow keys. - Integrated flow key resolution into the quote processing pipeline for improved accuracy.
nahimterrazas
requested review from
NicoMolinaOZ,
pepebndc and
shahnami
as code owners
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughThis PR adds flow-key–aware cost calculation and caching across the quote pipeline, introduces concurrent gas-price and USD-conversion paths, improves CoinGecko rate-limiting and cache-miss collapse, and refines settlement proof-delay polling and broadcaster readiness ordering. Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant QuoteAPI as Quote API
participant QuoteGen as QuoteGenerator
participant CostProfit as CostProfitService
participant Config as Config
participant Delivery as DeliveryService
participant Pricing as PricingService
Client->>QuoteAPI: process_quote_request
QuoteAPI->>QuoteGen: resolve_quote_flow_keys(request)
QuoteGen->>QuoteGen: convert inputs -> custody decisions -> flow keys
QuoteGen-->>QuoteAPI: Vec<String> flow_keys
QuoteAPI->>CostProfit: calculate_cost_context_for_flow_keys(request, context, config, flow_keys)
CostProfit->>Config: estimate_gas_units_from_flow_keys(flow_keys, config)
Config-->>CostProfit: (open, fill, claim) gas units
CostProfit->>Delivery: get_gas_price(origin_chain)
CostProfit->>Delivery: get_gas_price(dest_chain)
Note right of CostProfit: origin/dest gas-price fetched concurrently
CostProfit->>Pricing: wei_to_usd_or_zero(...) / value conversions
Pricing-->>CostProfit: USD values (concurrent, fall back to 0 on failure)
CostProfit-->>QuoteAPI: CostContext
QuoteAPI-->>Client: quote response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/solver-core/src/engine/cost_profit.rs (1)
392-400:⚠️ Potential issue | 🟠 MajorKeep the old public entrypoint on the same flow-resolution path.
calculate_cost_context()still derives gas flows fromrequest.flow_key(), which is a request-shape heuristic and defaults topermit2_escrow. The quote API now resolves flows from actual custody decisions, so the same request can produce different cost models depending on which public method a caller hits.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/solver-core/src/engine/cost_profit.rs` around lines 392 - 400, calculate_cost_context currently derives flow keys directly from request.flow_key(), diverging from the custody-based flow resolution used elsewhere; update calculate_cost_context to obtain flow_keys via the same custody/quote flow resolver used by the other public entrypoint (the custody-based flow-resolution codepath) and then pass those resolved flow_keys into calculate_cost_context_for_flow_keys(request, context, config, &flow_keys). Ensure you call the shared resolver (the same function/class used by the other public API) instead of request.flow_key() so both entrypoints follow the identical flow-resolution path.
🧹 Nitpick comments (1)
crates/solver-pricing/src/implementations/coingecko.rs (1)
45-46: Consider cleanup strategy forprice_fetch_locksto prevent unbounded growth.The
price_fetch_locksHashMap accumulates entries for each unique cache key but never removes them. In long-running processes querying many different tokens, this could lead to memory growth. Consider periodically pruning stale entries or using a bounded cache structure.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/solver-pricing/src/implementations/coingecko.rs` around lines 45 - 46, The per-key lock map price_fetch_locks (Arc<Mutex<HashMap<String, Arc<Mutex<()>>>>>) can grow unbounded; after you create/lookup the per-key lock and finish the critical section, remove entries for keys that are no longer in use (or replace the map with a bounded/evicting cache). Concretely, in the function that inserts/uses the lock (the code path that does the lookup/insert and then awaits the Arc<Mutex<()>>), after releasing the per-key mutex check Arc::strong_count(&per_key_arc) and if it is 1 (only the map holds it) remove the key from the inner HashMap; alternatively swap the HashMap for an eviction/bounded cache (e.g., moka/LruCache/DashMap with TTL/size limit) and use that for price_fetch_locks to prevent unbounded growth.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/solver-core/src/engine/cost_profit.rs`:
- Around line 1173-1180: The helper wei_to_usd_or_zero currently masks pricing
failures by returning Decimal::ZERO; change it to propagate errors instead of
zeroing out values so gas_open/gas_fill/gas_claim calculations fail on
conversion issues. Update wei_to_usd_or_zero to return a Result<Decimal, _> (or
Option<Decimal>) and replace the unwrap_or_else call with the ? operator when
calling pricing_service.wei_to_currency and bubble the parse error from
Decimal::from_str back to callers; then adjust callers that use
wei_to_usd_or_zero (e.g., where gas_open, gas_fill, gas_claim are computed) to
handle the error/result and abort or propagate instead of treating the value as
$0.
- Around line 1563-1607: estimate_gas_units_from_flow_keys currently seeds
open/fill/claim with the fallback and then takes max(), which prevents using any
configured value below the fallback; change the logic to accumulate configured
values independently and only apply the fallback if no configured value exists.
Concretely, in estimate_gas_units_from_flow_keys use Option<u64> (e.g. open_opt,
fill_opt, claim_opt) instead of seeding with fallback, update each Option by
taking the max of Some(values) when you find units.open/fill/claim (keeping
existing unwrap_or(fallback_*) for absent per-unit fields), set found if any
flow matched, and at the end return (open_opt.unwrap_or(fallback_open),
fill_opt.unwrap_or(fallback_fill), claim_opt.unwrap_or(fallback_claim)) so
configured lower values are preserved.
In `@crates/solver-service/src/apis/quote/generation.rs`:
- Around line 126-145: resolve_quote_flow_keys can return multiple flow keys
(e.g., both eip3009_escrow and permit2_escrow) which leads to downstream code
like generate_eip3009_order incorrectly applying one custody decision to all
inputs; detect mixed custody earlier and fail fast or partition the request.
Update resolve_quote_flow_keys (and/or call sites that use
flow_key_for_custody_decision) to detect when more than one distinct custody
flow_key is required and return a QuoteError (e.g., MixedCustodyRequest) or
split the request into per-custody batches so each batch has a single custody
decision; ensure the error type and handling are wired into the callers that
expect a single CustodyDecision (such as generate_eip3009_order) so
mixed-custody requests are rejected or properly partitioned before quote
generation.
- Around line 1285-1290: The current code silently substitutes "Unknown Token"
when self.get_cached_token_name(...) returns Err, which yields an incorrect
EIP-3009 domain name; instead make the token name resolution fail fast and
propagate the error so quote building aborts: remove the unwrap_or_else fallback
for token_name (and similarly ensure get_token_eip712_version errors are
propagated) and return or propagate the Err from the surrounding function (the
code block using token_name, token_version, and building the EIP-3009 domain) so
a missing name prevents signature creation.
In `@crates/solver-service/src/apis/quote/mod.rs`:
- Around line 109-113: The code is doing custody resolution twice:
QuoteGenerator::resolve_quote_flow_keys(...) already resolves custody for every
input but generate_quotes_with_costs() → generate_quotes() repeats the async
custody loop, doubling work and latency; modify the QuoteGenerator API so
generate_quotes_with_costs()/generate_quotes() accept the precomputed
resolved_flow_keys (or a flag to skip resolution) and remove the second custody
traversal in generate_quotes(), ensuring QuoteGenerator::new(...) (and its use
of settlement() and delivery()) reuses the resolved_flow_keys produced earlier
instead of rerunning the async custody resolution.
---
Outside diff comments:
In `@crates/solver-core/src/engine/cost_profit.rs`:
- Around line 392-400: calculate_cost_context currently derives flow keys
directly from request.flow_key(), diverging from the custody-based flow
resolution used elsewhere; update calculate_cost_context to obtain flow_keys via
the same custody/quote flow resolver used by the other public entrypoint (the
custody-based flow-resolution codepath) and then pass those resolved flow_keys
into calculate_cost_context_for_flow_keys(request, context, config, &flow_keys).
Ensure you call the shared resolver (the same function/class used by the other
public API) instead of request.flow_key() so both entrypoints follow the
identical flow-resolution path.
---
Nitpick comments:
In `@crates/solver-pricing/src/implementations/coingecko.rs`:
- Around line 45-46: The per-key lock map price_fetch_locks
(Arc<Mutex<HashMap<String, Arc<Mutex<()>>>>>) can grow unbounded; after you
create/lookup the per-key lock and finish the critical section, remove entries
for keys that are no longer in use (or replace the map with a bounded/evicting
cache). Concretely, in the function that inserts/uses the lock (the code path
that does the lookup/insert and then awaits the Arc<Mutex<()>>), after releasing
the per-key mutex check Arc::strong_count(&per_key_arc) and if it is 1 (only the
map holds it) remove the key from the inner HashMap; alternatively swap the
HashMap for an eviction/bounded cache (e.g., moka/LruCache/DashMap with TTL/size
limit) and use that for price_fetch_locks to prevent unbounded growth.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e6580d53-f6ef-4ce1-876d-a8ba53e48eb2
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
crates/solver-core/src/engine/cost_profit.rscrates/solver-core/src/monitoring/settlement.rscrates/solver-pricing/Cargo.tomlcrates/solver-pricing/src/implementations/coingecko.rscrates/solver-service/src/apis/quote/generation.rscrates/solver-service/src/apis/quote/mod.rscrates/solver-settlement/src/implementations/broadcaster.rs
Comment on lines
+1173
to
+1180
| async fn wei_to_usd_or_zero(&self, value_wei: &U256) -> Decimal { | ||
| let usd_value = self | ||
| .pricing_service | ||
| .wei_to_currency(&value_wei.to_string(), "USD") | ||
| .await | ||
| .unwrap_or_else(|_| "0".to_string()); | ||
|
|
||
| Decimal::from_str(&usd_value).unwrap_or(Decimal::ZERO) |
There was a problem hiding this comment.
Do not zero gas costs when ETH/USD conversion fails.
This helper turns pricing failures into $0, so a temporary wei_to_currency() outage now removes gas_open, gas_fill, and gas_claim from the quote instead of failing the calculation. That undercharges users and can make unprofitable orders look acceptable.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/solver-core/src/engine/cost_profit.rs` around lines 1173 - 1180, The
helper wei_to_usd_or_zero currently masks pricing failures by returning
Decimal::ZERO; change it to propagate errors instead of zeroing out values so
gas_open/gas_fill/gas_claim calculations fail on conversion issues. Update
wei_to_usd_or_zero to return a Result<Decimal, _> (or Option<Decimal>) and
replace the unwrap_or_else call with the ? operator when calling
pricing_service.wei_to_currency and bubble the parse error from
Decimal::from_str back to callers; then adjust callers that use
wei_to_usd_or_zero (e.g., where gas_open, gas_fill, gas_claim are computed) to
handle the error/result and abort or propagate instead of treating the value as
$0.
Comment on lines
+1563
to
+1607
| pub fn estimate_gas_units_from_flow_keys( | ||
| flow_keys: &[String], | ||
| config: &Config, | ||
| fallback_open: u64, | ||
| fallback_fill: u64, | ||
| fallback_claim: u64, | ||
| ) -> (u64, u64, u64) { | ||
| if flow_keys.is_empty() { | ||
| return (fallback_open, fallback_fill, fallback_claim); | ||
| } | ||
|
|
||
| if let Some(gcfg) = config.gas.as_ref() { | ||
| tracing::debug!( | ||
| "Available gas flows: {:?}", | ||
| gcfg.flows.keys().collect::<Vec<_>>() | ||
| ); | ||
|
|
||
| let mut found = false; | ||
| let mut open = fallback_open; | ||
| let mut fill = fallback_fill; | ||
| let mut claim = fallback_claim; | ||
|
|
||
| for flow in flow_keys { | ||
| if let Some(units) = gcfg.flows.get(flow.as_str()) { | ||
| found = true; | ||
| open = open.max(units.open.unwrap_or(fallback_open)); | ||
| fill = fill.max(units.fill.unwrap_or(fallback_fill)); | ||
| claim = claim.max(units.claim.unwrap_or(fallback_claim)); | ||
| } else { | ||
| tracing::warn!("Flow '{}' not found in gas config flows", flow); | ||
| } | ||
| } | ||
|
|
||
| if found { | ||
| return (open, fill, claim); | ||
| } | ||
| } | ||
|
|
||
| tracing::warn!( | ||
| "No gas config found for flows {:?}, using fallback estimates", | ||
| flow_keys | ||
| ); | ||
|
|
||
| (fallback_open, fallback_fill, fallback_claim) | ||
| } |
There was a problem hiding this comment.
The fallback seed makes lower configured gas values impossible to use.
open, fill, and claim start at the fallback and then take max(), so any configured value below the fallback is ignored. With the current 150k call-site fallback, a flow configured at 80k/90k/100k still resolves to 150k/150k/150k, which is a behavior change even for a single matching flow.
Suggested fix
if let Some(gcfg) = config.gas.as_ref() {
tracing::debug!(
"Available gas flows: {:?}",
gcfg.flows.keys().collect::<Vec<_>>()
);
let mut found = false;
- let mut open = fallback_open;
- let mut fill = fallback_fill;
- let mut claim = fallback_claim;
+ let mut open = None;
+ let mut fill = None;
+ let mut claim = None;
for flow in flow_keys {
if let Some(units) = gcfg.flows.get(flow.as_str()) {
found = true;
- open = open.max(units.open.unwrap_or(fallback_open));
- fill = fill.max(units.fill.unwrap_or(fallback_fill));
- claim = claim.max(units.claim.unwrap_or(fallback_claim));
+ open = Some(open.unwrap_or(0).max(units.open.unwrap_or(fallback_open)));
+ fill = Some(fill.unwrap_or(0).max(units.fill.unwrap_or(fallback_fill)));
+ claim = Some(claim.unwrap_or(0).max(units.claim.unwrap_or(fallback_claim)));
} else {
tracing::warn!("Flow '{}' not found in gas config flows", flow);
}
}
if found {
- return (open, fill, claim);
+ return (
+ open.unwrap_or(fallback_open),
+ fill.unwrap_or(fallback_fill),
+ claim.unwrap_or(fallback_claim),
+ );
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| pub fn estimate_gas_units_from_flow_keys( | |
| flow_keys: &[String], | |
| config: &Config, | |
| fallback_open: u64, | |
| fallback_fill: u64, | |
| fallback_claim: u64, | |
| ) -> (u64, u64, u64) { | |
| if flow_keys.is_empty() { | |
| return (fallback_open, fallback_fill, fallback_claim); | |
| } | |
| if let Some(gcfg) = config.gas.as_ref() { | |
| tracing::debug!( | |
| "Available gas flows: {:?}", | |
| gcfg.flows.keys().collect::<Vec<_>>() | |
| ); | |
| let mut found = false; | |
| let mut open = fallback_open; | |
| let mut fill = fallback_fill; | |
| let mut claim = fallback_claim; | |
| for flow in flow_keys { | |
| if let Some(units) = gcfg.flows.get(flow.as_str()) { | |
| found = true; | |
| open = open.max(units.open.unwrap_or(fallback_open)); | |
| fill = fill.max(units.fill.unwrap_or(fallback_fill)); | |
| claim = claim.max(units.claim.unwrap_or(fallback_claim)); | |
| } else { | |
| tracing::warn!("Flow '{}' not found in gas config flows", flow); | |
| } | |
| } | |
| if found { | |
| return (open, fill, claim); | |
| } | |
| } | |
| tracing::warn!( | |
| "No gas config found for flows {:?}, using fallback estimates", | |
| flow_keys | |
| ); | |
| (fallback_open, fallback_fill, fallback_claim) | |
| } | |
| pub fn estimate_gas_units_from_flow_keys( | |
| flow_keys: &[String], | |
| config: &Config, | |
| fallback_open: u64, | |
| fallback_fill: u64, | |
| fallback_claim: u64, | |
| ) -> (u64, u64, u64) { | |
| if flow_keys.is_empty() { | |
| return (fallback_open, fallback_fill, fallback_claim); | |
| } | |
| if let Some(gcfg) = config.gas.as_ref() { | |
| tracing::debug!( | |
| "Available gas flows: {:?}", | |
| gcfg.flows.keys().collect::<Vec<_>>() | |
| ); | |
| let mut found = false; | |
| let mut open = None; | |
| let mut fill = None; | |
| let mut claim = None; | |
| for flow in flow_keys { | |
| if let Some(units) = gcfg.flows.get(flow.as_str()) { | |
| found = true; | |
| open = Some(open.unwrap_or(0).max(units.open.unwrap_or(fallback_open))); | |
| fill = Some(fill.unwrap_or(0).max(units.fill.unwrap_or(fallback_fill))); | |
| claim = Some(claim.unwrap_or(0).max(units.claim.unwrap_or(fallback_claim))); | |
| } else { | |
| tracing::warn!("Flow '{}' not found in gas config flows", flow); | |
| } | |
| } | |
| if found { | |
| return ( | |
| open.unwrap_or(fallback_open), | |
| fill.unwrap_or(fallback_fill), | |
| claim.unwrap_or(fallback_claim), | |
| ); | |
| } | |
| } | |
| tracing::warn!( | |
| "No gas config found for flows {:?}, using fallback estimates", | |
| flow_keys | |
| ); | |
| (fallback_open, fallback_fill, fallback_claim) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/solver-core/src/engine/cost_profit.rs` around lines 1563 - 1607,
estimate_gas_units_from_flow_keys currently seeds open/fill/claim with the
fallback and then takes max(), which prevents using any configured value below
the fallback; change the logic to accumulate configured values independently and
only apply the fallback if no configured value exists. Concretely, in
estimate_gas_units_from_flow_keys use Option<u64> (e.g. open_opt, fill_opt,
claim_opt) instead of seeding with fallback, update each Option by taking the
max of Some(values) when you find units.open/fill/claim (keeping existing
unwrap_or(fallback_*) for absent per-unit fields), set found if any flow
matched, and at the end return (open_opt.unwrap_or(fallback_open),
fill_opt.unwrap_or(fallback_fill), claim_opt.unwrap_or(fallback_claim)) so
configured lower values are preserved.
Comment on lines
+126
to
+145
| pub(crate) async fn resolve_quote_flow_keys( | ||
| &self, | ||
| request: &GetQuoteRequest, | ||
| ) -> Result<Vec<String>, QuoteError> { | ||
| let mut flow_keys = Vec::new(); | ||
|
|
||
| for input in &request.intent.inputs { | ||
| let order_input: OrderInput = input.try_into()?; | ||
| let custody_decision = self | ||
| .custody_strategy | ||
| .decide_custody(&order_input, request.intent.origin_submission.as_ref()) | ||
| .await?; | ||
| let flow_key = Self::flow_key_for_custody_decision(&custody_decision)?; | ||
| if !flow_keys.contains(&flow_key) { | ||
| flow_keys.push(flow_key); | ||
| } | ||
| } | ||
|
|
||
| Ok(flow_keys) | ||
| } |
There was a problem hiding this comment.
Reject or partition mixed-custody requests before returning multiple flow keys.
This method can now return both eip3009_escrow and permit2_escrow, but downstream quote generation still applies a single CustodyDecision to the entire request. For a mixed request, one pass will emit an EIP-3009 quote over inputs that are only valid under Permit2 (and generate_eip3009_order() signs all inputs under the first token’s domain), so the API can return unusable quotes instead of failing fast.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/solver-service/src/apis/quote/generation.rs` around lines 126 - 145,
resolve_quote_flow_keys can return multiple flow keys (e.g., both eip3009_escrow
and permit2_escrow) which leads to downstream code like generate_eip3009_order
incorrectly applying one custody decision to all inputs; detect mixed custody
earlier and fail fast or partition the request. Update resolve_quote_flow_keys
(and/or call sites that use flow_key_for_custody_decision) to detect when more
than one distinct custody flow_key is required and return a QuoteError (e.g.,
MixedCustodyRequest) or split the request into per-custody batches so each batch
has a single custody decision; ensure the error type and handling are wired into
the callers that expect a single CustodyDecision (such as
generate_eip3009_order) so mixed-custody requests are rejected or properly
partitioned before quote generation.
Comment on lines
+1285
to
1290
| let (token_name, token_version) = tokio::join!( | ||
| self.get_cached_token_name(&alloy_token_address, chain_id), | ||
| self.get_token_eip712_version(&alloy_token_address, chain_id), | ||
| ); | ||
| let token_name = token_name.unwrap_or_else(|_| "Unknown Token".to_string()); | ||
|
|
There was a problem hiding this comment.
Do not synthesize an EIP-3009 domain name.
If name() fails, Line 1289 falls back to "Unknown Token" and still builds the quote. Those domain fields are signed, so the separator no longer matches the token contract and the quote becomes unusable. This should fail fast instead.
Suggested fix
let (token_name, token_version) = tokio::join!(
self.get_cached_token_name(&alloy_token_address, chain_id),
self.get_token_eip712_version(&alloy_token_address, chain_id),
);
- let token_name = token_name.unwrap_or_else(|_| "Unknown Token".to_string());
+ let token_name = token_name?;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/solver-service/src/apis/quote/generation.rs` around lines 1285 - 1290,
The current code silently substitutes "Unknown Token" when
self.get_cached_token_name(...) returns Err, which yields an incorrect EIP-3009
domain name; instead make the token name resolution fail fast and propagate the
error so quote building aborts: remove the unwrap_or_else fallback for
token_name (and similarly ensure get_token_eip712_version errors are propagated)
and return or propagate the Err from the surrounding function (the code block
using token_name, token_version, and building the EIP-3009 domain) so a missing
name prevents signature creation.
Comment on lines
+109
to
+113
| let settlement_service = solver.settlement(); | ||
| let delivery_service = solver.delivery(); | ||
| let quote_generator = QuoteGenerator::new(settlement_service.clone(), delivery_service.clone()); | ||
| let resolved_flow_keys = quote_generator.resolve_quote_flow_keys(&request).await?; | ||
|
|
There was a problem hiding this comment.
Reuse custody resolution instead of doing a second pass here.
resolve_quote_flow_keys() already walks every input and resolves custody, but generate_quotes_with_costs() later reaches generate_quotes(), which repeats the same async custody loop before building quotes. That turns each quote request into two full custody passes on the hot path and adds avoidable latency.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/solver-service/src/apis/quote/mod.rs` around lines 109 - 113, The code
is doing custody resolution twice: QuoteGenerator::resolve_quote_flow_keys(...)
already resolves custody for every input but generate_quotes_with_costs() →
generate_quotes() repeats the async custody loop, doubling work and latency;
modify the QuoteGenerator API so generate_quotes_with_costs()/generate_quotes()
accept the precomputed resolved_flow_keys (or a flag to skip resolution) and
remove the second custody traversal in generate_quotes(), ensuring
QuoteGenerator::new(...) (and its use of settlement() and delivery()) reuses the
resolved_flow_keys produced earlier instead of rerunning the async custody
resolution.
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Testing Process
Checklist
Summary by CodeRabbit
New Features
Improvements
Tests / Chores