Skip to content

Conversation

@grod220
Copy link
Member

@grod220 grod220 commented Oct 1, 2025

At the moment, ListView is returning ProgramError everywhere. This makes Pinnochio consumers required to map the error:

pub fn unpack_list_view(data: &[u8]) -> Result<ListViewReadOnly<'_, Bid>, PinocchioErr> {
    assert_discriminator(data)?;
    let (_, slice_region) = data.split_at(ArrayDiscriminator::LENGTH);
    let res = ListView::<Bid>::unpack(slice_region).map_err(|e| u64::from(e).into())?;
    Ok(res)
}

The primary goal of this change is to enable the direct use of the ? operator in consuming code by returning PodSliceError instead and implementing From traits to both program error types. After:

pub fn unpack_list_view(data: &[u8]) -> Result<ListViewReadOnly<'_, Bid>, PinocchioErr> {
    assert_discriminator(data)?;
    let (_, slice_region) = data.split_at(ArrayDiscriminator::LENGTH);
    let res = ListView::<Bid>::unpack(slice_region)?;
    Ok(res)
}

Change type

Breaking change. Several returned err types have changed. Will require major version bump. Open to thoughts on how to handle this or if this is the right change in general.

@grod220 grod220 force-pushed the list-view-err-update branch 2 times, most recently from 78b268f to ec6a4b6 Compare October 1, 2025 12:56
@grod220 grod220 force-pushed the list-view-err-update branch from ec6a4b6 to f24165f Compare October 1, 2025 12:58
Copy link
Contributor

@buffalojoec buffalojoec left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based. Gud fix. Just a few small things.

@grod220 grod220 requested a review from buffalojoec October 2, 2025 09:11
Copy link
Contributor

@buffalojoec buffalojoec left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool! Thanks for throwing in some extra variants.

@grod220 grod220 merged commit f0b2df3 into main Oct 2, 2025
29 checks passed
@grod220 grod220 deleted the list-view-err-update branch October 2, 2025 10:26
@grod220 grod220 mentioned this pull request Oct 2, 2025
@joncinque
Copy link
Contributor

joncinque commented Oct 2, 2025

Pinocchio will start using the same program error as the sdk soon, so I would prefer to stick with ProgramError: anza-xyz/pinocchio#112

Edit: mainly because it's a huge hassle to upgrade everything when there's a breaking change to spl-pod, I would avoid it

@grod220
Copy link
Member Author

grod220 commented Oct 2, 2025

Oh! If those types are converging, then this is indeed not necessary. Will revert.

grod220 added a commit that referenced this pull request Oct 2, 2025
grod220 added a commit that referenced this pull request Oct 2, 2025
Revert "Make `ListView` friendlier to `?` syntax (#166)"

This reverts commit f0b2df3.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants