feat request: add a trait to serialize instruction data

[publicqi]

right now all instructions are wrapped behind structs, and instruction data are mostly inside implementation inside invoke_signed.

we can add a trait to export/serialize these instructions.

there're two benefits for this:

1. we can easily test against other libraries (e.g. solana-program, anchor, etc.) to see if our pinocchio instruction has the same results
2. developers may have more control over the instruction data (not common but may be useful for some hacky stuff)

a draft interface:

pub trait IxSerialize {
    fn to_ix_data(&self) -> Box<[u8]>;
}

// For e.g. construction of transaction
impl IxSerialize for Transfer<'_> {
    fn to_ix_data(&self) -> Box<[u8]> {
        self.to_ix_data()
    }
}

and an example impl of token::transfer

impl Transfer<'_> {
    #[inline(always)]
    pub fn invoke(&self) -> ProgramResult {
        self.invoke_signed(&[])
    }

    pub fn invoke_signed(&self, signers: &[Signer]) -> ProgramResult {
        // account metadata
        let account_metas: [AccountMeta; 3] = [
            AccountMeta::writable(self.from.key()),
            AccountMeta::writable(self.to.key()),
            AccountMeta::readonly_signer(self.authority.key()),
        ];

        let instruction_data = self.to_ix_data();

        let instruction = Instruction {
            program_id: &crate::ID,
            accounts: &account_metas,
            data: &instruction_data,
        };

        invoke_signed(&instruction, &[self.from, self.to, self.authority], signers)
    }

    fn to_ix_data(&self) -> Box<[u8]> {
        // Instruction data layout:
        // -  [0]: instruction discriminator (1 byte, u8)
        // -  [1..9]: amount (8 bytes, u64)
        let mut instruction_data = Box::new_uninit_slice(9);

        // Set discriminator as u8 at offset [0]
        write_bytes(&mut instruction_data, &[3]);
        // Set amount as u64 at offset [1..9]
        write_bytes(&mut instruction_data[1..9], &self.amount.to_le_bytes());

        unsafe { instruction_data.assume_init() }
    }
}

impl<'a> IxSerialize for Transfer<'a> {
    fn to_ix_data(&self) -> Box<[u8]> {
        self.to_ix_data()
    }
}

[publicqi]

and this could have caught #107

[publicqi]

and i just realized for the example i wrote above (token::transfer) the discriminator is also wrong (4 bytes). we need to test against the original impl!

[febo]

I have been thinking on how to decouple the creation of instruction data and account metas on the clients, so we can add support to both invoke and invoke_unchecked without having to duplicate the code.

I think we can avoid using Box since it implies having an allocator. We can make a more flexible trait:

pub trait InstructionData: Deref<Target = [u8]> {}

Then where the "bytes" come from is an implementation detail.

[febo]

and i just realized for the example i wrote above (token::transfer) the discriminator is also wrong (4 bytes). we need to test against the original impl!

Not sure what you mean by 4 bytes.

[publicqi]

and i just realized for the example i wrote above (token::transfer) the discriminator is also wrong (4 bytes). we need to test against the original impl!

Not sure what you mean by 4 bytes.

I mean the current impl of token::tranfer only used one byte for discriminator but it should be 4 btyes.

[publicqi]

I have been thinking on how to decouple the creation of instruction data and account metas on the clients, so we can add support to both invoke and invoke_unchecked without having to duplicate the code.

I think we can avoid using Box since it implies having an allocator. We can make a more flexible trait:

pub trait InstructionData: Deref<Target = [u8]> {}
Then where the "bytes" come from is an implementation detail.

this look good

[febo]

and i just realized for the example i wrote above (token::transfer) the discriminator is also wrong (4 bytes). we need to test against the original impl!

Not sure what you mean by 4 bytes.

I mean the current impl of token::tranfer only used one byte for discriminator but it should be 4 btyes.

That one is just 1 byte since it is a BPF program. The native programs have 4 bytes discriminators.

[publicqi]

That one is just 1 byte since it is a BPF program. The native programs have 4 bytes discriminators.

ah right

[publicqi]

@febo hey any plan on this one? will this conflict with any of the upcoming updates? if not i can take this one.

pub trait InstructionData: Deref<Target = [u8]> {}

this is good enough right?

[febo]

@febo hey any plan on this one? will this conflict with any of the upcoming updates? if not i can take this one.

pub trait InstructionData: Deref<Target = [u8]> {}

this is good enough right?

I think it looks good. Happy for you to take this one – the changes on the main sdk should not conflict with this.

[Chazzzzzzz]

i m taking this one up.

it seems after using Deref we will need to implement Deref for all structs. May not be a good option there

[Chazzzzzzz]

#131

Heyyyy, implemented this for token program only.

can you help take a look?

@febo