Skip to content

v3.1: feat(ledger-tool): fix ed25519 program testnet deployment (backport of #9352)#9373

Merged
t-nelson merged 1 commit intov3.1from
mergify/bp/v3.1/pr-9352
Dec 3, 2025
Merged

v3.1: feat(ledger-tool): fix ed25519 program testnet deployment (backport of #9352)#9373
t-nelson merged 1 commit intov3.1from
mergify/bp/v3.1/pr-9352

Conversation

@mergify
Copy link
Copy Markdown

@mergify mergify Bot commented Dec 2, 2025

Problem

the ed25519 precompile program deployment on testnet has been broken since forever. this breeds confusion and introduces unnecessary corner case bugs

Summary of Changes

rewrite it to the expected values


This is an automatic backport of pull request #9352 done by Mergify.

@mergify mergify Bot requested a review from a team as a code owner December 2, 2025 19:25
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.2%. Comparing base (f9896ad) to head (5506b5d).

Additional details and impacted files
@@            Coverage Diff            @@
##             v3.1    #9373     +/-   ##
=========================================
- Coverage    83.2%    83.2%   -0.1%     
=========================================
  Files         865      865             
  Lines      375946   375946             
=========================================
- Hits       313033   312975     -58     
- Misses      62913    62971     +58     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@bw-solana
Copy link
Copy Markdown

this isn't going to mess up any of the account hash calculations because it's off curve, right?

@t-nelson
Copy link
Copy Markdown

t-nelson commented Dec 2, 2025

this isn't going to mess up any of the account hash calculations because it's off curve, right?

don't think so. solana-test-validator was happy with the hard fork snapshot

Copy link
Copy Markdown

@brooksprumo brooksprumo left a comment

Choose a reason for hiding this comment

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

LGTM from the accounts lt hash/snapshots side of things. I'll defer to others on correctness of the account data/owner.

Comment thread ledger-tool/src/main.rs
Comment on lines +2335 to +2338
ed25519_program_account.set_owner(native_loader::id());
ed25519_program_account.set_data_from_slice(b"ed25519_program");

bank.store_account(&ed25519_program::id(), &ed25519_program_account);
Copy link
Copy Markdown

@brooksprumo brooksprumo Dec 2, 2025

Choose a reason for hiding this comment

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

This PR correctly sets child_bank_required (line 2187 above), so we're safe to make changes to accounts here. The child bank should compute the accounts lt hash correctly during bank freeze as part of taking the snapshot.

It looks like the existing account has no data (line 2325 above). So I would expect that we will need to also pass in --enable-capitalization-change as well, since iiuc the account data size will change on this account.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

you should be right, but it's a builtin owned by native loader so subject to that stupid 1-lamport rent-exempt reserve special-case

@t-nelson t-nelson merged commit e28ecc7 into v3.1 Dec 3, 2025
44 checks passed
@t-nelson t-nelson deleted the mergify/bp/v3.1/pr-9352 branch December 3, 2025 16:37
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.

5 participants