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

Add example of deserializing the SlotHash sysvar while remaining with… #3440

Closed
wants to merge 4 commits into from

Conversation

SUMEETRM
Copy link
Contributor

@SUMEETRM SUMEETRM commented Aug 8, 2022

Normal decoding of the SlotHash sysvar will exceed the current BPF compute limit. This PR demonstrates how to manually decode the sysvar to avoid this issue: mvines/solana-bpf-program-template#4

It has been added to fix issue #945 in the examples folder and can be located by examples/rust/sysvar/src/slothash.rs

@mergify mergify bot added the community Community contribution label Aug 8, 2022
Copy link
Contributor

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

Rather than adding a separate program in this file, how about adding the decoding to the existing process_instruction in processor.rs in this same directory? then you can merge this test with the functional test that already exists, just adding the slot hashes sysvar and file.

sysvar::slot_hashes::id(),
sol_to_lamports(1.),
Pubkey::default(),
"slot_hashes.bin",
Copy link
Contributor

Choose a reason for hiding this comment

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

We'll need to add this file somewhere in this repo so the test can pick it up. Typically we put it in tests/fixtures. Check out https://github.com/mvines/solana-bpf-program-template/pull/4/files#diff-d78b61a40495314ce899038afe34465a2619be35049c25c487854e970c07ba1e

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tried to fix it in my most recent commit but not sure if that's what needs to be done. Couldn't find the tests/fixtures directory in rust examples so I added slot_hashes.bin to the same directory as processor.rs

I've added the code from slothashes.rs to processor.rs and have changed the function name. Is this how it's supposed to be done?

Copy link
Contributor

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

I've clarified these points, let me know if you need anything else!

Don't forget to also run cargo fmt --all and cargo +nightly-2022-02-24 clippy

Comment on lines 48 to 55
//Code for deserializing the SlotHash sysvar while remaining within the BPF compute limit

//entrypoint!(process_instruction2);
fn process_instruction2(
_program_id: &Pubkey,
accounts: &[AccountInfo],
_instruction_data: &[u8],
) -> ProgramResult {
Copy link
Contributor

Choose a reason for hiding this comment

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

What I meant was to have all this code happen after line 43, in the normal process_instruction function. Written this way, the processor will never be triggered

};

#[tokio::test]
async fn test_transaction() {
Copy link
Contributor

Choose a reason for hiding this comment

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

The idea is to merge this test with the one in examples/rust/sysvar/tests/functional.rs since they exercise the same code underneath. You can change the transaction to also include the slot hashes sysvar and rest should work very nicely

sysvar::slot_hashes::id(),
sol_to_lamports(1.),
Pubkey::default(),
"slot_hashes.bin",
Copy link
Contributor

Choose a reason for hiding this comment

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

Once this test is moved to examples/rust/sysvar/tests/functional.rs, then you can move the slot hashes file to examples/rust/sysvar/tests/fixtures/slot_hashes.bin

Copy link
Contributor

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

I can't comment on it, but be sure to remove examples/rust/sysvar/src/slot_hashes.bin. Otherwise, just run cargo fmt and clippy, clean up those errors, and we should be good to merge!

};

/// Instruction processor
entrypoint(process_instruction);
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be removed since the entrypoint is already defined in entrypoint.rs. Also, it's a macro, so it should normally be called as entrypoint!(process_instruction)

@github-actions github-actions bot added the stale [bot only] Added to stale content; will be closed soon label Aug 10, 2023
@github-actions github-actions bot closed this Aug 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community Community contribution stale [bot only] Added to stale content; will be closed soon
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants