Skip to content

[streaming-quote-api] Extract reusable assemble_quote_data (2/3) #4468

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.

Sign up for GitHub

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

Open
squadgazzz wants to merge 2 commits into
base: feat/streaming-quote-competition
Choose a base branch
from
+132 −35
Open

[streaming-quote-api] Extract reusable assemble_quote_data (2/3) #4468

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bc4e7a2
refactor(shared): extract reusable assemble_quote_data
squadgazzz Jun 3, 2026
da48ff4
refactor(shared): tighten assemble_quote_data visibility and cover bu…
squadgazzz Jun 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
167 changes: 132 additions & 35 deletions crates/shared/src/order_quoting.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -484,43 +484,13 @@ impl OrderQuoter {
.map_err(|err| (EstimatorKind::NativeBuy, err).into()),
)?;

let (quoted_sell_amount, quoted_buy_amount) = match &parameters.side {
OrderQuoteSide::Sell {
sell_amount: SellAmount::BeforeFee { value: sell_amount },
}
| OrderQuoteSide::Sell {
sell_amount: SellAmount::AfterFee { value: sell_amount },
} => (sell_amount.get(), trade_estimate.out_amount),
OrderQuoteSide::Buy {
buy_amount_after_fee: buy_amount,
} => (trade_estimate.out_amount, buy_amount.get()),
};

let fee_parameters = FeeParameters {
gas_amount: trade_estimate.gas as f64,
gas_price: effective_gas_price as f64,
let quote = assemble_quote_data(
parameters,
&trade_estimate,
effective_gas_price,
sell_token_price,
};

let quote_kind = quote_kind_from_signing_scheme(&parameters.signing_scheme);
let quote = QuoteData {
sell_token: parameters.sell_token,
buy_token: parameters.buy_token,
quoted_sell_amount,
quoted_buy_amount,
fee_parameters,
kind: trade_query.kind,
expiration,
quote_kind,
solver: trade_estimate.solver,
verified: trade_estimate.verified,
metadata: QuoteMetadataV1 {
interactions: trade_estimate.execution.interactions,
pre_interactions: trade_estimate.execution.pre_interactions,
jit_orders: trade_estimate.execution.jit_orders,
}
.into(),
};
);
Comment on lines +487 to +493

@gemini-code-assist gemini-code-assist Bot Jun 5, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Pass trade_estimate by value to assemble_quote_data to avoid unnecessary cloning of execution metadata.

        let quote = assemble_quote_data(
            parameters,
            trade_estimate,
            effective_gas_price,
            sell_token_price,
            expiration,
        );
References
  1. Prefer passing a value (by cloning at the call site) over passing a reference if the called function requires ownership and would have to clone the value internally anyway.


Ok(quote)
}
Expand Down Expand Up @@ -659,6 +629,55 @@ pub fn quote_kind_from_signing_scheme(scheme: &QuoteSigningScheme) -> QuoteKind
}
}

/// Assembles a `QuoteData` from a completed price estimate and its inputs.
///
/// `expiration` must be computed by the caller (it depends on `&self` state in
/// `OrderQuoter`).
fn assemble_quote_data(
parameters: &QuoteParameters,
estimate: &price_estimation::Estimate,
effective_gas_price: u128,
sell_token_price: f64,
expiration: DateTime<Utc>,
) -> QuoteData {
let (quoted_sell_amount, quoted_buy_amount, kind) = match &parameters.side {
OrderQuoteSide::Sell {
sell_amount: SellAmount::BeforeFee { value: sell_amount },
}
| OrderQuoteSide::Sell {
sell_amount: SellAmount::AfterFee { value: sell_amount },
} => (sell_amount.get(), estimate.out_amount, OrderKind::Sell),
OrderQuoteSide::Buy {
buy_amount_after_fee: buy_amount,
} => (estimate.out_amount, buy_amount.get(), OrderKind::Buy),
};

let fee_parameters = FeeParameters {
gas_amount: estimate.gas as f64,
gas_price: effective_gas_price as f64,
sell_token_price,
};

QuoteData {
sell_token: parameters.sell_token,
buy_token: parameters.buy_token,
quoted_sell_amount,
quoted_buy_amount,
fee_parameters,
kind,
expiration,
quote_kind: quote_kind_from_signing_scheme(&parameters.signing_scheme),
solver: estimate.solver,
verified: estimate.verified,
metadata: QuoteMetadataV1 {
interactions: estimate.execution.interactions.clone(),
pre_interactions: estimate.execution.pre_interactions.clone(),
jit_orders: estimate.execution.jit_orders.clone(),
}
.into(),
}
}
Comment on lines +636 to +679

@gemini-code-assist gemini-code-assist Bot Jun 5, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Change assemble_quote_data to accept estimate by value. This avoids cloning interactions, pre_interactions, and jit_orders vectors, which are heap-allocated and can be moved directly.

fn assemble_quote_data(
    parameters: &QuoteParameters,
    estimate: price_estimation::Estimate,
    effective_gas_price: u128,
    sell_token_price: f64,
    expiration: DateTime<Utc>,
) -> QuoteData {
    let (quoted_sell_amount, quoted_buy_amount, kind) = match &parameters.side {
        OrderQuoteSide::Sell {
            sell_amount: SellAmount::BeforeFee { value: sell_amount },
        }
        | OrderQuoteSide::Sell {
            sell_amount: SellAmount::AfterFee { value: sell_amount },
        } => (sell_amount.get(), estimate.out_amount, OrderKind::Sell),
        OrderQuoteSide::Buy {
            buy_amount_after_fee: buy_amount,
        } => (estimate.out_amount, buy_amount.get(), OrderKind::Buy),
    };

    let fee_parameters = FeeParameters {
        gas_amount: estimate.gas as f64,
        gas_price: effective_gas_price as f64,
        sell_token_price,
    };

    QuoteData {
        sell_token: parameters.sell_token,
        buy_token: parameters.buy_token,
        quoted_sell_amount,
        quoted_buy_amount,
        fee_parameters,
        kind,
        expiration,
        quote_kind: quote_kind_from_signing_scheme(&parameters.signing_scheme),
        solver: estimate.solver,
        verified: estimate.verified,
        metadata: QuoteMetadataV1 {
            interactions: estimate.execution.interactions,
            pre_interactions: estimate.execution.pre_interactions,
            jit_orders: estimate.execution.jit_orders,
        }
        .into(),
    }
}
References
  1. Prefer passing a value (by cloning at the call site) over passing a reference if the called function requires ownership and would have to clone the value internally anyway.


/// Used to store quote metadata in the database.
/// Versioning is used for the backward compatibility.
/// In case new metadata needs to be associated with a quote create a new
Expand Down Expand Up @@ -1767,4 +1786,82 @@ mod tests {
}
}
}

#[test]
fn assemble_quote_data_sell_order() {
let expiration = Utc::now() + chrono::Duration::seconds(60);
let parameters = QuoteParameters {
sell_token: Address::repeat_byte(1),
buy_token: Address::repeat_byte(2),
side: OrderQuoteSide::Sell {
sell_amount: SellAmount::AfterFee {

@MartinquaXD MartinquaXD Jun 5, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For optimal test coverage there should also be case for SellAmount::BeforeFee, no?
But since is just moving code into a separate function that gets executed in basically every e2e I'd also be fine with not having any additional tests.
AFAICS the only difference is that the order kind now gets decided by the match statement instead of reading it from the trade query.

value: NonZeroU256::try_from(100).unwrap(),
},
},
signing_scheme: QuoteSigningScheme::Eip712,
verification: Default::default(),
additional_gas: 0,
timeout: None,
};
let estimate = price_estimation::Estimate {
out_amount: AlloyU256::from(42),
gas: 3,
solver: Address::repeat_byte(7),
verified: true,
execution: Default::default(),
};

let data = assemble_quote_data(&parameters, &estimate, 2, 0.5, expiration);

@gemini-code-assist gemini-code-assist Bot Jun 5, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Pass estimate by value in the test.

        let data = assemble_quote_data(&parameters, estimate, 2, 0.5, expiration);
References
  1. Prefer passing a value (by cloning at the call site) over passing a reference if the called function requires ownership and would have to clone the value internally anyway.


assert_eq!(data.sell_token, Address::repeat_byte(1));
assert_eq!(data.buy_token, Address::repeat_byte(2));
assert_eq!(data.quoted_sell_amount, AlloyU256::from(100));
assert_eq!(data.quoted_buy_amount, AlloyU256::from(42));
assert_eq!(data.kind, OrderKind::Sell);
assert_eq!(data.expiration, expiration);
assert_eq!(data.quote_kind, QuoteKind::Standard);
assert_eq!(data.solver, Address::repeat_byte(7));
assert!(data.verified);
assert_eq!(
data.fee_parameters,
FeeParameters {
gas_amount: 3.,
gas_price: 2.,
sell_token_price: 0.5,
}
);
assert_eq!(data.metadata, QuoteMetadata::default());
}

#[test]
fn assemble_quote_data_buy_order() {
// Buy orders flip the assignment: the estimate's out_amount is the
// quoted sell amount, and the requested buy amount is fixed.
let expiration = Utc::now() + chrono::Duration::seconds(60);
let parameters = QuoteParameters {
sell_token: Address::repeat_byte(1),
buy_token: Address::repeat_byte(2),
side: OrderQuoteSide::Buy {
buy_amount_after_fee: NonZeroU256::try_from(100).unwrap(),
},
signing_scheme: QuoteSigningScheme::Eip712,
verification: Default::default(),
additional_gas: 0,
timeout: None,
};
let estimate = price_estimation::Estimate {
out_amount: AlloyU256::from(42),
gas: 3,
solver: Address::repeat_byte(7),
verified: false,
execution: Default::default(),
};

let data = assemble_quote_data(&parameters, &estimate, 2, 0.5, expiration);

@gemini-code-assist gemini-code-assist Bot Jun 5, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Pass estimate by value in the test.

        let data = assemble_quote_data(&parameters, estimate, 2, 0.5, expiration);
References
  1. Prefer passing a value (by cloning at the call site) over passing a reference if the called function requires ownership and would have to clone the value internally anyway.


assert_eq!(data.quoted_sell_amount, AlloyU256::from(42));
assert_eq!(data.quoted_buy_amount, AlloyU256::from(100));
assert_eq!(data.kind, OrderKind::Buy);
assert!(!data.verified);
}
}
Loading