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

Remove SVM's dependency on vote crate #3671

Merged
merged 9 commits into from
Nov 20, 2024
Merged

Conversation

pgarg66
Copy link

@pgarg66 pgarg66 commented Nov 15, 2024

Problem

SVM code is currently dependent on solana-vote crate. This dependency can be removed, that'll make it easier to move SVM to its own repo.

Summary of Changes

The loader was using vote accounts HashMap to access stake for a given vote account. It didn't need any other information from the account. This change creates a HashMap of vote account pubkey and it's stake, and passes it to SVM. This helped remove the dependency on solana-vote crate.

Fixes #

Copy link

mergify bot commented Nov 15, 2024

The Firedancer team maintains a line-for-line reimplementation of the
native programs, and until native programs are moved to BPF, those
implementations must exactly match their Agave counterparts.
If this PR represents a change to a native program implementation (not
tests), please include a reviewer from the Firedancer team. And please
keep refactors to a minimum.

@pgarg66 pgarg66 force-pushed the remove-vote-dep branch 2 times, most recently from 37a4b02 to 9c35d71 Compare November 15, 2024 21:33
@pgarg66 pgarg66 marked this pull request as ready for review November 15, 2024 21:38
dmakarov
dmakarov previously approved these changes Nov 15, 2024
@pgarg66
Copy link
Author

pgarg66 commented Nov 15, 2024

Leaving the PR open for time being, in case there are any other comments.

Copy link

@buffalojoec buffalojoec left a comment

Choose a reason for hiding this comment

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

Looks great! Just a few comments from my side. Thanks for making this change!

programs/bpf_loader/src/syscalls/mod.rs Outdated Show resolved Hide resolved
program-runtime/src/invoke_context.rs Outdated Show resolved Hide resolved
svm/src/transaction_processor.rs Outdated Show resolved Hide resolved
program-runtime/src/invoke_context.rs Show resolved Hide resolved
dmakarov
dmakarov previously approved these changes Nov 18, 2024
Copy link

@buffalojoec buffalojoec left a comment

Choose a reason for hiding this comment

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

Works for me! Just some small suggestions, mostly around the naming convention.

runtime/src/bank.rs Outdated Show resolved Hide resolved
svm/doc/spec.md Outdated Show resolved Hide resolved
svm/src/transaction_processing_callback.rs Outdated Show resolved Hide resolved
program-runtime/src/invoke_context.rs Outdated Show resolved Hide resolved
program-runtime/src/invoke_context.rs Outdated Show resolved Hide resolved
programs/bpf_loader/src/syscalls/mod.rs Show resolved Hide resolved
Copy link

@buffalojoec buffalojoec left a comment

Choose a reason for hiding this comment

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

Looks great, thanks!

@pgarg66 pgarg66 merged commit af0ed22 into anza-xyz:master Nov 20, 2024
52 checks passed
@pgarg66 pgarg66 deleted the remove-vote-dep branch November 20, 2024 21:21
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