feat: add fee payer to light token ixs#2189
Conversation
📝 WalkthroughWalkthroughAdds an optional Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant CTokenProgram
participant SysProg as SystemProgram
participant TokenProg as SPLToken
Client->>CTokenProgram: Send instruction + accounts (source/mint/dest/authority/[fee_payer?]/system_program)
alt fee_payer provided
CTokenProgram->>CTokenProgram: effective_payer = fee_payer
else no fee_payer
CTokenProgram->>CTokenProgram: effective_payer = authority (writable)
end
opt top-up required
CTokenProgram->>SysProg: multi_transfer_lamports(from: effective_payer → cmint, ctoken, ...) rgba(0,128,0,0.5)
SysProg-->>CTokenProgram: lamports transferred
end
CTokenProgram->>TokenProg: CPI to SPL token (mint/transfer/burn) rgba(0,0,255,0.5)
TokenProg-->>CTokenProgram: token operation result
CTokenProgram-->>Client: return success / error
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
programs/compressed-token/program/docs/ctoken/TRANSFER_CHECKED.md (1)
51-63: Update the intro/top-up wording to mention fee_payer.The initial compatibility section still states that the authority pays top-ups, which conflicts with the new optional fee_payer behavior. Please update the intro to reflect “fee_payer if provided, otherwise authority.” As per coding guidelines, documentation must match the actual behavior.
programs/compressed-token/program/docs/ctoken/TRANSFER.md (1)
50-61: Update payer wording in the intro/logic steps.This section adds fee_payer, but earlier text and the top-up step still state authority always pays. Please update those passages to “fee_payer if provided, otherwise authority” to keep the doc consistent with the new account layout. As per coding guidelines, documentation must match behavior.
programs/compressed-token/program/docs/ctoken/MINT_TO_CHECKED.md (1)
79-84: Clarify payer in the top-up transfer step.Line 83 states transfers are from authority, but the new optional
fee_payershould pay when present. Please update to reflect payer selection.✏️ Doc fix
- - Transfer lamports from authority to both accounts if needed + - Transfer lamports from payer (fee_payer if provided, otherwise authority) to both accounts if neededAs per coding guidelines, docs must match implementation.
🤖 Fix all issues with AI agents
In `@programs/compressed-token/program/docs/ctoken/BURN_CHECKED.md`:
- Around line 41-48: Update the BURN_CHECKED.md docs to match the
implementation: change the comparison table text from "Exactly 3 accounts" to
reflect an optional fourth account (e.g., "3 or 4 accounts (fee_payer
optional)"), update the security note so it states that the authority is the
top-up payer only when fee_payer is not supplied and that when a fee_payer is
provided it becomes the payer for rent top-ups; also correct the fee_payer
account attributes to match the diff (fee_payer is optional and must be a
signer/writable when used) and apply the same edits to the other referenced
section in the document that repeats these account/comparison details.
In `@programs/compressed-token/program/docs/ctoken/MINT_TO.md`:
- Around line 41-52: Update the execution/top-up wording in the MINT_TO.md
documentation to reflect the optional fee_payer behavior: change any line that
currently says "authority pays for top-ups" to "fee_payer if provided, otherwise
authority" (referencing the authority and fee_payer roles described in the
parameter list) so the top-up step matches the earlier parameter descriptions
and implementation.
In `@programs/compressed-token/program/src/ctoken/burn.rs`:
- Around line 119-124: The burn processors process_burn and process_burn_checked
expect exactly three accounts (token, mint, authority) but you're passing the
full accounts slice (which may include fee_payer) via effective_payer; before
calling the Pinocchio burn routines, take a slice of the original accounts
containing only the first three account entries and pass that sliced accounts
array to process_burn/process_burn_checked (and keep
calculate_and_execute_compressible_top_ups using effective_payer for top-ups as
before) so the processor contract is satisfied.
In `@programs/compressed-token/program/src/ctoken/transfer/checked.rs`:
- Around line 70-78: When fee_payer is present we are passing the full accounts
slice to pinocchio-token functions which require fewer accounts; update the
calls so process_transfer_checked and process_transfer receive only the expected
accounts: for process_transfer_checked (used in
process_transfer_extensions_transfer_checked and the hot path) pass the first
four accounts (source, mint, destination, authority) and for process_transfer
(the non-extension path when passing Some(decimals)) pass the first three
accounts; locate calls to process_transfer_extensions_transfer_checked,
process_transfer_checked, and process_transfer and slice the accounts input
accordingly when fee_payer != None, keeping TransferAccounts construction
(source, destination, authority, mint, fee_payer) unchanged for local use but
ensuring the underlying pinocchio calls use accounts[..4] or accounts[..3] as
appropriate.
In `@sdk-libs/token-sdk/src/instruction/burn_checked.rs`:
- Around line 119-136: The authority AccountMeta is incorrectly made readonly
unless both max_top_up is set and fee_payer is None; instead make authority
writable whenever fee_payer is None so it can be charged as the fallback payer.
In burn_checked.rs update the authority_meta construction (the variable
referenced as authority_meta) to check only self.fee_payer.is_none(): when
fee_payer is None use a writable AccountMeta for self.authority
(AccountMeta::new), otherwise use a readonly AccountMeta
(AccountMeta::new_readonly); ensure references to self.max_top_up are removed
from this writable-check.
In `@sdk-libs/token-sdk/src/instruction/mint_to.rs`:
- Around line 78-97: Remove the unused system_program account from the MintTo
CPI: delete the system_program field from the MintToCpi struct and any
constructors/builders that set it, and update the invoke() implementation to
pass only [self.mint, self.destination, self.authority, fee_payer?] (or
[self.mint, self.destination, self.authority] when fee_payer is None) to
invoke()—i.e., remove self.system_program from both account arrays; also remove
any other references/usages of system_program in MintToCpi (including tests or
builders) so the CPI account list matches MintTo::instruction() and avoids
Solana CPI validation failures.
In `@sdk-libs/token-sdk/src/instruction/transfer.rs`:
- Around line 30-35: The doc comment for the Transfer instruction wrongly states
that a system_program account is added when max_top_up is set; update the
comment on the max_top_up and fee_payer fields to remove that system_program
reference and instead note that the processor performs lamports transfers
internally (via multi_transfer_lamports) so no system_program account is
required in the instruction accounts; keep mentions of max_top_up: Option<u16>
and fee_payer: Option<Pubkey> and clarify that fee_payer, if present, pays
top-ups while authority pays otherwise.
♻️ Duplicate comments (3)
sdk-libs/token-sdk/src/instruction/transfer_checked.rs (1)
133-150: Same payer mutability issue as in BurnChecked.Line 135-139 uses
max_top_upto decide authority writability; if top-ups run for legacy payloads, payer may be readonly. Apply the same fix as inburn_checked.rs.sdk-libs/token-sdk/src/instruction/transfer.rs (1)
110-127: Same payer mutability issue as in BurnChecked.Line 112-116 uses
max_top_upto decide authority writability; apply the same fix as inburn_checked.rs.sdk-libs/token-sdk/src/instruction/mint_to_checked.rs (1)
119-136: Same payer mutability issue as in BurnChecked.Line 121-125 uses
max_top_upto decide authority writability; apply the same fix as inburn_checked.rs.
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
programs/compressed-token/program/src/ctoken/transfer/default.rs (1)
43-48: Guard pinocchio calls against extra accounts when fee_payer is present.Same issue that was flagged in
checked.rs: pinocchio'sprocess_transferexpects exactly 3 accounts (source, destination, authority). Whenfee_payeris present, the fullaccountsslice contains 5 elements (including system_program and fee_payer), which will be rejected or misinterpreted by pinocchio's account validation.Both the hot path (line 46) and the post-extension path (line 65) pass the full slice. Slice to the first 3 accounts when extras are present.
🔧 Proposed fix
// Hot path: 165-byte accounts have no extensions, skip all extension processing // SAFETY: accounts.len() >= 3 validated at function entry let source = &accounts[ACCOUNT_SOURCE]; let destination = &accounts[ACCOUNT_DESTINATION]; if source.data_len() == 165 && destination.data_len() == 165 { - return process_transfer(accounts, &instruction_data[..8], false) + // pinocchio expects exactly 3 accounts: source, destination, authority + let transfer_accounts = &accounts[..=ACCOUNT_AUTHORITY]; + return process_transfer(transfer_accounts, &instruction_data[..8], false) .map_err(convert_pinocchio_token_error); }let signer_is_validated = process_extensions(accounts, max_top_up)?; // Only pass the first 8 bytes (amount) to the SPL transfer processor - process_transfer(accounts, &instruction_data[..8], signer_is_validated) + // pinocchio expects exactly 3 accounts: source, destination, authority + let transfer_accounts = &accounts[..=ACCOUNT_AUTHORITY]; + process_transfer(transfer_accounts, &instruction_data[..8], signer_is_validated) .map_err(convert_pinocchio_token_error)Also applies to: 62-67
sdk-libs/token-sdk/src/instruction/transfer.rs (1)
135-141: Authority must be writable wheneverfee_payeris absent.Same issue as other instruction types: the processor can execute top-ups in legacy format without
max_top_up. Whenfee_payerisNone, authority needs write access to pay for top-ups.Proposed fix
- let authority_meta = if self.max_top_up.is_some() && self.fee_payer.is_none() { + let authority_meta = if self.fee_payer.is_none() { AccountMeta::new(self.authority, true) } else { AccountMeta::new_readonly(self.authority, true) };sdk-libs/token-sdk/src/instruction/transfer_checked.rs (1)
150-156: Authority must be writable wheneverfee_payeris absent.Consistent with other instruction types, the authority should be writable when it's the fallback payer, regardless of
max_top_uppresence.Proposed fix
- let authority_meta = if self.max_top_up.is_some() && self.fee_payer.is_none() { + let authority_meta = if self.fee_payer.is_none() { AccountMeta::new(self.authority, true) } else { AccountMeta::new_readonly(self.authority, true) };
🤖 Fix all issues with AI agents
In `@sdk-libs/token-sdk/src/instruction/burn_checked.rs`:
- Around line 146-147: Replace the placeholder zero Pubkey used for the System
Program with the actual System Program ID: change
AccountMeta::new_readonly(Pubkey::default(), false) to use
solana_program::system_program::id() (e.g.,
AccountMeta::new_readonly(system_program::id(), false)) in the instruction
construction in burn_checked.rs so the rent top-up CPI targets the real system
program account.
In `@sdk-libs/token-sdk/src/instruction/burn.rs`:
- Around line 128-134: The authority account is currently made writable only
when max_top_up.is_some() && fee_payer.is_none(), which breaks legacy (8-byte)
instructions because the processor will still perform top-ups and needs
authority to be writable when fee_payer is absent; update the logic that builds
authority_meta (the code using AccountMeta::new and AccountMeta::new_readonly)
so that authority is created writable whenever self.fee_payer.is_none(),
otherwise make it readonly — i.e., check fee_payer first and only make it
readonly when a fee_payer exists.
- Around line 140-142: Replace the placeholder zero pubkey used for the system
program with the actual system program ID: locate the
AccountMeta::new_readonly(Pubkey::default(), false) in burn.rs and change it to
use the real system program constant (e.g., system_program::ID) by importing the
symbol (for example add use solana_program::system_program::ID or the equivalent
solana_pubkey::system_program::ID depending on the crate you use) so the rent
top-up CPI references the correct System Program account.
In `@sdk-libs/token-sdk/src/instruction/transfer_checked.rs`:
- Around line 163-164: The System Program AccountMeta uses Pubkey::default()
which is incorrect for rent top-up CPIs; replace
AccountMeta::new_readonly(Pubkey::default(), false) with
AccountMeta::new_readonly(system_program::ID, false) (or system_program::id()),
and add the necessary import (use solana_program::system_program::ID or use
solana_program::system_program). Update the instruction builder in
transfer_checked.rs where AccountMeta::new_readonly(...) is created to reference
system_program::ID instead of Pubkey::default().
In `@sdk-libs/token-sdk/src/instruction/transfer.rs`:
- Around line 147-148: The AccountMeta for the system program is using
Pubkey::default(), which is incorrect; replace
AccountMeta::new_readonly(Pubkey::default(), false) with the real System Program
ID (e.g. use solana_program::system_program::ID or
solana_program::system_program::id() as the pubkey) so the rent top-up CPI
targets the actual system program; update the AccountMeta::new_readonly call in
transfer.rs accordingly.
♻️ Duplicate comments (3)
programs/compressed-token/program/src/ctoken/transfer/checked.rs (1)
52-55: Guard pinocchio calls against extra accounts when fee_payer is present.The hot path (line 53) passes the full
accountsslice toprocess_transfer_checked, which expects exactly 4 accounts. Similarly, the non-extension path (line 105) passes the full slice toprocess_transferwithSome(decimals), which also expects 4 accounts. When fee_payer is present, the slice has 6 elements.Note: The extension_decimals branch (lines 95-102) is already correct—it explicitly creates a 3-element slice.
🔧 Proposed fix
// Hot path: 165-byte accounts have no extensions, skip all extension processing if source.data_len() == 165 && destination.data_len() == 165 { - return process_transfer_checked(accounts, &instruction_data[..9], false) + // pinocchio expects exactly 4 accounts: source, mint, destination, authority + let transfer_accounts = &accounts[..=ACCOUNT_AUTHORITY]; + return process_transfer_checked(transfer_accounts, &instruction_data[..9], false) .map_err(convert_pinocchio_token_error); }} else { check_token_program_owner(mint)?; - process_transfer(accounts, amount, Some(decimals), signer_is_validated) + // pinocchio expects 4 accounts when Some(decimals): source, mint, destination, authority + let transfer_accounts = &accounts[..=ACCOUNT_AUTHORITY]; + process_transfer(transfer_accounts, amount, Some(decimals), signer_is_validated) .map_err(convert_pinocchio_token_error) }Also applies to: 103-107
programs/compressed-token/program/src/ctoken/burn.rs (1)
117-117: Slice accounts before calling pinocchio burn processors.The
process_burnandprocess_burn_checkedprocessors expect exactly 3 accounts (token, mint, authority). With the new account layout includingsystem_programand optionalfee_payer, passing the fullaccountsslice violates the processor contract and will cause failures.Proposed fix
- processor(accounts, &instruction_data[..BASE_LEN]).map_err(convert_pinocchio_token_error)?; + processor(&accounts[..3], &instruction_data[..BASE_LEN]).map_err(convert_pinocchio_token_error)?;sdk-libs/token-sdk/src/instruction/burn_checked.rs (1)
134-140: Authority must be writable wheneverfee_payeris absent.The processor executes top-ups even without
max_top_up. Whenfee_payerisNone, authority becomes the fallback payer and requires write access.Proposed fix
- let authority_meta = if self.max_top_up.is_some() && self.fee_payer.is_none() { + let authority_meta = if self.fee_payer.is_none() { AccountMeta::new(self.authority, true) } else { AccountMeta::new_readonly(self.authority, true) };
…p CPIs The CToken program needs System Program in transaction accounts when performing rent top-ups via CPI. This adds system_program at index 3 for MintTo, Burn, and Transfer instructions (and their checked variants), with fee_payer moved to index 4. Changes: - Program: Update account indices (system_program=3, fee_payer=4) - SDK: Add system_program to instruction builders and CPI structs - Tests: Update BurnCpi usages with system_program field
Add the missing system_program field to TransferCpi and TransferCheckedCpi struct initializers in the example test program, and pass the system_program account from the tests. This fixes the build failure caused by the new required system_program field added to the CPI structs for rent top-up support.
Pass only the expected accounts to pinocchio-token-program transfer functions as a defensive measure: - process_transfer: slice to 3 accounts [source, destination, authority] - process_transfer_checked: slice to 4 accounts [source, mint, destination, authority] While pinocchio tolerates extra accounts via `remaining @ ..` pattern, explicitly slicing is cleaner and more robust.
ananas-block
force-pushed
the
jorrit/chore-add-fee-payer-token-program
branch
from
52f2161 to
81a0773
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
programs/compressed-token/program/docs/ctoken/TRANSFER_CHECKED.md (1)
9-12: Update top-up payer wording to include fee_payer.The intro and logic step still say lamports are transferred from the authority account; with optional
fee_payer, this should be conditional. As per coding guidelines, docs must match implementation.✏️ Doc fix
-When accounts require rent top-up, lamports are transferred directly from the authority account to the token accounts. The authority must have sufficient lamports to cover the top-up amount. +When accounts require rent top-up, lamports are transferred from fee_payer when provided; otherwise the authority funds the top-up and must have sufficient lamports.- - Transfer lamports from authority to accounts if top-up needed: + - Transfer lamports from fee_payer when provided; otherwise authority pays:Also applies to: 109-112
programs/compressed-token/program/docs/ctoken/MINT_TO.md (1)
26-58: Addsystem_programto the account list and min‑account validation.The SDK and program now expect a system_program placeholder ahead of the optional fee_payer; the docs still list only 3 accounts + fee_payer and require 3 minimum, which shifts the fee_payer index and can mislead integrators. Please add system_program as account 4 and update the minimum account count (and numbering).
✏️ Suggested doc patch
@@ -3. authority +3. authority - (signer) - Mint authority of the CMint account @@ -4. fee_payer (optional) +4. system_program + - (readonly) + - System Program for rent top-up transfers + +5. fee_payer (optional) - (signer, writable) - Optional separate account to pay for rent top-ups @@ -1. **Validate minimum accounts:** - - Require at least 3 accounts (cmint, destination, authority) +1. **Validate minimum accounts:** + - Require at least 4 accounts (cmint, destination, authority, system_program)As per coding guidelines, documentation must reflect the actual account layout.
programs/compressed-token/program/docs/ctoken/BURN.md (1)
20-51: Document thesystem_programaccount (and shift fee_payer index).
sdk-libs/token-sdk/src/instruction/burn.rsnow includes system_program before the optional fee_payer (see BurnCpi fields and the accounts vector inBurn::instruction), but this doc still lists fee_payer as account 4 and minimum accounts as 3. Please align the account list and minimum accounts to match the SDK/program layout (system_program as account 4, fee_payer optional as account 5).✏️ Suggested doc patch
@@ -3. authority +3. authority - (signer) - Owner of the source CToken account @@ -4. fee_payer (optional) +4. system_program + - (readonly) + - System Program for rent top-up transfers + +5. fee_payer (optional) - (signer, writable) - Optional separate account to pay for rent top-ups @@ -1. **Validate minimum accounts:** - - Require at least 3 accounts (source CToken, CMint, authority/payer) +1. **Validate minimum accounts:** + - Require at least 4 accounts (source CToken, CMint, authority, system_program)As per coding guidelines, documentation must match the implementation and SDK surface.
programs/compressed-token/program/docs/ctoken/TRANSFER.md (2)
7-16: Update account layout/compatibility text for system_program + fee_payer.The program now reserves
ACCOUNT_SYSTEM_PROGRAMandACCOUNT_FEE_PAYERinprograms/compressed-token/program/src/ctoken/transfer/default.rs(lines 13‑15), so the docs should no longer state “same account layout as SPL” without clarifying the extra accounts. Please add system_program as account 4, fee_payer optional as account 5, and update the minimum account count.✏️ Suggested doc patch
@@ -**Important:** This instruction uses the same account layout as SPL Token transfer (source, destination, authority) but has extended instruction data format. +**Important:** The first three accounts match SPL Token transfer (source, destination, authority). Additional accounts (system_program, optional fee_payer) are appended for rent top-ups. @@ -3. authority +3. authority - (signer) @@ -4. fee_payer (optional) +4. system_program + - (readonly) + - System Program for rent top-up transfers + +5. fee_payer (optional) - (signer, writable) @@ -1. **Validate minimum accounts:** - - Require at least 3 accounts (source, destination, authority) +1. **Validate minimum accounts:** + - Require at least 4 accounts (source, destination, authority, system_program)As per coding guidelines, documentation must mirror the on‑chain account layout.
Also applies to: 37-67
11-11: Top‑up payer wording should include the fee_payer fallback.These sections still say the authority funds top‑ups. Update to “fee_payer if provided, otherwise authority” to match the new behavior.
✏️ Suggested doc patch
-When accounts require rent top-up, lamports are transferred directly from the authority account to the token accounts. +When accounts require rent top-up, lamports are transferred from fee_payer if provided, otherwise from the authority account. @@ - - Execute multi_transfer_lamports from authority to accounts + - Execute multi_transfer_lamports from fee_payer if provided, otherwise authorityAs per coding guidelines, documentation must match implementation behavior.
Also applies to: 106-109
sdk-libs/token-sdk/src/instruction/transfer.rs (1)
135-141: Authority must be writable wheneverfee_payeris None.Top-ups can still run without
max_top_up, so the fallback payer must always be writable.🔧 Suggested fix
- let authority_meta = if self.max_top_up.is_some() && self.fee_payer.is_none() { + let authority_meta = if self.fee_payer.is_none() { AccountMeta::new(self.authority, true) } else { AccountMeta::new_readonly(self.authority, true) };sdk-libs/token-sdk/src/instruction/transfer_checked.rs (1)
150-156: Authority must be writable wheneverfee_payeris None.Legacy format still allows top-ups, so the fallback payer must always be writable.
🔧 Suggested fix
- let authority_meta = if self.max_top_up.is_some() && self.fee_payer.is_none() { + let authority_meta = if self.fee_payer.is_none() { AccountMeta::new(self.authority, true) } else { AccountMeta::new_readonly(self.authority, true) };
🤖 Fix all issues with AI agents
In `@programs/compressed-token/program/docs/ctoken/MINT_TO_CHECKED.md`:
- Around line 44-55: Update the MINT_TO_CHECKED.md account list and descriptions
to include the required system_program account and to clarify the fee payer
fallback: add a new "system_program" account entry (readonly, not signer)
required by the SDK/program, update "authority" to be signer and writable when
it will act as the top-up payer (i.e., when no fee_payer provided) and readonly
if a separate fee_payer is supplied, and update "fee_payer (optional)" to state
it is signer and writable when present and that if omitted the authority must be
writable and have sufficient lamports to cover rent top-ups; also update the
"minimum accounts" and any top-up payer language elsewhere in the doc (including
the other occurrences noted) to match this flow so docs reflect the actual
implementation.
In `@sdk-libs/token-sdk/src/instruction/mint_to_checked.rs`:
- Around line 144-150: The authority Meta is currently only made writable when
max_top_up.is_some() && fee_payer.is_none(), but per review authority must be
writable whenever fee_payer.is_none(); update the construction of authority_meta
(the variable in mint_to_checked.rs) so it uses AccountMeta::new(self.authority,
true) whenever self.fee_payer.is_none(), and fall back to
AccountMeta::new_readonly(self.authority, true) only when fee_payer.is_some();
adjust the condition around self.max_top_up and self.fee_payer accordingly.
In `@sdk-libs/token-sdk/src/instruction/mint_to.rs`:
- Around line 138-144: The logic that builds authority_meta currently makes the
authority writable only when max_top_up.is_some() and fee_payer.is_none(), but
authority must be writable whenever fee_payer is None (it may be the fallback
payer even if max_top_up is None); update the condition used to construct
authority_meta (the place that sets AccountMeta::new vs
AccountMeta::new_readonly) to make it writable whenever
self.fee_payer.is_none(), e.g., use if self.fee_payer.is_none() {
AccountMeta::new(self.authority, true) } else {
AccountMeta::new_readonly(self.authority, true) } so the authority can be
debited in legacy top-up flows.
♻️ Duplicate comments (10)
programs/compressed-token/program/src/ctoken/burn.rs (1)
103-118: Slice accounts before calling pinocchio burn processors.
process_burn/process_burn_checkedare defined for exactly 3 accounts (token, mint, authority). Withsystem_program+ optionalfee_payerappended, passing the full slice can break the processor contract. Slice to the first three accounts and keep the full list for top-ups.🛠️ Proposed fix
- processor(accounts, &instruction_data[..BASE_LEN]).map_err(convert_pinocchio_token_error)?; + let burn_accounts = &accounts[..=PAYER_IDX]; + processor(burn_accounts, &instruction_data[..BASE_LEN]).map_err(convert_pinocchio_token_error)?;Pinocchio token program burn / burn_checked processor account requirements (expected accounts length/order)programs/compressed-token/program/docs/ctoken/BURN_CHECKED.md (1)
41-48: Comparison table & security notes still assume 3 accounts / authority-only payer.The accounts section now documents an optional
fee_payer, but the comparison table and security notes still say “Exactly 3 accounts” and “authority pays.” Update these to reflect 3-or-4 accounts and payer selection. As per coding guidelines, docs must match implementation.✏️ Doc fix
-| [source_ctoken, cmint, authority] | [source_ctoken, cmint, authority] | -| Exactly 3 accounts | Exactly 3 accounts | +| [source_ctoken, cmint, authority(, fee_payer?)] | [source_ctoken, cmint, authority(, fee_payer?)] | +| 3 or 4 accounts (fee_payer optional) | 3 or 4 accounts (fee_payer optional) |-2. **Top-Up Payer is Authority:** - - Authority (signer) serves as payer for rent top-ups +2. **Top-Up Payer Selection:** + - Payer is fee_payer when provided; otherwise authority (signer) paysAlso applies to: 160-175
programs/compressed-token/program/docs/ctoken/MINT_TO.md (1)
98-102: Top‑up funding source still says “authority only.”Step 5 should say “fee_payer if provided, otherwise authority” to match the new payer semantics.
✏️ Suggested doc patch
- - Use authority account (third account) as funding source + - Use fee_payer if provided, otherwise authority, as the funding sourceAs per coding guidelines, documentation must match implementation behavior.
sdk-libs/token-sdk/src/instruction/burn.rs (2)
126-134: Make authority writable wheneverfee_payerisNone.Top‑ups can happen even in the legacy format (no
max_top_up), so iffee_payeris absent the authority must still be writable to fund any required top‑ups.🐛 Proposed fix
- let authority_meta = if self.max_top_up.is_some() && self.fee_payer.is_none() { + let authority_meta = if self.fee_payer.is_none() { AccountMeta::new(self.authority, true) } else { AccountMeta::new_readonly(self.authority, true) };
136-142: Use the real System Program ID, notPubkey::default().
Pubkey::default()is the all‑zeros key, which is not the System Program. The CPI needs the actual System Program account.🐛 Proposed fix
-use solana_pubkey::Pubkey; +use solana_pubkey::Pubkey; +use solana_pubkey::system_program; @@ - AccountMeta::new_readonly(Pubkey::default(), false), + AccountMeta::new_readonly(system_program::ID, false),sdk-libs/token-sdk/src/instruction/burn_checked.rs (2)
134-140: Authority must be writable wheneverfee_payeris None.Same issue as noted previously: authority is still the fallback payer even without
max_top_up.🔧 Suggested fix
- let authority_meta = if self.max_top_up.is_some() && self.fee_payer.is_none() { + let authority_meta = if self.fee_payer.is_none() { AccountMeta::new(self.authority, true) } else { AccountMeta::new_readonly(self.authority, true) };
142-148: Use the real System Program ID.
Pubkey::default()is not the system program and will break CPI account validation.🔧 Suggested fix
-use solana_pubkey::Pubkey; +use solana_pubkey::{system_program, Pubkey}; ... - AccountMeta::new_readonly(Pubkey::default(), false), + AccountMeta::new_readonly(system_program::ID, false),sdk-libs/token-sdk/src/instruction/transfer.rs (2)
30-35: Doc comment misstates system_program behavior.The system program account is now always included in the instruction accounts; it’s not conditional on
max_top_up.✏️ Suggested fix
- /// When set, includes max_top_up in instruction data and adds system program account for compressible top-up + /// When set, includes max_top_up in instruction data
147-148: Use the real System Program ID.
Pubkey::default()won’t match the system program account info and will fail CPI account validation.🔧 Suggested fix
-use solana_pubkey::Pubkey; +use solana_pubkey::{system_program, Pubkey}; ... - AccountMeta::new_readonly(Pubkey::default(), false), + AccountMeta::new_readonly(system_program::ID, false),sdk-libs/token-sdk/src/instruction/transfer_checked.rs (1)
163-165: Use the real System Program ID.
Pubkey::default()is not the system program and will break CPI account validation.🔧 Suggested fix
-use solana_pubkey::Pubkey; +use solana_pubkey::{system_program, Pubkey}; ... - AccountMeta::new_readonly(Pubkey::default(), false), + AccountMeta::new_readonly(system_program::ID, false),
| 3. authority | ||
| - (signer, writable when top-ups needed) | ||
| - (signer) | ||
| - Mint authority of the CMint account | ||
| - Validated: must sign the transaction | ||
| - Also serves as payer for rent top-ups if needed | ||
| - If no fee_payer provided: also serves as payer for top-ups (must be writable) | ||
| - If fee_payer provided: readonly (only needs to sign) | ||
|
|
||
| 4. fee_payer (optional) | ||
| - (signer, writable) | ||
| - Optional separate account to pay for rent top-ups | ||
| - If not provided, authority account pays for top-ups | ||
| - Must have sufficient lamports to cover the top-up amount |
There was a problem hiding this comment.
Document the system_program account and payer fallback clearly.
The SDK/program path now expects a system_program placeholder plus optional fee_payer. The docs still list only 3 required accounts and say authority pays top-ups unconditionally. Please update the account list, minimum accounts, and top-up payer language to match the new flow. As per coding guidelines, docs must match the implementation.
📝 Suggested doc update
-3. authority
+3. authority
- (signer)
- Mint authority of the CMint account
- Validated: must sign the transaction
- If no fee_payer provided: also serves as payer for top-ups (must be writable)
- If fee_payer provided: readonly (only needs to sign)
-4. fee_payer (optional)
+4. system_program
+ - (readonly)
+ - System program used for rent/top-up transfers
+
+5. fee_payer (optional)
- (signer, writable)
- Optional separate account to pay for rent top-ups
- If not provided, authority account pays for top-ups
- Must have sufficient lamports to cover the top-up amount-1. **Validate minimum accounts:**
- - Require at least 3 accounts (cmint, destination, authority)
+1. **Validate minimum accounts:**
+ - Require at least 4 accounts (cmint, destination, authority, system_program)- - Transfer lamports from authority to both accounts if needed
+ - Transfer lamports from fee_payer if provided, otherwise authorityAlso applies to: 59-61, 79-84
🤖 Prompt for AI Agents
In `@programs/compressed-token/program/docs/ctoken/MINT_TO_CHECKED.md` around
lines 44 - 55, Update the MINT_TO_CHECKED.md account list and descriptions to
include the required system_program account and to clarify the fee payer
fallback: add a new "system_program" account entry (readonly, not signer)
required by the SDK/program, update "authority" to be signer and writable when
it will act as the top-up payer (i.e., when no fee_payer provided) and readonly
if a separate fee_payer is supplied, and update "fee_payer (optional)" to state
it is signer and writable when present and that if omitted the authority must be
writable and have sufficient lamports to cover rent top-ups; also update the
"minimum accounts" and any top-up payer language elsewhere in the doc (including
the other occurrences noted) to match this flow so docs reflect the actual
implementation.
| // Authority is writable only when max_top_up is set AND no fee_payer | ||
| // (authority pays for top-ups only if no separate fee_payer) | ||
| let authority_meta = if self.max_top_up.is_some() && self.fee_payer.is_none() { | ||
| AccountMeta::new(self.authority, true) | ||
| } else { | ||
| AccountMeta::new_readonly(self.authority, true) | ||
| }; |
There was a problem hiding this comment.
Authority must be writable whenever fee_payer is None.
Even in legacy format, top-ups can occur; the fallback payer must be writable.
🔧 Suggested fix
- let authority_meta = if self.max_top_up.is_some() && self.fee_payer.is_none() {
+ let authority_meta = if self.fee_payer.is_none() {
AccountMeta::new(self.authority, true)
} else {
AccountMeta::new_readonly(self.authority, true)
};🤖 Prompt for AI Agents
In `@sdk-libs/token-sdk/src/instruction/mint_to_checked.rs` around lines 144 -
150, The authority Meta is currently only made writable when
max_top_up.is_some() && fee_payer.is_none(), but per review authority must be
writable whenever fee_payer.is_none(); update the construction of authority_meta
(the variable in mint_to_checked.rs) so it uses AccountMeta::new(self.authority,
true) whenever self.fee_payer.is_none(), and fall back to
AccountMeta::new_readonly(self.authority, true) only when fee_payer.is_some();
adjust the condition around self.max_top_up and self.fee_payer accordingly.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (10)
programs/compressed-token/program/docs/ctoken/BURN.md (1)
34-45: Accounts list omitssystem_program, shiftingfee_payerto the wrong slot.The implementation expects account 4 =
system_programand account 5 = optionalfee_payer. The current doc listsfee_payeras account 4, which would make callers pass it in the wrong position and silently fall back to authority-as-payer. Please update the account list accordingly. As per coding guidelines, documentation must match implementation.📚 Proposed doc fix
3. authority - (signer) - Owner of the source CToken account - Must sign the transaction - If no fee_payer provided: also serves as payer for top-ups (must be writable) - If fee_payer provided: readonly (only needs to sign) -4. fee_payer (optional) - - (signer, writable) - - Optional separate account to pay for rent top-ups - - If not provided, authority account pays for top-ups - - Must have sufficient lamports to cover the top-up amount +4. system_program + - (readonly) + - Required for rent top-up CPIs + +5. fee_payer (optional) + - (signer, writable) + - Optional separate account to pay for rent top-ups + - If not provided, authority account pays for top-ups + - Must have sufficient lamports to cover the top-up amountsdk-libs/token-sdk/src/instruction/transfer_interface.rs (1)
132-139: Addsystem_programto LightToLight CPI account lists.The
Transfer::instruction()method always includessystem_programin its account metas (line 148 in transfer.rs), even whenfee_payer: None. However, the LightToLight CPI paths inTransferInterfaceCpi::invoke()(line 349) andinvoke_signed()(line 419) pass only[source_account, destination_account, authority]—missing the requiredsystem_programaccount.The instruction will fail at invocation because the account count won't match. Include
self.system_programin the account_infos array for both LightToLight branches.sdk-libs/token-sdk/src/instruction/transfer_checked.rs (1)
150-156: Authority must be writable whenever fee_payer is None, regardless of max_top_up.Authority serves as the fallback payer for rent top-ups when no separate fee_payer is provided. The current logic incorrectly gates authority's writability on
max_top_up.is_some(), but top-ups can occur even with the legacy instruction format (9 bytes, max_top_up = 0 = no limit). If fee_payer is None and top-ups are needed, authority must be writable—otherwise the instruction fails at runtime.🔧 Suggested fix
- let authority_meta = if self.max_top_up.is_some() && self.fee_payer.is_none() { + let authority_meta = if self.fee_payer.is_none() { AccountMeta::new(self.authority, true) } else { AccountMeta::new_readonly(self.authority, true) };sdk-libs/token-sdk/src/instruction/transfer.rs (1)
135-141: Authority must be writable when fee_payer is absent, regardless of max_top_up.When no fee_payer is provided, authority serves as the fallback payer for rent top-ups. Top-ups can occur even in 8-byte format (where max_top_up is not explicitly set), so tying writability to both
max_top_up.is_some()andfee_payer.is_none()incorrectly blocks required lamport transfers when max_top_up is omitted.🔧 Suggested fix
- let authority_meta = if self.max_top_up.is_some() && self.fee_payer.is_none() { + let authority_meta = if self.fee_payer.is_none() { AccountMeta::new(self.authority, true) } else { AccountMeta::new_readonly(self.authority, true) };programs/compressed-token/program/docs/ctoken/TRANSFER_CHECKED.md (1)
109-111: Update top-up execution step to reflect fee_payer behavior.The account descriptions (lines 56-63) correctly state that fee_payer pays for top-ups when provided, with authority as fallback. However, this execution step still says "Transfer lamports from authority to accounts." Update to: "Transfer lamports from fee_payer (or authority if no fee_payer provided) to accounts if top-up needed."
📝 Suggested wording
- Transfer lamports from authority to accounts if top-up needed: + Transfer lamports from fee_payer (or authority if no fee_payer) to accounts if top-up needed:programs/compressed-token/program/docs/ctoken/TRANSFER.md (1)
108-108: Update top-up execution step to reflect fee_payer behavior.The account descriptions (lines 54-61) state that fee_payer pays for top-ups when provided, otherwise authority pays. However, this line says "Execute multi_transfer_lamports from authority to accounts." Update to reflect that fee_payer is the payer when provided.
📝 Suggested wording
- Execute multi_transfer_lamports from authority to accounts + Execute multi_transfer_lamports from fee_payer (or authority if no fee_payer) to accountsprograms/compressed-token/program/docs/ctoken/MINT_TO.md (1)
100-100: Incorrect payer reference in top-up execution step.This line states "Use authority account (third account) as funding source," which contradicts the account descriptions at lines 45-52. The fee_payer (when provided) should be the funding source, with authority as fallback. This was flagged in a past review but wasn't fully addressed in the execution logic.
📝 Suggested fix
- Use authority account (third account) as funding source + Use fee_payer if provided, otherwise authority account (third account) as funding sourceprograms/compressed-token/program/docs/ctoken/MINT_TO_CHECKED.md (1)
83-83: Update top-up execution step to reflect fee_payer behavior.The account descriptions (lines 48-55) correctly state that fee_payer pays for top-ups when provided, otherwise authority pays. However, this execution step says "Transfer lamports from authority to both accounts if needed." Update to reflect the conditional payer logic.
📝 Suggested wording
- Transfer lamports from authority to both accounts if needed + Transfer lamports from fee_payer (or authority if no fee_payer) to both accounts if neededprograms/compressed-token/program/docs/ctoken/BURN_CHECKED.md (2)
162-165: Update comparison table to reflect optional fee_payer account.The table states "Exactly 3 accounts" but with the addition of optional fee_payer, the instruction now accepts 3 or 4 accounts. This was flagged in a past review but the table wasn't updated.
📝 Suggested fix
-| [source, mint, authority, ...signers] | [source_ctoken, cmint, authority] | -| 3+ accounts (for multisig) | Exactly 3 accounts | +| [source, mint, authority, ...signers] | [source_ctoken, cmint, authority(, fee_payer?)] | +| 3+ accounts (for multisig) | 3 or 4 accounts (fee_payer optional) |
173-175: Update security note to reflect fee_payer as payer when provided.This security note states "Authority (signer) serves as payer for rent top-ups," which is incorrect when fee_payer is provided. The note should reflect that fee_payer is the payer when present, otherwise authority pays. This was flagged in a past review but wasn't corrected.
📝 Suggested fix
2. **Top-Up Payer is Authority:** - - Authority (signer) serves as payer for rent top-ups - - Burning tokens may require additional lamports from the authority's account + - Payer is fee_payer when provided; otherwise authority (signer) pays for rent top-ups + - Burning tokens may require additional lamports from the payer's accountOr retitle the section to "Top-Up Payer:" for accuracy.
🤖 Fix all issues with AI agents
In `@sdk-libs/token-sdk/src/instruction/mint_to_checked.rs`:
- Around line 144-150: The authority_meta logic incorrectly makes authority
writable only when max_top_up is Some; change it so authority is writable
whenever fee_payer is None regardless of max_top_up. In the mint_to_checked
construction update the branch that sets authority_meta (refer to
authority_meta, self.max_top_up, self.fee_payer, AccountMeta::new and
AccountMeta::new_readonly) to use AccountMeta::new(self.authority, true)
whenever self.fee_payer.is_none(), otherwise use
AccountMeta::new_readonly(self.authority, true).
- Around line 156-158: The AccountMeta using Pubkey::default() for the System
Program is incorrect; update the AccountMeta::new_readonly(...) in
mint_to_checked.rs to use the canonical System Program ID
(11111111111111111111111111111111) instead — e.g. construct the pubkey via
solana_pubkey::pubkey!() or a module-level const SYSTEM_PROGRAM_ID and pass that
into AccountMeta::new_readonly so the rent top-up CPI references the real System
Program account.
In `@sdk-libs/token-sdk/src/instruction/mint_to.rs`:
- Around line 150-152: Replace the placeholder system program Pubkey in the
AccountMeta entry used for rent top-up CPIs: locate the
AccountMeta::new_readonly(Pubkey::default(), false) in mint_to.rs and change it
to use the canonical system program pubkey via the pubkey! macro (the
"11111111111111111111111111111111" system program ID) so CPI validation succeeds
and matches the Light SDK pattern.
♻️ Duplicate comments (7)
programs/compressed-token/program/src/ctoken/burn.rs (1)
117-128: Slice accounts before invoking pinocchio burn processors.With system_program/fee_payer appended, the slice can be 4–5 accounts, but pinocchio burn/burn_checked expects exactly
[ctoken, cmint, authority]. Passing extras will fail account validation.🔧 Proposed fix
- processor(accounts, &instruction_data[..BASE_LEN]).map_err(convert_pinocchio_token_error)?; + let pinocchio_accounts = &accounts[..=PAYER_IDX]; + processor(pinocchio_accounts, &instruction_data[..BASE_LEN]) + .map_err(convert_pinocchio_token_error)?;pinocchio-token-program process_burn process_burn_checked required accountssdk-libs/token-sdk/src/instruction/burn.rs (2)
128-134: Authority must stay writable when no fee_payer is supplied.
With fee_payer = None, authority is the fallback payer for top‑ups; gating writability on max_top_up can still break lamport debits in legacy/implicit flows. Please keep authority writable whenever fee_payer is absent.🔧 Suggested fix
- let authority_meta = if self.max_top_up.is_some() && self.fee_payer.is_none() { + let authority_meta = if self.fee_payer.is_none() { AccountMeta::new(self.authority, true) } else { AccountMeta::new_readonly(self.authority, true) };
140-142: Use the real System Program ID (notPubkey::default()).
A zero pubkey will fail the CPI; this account must be the System Program.🔧 Suggested fix
- AccountMeta::new_readonly(Pubkey::default(), false), + AccountMeta::new_readonly(solana_pubkey::system_program::ID, false),sdk-libs/token-sdk/src/instruction/transfer.rs (1)
147-148: Use the real System Program ID (notPubkey::default()).
The placeholder pubkey will fail CPI validation.🔧 Suggested fix
- AccountMeta::new_readonly(Pubkey::default(), false), + AccountMeta::new_readonly(solana_pubkey::system_program::ID, false),sdk-libs/token-sdk/src/instruction/transfer_checked.rs (1)
163-164: Use the real System Program ID (notPubkey::default()).
The placeholder pubkey will fail CPI validation.🔧 Suggested fix
- AccountMeta::new_readonly(Pubkey::default(), false), + AccountMeta::new_readonly(solana_pubkey::system_program::ID, false),sdk-libs/token-sdk/src/instruction/burn_checked.rs (2)
134-140: Authority must stay writable when fee_payer is absent.
Authority is the fallback payer for top‑ups; tying writability to max_top_up can still block lamport debits.🔧 Suggested fix
- let authority_meta = if self.max_top_up.is_some() && self.fee_payer.is_none() { + let authority_meta = if self.fee_payer.is_none() { AccountMeta::new(self.authority, true) } else { AccountMeta::new_readonly(self.authority, true) };
146-147: Use the real System Program ID (notPubkey::default()).
A zero pubkey is not a valid System Program account for the top‑up CPI.🔧 Suggested fix
- AccountMeta::new_readonly(Pubkey::default(), false), + AccountMeta::new_readonly(solana_pubkey::system_program::ID, false),
2 participants
- Added fee_payer: Option<&'a AccountInfo> field to TransferAccounts struct in shared.rs
- Updated transfer_top_up function to use fee_payer with fallback to authority
- Added ACCOUNT_FEE_PAYER constant and optional fee_payer extraction in default.rs (index 3)
- Added ACCOUNT_FEE_PAYER constant and optional fee_payer extraction in checked.rs (index 4)
- Added FEE_PAYER_IDX constant (index 3) in burn.rs
- Implemented effective_payer logic using fee_payer with fallback to authority for shared supply change operations
- Changes apply to both burn.rs and mint_to.rs through shared process_ctoken_supply_change_inner function
- Added fee_payer: Option field to Transfer struct
- Added fee_payer: Option<AccountInfo<'info>> field to TransferCpi struct
- Implemented conditional CPI invocation with fixed-size arrays (one path with fee_payer, one without)
- Authority account mutability now depends on whether fee_payer is provided
- Added fee_payer field to TransferChecked and TransferCheckedCpi structs
- Implemented same conditional CPI pattern as Transfer
- Added fee_payer field to Burn and BurnCpi structs
- Implemented conditional CPI with fixed-size arrays
- Added fee_payer field to BurnChecked and BurnCheckedCpi structs
- Added fee_payer field to MintTo and MintToCpi structs
- Implemented conditional CPI pattern
- Added fee_payer field to MintToChecked and MintToCheckedCpi structs
- Updated TRANSFER.md, TRANSFER_CHECKED.md, BURN.md, BURN_CHECKED.md, MINT_TO.md, MINT_TO_CHECKED.md
- Documented new optional fee_payer account with its properties and behavior
- Fixed all struct initializers across test files to include fee_payer: None
- Updated 35+ test files in program-tests and sdk-tests directories
Summary by CodeRabbit
New Features
Bug Fixes / Errors
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.