perf: ctoken remove return value of verify_owner_or_delegate_signer

[ananas-block]

Summary by CodeRabbit

• Refactor
  • Improved internal owner and delegate authorization validation logic with enhanced error diagnostics for better maintainability.

[coderabbitai]

Walkthrough

The function verify_owner_or_delegate_signer return type is simplified from Result<Option<&'a AccountInfo>, ProgramError> to Result<(), ProgramError>, removing the delegate account return value. Call sites are updated to accommodate the new signature and refactored to use the original delegate input directly.

Changes

Cohort / File(s)	Summary
Signature Simplification programs/compressed-token/program/src/shared/owner_validation.rs	Function return type changed from Result<Option<&'a AccountInfo>, ProgramError> to Result<(), ProgramError>. Success branches now uniformly return Ok(()). Error handling expanded with additional context logging before returning errors.
Call Site Adaptation programs/compressed-token/program/src/shared/token_input.rs	Function call result no longer bound to variable. delegate_account field now references the original input directly instead of verified delegate result for hashing computation.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

• Verify all call sites of verify_owner_or_delegate_signer have been updated to handle the new return type
• Confirm the delegate account reference in token_input.rs still correctly captures the intended input for hashing
• Review the expanded error logging in the delegate-present branch for consistency and message clarity

Poem

🐰 We simplified the delegate dance—
No need to return what we've chanced,
Just whisper "ok" and move along,
The verification stays just as strong! 🌟

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The PR title "perf: ctoken remove return value of verify_owner_or_delegate_signer" directly corresponds to the main change in the pull request. The primary modification is the function signature update where verify_owner_or_delegate_signer changes its return type from Result<Option<&'a AccountInfo>, ProgramError> to Result<(), ProgramError>, effectively removing the return value. The title is specific, concise, and uses the conventional "perf:" prefix to indicate a performance optimization. A teammate scanning the repository history would clearly understand that this PR removes the return value from that specific function without ambiguity.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch jorrit/perf-ctoken-verify_owner_or_delegate_signer

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

programs/compressed-token/program/src/shared/owner_validation.rs (1)

8-10: Docstring is stale after signature change; update to match Result<(), ProgramError>.

The comment still says it returns the delegate AccountInfo. It no longer does. Please update for accuracy.

-/// Verify owner or delegate signer authorization for token operations
-/// Returns the delegate account info if delegate is used, None otherwise
+/// Verify owner or delegate signer authorization for token operations.
+/// Succeeds if either the delegate (when provided) or the owner is a signer.
+/// Returns Ok(()) on success.

This helper continues to build on centralized signer checks from light_account_checks, which is consistent with keeping validation helpers unified. Based on learnings.

Also applies to: 14-14

🧹 Nitpick comments (1)

programs/compressed-token/program/src/shared/token_input.rs (1)

121-123: Confirm hashing semantics with delegate present but owner signing.

You now hash the delegate directly from input when present, regardless of who signed. That’s likely correct (account state should encode the delegate independent of signer), but if the previous code intentionally dropped the delegate when only the owner signed, this changes the hash.

• Please confirm intended behavior for:
  • delegate present + owner signs (no delegate sig): delegate should still be hashed.
  • delegate present + delegate signs: delegate hashed (unchanged).
  • no delegate: None (unchanged).

If helpful, add a clarifying comment:

-                let hashed_delegate =
-                    delegate_account.map(|delegate| hash_cache.get_or_hash_pubkey(delegate.key()));
+                // Hash the configured delegate (if any), independent of which signer authorized.
+                let hashed_delegate =
+                    delegate_account.map(|delegate| hash_cache.get_or_hash_pubkey(delegate.key()));

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1bbc3ee and 88fe6db.

📒 Files selected for processing (2)

• programs/compressed-token/program/src/shared/owner_validation.rs (3 hunks)
• programs/compressed-token/program/src/shared/token_input.rs (2 hunks)

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2025-10-11T21:59:25.222Z

Learnt from: CR
PR: Lightprotocol/light-protocol#0
File: program-libs/account-checks/CLAUDE.md:0-0
Timestamp: 2025-10-11T21:59:25.222Z
Learning: Applies to program-libs/account-checks/src/checks.rs : Expose and maintain account validation helpers (check_owner, check_program, check_mut/non_mut, check_signer, check_discriminator, set_discriminator, check_pda_seeds, check_account_balance_is_rent_exempt, account_info_init) in checks.rs

Applied to files:

• programs/compressed-token/program/src/shared/owner_validation.rs

🧬 Code graph analysis (1)

programs/compressed-token/program/src/shared/token_input.rs (1)

programs/compressed-token/program/src/shared/owner_validation.rs (1)

• verify_owner_or_delegate_signer (11-51)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (18)

• GitHub Check: stateless-js-v1
• GitHub Check: programs (light-system-program-address, ["cargo-test-sbf -p system-test -- test_with_address", "c...
• GitHub Check: programs (account-compression-and-registry, ["cargo-test-sbf -p account-compression-test", "cargo...
• GitHub Check: programs (system-cpi-test-v2-functional-account-infos, ["cargo-test-sbf -p system-cpi-v2-test -- ...
• GitHub Check: programs (system-cpi-test-v2-functional-read-only, ["cargo-test-sbf -p system-cpi-v2-test -- func...
• GitHub Check: programs (system-cpi-test, ["cargo-test-sbf -p system-cpi-test", "cargo test -p light-system-prog...
• GitHub Check: programs (compressed-token-batched-tree, ["cargo-test-sbf -p compressed-token-test -- test_transf...
• GitHub Check: programs (compressed-token-and-e2e, ["cargo-test-sbf -p compressed-token-test --test v1", "cargo-...
• GitHub Check: programs (light-system-program-compression, ["cargo-test-sbf -p system-test -- test_with_compress...
• GitHub Check: lint
• GitHub Check: system-programs (anchor & pinocchio, ["cargo-test-sbf -p sdk-anchor-test", "cargo-test-sbf -p sdk...
• GitHub Check: cli-v1
• GitHub Check: system-programs (native, ["cargo-test-sbf -p sdk-native-test", "cargo-test-sbf -p sdk-v1-native-t...
• GitHub Check: system-programs (sdk-libs, light-sdk-macros light-sdk light-program-test light-client light-compr...
• GitHub Check: cli-v2
• GitHub Check: system-programs (token test, ["cargo-test-sbf -p sdk-token-test"])
• GitHub Check: Forester e2e test
• GitHub Check: stateless-js-v2

🔇 Additional comments (2)

programs/compressed-token/program/src/shared/token_input.rs (1)

87-87: Call site updated correctly.

Authorization is enforced and no unused binding remains. Good.

programs/compressed-token/program/src/shared/owner_validation.rs (1)

38-38: Verification complete—all call sites are compatible.

The function correctly returns Result<(), ProgramError> with both success paths returning Ok(()) at lines 38 and 49. The only call site (token_input.rs:87) properly handles the Result type using the ? operator. No problematic patterns detected (no variable assignments or Option method chains on the result).

[SwenSchaeferjohann]

LGTM