[spl-type-length-value]: u64 padding not properly supported

[buffalojoec]

Problem

When working with spl-type-length-value, it's possible to use types that implement bytemuck's Pod and Zeroable trait, as well as spl_discriminator::SplDiscriminate to create TLV entries and utilize the library's tooling.

However, it appears that Pod types using u64 and higher are not supported by the library, since they require proepr alignment.

Observe the following test:

use {
    bytemuck::{Pod, Zeroable},
    spl_discriminator::SplDiscriminate,
};

// Creating four structs, each with different integer types, is completely
// valid by bytemuck's standards.
// We can also add SPL Discriminators, without error, to prepare these structs
// to be used in TLV structures.

#[repr(C)]
#[derive(Clone, Copy, Default, Debug, PartialEq, Pod, Zeroable, SplDiscriminate)]
#[discriminator_hash_input("struct_u8")]
struct StructWithU8 {
    field1: u8,
    field2: u8,
}

#[repr(C)]
#[derive(Clone, Copy, Default, Debug, PartialEq, Pod, Zeroable, SplDiscriminate)]
#[discriminator_hash_input("struct_u16")]
struct StructWithU16 {
    field1: u16,
    field2: u16,
}

#[repr(C)]
#[derive(Clone, Copy, Default, Debug, PartialEq, Pod, Zeroable, SplDiscriminate)]
#[discriminator_hash_input("struct_u32")]
struct StructWithU32 {
    field1: u32,
    field2: u32,
}

#[repr(C)]
#[derive(Clone, Copy, Default, Debug, PartialEq, Pod, Zeroable, SplDiscriminate)]
#[discriminator_hash_input("struct_u64")]
struct StructWithU64 {
    field1: u64,
    field2: u64,
}

#[repr(C)]
#[derive(Clone, Copy, Default, Debug, PartialEq, Pod, Zeroable, SplDiscriminate)]
#[discriminator_hash_input("struct_u128")]
struct StructWithU128 {
    field1: u128,
    field2: u128,
}

// Now lets' try to use these to make some TLV entries.

#[cfg(test)]
mod tests {
    use {
        super::*,
        std::mem::size_of,
        solana_program::program_error::ProgramError,
        spl_type_length_value::state::{TlvState, TlvStateBorrowed, TlvStateMut},
    };

    fn test_tlv<T: Default + Pod + Zeroable + SplDiscriminate>(entry: T) -> Result<T, ProgramError> {
        let account_size = TlvStateBorrowed::get_base_len()
            + size_of::<T>();

        let mut buffer = vec![0; account_size];
        let mut state = TlvStateMut::unpack(&mut buffer).unwrap();

        let data = state.init_value::<T>(false).map(|res| res.0)?;
        *data = entry;

        state.get_first_value::<T>().map(|data| data.clone()) // Just cloning for example's sake to avoid lifetime
    }

    // We can see that `u8`, `u16`, and `u32` all work as expected.

    #[test]
    fn test_u8() {
        assert_eq!(
            test_tlv(StructWithU8 {
                field1: 1u8,
                field2: 2u8,
            }),
            Ok(StructWithU8 {
                field1: 1u8,
                field2: 2u8,
            })
        );
    }

    #[test]
    fn test_u16() {
        assert_eq!(
            test_tlv(StructWithU16 {
                field1: 1u16,
                field2: 2u16,
            }),
            Ok(StructWithU16 {
                field1: 1u16,
                field2: 2u16,
            })
        );
    }

    #[test]
    fn test_u32() {
        assert_eq!(
            test_tlv(StructWithU32 {
                field1: 1u32,
                field2: 2u32,
            }),
            Ok(StructWithU32 {
                field1: 1u32,
                field2: 2u32,
            })
        );
    }

    // However, anything larger than `u64` throws on account of the alignment.
    // The error actually comes from `pod_from_bytes(..)`, and it's a bytemuck
    // error: `TargetAlignmentGreaterAndInputNotAligned`.

    #[test]
    fn test_u64() {
        assert_eq!(
            test_tlv(StructWithU64 {
                field1: 1u64,
                field2: 2u64,
            }),
            Err(ProgramError::InvalidArgument)
        );
    }

    #[test]
    fn test_u128() {
        assert_eq!(
            test_tlv(StructWithU128 {
                field1: 1u128,
                field2: 2u128,
            }),
            Err(ProgramError::InvalidArgument)
        );
    }
}

Now, if we change the PodLength within the library to be a PodU64, suddenly we can see that both the u64 and u128 types work! This is because the alignment is now 8.

Solution

We need to figure out a way to make this relationship amongst Pod types work with various integer sizes.

There are a few solutions:

1. Add padding to the length value within the library.
2. Make the length value use the same integer size required by a type's padding.
3. Allow a user to specify the integer type for length.

Regardless, it's also worth mentioning that the error coming back from pod_from_bytes(..) is not very helpful (ProgramError::InvalidArgument). We should also introduce a pass-through error for bytemuck errors.