p-token: Fix error codes (solana-program#52)

* Fix error codes

* Fix formatting

* Add validation on load

* Fix formatting

* Fix typo

* Relax instruction data length check

* Another relax length check

* More relax instruction data length check

* Move account validation

* Fix rent exempt check

* Update pinocchio reference

* Improve coption validation

* Streamline initialize check

* Add unpack helpers

Author: febo

Cargo.lock

@@ -12,8 +12,8 @@ readme = "./README.md"
 crate-type = ["rlib"]
 
 [dependencies]
-pinocchio = "0.8"
-pinocchio-pubkey = "0.2.4"
+pinocchio = "0.8.4"
+pinocchio-pubkey = "0.2"
 
 [dev-dependencies]
 strum = "0.27"

interface/Cargo.toml

@@ -1,6 +1,6 @@
 //! Instruction types.
 
-use pinocchio::program_error::ProgramError;
+use {crate::error::TokenError, pinocchio::program_error::ProgramError};
 
 /// Instructions supported by the token program.
 #[repr(u8)]
@@ -531,7 +531,7 @@ impl TryFrom<u8> for TokenInstruction {
         match value {
             // SAFETY: `value` is guaranteed to be in the range of the enum variants.
             0..=24 | 38 | 255 => Ok(unsafe { core::mem::transmute::<u8, TokenInstruction>(value) }),
-            _ => Err(ProgramError::InvalidInstructionData),
+            _ => Err(TokenError::InvalidInstruction.into()),
         }
     }
 }
@@ -559,7 +559,7 @@ impl TryFrom<u8> for AuthorityType {
         match value {
             // SAFETY: `value` is guaranteed to be in the range of the enum variants.
             0..=3 => Ok(unsafe { core::mem::transmute::<u8, AuthorityType>(value) }),
-            _ => Err(ProgramError::InvalidInstructionData),
+            _ => Err(TokenError::InvalidInstruction.into()),
         }
     }
 }

interface/src/instruction.rs

@@ -1,6 +1,6 @@
 use {
     super::{account_state::AccountState, COption, Initializable, Transmutable},
-    pinocchio::pubkey::Pubkey,
+    pinocchio::{program_error::ProgramError, pubkey::Pubkey},
 };
 
 /// Incinerator address.
@@ -27,7 +27,7 @@ pub struct Account {
     delegate: COption<Pubkey>,
 
     /// The account's state.
-    pub state: AccountState,
+    state: u8,
 
     /// Indicates whether this account represents a native token or not.
     is_native: [u8; 4],
@@ -46,6 +46,16 @@ pub struct Account {
 }
 
 impl Account {
+    #[inline(always)]
+    pub fn set_account_state(&mut self, state: AccountState) {
+        self.state = state as u8;
+    }
+
+    #[inline(always)]
+    pub fn account_state(&self) -> Result<AccountState, ProgramError> {
+        AccountState::try_from(self.state)
+    }
+
     #[inline(always)]
     pub fn set_amount(&mut self, amount: u64) {
         self.amount = amount.to_le_bytes();
@@ -131,8 +141,8 @@ impl Account {
     }
 
     #[inline(always)]
-    pub fn is_frozen(&self) -> bool {
-        self.state == AccountState::Frozen
+    pub fn is_frozen(&self) -> Result<bool, ProgramError> {
+        AccountState::try_from(self.state).map(|state| state == AccountState::Frozen)
     }
 
     #[inline(always)]
@@ -147,7 +157,7 @@ impl Transmutable for Account {
 
 impl Initializable for Account {
     #[inline(always)]
-    fn is_initialized(&self) -> bool {
-        self.state != AccountState::Uninitialized
+    fn is_initialized(&self) -> Result<bool, ProgramError> {
+        AccountState::try_from(self.state).map(|state| state != AccountState::Uninitialized)
     }
 }

interface/src/state/account.rs

@@ -1,3 +1,5 @@
+use pinocchio::program_error::ProgramError;
+
 #[repr(u8)]
 #[derive(Clone, Copy, Debug, PartialEq)]
 pub enum AccountState {
@@ -13,3 +15,16 @@ pub enum AccountState {
     /// this account.
     Frozen,
 }
+
+impl TryFrom<u8> for AccountState {
+    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..=2 => Ok(unsafe { core::mem::transmute::<u8, AccountState>(value) }),
+            _ => Err(ProgramError::InvalidAccountData),
+        }
+    }
+}

interface/src/state/account_state.rs

@@ -1,6 +1,6 @@
 use {
     super::{COption, Initializable, Transmutable},
-    pinocchio::pubkey::Pubkey,
+    pinocchio::{program_error::ProgramError, pubkey::Pubkey},
 };
 
 /// Internal representation of a mint data.
@@ -10,7 +10,7 @@ pub struct Mint {
     /// be provided during mint creation. If no mint authority is present
     /// then the mint has a fixed supply and no further tokens may be
     /// minted.
-    pub mint_authority: COption<Pubkey>,
+    mint_authority: COption<Pubkey>,
 
     /// Total supply of tokens.
     supply: [u8; 8],
@@ -24,7 +24,7 @@ pub struct Mint {
     // Indicates whether the freeze authority is present or not.
     //freeze_authority_option: [u8; 4],
     /// Optional authority to freeze token accounts.
-    pub freeze_authority: COption<Pubkey>,
+    freeze_authority: COption<Pubkey>,
 }
 
 impl Mint {
@@ -91,7 +91,11 @@ impl Transmutable for Mint {
 
 impl Initializable for Mint {
     #[inline(always)]
-    fn is_initialized(&self) -> bool {
-        self.is_initialized == 1
+    fn is_initialized(&self) -> Result<bool, ProgramError> {
+        match self.is_initialized {
+            0 => Ok(false),
+            1 => Ok(true),
+            _ => Err(ProgramError::InvalidAccountData),
+        }
     }
 }

interface/src/state/mint.rs

@@ -23,7 +23,7 @@ pub trait Transmutable {
 /// Trait to represent a type that can be initialized.
 pub trait Initializable {
     /// Return `true` if the object is initialized.
-    fn is_initialized(&self) -> bool;
+    fn is_initialized(&self) -> Result<bool, ProgramError>;
 }
 
 /// Return a reference for an initialized `T` from the given bytes.
@@ -35,7 +35,7 @@ pub trait Initializable {
 pub unsafe fn load<T: Initializable + Transmutable>(bytes: &[u8]) -> Result<&T, ProgramError> {
     load_unchecked(bytes).and_then(|t: &T| {
         // checks if the data is initialized
-        if t.is_initialized() {
+        if t.is_initialized()? {
             Ok(t)
         } else {
             Err(ProgramError::UninitializedAccount)
@@ -69,7 +69,7 @@ pub unsafe fn load_mut<T: Initializable + Transmutable>(
 ) -> Result<&mut T, ProgramError> {
     load_mut_unchecked(bytes).and_then(|t: &mut T| {
         // checks if the data is initialized
-        if t.is_initialized() {
+        if t.is_initialized()? {
             Ok(t)
         } else {
             Err(ProgramError::UninitializedAccount)

interface/src/state/mod.rs

@@ -1,6 +1,6 @@
 use {
     super::{Initializable, Transmutable},
-    pinocchio::pubkey::Pubkey,
+    pinocchio::{program_error::ProgramError, pubkey::Pubkey},
 };
 
 /// Minimum number of multisignature signers (min N)
@@ -18,10 +18,10 @@ pub struct Multisig {
     /// Number of valid signers.
     pub n: u8,
 
-    /// Is `true` if this structure has been initialized
+    /// Is `true` if this structure has been initialized.
     is_initialized: u8,
 
-    /// Signer public keys
+    /// Signer public keys.
     pub signers: [Pubkey; MAX_SIGNERS as usize],
 }
 
@@ -39,13 +39,17 @@ impl Multisig {
 }
 
 impl Transmutable for Multisig {
-    /// The length of the `Mint` account data.
+    /// The length of the `Multisig` account data.
     const LEN: usize = core::mem::size_of::<Multisig>();
 }
 
 impl Initializable for Multisig {
     #[inline(always)]
-    fn is_initialized(&self) -> bool {
-        self.is_initialized == 1
+    fn is_initialized(&self) -> Result<bool, ProgramError> {
+        match self.is_initialized {
+            0 => Ok(false),
+            1 => Ok(true),
+            _ => Err(ProgramError::InvalidAccountData),
+        }
     }
 }

interface/src/state/multisig.rs

@@ -15,7 +15,7 @@ crate-type = ["cdylib"]
 logging = []
 
 [dependencies]
-pinocchio = "0.8"
+pinocchio = "0.8.4"
 pinocchio-log = { version = "0.4", default-features = false }
 spl-token-interface = { version = "^0", path = "../interface" }
 

p-token/Cargo.toml

@@ -36,7 +36,7 @@ pub fn process_instruction(
     instruction_data: &[u8],
 ) -> ProgramResult {
     let [discriminator, remaining @ ..] = instruction_data else {
-        return Err(ProgramError::InvalidInstructionData);
+        return Err(TokenError::InvalidInstruction.into());
     };
 
     let result = if *discriminator == 255 {
@@ -78,7 +78,7 @@ pub(crate) fn inner_process_instruction(
     instruction_data: &[u8],
 ) -> ProgramResult {
     let [discriminator, instruction_data @ ..] = instruction_data else {
-        return Err(ProgramError::InvalidInstructionData);
+        return Err(TokenError::InvalidInstruction.into());
     };
 
     match *discriminator {
@@ -194,7 +194,7 @@ fn inner_process_remaining_instruction(
             #[cfg(feature = "logging")]
             pinocchio::msg!("Instruction: Revoke");
 
-            process_revoke(accounts, instruction_data)
+            process_revoke(accounts)
         }
         // 6 - SetAuthority
         6 => {
@@ -280,6 +280,6 @@ fn inner_process_remaining_instruction(
 
             process_withdraw_excess_lamports(accounts)
         }
-        _ => Err(ProgramError::InvalidInstructionData),
+        _ => Err(TokenError::InvalidInstruction.into()),
     }
 }