Skip to content

Feature - Write lock demotion exemption for loader-v4#4575

Closed
Lichtso wants to merge 1 commit into
anza-xyz:masterfrom
Lichtso:feature/loader_v4_write_lock_demotion_exemption
Closed

Feature - Write lock demotion exemption for loader-v4#4575
Lichtso wants to merge 1 commit into
anza-xyz:masterfrom
Lichtso:feature/loader_v4_write_lock_demotion_exemption

Conversation

@Lichtso
Copy link
Copy Markdown

@Lichtso Lichtso commented Jan 22, 2025

Problem

Split off from #2796.

Summary of Changes

Wires a enable_loader_v4 parameter through all Message constructors which then ends up in is_upgradeable_loader_present(). The parameter is a bool and not &FeatureSet on purpose so that the feature check can be hoisted outside of loops at the call sites. All consensus irrelevant parts (such as tests, benchmarks and some tools) have it hard coded to true.

@Lichtso Lichtso requested review from a team as code owners January 22, 2025 10:49
@mergify
Copy link
Copy Markdown

mergify Bot commented Jan 22, 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 @Lichtso.

Comment thread sdk/message/src/sanitized.rs Outdated
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

this crate follows semver so this will probably have to wait until SDK crates are moved to a new repo and given a new release schedule

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.

Won't we have a new version bump before then? Also, we could deprecate the constructor and have a new one with the feature gating for consensus relevant call sites.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

haven't heard anything about 3.0 coming soon. I think the SDK repo is the most imminent thing that will enable this

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.

Do we have a rough time line? Otherwise I will look for alternative implementations

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@joncinque can comment on timeline

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 think we discussed this offline last week, but for public visibility, the sdk will be moved into a new repo with the v2.2 branch cut, likely in the next 7-10 days.

After that point, we can consider making breaking changes to solana-message or any other crate. We need to choose what to do with solana-program and solana-sdk -- my inclination is to bump major versions much less often for them, or stop publishing updates altogether and encourage people to use the split-up crates.

Comment thread sdk/message/src/legacy.rs Outdated
Comment on lines 730 to 733
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'd need a similar piping of this variable to the transaction view; see ResolvedTransactionView::cache_is_writable and test_demote_writable_program in the same file.

@Lichtso Lichtso force-pushed the feature/loader_v4_write_lock_demotion_exemption branch from ce3fc48 to ea0913b Compare January 24, 2025 10:47
@Lichtso Lichtso force-pushed the feature/loader_v4_write_lock_demotion_exemption branch from ea0913b to 850cfb5 Compare January 24, 2025 11:01
@joncinque
Copy link
Copy Markdown

This PR contains changes to the solana sdk, which will be moved to a new repo within the next week, when v2.2 is branched from master.

Please merge or close this PR as soon as possible, or re-create the sdk changes when the new repository is ready at https://github.com/anza-xyz/solana-sdk

@Lichtso Lichtso closed this Mar 11, 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