Conversation
grod220
force-pushed
the
wincode-state
branch
2 times, most recently
from
e0471c2 to
3b7d1b1
Compare
joncinque
left a comment
joncinque
left a comment
There was a problem hiding this comment.
Looks great overall! Mostly small things on my side
febo
left a comment
febo
left a comment
There was a problem hiding this comment.
I got a few more small comments.
grod220
force-pushed
the
wincode-state
branch
from
f28e244 to
a08e638
Compare
grod220
force-pushed
the
wincode-state
branch
from
a08e638 to
d610418
Compare
| #[test] | ||
| fn tag_len_is_4_bytes() { | ||
| assert_eq!(StakeStateV2Tag::TAG_LEN, 4); | ||
| } |
There was a problem hiding this comment.
This one feels that it could be a static assert instead of a test.
const _: () = assert!(StakeStateV2Tag::TAG_LEN == size_of::<u32>());
| /// Parse stake account data into a read-only reference. | ||
| pub fn from_bytes(data: &[u8]) -> Result<&Self, StakeStateError> { | ||
| let state = <Self as ZeroCopy>::from_bytes(data).map_err(|_| StakeStateError::Decode)?; | ||
| StakeStateV2Tag::assert_valid_tag(u32::from(state.tag))?; | ||
| Ok(state) | ||
| } | ||
|
|
||
| /// Parse stake account data into a mutable reference. | ||
| pub fn from_bytes_mut(data: &mut [u8]) -> Result<&mut Self, StakeStateError> { | ||
| let state = | ||
| <Self as ZeroCopy>::from_bytes_mut(data).map_err(|_| StakeStateError::Decode)?; | ||
| StakeStateV2Tag::assert_valid_tag(u32::from(state.tag))?; | ||
| Ok(state) | ||
| } |
There was a problem hiding this comment.
I am not sure we can rely on the assert_valid_tag to always being used. There is a scenario where I can add a stake: StakeStateV2 field to my zero-copy type. When a load my type, these helpers won't be used (from_bytes or from_bytes_mut), so no validation will happen. Then if I use the stake.tag() method I can be in UB.
| #[derive(Clone, Debug, PartialEq, SchemaWrite, SchemaRead)] | ||
| #[wincode(assert_zero_copy)] | ||
| pub struct StakeStateV2 { | ||
| tag: PodU32, |
There was a problem hiding this comment.
@joncinque Do you think we can split this to be:
tag: u8,
_unused: [u8; 3],This way the conversion won't require going from [u8; 4] to u32 everytime. The other alternative is adding a TryFrom<PodU32> that looks at the first byte only.
| #[inline] | ||
| pub(crate) unsafe fn from_u32_unchecked(v: u32) -> Self { | ||
| debug_assert!(v <= Self::RewardsPool as u32); | ||
| core::mem::transmute::<u32, StakeStateV2Tag>(v) | ||
| } | ||
|
|
||
| #[inline] | ||
| pub(crate) fn assert_valid_tag(v: u32) -> Result<(), StakeStateError> { | ||
| match v { | ||
| 0..=3 => Ok(()), | ||
| other => Err(StakeStateError::InvalidTag(other)), | ||
| } | ||
| } |
There was a problem hiding this comment.
Not sure if these helpers can be used (see comment here). We might need to move this logic to the TryFrom impl.
febo
left a comment
febo
left a comment
There was a problem hiding this comment.
Few more comments, mainly related to StakeStateV2Tag.
3 participants
Adds a new zero‑copy state API backed by wincode.
Key Changes
p-stake-interfacecrate (#![no_std]compatible)StakeStateV2:StakeStateV2::from_bytes(&[u8]) -> Result<&StakeStateV2, StakeStateError>for read-only accessStakeStateV2::from_bytes_mut(&mut [u8]) -> Result<&mut StakeStateV2, StakeStateError>formutations
meta(),stake(),meta_mut(),stake_mut()initialize(),delegate()Motivation
Deserialization via bincode/borsh is computationally expensive for programs that only need to access specific fields or verify the state. This zero-copy approach allows for significantly lower compute unit (CU) usage when interacting with stake accounts.