Skip to content

misc crates: drop SysvarSerialize - #14284

Merged
grod220 merged 5 commits into
anza-xyz:masterfrom
grod220:serialize-direct-cont
Aug 5, 2026
Merged

misc crates: drop SysvarSerialize#14284
grod220 merged 5 commits into
anza-xyz:masterfrom
grod220:serialize-direct-cont

Conversation

@grod220

@grod220 grod220 commented Aug 3, 2026

Copy link
Copy Markdown

Removes the last bit of direct SysvarSerialize usage. Next is remaining test helpers.

Continuing to break down #12245. Progresses #10672.

@grod220
grod220 requested a review from a team as a code owner August 3, 2026 10:55
{
let mut account = AccountSharedData::new(1, solana_epoch_schedule::SIZE, &sysvar::id());
bincode::serialize_into(account.data_as_mut_slice(), &EpochSchedule::default()).unwrap();
wincode::serialize_into(account.data_as_mut_slice(), &EpochSchedule::default()).unwrap();

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Follow up from #14246 (comment)

Comment on lines -117 to -124
// Slot History
// (Not fixed-size, but also not supported)
msg!("SlotHistory identifier:");
sysvar::slot_history::id().log();
assert_eq!(
Err(ProgramError::UnsupportedSysvar),
SlotHistory::from_account_info(&accounts[8])
);

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Not something we can assert anymore as SlotHistory::from_account_info() isn't used anymore. But it anyway just existed to test that SysvarSerialize method couldn't be used for this sysvar.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The purpose of this test is to demonstrate that the syscall doesn't support SlotHistory, not the API:

// this is exposed for SyscallGetSysvar and should not otherwise be used
pub fn sysvar_id_to_buffer(&self, sysvar_id: &Pubkey) -> &Option<Vec<u8>> {
if Clock::check_id(sysvar_id) {
&self.clock
} else if EpochSchedule::check_id(sysvar_id) {
&self.epoch_schedule
} else if EpochRewards::check_id(sysvar_id) {
&self.epoch_rewards
} else if Rent::check_id(sysvar_id) {
&self.rent
} else if SlotHashes::check_id(sysvar_id) {
&self.slot_hashes
} else if StakeHistory::check_id(sysvar_id) {
&self.stake_history
} else if LastRestartSlot::check_id(sysvar_id) {
&self.last_restart_slot
} else {
&None
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The intent of the test is to make sure that using the syscalls is the same as deserializing from the account passed in.

In that spirit, it would be good to compare some slice of the sysvar account with the corresponding slice from the result of the syscall.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

The rest of the assertions in this section are comparing deserialized account sysvars are equivalent to the syscall values. We can't quite do that cleanly for SlotHistory, but I can manually inspect some account bytes as suggested and also assert a failure in the syscall.

Comment thread programs/sbf/Cargo.toml
solana-svm-type-overrides = { path = "../../svm-type-overrides", version = "=4.3.0-alpha.2" }
solana-system-interface = { version = "=3.2", features = ["bincode"] }
solana-sysvar = "=4.1.0"
solana-sysvar-id = "=3.1.0"

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Sdk libraries seems to require = here

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

They're just tests, so it's not a huge deal. We could probably relax all those deps in a future PR

Comment thread programs/sbf/Cargo.toml
solana-vote-program = { path = "../../programs/vote", version = "=4.3.0-alpha.2" }
test-case = "3.3.1"
thiserror = "2.0"
wincode = { version = "0.5.5", default-features = false }

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

default-features = false to avoid bringing in std given this is programs context

@grod220
grod220 force-pushed the serialize-direct-cont branch 2 times, most recently from a6430ae to ca002d8 Compare August 3, 2026 11:13
@codecov-commenter

codecov-commenter commented Aug 3, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 12.50000% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.9%. Comparing base (6bd615d) to head (62d5bb5).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #14284   +/-   ##
=======================================
  Coverage    81.9%    81.9%           
=======================================
  Files         914      914           
  Lines      356973   356976    +3     
=======================================
+ Hits       292617   292626    +9     
+ Misses      64356    64350    -6     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread ledger-tool/Cargo.toml Outdated
solana-core = { workspace = true, features = ["dev-context-only-utils"] }
solana-cost-model = { workspace = true }
solana-entry = { workspace = true }
solana-epoch-schedule = { version = "3.2.0", features = ["wincode"] }

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Why not use the workspace version?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

True. It's workspace is the dev-bins. Moved.

Comment on lines -117 to -124
// Slot History
// (Not fixed-size, but also not supported)
msg!("SlotHistory identifier:");
sysvar::slot_history::id().log();
assert_eq!(
Err(ProgramError::UnsupportedSysvar),
SlotHistory::from_account_info(&accounts[8])
);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The purpose of this test is to demonstrate that the syscall doesn't support SlotHistory, not the API:

// this is exposed for SyscallGetSysvar and should not otherwise be used
pub fn sysvar_id_to_buffer(&self, sysvar_id: &Pubkey) -> &Option<Vec<u8>> {
if Clock::check_id(sysvar_id) {
&self.clock
} else if EpochSchedule::check_id(sysvar_id) {
&self.epoch_schedule
} else if EpochRewards::check_id(sysvar_id) {
&self.epoch_rewards
} else if Rent::check_id(sysvar_id) {
&self.rent
} else if SlotHashes::check_id(sysvar_id) {
&self.slot_hashes
} else if StakeHistory::check_id(sysvar_id) {
&self.stake_history
} else if LastRestartSlot::check_id(sysvar_id) {
&self.last_restart_slot
} else {
&None
}
}

Comment thread programs/sbf/rust/sysvar/src/lib.rs

@joncinque joncinque left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Looks good to me with Joe's comments resolved!

Comment thread programs/sbf/Cargo.toml
solana-svm-type-overrides = { path = "../../svm-type-overrides", version = "=4.3.0-alpha.2" }
solana-system-interface = { version = "=3.2", features = ["bincode"] }
solana-sysvar = "=4.1.0"
solana-sysvar-id = "=3.1.0"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

They're just tests, so it's not a huge deal. We could probably relax all those deps in a future PR

Comment on lines -117 to -124
// Slot History
// (Not fixed-size, but also not supported)
msg!("SlotHistory identifier:");
sysvar::slot_history::id().log();
assert_eq!(
Err(ProgramError::UnsupportedSysvar),
SlotHistory::from_account_info(&accounts[8])
);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The intent of the test is to make sure that using the syscalls is the same as deserializing from the account passed in.

In that spirit, it would be good to compare some slice of the sysvar account with the corresponding slice from the result of the syscall.

@grod220
grod220 force-pushed the serialize-direct-cont branch from ca002d8 to 5dc6572 Compare August 5, 2026 09:34

@buffalojoec buffalojoec left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Works for me, thanks!

@grod220
grod220 added this pull request to the merge queue Aug 5, 2026
Merged via the queue into anza-xyz:master with commit 47c6082 Aug 5, 2026
50 checks passed
@grod220
grod220 deleted the serialize-direct-cont branch August 5, 2026 12:59
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