Use sol_get_sysvar instead of sol_get_<NAME>_sysvar, fixed #458

rustopian · 2025-11-25T10:53:52Z

[WIP]

This addresses Issue #235, fixing two issues with the original (reverted) PR:

The contract of from_raw_parts_mut was being violated, in a way that miri considered safe, but which could have theoretically caused trouble in the future.
The implementation assumed that the number of bytes returned by the runtime was exactly
mem::size_of::<Self>(), which was not always true. Rent, for example, contains a u64, an f64 and a u8. In memory the compiler pads this struct to 24 bytes, but the runtime stores its contents as a 17‑byte bincode serialization. The get_sysvar call thus returns only 17 bytes. Passing a 24‑byte buffer results in an OFFSET_LENGTH_EXCEEDS_SYSVAR error and undefined behavior when uninitialized bytes are interpreted as f64.

Note: this PR only updates the three sysvars Rent, EpochSchedule, and EpochRewards, which require two different approaches as commented below. Clock and LastRestartSlot do not require special handling.

Thanks to jamie-osec for investigating the cause here and noticing the from_raw_parts_mut violation. I'll add you as contributor to the final.

github-actions · 2025-11-25T10:54:03Z

If this PR represents a change to the sysvar layout, please open a follow-up PR to update the JavaScript client @solana/sysvars (example)
Thank you for keeping the JavaScript clients in sync with the Rust clients.

github-actions · 2025-11-25T15:23:37Z

If this PR represents a change to the sysvar layout, please open a follow-up PR to update the JavaScript client @solana/sysvars (example)
Thank you for keeping the JavaScript clients in sync with the Rust clients.

github-actions · 2025-11-25T15:26:39Z

If this PR represents a change to the sysvar layout, please open a follow-up PR to update the JavaScript client @solana/sysvars (example)
Thank you for keeping the JavaScript clients in sync with the Rust clients.

github-actions · 2025-11-25T15:34:17Z

If this PR represents a change to the sysvar layout, please open a follow-up PR to update the JavaScript client @solana/sysvars (example)
Thank you for keeping the JavaScript clients in sync with the Rust clients.

rustopian · 2025-11-25T15:36:35Z

sysvar/src/epoch_rewards.rs

+#[repr(C, packed)]
+#[derive(Clone, Copy)]
+struct EpochRewardsPacked {
+    distribution_starting_block_height: u64,
+    num_partitions: u64,
+    parent_blockhash: [u8; 32],
+    total_points: u128,
+    total_rewards: u64,
+    distributed_rewards: u64,
+    active: u8, // bool as u8
+}
+
+const _: () = assert!(core::mem::size_of::<EpochRewardsPacked>() == 81);
+
+impl From<EpochRewardsPacked> for EpochRewards {
+    fn from(p: EpochRewardsPacked) -> Self {


@febo please lmk what you think of this approach.

Originally I did a whole macro setup, but it was overkill given that we're only worried about 3 padded sysvars here.

Compile-time checks just below ensure 1) field parity between the two structs (repr(C) and packed) and 2) size

Left a comment below about the use of packed.

rustopian · 2025-11-25T15:37:37Z

define-syscall/src/lib.rs

 #[macro_export]
 macro_rules! define_syscall {
-    (fn $name:ident($($arg:ident: $typ:ty),*) -> $ret:ty) => {
+    ($(#[$attr:meta])* fn $name:ident($($arg:ident: $typ:ty),*) -> $ret:ty) => {


Enabling deprecated to be passed through (as in the original sol_get_sysvar PR)

github-actions · 2025-11-25T15:41:49Z

If this PR represents a change to the sysvar layout, please open a follow-up PR to update the JavaScript client @solana/sysvars (example)
Thank you for keeping the JavaScript clients in sync with the Rust clients.

github-actions · 2025-11-25T15:48:22Z

If this PR represents a change to the sysvar layout, please open a follow-up PR to update the JavaScript client @solana/sysvars (example)
Thank you for keeping the JavaScript clients in sync with the Rust clients.

github-actions · 2025-11-25T15:49:46Z

If this PR represents a change to the sysvar layout, please open a follow-up PR to update the JavaScript client @solana/sysvars (example)
Thank you for keeping the JavaScript clients in sync with the Rust clients.

github-actions · 2025-11-25T15:52:58Z

If this PR represents a change to the sysvar layout, please open a follow-up PR to update the JavaScript client @solana/sysvars (example)
Thank you for keeping the JavaScript clients in sync with the Rust clients.

febo · 2025-11-25T17:24:38Z

sysvar/src/epoch_schedule.rs

+#[repr(C, packed)]
+#[derive(Clone, Copy)]
+struct EpochSchedulePacked {
+    slots_per_epoch: u64,
+    leader_schedule_slot_offset: u64,
+    warmup: u8, // bool as u8
+    first_normal_epoch: u64,
+    first_normal_slot: u64,
+}


Using packed is problematic when there are unaligned fields – both first_normal_epoch and first_normal_slot are unaligned so can't be read directly.

What do you think if we change the definition of all sysvars to be like?

pub struct EpochSchedule { /// The maximum number of slots in each epoch. slots_per_epoch: [u8; 8], /// A number of slots before beginning of an epoch to calculate /// a leader schedule for that epoch. leader_schedule_slot_offset: [u8; 8], /// Whether epochs start short and grow. warmup: u8, /// The first epoch after the warmup period. /// /// Basically: `log2(slots_per_epoch) - log2(MINIMUM_SLOTS_PER_EPOCH)`. first_normal_epoch: [u8; 8], /// The first slot after the warmup period. /// /// Basically: `MINIMUM_SLOTS_PER_EPOCH * (2.pow(first_normal_epoch) - 1)`. first_normal_slot: [u8; 8], } impl EpochSchedule { pub fn slots_per_epoch(&self) -> u64 { u64::from_le_bytes(self.slots_per_epoch) } pub fn leader_schedule_slot_offset(&self) -> u64 { u64::from_le_bytes(self.leader_schedule_slot_offset) } // Could also return the `u8` value directly. pub fn warmup(&self) -> Result<bool, ProgramError> { match self.warmup { 0 => Ok(false), 1 => Ok(true), _ => Err(ProgramError::InvalidAccountData), } } // other fields... pub fn first_normal_slot(&self) -> u64 { u64::from_le_bytes(self.first_normal_slot) } }

There is no "penalty" for using u64::from_le_bytes, although the implementation is a bit more verbose.

Ah, of course. The packed struct works currently since it’s just used for From, but that of course copies everything out.

I’ll implement your approach, thanks.

(Despite appearances here, I did in fact attend your optimization talk which mentioned this. exact. pattern. Ahem.)

Doesn't this constitute a breaking change (public fields removed)? I believe the internal-only packed version is better here - an unaligned read from a packed field produces the same IR/code as reading it as an array.

@febo I've implemented your suggestion in this PR, and it's much nicer looking (and zero copy). However, as @jamie-osec mentions, the accessors are a breaking change. What formerly was e.g. .slots_per_epoch becomes .slots_per_epoch()

Note: I've only implemented it for the problematic Rent, EpochRewards, and EpochSchedule for now, pending our decision here.

Doesn't this constitute a breaking change (public fields removed)?

Yes, that is a drawback of the approach. We should be probably ok since crates are versioned individually.

I believe the internal-only packed version is better here - an unaligned read from a packed field produces the same IR/code as reading it as an array.

The issue is that we do an extra copy to return the "non-packed" version, which is what we would like to avoid. Returning the "packed" version is problematic – easy to end up in UB by taking a reference to an unaligned field.

cc @joncinque

I think this break would be annoying for Agave and some downstream programs that directly access any of these fields. We've introduced Pod* types for other sysvars, so I would recommend doing the same thing here

github-actions · 2025-12-02T23:29:36Z

If this PR represents a change to the sysvar layout, please open a follow-up PR to update the JavaScript client @solana/sysvars (example)
Thank you for keeping the JavaScript clients in sync with the Rust clients.

LStan · 2025-12-03T11:14:48Z

There is a bool in EpochRewards. Is it safe to load it without Pod?

rustopian · 2025-12-03T11:26:17Z

There is a bool in EpochRewards. Is it safe to load it without Pod?

EpochRewards is 16-aligned:

#[repr(C, align(16))]
...
pub struct EpochRewards {
    /// The starting block height of the rewards distribution in the current
    /// epoch
    pub distribution_starting_block_height: u64,

...

    /// Whether the rewards period (including calculation and distribution) is
    /// active
    pub active: bool,
}

So when we call impl_sysvar_get for it, we specify 15 zeroes of padding:

impl Sysvar for EpochRewards {
    impl_sysvar_get!(id(), 15);
}

...just as we do for Rent (but with 7). This zero-fills in the resulting get() function:

macro_rules! impl_sysvar_get {
    ...
    ($sysvar_id:expr, $padding:literal) => {
        fn get() -> Result<Self, $crate::__private::ProgramError> {
            ...
            let result =
                unsafe { $crate::get_sysvar_unchecked(var_addr, sysvar_id_ptr, 0, length as u64) };
            match result {
                Ok(()) => {
                    // SAFETY: All bytes now initialized: syscall filled data
                    // bytes and we zeroed padding.
                    unsafe {
                        var_addr.add(length).write_bytes(0, $padding);
                        Ok(var.assume_init())
                    }
                }
                // Unexpected errors are folded into `UnsupportedSysvar`.
                Err(_) => Err($crate::__private::ProgramError::UnsupportedSysvar),
            }
        }
    };
    // Variant for sysvars without padding (struct size matches bincode size).
    ($sysvar_id:expr) => {
        $crate::impl_sysvar_get!($sysvar_id, 0);
    };
}

We can do this easily (as in Rent) since the bool is at the end of the struct. The only reason EpochSchedule needs special handling (which this PR now does with PodEpochSchedule) is that it has a bool right in the middle:

pub struct EpochSchedule {
    pub slots_per_epoch: u64,
    pub leader_schedule_slot_offset: u64,
    pub warmup: bool,
    pub first_normal_epoch: u64,
    pub first_normal_slot: u64,
}

LStan · 2025-12-03T11:31:33Z

I meant the casting of u8 to bool. What if that byte for some reason contains not 0 or 1?

sysvar/src/epoch_schedule.rs

febo · 2025-12-03T12:06:33Z

I meant the casting of u8 to bool. What if that byte for some reason contains not 0 or 1?

@rustopian This is a good point, we can't cast a value directly to bool unless we validate it is a 0 or 1. Left a comment about this here.

Co-authored-by: Fernando Otero <[email protected]>

rustopian · 2025-12-03T12:52:41Z

I meant the casting of u8 to bool. What if that byte for some reason contains not 0 or 1?

@rustopian This is a good point, we can't cast a value directly to bool unless we validate it is a 0 or 1. Left a comment about this here.

I implemented your suggestion, but the question still remains for EpochRewards, which uses the same impl_sysvar_get approach as Rent (pad the end) but with a final item bool.

But I think we could make a case for allowing this (otherwise, I'll have to do PodEpochRewards too). Because of an upstream invariant: sysvar data is created by the Solana runtime, and of course serializes bool as exactly 0 for false, 1 for true.

e.g.

// SAFETY: upstream invariant: the sysvar data is created exclusively
// by the Solana runtime and serializes bool as 0x00 or 0x01.

Incidentally, this also can apply to PodEpochSchedule::warmup(). Yes, we get UB on values other than 0 or 1, but Solana runtime guarantees this is a serialized bool and thus cannot be anything else in solana target.

I can simply validate when target is not solana.

Wdyt @febo @LStan

LStan · 2025-12-03T13:01:42Z

I meant exactly what @febo said. You could use bool for warmup in PodEpochSchedule, but you didn't because it's UB.

febo · 2025-12-04T23:51:17Z

I meant the casting of u8 to bool. What if that byte for some reason contains not 0 or 1?

@rustopian This is a good point, we can't cast a value directly to bool unless we validate it is a 0 or 1. Left a comment about this here.

I implemented your suggestion, but the question still remains for EpochRewards, which uses the same impl_sysvar_get approach as Rent (pad the end) but with a final item bool.

But I think we could make a case for allowing this (otherwise, I'll have to do PodEpochRewards too). Because of an upstream invariant: sysvar data is created by the Solana runtime, and of course serializes bool as exactly 0 for false, 1 for true.

e.g.
// SAFETY: upstream invariant: the sysvar data is created exclusively
// by the Solana runtime and serializes bool as 0x00 or 0x01.
Incidentally, this also can apply to PodEpochSchedule::warmup(). Yes, we get UB on values other than 0 or 1, but Solana runtime guarantees this is a serialized bool and thus cannot be anything else in solana target.

I can simply validate when target is not solana.

Wdyt @febo @LStan

I double checked this and you are right, we can rely on the runtime always storing valid values. The sysvar account is always serialized with bincode and bincode encodes bool values as 0 or 1. We could revert the change to warmup as well.

rustopian · 2025-12-05T11:00:40Z

@febo great, I rolled back warmup() change

safe sol_get_sysvar from packed reprs

2d0d5e6

use decorators instead of macro, more compile-time checks

08936c4

clean up to_bytes

70cae6a

tests: rm redundant, clean up bincode gating

732d1e8

rustopian commented Nov 25, 2025

View reviewed changes

rustopian requested a review from febo November 25, 2025 15:36

rustopian marked this pull request as ready for review November 25, 2025 15:36

rustopian commented Nov 25, 2025

View reviewed changes

rm test util duplication

07e5fb1

rm from_raw_parts_mut ub

a359358

rustopian force-pushed the sysvar-get-packed-reprs branch from 6c827fd to a359358 Compare November 25, 2025 15:49

readd intentionally redundant arm

6070ca0

febo reviewed Nov 25, 2025

View reviewed changes

rustopian marked this pull request as draft November 25, 2025 18:31

rustopian changed the title ~~Use sol_get_sysvar instead of sol_get_<NAME>_sysvar, now with deserialization~~ Use sol_get_sysvar instead of sol_get_<NAME>_sysvar, fixed Nov 26, 2025

rustopian force-pushed the sysvar-get-packed-reprs branch 2 times, most recently from d558879 to e1ef624 Compare November 26, 2025 11:02

rework sysvars to byte arrays and accessors

201ffb1

rustopian force-pushed the sysvar-get-packed-reprs branch from b2c4dd2 to 201ffb1 Compare November 26, 2025 12:21

rustopian marked this pull request as ready for review November 26, 2025 12:31

rustopian requested a review from febo November 26, 2025 12:31

rm outdated comment lines

ff3c1f9

febo reviewed Dec 3, 2025

View reviewed changes

sysvar/src/epoch_schedule.rs Show resolved Hide resolved

valid bool values only

f5af464

Co-authored-by: Fernando Otero <[email protected]>

rustopian force-pushed the sysvar-get-packed-reprs branch from 53e6a87 to 021e240 Compare December 3, 2025 12:53

specifier, fmt

b36f96e

rustopian force-pushed the sysvar-get-packed-reprs branch from 021e240 to b36f96e Compare December 3, 2025 12:57

anza-xyz deleted a comment from github-actions bot Dec 3, 2025

rustopian requested a review from febo December 3, 2025 14:48

rustopian added 2 commits December 5, 2025 10:26

use bool directly, safety comments

e8ab6ac

allow depr in test

190e542

rustopian force-pushed the sysvar-get-packed-reprs branch from d710b1e to 190e542 Compare December 5, 2025 12:52

rustopian added 2 commits December 5, 2025 12:57

now unneeded TryFrom -> From

372c65b

rm now unused fetch_pod

4f49270

anza-xyz deleted a comment from github-actions bot Dec 5, 2025

Use sol_get_sysvar instead of sol_get_<NAME>_sysvar, fixed #458

Are you sure you want to change the base?

Use sol_get_sysvar instead of sol_get_<NAME>_sysvar, fixed #458

Uh oh!

Conversation

rustopian commented Nov 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Nov 25, 2025

Uh oh!

github-actions bot commented Nov 25, 2025

Uh oh!

github-actions bot commented Nov 25, 2025

Uh oh!

github-actions bot commented Nov 25, 2025

Uh oh!

rustopian Nov 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

febo Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

rustopian Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Nov 25, 2025

Uh oh!

github-actions bot commented Nov 25, 2025

Uh oh!

github-actions bot commented Nov 25, 2025

Uh oh!

github-actions bot commented Nov 25, 2025

Uh oh!

febo Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

rustopian Nov 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jamie-osec Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

rustopian Nov 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

febo Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

rustopian Nov 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

joncinque Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Dec 2, 2025

Uh oh!

LStan commented Dec 3, 2025

Uh oh!

rustopian commented Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

LStan commented Dec 3, 2025

Uh oh!

Uh oh!

febo commented Dec 3, 2025

Uh oh!

rustopian commented Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

LStan commented Dec 3, 2025

Uh oh!

febo commented Dec 4, 2025

Uh oh!

rustopian commented Dec 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustopian commented Nov 25, 2025 •

edited

Loading

rustopian Nov 25, 2025 •

edited

Loading

rustopian Nov 25, 2025 •

edited

Loading

rustopian Nov 26, 2025 •

edited

Loading

rustopian Nov 26, 2025 •

edited

Loading

rustopian commented Dec 3, 2025 •

edited

Loading

rustopian commented Dec 3, 2025 •

edited

Loading

rustopian commented Dec 5, 2025 •

edited

Loading