Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sdk: Extract slot-history sysvar #3249

Merged
merged 6 commits into from
Oct 28, 2024

Conversation

joncinque
Copy link

Problem

Just about all of the sysvars exist in separate crates, except for slot history.

Summary of changes

Extract it into a new crate. Looks pretty similar to slot hashes. All of the info! calls and logging dependencies were removed.

@joncinque joncinque requested review from febo and kevinheavey October 21, 2024 19:51
Comment on lines 5 to 7
//! The sysvar ID is declared in [`sysvar`].
//!
//! [`sysvar`]: crate::sysvar
Copy link
Author

Choose a reason for hiding this comment

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

This will get done in future work, once all the sysvar types are extracted

Choose a reason for hiding this comment

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

Let's update the doc link?

Copy link
Author

Choose a reason for hiding this comment

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

Sure, done

#### Problem

Just about all of the sysvars exist in separate crates, except for slot
history.

#### Summary of changes

Extract it into a new crate. Looks pretty similar to slot hashes, except
all of the `info!` calls were removed in the tests.
@joncinque joncinque force-pushed the extract-slot-history branch from 7c61908 to 99035a3 Compare October 22, 2024 10:50
//!
//! The sysvar ID is declared in [`sysvar::slot_history`].
//!
//! [`sysvar`]: https://docs.rs/solana-program/latest/solana_program/sysvar/slot_history

Choose a reason for hiding this comment

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

this should be [`sysvar::slot_history`]

Copy link
Author

Choose a reason for hiding this comment

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

oops, that's right, thanks

assert_eq!(slot_history.oldest(), 1);
}
}
pub use {solana_clock::Slot, solana_slot_history::*};
Copy link

Choose a reason for hiding this comment

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

Should we add the deprecated message on this re-export?

Copy link
Author

Choose a reason for hiding this comment

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

Done!

@febo
Copy link

febo commented Oct 23, 2024

Looks good - I think we are only missing the crate owner for CI.

@joncinque
Copy link
Author

@yihau can you accept the ownership of solana-slot-history?

@yihau
Copy link
Member

yihau commented Oct 27, 2024

@joncinque
Copy link
Author

@febo can I get an approval to merge this?

@joncinque joncinque merged commit 0f7a4c7 into anza-xyz:master Oct 28, 2024
52 checks passed
@joncinque joncinque deleted the extract-slot-history branch October 28, 2024 16:08
joncinque added a commit that referenced this pull request Oct 29, 2024
* sdk: Extract `solana-sysvar` crate

#### 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.

* Add solana-define-syscall for solana builds

* Rename solana-sysvar -> solana-sysvar-id

* Move back `get_sysvar` to solana-program

* Update lockfile for v2.2
mergify bot pushed a commit that referenced this pull request Oct 29, 2024
* sdk: Extract `solana-sysvar` crate

#### 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.

* Add solana-define-syscall for solana builds

* Rename solana-sysvar -> solana-sysvar-id

* Move back `get_sysvar` to solana-program

* Update lockfile for v2.2

(cherry picked from commit 838c195)

# Conflicts:
#	Cargo.toml
joncinque added a commit that referenced this pull request Oct 30, 2024
* sdk: Extract `solana-sysvar` crate

#### 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.

* Add solana-define-syscall for solana builds

* Rename solana-sysvar -> solana-sysvar-id

* Move back `get_sysvar` to solana-program

* Update lockfile for v2.2

(cherry picked from commit 838c195)

# Conflicts:
#	Cargo.toml
joncinque added a commit that referenced this pull request Oct 30, 2024
sdk: Extract `solana-sysvar-id` crate (#3309)

* sdk: Extract `solana-sysvar` crate

#### 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.

* Add solana-define-syscall for solana builds

* Rename solana-sysvar -> solana-sysvar-id

* Move back `get_sysvar` to solana-program

* Update lockfile for v2.2

(cherry picked from commit 838c195)

# Conflicts:
#	Cargo.toml

Co-authored-by: Jon C <me@jonc.dev>
ray-kast pushed a commit to abklabs/agave that referenced this pull request Nov 27, 2024
* sdk: Extract slot-history sysvar

#### Problem

Just about all of the sysvars exist in separate crates, except for slot
history.

#### Summary of changes

Extract it into a new crate. Looks pretty similar to slot hashes, except
all of the `info!` calls were removed in the tests.

* Remove clock dependency

* Update doc link

* Fix rebase errors

* Update doc link (again)

* Add deprecation warning
ray-kast pushed a commit to abklabs/agave that referenced this pull request Nov 27, 2024
* sdk: Extract `solana-sysvar` crate

#### 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 anza-xyz#3249 and anza-xyz#3272 when they land.

* Add solana-define-syscall for solana builds

* Rename solana-sysvar -> solana-sysvar-id

* Move back `get_sysvar` to solana-program

* Update lockfile for v2.2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants