Skip to content

deps: Bump all sdk and spl crates to v3 compatibility#7492

Merged
joncinque merged 30 commits intoanza-xyz:masterfrom
joncinque:bump-sdk
Aug 14, 2025
Merged

deps: Bump all sdk and spl crates to v3 compatibility#7492
joncinque merged 30 commits intoanza-xyz:masterfrom
joncinque:bump-sdk

Conversation

@joncinque
Copy link
Copy Markdown

@joncinque joncinque commented Aug 13, 2025

Problem

Agave is about to cut its breaking v3 branch, but it's still on sdk v2 crates.

Summary of Changes

It was a bit tough to break it up logically, but here's a breakdown of the commits:

  • cb21aeb / b5e286c / f772d4a / b2c2792 / 47641ee: dependency bumps, not too much to see here
  • 7f4ab2d / 8dea3a1: programs/sbf changes
  • d3957ed: simpler renames of existing functions
  • 91c3a03: IMPORTANT! This commit has more touchy changes based on the breaking changes to AccountInfo
  • 50ec07c: gossip updates. These were a little touchier because they used the removed Keypair::generate
  • All of the commits afterwards are mostly dealing with the removal of sol_to_lamports and lamports_to_sol

Overall breakdown of changes:

  • Rename VoteState -> VoteStateV3
  • Rename convert_to_current -> convert_to_v3
  • Rename new_current -> new_v3
  • Since PubkeyError no longer implements FromPrimitive, we had to do some conversions by hand. Note! This showed that we've been doing an incorrect conversion from PubkeyError to InstructionError by just converting from one number to another, but I simply kept the existing behavior.
  • Rename Sysvar -> SysvarSerialize (in most places)
  • Import StakeHistory from solana_stake_interface instead of solana_sysvar
  • Use Hash::as_bytes() instead of accessing the bytes directly, since they were made private
  • Use AccountInfo::_unused instead of rent_epoch
  • Change BorshIoError(String) -> BorshIoError
  • Use solana-feature-gate-interface instead of solana-feature-gate-program-client
  • Use solana-config-interface instead of solana-config-program-client
  • Import ClusterType from solana-cluster-type instead of solana-genesis
  • Mark all onchain memory operations usage as unsafe
  • Use Keypair::try_from instead of Keypair::from_bytes
  • Use hashers explicitly for on-chain program tests

This is the last important step of https://github.com/orgs/anza-xyz/projects/27/views/1 !

@joncinque joncinque requested a review from a team as a code owner August 13, 2025 12:06
@mergify
Copy link
Copy Markdown

mergify Bot commented Aug 13, 2025

If this PR represents a change to the public RPC API:

  1. Make sure it includes a complementary update to rpc-client/ (example)
  2. Open a follow-up PR to update the JavaScript client @solana/kit (example)

Thank you for keeping the RPC clients in sync with the server API @joncinque.

@mergify
Copy link
Copy Markdown

mergify Bot commented Aug 13, 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.

@mergify
Copy link
Copy Markdown

mergify Bot commented Aug 13, 2025

For your information, the zk-keygen and zk-sdk directories are
scheduled to be relocated to solana-program/zk-elgamal-proof in a
separate repository. Additionally, the zk-token-sdk directory will
be removed. Please take these upcoming changes into account when
making modifications.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@Lichtso Can you take a close look at the changes here?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@Lichtso Can you check this to make sure it's ok?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@Lichtso Please check this too to make sure you're ok with the change

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@buffalojoec These changes are important, so please double-check me here

Copy link
Copy Markdown

@buffalojoec buffalojoec Aug 13, 2025

Choose a reason for hiding this comment

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

That's funny because we did it "right" with ProgramError.
https://github.com/anza-xyz/solana-sdk/blob/252b59c04c6ad0650e1b203099af56b47aee08b8/program-error/src/lib.rs#L366-L374

The backwards compatibility change looks good to me, though.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Yeah I was pretty surprised about that too 😅 thanks!

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@buffalojoec The error change here is important, so please double check

Comment thread syscalls/src/cpi.rs
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@Lichtso can you look at these changes? They should be fine, but I want to be safe

@joncinque
Copy link
Copy Markdown
Author

To help with the review, the old code for converting from PubkeyError to a primitive can be found at https://github.com/anza-xyz/solana-sdk/blob/maintenance/v2.x/pubkey/src/lib.rs

.map(|seeds| Pubkey::create_program_address(seeds, caller_program_id))
.collect::<Result<Vec<Pubkey>, solana_pubkey::PubkeyError>>()?;
.collect::<Result<Vec<Pubkey>, solana_pubkey::PubkeyError>>()
.map_err(|e| e as u64)?;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Where was this conversion originally defined? I assume there was an InstructionError::From<PubkeyError>.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

assert_eq!(accounts[ARGUMENT_INDEX].data_len(), 100);
assert!(accounts[ARGUMENT_INDEX].is_signer);
assert!(accounts[ARGUMENT_INDEX].is_writable);
assert_eq!(accounts[ARGUMENT_INDEX].rent_epoch, u64::MAX);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

We still must ensure that there is a deterministic value here, even if it is unused.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Is it ok if that's 0? Because the entrypoint no longer deserializes the rent epoch at all

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Alright, that check here is useless.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

The tests on the deprecated entrypoint continue to deserialize and check that the rent epoch is u64::MAX, if that helps

Copy link
Copy Markdown

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

Changes lgtm! Just waiting for the CI slog to add my stamp.

P. S. Thanks for taking the vote-interface plumbing off my hands!

Comment thread runtime/src/bank_client.rs Outdated
Comment thread syscalls/src/sysvar.rs Outdated
@joncinque joncinque force-pushed the bump-sdk branch 2 times, most recently from a06ad39 to c8578c9 Compare August 13, 2025 20:30
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Aug 13, 2025

Codecov Report

❌ Patch coverage is 85.48644% with 91 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.3%. Comparing base (b834292) to head (447baba).

Additional details and impacted files
@@            Coverage Diff            @@
##           master    #7492     +/-   ##
=========================================
- Coverage    83.3%    83.3%   -0.1%     
=========================================
  Files         811      811             
  Lines      364853   364984    +131     
=========================================
+ Hits       304174   304241     +67     
- Misses      60679    60743     +64     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

buffalojoec
buffalojoec previously approved these changes Aug 14, 2025
rustopian
rustopian previously approved these changes Aug 14, 2025
Copy link
Copy Markdown

@rustopian rustopian left a comment

Choose a reason for hiding this comment

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

Combed through. Sweeping but consistent changes, as advertised. All look good to me (pending fix of conflicts etc)

* VoteState -> VoteStateV3
* VoteStateVersions::convert_to_current -> convert_to_v3
* Sysvar -> SysvarSerialize
* Import StakeHistory from solana_stake_interface
* BorshIoError(String) -> BorshIoError
* Import ClusterType from solana_cluster_tpe
* Keypair::from_bytes -> Keypair::try_from
* Hash.0 -> Hash::as_bytes
AccountInfo renamed `rent_epoch` to `_unused` and the entrypoint no
longer deserializes the field at all, so make sure this won't cause any
divergence in the runtime.
@joncinque joncinque added the automerge automerge Merge this Pull Request automatically once CI passes label Aug 14, 2025
@joncinque joncinque merged commit 06464c5 into anza-xyz:master Aug 14, 2025
52 checks passed
@joncinque joncinque deleted the bump-sdk branch August 14, 2025 16:08
Comment thread runtime/src/bank.rs
#[cfg_attr(
feature = "frozen-abi",
frozen_abi(digest = "5dfDCRGWPV7thfoZtLpTJAV8cC93vQUXgTm6BnrfeUsN")
frozen_abi(digest = "2mR2EKFguLhheKtDzbFxoQonSmUtM9svd8kkgeKpe2vu")
Copy link
Copy Markdown

@brooksprumo brooksprumo Aug 15, 2025

Choose a reason for hiding this comment

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

Looks like this was an ABI-breaking change to the serialization of the status cache (which breaks snapshots)

❯ diff -u bank__BankSlotDelta_frozen_abi__test_abi_digest_5dfDCRGWPV7thfoZtLpTJAV8cC93vQUXgTm6BnrfeUsN bank__BankSlotDelta_frozen_abi__test_abi_digest_2mR2EKFguLhheKtDzbFxoQonSmUtM9svd8kkgeKpe2vu
--- bank__BankSlotDelta_frozen_abi__test_abi_digest_5dfDCRGWPV7thfoZtLpTJAV8cC93vQUXgTm6BnrfeUsN	2025-08-15 10:38:42
+++ bank__BankSlotDelta_frozen_abi__test_abi_digest_2mR2EKFguLhheKtDzbFxoQonSmUtM9svd8kkgeKpe2vu	2025-08-15 10:53:57
@@ -139,7 +139,7 @@
                                                     variant(8) InstructionError (fields = 2)
                                                         field u8
                                                             primitive u8
-                                                        field solana_instruction::error::InstructionError
+                                                        field solana_instruction_error::InstructionError
                                                             enum InstructionError (variants = 54)
                                                                 variant(0) GenericError (unit)
                                                                 variant(1) InvalidArgument (unit)
@@ -186,8 +186,7 @@
                                                                 variant(41) ProgramFailedToCompile (unit)
                                                                 variant(42) Immutable (unit)
                                                                 variant(43) IncorrectAuthority (unit)
-                                                                variant(44) BorshIoError(alloc::string::String) (newtype)
-                                                                    primitive str
+                                                                variant(44) BorshIoError (unit)
                                                                 variant(45) AccountNotRentExempt (unit)
                                                                 variant(46) InvalidAccountOwner (unit)
                                                                 variant(47) ArithmeticOverflow (unit)

Notice that the BorshIoError variant used to be a tuple struct with a String, and now is a unit type.

More discussion on Discord here: https://discord.com/channels/428295358100013066/1027231858565586985/1405926131038683259

steviez pushed a commit that referenced this pull request Aug 16, 2025
joncinque added a commit that referenced this pull request Aug 20, 2025
#7592)

* Reapply "deps: Bump all sdk and spl crates to v3 compatibility (#7492)" (#7556)

This reverts commit f149dec.

* Update gossip test hash and random key generation
ksn6 added a commit to anza-xyz/alpenglow that referenced this pull request Aug 28, 2025
Just a single commit that updates us to 3.0.0:
anza-xyz/agave#7492

---------

Co-authored-by: Jon C <me@jonc.dev>
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.

6 participants