Skip to content

SIMD-0266: Efficient Token program#6630

Closed
febo wants to merge 5 commits into
anza-xyz:masterfrom
febo:ptoken-feature-gate
Closed

SIMD-0266: Efficient Token program#6630
febo wants to merge 5 commits into
anza-xyz:masterfrom
febo:ptoken-feature-gate

Conversation

@febo
Copy link
Copy Markdown

@febo febo commented Jun 17, 2025

Problem

SIMD-0266 proposes to replace the current verion of SPL Token with p-token.

Summary of Changes

Add a feature to agave-feature-set and its activation on bank.rs, which uses the upgrade_core_bpf_program to replace the SPL Token program data.

@febo febo requested review from buffalojoec and joncinque June 18, 2025 09:52
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.

Woohoo!

Comment thread feature-set/src/lib.rs Outdated
Comment thread feature-set/src/lib.rs Outdated
Comment thread feature-set/src/lib.rs
Comment thread runtime/src/bank.rs Outdated
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Jun 18, 2025

Codecov Report

Attention: Patch coverage is 42.85714% with 8 lines in your changes missing coverage. Please review.

Project coverage is 82.8%. Comparing base (3281f61) to head (13c03ec).
Report is 6 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##           master    #6630     +/-   ##
=========================================
- Coverage    82.8%    82.8%   -0.1%     
=========================================
  Files         849      849             
  Lines      379373   379387     +14     
=========================================
- Hits       314300   314280     -20     
- Misses      65073    65107     +34     
🚀 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 Jun 18, 2025
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.

Ship it, but not until after solana-foundation/solana-improvement-documents#266 is accepted and merged.

@buffalojoec buffalojoec dismissed their stale review June 18, 2025 12:34

Dismissing until SIMD is merged.

Comment thread runtime/src/bank.rs
Comment on lines +6617 to +6621
if let Err(e) = self.upgrade_core_bpf_program(
&agave_feature_set::replace_spl_token_with_p_token::SPL_TOKEN_PROGRAM_ID,
&agave_feature_set::replace_spl_token_with_p_token::PTOKEN_PROGRAM_BUFFER,
"replace_spl_token_with_p_token",
) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I looked into this code, and we're going to have some problems. SPL Token isn't owned by the upgradeable loader, so we're going to fail most of the checks here:

during upgrade_core_bpf_program.

We might need to create a new target for non-upgradeable programs, so that we update the program account to be a proper loader-v3 account.

Be sure to run an end-to-end test against a solana-test-validator built with this change to make sure that enabling the feature does the right thing.

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.

Good point! I think we need a new target – I will create a separate PR for it.

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.

Actually, we said that we would use Loader v4.

Copy link
Copy Markdown

@buffalojoec buffalojoec Jun 20, 2025

Choose a reason for hiding this comment

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

Yeah this is a good call. I forgot this mechanism only supports loader v3. 😅

What you'll actually have to do is refactor the migration module a bit to go from a source-target design to a source-target-result design.

Source(ProgramV2 | ProgramV3 | ProgramV4 | Buffer),
Target(ProgramV2 | ProgramV3 | ProgramV4),
Result(ProgramV2 | ProgramV3 | ProgramV4 | null),

It could end up being a bit of a slog. Lmk if you want my help!

@febo
Copy link
Copy Markdown
Author

febo commented Jul 23, 2025

Closed in favour of #7125

@febo febo closed this Jul 23, 2025
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