[RFC] Solana Transaction Structures

[kespinola]

Background

In Solana, the structure of transaction object ReplicaTransactionInfoVersions is optimized for minimal size. This optimization requires clients to deserialize the hierarchy of instruction executions by referencing indices and counters, such as index references to a shared list of account keys.

Issue

The current layout of instructions fails to accurately reflect invocations from nested (or inner) instructions due to its omission of the stack_height attribute that is now available. This oversight leads to DAS incorrectly categorizing instructions.

Suggested Improvement

We propose that, prior to the application of program transformers, Solana instructions be denormalized. This process should restructure the instructions to more precisely depict the parent-child relationships among them. The following Rust structs provide a conceptual framework for the suggested changes:

pub struct Instruction {
    pub program_id: Pubkey,
    pub data: Vec<u8>,
    pub instructions: Vec<Instruction>,
    pub accounts: Vec<Pubkey>,
}

pub struct Transaction {
    pub slot: Slot,
    pub block_time: Option<UnixTimestamp>,
    pub instructions: Vec<Instruction>,
    pub signatures: Vec<Signature>
}

For further discussion, please refer to the following pull request comment: GitHub Discussion.

[kespinola]

Excerpt from slack conversation from Pedro at Helius. The below is from them:

pub struct TritonTransactionInfo {
    accounts: Vec<Pubkey>,
    message_instructions: Vec<CompiledInstruction>,
    meta_inner_instructions: Vec<InnerInstructions>,
}

#[derive(Debug, Clone, PartialEq, Eq)]
pub struct InstructionGroup {
    outer_instruction: Instruction,
    inner_instructions: Vec<Instruction>,
}

#[derive(Debug, Clone, PartialEq, Eq)]
pub struct Instruction {
    data: Vec<u8>,
    program_id: Pubkey,
    accounts: Vec<Pubkey>,
}

fn parse_transaction_groups(transaction_info: TritonTransactionInfo) -> Vec<InstructionGroup> {
    let mut instruction_groups = Vec::new();
    for instruction in transaction_info.message_instructions {
        let program_id = transaction_info.accounts[instruction.program_id_index as usize];
        let accounts = instruction.accounts;
        let data = instruction.data;
        instruction_groups.push(InstructionGroup {
            outer_instruction: Instruction {
                data,
                program_id,
                accounts: accounts
                    .iter()
                    .map(|index| transaction_info.accounts[*index as usize])
                    .collect(),
            },
            inner_instructions: Vec::new(),
        });
    }

    for inner_instructions in transaction_info.meta_inner_instructions.iter() {
        for inner_instruction in inner_instructions.instructions {
            let program_id =
                transaction_info.accounts[inner_instruction.instruction.program_id_index as usize];
            let accounts = inner_instruction
                .instruction
                .accounts
                .iter()
                .map(|index| transaction_info.accounts[*index as usize]);
            let data = inner_instruction.instruction.data;
            instruction_groups[inner_instructions.index as usize]
                .inner_instructions
                .push(Instruction {
                    data,
                    program_id,
                    accounts: inner_instruction.instruction.accounts.iter()
                        .map(|index| transaction_info.accounts[*index as usize])
                        .collect(),
                });
        }
    }
    instruction_groups
}

I am more opinionated about the structs that anything. The AccountInfo looks good me but can we simply the message_instructions and the meta_inner_instructions

#[derive(Debug, Clone, PartialEq, Eq)]
pub struct AccountInfo {
    pub slot: u64,
    pub pubkey: Pubkey,
    pub owner: Pubkey,
    pub data: Vec<u8>,
}

#[derive(Debug, Clone, PartialEq, Eq)]
pub struct TransactionInfo {
    pub slot: u64,
    pub signature: Signature,
    pub account_keys: Vec<Pubkey>,
    pub message_instructions: Vec<CompiledInstruction>,
    pub meta_inner_instructions: Vec<InnerInstructions>,
}
Can we change TransactionInfo to the following:
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct InstructionGroup {
    outer_instruction: Instruction
    inner_instructions: Vec<Instruction>
}

#[derive(Debug, Clone, PartialEq, Eq)]
pub struct Instruction {
    data: Vec<8>,
    program_id: Pubkey,
    accounts: Vec<Pubkey>
}

#[derive(Debug, Clone, PartialEq, Eq)]
pub struct TransactionInfo {
    pub slot: u64,
    pub signature: Signature,
    pub account_keys: Vec<Pubkey>,
    pub instruction_groups: Vec<InstructionGroup>,
}

I think that the structure I am proposing is much easier to read and understand. If I wasn’t an expert in this topic at this point, I wouldn’t know at a glance what the following fields represented.

pub message_instructions: Vec<CompiledInstruction>,
pub meta_inner_instructions: Vec<InnerInstructions>,

Unless you were an expert in this topic it wouldn’t be immediately clear:

What are message_instructions ? I know that these are outer instructions, but it might not be immediately obvious to new developers.

What is the link between message_instructions and meta_inner_instructions?

Why do meta_inner_instructions have a meta prefix? What is meta about them? I know that it’s because it’s additional metadata information that was parsed from the transaction, but this piece of info is not obvious and it’s not really relevant to the indexing interface.

What are CompiledInstruction? How do they differ in structure from InnerInstructions?

[danenbm]

Should we keep write_version in the AccountInfo struct taken from the original serialized types? I have been suspecting we might want to utilize it at some point in the future to help with account-based updates.

Background on what that value is: https://docs.rs/solana-geyser-plugin-interface/1.18.5/solana_geyser_plugin_interface/geyser_plugin_interface/struct.ReplicaAccountInfoV2.html#structfield.write_version

[austbot]

One note. We knew about the stack height issue and this is why when 1.16 was out i put an issue to update to it.

[danenbm]

One note. We knew about the stack height issue and this is why when 1.16 was out i put an issue to update to it.

Yes we didn't forget about that issue you raised. I think the description implies that this RFC came about mainly due to detecting issues. Although there was one indexing issue we debugged on the RPC channel, where someone did a no-op in their own program (where their own program also CPI'd into bubblegum), I would say this RFC mainly came about due to discussion during a wider refactor.

The refactor effort led by @fanatid and @kespinola was for a decoupling of program_transformers to allow for alternate ingest engines. During that discussion @pmantica11 suggested the new layout for better usability (see above), and at this point we suggested resolving the stack height issue as part of it:

metaplex-foundation/blockbuster#38 (comment)