Skip to content
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

SIMD-0093: Disable bpf loader (V2) instructions #93

Merged

Conversation

HaoranYi
Copy link
Contributor

@HaoranYi HaoranYi commented Dec 13, 2023

Add disable bpf loader (V2) instructions simd

@HaoranYi HaoranYi changed the title SIMD: disable bpf loader (V1) instructions SIMD-0093: disable bpf loader (V1) instructions Dec 13, 2023
@HaoranYi HaoranYi changed the title SIMD-0093: disable bpf loader (V1) instructions SIMD-0093: disable bpf loader (V2) instructions Dec 15, 2023
Copy link
Contributor

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

Some nits


## Summary

Add a new feature to disable Bpf loader V2 program deployment.
Copy link
Contributor

Choose a reason for hiding this comment

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

I know in the Solana Labs code we call these feature gates. Maybe this is sufficiently client-agnostic and should be used here too?

Suggested change
Add a new feature to disable Bpf loader V2 program deployment.
Add a new feature gate to disable Bpf loader V2 program deployment.

Or even remove "feature" entirely:

Disable BPF Loader V2 for program deployment.

I think that's my preferred one.

Copy link
Contributor

Choose a reason for hiding this comment

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

We use the same terminology and feature gating structure on FD

Comment on lines 17 to 27
## Motivation

We want to deprecate the usage of *executable* metadata on account for program
runtime. The new variant of Bpf loader (i.e. V3/V4 etc.) no longer requires
*executable* metadata. However, the old Bpf loader (v2) still use *executable*
metadata during its program deployment. And this is a blocker for deprecating
the usage of *executable* metadata for program runtime. Therefore, as we are
migrating from the old Bpf loader V2 to the new Bpf loader (V3/V4), we are going
to add a feature to disable old V2 Bpf program deployment so that we can
activate the feature and deprecate *executable* metadata in program runtime for
the new kinds of Bpf loaders.
Copy link
Contributor

Choose a reason for hiding this comment

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

The current wording requires that the reader knows what "executable metadata" is. Wdyt about first defining what "executable metadata" is?

proposals/0093-disable-bpf-loader-instructions.md Outdated Show resolved Hide resolved
proposals/0093-disable-bpf-loader-instructions.md Outdated Show resolved Hide resolved
proposals/0093-disable-bpf-loader-instructions.md Outdated Show resolved Hide resolved
proposals/0093-disable-bpf-loader-instructions.md Outdated Show resolved Hide resolved
Copy link
Contributor

@lheeger-jump lheeger-jump left a comment

Choose a reason for hiding this comment

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

Approved

@lheeger-jump lheeger-jump changed the title SIMD-0093: disable bpf loader (V2) instructions SIMD-0093: Disable bpf loader (V2) instructions Jan 6, 2024
@lheeger-jump
Copy link
Contributor

I will merge this SIMD, barring core contributor objections, on 11/01/2024, or as soon as labs signs off.

@HaoranYi
Copy link
Contributor Author

HaoranYi commented Jan 6, 2024

I don't have write access to this repo. Can someone with write access to merge the PR?

@jacobcreech
Copy link
Contributor

Looks like we've reached consensus. Thanks @HaoranYi!

@jacobcreech jacobcreech merged commit 37ba288 into solana-foundation:main Jan 6, 2024
4 checks passed
@jeffwashington jeffwashington requested review from jeffwashington and removed request for jeffwashington January 8, 2024 23:01
@pgarg66
Copy link

pgarg66 commented Jan 8, 2024

Looks good!

@sakridge sakridge added the core Standard SIMD with type Core label Feb 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Standard SIMD with type Core
Projects
Status: SIMDs
Development

Successfully merging this pull request may close these issues.

8 participants