feat: enable refunds

[taco-paco]

In case transfer to destination account fails, which is done via action, the refund action will be created that will send funds to source. Refund also creates a callback to itself, so if refund action fails it will get recursively rescheduled until success

Summary by CodeRabbit

• New Features

  • Added a refund-on-failure callback to automatically schedule refunds for failed token transfers.
  • Added a new internal instruction to trigger the refund callback.

• Updates

  • Transfer callback execution updated to use the refund-on-failure flow.
  • Queue processing refactored to use authorized standalone actions for transfers.

• Tests

  • New integration tests validating success and failure refund behaviors.

[taco-paco]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 7

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@e-token/src/processor/internal/callbacks.rs`:
- Around line 43-45: The current deserialization in callbacks.rs reads a byte
via read_u8 and treats any non-zero as true (let ok = ... != 0), which violates
bincode bool encoding; change the logic to read the byte into a temporary (using
read_u8), then explicitly match the byte: 0 -> false, 1 -> true, and any other
value should return ProgramError::InvalidInstructionData so malformed bytes
(e.g. 2) fail to parse; update the code around the ok binding and keep
ProgramError::InvalidInstructionData for invalid values.
- Around line 66-74: The current construction of `signature` uses an unsafe
pointer cast to create a `&Signature` from `bytes`; replace that unsafe cast
with the safe `TryFrom<&[u8]>` conversion provided by the `solana-signature`
crate: after `read_slice(src, &mut cur, 64)` succeeds, call
`Signature::try_from(bytes).ok_or(ProgramError::InvalidInstructionData)?` (or
`TryFrom::try_from(bytes)`) to produce the signature instead of using `unsafe {
&*(bytes.as_ptr() as *const Signature) }`, keeping the surrounding match on
`tag`, the `read_slice` call, and the `signature: Option<&Signature>` logic
intact.

In `@e-token/src/processor/internal/queue_authorized_action.rs`:
- Around line 84-85: The code currently calls args.encode().unwrap() when
building the ESplInternalInstruction::ExecuteReadyQueuedTransfer payload which
will panic on encode failure; change the signature of new() to return
Result<Self, ProgramError>, replace unwrap() with propagating the encode error
(e.g. use args.encode().map_err(|e| /* convert to ProgramError */)? ) and adjust
callers to handle the Result so encoding failures surface as ProgramError
instead of aborting; locate this change around
ESplInternalInstruction::ExecuteReadyQueuedTransfer and the new() constructor to
apply the fix.

In `@e-token/src/processor/refund_on_failure_callback.rs`:
- Around line 170-176: The TODO about client_ref_id in ExecuteQueuedTransferArgs
should be resolved: determine whether refund_on_failure_callback requires a
client_ref_id for idempotency or external tracking and either populate
client_ref_id with an appropriate unique identifier (e.g., derive from
refund_id, transaction_id, or escrow_index) in ExecuteQueuedTransferArgs or
remove the TODO and add a comment explaining why None is intentional (e.g.,
refunds are idempotent via escrow_index and flags like
QUEUED_TRANSFER_FLAG_CREATE_IDEMPOTENT_ATA), and if this decision is deferred,
create a short tracked issue referencing refund_on_failure_callback.rs and
ExecuteQueuedTransferArgs so future work is captured.
- Line 24: Duplicate constant EXECUTE_READY_QUEUED_TRANSFER_ESCROW_INDEX is
defined in refund_on_failure_callback.rs and queue_authorized_action.rs; move
the constant into a shared module (e.g., create or use internal::mod.rs) as a
single pub const EXECUTE_READY_QUEUED_TRANSFER_ESCROW_INDEX: u8 = 0, then
replace the local definitions in both refund_on_failure_callback.rs and
queue_authorized_action.rs with a use
internal::EXECUTE_READY_QUEUED_TRANSFER_ESCROW_INDEX (or appropriate path) so
both modules reference the single shared symbol and avoid the duplicate.

In `@e-token/src/processor/transfer_queue_tick.rs`:
- Around line 258-264: Reject noncanonical Magic context accounts before
scheduling by validating tick_accounts.magic_context_info.key (or .address())
equals the expected MAGIC_CONTEXT_ID at every place you build callback metadata
or call invoke_standalone_action (e.g., where create_action_callback_accounts is
called and before invoke_standalone_action). If the keys do not match, return an
error instead of scheduling; do this check in the code paths that create
standalone_action_callback_accounts and the similar blocks around
queued_transfer and queue_state usage so the scheduled action and its callback
use the same canonical magic context account.

In `@e-token/tests/execute_transfer_callback.rs`:
- Around line 203-214: The test adds magic_fee_vault and MAGIC_CONTEXT_ID to
callback_ix but setup_context doesn't create or seed those accounts, so update
the fixture setup_context to allocate and seed a magic fee vault account (use
magic_fee_vault_pubkey(validator) as the PDA/seed and set the correct
owner/balance/token-account layout expected by the program) and
create/initialize the magic context account at MAGIC_CONTEXT_ID with the
expected account data/layout (or call the same initializer the program expects),
then ensure the new accounts are included in the test transaction context used
by callback_ix; apply the same seeding/creation for the other callback usage at
lines ~267-269 so both callback invocations use real seeded accounts rather than
placeholders.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: e4702b14-67bc-4aee-8394-5172a1b07574

📥 Commits

Reviewing files that changed from the base of the PR and between 5a8358c and 3d5508e.

📒 Files selected for processing (17)

• e-token/src/entrypoint.rs
• e-token/src/instruction.rs
• e-token/src/processor/deposit_and_delegate_shuttle_ephemeral_ata_with_merge_and_private_transfer.rs
• e-token/src/processor/deposit_and_queue_transfer.rs
• e-token/src/processor/execute_transfer_callback.rs
• e-token/src/processor/internal/callbacks.rs
• e-token/src/processor/internal/group_receipt.rs
• e-token/src/processor/internal/group_receipt_accounts.rs
• e-token/src/processor/internal/mod.rs
• e-token/src/processor/internal/queue_authorized_action.rs
• e-token/src/processor/mod.rs
• e-token/src/processor/refund_on_failure_callback.rs
• e-token/src/processor/transfer_queue_tick.rs
• e-token/src/processor/utils/mod.rs
• e-token/tests/execute_transfer_callback.rs
• e-token/tests/refund_on_failure_callback.rs
• e-token/tests/utils.rs

💤 Files with no reviewable changes (2)

• e-token/src/processor/utils/mod.rs
• e-token/src/processor/internal/group_receipt.rs

[coderabbitai]

🧹 Nitpick | 🔵 Trivial

Address or track the TODO comment.

The TODO on client_ref_id: None suggests uncertainty about whether this field is needed. Clarify the requirement and either implement it or remove the TODO with documentation explaining why None is intentional for refund callbacks.

Would you like me to open an issue to track this?

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@e-token/src/processor/refund_on_failure_callback.rs` around lines 170 - 176,
The TODO about client_ref_id in ExecuteQueuedTransferArgs should be resolved:
determine whether refund_on_failure_callback requires a client_ref_id for
idempotency or external tracking and either populate client_ref_id with an
appropriate unique identifier (e.g., derive from refund_id, transaction_id, or
escrow_index) in ExecuteQueuedTransferArgs or remove the TODO and add a comment
explaining why None is intentional (e.g., refunds are idempotent via
escrow_index and flags like QUEUED_TRANSFER_FLAG_CREATE_IDEMPOTENT_ATA), and if
this decision is deferred, create a short tracked issue referencing
refund_on_failure_callback.rs and ExecuteQueuedTransferArgs so future work is
captured.

[taco-paco]

@GabrielePicco Do we need to pass this?

[coderabbitai]

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

[snawaz]

The PR looks pretty good overall.

Anyway, here are minor feedback that can be addressed in this PR itself.

• currently RefundOnFailureCallback is infinitely recursive. Please put a limit on max number of retries. I guess 5 should be sufficient.
• ExecuteTransferCallback is not backward-compatible with the old 11-account shape. If there are already scheduled callbacks in flight, they will fail after this deployment. Maybe we support the backward-compatibility for few days (with a TODO (edwin/snawaz): remove it in the next deployment), as all scheduled callbacks will be executed almost immediately?
• the processor module should have the process_* function first, then the rest of the stuff should be after it and the common stuff (such as schedule_refund_on_failure) should be moved to internal/refund.rs or so.