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

Feature Gate Program #4987

Closed

Conversation

buffalojoec
Copy link
Contributor

This PR adds a new program to SPL: The Feature Gate Program.

This is the exact program proposed in this issue on the Solana monorepo.

In summary, this program takes the place of the otherwise empty and currently utilized account address at Feature111111111111111111111111111111111111 . Once we've replaced that account with this program, it will be able to control (and most importantly revoke) pending feature activations, as described in the linked issue.

@buffalojoec buffalojoec requested a review from joncinque August 10, 2023 20:25
Copy link
Contributor

@joncinque joncinque left a comment

Choose a reason for hiding this comment

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

Great start! I did a pass over the whole thing, but let's nail down the interface first and foremost, then we can look at the logic more closely.

feature-gate/cli/Cargo.toml Outdated Show resolved Hide resolved
feature-gate/cli/Cargo.toml Outdated Show resolved Hide resolved
feature-gate/cli/src/main.rs Outdated Show resolved Hide resolved
feature-gate/cli/src/main.rs Outdated Show resolved Hide resolved
feature-gate/cli/src/main.rs Outdated Show resolved Hide resolved
feature-gate/program/src/processor.rs Show resolved Hide resolved
feature-gate/program/src/processor.rs Show resolved Hide resolved
feature-gate/program/src/instruction.rs Outdated Show resolved Hide resolved
feature-gate/program/src/instruction.rs Outdated Show resolved Hide resolved
feature-gate/program/tests/functional.rs Outdated Show resolved Hide resolved
@buffalojoec
Copy link
Contributor Author

Great start! I did a pass over the whole thing, but let's nail down the interface first and foremost, then we can look at the logic more closely.

Sounds good. I addressed any feedback you had for now, and we can just let this sit until talks finalize on the linked issue/discussion.

@mvines
Copy link
Member

mvines commented Aug 11, 2023

Great start! I did a pass over the whole thing, but let's nail down the interface first and foremost, then we can look at the logic more closely.

Do you mean the Revoke instruction interface? If so, wouldn't it be better to land just an empty noop feature gate program for now? And deal with the Revoke instruction once we're waiting for the noop program to be shipped to all clusters?

@joncinque
Copy link
Contributor

Do you mean the Revoke instruction interface? If so, wouldn't it be better to land just an empty noop feature gate program for now? And deal with the Revoke instruction once we're waiting for the noop program to be shipped to all clusters?

I've been wondering why you bring up the no-op program so much... I might be missing something, but we can simply ship the feature in the validator without any program deployed at the address. By the time the feature's ready to be enabled, we'll have the program ready

Do you expect the no-op program uploaded to the source address before shipping the validator change?

@mvines
Copy link
Member

mvines commented Aug 11, 2023

Got it. Yeah so given how changing this part of the system is incredibly sensitive, my mind immediately wants to break down the change into many small steps that we can trivially (or close to) conclude will be safe.

Step 1. Install a no-op feature gate program at the right address. This requires the runtime changes and a bpf program to swap in. If the bpf program is empty, now we only need to narrowly focus on thinking about and auditing the runtime changes.

Step 2. With the scary runtime changes out of the way, and an upgradable feature-gate program installed, we can then turn our narrow focus on thinking about and auditing the program-specific changes to support revoking a pending activation.

Combining these steps to me just unnecessarily increases the risk that we'll overlook something important

@joncinque
Copy link
Contributor

Maybe I'm being a little 🤠 but those two steps are well-separated, with or without a no-op program. On the flipside, if the upgrade goes wrong, then a no-op program can't do any damage, so maybe that's the biggest benefit.

Either way, we can table this until solana-labs/solana#32783 goes in if it's too much all at once

@buffalojoec
Copy link
Contributor Author

Closing in favor of #5510

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-close Add this tag to exempt a PR / issue from being closed automatically
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants