Repository navigation
test(svm): add reproducible legacy and V5 CU benchmarks - #1568
Reinis-FRP wants to merge 4 commits into
Conversation
droplet-rl
left a comment
droplet-rl
left a comment
There was a problem hiding this comment.
COMMENT, not blocking. The runner looks correct and well guarded: delivery, balances, fill status, payer rent and buffer closure are all checked, and the provenance is thorough. My main concern is how the headline numbers get read. PDA/ATA bump search explains almost all of the spread between samples and a large share of the gaps between flows. The bumps that drive it most aren't recorded in the fixture.
What I checked
- Recomputed
fixtureSha256from the committedbenchmarkCu.ts/config.ts/measure.ts/wire.ts. It matches the baseline, so the snapshot corresponds to the code in this PR. - Recomputed every median in the PR table from
baseline.json. All match, including the sample-3 reversal (−3,209). wire.tsis a verbatim move apart from the widenedu64param type.v5Encoding.ts/reference.tsre-export the same symbols, so encoding is unchanged.- I did not run the benchmark (no Gateway source access here).
Bump search explains the variance exactly
Each non-canonical bump step costs one more create_program_address (1,500 CU). I derived the unreported ATA bumps from the fixed seeds and fit a linear model. It reproduces all 25 execution values and all 15 buffer values with zero residual:
| Flow | Bump terms (1,500 CU per extra iteration) | Execution at canonical bumps (all 255) | Baseline median |
|---|---|---|---|
| legacy-deposit | source ATA + Spoke vault ATA + delegate | 31,707 | 34,707 |
| legacy-fill | status + delegate + recipient ATA | 38,021 | 41,021 |
| v5-deposit | funding + 2× Gateway vault ATA + Spoke vault ATA | 67,530 | 73,530 |
| v5-external-fill | status + 2× Gateway vault ATA + recipient ATA | 78,660 | 84,660 |
| v5-inplace-fill | status + 3× Gateway vault ATA + recipient ATA | 71,369 | 77,369 |
| buffer (every V5 flow) | 3 × buffer PDA (init + 2 writes) | 12,329 | 12,329 / 16,829 / 12,329 |
What this means:
- External minus in-place execution is a constant 7,291 CU at canonical bumps. The sample-3 "reversal" comes entirely from that fixture's Gateway vault ATA bump (250, 5 extra iterations, and in-place derives that ATA once more than external) plus a 2-step gap in status bump. It is not behaviour that depends on the path.
- The headline gap in total CU between external and in-place fills is 13,291. About 6,000 of that (≈45%) is bump luck: 4,500 from the buffer PDA and 1,500 from execution.
- The buffer column tells you nothing about the flows themselves. All three V5 flows send the same three transactions, and every difference comes from
bufferBump.
Inline suggestions: record the ATA bumps, consider grinding seeds to canonical bumps (or adding a bump-normalized column), and document or justify the 800-byte fragment size. The PR body's reversal paragraph could probably be reworded to cover the bump explanation.
Side observations (not for this PR)
- From the CU arithmetic, the buffer PDA is re-derived in every
initialize/writetx, and the vault ATA's bump cost is paid 2–3× per execution. A stored bump or a cached ATA would save 1,500 CU per extra iteration per derivation. Possibly worth a Gateway issue. This is inferred from the numbers; I haven't seen the Gateway source. approval = 310looks like p-token rather than classic SPL Token (inline note on the README).- Minor: the code that spawns the validator, waits for readiness, picks ports (
freePort) and validates the Gateway checkout is now duplicated withtestRealGateway.ts. The copies already differ (getHealthvsgetSlot() > 10, HTTPS vs SSH clone). Could be shared in a follow-up.
| await send("setup:clock", [ | ||
| await spoke.methods.setCurrentTime(NOW).accountsStrict({ state, signer: owner }).instruction(), | ||
| ]); | ||
| const fixture = { mint: mint.toBase58(), recipient: recipient.toBase58(), state: state.toBase58(), stateBump }; |
There was a problem hiding this comment.
fixture records the state, status, buffer, funding and delegate bumps, but not the ATA bumps, which drive most of the V5 variance. From the fixed seeds:
| sample | source | recipient | Spoke vault | Gateway vault |
|---|---|---|---|---|
| 0 | 255 | 255 | 252 | 253 |
| 1 | 254 | 254 | 254 | 255 |
| 2 | 254 | 255 | 254 | 254 |
| 3 | 253 | 255 | 255 | 250 |
| 4 | 255 | 255 | 255 | 255 |
The Gateway vault ATA's bump is paid 2× (deposit, external fill) or 3× (in-place fill) per execution, so sample 3's vault bump alone adds 15,000 or 22,500 CU. That fully explains the in-place > external reversal mentioned in the PR body. Suggest adding sourceBump, recipientAtaBump, spokeVaultBump and vaultBump here, e.g. PublicKey.findProgramAddressSync([authority.toBuffer(), TOKEN_PROGRAM_ID.toBuffer(), mint.toBuffer()], ASSOCIATED_TOKEN_PROGRAM_ID)[1]. Then the claim that "reported bumps make the variation inspectable" actually holds.
There was a problem hiding this comment.
Addressed in b02518a. Added sourceBump, recipientAtaBump, spokeVaultBump, and vaultBump to every fixture and regenerated the baseline. The README clarifies that recipientAtaBump is the final recipient ATA even for in-place fills. A fresh complete run preserved all 25 CU measurements and the program/runtime hashes.
Sent from Reinis Martinsons's Codex CLI Agent using gpt-6-astra 🤖
| export const LEGACY_COMMIT = "7445f72de17900544605c7e6706c5fb3b3784738"; | ||
| export const VALIDATOR_VERSION = "4.1.2"; | ||
| export const SPOKE = new PublicKey("DLv3NggMiSaef97YCkew5xKUHDh13tVGZ7tydt3ZeAru"); | ||
| export const SAMPLES = [0, 1, 2, 3, 4]; |
There was a problem hiding this comment.
Optional, but each bump step is a flat 1,500 CU, so a deterministic canonical case would make comparisons between flows exact. You could grind the per-sample labels (mint keypair, path salt, deposit id/nonce) until every PDA/ATA that depends on a fixture has bump 255. That could be one extra "canonical" fixture next to the five spread samples, or a bump-normalized column. The program-level PDAs (event authority, fill payer, delegates, Gateway config) are fixed and apply equally in production, so they can stay as they are.
From this baseline, canonical execution would be 31,707 / 38,021 / 67,530 / 78,660 / 71,369, and buffer would be 12,329 for every V5 flow. The medians of the five current seeds sit 3,000–6,000 CU above those, by a different amount for each flow.
There was a problem hiding this comment.
Addressed in b02518a. Kept the five address-spread fixtures and added a baseline-specific normalized table plus per-flow formulas to the README. Checked the formulas against all 25 execution rows and all 15 buffer rows with zero residual. The text distinguishes analytical normalization to bump 255 from actual measurements (all existing bumps are already canonical). The PR description now explains the 7,291-CU adjusted gap and the exact sample-3 reversal.
Sent from Reinis Martinsons's Codex CLI Agent using gpt-6-astra 🤖
| const [buffer, bufferBump] = PublicKey.findProgramAddressSync( | ||
| [Buffer.from("execute_params"), owner.toBuffer(), digest], | ||
| GATEWAY | ||
| ); |
There was a problem hiding this comment.
Across all 15 V5 rows, buffer CU is exactly 12,329 + 4,500 × (255 − bufferBump). Every flow sends init + 2 writes: 891, 1,194 and 1,173 bytes each split into two ≤800-byte fragments. Each of those three txs appears to re-derive this PDA. So the buffer medians in the PR table (12,329 / 16,829 / 12,329) differ only by bump luck. Either say that in the README or grind bufferBump to 255 (via the path salt).
There was a problem hiding this comment.
Addressed in b02518a. Documented 12,329 + 4,500 * (255 - bufferBump) and why the three flows have the same upload count. Rows now record bufferFragmentBytes and bufferWrites; the README separates measured and normalized totals. The stored-bump optimization is tracked separately in https://github.com/across-protocol/solana-v5/issues/79.
Sent from Reinis Martinsons's Codex CLI Agent using gpt-6-astra 🤖
| .accountsStrict({ submitter: owner, executeParams: buffer, systemProgram: SystemProgram.programId }) | ||
| .instruction(), | ||
| ]); | ||
| for (let offset = 0; offset < params.length; offset += 800) |
There was a problem hiding this comment.
The 800-byte fragment size directly sets how many write txs are sent, which feeds the buffer column, but it isn't documented or configurable. This tx shape (compute-budget ix + 2-account writeExecuteParamsFragment) fits about 940 data bytes under the 1,232-byte packet limit. I measured 891 data bytes → a 1,183-byte tx. At max size, v5-deposit's 891-byte params would need one write instead of two. If 800 matches what the production submitter does, a note in config.ts/README would help. Otherwise consider the largest size that fits, or a named constant, so the buffer column reflects a realistic submitter.
There was a problem hiding this comment.
Addressed in b02518a. Named the policy BUFFER_FRAGMENT_BYTES = 800 and documented that it is a conservative test-helper choice, not a production submitter setting or maximum packet utilization. Kept it fixed so all three fixtures retain two writes, and explicitly noted that a larger fragment can reduce deposit uploads. Production transaction composition belongs in SDK/backend code and is tracked in across-protocol/sdk#1541.
Sent from Reinis Martinsons's Codex CLI Agent using gpt-6-astra 🤖
| and transfer, then checks the floor and transfers in Gateway. Different account validation and PDA derivation costs | ||
| also contribute, so a total-CU difference does not isolate the cost of a single command. | ||
| Five fixed seeds expose some of that variation but are not a worst-case bound or a statistical production estimate. | ||
| The validator's bundled token programs and activated features are part of this baseline; it is not a mainnet CU budget. |
There was a problem hiding this comment.
Could this name the token program? approval = 310 is 150 for SetComputeUnitLimit plus 160 for ApproveChecked. That's p-token-level cost; classic SPL Token's ApproveChecked costs several thousand CU. The executable hash is recorded, but the choice affects the comparison itself, not just the absolute numbers. V5 flows make more token CPIs than legacy ones (funding transfer into the vault, approve, delivery transfer), so a cheaper token program also narrows the legacy/V5 ratio. One sentence saying which token program the bundled hash is, and whether that matches mainnet at the time of the run, would help readers interpret the table.
There was a problem hiding this comment.
Addressed in b02518a. Confirmed Agave 4.1.2 bundles p-token 1.0.0-rc.1 at Tokenkeg: downloaded the upstream binary and its SHA-256 exactly matches the recorded 8190d3f7...1a98f697. Added the upstream source link, full hash, 150 + 160 CU approval breakdown, and the effect on relative V5/legacy costs. No mainnet dump was captured for the benchmark, so the README explicitly says mainnet equivalence at run time was not verified.
Sent from Reinis Martinsons's Codex CLI Agent using gpt-6-astra 🤖
droplet-rl
left a comment
droplet-rl
left a comment
There was a problem hiding this comment.
Re-reviewed at b02518a. All points from my earlier review are addressed, and I checked each one against the data independently.
- Every pre-existing baseline value is unchanged. The program and runtime provenance is byte-identical to
e80949f. All 25 rows keep their CU values and their previous fixture fields; only the new fields are added. The recomputedfixtureSha256matches the committedbenchmarkCu.ts/config.ts/measure.ts/wire.ts. - ATA bumps:
sourceBump,recipientAtaBump,spokeVaultBumpandvaultBumpin every row match what I derived on my own from the fixed seeds. - Normalization formulas: I applied the README's per-flow formulas to every row. Each flow collapses to a single value: execution 31,707 / 38,021 / 67,530 / 78,660 / 71,369, buffer 12,329 for all V5 flows, totals 32,017 / 38,331 / 80,169 / 90,989 / 83,698. That matches the table exactly.
bufferWritesequalsceil(paramsBytes / bufferFragmentBytes)in every V5 row. - Fragment policy:
BUFFER_FRAGMENT_BYTESis now a named constant, recorded per row, and documented as a fixed test policy. The client-side packing question is split out to sdk#1541. That's reasonable. - Token program: the Agave v4.1.2
program-binaries/src/lib.rsL19–L24 link does point atspl_p_token-1.0.0-rc.1.sofor the Token address. I downloaded that upstream binary, and its SHA-256 is8190d3f7…a98f697, matching the baseline's executable hash.
Leaving the bumps as they are and normalizing analytically is a fair call, given that the formulas, the per-row inputs and the caveats are all documented. I didn't run the benchmark myself (no Gateway source access here).
Optional cleanup for later, not blocking: the code that spawns the validator, waits for readiness, picks ports and validates the Gateway checkout is still duplicated with testRealGateway.ts.
|
Addressed the remaining runner-duplication cleanup from the review in 24e07c9. Both runners now use Validated on Node 22.18: 5 helper tests, strict TypeScript, 4 tests through the real-Gateway entry point (encoding vectors, external fill, prefunded/in-place fill), and the complete 25-measurement CU benchmark. Every measurement and program/IDL/runtime hash is unchanged from the reviewed baseline. The refreshed snapshot records Node 22.18 and includes the shared helper in its fixture hash. Repository commit hooks passed. Sent from Reinis Martinsons's Codex CLI Agent using gpt-6-astra 🤖 |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
81a2211 to
8861526
Compare
70888c2 to
3d3178f
Compare
8861526 to
48ef30e
Compare
Signed-off-by: Reinis Martinsons <reinis@umaproject.org>
Signed-off-by: Reinis Martinsons <reinis@umaproject.org>
Signed-off-by: Reinis Martinsons <reinis@umaproject.org>
Signed-off-by: Reinis Martinsons <reinis@umaproject.org>
48ef30e to
e210d80
Compare
3d3178f to
a537c44
Compare
2 participants
Reproduce legacy Spoke and V5 Gateway CU comparisons with
yarn bench-svm-cu. The runner builds pinned legacy/Gateway sources and the current V5 Spoke, generates IDLs, and measures five deterministic fixtures per flow on fresh validators. It asserts delivery, consumed balances, fill status, payer rent and buffer closure before accepting results.The benchmark and real-Gateway runners share pinned-checkout validation and validator lifecycle helpers. Results retain raw receipts plus source, fixture, toolchain, binary, IDL and runtime provenance. Execution, approval and parameter-buffer CU are reported separately; normal runs compare against the baseline without changing it or enforcing a regression threshold.
Restacked on #1565 with #1564's self-transfer semantics. The in-place fixture now supplies and approves the fill delegate, performs the self-transfer, clears the remaining allowance, then checks the floor and consumes the vault. The extracted wire helpers retain explicit approval amounts. The refreshed baseline and README replace the former no-transfer measurements.
Median consumed CU in the current snapshot:
Totals are medians of per-sample sums; component medians need not add to them. The README documents the exact baseline-specific bump normalization. All V5 fixtures use initialization plus two parameter writes under the fixed 800-byte fragment policy; this is a test policy, not a production upload requirement.
Validation after restacking: all 25 CU measurements, 16 Rust tests, 21 shared-validator/build-guard tests, 9 focused real-Gateway tests and strict TypeScript checking passed. Measurements use Node 22.22.0 and Agave 4.1.2 with its bundled token programs, existing ATAs and fixed Spoke clocks. Legacy/V5 use different pinned compilers. These are local integration measurements, not verified production builds or mainnet CU budgets.
Targets #1565 (
reinis/svm-v5-gateway-repin).Sent from Reinis Martinsons's Codex CLI Agent using gpt-6-astra 🤖