Add support for ListView #92

grod220 · 2025-06-06T12:17:37Z

Currently, PodSliceMut has served as the data structure for dynamically sized list. As we expand functionality to this, it is starting to feel misnamed (as Slices generally aren't things that are resized).

Changes made

Add new ListView struct that serves as the future home for mutations involving ordered lists
Copy over methods 1:1 from PodSliceMut: unpack, init, push
Add new remove methods: remove_at, remove_first_where, len + new tests
Mark PodSliceMut as deprecated

grod220 · 2025-06-06T12:19:16Z

pod/src/list.rs

+    fn unpack_internal<'a>(data: &'a mut [u8], init: bool) -> Result<Self, ProgramError>
+    where
+        'a: 'data,
+    {
+        if data.len() < LENGTH_SIZE {
+            return Err(PodSliceError::BufferTooSmall.into());
+        }
+        let (length, data) = data.split_at_mut(LENGTH_SIZE);
+        let length = pod_from_bytes_mut::<PodU32>(length)?;
+        if init {
+            *length = 0.into();
+        }
+        let max_length = max_len_for_type::<T>(data.len(), u32::from(*length) as usize)?;
+        let data = pod_slice_from_bytes_mut(data)?;
+        Ok(Self {
+            length,
+            data,
+            max_length,
+        })
+    }
+
+    /// Unpack the mutable buffer into a mutable slice
+    pub fn unpack<'a>(data: &'a mut [u8]) -> Result<Self, ProgramError>
+    where
+        'a: 'data,
+    {
+        Self::unpack_internal(data, /* init */ false)
+    }
+
+    /// Unpack the mutable buffer into a mutable slice, and initialize the
+    /// slice to 0-length
+    pub fn init<'a>(data: &'a mut [u8]) -> Result<Self, ProgramError>
+    where
+        'a: 'data,
+    {
+        Self::unpack_internal(data, /* init */ true)
+    }
+
+    /// Add another item to the slice
+    pub fn push(&mut self, t: T) -> Result<(), ProgramError> {
+        let length = u32::from(*self.length);
+        if length as usize == self.max_length {
+            Err(PodSliceError::BufferTooSmall.into())
+        } else {
+            self.data[length as usize] = t;
+            *self.length = length.saturating_add(1).into();
+            Ok(())
+        }
+    }


These are 1:1 copied from PodSliceMut

I was actually hoping we could turn this into free functions and that way we don't have to maintain two at once, but instead we can just call into them from each API. What do you think?

It seems like ListView is a superset of PodSliceMut functionality. Looks like it can just use the new thing under the hood with the same effect. Have a look and let me know what you think.

joncinque

Mostly small points, looks good to me!

As another naming idea, what we're really providing is a View on existing data, so it's like a Vec that won't reallocate, which also similar to an arena allocator.

So we could go with ListView or NoAllocVec or ArenaList. I kind of like the last one, since it might describe the situation best.

pod/Cargo.toml

pod/src/slice.rs

tlv-account-resolution/src/state.rs

febo · 2025-06-07T00:46:45Z

pod/src/list.rs

+/// ## Memory Layout
+///
+/// The structure assumes the underlying byte buffer is formatted as follows:
+/// 1.  **Length**: A `u32` value (`PodU32`) at the beginning of the buffer,


Having 4 bytes as a "header" shifts the offset for the array of Ts to only accept types with alignments 1, 2, or 4 in the case that the byte array has alignment 8. It would be more flexible to have 8 bytes (PodU64) so the offset is then compatible with 1, 2, 4 and 8. Another alternative is to have the length as a generic type.

It should probably be a generic, so that we can offer a way for existing implementations dependent on PodSlice/PodSliceMut to reliably read their data, which is going to be serialized with a u32 length prefix.

Sounds good!

Added generic for Length 👍

pod/src/list.rs

febo

Looks very nice – left a few comments. The main one is about the alignment. It would be nice if we could make it more flexible to support types 8-byte aligned.

febo · 2025-06-07T01:35:14Z

So we could go with ListView or NoAllocVec or ArenaList. I kind of like the last one, since it might describe the situation best.

I have a slight preference for ListView – I am not sure if users will make the connection between arena allocator and ArenaList. 😊

buffalojoec · 2025-06-07T04:20:12Z

So we could go with ListView or NoAllocVec or ArenaList. I kind of like the last one, since it might describe the situation best.

I have a slight preference for ListView – I am not sure if users will make the connection between arena allocator and ArenaList. 😊

Agreed, I personally have never heard the term "arena" in this context before, and I assume most developers will whiff on the underlying meaning at first. However, "view" implies readonly, and that's not the case here.

Here are a few ideas:

PodList (I think it's actually ok)
StackList
BoxList
Dynamic + any of the above
PodSpan

buffalojoec · 2025-06-07T04:21:13Z

pod/src/list.rs

+    fn unpack_internal<'a>(data: &'a mut [u8], init: bool) -> Result<Self, ProgramError>
+    where
+        'a: 'data,
+    {
+        if data.len() < LENGTH_SIZE {
+            return Err(PodSliceError::BufferTooSmall.into());
+        }
+        let (length, data) = data.split_at_mut(LENGTH_SIZE);
+        let length = pod_from_bytes_mut::<PodU32>(length)?;
+        if init {
+            *length = 0.into();
+        }
+        let max_length = max_len_for_type::<T>(data.len(), u32::from(*length) as usize)?;
+        let data = pod_slice_from_bytes_mut(data)?;
+        Ok(Self {
+            length,
+            data,
+            max_length,
+        })
+    }
+
+    /// Unpack the mutable buffer into a mutable slice
+    pub fn unpack<'a>(data: &'a mut [u8]) -> Result<Self, ProgramError>
+    where
+        'a: 'data,
+    {
+        Self::unpack_internal(data, /* init */ false)
+    }
+
+    /// Unpack the mutable buffer into a mutable slice, and initialize the
+    /// slice to 0-length
+    pub fn init<'a>(data: &'a mut [u8]) -> Result<Self, ProgramError>
+    where
+        'a: 'data,
+    {
+        Self::unpack_internal(data, /* init */ true)
+    }
+
+    /// Add another item to the slice
+    pub fn push(&mut self, t: T) -> Result<(), ProgramError> {
+        let length = u32::from(*self.length);
+        if length as usize == self.max_length {
+            Err(PodSliceError::BufferTooSmall.into())
+        } else {
+            self.data[length as usize] = t;
+            *self.length = length.saturating_add(1).into();
+            Ok(())
+        }
+    }


I was actually hoping we could turn this into free functions and that way we don't have to maintain two at once, but instead we can just call into them from each API. What do you think?

buffalojoec · 2025-06-07T04:22:10Z

pod/src/list.rs

+#[cfg(test)]
+mod tests {


Building on my other comment about the reusable free functions, we could probably either test just the free functions (in one place) or make the tests generic over each collection API.

If PodSliceMut uses ListView under the hood, is seems like it should get same coverage guarantees.

buffalojoec · 2025-06-07T04:23:04Z

pod/src/slice.rs

+#[deprecated(
+    since = "0.6.0",
+    note = "This struct will be removed in the next major release (1.0.0). Please use `PodList` instead."
+)]


I wonder if deprecation is really necessary here. I don't really see any harm in leaving the slice-based ones alone, even if the mutable one isn't super aptly-named. I could be swayed here, though.

Note that if we're deprecating then I suppose we don't need the free functions, but it may not be a bad idea regardless, especially if we ever build more list-based APIs.

Personally, given the functionality overlap, I'm in favor of sunsetting it so we don't need to maintain both.

febo · 2025-06-07T10:18:11Z

So we could go with ListView or NoAllocVec or ArenaList. I kind of like the last one, since it might describe the situation best.

I have a slight preference for ListView – I am not sure if users will make the connection between arena allocator and ArenaList. 😊

Agreed, I personally have never heard the term "arena" in this context before, and I assume most developers will whiff on the underlying meaning at first. However, "view" implies readonly, and that's not the case here.

Here are a few ideas:

PodList (I think it's actually ok)

StackList

BoxList

Dynamic + any of the above

PodSpan

Agree with your point on the "View" implying read-only, but while the type has a "dynamic" size its capacity is fixed. In that sense it is a view. 😊

The only concern that I have with using Pod* is that the type itself is not Pod, so it might look a bit odd.

grod220

Sorry for the delay on this! Wasn't quite sure we were going to need this until recent confirmations in the Unstake program.

grod220 · 2025-07-09T11:07:33Z

pod/src/list.rs

+/// ## Memory Layout
+///
+/// The structure assumes the underlying byte buffer is formatted as follows:
+/// 1.  **Length**: A `u32` value (`PodU32`) at the beginning of the buffer,


Added generic for Length 👍

grod220 · 2025-07-09T11:08:25Z

pod/src/list.rs

+    fn unpack_internal<'a>(data: &'a mut [u8], init: bool) -> Result<Self, ProgramError>
+    where
+        'a: 'data,
+    {
+        if data.len() < LENGTH_SIZE {
+            return Err(PodSliceError::BufferTooSmall.into());
+        }
+        let (length, data) = data.split_at_mut(LENGTH_SIZE);
+        let length = pod_from_bytes_mut::<PodU32>(length)?;
+        if init {
+            *length = 0.into();
+        }
+        let max_length = max_len_for_type::<T>(data.len(), u32::from(*length) as usize)?;
+        let data = pod_slice_from_bytes_mut(data)?;
+        Ok(Self {
+            length,
+            data,
+            max_length,
+        })
+    }
+
+    /// Unpack the mutable buffer into a mutable slice
+    pub fn unpack<'a>(data: &'a mut [u8]) -> Result<Self, ProgramError>
+    where
+        'a: 'data,
+    {
+        Self::unpack_internal(data, /* init */ false)
+    }
+
+    /// Unpack the mutable buffer into a mutable slice, and initialize the
+    /// slice to 0-length
+    pub fn init<'a>(data: &'a mut [u8]) -> Result<Self, ProgramError>
+    where
+        'a: 'data,
+    {
+        Self::unpack_internal(data, /* init */ true)
+    }
+
+    /// Add another item to the slice
+    pub fn push(&mut self, t: T) -> Result<(), ProgramError> {
+        let length = u32::from(*self.length);
+        if length as usize == self.max_length {
+            Err(PodSliceError::BufferTooSmall.into())
+        } else {
+            self.data[length as usize] = t;
+            *self.length = length.saturating_add(1).into();
+            Ok(())
+        }
+    }


It seems like ListView is a superset of PodSliceMut functionality. Looks like it can just use the new thing under the hood with the same effect. Have a look and let me know what you think.

pod/src/list.rs

grod220 · 2025-07-09T11:10:05Z

pod/src/list.rs

+#[cfg(test)]
+mod tests {


If PodSliceMut uses ListView under the hood, is seems like it should get same coverage guarantees.

grod220 · 2025-07-09T11:11:06Z

pod/src/slice.rs

+#[deprecated(
+    since = "0.6.0",
+    note = "This struct will be removed in the next major release (1.0.0). Please use `PodList` instead."
+)]


Personally, given the functionality overlap, I'm in favor of sunsetting it so we don't need to maintain both.

pod/src/list.rs

buffalojoec

Awesome!! This is looking fantastic, and the diagrams under unpack_internal are really an elegant touch for helping follow the slice processing.

buffalojoec · 2025-07-12T03:47:45Z

pod/src/list.rs

+    }
+}
+
+/// A mutable, variable-length collection of `Pod` types backed by a byte buffer.


The last bit "backed by a byte buffer" isn't super clear. How about something like this?

"An API for interpreting a raw buffer (&[u8]) as a mutable, variable-length collection of Pod elements."

buffalojoec · 2025-07-12T03:51:33Z

pod/src/list.rs

+/// such as Solana programs, allowing for dynamic-length data structures within a
+/// fixed-size account.


Suggested change

/// such as Solana programs, allowing for dynamic-length data structures within a

/// fixed-size account.

/// such as Solana programs, allowing for efficient reads and manipulation of

/// dynamic-length data structures.

buffalojoec · 2025-07-12T04:02:02Z

pod/src/primitives.rs

+/// Trait for types that can be used as length fields in Pod data structures
+pub trait PodLength: bytemuck::Pod + Copy {
+    fn as_usize(&self) -> usize;
+
+    fn from_usize(val: usize) -> Result<Self, crate::error::PodSliceError>;
+}


Bytemuck already requires Copy for Pod: https://docs.rs/bytemuck/latest/bytemuck/trait.Pod.html

Also, I don't think this belongs in here. I think PodLength should have its own module.

Aren't these methods just Into<usize> and TryFrom<usize> after all? Why not just add those to the trait constraints?

pub trait PodLength: bytemuck::Pod + Into<usize> + TryFrom<usize> {}

buffalojoec · 2025-07-12T04:06:40Z

pod/src/primitives.rs

+        u16::from(*self) as usize
+    }
+
+    fn from_usize(val: usize) -> Result<Self, crate::error::PodSliceError> {


Suggested change

fn from_usize(val: usize) -> Result<Self, crate::error::PodSliceError> {

fn try_from_usize(val: usize) -> Result<Self, crate::error::PodSliceError> {

buffalojoec · 2025-07-12T04:14:59Z

pod/src/slice.rs

-    length: &'data mut PodU32,
-    data: &'data mut [T],
-    max_length: usize,
+    inner: ListView<'data, T, PodU32>,


Nice! This deprecation strategy works for me!

buffalojoec · 2025-07-12T04:23:24Z

pod/src/list.rs

+    let remainder = length_size
+        .checked_rem(data_align)
+        .ok_or(ProgramError::ArithmeticOverflow)?;
+    if remainder == 0 {
+        Ok(0)
+    } else {
+        data_align
+            .checked_sub(remainder)
+            .ok_or(ProgramError::ArithmeticOverflow)
+    }


This block could be simplified:

Suggested change

let remainder = length_size

.checked_rem(data_align)

.ok_or(ProgramError::ArithmeticOverflow)?;

if remainder == 0 {

Ok(0)

} else {

data_align

.checked_sub(remainder)

.ok_or(ProgramError::ArithmeticOverflow)

}

length_size

.checked_rem(data_align)

.and_then(|rem| data_align.checked_sub(rem))

.ok_or(ProgramError::ArithmeticOverflow)

buffalojoec · 2025-07-12T04:28:00Z

pod/src/list.rs

+    }
+
+    /// Add another item to the slice
+    pub fn push(&mut self, t: T) -> Result<(), ProgramError> {


Since this is an outward-facing API method:

Suggested change

pub fn push(&mut self, t: T) -> Result<(), ProgramError> {

pub fn push(&mut self, item: T) -> Result<(), ProgramError> {

buffalojoec · 2025-07-12T04:29:12Z

pod/src/list.rs

+    /// Add another item to the slice
+    pub fn push(&mut self, t: T) -> Result<(), ProgramError> {
+        let length = self.length.as_usize();
+        if length == self.max_length {


I know we can't really end up here because of how unpack works, but maybe >= is safer?

buffalojoec · 2025-07-12T04:33:23Z

pod/src/list.rs

+
+    /// Find the first element that satisfies `predicate` and remove it,
+    /// returning the element.
+    pub fn remove_first_where<P>(&mut self, predicate: P) -> Result<T, ProgramError>


I can be swayed on this, but I would actually say we don't offer this method. It's not bad to implement on the caller side.

buffalojoec · 2025-07-12T04:36:23Z

pod/src/list.rs

+
+    /// Remove and return the element at `index`, shifting all later
+    /// elements one position to the left.
+    pub fn remove_at(&mut self, index: usize) -> Result<T, ProgramError> {


You can just call this remove, since it behaves exactly like Vec::remove!

Suggested change

pub fn remove_at(&mut self, index: usize) -> Result<T, ProgramError> {

pub fn remove(&mut self, index: usize) -> Result<T, ProgramError> {

grod220 commented Jun 6, 2025

View reviewed changes

grod220 force-pushed the podslice-remove-methods branch 3 times, most recently from 4f972f1 to 817eb9f Compare June 6, 2025 12:33

grod220 requested review from febo, joncinque and buffalojoec June 6, 2025 12:36

joncinque reviewed Jun 6, 2025

View reviewed changes

pod/Cargo.toml Outdated Show resolved Hide resolved

pod/src/slice.rs Outdated Show resolved Hide resolved

tlv-account-resolution/src/state.rs Outdated Show resolved Hide resolved