Bug: Instruction Attribute cannot be shared between multiple composite structs

[nabeel99]

Problem : the ix attribute under

#[derive(Accounts)
#[instruction(..ix _var)
pub struct ParentAccountIx<'info> {
pub struct : ChildAccount<'info>,

}
#[derive(Accounts)
#[instruction(..ix _var)
pub struct ChildAccount<'info> {
pub struct : Someaccount<'info>,

}

fails if the parent derive accounts is composed of composite deriveAccounts, the reason is that while deserialization it takes a &mut refrence to the ix buffer and after deserializing it for the first time in ParentAccountIx pointer is updated so subsequent deserialization in other composite structs such as ChildAccount fails as the resulting buffer is updated to its past the fields the second ix requires and deserialization fails with error 102, this is the borsh deserilization trait which documents the updating of the buffer. Note : I am unsure if this is a bug or just need to a limitation of composite structs that needs to be documented.

pub trait BorshDeserialize: Sized {
    /// Deserializes this instance from a given slice of bytes.
    /// Updates the buffer to point at the remaining bytes.
    fn deserialize(buf: &mut &[u8]) -> Result<Self> {
        Self::deserialize_reader(&mut *buf)
    }

[nabeel99]

is the solution to this is to just add documentation for this ? or we are still on the border of classifying it as a bug for a feature ? @acheroncrypto

[nabeel99]

kinda confused how this is handled when the instruction takes in a parameter, it deserializes twice once in the instruction handler and secondly in the try_accounts handler for derive(accounts) struct, deserialization in the function handler :

 #[inline(never)]
        pub fn stake<'info>(
            __program_id: &Pubkey,
            __accounts: &'info [AccountInfo<'info>],
            __ix_data: &[u8],
        ) -> anchor_lang::Result<()> {
            ::solana_program::log::sol_log("Instruction: Stake");
            let ix = instruction::Stake::deserialize(&mut &__ix_data[..])
                .map_err(|_| {
                    anchor_lang::error::ErrorCode::InstructionDidNotDeserialize
                })?;
                    
                 //   and in the account handler try_accounts impl  which callled in the same function
                 let mut __accounts = Stake::try_accounts(
                __program_id,
                &mut __remaining_accounts,
                __ix_data,
                &mut __bumps,
                &mut __reallocs,
            )?;

**Deserialization in the try_accounts**

           

                      #[inline(never)]
            fn try_accounts(
                __program_id: &anchor_lang::solana_program::pubkey::Pubkey,
                __accounts: &mut &'info [anchor_lang::solana_program::account_info::AccountInfo<
                    'info,
                >],
                __ix_data: &[u8],
                __bumps: &mut StakeBumps,
                __reallocs: &mut std::collections::BTreeSet<
                    anchor_lang::solana_program::pubkey::Pubkey,
                >,
            ) -> anchor_lang::Result<Self> {
                let mut __ix_data = __ix_data;
                struct __Args {
                    stake_amount: u64,
                }
                impl borsh::ser::BorshSerialize for __Args
                where
                    u64: borsh::ser::BorshSerialize,
                {
                    fn serialize<W: borsh::maybestd::io::Write>(
                        &self,
                        writer: &mut W,
                    ) -> ::core::result::Result<(), borsh::maybestd::io::Error> {
                        borsh::BorshSerialize::serialize(&self.stake_amount, writer)?;
                        Ok(())
                    }
                }
                impl borsh::de::BorshDeserialize for __Args
                where
                    u64: borsh::BorshDeserialize,
                {
                    fn deserialize_reader<R: borsh::maybestd::io::Read>(
                        reader: &mut R,
                    ) -> ::core::result::Result<Self, borsh::maybestd::io::Error> {
                        Ok(Self {
                            stake_amount: borsh::BorshDeserialize::deserialize_reader(
                                reader,
                            )?,
                        })
                    }
                }
                let __Args { stake_amount } = __Args::deserialize(&mut __ix_data)
                    .map_err(|_| {
                        anchor_lang::error::ErrorCode::InstructionDidNotDeserialize
                    })?;
                   

@acheroncrypto not sure if the way deserialization occurs is the buffer gets updated with the same logic shdnt the deserialization fail in the try_accounts_handler since the same buffer is passed around, but if this works it should be able to work in composite structs as well ? but i cant figure out why it works in the try_accounts if the impl is for the ix data buffer to get updated after deserialization.

[nabeel99]

@acheroncrypto,

After diving into the deserialization process in the Anchor framework which just uses borsh under the hood and reading the impl of read_exact, it seems the issue lies in shadowing the source buffer into a new mutable variable with let mut __ix_data = __ix_data;. This approach advances the buffer cursor when passed to __Args::deserialize(&mut __ix_data), causing deserialization failures in composite structs that expect the same arguments.

Proposed Solution

Instead of shadowing the source buffer, we can use an immutable reference to the source buffer and pass a mutable reference to this immutable reference during deserialization. This way, the original buffer remains unchanged and reusable for further deserialization in the composite structs try_accounts impl.

Implementation

Current approach:

let mut __ix_data = __ix_data;
__Args::deserialize(&mut __ix_data)?;

Proposed approach:

__Args::deserialize(&mut &__ix_data[..])?;

[nabeel99]

as per my p.o.v this seems a bug / wrong implementation, rather than a feature.

[acheroncrypto]

Hey, sorry for taking long to reply to this.

Instead of shadowing the source buffer, we can use an immutable reference to the source buffer and pass a mutable reference to this immutable reference during deserialization. This way, the original buffer remains unchanged and reusable for further deserialization in the composite structs try_accounts impl.

Implementation

Current approach:

let mut __ix_data = __ix_data;
__Args::deserialize(&mut __ix_data)?;

Proposed approach:

__Args::deserialize(&mut &__ix_data[..])?;

At first glance, this makes sense, and we also do something similar in instruction handlers:

https://github.com/coral-xyz/anchor/blob/56017029b40371d10d6d65cb1308aef2c571dbb8/lang/syn/src/codegen/program/handlers.rs#L126

[nabeel99]

yes, i dont see any downside to this, will impl a quick pr, ty for confirming ser.