InstructionDecoder Pipeline Incorrectly Matches Programs With Similar Instruction Discriminators

[bonedaddy]

Overview

When using the carbon cli to generate a decoder crate, the implemented carbon_core::instruction::InstructionDecoder trait incorrectly matches program instructions if that program shares instruction discriminators with other programs, as long as instruction data can be deserialized into the mismatched instruction, and the mismatched instruction takes places within the same transaction.

This is because the the implemented InstructionDecoder uses the macro carbon_core::try_decode_instructions! which doesn't validate the program id, instead simply checking that the instruction data can be deserialized into the corresponding type

carbon/crates/macros/src/try_decode_ixs.rs

Line 81 in 5e91c9f

if let Some(decoded_instruction) = <$ty>::deserialize($instruction.data.as_slice()) {

For example the spl-token program uses the instruction discriminator 0 to identify the InitializeMint instruction.

If my program also uses the instruction discriminator 0, and I submit a transaction for this instruction which also triggers the InitializeMint instruction from the spl-token program, and my program's instruction data can be deserialized into the InitializeMint instruction, then the InstructionDecoder trait will decode both instructions as if they were part of my program.

Impacted Version

This has occured as of the 0.8.1 but I suspect it will still be an issue with the latest release (0.9.0)

Temporary Work Around

I've been using the following work around that intercepts the decode_instruction invocation first checking that the program id is my program.

use program_decoder::instructions::ProgramInstruction;

pub struct TxDecoder;

impl<'a> carbon_core::instruction::InstructionDecoder<'a> for TxDecoder {
    type InstructionType = ProgramInstruction;
    fn decode_instruction(
        &self,
        instruction: &'a solana_instruction::Instruction,
    ) -> Option<carbon_core::instruction::DecodedInstruction<Self::InstructionType>> {
        if !instruction.program_id.eq(&sdk::PROGRAM_ID) {
            return None;
        }
        program_decoder::ProgramDecoder.decode_instruction(instruction)
    }
}

pub struct FooInstructionDecoder {
    updates: tokio::sync::mpsc::Sender<CarbonDecoderUpdate>,
}

#[async_trait::async_trait]
impl Processor for FooInstructionDecoder {
    type InputType = types::CarbonDecoderUpdate;

    async fn process(
        &mut self,
        data: Self::InputType,
        _metrics: Arc<MetricsCollection>,
    ) -> CarbonResult<()> {
        log::info!("found tx {}", data.0.transaction_metadata.signature);

        // skip updates for which the transaction failed
        if data.0.transaction_metadata.meta.status.is_err() {
            log::debug!("skipping failed transaction");
            return Ok(());
        }
        if let Err(err) = self.updates.send(data).await {
            return Err(carbon_core::error::Error::FailedToConsumeDatasource(
                format!("failed to send decoder update {err:#?}"),
            ));
        }
        Ok(())
    }
}

Which can then be used within an InstructionDecoder pipeline like so

            .instruction(
                TxDecoder,
                FooInstructionDecoder {
                    updates: decoder_updates_tx,
                },
            )

Solution

IMO the InstructionDecoder should validate the program id the instruction is for before attempting to decode the instruction.

[KellianDev]

Hey, I'm reworking the decoder generation with a Codama renderer right now (see #386). This step will happen for the new decoders that we publish once this is merged - and we will also update all decoders.