add cu benchmarking by kaze-cow · Pull Request #67 · cowprotocol/solana-programs

kaze-cow · 2026-07-10T03:53:07Z

Description

Record the CU, account, and transaction size usage of integration tests through a wrapper function. Also hook up a good selection of existing tests to use the benchmarks.

Context

The benchmark results are written by a dedicated just bench function. The bench-report.json file is checked into the repo so that when the benchmarking results change, they will be displayed along with the applicable changes in a highly readable format.

The CI checks that a regenerated bench-report.json does not drift.

Dependency Change

Adds bincode as a dev-dependency (programs/settlement/Cargo.toml), so it is only ever built for tests and never linked into the on-chain program. It was already in our dependency tree. The bincode package is one of the most popular package on Crates, and is officially used by solana. However, bincode package has been marked as unmaintained. The near-replacement suggested (wincode) causes conflicts/issues way outside of the scope of what should be done within this PR.

How to test

Ensure the just bench command works on your machine and doesn't present any changes in the bench-report.json file. There is a possibility that differences in local tooling could affect the output.

linear-code · 2026-07-10T03:53:09Z

SC-256

fedgiac

A few comments but it looks reasonable.

Come comments:

The PR is fairly small, I think it's ok to already use the new function everywhere. Right now, I had to go and ask Claude to add this function everywhere. (It found interesting things, like: we possibly want to skip recording tests that revert.)
What do you think about adding two extra fields, accounts and bytes, that just print the number of accounts/bytes used by a transaction? It's fairly easy to add and it's relevant for benchmarks.
I'm in favor of checking in this file, so we always see the changes. In case, there should be CI for it. Doesn't need to be this PR though.

fedgiac · 2026-08-07T10:34:46Z

+
+    fs::write(
+        path,
+        serde_json::to_string_pretty(&report).expect("CU report should serialize"),


I'd kind of prefer yaml or toml here for readability, but ultimately it doesn't matter.

yaml tooling is a lot less ubiquitous and . for example, we use jq to format the json from jsonl, but the equivalent tool yq isn't commonly installed on workstations.

JSON can be very readable if its formatted pretty, so I would like to leave it as is.

kaze-cow · 2026-08-07T11:56:10Z

The PR is fairly small, I think it's ok to already use the new function everywhere. Right now, I had to go and ask Claude to add this function everywhere. (It found interesting things, like: we possibly want to skip recording tests that revert.)

First time I have ever been told to grow the size of a PR, progress 😆

What do you think about adding two extra fields, accounts and bytes, that just print the number of accounts/bytes used by a transaction? It's fairly easy to add and it's relevant for benchmarks.

Wanted to keep the PR scoped but yea it shouldn't be a big ask

I'm in favor of checking in this file, so we always see the changes. In case, there should be CI for it. Doesn't need to be this PR though.

Agreed! Right now there was nothing to check in. Will add the tests.

# Description Make tests deterministic by replacing `Pubkey::new_unique()` or `Keypair::new()` with deterministic randomness from a seed. # Motivation Enables #67 because right now the random key generation can cause CU to shift randomly from test run to test run due to bump searching. As seen in #94 # Strategy Replace all usages of `Pubkey::new_unique()` or `Keypair::new()` with new `common` methods which draw from a random source from a seed, reset prior to every test in `setup()`. Utilize a `clippy.toml` rule to prevent usage of the non-deterministic functions in the integration tests only. It is also necessary to replace the `CreateMint` helper provided by `litesvm` because it uses those functions under the hood, and there doesn't seem to be away around. Here is what the clippy rule firing looks like: <img width="879" height="212" alt="image" src="https://github.com/user-attachments/assets/67180860-d310-4cb2-82c3-869b6833316b" /> # How to test Verify the test results. Run the tests multiple times to verify the CU is not changing from run to run in #67 . I did so manually by running `just test` a couple times and comparing the output `target/cu-report.json` against previous runs. If you would like, confirm that the clippy rule is applying as expected in the integration tests, and the clippy rule is *NOT* applied elsewhere (ex. in the program unit tests, which don't really need to be deterministic) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Federico Giacon <58218759+fedgiac@users.noreply.github.com>

Co-authored-by: Federico Giacon <58218759+fedgiac@users.noreply.github.com>

now its much simpler

fedgiac

Very nice, I like the result, and this is only 300 lines of code!

My only observation that could require some work would be to use the metered function everywhere in the tests where it doesn't revert. And I'm even considering using it for reverting transaction and add a flag in the label, like /r. No strong opinion here, I'll leave to you to which extent we should do it.

Otherwise, I only have small things in the comments.

fedgiac · 2026-08-10T20:37:32Z

+  "accounts_readable": {
+    "finalizes_with_no_pushes/settle": 4,
+    "happy_path_creates_initialized_buffer_token_account/create_buffers": 4,
+    "happy_path_creates_multiple_buffers_in_one_instruction/create_buffers": 6,
+    "happy_path_creates_order_pda_with_expected_body/create_order": 2,
+    "happy_path_initializes_state_pda_with_expected_data/initialize": 2,
+    "happy_path_returns_lamports_and_closes_pda/reclaim_order": 1,
+    "max_buffers_in_one_instruction/create_buffers": 33,
+    "pulls_from_multiple_orders/settle": 4,
+    "pulls_funds_to_destination/settle": 4,
+    "pulls_to_multiple_destinations/settle": 4,
+    "pushes_a_single_order/settle": 4,
+    "pushes_several_orders_from_different_buffers/settle": 4,
+    "pushes_several_orders_from_one_buffer/settle": 4,
+    "settles_a_single_order/settle": 4,
+    "settles_multiple_orders/settle": 4
+  },
+  "accounts_writable": {
+    "finalizes_with_no_pushes/settle": 1,
+    "happy_path_creates_initialized_buffer_token_account/create_buffers": 2,
+    "happy_path_creates_multiple_buffers_in_one_instruction/create_buffers": 4,
+    "happy_path_creates_order_pda_with_expected_body/create_order": 2,
+    "happy_path_initializes_state_pda_with_expected_data/initialize": 2,
+    "happy_path_returns_lamports_and_closes_pda/reclaim_order": 3,
+    "max_buffers_in_one_instruction/create_buffers": 31,
+    "pulls_from_multiple_orders/settle": 11,
+    "pulls_funds_to_destination/settle": 6,
+    "pulls_to_multiple_destinations/settle": 7,
+    "pushes_a_single_order/settle": 5,
+    "pushes_several_orders_from_different_buffers/settle": 9,
+    "pushes_several_orders_from_one_buffer/settle": 8,
+    "settles_a_single_order/settle": 5,
+    "settles_multiple_orders/settle": 13
+  },


Since the benchmarks are intended to be human-readable, we could make an entry more information dense and do something like:

"max_buffers_in_one_instruction/create_buffers": "64 (31 w / 33 r)",

In general I suggest always using strings, so that if there's an isolated diff we know by the unit specified in the unit what this number refers to:

"max_buffers_in_one_instruction/create_buffers": "163418 CU", [...] "max_buffers_in_one_instruction/create_buffers": "1 b",

I kept them as numbers without compressing so that it would be easier for scripts/other tooling that may come in the future to parse the file or changes. When reading the diff I think it should still be clear enough as the section names will be shown in most diffing tools, right?

As discussed privately, it's fine to keep like this but have the sum instead of read/write separate.

it turns out that solana only limits the total number of accounts that may be locked, and not the number of accounts that can be locked writable vs. the amount that can be locked readable. So we only need to measure the sum as @fedgiac said.

Measuring the sum only in da2e8d4

Co-authored-by: Federico Giacon <58218759+fedgiac@users.noreply.github.com>

adds bincode dependency

kaze-cow · 2026-08-11T07:28:45Z

My only observation that could require some work would be to use the metered function everywhere in the tests where it doesn't revert. And I'm even considering using it for reverting transaction and add a flag in the label, like /r. No strong opinion here, I'll leave to you to which extent we should do it.

See comment here, I tested this out (even for reverting functions!), but I didn't really like it. I think what we have now is pretty ideal, actually. Its like proptest--doesn't need to go everywhere, but we can add benchmarking when we believe something to be important to watch.

…ng-cu-consumption-benchmarks

turns out that solana limits by total accounts locked, whether the account is writable or readable is irrelevant

fedgiac

Looks good!

Base automatically changed from push-funds-to-user-processing-in-finalize-settle to main July 23, 2026 08:10

kaze-cow force-pushed the kaze/sc-256-dev-tooling-cu-consumption-benchmarks branch from 9c036fe to 694f57b Compare July 28, 2026 08:35

kaze-cow marked this pull request as ready for review July 28, 2026 08:35

kaze-cow requested a review from a team as a code owner July 28, 2026 08:35

This was referenced Aug 6, 2026

Rework the instruction parse API around per-input account borrows #93

Merged

Make tests deterministic #96

Merged

kaze-cow force-pushed the kaze/sc-256-dev-tooling-cu-consumption-benchmarks branch from a7925ff to ee38f6e Compare August 7, 2026 04:09

kaze-cow changed the base branch from main to kaze/sc-256-pin-test-keypairs-for-deterministic-cu August 7, 2026 04:09

kaze-cow force-pushed the kaze/sc-256-dev-tooling-cu-consumption-benchmarks branch 3 times, most recently from 85c36c4 to fa68b93 Compare August 7, 2026 04:41

kaze-cow force-pushed the kaze/sc-256-dev-tooling-cu-consumption-benchmarks branch from fa68b93 to 03545e4 Compare August 7, 2026 04:42

fedgiac reviewed Aug 7, 2026

View reviewed changes

Base automatically changed from kaze/sc-256-pin-test-keypairs-for-deterministic-cu to main August 7, 2026 12:51

kaze-cow force-pushed the kaze/sc-256-dev-tooling-cu-consumption-benchmarks branch from 701ebcc to e290fc0 Compare August 7, 2026 12:51

kaze-cow and others added 7 commits August 10, 2026 21:24

add a cu benchmarking

61d63de

move into dedicated benchmark.rs file

100acfa

Update programs/settlement/tests/common/benchmark.rs

8adee50

Co-authored-by: Federico Giacon <58218759+fedgiac@users.noreply.github.com>

rethink how tests are written and handled

6afba96

now its much simpler

also write accounts and instruction bytes, since its relatively easy

b061ace

test name included in benchmarking results

7d23da1

give up on bench_test macro, as it can't look exactly like #[test]

9c7486b

kaze-cow force-pushed the kaze/sc-256-dev-tooling-cu-consumption-benchmarks branch from 735eaaa to 9c7486b Compare August 10, 2026 12:24

kaze-cow added 2 commits August 10, 2026 21:29

run bench instead of test and ensure no drift in CI

233868e

add bench-report and rename

1a261a9

kaze-cow added 3 commits August 10, 2026 21:38

have exit-code

057f687

fix bench report

082820e

fail aggressively if label collision

732b7a5

kaze-cow requested a review from fedgiac August 10, 2026 12:52

Updates from pre-review agent

333c201

fedgiac reviewed Aug 10, 2026

View reviewed changes

kaze-cow and others added 5 commits August 11, 2026 13:54

Update programs/settlement/tests/common/benchmark.rs

6c638c3

Co-authored-by: Federico Giacon <58218759+fedgiac@users.noreply.github.com>

replace bench report to start with label rather than test name

66e27a3

measure transaction size in benchmark instead of ix

2fd3734

adds bincode dependency

correct bench report

dc5d91f

only respond to "true" for TEST_BENCHMARK

7ed6ee3

kaze-cow added 2 commits August 11, 2026 17:13

pre review agent

4a4abba

Merge remote-tracking branch 'origin/main' into kaze/sc-256-dev-tooli…

4eba165

…ng-cu-consumption-benchmarks

kaze-cow requested a review from fedgiac August 11, 2026 08:15

kaze-cow added 2 commits August 11, 2026 17:16

fix the bench report again

b5fda96

put readable and writable accounts together

da2e8d4

turns out that solana limits by total accounts locked, whether the account is writable or readable is irrelevant

kaze-cow mentioned this pull request Aug 11, 2026

Unrelated to this PR, but I'm wondering if by mapping the error we're losing valuable information in the test. #103

Open

fedgiac approved these changes Aug 11, 2026

View reviewed changes

kaze-cow merged commit b257861 into main Aug 11, 2026
14 checks passed

kaze-cow deleted the kaze/sc-256-dev-tooling-cu-consumption-benchmarks branch August 11, 2026 09:24

Uh oh!

Conversation

kaze-cow commented Jul 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Context

Dependency Change

How to test

Uh oh!

linear-code Bot commented Jul 10, 2026

Uh oh!

fedgiac left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fedgiac Aug 7, 2026

Choose a reason for hiding this comment

Uh oh!

kaze-cow Aug 10, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

kaze-cow commented Aug 7, 2026

Uh oh!

fedgiac left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fedgiac Aug 10, 2026

Choose a reason for hiding this comment

Uh oh!

kaze-cow Aug 11, 2026

Choose a reason for hiding this comment

Uh oh!

fedgiac Aug 11, 2026

Choose a reason for hiding this comment

Uh oh!

kaze-cow Aug 11, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

kaze-cow commented Aug 11, 2026

Uh oh!

fedgiac left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

kaze-cow commented Jul 10, 2026 •

edited

Loading

fedgiac left a comment •

edited

Loading