Conversation
There was a problem hiding this comment.
further minor optimizations are possible to save an additional 60 CU in the most critical program path.
I think we can save even more CU by performing the same optimization on OrderIntent, though the effect will be less dramatic, since we don't read every single field. Still worth looking into if we don't already have a ticket for it right? for the next optimization PR.
other than that, looks good.
| if !is_pda_with_signer_seeds( | ||
| order_pda.address(), | ||
| program_id, | ||
| order_pda_signer_seeds(&order.intent_uid(), &[order.bump()]), |
There was a problem hiding this comment.
re-slicing the bump can cost a surprising amount of CU because it results in a reinitialization of an array. or at least it did in the past for me.
When I added a order.bump_slice() function and used the slice directly, the amount of CU reduced by 2.
| let mut order_pda = *order_pda; | ||
| order_pda.try_borrow_mut()?.copy_from_slice(&updated); | ||
| let mut data = order_pda.try_borrow_mut()?; | ||
| OrderAccount::attach(&mut data[..])?.set_amounts(final_amounts); |
There was a problem hiding this comment.
this technically causes a second attachment, which means repeating the associated validations.
By refactoring the code to remove the use of load_from_pda and keeping a long running mutable reference on the order data, I was able to reduce the CU usage by a further 48 CU. I don't think I accidentally dropped any checks.
Additionally, Claude identified that there was an address copy happening in the pda address increasing check happening elsewhere which was extremely easy to undo (just add the reference and the borrow checker liked it, all good). This reduced CU by a further 8. Nothing major, but so easy.
there is no evidence that I have found that maintaining the mutable reference on the account view is a problem while executing a CPI.
See example here (you would want to refactor it neatly of course if you like it/think its worth) https://github.com/cowprotocol/solana-programs/compare/refactor-order-access...refactor-order-access-squared?expand=1
2 participants
Rework the order account so the program reads and writes its PDA bytes in place instead of round-tripping through an owned decoded struct, matching the shape of
data/state.rs. The same as #118, but for the order PDA: it uses the exact same pattern for the design and the accessors.The decoded form moves to the client as
DecodedOrderAccount, like forDecodedStateAccount.The motivation is a decrease in CU cost in the settlement, as well as a cleaner design overall. This in particular fixes the extra re-encoded of the whole 264-byte body on every fill that I discussed together with @kaze-cow (but with a lot more changes 😅 ).
I also introduced a new struct
FillAmountssince no operation really involves only one of the two fields, so we can always keep them together.Otherwise, there are no logic changes to the programs.
Compute-unit improvements
settle/settles_a_single_order: 12558 → 12291 (−267)settle/settles_multiple_orders(3 orders): 23251 → 22468 (−783, ≈ −261/order)settle/pushes_a_single_order: 12540 → 12273 (−267)settle/pushes_several_orders_from_one_buffer: 17869 → 17344 (−525)settle/pushes_several_orders_from_different_buffers: 17869 → 17344 (−525)settle/pulls_funds_to_destination: 13695 → 13428 (−267)settle/pulls_to_multiple_destinations: 14847 → 14580 (−267)settle/pulls_from_multiple_orders: 20184 → 19659 (−525)settle/settling_a_self_order_withdraws_the_buffered_fees: 13688 → 13421 (−267)settle/finalizes_with_no_pushes: 7128 (unchanged, this leg writes no order body)How to review
I recommend reading the diff for all files but
interface/src/data/order.rsfirst. Do you like how it looks like?Then, read that file directly and compare it to
interface/src/data/state.rsto confirm that the design matches; the diff is unfortunately more or less unreadable. Also, make sure no unintended tests have been dropped.How to test
CI.