Skip to content

replace solana-inline-spl with spl-generic-token #5805

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

Draft
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

2501babe
Copy link
Member

Problem

solana-inline-spl is used to provide very minimal token parsing functionality for agave crates that cannot depend on spl-token and spl-token-2022. we need to use this functionality in svm, but the svm crate is being removed to its own repo. thus this functionality cannot live in either repo

in addition solana-inline-spl is outdated compared to the copy-pasted version of the same code in spl-token-2022. by localizing this functionality in one repo we can reduce duplication and ensure bugfixes happen in one place

Summary of Changes

remove solana-inline-spl and replace it with spl-generic-token. this new crate has substantially improved functionality, for details see solana-program/libraries#70

there may be opportunities to refactor some code in agave to eliminate some usage of spl-token and spl-token-2022 entirely, given the enhanced functionality. this pr, however, merely replaces all uses of solana-inline-spl with spl-generic-token as a drop-in replacement, because its purpose is to unblock #5588 which will unblock #4253 and finally enable simd83

i take it for granted that removing solana-inline-spl in this way merely prevents new releases from ever being made, and anyone using it third party (although they shouldnt do that because its an internal crate) will not be broken until we take agave to 3.0. but worth pinging releng

NOTE this should not be merged until we do a thorough check on https://github.com/solana-program/libraries repo permissions. at the very least i need to enable mandatory code review, but there may be other boxes to tick like push to master etc

Copy link

mergify bot commented Apr 14, 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/web3.js (example)

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

Comment on lines 8 to 11
pub static ref STATIC_IDS: Vec<Pubkey> = vec![
associated_token_account::id(),
associated_token_account::program_v1_1_0::id(),
token::id(),
token::native_mint::id(),
Copy link
Member Author

@2501babe 2501babe Apr 14, 2025

Choose a reason for hiding this comment

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

this is a safe change because the only place this static is used is a for loop for snapshot size reduction, and the old ata program account does not exist on mainnet. we removed this id and the old token program id from spl-generic-token

Comment on lines +18 to +24
pub use {
solana_account_decoder_client_types::token::{
real_number_string, real_number_string_trimmed, TokenAccountType, UiAccountState, UiMint,
UiMultisig, UiTokenAccount, UiTokenAmount,
},
spl_generic_token::{is_known_spl_token_id, spl_token_ids},
};
Copy link
Member Author

Choose a reason for hiding this comment

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

we pub use these to avoid a breaking change because solana-account-decoder is a crate that (unlike solana-inline-spl) is very reasonable for downstream projects to include

@2501babe
Copy link
Member Author

hahaha... nevermind, maybe. i think the test failures are because of the bug i fixed in spl-generic-token. this could, although it is highly unlikely, have consensus implications. my goal is to unblock simd83, this can wait. when im back at my computer i am going to use spl-generic-token in the balance collector pr and worry about replacing solana-inline-spl after simd83

cc @apfitzge for status update (we are almost out of the woods now!)

@2501babe 2501babe force-pushed the 20250414_rminline branch from d8902dd to 3d0b8a6 Compare April 18, 2025 19:07
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.9%. Comparing base (44bd7c7) to head (2ab82be).
Report is 16 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##           master    #5805     +/-   ##
=========================================
- Coverage    83.0%    82.9%   -0.1%     
=========================================
  Files         828      826      -2     
  Lines      375869   376464    +595     
=========================================
+ Hits       312178   312438    +260     
- Misses      63691    64026    +335     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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.

2 participants