refactor(program): simplify after duplication and dead-code review#68
Merged
refactor(program): simplify after duplication and dead-code review#68
Conversation
- Delete unused init_header (duplicate of Header::init) - Use existing get_token_account_owner helper in fixed/recurring transfer paths - Add SECS_PER_HOUR const + PlanTerms::period_length_secs helper; replace 3600 magic numbers - Remove redundant plan_bump check in subscribe (verify_plan_pda already enforces bump) - Single-pass Plan::check_destination (drop iterator clone) - Move state/versioning logic from mod.rs into core.rs (mod.rs convention) - Extract resolve_optional_payer helper in helpers/system.rs (reused 3x) - Unify FixedTransferAccounts and RecurringTransferAccounts into shared DelegationTransferAccounts - Strip narrating WHAT-comments in transfer/cancel handlers; keep WHY-comments No behavior change; no IDL drift. All 38 unit tests pass under cargo test-sbf; clippy -D warnings clean for subscriptions crate.
Compute Unit Report
Generated: 2026-04-30 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
init_header(duplicate ofHeader::init); use existingget_token_account_ownerhelper in fixed/recurring transfer paths.SECS_PER_HOURconst andPlanTerms::period_length_secshelper; replace3600magic numbers across program code and tests.data.plan_bump != plan.bumpcheck insubscribe(verify_plan_pdaalready enforces bump correctness via PDA derivation).Plan::check_destination(drop iterator clone); movestate/versioninglogic out ofmod.rsintocore.rs(mod.rsconvention from CLAUDE.md).resolve_optional_payerhelper inhelpers/system.rs(reused by 3 instruction processors); unifyFixedTransferAccountsandRecurringTransferAccountsinto sharedDelegationTransferAccountsinhelpers/transfer_utils.rs.No behavior change. Net ~80 LOC removed across instruction handlers.
Test Plan
cargo check -p subscriptionscleancargo clippy -p subscriptions --all-targets -- -D warningscleancargo test-sbf— 38/38 unit tests passgit diff idl/empty (no IDL drift; Codama attrs untouched)just fmtcleanSkipped Findings (deliberate)
find_program_addressfor the delegator ATA intransfer_utils.rswas flagged as a CU win, but the owner+mint check on the token account is not sufficient — the SubscriptionAuthority PDA may have been approved on multiple token accounts of the same user. Leaving the canonical-ATA derivation in place..clone()of packed structs) is invasive and audit-relevant; deferred.SubscriptionAuthority(check + load) deferred — fix forces a wider trait refactor.