feat(solana-solvers): PR 3 solution assembly (custom interactions, prices, lookup tables)#4652
feat(solana-solvers): PR 3 solution assembly (custom interactions, prices, lookup tables)#4652squadgazzz wants to merge 14 commits into
Conversation
…ices, lookup tables)
|
Claude finished @squadgazzz's task in 4m 43s —— View job Review: PR #4652 — Solana solvers solution assembly
The clearing-price math is sound: pricing each mint at the counter amount ( Two non-blocking points, both left inline on
Nothing here blocks merge. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| #[serde_as(as = "HashMap<serde_with::DisplayFromStr, _>")] | ||
| pub prices: HashMap<Pubkey, u64>, |
There was a problem hiding this comment.
prices (and Trade::executed_amount / fee below) go on the wire as raw u64 JSON numbers, whereas the established /solve DTO in crates/solvers-dto/src/solution.rs serializes every price/amount/fee as a HexOrDecimalU256 string:
#[serde_as(as = "HashMap<_, HexOrDecimalU256>")]
pub prices: HashMap<Address, U256>,
...
#[serde_as(as = "HexOrDecimalU256")]
pub executed_amount: U256,Two concerns:
- Consistency — the driver already understands the EVM wire convention (decimal/hex strings). Emitting bare numbers means the Solana
/solvecontract diverges from the rest of the protocol for no stated reason (the description pins the wire shape but doesn't call out the number-vs-string choice). - Precision — a Solana token amount / price is a full
u64, whose max (~1.8e19) exceeds2^53. Any non-Rust JSON consumer that reads these as IEEE-754 doubles will silently corrupt large values. Strings sidestep that.
Suggest serializing these as strings to match the EVM DTO. Fix this →
| executed_amount, | ||
| fee: 0, | ||
| }], | ||
| interactions: swap.instructions.iter().map(Interaction::custom).collect(), |
There was a problem hiding this comment.
Minor: swap is owned here and swap.instructions is never read again, but Interaction::custom takes &Instruction and so clones data and rebuilds every AccountMeta. Consuming the vec by value would let you move the data out instead of cloning:
interactions: swap.instructions.into_iter().map(Interaction::custom).collect(),with custom(instruction: Instruction) moving instruction.data. This requires reading swap.instructions before swap.address_lookup_tables is moved (already the case in this struct literal).
| #[serde_as(as = "HashMap<serde_with::DisplayFromStr, _>")] | ||
| pub prices: HashMap<Pubkey, u64>, |
There was a problem hiding this comment.
prices (and Trade::executed_amount / fee below) go on the wire as raw u64 JSON numbers, whereas the established /solve DTO in crates/solvers-dto/src/solution.rs serializes every price/amount/fee as a HexOrDecimalU256 string:
#[serde_as(as = "HashMap<_, HexOrDecimalU256>")]
pub prices: HashMap<Address, U256>,
...
#[serde_as(as = "HexOrDecimalU256")]
pub executed_amount: U256,Two concerns:
- Consistency — the driver already understands the EVM wire convention (decimal/hex strings). Emitting bare numbers here means the Solana
/solvecontract diverges from the rest of the protocol for no stated reason (the PR description pins the wire shape but doesn't call out the number-vs-string choice). - Precision — a Solana token amount / price is a full
u64, whose max (~1.8e19) exceeds2^53. Any non-Rust JSON consumer (or a generic JSON tool) that reads these as IEEE-754 doubles will silently corrupt large values. Strings sidestep that entirely.
Suggest serializing these as strings to match the EVM DTO. Fix this →
| executed_amount, | ||
| fee: 0, | ||
| }], | ||
| interactions: swap.instructions.iter().map(Interaction::custom).collect(), |
There was a problem hiding this comment.
Minor: swap is owned here and swap.instructions is never read again, but Interaction::custom takes &Instruction and so clones data and rebuilds every AccountMeta. Consuming the vec by value would let you move the data out instead of cloning:
interactions: swap.instructions.into_iter().map(Interaction::custom).collect(),with custom(instruction: Instruction) moving instruction.data. Note this requires reading swap.instructions before swap.address_lookup_tables is moved (already the case in this struct literal).
…tching driver spec
…cu_estimate assertion
| // Unwrap: the destination length always matches the input. | ||
| const_hex::encode_to_slice(self.0.as_slice(), &mut bytes[2..]).unwrap(); | ||
| // Unwrap: hex output is always valid UTF-8. | ||
| f.write_str(std::str::from_utf8(&bytes).unwrap()) |
There was a problem hiding this comment.
Things like this always make me wonder if matching and adding an unreachable on the error path would make this faster or not, specially given the amount of times order uids get printed in the system
(though for the utf8 there's probably an unchecked version)
| /// Uniform clearing prices keyed by mint. Values are decimal strings. | ||
| #[serde_as(as = "HashMap<serde_with::DisplayFromStr, serde_with::DisplayFromStr>")] | ||
| pub prices: HashMap<Pubkey, u64>, |
| /// Sell and buy mint coincide, so a uniform clearing-price map (one price | ||
| /// per mint) cannot represent the trade. | ||
| #[error("sell and buy mint are the same")] | ||
| SameMint, |
There was a problem hiding this comment.
given all the effort done into removing UCPs, did we check with solvers if we should actually be adding the UCP concept back in?
| let solution = Solution::single(42, OrderUid([8; 32]), &order, swap()).unwrap(); | ||
|
|
||
| assert_eq!(solution.id, 42); | ||
| // Clearing prices: sell mint priced at the output amount, buy mint at | ||
| // the input amount, so executed × price matches on both sides. | ||
| assert_eq!(solution.prices[&order.sell_mint], 2_000); | ||
| assert_eq!(solution.prices[&order.buy_mint], 1_000); | ||
| assert_eq!(solution.trades.len(), 1); | ||
| assert_eq!(solution.trades[0].order_uid, OrderUid([8; 32])); |
There was a problem hiding this comment.
should we const come of the repeated values like the orderuid? so its easier to track what they are etc?
| .unwrap() | ||
| ); | ||
| assert_eq!(json["addressLookupTables"][0], pubkey(7).to_string()); | ||
| } |
There was a problem hiding this comment.
couldn't you write the json using the json! macro and just eq on that?
value impls partialeq which is what the assert_eq requires
2 participants
Description
PR 3 of the Jupiter solver plan (after #4632, #4635, #4637): turn one quoted swap into the single-order solution the driver consumes on
/solve.A solution is a receipt the driver can act on: which prices clear the trade, which order it fills, and the exact instructions to splice into the settlement transaction. The solver's only degrees of freedom are the clearing prices, derived from the quoted amounts so executed times price matches on both sides, and the interactions array, which carries Jupiter's instructions verbatim with full account metas. Address lookup tables ride along so the driver can build the v0 transaction the instructions assume.
Amounts go out as decimal strings, not JSON numbers. A Solana
u64reaches ~1.8e19, but a JSON number read as a double is only exact up to 2^53 (~9e15), and anything past that rounds to the nearest value the double can hold. Real amounts get there (10M SOL is 1e16 lamports), so a bare number risks silent corruption in a reader like JavaScript, whose only number type is a double. A string carries the exact digits and each consumer parses it back whole. The EVM solution DTO strings its amounts for the same reason.Deliberately minimal:
cu_estimateis optional and left unset. The driver sizes the CU limit by simulating the whole settlement transaction (driver §7.2), which this solver never sees, so it has no meaningful total to report. This mirrors the EVM solution's optionalgas. The adapter still drops Jupiter's own compute-budget instructions, since the driver emits its own.FinalizeSettlesolana-programs#62).Vec<Instruction>, matching the driver's single interaction kind at MVP. There is no liquidity-interaction variant: the on-chain liquidity subsystem is frozen, so every interaction is a solver-supplied instruction.Guards: same-mint orders are rejected (a uniform price map cannot hold two prices for one mint), and zero quoted amounts are rejected.
Changes
domain::solutionmodule:Solution,Trade, and a flatInstructionDTO (matching driver §2'sVec<Instruction>), wire serialization (base58 keys, hex order uid, base64 instruction data), andSolution::singlemapping adex::Swapinto the DTOpricesvalues,executed_amount,fee) serialize as decimal strings (see the precision note above)cu_estimate: Option<u64>on the solution, leftNone(the driver sizes the CU limit from simulation, mirroring EVM's optionalgas)OrderUidnewtype (32-bytehash(intent)), hex-formatted the same way as the EVMmodelcrateHow to test
New unit tests: sell and buy mapping, clearing prices, verbatim instruction carry-over, lookup-table pass-through, rejection guards, and a wire-format test pinning the JSON shape.