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

Replace solana_program::stake with solana_stake_interface #4664

Merged
merged 34 commits into from
Jan 31, 2025

Conversation

kevinheavey
Copy link

@kevinheavey kevinheavey commented Jan 28, 2025

Closes #4529

Changes:

  • re-export the contents of solana-stake-interface to replace what's in solana_program::stake
  • Update a bunch of patches for CI
  • Use small crates in cargo-build-sbf tests because this was easier than applying all the patches needed for those test crates

Copy link

mergify bot commented Jan 28, 2025

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.

@kevinheavey kevinheavey force-pushed the stake-interface-replace branch from 04cf92a to 2065824 Compare January 29, 2025 07:59
@kevinheavey kevinheavey changed the title Replace solana_program::stake_interface with solana_stake_interface Replace solana_program::stake with solana_stake_interface Jan 30, 2025
@kevinheavey kevinheavey marked this pull request as ready for review January 30, 2025 16:00
@kevinheavey kevinheavey requested a review from a team as a code owner January 30, 2025 16:00
@kevinheavey kevinheavey requested a review from joncinque January 30, 2025 16:01
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.

Looks great, thanks for fighting through this! Just a couple of small comments

Comment on lines +13 to +16
solana-account-info= { path = "../../../../../sdk/account-info", version = "=2.2.0" }
solana-program-entrypoint = { path = "../../../../../sdk/program-entrypoint", version = "=2.2.0" }
solana-program-error= { path = "../../../../../sdk/program-error", version = "=2.2.0" }
solana-pubkey = { path = "../../../../../sdk/pubkey", version = "=2.2.0" }

Choose a reason for hiding this comment

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

Nice, this is so much simpler, sorry I didn't think of it earlier.

@@ -34,6 +34,7 @@ tempfile = { workspace = true }

[dev-dependencies]
solana-runtime = { workspace = true, features = ["dev-context-only-utils"] }
solana-stake-interface = { workspace = true, features = ["borsh"] }

Choose a reason for hiding this comment

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

Sorry, it's not clear from this PR or the code in genesis -- why is this dependency needed?

"dep:solana-frozen-abi",
"dep:solana-frozen-abi-macro",
"solana-stake-interface/frozen-abi"
]
serde = [

Choose a reason for hiding this comment

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

Can we enable the serde feature on solana-stake-interface if the serde feature on this crate is enabled?

@kevinheavey kevinheavey force-pushed the stake-interface-replace branch from 7c49055 to be32f23 Compare January 31, 2025 09:00
@kevinheavey kevinheavey added the automerge automerge Merge this Pull Request automatically once CI passes label Jan 31, 2025
@mergify mergify bot merged commit 989eb2f into anza-xyz:master Jan 31, 2025
61 checks passed
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 need:merge-assist
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants