Trailing bytes allowed when deserializing a ProgramInstruction

[Eduard-Voiculescu]

I would like to better understand why trailing bytes are allowed when extending a table lookup with new addresses.

Problem

I found the issue with this transaction. Toggling the raw data shows all the data that is used in the below code snippet.

pub mod tests {
    use super::*;
    use crate::pubkey::Pubkey;
    use crate::address_lookup_table::instruction::ProgramInstruction;
    use std::str::FromStr;

    #[test]
    fn test_deserialize_program_instruction() {
        let data = decode_hex("02000000010000000000000000000000020000000634ff9fb638edb36d9e9e1730ef57bc9286bb905ca2a33e4eca42136e575f810277a6af97339b7ac88d1892c90446f50002309266f62e53c118244982000000").unwrap();
        let decoded: ProgramInstruction = bincode::deserialize( &data).unwrap();
        let item = limited_deserialize::<ProgramInstruction>(&data).unwrap();

        let expected_program_instruction = ProgramInstruction::ExtendLookupTable {
            new_addresses: vec![Pubkey::from_str("11112CCZE56WMyrQx6ToZt1KJPqyn9b3SyBFtW88us").unwrap()],
        };

        assert_eq!(expected_program_instruction, item);
    }
}

pub fn decode_hex(s: &str) -> Result<Vec<u8>, ParseIntError> {
    (0..s.len())
        .step_by(2)
        .map(|i| u8::from_str_radix(&s[i..i + 2], 16))
        .collect()
}

Changing the first 1 to a 2 would enable to extend a table account with 2 new addresses.
I see that it comes down to this line of code: https://github.com/solana-labs/solana/blob/master/sdk/program/src/program_utils.rs#L16. I do not understand the comment mentioning: // to retain the behavior of bincode::deserialize with the new options() method

Proposed Solution

No proposed solution, more a question of understanding why.

[CriesofCarrots]

Frankly, this pattern is common across native and SPL programs; even manual unpacking ignores trailing bytes. But other programs are free to use strict length checks.
I guess the better question to my mind is, is there a reason such a transaction ought to fail?

[Eduard-Voiculescu]

Ok interesting, I didn't know that it was common across native and SPL programs. My guess was more along the lines to avoid having garbage data on SPL programs. In this case, someone could dump any data they would want. All of this persisting in the block with no reason whatsoever.

[CriesofCarrots]

someone could dump any data they would want. All of this persisting in the block with no reason whatsoever.

Even if these instructions were to fail on trailing bytes, they would still be persisted in the ledger. A transaction that fails on any InstructionError is persisted; this is so that fees are collected on transactions that make it this far in processing and use up resources. There are overall caps to the amount of transaction/instruction data that can be included, though. And there is active work on determining the most sensible economics to provide for short- and long-term historical storage of ledger data.

Anyway, since this isn't a big report, I am going to close this issue to help keep our signal strong in the github issue tracker. For discussion about economics, your best bet is probably forums.solana.com, and you can always as questions about on-chain programs and how processing works on Solana Stack Exchange