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: Slot Hashes: Add queries using sol_get_sysvar #1622

Merged
merged 10 commits into from
Jun 12, 2024

Conversation

buffalojoec
Copy link

@buffalojoec buffalojoec commented Jun 6, 2024

Problem

Some Core BPF programs - such as Address Lookup Table - will need to be able to query
for a slot's corresponding hash or position in the SlotHashes sysvar.

Thanks to #1307, we can leverage the new sol_get_sysvar to enable this
functionality for BPF programs.

Summary of Changes

First add a general helper API for working with sol_get_sysvar. This can be used by
many sysvar APIs.

Next add a new API - SlotHashesSysvar - offering two queries: get and position.

Example Usage

fn process_instruction(/* ... */) -> ProgramResult {
    if let Some(hash) = SlotHashesSysvar::get(&0)? {
        msg!("Found Hash: {:?}", hash.to_bytes());
    }

    if let Some(position) = SlotHashesSysvar::position(&312)? {
        msg!("Found Hash Position: {}", position);
    }

    Ok(())
}

A Note on Use of sol_get_sysvar

It turns out it was actually cheaper on compute to load the entire buffer for SlotHashes
without deserializing, and implement binary search on the raw data. Working with chunks
at a time was more expensive on compute.

@buffalojoec buffalojoec requested review from joncinque and 2501babe June 6, 2024 04:39
@buffalojoec
Copy link
Author

@2501babe I based this off of your implementation for StakeHistory, but I was able to
cut some corners by avoiding bincode, since Hash is just bytes.

I think we can also maybe share the first commit if you are interested, since our methods
both need to use the same call.

@buffalojoec
Copy link
Author

@joncinque @2501babe Requesting initial review as a draft before adding tests.

Copy link

@joncinque joncinque left a comment

Choose a reason for hiding this comment

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

Thanks for getting this up! It's good to see its potential usage

sdk/program/src/sysvar/mod.rs Outdated Show resolved Hide resolved
sdk/program/src/sysvar/mod.rs Outdated Show resolved Hide resolved
sdk/program/src/sysvar/slot_hashes.rs Outdated Show resolved Hide resolved
@buffalojoec buffalojoec force-pushed the get-sysvar-slot-hashes-2 branch from 031ac38 to f904e15 Compare June 6, 2024 22:20
@buffalojoec buffalojoec marked this pull request as ready for review June 6, 2024 22:20
Copy link

@joncinque joncinque left a comment

Choose a reason for hiding this comment

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

This is starting to look very nice, just a few more bits to refine it

sdk/program/src/sysvar/mod.rs Outdated Show resolved Hide resolved
sdk/program/src/sysvar/slot_hashes.rs Outdated Show resolved Hide resolved
sdk/program/src/sysvar/slot_hashes.rs Outdated Show resolved Hide resolved
@buffalojoec buffalojoec force-pushed the get-sysvar-slot-hashes-2 branch from e4258b7 to aba85df Compare June 11, 2024 18:38
Copy link

@joncinque joncinque left a comment

Choose a reason for hiding this comment

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

Just two last nits, then this is good to go! I'm a big fan of how it's turned out

sdk/program/src/sysvar/slot_hashes.rs Outdated Show resolved Hide resolved
sdk/program/src/sysvar/slot_hashes.rs Outdated Show resolved Hide resolved
Copy link

@joncinque joncinque left a comment

Choose a reason for hiding this comment

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

hash

@buffalojoec buffalojoec added the automerge automerge Merge this Pull Request automatically once CI passes label Jun 12, 2024
@mergify mergify bot merged commit a6035da into anza-xyz:master Jun 12, 2024
51 checks passed
@2501babe
Copy link
Member

late to the party while i close out the new stake instructions but i love this, im going to rearrange my stake history stuff to also use your get_sysvar function

samkim-crypto pushed a commit to samkim-crypto/agave that referenced this pull request Jul 31, 2024
* sysvars: add `get_sysvar` handler

* sysvars: add `SlotHashesSysvar` trait

* use bytemuck slices for binary search

* refactor get_sysvar API

* add tests

* add pointer alignment check

* make the API a struct

* move allocation check

* use `PodSlotHash` for layout

* cleanup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge automerge Merge this Pull Request automatically once CI passes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants