Repository navigation
solana-driver: cut the execution costs from the fills - #5115
Conversation
|
Claude finished @squadgazzz's task in 4m 34s —— View job Review: cut the execution costs from the fillsThorough read of the diff against No correctness or safety issues found: every cut is re-validated against the signed limit via I posted 2 inline notes, both low-severity "drops otherwise-valid solutions" observations rather than bugs:
Nothing blocking from my side. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
…1-quote-network-fee # Conflicts: # crates/solana-driver/src/domain/priority_fee.rs
…ervices/be-410-driver-cut # Conflicts: # crates/solana-driver/tests/api.rs
…0-driver-cut # Conflicts: # crates/solana-driver/src/infra/api/routes/quote/mod.rs
tilacog
left a comment
tilacog
left a comment
There was a problem hiding this comment.
LGTM. I just posted a few nit comments.
| let trades = solution | ||
| .trades | ||
| .iter() | ||
| .map(|trade| { | ||
| let order = orders | ||
| .get(&trade.order_uid) | ||
| .copied() | ||
| .ok_or(Rejected::UnknownOrder(trade.order_uid))?; | ||
| let rent = if rent_charged.insert(order.uid) { | ||
| self.rent(order) | ||
| } else { | ||
| 0 | ||
| }; | ||
| let lamports = rent + u128::from(fee_share); | ||
| let atoms = self | ||
| .cut(order, lamports) | ||
| .ok_or(Rejected::MissingPrice(order.uid))?; | ||
| tracing::debug!( | ||
| solution_id, | ||
| order = %order.uid, | ||
| lamports, | ||
| atoms, | ||
| "cutting the execution cost from the fill" | ||
| ); | ||
| cut_trade(trade, order, atoms) | ||
| }) | ||
| .collect::<Result<Vec<_>, _>>()?; | ||
| solution.trades = trades; |
There was a problem hiding this comment.
nit: to save a vector allocation for trades, this can also be represented as an open for loop over solution.trades.iter_mut()
There was a problem hiding this comment.
If, for example, the third trade fails, iter_mut would have already cut the first two. Collecting first keeps the solution untouched on Err.
| // The idempotent creation tops a pre-funded address up to | ||
| // the minimum. | ||
| let owed = rent | ||
| .minimum_balance(mint.ata_len) | ||
| .saturating_sub(snapshot.lamports(&order.buy_token_account)); |
| let network_fee = match buy_token_accounts { | ||
| Some(_) => self.network_fee_ceiling().await?, | ||
| None => 0, | ||
| }; |
There was a problem hiding this comment.
Question: do we use self.network_fee_ceiling() as a placeholder because we can't truly estimate this?
There was a problem hiding this comment.
Yes. No route yet, so no accounts and no CU count to price. The fill is cut by its own route's fee afterwards. That cut still prices 1.4M CU, nothing fills cuEstimate before the simulation. Opened BE-446 to track it.
| let mut charged = Vec::with_capacity(solutions.len()); | ||
| for mut solution in solutions { |
There was a problem hiding this comment.
Same as my other comment. Since solutions is mut, maybe we just filter/retain it instead of allocating a new vector?
There was a problem hiding this comment.
The loop awaits and can bail early, retain_mut is sync. Not worth an index loop for one vector.
2 participants
Description
The solver keypair pays each settlement's execution cost and nothing charges it back. With the default
fee-handler = "driver"the user gets the engine's fill minus that cost, and the solver keeps the rest.solver-fee-bps.Nothing new per trade:
solver_feestays the volume fee. A solver earns its settlements' buffer deltas, cut plus fee minus route shortfall, swept by BE-374.Changes
fee-handler = "solver"sends the engine the signed limits and themissingBuyTokenAccountflag,solver-fee-bpsoffHow to test
Updated unit and API tests.
Related issues
BE-410