sdk: Extract solana-sysvar-id crate

[joncinque]

Problem

The sysvars in solana-program are tightly coupled with types that exist in solana-program. For example, all of the special sysvar getters like Rent::get() are implemented through a macro that falls back to using program_stubs.

Because of this tight coupling, it's difficult to pull out bits for the sysvars.

Summary of changes

After numerous attempts, I've decided to keep it simple and only extract SysvarId, its helper macros, and get_sysvar.

To go along with that, all of the separated sysvar crates now implement the sysvar ids themselves under a new sysvar feature. This new feature might be overkill, so let me know if we should just include the sysvar ids by default. I went with a feature to include an implementation using the sol_get_sysvar syscall in the future.

It was really messy to include the Sysvar trait from solana-program because it falls back to using bincode, which we know performs poorly for on-chain programs. So the future idea is to create a new Sysvar trait in solana-sysvar which will require fewer bits to deserialize sysvars.

Let me know what you think about this PR and the future idea! Note that I'll need to rebase this on top of #3249 and #3272 when they land.

[febo]

Maybe I missed something, but my current understanding is that we would still need to use both the sysvar specific crate + solana-program to get a sysvar. This is because impl_sysvar_get! still under solana-program.

[joncinque]

Maybe I missed something, but my current understanding is that we would still need to use both the sysvar specific crate + solana-program to get a sysvar. This is because impl_sysvar_get! still under solana-program.

That's correct. The Sysvar trait as it exists in solana-program requires bincode + serde, which shouldn't be a requirement for deserializing a sysvar. In future work, I'll add a new Sysvar trait that will avoid bincode + serde and use the sol_get_sysvar syscall under the hood, so we can deprecate the old one.

cc @buffalojoec since he's been looking at sol_get_sysvar, to make sure we're aligned

[kevinheavey]

The current Sysvar trait is used as a constraint in a few places in SDK code, for example:

pub fn create_account_with_fields<S: Sysvar>(

Changing this constraint would be a breaking change. So the current Sysvar trait would have to stick around for quite some time, so we'd still want to pull it out of solana-program

[joncinque]

Changing this constraint would be a breaking change. So the current Sysvar trait would have to stick around for quite some time, so we'd still want to pull it out of solana-program

Hm, yeah I see what you mean. In that case, I can change this to solana-sysvar-id just for SysvarId, and then create a separate solana-sysvar crate with the trait.

But then I'm not sure where the blocks like impl Sysvar for Rent blocks should go -- they need to be either in solana-sysvar or solana-rent, in that example.

Since the current Sysvar trait requires serde, bincode, and solana-account-info, I would lean towards keeping it all in solana-sysvar, but then solana-sysvar needs to pull in all of the sysvar crates like solana-rent. When I put it all in solana-rent, it blew up the requirements for the "sysvar" feature. Maybe that's ok in the short term, and we reduce that complexity with a breaking change to the Sysvar trait in the future. Any thoughts?

[kevinheavey]

I think until the next breaking change we'll have a solana-sysvar crate that is larger than we'd like, because it'll include all the impls of the trait. Also I have not confirmed this but I think it will need to include program_stubs.rs, because unlike other things that use program_stubs, Sysvar couples itself to program_stubs.

We can check if this is correct but basically I think program_stubs and Sysvar need to be in the same crate for program_stubs to work. So I would just move program_stubs to the new solana-sysvar crate unless you can think of a catchy name that conveys "sysvar and program_stubs"

[buffalojoec]

That's correct. The Sysvar trait as it exists in solana-program requires bincode + serde, which shouldn't be a requirement for deserializing a sysvar. In future work, I'll add a new Sysvar trait that will avoid bincode + serde and use the sol_get_sysvar syscall under the hood, so we can deprecate the old one.

cc @buffalojoec since he's been looking at sol_get_sysvar, to make sure we're aligned

Yep, this sounds great! This encompasses mine and @2501babe's plan for the others.

[2501babe]

yep, theres also a note in sdk/program/src/syscalls/definitions.rs that all the sysvar syscalls can be removed and replaced with get_sysvar when we are ready to make that change. we will also be able to remove 32kb from SysvarCache when the vote and stake programs switch to use get_sysvar

i havent looked at the code here yet but we should plan any new sysvar trait around the removal of the Fees sysvar. this one isnt supported by get_sysvar and has been deprecated since 1.9. its on my 3.0 wishlist

[joncinque]

We can check if this is correct but basically I think program_stubs and Sysvar need to be in the same crate for program_stubs to work. So I would just move program_stubs to the new solana-sysvar crate unless you can think of a catchy name that conveys "sysvar and program_stubs"

Gotcha, yeah I think that makes the most sense. I'll change this to solana-sysvar-id to only have the sysvar id trait then, and then do another for a big solana-sysvar crate.

Thanks to everyone for chiming in!

[joncinque]

This should be ready for another look

[mergify]

Backports to the beta branch are to be avoided unless absolutely necessary for fixing bugs, security issues, and perf regressions. Changes intended for backport should be structured such that a minimum effective diff can be committed separately from any refactoring, plumbing, cleanup, etc that are not strictly necessary to achieve the goal. Any of the latter should go only into master and ride the normal stabilization schedule. Exceptions include CI/metrics changes, CLI improvements and documentation updates on a case by case basis.

[febo]

Looks good!

[joncinque]

Ok great! I sent over solana-sysvar-id to anza-team, so once the crate check passes, we can merge this in. @yihau can you accept the ownership?