Skip to content

Include the canonical bump in the order PDA - #90

Merged
kaze-cow merged 13 commits into
mainfrom
kaze/sc-257-move-bumps-to-account-data-when-possible
Aug 12, 2026
Merged

Include the canonical bump in the order PDA#90
kaze-cow merged 13 commits into
mainfrom
kaze/sc-257-move-bumps-to-account-data-when-possible

Conversation

@kaze-cow

@kaze-cow kaze-cow commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Description

Include the order's own canonical bump in the order PDA. This allows for the bump parameter to be removed from ReclaimOrder and BeginSettle.

Motivation

  • Overall reduces the complexity of BeginSettle's input, the most complicated instruction in the program at this time
  • Improves IDL compatibility by removing a "parallel array". There is still more that would have to be done here
  • Seems to be more in line with how most modern solana programs handle the problem of bump rederivation.

CU impact

Measured with the CU benchmarking helper added on this branch, which reads the settlement program's own compute cost out of the transaction log trace (CPI to SPL Token excluded) and writes it to target/cu-report.json. Baseline is 03545e4, the commit directly before the functional change, so the only difference is the bump move — the test addresses and the benchmark helper are already pinned in both.

Measurement Before After Δ
create_order 3391 3418 +27
reclaim_order 2138 2135 −3
settle, 1 order, no pulls 12165 12148 −17
settle, 2 orders, 1 pull each 19399 19376 −23
settle, 3 orders, no pulls 22345 22324 −21

The settle rows are the whole [BeginSettle, FinalizeSettle] transaction, because BeginSettle is never submitted alone; FinalizeSettle is untouched by this PR, so the delta is BeginSettle's.

Reproduce with:

just build-program
cargo test --test create_order --test reclaim_order --test begin_settle_orders
cat target/cu-report.json

Out of Scope

The original ticket said to move all storage, but it seems it is most valuable to include only for the order pda at this time. This is because for the other two current PDA types:

  • Buffer accounts are actual token account PDAs, so we can't control the storage inside of them/add a bump. We can add another PR to always rederive the PDA instead if we still want to remove the PDA as a parameter.
  • the single state pda is currently rederived every time it is loaded rather than accepting as an input. Considering that this change would look/shape much differently from the Order PDA changes, it should be included in a separate PR

How to test

This is mostly refactoring, so most of the tests should work the same as before the change. Confirm the overall methodology

@kaze-cow kaze-cow self-assigned this Aug 5, 2026
@linear-code

linear-code Bot commented Aug 5, 2026

Copy link
Copy Markdown

SC-257

@kaze-cow
kaze-cow force-pushed the kaze/sc-257-move-bumps-to-account-data-when-possible branch 2 times, most recently from f5de5fc to 87493fb Compare August 7, 2026 04:12
@kaze-cow
kaze-cow changed the base branch from main to kaze/sc-256-dev-tooling-cu-consumption-benchmarks August 7, 2026 04:12
@kaze-cow
kaze-cow force-pushed the kaze/sc-257-move-bumps-to-account-data-when-possible branch from 87493fb to bd2e69d Compare August 7, 2026 04:34
@kaze-cow
kaze-cow force-pushed the kaze/sc-257-move-bumps-to-account-data-when-possible branch from bd2e69d to e1f31a5 Compare August 7, 2026 04:42
@kaze-cow
kaze-cow marked this pull request as ready for review August 7, 2026 05:01
@kaze-cow
kaze-cow requested a review from a team as a code owner August 7, 2026 05:01
Base automatically changed from kaze/sc-256-dev-tooling-cu-consumption-benchmarks to main August 11, 2026 09:24
kaze-cow and others added 2 commits August 11, 2026 18:32
Wire `send_transaction_metered` into one representative happy-path test
per instruction under measurement, so the before/after CU numbers for
this branch come out of the existing tests rather than a bespoke harness.

`BeginSettle` only ever ships paired with a `FinalizeSettle`, so its
figure is the settlement program's own cost across the whole transaction
(SPL-token CPI excluded); `send_metered` wraps `send` to record it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@kaze-cow
kaze-cow force-pushed the kaze/sc-257-move-bumps-to-account-data-when-possible branch from e1f31a5 to 0ade7a0 Compare August 11, 2026 09:36

@fedgiac fedgiac left a comment

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.

The design looks good, I think it's close to merging.

Some notes on the scope (the current one is ok, this is future code):

Buffer accounts are actual token account PDAs, so we can't control the storage inside of them/add a bump. We can add another PR to always rederive the PDA instead if we still want to remove the PDA as a parameter.

We can test this, but I expect this to be overall more expensive (and non-deterministically so), so I fear we're going to keep it.

the single state pda is currently rederived every time it is loaded rather than accepting as an input. Considering that this change would look/shape much differently from the Order PDA changes, it should be included in a separate PR

Completely agree. The security angle is a bit trickier there imho.

Comment thread interface/src/data/order.rs
Comment thread interface/src/data/order.rs Outdated
Comment thread interface/src/data/order.rs Outdated
Comment thread interface/src/pda/buffer.rs Outdated
Comment thread programs/settlement/src/create_order.rs
Comment thread programs/settlement/tests/common/mod.rs Outdated
Comment thread programs/settlement/tests/begin_settle_orders.rs Outdated
Comment thread bench-report.json Outdated
Comment thread bench-report.json
kaze-cow and others added 6 commits August 12, 2026 15:50
Co-authored-by: Federico Giacon <58218759+fedgiac@users.noreply.github.com>
Co-authored-by: Federico Giacon <58218759+fedgiac@users.noreply.github.com>
…f github.com:cowprotocol/solana-programs into kaze/sc-257-move-bumps-to-account-data-when-possible
@kaze-cow
kaze-cow requested a review from fedgiac August 12, 2026 08:35

@fedgiac fedgiac left a comment

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.

Looks good!

@kaze-cow
kaze-cow merged commit f6ded86 into main Aug 12, 2026
14 checks passed
@kaze-cow
kaze-cow deleted the kaze/sc-257-move-bumps-to-account-data-when-possible branch August 12, 2026 09:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants