Skip to content

refactor(cu-tracker): take explicit instruction name#159

Merged
dev-jodee merged 8 commits into
mainfrom
refactor/cu-tracker-explicit-name
Jun 8, 2026
Merged

refactor(cu-tracker): take explicit instruction name#159
dev-jodee merged 8 commits into
mainfrom
refactor/cu-tracker-explicit-name

Conversation

@dev-jodee

Copy link
Copy Markdown
Collaborator

Summary

  • Make the CU tracker generic: record_cu(name, cus) instead of decoding the program instruction enum inside the tracker, so the same cu_tracker.rs can be shared across program repos (rewards/escrow/vault).
  • Move the SubscriptionsInstruction::from_bytes name decode into the build_and_send_transaction test glue (the single recording site) — zero call-site churn.
  • Behavior and the cu_report.md output format are unchanged.

Why

Part of standardizing CU benchmarking across program repos. The shared reusable GitHub Action consumes the cu_report.md table; the tracker that produces it should be program-agnostic (explicit name) so non-Pinocchio repos can reuse it.

Test Plan

  • cargo check -p tests-subscriptions --tests passes.
  • cargo fmt -p tests-subscriptions --check clean.
  • Full CU_REPORT=1 just integration-test run regenerates the same report (requires the program .so); change is a behavior-equivalent relocation of the name decode.

@vercel

vercel Bot commented Jun 8, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
solana-subscriptions-program Ready Ready Preview, Comment Jun 8, 2026 8:43pm

Request Review

@greptile-apps

greptile-apps Bot commented Jun 8, 2026

Copy link
Copy Markdown

Greptile Summary

This PR makes the CU tracker program-agnostic by replacing record_transaction(result, ix) (which internally decoded SubscriptionsInstruction) with record_cu(name, cus), moving the name-decode responsibility into build_and_send_transaction in test_helpers.rs. The benchmark workflow's inline PR-comment shell script is replaced by the reusable solana-developers/github-actions/cu-benchmark action.

  • cu_tracker.rs: Drops all dependencies on litesvm, solana_instruction, and SubscriptionsInstruction; collapses min/max/avg statistics into a single minimum-CU value; fixes the long-standing LAMPOSTS_PER_SOL typo.
  • test_helpers.rs: Moves SubscriptionsInstruction::from_bytes decode to the single recording site; behaviour is equivalent to before.
  • benchmark.yml: Delegates CU-report comment posting to the pinned external action, but drops contents: read from the permissions block, which will prevent actions/checkout from completing.

Confidence Score: 4/5

The Rust-side refactor is clean and correct, but the workflow change drops contents: read from its permissions block, which will cause the actions/checkout step to fail on every benchmark run until restored.

The benchmark workflow now omits contents: read while still using actions/checkout@v6. Any permission not listed in an explicit permissions: block defaults to none, so the checkout step will receive a 403 and the CI job will never reach the benchmark or report steps. The Rust refactor itself — decoupling the tracker, moving the name decode, and fixing the typo — is straightforward and correct.

.github/workflows/benchmark.yml needs contents: read added back to the permissions block.

Important Files Changed

Filename Overview
.github/workflows/benchmark.yml Replaces inline PR-comment shell script with a reusable cu-benchmark action; removes contents: read permission which will break the actions/checkout step, and widens fetch-depth to full history.
tests/integration-tests/src/utils/cu_tracker.rs Decouples the tracker from SubscriptionsInstruction; changes public API to record_cu(name, cus); collapses min/max/avg stats into a single minimum value; fixes LAMPOSTS_PER_SOL typo.
tests/integration-tests/src/utils/test_helpers.rs Moves SubscriptionsInstruction::from_bytes name decode from the tracker into build_and_send_transaction; logic is equivalent to the previous record_transaction call.

Sequence Diagram

sequenceDiagram
    participant Test as Integration Test
    participant TH as test_helpers.rs
    participant SI as SubscriptionsInstruction
    participant CT as cu_tracker.rs
    participant FS as cu_report.md

    Test->>TH: build_and_send_transaction(litesvm, signers, payer, ix)
    TH->>TH: litesvm.send_transaction(tx)
    TH->>TH: is_tracking_enabled()?
    alt CU_REPORT set
        TH->>SI: "from_bytes(&ix.data) → name"
        TH->>CT: record_cu(name, meta.compute_units_consumed)
        CT->>CT: tracker.record(name, cus)
    end
    TH-->>Test: TransactionResult

    Note over CT,FS: On process exit (dtor)
    CT->>CT: compute_stats() → min CUs per instruction
    CT->>FS: write_to_file("cu_report.md")
Loading

Reviews (9): Last reviewed commit: "ci: drop unused CU prettier-ignore (repo..." | Re-trigger Greptile

Comment thread tests/integration-tests/src/utils/cu_tracker.rs Outdated
@github-actions

github-actions Bot commented Jun 8, 2026

Copy link
Copy Markdown

Compute Unit Report

Instruction Samples CUs Est Cost (Low) [SOL] Est Cost (Med) [SOL] Est Cost (High) [SOL] Δ Avg vs main
cancel_subscription 22 1720 0.000005000 0.000005068 0.000005860
close_subscription_authority 10 1803 0.000005000 0.000005072 0.000005901
create_fixed_delegation 41 3520 0.000005001 0.000005140 0.000006760
create_plan 97 3436 0.000005001 0.000005137 0.000006718
create_recurring_delegation 30 3555 0.000005001 0.000005142 0.000006777
delete_plan 9 359 0.000005000 0.000005014 0.000005179
init_subscription_authority 174 6226 0.000005001 0.000005249 0.000008113
resume_subscription 3 1723 0.000005000 0.000005068 0.000005861
revoke_delegation 19 255 0.000005000 0.000005010 0.000005127
subscribe 32 6485 0.000005001 0.000005259 0.000008242
transfer_fixed 9 5479 0.000005001 0.000005219 0.000007739
transfer_recurring 20 5591 0.000005001 0.000005223 0.000007795
transfer_subscription 10 5838 0.000005001 0.000005233 0.000007919
update_plan 22 424 0.000005000 0.000005016 0.000005212

🔺 increase · 🔻 decrease · – unchanged · 🆕 new · 🗑 removed (vs main)

Generated: 2026-06-08

Make the CU tracker generic (record_cu(name, cus)) instead of decoding
the program instruction enum inside the tracker, so the same module can
be shared across program repos. The SubscriptionsInstruction::from_bytes
name decode now lives in the build_and_send_transaction test glue.

Behavior and the cu_report.md output format are unchanged.
dev-jodee and others added 2 commits June 8, 2026 15:38
Replace the inline gh-api comment step with solana-developers/github-actions/cu-benchmark,
which diffs against a committed cu_baseline.md and posts per-instruction deltas.
CU is not deterministic across runs (on-chain ATA find_program_address
bump search over random keypair owners), so a committed baseline churns
every run. Switch to report-only: commit-baseline off, drop contents:write,
remove cu_baseline.md, track Min CUs (most stable single value).
Comment thread .github/workflows/benchmark.yml
@dev-jodee dev-jodee merged commit 41f27d0 into main Jun 8, 2026
15 checks passed
@dev-jodee dev-jodee deleted the refactor/cu-tracker-explicit-name branch June 8, 2026 20:59
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.

1 participant