address rabbit's feedback

Author: snawaz

src/args/commit_state.rs

@@ -9,7 +9,6 @@ use crate::args::{ArgsWithBuffer, Boolean};
 #[repr(C)]
 #[derive(Copy, Clone, Pod, Zeroable, Default)]
 pub struct CommitBumps {
-    /// bumps of the PDA accounts to be validated by ix
     pub delegation_record: u8,
     pub delegation_metadata: u8,
     pub validator_fees_vault: u8,
@@ -28,7 +27,7 @@ pub struct CommitFinalizeArgs {
     /// whether the account can be undelegated after the commit completes
     pub allow_undelegation: Boolean,
 
-    /// whether the account can be undelegated after the commit completes
+    /// whether the data (in the ixdata or in the data account) is diff or full state.
     pub data_is_diff: Boolean,
 
     /// bumps of the PDA accounts to be validated by the ix

src/args/types.rs

@@ -3,7 +3,7 @@ use std::ops::Deref;
 use bytemuck::{Pod, Zeroable};
 use pinocchio::program_error::ProgramError;
 
-use crate::pod_view::PodView;
+use crate::{pod_view::PodView, require_ge};
 
 ///
 /// Boolean
@@ -31,24 +31,25 @@ impl From<bool> for Boolean {
 ///
 /// ArgsWithBuffer
 ///
-pub struct ArgsWithBuffer<'a, H> {
-    header: &'a H,
+pub struct ArgsWithBuffer<'a, Header> {
+    header: &'a Header,
     pub buffer: &'a [u8],
 }
 
-impl<'a, H: PodView> ArgsWithBuffer<'a, H> {
+impl<'a, Header: PodView> ArgsWithBuffer<'a, Header> {
     pub fn from_bytes(input: &'a [u8]) -> Result<Self, ProgramError> {
-        let header_size = size_of::<H>();
-
-        if input.len() < header_size {
-            return Err(ProgramError::InvalidInstructionData);
-        }
-
-        let (header_bytes, buffer) = input.split_at(header_size);
-
-        let header = H::try_view_from(header_bytes)?;
-
-        Ok(Self { header, buffer })
+        require_ge!(
+            input.len(),
+            Header::SPACE,
+            ProgramError::InvalidInstructionData
+        );
+
+        let (header_bytes, buffer) = input.split_at(Header::SPACE);
+
+        Ok(Self {
+            header: Header::try_view_from(header_bytes)?,
+            buffer,
+        })
     }
 }
 

src/error.rs

@@ -154,6 +154,7 @@ impl From<DlpError> for pinocchio::program_error::ProgramError {
     }
 }
 
+#[cfg(not(feature = "sdk"))]
 impl pinocchio::program_error::ToStr for DlpError {
     fn to_str<E>(&self) -> &'static str
     where

src/instruction_builder/commit_finalize.rs

@@ -85,7 +85,7 @@ pub fn commit_finalize(
 }
 
 ///
-/// Returns accounts-data-size budget for commit_state instruction.
+/// Returns accounts-data-size budget for commit_finalize instruction.
 ///
 /// This value can be used with ComputeBudgetInstruction::SetLoadedAccountsDataSizeLimit
 ///

src/pod_view.rs

@@ -137,20 +137,8 @@ impl<T: bytemuck::Pod> PodView for T {
     }
 
     #[cfg(feature = "unit_test_config")]
-    fn try_from_unaligned(unaligned_buffer: &[u8]) -> Result<Self, ProgramError> {
-        if unaligned_buffer.len() != Self::SPACE {
-            return Err(ProgramError::InvalidArgument);
-        }
-        let mut oversized_memory = vec![0; Self::SPACE + Self::ALIGN - 1];
-
-        let aligned_slice = {
-            let base = oversized_memory.as_mut_ptr() as usize;
-            let offset = (base + Self::ALIGN - 1) & !(Self::ALIGN - 1) - base;
-            let aligned = &mut oversized_memory[offset..offset + Self::SPACE];
-            aligned.copy_from_slice(unaligned_buffer);
-            aligned
-        };
-
-        Self::try_view_from(aligned_slice).map(|view| *view)
+    fn try_from_unaligned(possibly_unaligned_buffer: &[u8]) -> Result<Self, ProgramError> {
+        bytemuck::try_pod_read_unaligned(possibly_unaligned_buffer)
+            .map_err(|_| ProgramError::InvalidArgument)
     }
 }

src/processor/fast/commit_finalize.rs

@@ -8,37 +8,22 @@ use crate::processor::fast::internal::{
 use crate::processor::fast::NewState;
 use crate::{require_n_accounts, DiffSet};
 
-/// Commit a new state of a delegated PDA
+/// Commit a new state, or a diff, directly to the delegated account. Unlike, CommitState and
+/// CommitDiff variants, this instruction does not write to any temporary account first. In other
+/// words, this instruction commits and finalizes both.
 ///
 /// Accounts:
 ///
 /// 0: `[signer]`   the validator requesting the commit
 /// 1: `[]`         the delegated account
-/// 2: `[writable]` the PDA storing the new state
-/// 3: `[writable]` the PDA storing the commit record
-/// 4: `[]`         the delegation record
-/// 5: `[writable]` the delegation metadata
-/// 6: `[]`         the validator fees vault
-/// 7: `[]`         the program config account
+/// 2: `[]`         the delegation record
+/// 3: `[writable]` the delegation metadata
+/// 4: `[]`         the validator fees vault
+/// 5: `[]`         the program config account
+/// 6: `[]`         system program
 ///
 /// Instruction Data: CommitFinalizeArgsWithBuffer
 ///
-/// Requirements:
-///
-/// - delegation record is initialized
-/// - delegation metadata is initialized
-/// - validator fees vault is initialized
-/// - program config is initialized
-/// - commit state is uninitialized
-/// - commit record is uninitialized
-/// - delegated account holds at least the lamports indicated in the delegation record
-/// - account was not committed at a later slot
-///
-/// Steps:
-/// 1. Check that the pda is delegated
-/// 2. Init a new PDA to store the new state
-/// 3. Copy the new state to the new PDA
-/// 4. Init a new PDA to store the record of the new state commitment
 pub fn process_commit_finalize(
     _program_id: &Pubkey,
     accounts: &[AccountInfo],

src/processor/fast/internal/commit_finalize_internal.rs

@@ -9,8 +9,8 @@ use crate::processor::fast::{to_pinocchio_program_error, NewState};
 use crate::state::{DelegationMetadataFast, DelegationRecord, ProgramConfig};
 use crate::{
     apply_diff_in_place, pda, require, require_eq, require_eq_keys, require_ge,
-    require_initialized_pda, require_initialized_pda_unsafe, require_owned_by,
-    require_program_config, require_program_config_unsafe, require_signer,
+    require_initialized_pda, require_initialized_pda_fast, require_owned_by,
+    require_program_config, require_program_config_fast, require_signer,
 };
 
 /// Arguments for the commit state internal function
@@ -71,7 +71,7 @@ pub(crate) fn process_commit_finalize_internal(
             false
         );
     } else {
-        require_initialized_pda_unsafe!(
+        require_initialized_pda_fast!(
             args.delegation_record_account,
             &[
                 pda::DELEGATION_RECORD_TAG,
@@ -80,11 +80,10 @@ pub(crate) fn process_commit_finalize_internal(
                 &crate::fast::ID,
                 PDA_MARKER
             ],
-            &crate::fast::ID,
             false
         );
 
-        require_initialized_pda_unsafe!(
+        require_initialized_pda_fast!(
             args.delegation_metadata_account,
             &[
                 pda::DELEGATION_METADATA_TAG,
@@ -93,11 +92,10 @@ pub(crate) fn process_commit_finalize_internal(
                 &crate::fast::ID,
                 PDA_MARKER
             ],
-            &crate::fast::ID,
             true
         );
 
-        require_initialized_pda_unsafe!(
+        require_initialized_pda_fast!(
             args.validator_fees_vault,
             &[
                 pda::VALIDATOR_FEES_VAULT_TAG,
@@ -106,7 +104,6 @@ pub(crate) fn process_commit_finalize_internal(
                 &crate::fast::ID,
                 PDA_MARKER
             ],
-            &crate::fast::ID,
             false
         );
     }
@@ -125,14 +122,8 @@ pub(crate) fn process_commit_finalize_internal(
         );
     }
 
-    // Load delegation record
     let delegation_record_data = args.delegation_record_account.try_borrow_data()?;
-    let delegation_record = if false {
-        DelegationRecord::try_from_bytes_with_discriminator(&delegation_record_data)
-            .map_err(to_pinocchio_program_error)?
-    } else {
-        DelegationRecord::try_view_from(&delegation_record_data.as_ref()[8..])?
-    };
+    let delegation_record = DelegationRecord::try_view_from(&delegation_record_data.as_ref()[8..])?;
 
     // Check that the authority is allowed to commit
     require_eq_keys!(
@@ -168,7 +159,7 @@ pub(crate) fn process_commit_finalize_internal(
                 false
             )
         } else {
-            require_program_config_unsafe!(
+            require_program_config_fast!(
                 args.program_config_account,
                 delegation_record.owner.as_array(),
                 args.bumps.program_config,

src/processor/fast/utils/requires.rs

@@ -264,31 +264,28 @@ macro_rules! require_initialized_pda {
 }
 
 #[macro_export]
-macro_rules! require_initialized_pda_unsafe {
-    ($info:expr, $seeds: expr, $program_id: expr, $is_writable: expr) => {{
+macro_rules! require_initialized_pda_fast {
+    ($info:expr, $seeds: expr, $is_writable: expr) => {{
         use solana_sha256_hasher::hashv;
         let pda = hashv($seeds).to_bytes();
         if !pubkey_eq($info.key(), &pda) {
             log!(
-                "require_initialized_pda!({}, {}, {}, {}); pubkey_eq failed",
+                "require_initialized_pda!({}, {}, {}); pubkey_eq failed",
                 stringify!($info),
                 stringify!($seeds),
-                stringify!($program_id),
                 stringify!($is_writable)
             );
             pubkey::log($info.key());
-            pubkey::log($program_id);
             return Err(ProgramError::InvalidSeeds);
         }
 
-        require_owned_by!($info, $program_id);
+        require_owned_by!($info, &$crate::fast::ID);
 
         if $is_writable && !$info.is_writable() {
             log!(
-                "require_initialized_pda!({}, {}, {}, {}); is_writable expectation failed",
+                "require_initialized_pda!({}, {}, {}); is_writable expectation failed",
                 stringify!($info),
                 stringify!($seeds),
-                stringify!($program_id),
                 stringify!($is_writable)
             );
             pubkey::log($info.key());
@@ -341,29 +338,26 @@ macro_rules! require_pda {
 }
 
 #[macro_export]
-macro_rules! require_pda_unsafe {
-    ($info:expr, $seeds: expr, $program_id: expr, $is_writable: expr) => {{
+macro_rules! require_pda_fast {
+    ($info:expr, $seeds: expr, $is_writable: expr) => {{
         use solana_sha256_hasher::hashv;
         let pda = hashv($seeds).to_bytes();
         if !pubkey_eq($info.key(), &pda) {
             log!(
-                "require_pda!({}, {}, {}, {}); pubkey_eq failed",
+                "require_pda!({}, {}, {}); pubkey_eq failed",
                 stringify!($info),
                 stringify!($seeds),
-                stringify!($program_id),
                 stringify!($is_writable)
             );
             pubkey::log($info.key());
-            pubkey::log($program_id);
             return Err(ProgramError::InvalidSeeds);
         }
 
         if $is_writable && !$info.is_writable() {
             log!(
-                "require_pda!({}, {}, {}, {}); is_writable expectation failed",
+                "require_pda!({}, {}, {}); is_writable expectation failed",
                 stringify!($info),
                 stringify!($seeds),
-                stringify!($program_id),
                 stringify!($is_writable)
             );
             pubkey::log($info.key());
@@ -392,9 +386,9 @@ macro_rules! require_program_config {
 }
 
 #[macro_export]
-macro_rules! require_program_config_unsafe {
+macro_rules! require_program_config_fast {
     ($program_config: expr, $program: expr, $bump: expr, $is_writable: expr) => {{
-        $crate::require_pda_unsafe!(
+        $crate::require_pda_fast!(
             $program_config,
             &[
                 pda::PROGRAM_CONFIG_TAG,
@@ -403,7 +397,6 @@ macro_rules! require_program_config_unsafe {
                 &$crate::fast::ID,
                 PDA_MARKER
             ],
-            &$crate::fast::ID,
             $is_writable
         );
         !pubkey_eq($program_config.owner(), &pinocchio_system::ID)

src/state/delegation_metadata.rs

@@ -1,6 +1,9 @@
 use std::ptr;
 
-use crate::{impl_to_bytes_with_discriminator_borsh, impl_try_from_bytes_with_discriminator_borsh};
+use crate::{
+    impl_to_bytes_with_discriminator_borsh, impl_try_from_bytes_with_discriminator_borsh,
+    require_ge,
+};
 use borsh::{BorshDeserialize, BorshSerialize};
 use pinocchio::{
     account_info::{AccountInfo, RefMut},
@@ -32,6 +35,15 @@ pub struct DelegationMetadataFast<'a> {
 
 impl<'a> DelegationMetadataFast<'a> {
     pub fn from_account(account: &'a AccountInfo) -> Result<Self, ProgramError> {
+        require_ge!(
+            account.data_len(),
+            8  // last_update_nonce
+            + 1 // is_undelegatable
+            + 32 // rent_payer
+            + 4, // seeds (at least 4)
+            ProgramError::InvalidAccountData
+        );
+
         Ok(Self {
             data: account.try_borrow_mut_data()?,
         })