Conversation
Contributor
WalkthroughAdds a new CallHandlerV2 instruction and processing path, a deprecated legacy builder, a new instruction builder and size-budget helper, escrow-related error constants, processor/account changes for CPI-style handling, module re-exports, and extensive Rust/JS tests exercising finalize and undelegate flows. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@src/discriminator.rs`:
- Around line 46-47: The doc comment for the enum variant CallHandlerV2 in
discriminator.rs is missing a trailing period; update the comment text "See
[crate::processor::process_call_handler_v2] for docs" to end with a period so it
matches the style of other variant comments (i.e., "See
[crate::processor::process_call_handler_v2] for docs.").
In `@src/processor/call_handler_v2.rs`:
- Around line 80-87: The log message in the executable check for source_program
prints destination_program.key by mistake; update the msg! call inside the
non-executable branch (the block that returns
Err(ProgramError::InvalidAccountData)) to reference source_program.key instead
of destination_program.key so the error correctly identifies the source program.
- Around line 19-31: The doc comment misnumbers accounts (duplicate index 3) and
makes the PDA origin ambiguous; update the account index list so indices are
sequential and match the named accounts: 0 = `[signer]` validator, 1 = `[]`
validator fee vault, 2 = `[]` destination program, 3 = `[]` source program, 4 =
`[]` escrow_authority, 5 = `[writable]` non-delegated escrow PDA created from
escrow_authority (index 4), 6 = `[readonly/writable]` other accounts needed for
action, 7 = `[readonly/writable]` other accounts needed for action, etc.; also
replace the ambiguous phrase “escrow pda created from 3” with “escrow PDA
created from escrow_authority (index 4)” to keep the documentation consistent
with function semantics in call_handler_v2.
In `@tests/test_call_handler_v2.rs`:
- Around line 186-202: The delegation metadata is being created with the wrong
seed index (using ephemeral_balance_seeds_from_payer!(payer.pubkey(), 0)) while
the actual ephemeral balance PDA (ephemeral_balance_pda) was derived with index
1; update the seeds passed to create_delegation_metadata_data and the
program_test.add_account call so they use
ephemeral_balance_seeds_from_payer!(payer.pubkey(), 1) (or otherwise match the
index used to derive ephemeral_balance_pda) so
delegation_metadata_pda_from_delegated_account(&ephemeral_balance_pda) and
create_delegation_metadata_data(...) use the same seed index.
taco-paco
commented
Feb 12, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@tests/integration/programs/test-delegation/src/lib.rs`:
- Around line 120-161: Both v2 handlers duplicate v1 logic; extract the shared
transfer logic into a private helper (e.g.,
perform_escrow_transfer(ctx_accounts: &T, amount: u64) or similar) and call it
from commit_base_action_handler_v2 and undelegate_base_action_handler_v2 to
remove verbatim duplication while preserving the extra counter update in
undelegate_base_action_handler_v2; ensure the helper accepts the context types
or account infos (used in Transfer creation) and that undelegate still
increments the Counter after calling the helper.
In `@tests/integration/tests/test-delegation.ts`:
- Around line 660-696: createCallHandlerV2Instruction relies on Buffer.alloc
zero-fill for bytes 1–7 of the 8-byte discriminator (only byte 0 is set), so
explicitly write the full 8-byte discriminator to the data buffer (e.g., use a
64-bit write like writeBigUInt64LE(BigInt(20), 0) or explicitly zero bytes 1–7
and set byte 0) before writing escrowIndex and lengths; also review the top-up
helper (the function that takes amount at line ~633) and consider using BigInt
for its amount parameter or clearly document/guard against
Number.MAX_SAFE_INTEGER to avoid silent precision loss.
- Around line 630-658: The discriminator for TopUpEphemeralBalance in
createTopUpEphemeralBalanceInstruction only writes the first byte and relies on
Buffer.alloc zero-fill; change it to the same explicit Buffer.from([...])
pattern used elsewhere by encoding all 8 discriminator bytes explicitly (instead
of data.writeUint8(9,0)), then append the u64 amount and u8 index bytes in order
so the final data buffer matches other instruction builders (keep references to
ephemeralBalancePdaFromPayer and DELEGATION_PROGRAM_ID intact).
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/processor/call_handler_v2.rs`:
- Around line 66-73: Add the same explicit executability check for
destination_program as is done for source_program: verify
destination_program.executable, emit a clear msg! referencing
destination_program.key (e.g., "{} program is not executable: destination
program"), and return Err(ProgramError::InvalidAccountData) on failure so
callers get a specific early error instead of a generic CPI failure; locate this
change near the existing source_program.executable check in call_handler_v2.rs.
GabrielePicco
approved these changes
Feb 18, 2026
Contributor
GabrielePicco
left a comment
GabrielePicco
left a comment
There was a problem hiding this comment.
LGTM!
Left a comment to address before merging
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/processor/call_handler_v2.rs`:
- Around line 79-95: The current construction of accounts_meta and
handler_accounts can duplicate entries when other_accounts contains keys equal
to source_program, escrow_authority_account, or escrow_account; update the
iterator used to build (accounts_meta, handler_accounts) to exclude any account
whose key equals validator.key OR equals source_program.key OR equals
escrow_authority_account.key OR equals escrow_account.key (or alternatively
deduplicate by key before appending the three fixed accounts) so the final three
entries are guaranteed to be the appended fixed accounts; adjust the map that
creates AccountMeta and the collected handler_accounts accordingly to avoid
duplicates.
- Around line 67-77: The code never verifies that the plain account passed as
source_program is executable, so add an explicit executability check for
source_program (e.g., right after the PDA checks for escrow_account) and return
a clear error (use or add INVALID_SOURCE_PROGRAM) if source_program.executable
is false; implement this by testing source_program.executable (or call the
project helper like require_executable if available) and returning the
appropriate ProgramError/early Err from the function before performing the CPI
so an arbitrary data account cannot be supplied as the source.
Comment on lines
+67
to
+77
| // verify passed escrow_account derived from escrow authority | ||
| let escrow_seeds: &[&[u8]] = | ||
| ephemeral_balance_seeds_from_payer!(escrow_authority_account.key, args.escrow_index); | ||
| let escrow_bump = load_pda( | ||
| escrow_account, | ||
| escrow_seeds, | ||
| &crate::id(), | ||
| true, | ||
| INVALID_ESCROW_PDA, | ||
| )?; | ||
| load_owned_pda(escrow_account, &system_program::id(), INVALID_ESCROW_OWNER)?; |
Contributor
There was a problem hiding this comment.
source_program executability is never verified.
source_program is passed as a plain account in the CPI (not the program_id), so the Solana runtime will not enforce that it is executable. Without an explicit check, any arbitrary data account can be supplied as source_program, giving the destination program a misleading "source" key — which directly undermines the feature's stated goal ("users must be able to get source program from where the action was scheduled").
A prior code review on this PR observed that source_program was checked for executability ("source_program is checked for executability, correctly, since it's passed as a plain account to CPI and the runtime won't validate it"), implying the check may have been removed in a later iteration.
🛡️ Proposed fix
// verify passed escrow_account derived from escrow authority
+ if !source_program.executable {
+ return Err(ProgramError::InvalidAccountData);
+ }
let escrow_seeds: &[&[u8]] =
ephemeral_balance_seeds_from_payer!(escrow_authority_account.key, args.escrow_index);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/processor/call_handler_v2.rs` around lines 67 - 77, The code never
verifies that the plain account passed as source_program is executable, so add
an explicit executability check for source_program (e.g., right after the PDA
checks for escrow_account) and return a clear error (use or add
INVALID_SOURCE_PROGRAM) if source_program.executable is false; implement this by
testing source_program.executable (or call the project helper like
require_executable if available) and returning the appropriate
ProgramError/early Err from the function before performing the CPI so an
arbitrary data account cannot be supplied as the source.
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
2 participants
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.
Problem
What problem are you trying to solve?
The users must be able to get source program from where the action was scheduled
Solution
How did you solve the problem?
Before & After Screenshots
Insert screenshots of example code output
BEFORE:
[insert screenshot here]
AFTER:
[insert screenshot here]
Other changes (e.g. bug fixes, small refactors)
Deploy Notes
Notes regarding deployment of the contained body of work. These should note any
new dependencies, new scripts, etc.
New scripts:
script: script detailsNew dependencies:
dependency: dependency detailsSummary by CodeRabbit