Skip to content

p-token: Add batch instruction #35

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Mar 11, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 19 additions & 2 deletions interface/src/instruction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -477,6 +477,24 @@ pub enum TokenInstruction {
///
/// - `&str` The `ui_amount` of tokens to reformat.
UiAmountToAmount,

/// Executes a batch of instructions. The instructions to be executed are specified
/// in sequence on the instruction data. Each instruction provides:
/// - `u8`: number of accounts
/// - `u8`: instruction data length (includes the discriminator)
/// - `u8`: instruction discriminator
/// - `[u8]`: instruction data
///
/// Accounts follow a similar pattern, where accounts for each instruction are
/// specified in sequence. Therefore, the number of accounts expected by this
/// instruction is variable, i.e., it depends on the instructions provided.
///
/// Both the number of accounts and instruction data length are used to identify
/// the slice of accounts and instruction data for each instruction.
///
/// Note that it is not sound to have a `batch` instruction that contains other
/// `batch` instruction; an error will be raised when this is detected.
Batch = 255,
// Any new variants also need to be added to program-2022 `TokenInstruction`, so that the
// latter remains a superset of this instruction set. New variants also need to be added to
// token/js/src/instructions/types.ts to maintain @solana/spl-token compatibility
Expand All @@ -485,11 +503,10 @@ pub enum TokenInstruction {
impl TryFrom<u8> for TokenInstruction {
type Error = ProgramError;

#[inline(always)]
fn try_from(value: u8) -> Result<Self, Self::Error> {
match value {
// SAFETY: `value` is guaranteed to be in the range of the enum variants.
0..=24 => Ok(unsafe { core::mem::transmute::<u8, TokenInstruction>(value) }),
0..=24 | 255 => Ok(unsafe { core::mem::transmute::<u8, TokenInstruction>(value) }),
_ => Err(ProgramError::InvalidInstructionData),
}
}
Expand Down
1 change: 0 additions & 1 deletion p-token/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ crate-type = ["cdylib"]

[features]
logging = []
test-sbf = []

[dependencies]
pinocchio = { version = "0.7", git = "https://github.com/febo/pinocchio.git", branch = "febo/close-unstable" }
Expand Down
140 changes: 83 additions & 57 deletions p-token/src/entrypoint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ use pinocchio::{
account_info::AccountInfo, default_panic_handler, no_allocator, program_entrypoint,
program_error::ProgramError, pubkey::Pubkey, ProgramResult,
};
use spl_token_interface::instruction::TokenInstruction;

use crate::processor::*;

Expand All @@ -14,218 +13,245 @@ default_panic_handler!();

/// Process an instruction.
///
/// In the first stage, the entrypoint checks the discriminator of the instruction data
/// to determine whether the instruction is a "batch" instruction or a "regular" instruction.
/// This avoids nesting of "batch" instructions, since it is not sound to have a "batch"
/// instruction inside another "batch" instruction.
#[inline(always)]
pub fn process_instruction(
_program_id: &Pubkey,
accounts: &[AccountInfo],
instruction_data: &[u8],
) -> ProgramResult {
let [discriminator, remaining @ ..] = instruction_data else {
return Err(ProgramError::InvalidInstructionData);
};

if *discriminator == 255 {
// 255 - Batch
#[cfg(feature = "logging")]
pinocchio::msg!("Instruction: Batch");

return process_batch(accounts, remaining);
}

inner_process_instruction(accounts, instruction_data)
}

/// Process a "regular" instruction.
///
/// The processor of the token program is divided into two parts to reduce the overhead
/// of having a large `match` statement. The first part of the processor handles the
/// most common instructions, while the second part handles the remaining instructions.
///
/// The rationale is to reduce the overhead of making multiple comparisons for popular
/// instructions.
///
/// Instructions on the first part of the processor:
/// Instructions on the first part of the inner processor:
///
/// - `0`: `InitializeMint`
/// - `3`: `Transfer`
/// - `7`: `MintTo`
/// - `9`: `CloseAccount`
/// - `0`: `InitializeMint`
/// - `1`: `InitializeAccount`
/// - `3`: `Transfer`
/// - `7`: `MintTo`
/// - `9`: `CloseAccount`
/// - `16`: `InitializeAccount2`
Comment on lines +52 to +57
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to make sure, were these updated for better performance as a result of this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the idea was to keep all the Initialize*, MintTo, CloseAccount and Transfer on the "faster" path. We can tweak the list if you thing there are other instructions that are more critical.

/// - `18`: `InitializeAccount3`
/// - `20`: `InitializeMint2`
#[inline(always)]
pub fn process_instruction(
_program_id: &Pubkey,
pub(crate) fn inner_process_instruction(
accounts: &[AccountInfo],
instruction_data: &[u8],
) -> ProgramResult {
let (discriminator, instruction_data) = instruction_data
.split_first()
.ok_or(ProgramError::InvalidInstructionData)?;
let instruction = TokenInstruction::try_from(*discriminator)?;
Comment on lines -37 to -40
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was the usage of TokenInstruction supposed to be reverted?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, using the TokenInstruction enum increases 20 CUs at least once I added the Batch variant. I could not find a way to make it efficient.

let [discriminator, instruction_data @ ..] = instruction_data else {
return Err(ProgramError::InvalidInstructionData);
};

match instruction {
match *discriminator {
// 0 - InitializeMint
TokenInstruction::InitializeMint => {
0 => {
#[cfg(feature = "logging")]
pinocchio::msg!("Instruction: InitializeMint");

process_initialize_mint(accounts, instruction_data)
}
// 1 - InitializeAccount
1 => {
#[cfg(feature = "logging")]
pinocchio::msg!("Instruction: InitializeAccount");

process_initialize_account(accounts)
}
// 3 - Transfer
TokenInstruction::Transfer => {
3 => {
#[cfg(feature = "logging")]
pinocchio::msg!("Instruction: Transfer");

process_transfer(accounts, instruction_data)
}
// 7 - MintTo
TokenInstruction::MintTo => {
7 => {
#[cfg(feature = "logging")]
pinocchio::msg!("Instruction: MintTo");

process_mint_to(accounts, instruction_data)
}
// 9 - CloseAccount
TokenInstruction::CloseAccount => {
9 => {
#[cfg(feature = "logging")]
pinocchio::msg!("Instruction: CloseAccount");

process_close_account(accounts)
}
// 16 - InitializeAccount2
16 => {
#[cfg(feature = "logging")]
pinocchio::msg!("Instruction: InitializeAccount2");

process_initialize_account2(accounts, instruction_data)
}
// 18 - InitializeAccount3
TokenInstruction::InitializeAccount3 => {
18 => {
#[cfg(feature = "logging")]
pinocchio::msg!("Instruction: InitializeAccount3");

process_initialize_account3(accounts, instruction_data)
}
// 20 - InitializeMint2
TokenInstruction::InitializeMint2 => {
20 => {
#[cfg(feature = "logging")]
pinocchio::msg!("Instruction: InitializeMint2");

process_initialize_mint2(accounts, instruction_data)
}
_ => process_remaining_instruction(accounts, instruction_data, instruction),
d => inner_process_remaining_instruction(accounts, instruction_data, d),
}
}

/// Process the remaining instructions.
/// Process a remaining "regular" instruction.
///
/// This function is called by the `process_instruction` function if the discriminator
/// This function is called by the [`inner_process_instruction`] function if the discriminator
/// does not match any of the common instructions. This function is used to reduce the
/// overhead of having a large `match` statement in the `process_instruction` function.
fn process_remaining_instruction(
/// overhead of having a large `match` statement in the [`inner_process_instruction`] function.
fn inner_process_remaining_instruction(
accounts: &[AccountInfo],
instruction_data: &[u8],
instruction: TokenInstruction,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here: was this supposed to be reverted?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it is more efficient to use the u8 value directly.

discriminator: u8,
) -> ProgramResult {
match instruction {
// 1 - InitializeAccount
TokenInstruction::InitializeAccount => {
#[cfg(feature = "logging")]
pinocchio::msg!("Instruction: InitializeAccount");

process_initialize_account(accounts)
}
match discriminator {
// 2 - InitializeMultisig
TokenInstruction::InitializeMultisig => {
2 => {
#[cfg(feature = "logging")]
pinocchio::msg!("Instruction: InitializeMultisig");

process_initialize_multisig(accounts, instruction_data)
}
// 4 - Approve
TokenInstruction::Approve => {
4 => {
#[cfg(feature = "logging")]
pinocchio::msg!("Instruction: Approve");

process_approve(accounts, instruction_data)
}
// 5 - Revoke
TokenInstruction::Revoke => {
5 => {
#[cfg(feature = "logging")]
pinocchio::msg!("Instruction: Revoke");

process_revoke(accounts, instruction_data)
}
// 6 - SetAuthority
TokenInstruction::SetAuthority => {
6 => {
#[cfg(feature = "logging")]
pinocchio::msg!("Instruction: SetAuthority");

process_set_authority(accounts, instruction_data)
}
// 8 - Burn
TokenInstruction::Burn => {
8 => {
#[cfg(feature = "logging")]
pinocchio::msg!("Instruction: Burn");

process_burn(accounts, instruction_data)
}
// 10 - FreezeAccount
TokenInstruction::FreezeAccount => {
10 => {
#[cfg(feature = "logging")]
pinocchio::msg!("Instruction: FreezeAccount");

process_freeze_account(accounts)
}
// 11 - ThawAccount
TokenInstruction::ThawAccount => {
11 => {
#[cfg(feature = "logging")]
pinocchio::msg!("Instruction: ThawAccount");

process_thaw_account(accounts)
}
// 12 - TransferChecked
TokenInstruction::TransferChecked => {
12 => {
#[cfg(feature = "logging")]
pinocchio::msg!("Instruction: TransferChecked");

process_transfer_checked(accounts, instruction_data)
}
// 13 - ApproveChecked
TokenInstruction::ApproveChecked => {
13 => {
#[cfg(feature = "logging")]
pinocchio::msg!("Instruction: ApproveChecked");

process_approve_checked(accounts, instruction_data)
}
// 14 - MintToChecked
TokenInstruction::MintToChecked => {
14 => {
#[cfg(feature = "logging")]
pinocchio::msg!("Instruction: MintToChecked");

process_mint_to_checked(accounts, instruction_data)
}
// 15 - BurnChecked
TokenInstruction::BurnChecked => {
15 => {
#[cfg(feature = "logging")]
pinocchio::msg!("Instruction: BurnChecked");

process_burn_checked(accounts, instruction_data)
}
// 16 - InitializeAccount2
TokenInstruction::InitializeAccount2 => {
#[cfg(feature = "logging")]
pinocchio::msg!("Instruction: InitializeAccount2");

process_initialize_account2(accounts, instruction_data)
}
// 17 - SyncNative
TokenInstruction::SyncNative => {
17 => {
#[cfg(feature = "logging")]
pinocchio::msg!("Instruction: SyncNative");

process_sync_native(accounts)
}
// 19 - InitializeMultisig2
TokenInstruction::InitializeMultisig2 => {
19 => {
#[cfg(feature = "logging")]
pinocchio::msg!("Instruction: InitializeMultisig2");

process_initialize_multisig2(accounts, instruction_data)
}
// 21 - GetAccountDataSize
TokenInstruction::GetAccountDataSize => {
21 => {
#[cfg(feature = "logging")]
pinocchio::msg!("Instruction: GetAccountDataSize");

process_get_account_data_size(accounts)
}
// 22 - InitializeImmutableOwner
TokenInstruction::InitializeImmutableOwner => {
22 => {
#[cfg(feature = "logging")]
pinocchio::msg!("Instruction: InitializeImmutableOwner");

process_initialize_immutable_owner(accounts)
}
// 23 - AmountToUiAmount
TokenInstruction::AmountToUiAmount => {
23 => {
#[cfg(feature = "logging")]
pinocchio::msg!("Instruction: AmountToUiAmount");

process_amount_to_ui_amount(accounts, instruction_data)
}
// 24 - UiAmountToAmount
TokenInstruction::UiAmountToAmount => {
24 => {
#[cfg(feature = "logging")]
pinocchio::msg!("Instruction: UiAmountToAmount");

Expand Down
Loading