From e44b6025585d65c158e5e953ce5972a73a2bdca6 Mon Sep 17 00:00:00 2001 From: Joe Date: Thu, 10 Aug 2023 14:21:57 -0600 Subject: [PATCH 1/7] feature gate program --- Cargo.lock | 24 ++ Cargo.toml | 2 + feature-gate/README.md | 10 + feature-gate/cli/Cargo.toml | 24 ++ feature-gate/cli/src/main.rs | 268 +++++++++++++++++++++++ feature-gate/program/Cargo.toml | 27 +++ feature-gate/program/src/entrypoint.rs | 17 ++ feature-gate/program/src/error.rs | 14 ++ feature-gate/program/src/instruction.rs | 87 ++++++++ feature-gate/program/src/lib.rs | 16 ++ feature-gate/program/src/processor.rs | 89 ++++++++ feature-gate/program/tests/functional.rs | 235 ++++++++++++++++++++ 12 files changed, 813 insertions(+) create mode 100644 feature-gate/README.md create mode 100644 feature-gate/cli/Cargo.toml create mode 100644 feature-gate/cli/src/main.rs create mode 100644 feature-gate/program/Cargo.toml create mode 100644 feature-gate/program/src/entrypoint.rs create mode 100644 feature-gate/program/src/error.rs create mode 100644 feature-gate/program/src/instruction.rs create mode 100644 feature-gate/program/src/lib.rs create mode 100644 feature-gate/program/src/processor.rs create mode 100644 feature-gate/program/tests/functional.rs diff --git a/Cargo.lock b/Cargo.lock index 26b44e0cd1d..08aa41ea0c4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6523,6 +6523,30 @@ dependencies = [ "spl-token 4.0.0", ] +[[package]] +name = "spl-feature-gate" +version = "0.0.1" +dependencies = [ + "borsh 0.10.3", + "solana-program", + "solana-program-test", + "solana-sdk", + "spl-program-error", +] + +[[package]] +name = "spl-feature-gate-cli" +version = "1.2.0" +dependencies = [ + "clap 2.34.0", + "solana-clap-utils", + "solana-cli-config", + "solana-client", + "solana-logger", + "solana-sdk", + "spl-feature-gate", +] + [[package]] name = "spl-feature-proposal" version = "1.0.0" diff --git a/Cargo.toml b/Cargo.toml index 84f8f1eedd9..a9709845759 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -13,6 +13,8 @@ members = [ "examples/rust/sysvar", "examples/rust/transfer-lamports", "examples/rust/transfer-tokens", + "feature-gate/cli", + "feature-gate/program", "feature-proposal/program", "feature-proposal/cli", "governance/addin-mock/program", diff --git a/feature-gate/README.md b/feature-gate/README.md new file mode 100644 index 00000000000..e83c14df86a --- /dev/null +++ b/feature-gate/README.md @@ -0,0 +1,10 @@ +# Feature Gate Program + +This program serves to manage new features on Solana. + +It serves two main purposes: activating new features and revoking features that +are pending activation. + +More information & documentation will follow as this program matures, but you +can follow the discussions +[here](https://github.com/solana-labs/solana/issues/32780)! diff --git a/feature-gate/cli/Cargo.toml b/feature-gate/cli/Cargo.toml new file mode 100644 index 00000000000..6db903ea523 --- /dev/null +++ b/feature-gate/cli/Cargo.toml @@ -0,0 +1,24 @@ +[package] +name = "spl-feature-gate-cli" +version = "1.2.0" +description = "SPL Feature Gate Command-line Utility" +authors = ["Solana Labs Maintainers "] +repository = "https://github.com/solana-labs/solana-program-library" +license = "Apache-2.0" +edition = "2021" + +[dependencies] +clap = "2.33.3" +solana-clap-utils = "1.16.3" +solana-cli-config = "1.16.3" +solana-client = "1.16.3" +solana-logger = "1.16.3" +solana-sdk = "1.16.3" +spl-feature-gate = { version = "0.0.1", path = "../program", features = ["no-entrypoint"] } + +[[bin]] +name = "spl-feature-gate" +path = "src/main.rs" + +[package.metadata.docs.rs] +targets = ["x86_64-unknown-linux-gnu"] diff --git a/feature-gate/cli/src/main.rs b/feature-gate/cli/src/main.rs new file mode 100644 index 00000000000..f679d4efa95 --- /dev/null +++ b/feature-gate/cli/src/main.rs @@ -0,0 +1,268 @@ +#![allow(clippy::integer_arithmetic)] + +use { + clap::{crate_description, crate_name, crate_version, App, AppSettings, Arg, SubCommand}, + solana_clap_utils::{ + input_parsers::{keypair_of, pubkey_of}, + input_validators::{is_keypair, is_url, is_valid_pubkey}, + }, + solana_client::rpc_client::RpcClient, + solana_sdk::{ + commitment_config::CommitmentConfig, + feature::Feature, + pubkey::Pubkey, + rent::Rent, + signature::{read_keypair_file, Keypair, Signer}, + system_instruction, + transaction::Transaction, + }, + spl_feature_gate::instruction::{activate, revoke}, +}; + +fn keypair_clone(kp: &Keypair) -> Keypair { + Keypair::from_bytes(&kp.to_bytes()).expect("failed to copy keypair") +} + +#[allow(dead_code)] +struct Config { + keypair: Keypair, + json_rpc_url: String, + verbose: bool, +} + +fn main() -> Result<(), Box> { + let app_matches = App::new(crate_name!()) + .about(crate_description!()) + .version(crate_version!()) + .setting(AppSettings::SubcommandRequiredElseHelp) + .arg({ + let arg = Arg::with_name("config_file") + .short("C") + .long("config") + .value_name("PATH") + .takes_value(true) + .global(true) + .help("Configuration file to use"); + if let Some(ref config_file) = *solana_cli_config::CONFIG_FILE { + arg.default_value(config_file) + } else { + arg + } + }) + .arg( + Arg::with_name("keypair") + .long("keypair") + .value_name("KEYPAIR") + .validator(is_keypair) + .takes_value(true) + .global(true) + .help("Filepath or URL to a keypair [default: client keypair]"), + ) + .arg( + Arg::with_name("verbose") + .long("verbose") + .short("v") + .takes_value(false) + .global(true) + .help("Show additional information"), + ) + .arg( + Arg::with_name("json_rpc_url") + .long("url") + .value_name("URL") + .takes_value(true) + .global(true) + .validator(is_url) + .help("JSON RPC URL for the cluster [default: value from configuration file]"), + ) + .subcommand( + SubCommand::with_name("activate") + .about("Activate a feature") + .arg( + Arg::with_name("feature_keypair") + .value_name("FEATURE_KEYPAIR") + .validator(is_keypair) + .index(1) + .required(true) + .help("Path to keypair of the feature"), + ) + .arg( + Arg::with_name("authority_keypair") + .value_name("AUTHORITY_KEYPAIR") + .validator(is_keypair) + .required(true) + .help("Path to keypair of the authority"), + ) + .arg( + Arg::with_name("payer_keypair") + .value_name("PAYER_KEYPAIR") + .validator(is_keypair) + .help( + "Path to keypair of the payer to fund the feature account (defaults \ + to authority)", + ), + ), + ) + .subcommand( + SubCommand::with_name("revoke") + .about("Revoke a pending feature activation") + .arg( + Arg::with_name("feature_id") + .value_name("FEATURE_ID") + .validator(is_valid_pubkey) + .index(1) + .required(true) + .help("The address of the feature (feature ID)"), + ) + .arg( + Arg::with_name("destination") + .value_name("DESTINATION") + .validator(is_valid_pubkey) + .index(2) + .required(true) + .help("The address of the destination for the refunded lamports"), + ) + .arg( + Arg::with_name("authority_keypair") + .value_name("AUTHORITY_KEYPAIR") + .validator(is_keypair) + .required(true) + .help("Path to keypair of the authority"), + ), + ) + .get_matches(); + + let (sub_command, sub_matches) = app_matches.subcommand(); + let matches = sub_matches.unwrap(); + + let config = { + let cli_config = if let Some(config_file) = matches.value_of("config_file") { + solana_cli_config::Config::load(config_file).unwrap_or_default() + } else { + solana_cli_config::Config::default() + }; + + Config { + json_rpc_url: matches + .value_of("json_rpc_url") + .unwrap_or(&cli_config.json_rpc_url) + .to_string(), + keypair: read_keypair_file( + matches + .value_of("keypair") + .unwrap_or(&cli_config.keypair_path), + )?, + verbose: matches.is_present("verbose"), + } + }; + solana_logger::setup_with_default("solana=info"); + let rpc_client = + RpcClient::new_with_commitment(config.json_rpc_url.clone(), CommitmentConfig::confirmed()); + + match (sub_command, sub_matches) { + ("activate", Some(arg_matches)) => { + let feature_keypair = keypair_of(arg_matches, "feature_keypair").unwrap(); + let authority_keypair = keypair_of(arg_matches, "authority_keypair").unwrap(); + let payer_keypair = keypair_of(arg_matches, "payer_keypair") + .unwrap_or(keypair_clone(&authority_keypair)); + + process_activate( + &rpc_client, + &config, + &feature_keypair, + &payer_keypair, + &authority_keypair, + ) + } + ("revoke", Some(arg_matches)) => { + let feature_id = pubkey_of(arg_matches, "feature_id").unwrap(); + let destination = pubkey_of(arg_matches, "destination").unwrap(); + let authority_keypair = keypair_of(arg_matches, "authority_keypair").unwrap(); + + process_revoke( + &rpc_client, + &config, + &feature_id, + &destination, + &authority_keypair, + ) + } + _ => unreachable!(), + } +} + +fn process_activate( + rpc_client: &RpcClient, + config: &Config, + feature_keypair: &Keypair, + payer_keypair: &Keypair, + authority_keypair: &Keypair, +) -> Result<(), Box> { + println!(); + println!("Activating feature..."); + println!("Feature ID: {}", feature_keypair.pubkey()); + println!("Payer: {}", payer_keypair.pubkey()); + println!("Authority: {}", authority_keypair.pubkey()); + println!(); + println!("JSON RPC URL: {}", config.json_rpc_url); + println!(); + + let rent_lamports = Rent::default().minimum_balance(Feature::size_of()); + + let transaction = Transaction::new_signed_with_payer( + &[ + system_instruction::transfer( + &payer_keypair.pubkey(), + &feature_keypair.pubkey(), + rent_lamports, + ), + activate( + &spl_feature_gate::id(), + &feature_keypair.pubkey(), + &authority_keypair.pubkey(), + ), + ], + Some(&payer_keypair.pubkey()), + &[payer_keypair, feature_keypair, authority_keypair], + rpc_client.get_latest_blockhash()?, + ); + rpc_client.send_and_confirm_transaction_with_spinner(&transaction)?; + + println!(); + println!("Feature is marked for activation!"); + Ok(()) +} + +fn process_revoke( + rpc_client: &RpcClient, + config: &Config, + feature_id: &Pubkey, + destination: &Pubkey, + authority_keypair: &Keypair, +) -> Result<(), Box> { + println!(); + println!("Revoking feature..."); + println!("Feature ID: {}", feature_id); + println!("Destination: {}", destination); + println!("Authority: {}", authority_keypair.pubkey()); + println!(); + println!("JSON RPC URL: {}", config.json_rpc_url); + println!(); + + let transaction = Transaction::new_signed_with_payer( + &[revoke( + &spl_feature_gate::id(), + feature_id, + destination, + &authority_keypair.pubkey(), + )], + Some(&authority_keypair.pubkey()), + &[authority_keypair], + rpc_client.get_latest_blockhash()?, + ); + rpc_client.send_and_confirm_transaction_with_spinner(&transaction)?; + + println!(); + println!("Feature successfully revoked!"); + Ok(()) +} diff --git a/feature-gate/program/Cargo.toml b/feature-gate/program/Cargo.toml new file mode 100644 index 00000000000..c75b68556d8 --- /dev/null +++ b/feature-gate/program/Cargo.toml @@ -0,0 +1,27 @@ +[package] +name = "spl-feature-gate" +version = "0.0.1" +description = "Solana Program Library Feature Gate Program" +authors = ["Solana Labs Maintainers "] +repository = "https://github.com/solana-labs/solana-program-library" +license = "Apache-2.0" +edition = "2021" + +[features] +no-entrypoint = [] +test-sbf = [] + +[dependencies] +borsh = "0.10.3" +solana-program = "1.16.3" +spl-program-error = { version = "0.2.0", path = "../../libraries/program-error" } + +[dev-dependencies] +solana-program-test = "1.16.3" +solana-sdk = "1.16.3" + +[lib] +crate-type = ["cdylib", "lib"] + +[package.metadata.docs.rs] +targets = ["x86_64-unknown-linux-gnu"] diff --git a/feature-gate/program/src/entrypoint.rs b/feature-gate/program/src/entrypoint.rs new file mode 100644 index 00000000000..c261a918a42 --- /dev/null +++ b/feature-gate/program/src/entrypoint.rs @@ -0,0 +1,17 @@ +//! Program entrypoint + +use { + crate::processor, + solana_program::{ + account_info::AccountInfo, entrypoint, entrypoint::ProgramResult, pubkey::Pubkey, + }, +}; + +entrypoint!(process_instruction); +fn process_instruction( + program_id: &Pubkey, + accounts: &[AccountInfo], + input: &[u8], +) -> ProgramResult { + processor::process(program_id, accounts, input) +} diff --git a/feature-gate/program/src/error.rs b/feature-gate/program/src/error.rs new file mode 100644 index 00000000000..a3acee031dc --- /dev/null +++ b/feature-gate/program/src/error.rs @@ -0,0 +1,14 @@ +//! Program error types + +use spl_program_error::*; + +/// Program specific errors +#[spl_program_error] +pub enum FeatureGateError { + /// Operation overflowed + #[error("Operation overflowed")] + Overflow, + /// Feature account must be a system account + #[error("Feature account must be a system account")] + FeatureNotSystemAccount, +} diff --git a/feature-gate/program/src/instruction.rs b/feature-gate/program/src/instruction.rs new file mode 100644 index 00000000000..f4a3321a074 --- /dev/null +++ b/feature-gate/program/src/instruction.rs @@ -0,0 +1,87 @@ +//! Program instructions + +use { + borsh::{BorshDeserialize, BorshSchema, BorshSerialize}, + solana_program::{ + instruction::{AccountMeta, Instruction}, + pubkey::Pubkey, + system_program, + }, +}; + +/// Feature Gate program instructions +#[derive(Clone, Debug, BorshSerialize, BorshDeserialize, BorshSchema, PartialEq)] +pub enum FeatureGateInstruction { + /// Submit a feature for activation. + /// + /// Accounts expected by this instruction: + /// + /// 0. `[ws]` Feature account (must be a system account) + /// 1. `[s]` Authority + /// 3. `[]` System program + Activate, + /// Revoke a pending feature activation. + /// + /// Accounts expected by this instruction: + /// + /// 0. `[w]` Feature account + /// 1. `[w]` Destination (for rent lamports) + /// 2. `[s]` Authority + Revoke, +} + +/// Creates an 'Activate' instruction. +pub fn activate(program_id: &Pubkey, feature: &Pubkey, authority: &Pubkey) -> Instruction { + let accounts = vec![ + AccountMeta::new(*feature, true), + AccountMeta::new_readonly(*authority, true), + AccountMeta::new_readonly(system_program::id(), false), + ]; + + Instruction { + program_id: *program_id, + accounts, + data: FeatureGateInstruction::Activate.try_to_vec().unwrap(), + } +} + +/// Creates a 'Revoke' instruction. +pub fn revoke( + program_id: &Pubkey, + feature: &Pubkey, + destination: &Pubkey, + authority: &Pubkey, +) -> Instruction { + let accounts = vec![ + AccountMeta::new(*feature, false), + AccountMeta::new(*destination, false), + AccountMeta::new_readonly(*authority, false), + ]; + + Instruction { + program_id: *program_id, + accounts, + data: FeatureGateInstruction::Revoke.try_to_vec().unwrap(), + } +} + +#[cfg(test)] +mod test { + use super::*; + + fn test_pack_unpack(instruction: &FeatureGateInstruction) { + let packed = instruction.try_to_vec().unwrap(); + let unpacked = FeatureGateInstruction::try_from_slice(&packed).unwrap(); + assert_eq!(instruction, &unpacked); + } + + #[test] + fn test_pack_unpack_activate() { + test_pack_unpack(&FeatureGateInstruction::Activate); + } + + #[test] + fn test_pack_unpack_revoke() { + test_pack_unpack(&FeatureGateInstruction::Revoke); + } +} diff --git a/feature-gate/program/src/lib.rs b/feature-gate/program/src/lib.rs new file mode 100644 index 00000000000..867c53afe8e --- /dev/null +++ b/feature-gate/program/src/lib.rs @@ -0,0 +1,16 @@ +//! Feature Gate program + +#![allow(clippy::integer_arithmetic)] +#![deny(missing_docs)] +#![cfg_attr(not(test), forbid(unsafe_code))] + +mod entrypoint; +pub mod error; +pub mod instruction; +pub mod processor; + +// Export current SDK types for downstream users building with a different SDK +// version +pub use solana_program; + +solana_program::declare_id!("Feature111111111111111111111111111111111111"); diff --git a/feature-gate/program/src/processor.rs b/feature-gate/program/src/processor.rs new file mode 100644 index 00000000000..ee41ba77c53 --- /dev/null +++ b/feature-gate/program/src/processor.rs @@ -0,0 +1,89 @@ +//! Program state processor + +use { + crate::{error::FeatureGateError, instruction::FeatureGateInstruction}, + borsh::BorshDeserialize, + solana_program::{ + account_info::{next_account_info, AccountInfo}, + entrypoint::ProgramResult, + feature::Feature, + msg, + program::invoke, + program_error::ProgramError, + pubkey::Pubkey, + system_instruction, system_program, + }, +}; + +/// Processes an [Activate](enum.FeatureGateInstruction.html) instruction. +pub fn process_activate(program_id: &Pubkey, accounts: &[AccountInfo]) -> ProgramResult { + let account_info_iter = &mut accounts.iter(); + + let feature_info = next_account_info(account_info_iter)?; + let authority_info = next_account_info(account_info_iter)?; + let _system_program_info = next_account_info(account_info_iter)?; + + if !feature_info.is_signer || !authority_info.is_signer { + return Err(ProgramError::MissingRequiredSignature); + } + + if feature_info.owner != &system_program::id() { + return Err(FeatureGateError::FeatureNotSystemAccount.into()); + } + + invoke( + &system_instruction::allocate(feature_info.key, Feature::size_of() as u64), + &[feature_info.clone()], + )?; + invoke( + &system_instruction::assign(feature_info.key, program_id), + &[feature_info.clone()], + )?; + + Ok(()) +} + +/// Processes an [revoke](enum.FeatureGateInstruction.html) instruction. +pub fn process_revoke(program_id: &Pubkey, accounts: &[AccountInfo]) -> ProgramResult { + let account_info_iter = &mut accounts.iter(); + + let feature_info = next_account_info(account_info_iter)?; + let destination_info = next_account_info(account_info_iter)?; + let authority_info = next_account_info(account_info_iter)?; + + if !authority_info.is_signer { + return Err(ProgramError::MissingRequiredSignature); + } + + if feature_info.owner != program_id { + return Err(ProgramError::IllegalOwner); + } + + let new_destination_lamports = feature_info + .lamports() + .checked_add(destination_info.lamports()) + .ok_or::(FeatureGateError::Overflow.into())?; + + **feature_info.try_borrow_mut_lamports()? = 0; + **destination_info.try_borrow_mut_lamports()? = new_destination_lamports; + + feature_info.realloc(0, true)?; + feature_info.assign(&system_program::id()); + + Ok(()) +} + +/// Processes an [Instruction](enum.Instruction.html). +pub fn process(program_id: &Pubkey, accounts: &[AccountInfo], input: &[u8]) -> ProgramResult { + let instruction = FeatureGateInstruction::try_from_slice(input)?; + match instruction { + FeatureGateInstruction::Activate => { + msg!("Instruction: Activate"); + process_activate(program_id, accounts) + } + FeatureGateInstruction::Revoke => { + msg!("Instruction: Revoke"); + process_revoke(program_id, accounts) + } + } +} diff --git a/feature-gate/program/tests/functional.rs b/feature-gate/program/tests/functional.rs new file mode 100644 index 00000000000..417c703e713 --- /dev/null +++ b/feature-gate/program/tests/functional.rs @@ -0,0 +1,235 @@ +#![cfg(feature = "test-sbf")] + +use { + solana_program::instruction::InstructionError, + solana_program_test::{processor, tokio, ProgramTest}, + solana_sdk::{ + account::Account as SolanaAccount, + feature::Feature, + pubkey::Pubkey, + signature::{Keypair, Signer}, + system_instruction, + transaction::{Transaction, TransactionError}, + }, + spl_feature_gate::{ + error::FeatureGateError, + instruction::{activate, revoke}, + }, +}; + +#[tokio::test] +async fn test_functional() { + let mock_feature_keypair = Keypair::new(); + let feature_keypair = Keypair::new(); + let authority_keypair = Keypair::new(); + let destination = Pubkey::new_unique(); + + let mut program_test = ProgramTest::new( + "spl_feature_gate", + spl_feature_gate::id(), + processor!(spl_feature_gate::processor::process), + ); + + // Create the authority account with some lamports for transaction fees + program_test.add_account( + authority_keypair.pubkey(), + SolanaAccount { + lamports: 500_000_000, + ..SolanaAccount::default() + }, + ); + + // Add a mock feature for testing later + program_test.add_account( + mock_feature_keypair.pubkey(), + SolanaAccount { + lamports: 500_000_000, + owner: spl_feature_gate::id(), + ..SolanaAccount::default() + }, + ); + + let mut context = program_test.start_with_context().await; + let rent = context.banks_client.get_rent().await.unwrap(); + let rent_lamports = rent.minimum_balance(Feature::size_of()); + + // Activate: Fail feature not signer + let mut activate_ix = activate( + &spl_feature_gate::id(), + &feature_keypair.pubkey(), + &authority_keypair.pubkey(), + ); + activate_ix.accounts[0].is_signer = false; + let transaction = Transaction::new_signed_with_payer( + &[ + system_instruction::transfer( + &context.payer.pubkey(), + &feature_keypair.pubkey(), + rent_lamports, + ), + activate_ix, + ], + Some(&context.payer.pubkey()), + &[&context.payer, &authority_keypair], + context.last_blockhash, + ); + let error = context + .banks_client + .process_transaction(transaction) + .await + .unwrap_err() + .unwrap(); + assert_eq!( + error, + TransactionError::InstructionError(1, InstructionError::MissingRequiredSignature) + ); + + // Activate: Fail authority not signer + let mut activate_ix = activate( + &spl_feature_gate::id(), + &feature_keypair.pubkey(), + &authority_keypair.pubkey(), + ); + activate_ix.accounts[1].is_signer = false; + let transaction = Transaction::new_signed_with_payer( + &[ + system_instruction::transfer( + &context.payer.pubkey(), + &feature_keypair.pubkey(), + rent_lamports, + ), + activate_ix, + ], + Some(&context.payer.pubkey()), + &[&context.payer, &feature_keypair], + context.last_blockhash, + ); + let error = context + .banks_client + .process_transaction(transaction) + .await + .unwrap_err() + .unwrap(); + assert_eq!( + error, + TransactionError::InstructionError(1, InstructionError::MissingRequiredSignature) + ); + + // Activate: Fail feature not owned by system program + let transaction = Transaction::new_signed_with_payer( + &[activate( + &spl_feature_gate::id(), + &mock_feature_keypair.pubkey(), + &authority_keypair.pubkey(), + )], + Some(&authority_keypair.pubkey()), + &[&mock_feature_keypair, &authority_keypair], + context.last_blockhash, + ); + let error = context + .banks_client + .process_transaction(transaction) + .await + .unwrap_err() + .unwrap(); + assert_eq!( + error, + TransactionError::InstructionError( + 0, + InstructionError::Custom(FeatureGateError::FeatureNotSystemAccount as u32), + ) + ); + + // Submit a feature for activation + let transaction = Transaction::new_signed_with_payer( + &[ + system_instruction::transfer( + &context.payer.pubkey(), + &feature_keypair.pubkey(), + rent_lamports, + ), + activate( + &spl_feature_gate::id(), + &feature_keypair.pubkey(), + &authority_keypair.pubkey(), + ), + ], + Some(&context.payer.pubkey()), + &[&context.payer, &feature_keypair, &authority_keypair], + context.last_blockhash, + ); + + context + .banks_client + .process_transaction(transaction) + .await + .unwrap(); + + // Confirm feature account exists with proper configurations + let feature_account = context + .banks_client + .get_account(feature_keypair.pubkey()) + .await + .unwrap() + .unwrap(); + assert_eq!(feature_account.owner, spl_feature_gate::id()); + + // Revoke: Fail authority not signer + let mut revoke_ix = revoke( + &spl_feature_gate::id(), + &feature_keypair.pubkey(), + &destination, + &authority_keypair.pubkey(), + ); + revoke_ix.accounts[2].is_signer = false; + let transaction = Transaction::new_signed_with_payer( + &[revoke_ix], + Some(&context.payer.pubkey()), + &[&context.payer], + context.last_blockhash, + ); + let error = context + .banks_client + .process_transaction(transaction) + .await + .unwrap_err() + .unwrap(); + assert_eq!( + error, + TransactionError::InstructionError(0, InstructionError::MissingRequiredSignature) + ); + + // Revoke a feature activation + let transaction = Transaction::new_signed_with_payer( + &[revoke( + &spl_feature_gate::id(), + &feature_keypair.pubkey(), + &destination, + &authority_keypair.pubkey(), + )], + Some(&authority_keypair.pubkey()), + &[&authority_keypair], + context.last_blockhash, + ); + + context + .banks_client + .process_transaction(transaction) + .await + .unwrap(); + + // Confirm feature account was closed and destination account received lamports + let feature_account = context + .banks_client + .get_account(feature_keypair.pubkey()) + .await + .unwrap(); + assert!(feature_account.is_none()); + let destination_account = context + .banks_client + .get_account(destination) + .await + .unwrap() + .unwrap(); + assert_eq!(destination_account.lamports, rent_lamports); +} From 770c3bc1044d379a81821c2b97232ba3ae5c28bd Mon Sep 17 00:00:00 2001 From: Joe Date: Thu, 10 Aug 2023 15:45:18 -0600 Subject: [PATCH 2/7] address CLI feedback --- Cargo.lock | 2 +- feature-gate/cli/Cargo.toml | 12 ++++++------ feature-gate/cli/src/main.rs | 18 +++++++----------- 3 files changed, 14 insertions(+), 18 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 08aa41ea0c4..e4a35255317 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6536,7 +6536,7 @@ dependencies = [ [[package]] name = "spl-feature-gate-cli" -version = "1.2.0" +version = "1.0.0" dependencies = [ "clap 2.34.0", "solana-clap-utils", diff --git a/feature-gate/cli/Cargo.toml b/feature-gate/cli/Cargo.toml index 6db903ea523..192d26ae3a6 100644 --- a/feature-gate/cli/Cargo.toml +++ b/feature-gate/cli/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "spl-feature-gate-cli" -version = "1.2.0" +version = "1.0.0" description = "SPL Feature Gate Command-line Utility" authors = ["Solana Labs Maintainers "] repository = "https://github.com/solana-labs/solana-program-library" @@ -9,11 +9,11 @@ edition = "2021" [dependencies] clap = "2.33.3" -solana-clap-utils = "1.16.3" -solana-cli-config = "1.16.3" -solana-client = "1.16.3" -solana-logger = "1.16.3" -solana-sdk = "1.16.3" +solana-clap-utils = "=1.16.3" +solana-cli-config = "=1.16.3" +solana-client = "=1.16.3" +solana-logger = "=1.16.3" +solana-sdk = "=1.16.3" spl-feature-gate = { version = "0.0.1", path = "../program", features = ["no-entrypoint"] } [[bin]] diff --git a/feature-gate/cli/src/main.rs b/feature-gate/cli/src/main.rs index f679d4efa95..d1bd8a3e9fa 100644 --- a/feature-gate/cli/src/main.rs +++ b/feature-gate/cli/src/main.rs @@ -1,10 +1,10 @@ -#![allow(clippy::integer_arithmetic)] +//! Feature gate CLI use { clap::{crate_description, crate_name, crate_version, App, AppSettings, Arg, SubCommand}, solana_clap_utils::{ input_parsers::{keypair_of, pubkey_of}, - input_validators::{is_keypair, is_url, is_valid_pubkey}, + input_validators::{is_keypair, is_url, is_valid_pubkey, is_valid_signer}, }, solana_client::rpc_client::RpcClient, solana_sdk::{ @@ -19,13 +19,9 @@ use { spl_feature_gate::instruction::{activate, revoke}, }; -fn keypair_clone(kp: &Keypair) -> Keypair { - Keypair::from_bytes(&kp.to_bytes()).expect("failed to copy keypair") -} - #[allow(dead_code)] struct Config { - keypair: Keypair, + keypair: Box, json_rpc_url: String, verbose: bool, } @@ -53,7 +49,7 @@ fn main() -> Result<(), Box> { Arg::with_name("keypair") .long("keypair") .value_name("KEYPAIR") - .validator(is_keypair) + .validator(is_valid_signer) .takes_value(true) .global(true) .help("Filepath or URL to a keypair [default: client keypair]"), @@ -147,11 +143,11 @@ fn main() -> Result<(), Box> { .value_of("json_rpc_url") .unwrap_or(&cli_config.json_rpc_url) .to_string(), - keypair: read_keypair_file( + keypair: Box::new(read_keypair_file( matches .value_of("keypair") .unwrap_or(&cli_config.keypair_path), - )?, + )?), verbose: matches.is_present("verbose"), } }; @@ -164,7 +160,7 @@ fn main() -> Result<(), Box> { let feature_keypair = keypair_of(arg_matches, "feature_keypair").unwrap(); let authority_keypair = keypair_of(arg_matches, "authority_keypair").unwrap(); let payer_keypair = keypair_of(arg_matches, "payer_keypair") - .unwrap_or(keypair_clone(&authority_keypair)); + .unwrap_or(authority_keypair.insecure_clone()); process_activate( &rpc_client, From c63d6a126d118cbb02242e3da933bc38e5c4a66a Mon Sep 17 00:00:00 2001 From: Joe Date: Thu, 10 Aug 2023 16:05:03 -0600 Subject: [PATCH 3/7] address initial program feedback --- Cargo.lock | 2 +- feature-gate/cli/Cargo.toml | 2 +- feature-gate/program/Cargo.toml | 2 +- feature-gate/program/src/instruction.rs | 18 ++++-- feature-gate/program/src/lib.rs | 1 - feature-gate/program/src/processor.rs | 4 +- feature-gate/program/tests/functional.rs | 70 +++++++++++++++++++++++- 7 files changed, 85 insertions(+), 14 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e4a35255317..d6276621de4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6525,7 +6525,7 @@ dependencies = [ [[package]] name = "spl-feature-gate" -version = "0.0.1" +version = "0.1.0" dependencies = [ "borsh 0.10.3", "solana-program", diff --git a/feature-gate/cli/Cargo.toml b/feature-gate/cli/Cargo.toml index 192d26ae3a6..fb70241843a 100644 --- a/feature-gate/cli/Cargo.toml +++ b/feature-gate/cli/Cargo.toml @@ -14,7 +14,7 @@ solana-cli-config = "=1.16.3" solana-client = "=1.16.3" solana-logger = "=1.16.3" solana-sdk = "=1.16.3" -spl-feature-gate = { version = "0.0.1", path = "../program", features = ["no-entrypoint"] } +spl-feature-gate = { version = "0.1.0", path = "../program", features = ["no-entrypoint"] } [[bin]] name = "spl-feature-gate" diff --git a/feature-gate/program/Cargo.toml b/feature-gate/program/Cargo.toml index c75b68556d8..247dce9dab3 100644 --- a/feature-gate/program/Cargo.toml +++ b/feature-gate/program/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "spl-feature-gate" -version = "0.0.1" +version = "0.1.0" description = "Solana Program Library Feature Gate Program" authors = ["Solana Labs Maintainers "] repository = "https://github.com/solana-labs/solana-program-library" diff --git a/feature-gate/program/src/instruction.rs b/feature-gate/program/src/instruction.rs index f4a3321a074..2868da38cb1 100644 --- a/feature-gate/program/src/instruction.rs +++ b/feature-gate/program/src/instruction.rs @@ -14,11 +14,17 @@ use { pub enum FeatureGateInstruction { /// Submit a feature for activation. /// + /// Note: This instruction expects the account to exist and be owned by the + /// system program. The account should also have enough rent-exempt lamports + /// to cover the cost of the account creation for a + /// `solana_program::feature::Feature` state prior to invoking this + /// instruction. + /// /// Accounts expected by this instruction: /// /// 0. `[ws]` Feature account (must be a system account) /// 1. `[s]` Authority - /// 3. `[]` System program + /// 2. `[]` System program Activate, /// Revoke a pending feature activation. /// @@ -27,7 +33,7 @@ pub enum FeatureGateInstruction { /// 0. `[w]` Feature account /// 1. `[w]` Destination (for rent lamports) /// 2. `[s]` Authority - Revoke, + RevokePendingActivation, } /// Creates an 'Activate' instruction. @@ -45,7 +51,7 @@ pub fn activate(program_id: &Pubkey, feature: &Pubkey, authority: &Pubkey) -> In } } -/// Creates a 'Revoke' instruction. +/// Creates a 'RevokePendingActivation' instruction. pub fn revoke( program_id: &Pubkey, feature: &Pubkey, @@ -61,7 +67,9 @@ pub fn revoke( Instruction { program_id: *program_id, accounts, - data: FeatureGateInstruction::Revoke.try_to_vec().unwrap(), + data: FeatureGateInstruction::RevokePendingActivation + .try_to_vec() + .unwrap(), } } @@ -82,6 +90,6 @@ mod test { #[test] fn test_pack_unpack_revoke() { - test_pack_unpack(&FeatureGateInstruction::Revoke); + test_pack_unpack(&FeatureGateInstruction::RevokePendingActivation); } } diff --git a/feature-gate/program/src/lib.rs b/feature-gate/program/src/lib.rs index 867c53afe8e..b8a1bde2cbb 100644 --- a/feature-gate/program/src/lib.rs +++ b/feature-gate/program/src/lib.rs @@ -1,6 +1,5 @@ //! Feature Gate program -#![allow(clippy::integer_arithmetic)] #![deny(missing_docs)] #![cfg_attr(not(test), forbid(unsafe_code))] diff --git a/feature-gate/program/src/processor.rs b/feature-gate/program/src/processor.rs index ee41ba77c53..9b3daded94f 100644 --- a/feature-gate/program/src/processor.rs +++ b/feature-gate/program/src/processor.rs @@ -81,8 +81,8 @@ pub fn process(program_id: &Pubkey, accounts: &[AccountInfo], input: &[u8]) -> P msg!("Instruction: Activate"); process_activate(program_id, accounts) } - FeatureGateInstruction::Revoke => { - msg!("Instruction: Revoke"); + FeatureGateInstruction::RevokePendingActivation => { + msg!("Instruction: RevokePendingActivation"); process_revoke(program_id, accounts) } } diff --git a/feature-gate/program/tests/functional.rs b/feature-gate/program/tests/functional.rs index 417c703e713..c8400761246 100644 --- a/feature-gate/program/tests/functional.rs +++ b/feature-gate/program/tests/functional.rs @@ -2,7 +2,7 @@ use { solana_program::instruction::InstructionError, - solana_program_test::{processor, tokio, ProgramTest}, + solana_program_test::{processor, tokio, ProgramTest, ProgramTestContext}, solana_sdk::{ account::Account as SolanaAccount, feature::Feature, @@ -17,12 +17,42 @@ use { }, }; +async fn setup_feature( + context: &mut ProgramTestContext, + feature_keypair: &Keypair, + authority_keypair: &Keypair, + rent_lamports: u64, +) { + let transaction = Transaction::new_signed_with_payer( + &[ + system_instruction::transfer( + &context.payer.pubkey(), + &feature_keypair.pubkey(), + rent_lamports, + ), + activate( + &spl_feature_gate::id(), + &feature_keypair.pubkey(), + &authority_keypair.pubkey(), + ), + ], + Some(&context.payer.pubkey()), + &[&context.payer, feature_keypair, authority_keypair], + context.last_blockhash, + ); + + context + .banks_client + .process_transaction(transaction) + .await + .unwrap(); +} + #[tokio::test] -async fn test_functional() { +async fn test_activate() { let mock_feature_keypair = Keypair::new(); let feature_keypair = Keypair::new(); let authority_keypair = Keypair::new(); - let destination = Pubkey::new_unique(); let mut program_test = ProgramTest::new( "spl_feature_gate", @@ -173,6 +203,40 @@ async fn test_functional() { .unwrap() .unwrap(); assert_eq!(feature_account.owner, spl_feature_gate::id()); +} + +#[tokio::test] +async fn test_revoke() { + let feature_keypair = Keypair::new(); + let authority_keypair = Keypair::new(); + let destination = Pubkey::new_unique(); + + let mut program_test = ProgramTest::new( + "spl_feature_gate", + spl_feature_gate::id(), + processor!(spl_feature_gate::processor::process), + ); + + // Create the authority account with some lamports for transaction fees + program_test.add_account( + authority_keypair.pubkey(), + SolanaAccount { + lamports: 500_000_000, + ..SolanaAccount::default() + }, + ); + + let mut context = program_test.start_with_context().await; + let rent = context.banks_client.get_rent().await.unwrap(); + let rent_lamports = rent.minimum_balance(Feature::size_of()); + + setup_feature( + &mut context, + &feature_keypair, + &authority_keypair, + rent_lamports, + ) + .await; // Revoke: Fail authority not signer let mut revoke_ix = revoke( From d9e3828bb1be7a56675f2296f71e88ea33281f10 Mon Sep 17 00:00:00 2001 From: Joe Date: Thu, 10 Aug 2023 16:16:02 -0600 Subject: [PATCH 4/7] added inactive feature check --- feature-gate/program/src/error.rs | 3 ++ feature-gate/program/src/processor.rs | 7 ++++ feature-gate/program/tests/functional.rs | 43 +++++++++++++++++++++++- 3 files changed, 52 insertions(+), 1 deletion(-) diff --git a/feature-gate/program/src/error.rs b/feature-gate/program/src/error.rs index a3acee031dc..d4241a01c14 100644 --- a/feature-gate/program/src/error.rs +++ b/feature-gate/program/src/error.rs @@ -11,4 +11,7 @@ pub enum FeatureGateError { /// Feature account must be a system account #[error("Feature account must be a system account")] FeatureNotSystemAccount, + /// Feature not inactive + #[error("Feature not inactive")] + FeatureNotInactive, } diff --git a/feature-gate/program/src/processor.rs b/feature-gate/program/src/processor.rs index 9b3daded94f..f160de647c2 100644 --- a/feature-gate/program/src/processor.rs +++ b/feature-gate/program/src/processor.rs @@ -59,6 +59,13 @@ pub fn process_revoke(program_id: &Pubkey, accounts: &[AccountInfo]) -> ProgramR return Err(ProgramError::IllegalOwner); } + if Feature::from_account_info(feature_info)? + .activated_at + .is_some() + { + return Err(FeatureGateError::FeatureNotInactive.into()); + } + let new_destination_lamports = feature_info .lamports() .checked_add(destination_info.lamports()) diff --git a/feature-gate/program/tests/functional.rs b/feature-gate/program/tests/functional.rs index c8400761246..c0e73b177b7 100644 --- a/feature-gate/program/tests/functional.rs +++ b/feature-gate/program/tests/functional.rs @@ -1,4 +1,4 @@ -#![cfg(feature = "test-sbf")] +// #![cfg(feature = "test-sbf")] use { solana_program::instruction::InstructionError, @@ -210,6 +210,7 @@ async fn test_revoke() { let feature_keypair = Keypair::new(); let authority_keypair = Keypair::new(); let destination = Pubkey::new_unique(); + let mock_active_feature_keypair = Keypair::new(); let mut program_test = ProgramTest::new( "spl_feature_gate", @@ -226,6 +227,20 @@ async fn test_revoke() { }, ); + // Add a mock feature that might be active for testing later + program_test.add_account( + mock_active_feature_keypair.pubkey(), + SolanaAccount { + lamports: 500_000_000, + owner: spl_feature_gate::id(), + data: vec![ + 1, // `Some()` + 45, 0, 0, 0, 0, 0, 0, 0, // Random slot `u64` + ], + ..SolanaAccount::default() + }, + ); + let mut context = program_test.start_with_context().await; let rent = context.banks_client.get_rent().await.unwrap(); let rent_lamports = rent.minimum_balance(Feature::size_of()); @@ -263,6 +278,32 @@ async fn test_revoke() { TransactionError::InstructionError(0, InstructionError::MissingRequiredSignature) ); + // Revoke: Fail feature not inactive + let transaction = Transaction::new_signed_with_payer( + &[revoke( + &spl_feature_gate::id(), + &mock_active_feature_keypair.pubkey(), + &destination, + &authority_keypair.pubkey(), + )], + Some(&authority_keypair.pubkey()), + &[&authority_keypair], + context.last_blockhash, + ); + let error = context + .banks_client + .process_transaction(transaction) + .await + .unwrap_err() + .unwrap(); + assert_eq!( + error, + TransactionError::InstructionError( + 0, + InstructionError::Custom(FeatureGateError::FeatureNotInactive as u32) + ) + ); + // Revoke a feature activation let transaction = Transaction::new_signed_with_payer( &[revoke( From 6b53fc5753dee1a5c3d1de87a46da42b65c8bf9b Mon Sep 17 00:00:00 2001 From: Joe Date: Thu, 10 Aug 2023 16:21:33 -0600 Subject: [PATCH 5/7] added rent transfer helper --- feature-gate/program/src/instruction.rs | 20 +++++++++++++- feature-gate/program/tests/functional.rs | 33 +++++++----------------- 2 files changed, 29 insertions(+), 24 deletions(-) diff --git a/feature-gate/program/src/instruction.rs b/feature-gate/program/src/instruction.rs index 2868da38cb1..68a0be10292 100644 --- a/feature-gate/program/src/instruction.rs +++ b/feature-gate/program/src/instruction.rs @@ -3,9 +3,11 @@ use { borsh::{BorshDeserialize, BorshSchema, BorshSerialize}, solana_program::{ + feature::Feature, instruction::{AccountMeta, Instruction}, pubkey::Pubkey, - system_program, + rent::Rent, + system_instruction, system_program, }, }; @@ -51,6 +53,22 @@ pub fn activate(program_id: &Pubkey, feature: &Pubkey, authority: &Pubkey) -> In } } +/// Creates a set of two instructions: +/// * One to fund the feature account with rent-exempt lamports +/// * Another is the Feature Gate Program's 'Activate' instruction +pub fn activate_with_rent_transfer( + program_id: &Pubkey, + feature: &Pubkey, + authority: &Pubkey, + payer: &Pubkey, +) -> [Instruction; 2] { + let lamports = Rent::default().minimum_balance(Feature::size_of()); + [ + system_instruction::transfer(payer, feature, lamports), + activate(program_id, feature, authority), + ] +} + /// Creates a 'RevokePendingActivation' instruction. pub fn revoke( program_id: &Pubkey, diff --git a/feature-gate/program/tests/functional.rs b/feature-gate/program/tests/functional.rs index c0e73b177b7..4cf4d7ec3f3 100644 --- a/feature-gate/program/tests/functional.rs +++ b/feature-gate/program/tests/functional.rs @@ -1,4 +1,4 @@ -// #![cfg(feature = "test-sbf")] +#![cfg(feature = "test-sbf")] use { solana_program::instruction::InstructionError, @@ -13,7 +13,7 @@ use { }, spl_feature_gate::{ error::FeatureGateError, - instruction::{activate, revoke}, + instruction::{activate, activate_with_rent_transfer, revoke}, }, }; @@ -21,21 +21,14 @@ async fn setup_feature( context: &mut ProgramTestContext, feature_keypair: &Keypair, authority_keypair: &Keypair, - rent_lamports: u64, ) { let transaction = Transaction::new_signed_with_payer( - &[ - system_instruction::transfer( - &context.payer.pubkey(), - &feature_keypair.pubkey(), - rent_lamports, - ), - activate( - &spl_feature_gate::id(), - &feature_keypair.pubkey(), - &authority_keypair.pubkey(), - ), - ], + &activate_with_rent_transfer( + &spl_feature_gate::id(), + &feature_keypair.pubkey(), + &authority_keypair.pubkey(), + &context.payer.pubkey(), + ), Some(&context.payer.pubkey()), &[&context.payer, feature_keypair, authority_keypair], context.last_blockhash, @@ -243,15 +236,9 @@ async fn test_revoke() { let mut context = program_test.start_with_context().await; let rent = context.banks_client.get_rent().await.unwrap(); - let rent_lamports = rent.minimum_balance(Feature::size_of()); + let rent_lamports = rent.minimum_balance(Feature::size_of()); // For checking account balance later - setup_feature( - &mut context, - &feature_keypair, - &authority_keypair, - rent_lamports, - ) - .await; + setup_feature(&mut context, &feature_keypair, &authority_keypair).await; // Revoke: Fail authority not signer let mut revoke_ix = revoke( From f8f86c4de373169b78bb5c13f610a6ac27d4a2f7 Mon Sep 17 00:00:00 2001 From: Joe Date: Thu, 10 Aug 2023 16:37:25 -0600 Subject: [PATCH 6/7] bye borsh --- Cargo.lock | 1 - feature-gate/program/Cargo.toml | 1 - feature-gate/program/src/instruction.rs | 55 ++++++++++++++++++------- feature-gate/program/src/processor.rs | 3 +- 4 files changed, 40 insertions(+), 20 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d6276621de4..683b1bbf374 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6527,7 +6527,6 @@ dependencies = [ name = "spl-feature-gate" version = "0.1.0" dependencies = [ - "borsh 0.10.3", "solana-program", "solana-program-test", "solana-sdk", diff --git a/feature-gate/program/Cargo.toml b/feature-gate/program/Cargo.toml index 247dce9dab3..5eac95e4bfd 100644 --- a/feature-gate/program/Cargo.toml +++ b/feature-gate/program/Cargo.toml @@ -12,7 +12,6 @@ no-entrypoint = [] test-sbf = [] [dependencies] -borsh = "0.10.3" solana-program = "1.16.3" spl-program-error = { version = "0.2.0", path = "../../libraries/program-error" } diff --git a/feature-gate/program/src/instruction.rs b/feature-gate/program/src/instruction.rs index 68a0be10292..92abebbf586 100644 --- a/feature-gate/program/src/instruction.rs +++ b/feature-gate/program/src/instruction.rs @@ -1,18 +1,16 @@ //! Program instructions -use { - borsh::{BorshDeserialize, BorshSchema, BorshSerialize}, - solana_program::{ - feature::Feature, - instruction::{AccountMeta, Instruction}, - pubkey::Pubkey, - rent::Rent, - system_instruction, system_program, - }, +use solana_program::{ + feature::Feature, + instruction::{AccountMeta, Instruction}, + program_error::ProgramError, + pubkey::Pubkey, + rent::Rent, + system_instruction, system_program, }; /// Feature Gate program instructions -#[derive(Clone, Debug, BorshSerialize, BorshDeserialize, BorshSchema, PartialEq)] +#[derive(Clone, Debug, PartialEq)] pub enum FeatureGateInstruction { /// Submit a feature for activation. /// @@ -37,6 +35,29 @@ pub enum FeatureGateInstruction { /// 2. `[s]` Authority RevokePendingActivation, } +impl FeatureGateInstruction { + /// Unpacks a byte buffer into a + /// [FeatureGateInstruction](enum.FeatureGateInstruction.html). + pub fn unpack(input: &[u8]) -> Result { + if input.is_empty() { + return Err(ProgramError::InvalidInstructionData); + } + match input[0] { + 0 => Ok(Self::Activate), + 1 => Ok(Self::RevokePendingActivation), + _ => Err(ProgramError::InvalidInstructionData), + } + } + + /// Packs a [FeatureGateInstruction](enum.FeatureGateInstruction.html) into + /// a byte buffer. + pub fn pack(&self) -> Vec { + match self { + Self::Activate => vec![0], + Self::RevokePendingActivation => vec![1], + } + } +} /// Creates an 'Activate' instruction. pub fn activate(program_id: &Pubkey, feature: &Pubkey, authority: &Pubkey) -> Instruction { @@ -46,10 +67,12 @@ pub fn activate(program_id: &Pubkey, feature: &Pubkey, authority: &Pubkey) -> In AccountMeta::new_readonly(system_program::id(), false), ]; + let data = FeatureGateInstruction::Activate.pack(); + Instruction { program_id: *program_id, accounts, - data: FeatureGateInstruction::Activate.try_to_vec().unwrap(), + data, } } @@ -82,12 +105,12 @@ pub fn revoke( AccountMeta::new_readonly(*authority, false), ]; + let data = FeatureGateInstruction::RevokePendingActivation.pack(); + Instruction { program_id: *program_id, accounts, - data: FeatureGateInstruction::RevokePendingActivation - .try_to_vec() - .unwrap(), + data, } } @@ -96,8 +119,8 @@ mod test { use super::*; fn test_pack_unpack(instruction: &FeatureGateInstruction) { - let packed = instruction.try_to_vec().unwrap(); - let unpacked = FeatureGateInstruction::try_from_slice(&packed).unwrap(); + let packed = instruction.pack(); + let unpacked = FeatureGateInstruction::unpack(&packed).unwrap(); assert_eq!(instruction, &unpacked); } diff --git a/feature-gate/program/src/processor.rs b/feature-gate/program/src/processor.rs index f160de647c2..d214ad9818c 100644 --- a/feature-gate/program/src/processor.rs +++ b/feature-gate/program/src/processor.rs @@ -2,7 +2,6 @@ use { crate::{error::FeatureGateError, instruction::FeatureGateInstruction}, - borsh::BorshDeserialize, solana_program::{ account_info::{next_account_info, AccountInfo}, entrypoint::ProgramResult, @@ -82,7 +81,7 @@ pub fn process_revoke(program_id: &Pubkey, accounts: &[AccountInfo]) -> ProgramR /// Processes an [Instruction](enum.Instruction.html). pub fn process(program_id: &Pubkey, accounts: &[AccountInfo], input: &[u8]) -> ProgramResult { - let instruction = FeatureGateInstruction::try_from_slice(input)?; + let instruction = FeatureGateInstruction::unpack(input)?; match instruction { FeatureGateInstruction::Activate => { msg!("Instruction: Activate"); From c1ca9deed45722ee4481fbab8377ed3bdcd86f4f Mon Sep 17 00:00:00 2001 From: Joe Date: Thu, 10 Aug 2023 16:56:48 -0600 Subject: [PATCH 7/7] removed extra authority --- feature-gate/cli/src/main.rs | 82 ++++--------------- feature-gate/program/src/instruction.rs | 24 ++---- feature-gate/program/src/processor.rs | 6 +- feature-gate/program/tests/functional.rs | 100 ++++------------------- 4 files changed, 41 insertions(+), 171 deletions(-) diff --git a/feature-gate/cli/src/main.rs b/feature-gate/cli/src/main.rs index d1bd8a3e9fa..376558d31e6 100644 --- a/feature-gate/cli/src/main.rs +++ b/feature-gate/cli/src/main.rs @@ -81,34 +81,18 @@ fn main() -> Result<(), Box> { .index(1) .required(true) .help("Path to keypair of the feature"), - ) - .arg( - Arg::with_name("authority_keypair") - .value_name("AUTHORITY_KEYPAIR") - .validator(is_keypair) - .required(true) - .help("Path to keypair of the authority"), - ) - .arg( - Arg::with_name("payer_keypair") - .value_name("PAYER_KEYPAIR") - .validator(is_keypair) - .help( - "Path to keypair of the payer to fund the feature account (defaults \ - to authority)", - ), ), ) .subcommand( SubCommand::with_name("revoke") .about("Revoke a pending feature activation") .arg( - Arg::with_name("feature_id") - .value_name("FEATURE_ID") - .validator(is_valid_pubkey) + Arg::with_name("feature_keypair") + .value_name("FEATURE_KEYPAIR") + .validator(is_keypair) .index(1) .required(true) - .help("The address of the feature (feature ID)"), + .help("Path to keypair of the feature"), ) .arg( Arg::with_name("destination") @@ -117,13 +101,6 @@ fn main() -> Result<(), Box> { .index(2) .required(true) .help("The address of the destination for the refunded lamports"), - ) - .arg( - Arg::with_name("authority_keypair") - .value_name("AUTHORITY_KEYPAIR") - .validator(is_keypair) - .required(true) - .help("Path to keypair of the authority"), ), ) .get_matches(); @@ -158,30 +135,14 @@ fn main() -> Result<(), Box> { match (sub_command, sub_matches) { ("activate", Some(arg_matches)) => { let feature_keypair = keypair_of(arg_matches, "feature_keypair").unwrap(); - let authority_keypair = keypair_of(arg_matches, "authority_keypair").unwrap(); - let payer_keypair = keypair_of(arg_matches, "payer_keypair") - .unwrap_or(authority_keypair.insecure_clone()); - process_activate( - &rpc_client, - &config, - &feature_keypair, - &payer_keypair, - &authority_keypair, - ) + process_activate(&rpc_client, &config, &feature_keypair) } ("revoke", Some(arg_matches)) => { - let feature_id = pubkey_of(arg_matches, "feature_id").unwrap(); + let feature_keypair = keypair_of(arg_matches, "feature_keypair").unwrap(); let destination = pubkey_of(arg_matches, "destination").unwrap(); - let authority_keypair = keypair_of(arg_matches, "authority_keypair").unwrap(); - process_revoke( - &rpc_client, - &config, - &feature_id, - &destination, - &authority_keypair, - ) + process_revoke(&rpc_client, &config, &feature_keypair, &destination) } _ => unreachable!(), } @@ -191,14 +152,10 @@ fn process_activate( rpc_client: &RpcClient, config: &Config, feature_keypair: &Keypair, - payer_keypair: &Keypair, - authority_keypair: &Keypair, ) -> Result<(), Box> { println!(); println!("Activating feature..."); println!("Feature ID: {}", feature_keypair.pubkey()); - println!("Payer: {}", payer_keypair.pubkey()); - println!("Authority: {}", authority_keypair.pubkey()); println!(); println!("JSON RPC URL: {}", config.json_rpc_url); println!(); @@ -208,18 +165,14 @@ fn process_activate( let transaction = Transaction::new_signed_with_payer( &[ system_instruction::transfer( - &payer_keypair.pubkey(), + &config.keypair.pubkey(), &feature_keypair.pubkey(), rent_lamports, ), - activate( - &spl_feature_gate::id(), - &feature_keypair.pubkey(), - &authority_keypair.pubkey(), - ), + activate(&spl_feature_gate::id(), &feature_keypair.pubkey()), ], - Some(&payer_keypair.pubkey()), - &[payer_keypair, feature_keypair, authority_keypair], + Some(&config.keypair.pubkey()), + &[config.keypair.as_ref(), feature_keypair], rpc_client.get_latest_blockhash()?, ); rpc_client.send_and_confirm_transaction_with_spinner(&transaction)?; @@ -232,15 +185,13 @@ fn process_activate( fn process_revoke( rpc_client: &RpcClient, config: &Config, - feature_id: &Pubkey, + feature_keypair: &Keypair, destination: &Pubkey, - authority_keypair: &Keypair, ) -> Result<(), Box> { println!(); println!("Revoking feature..."); - println!("Feature ID: {}", feature_id); + println!("Feature ID: {}", feature_keypair.pubkey()); println!("Destination: {}", destination); - println!("Authority: {}", authority_keypair.pubkey()); println!(); println!("JSON RPC URL: {}", config.json_rpc_url); println!(); @@ -248,12 +199,11 @@ fn process_revoke( let transaction = Transaction::new_signed_with_payer( &[revoke( &spl_feature_gate::id(), - feature_id, + &feature_keypair.pubkey(), destination, - &authority_keypair.pubkey(), )], - Some(&authority_keypair.pubkey()), - &[authority_keypair], + Some(&config.keypair.pubkey()), + &[config.keypair.as_ref()], rpc_client.get_latest_blockhash()?, ); rpc_client.send_and_confirm_transaction_with_spinner(&transaction)?; diff --git a/feature-gate/program/src/instruction.rs b/feature-gate/program/src/instruction.rs index 92abebbf586..59cfe7c9da1 100644 --- a/feature-gate/program/src/instruction.rs +++ b/feature-gate/program/src/instruction.rs @@ -22,17 +22,15 @@ pub enum FeatureGateInstruction { /// /// Accounts expected by this instruction: /// - /// 0. `[ws]` Feature account (must be a system account) - /// 1. `[s]` Authority - /// 2. `[]` System program + /// 0. `[w+s]` Feature account (must be a system account) + /// 1. `[]` System program Activate, /// Revoke a pending feature activation. /// /// Accounts expected by this instruction: /// - /// 0. `[w]` Feature account + /// 0. `[w+s]` Feature account /// 1. `[w]` Destination (for rent lamports) - /// 2. `[s]` Authority RevokePendingActivation, } impl FeatureGateInstruction { @@ -60,10 +58,9 @@ impl FeatureGateInstruction { } /// Creates an 'Activate' instruction. -pub fn activate(program_id: &Pubkey, feature: &Pubkey, authority: &Pubkey) -> Instruction { +pub fn activate(program_id: &Pubkey, feature: &Pubkey) -> Instruction { let accounts = vec![ AccountMeta::new(*feature, true), - AccountMeta::new_readonly(*authority, true), AccountMeta::new_readonly(system_program::id(), false), ]; @@ -82,27 +79,20 @@ pub fn activate(program_id: &Pubkey, feature: &Pubkey, authority: &Pubkey) -> In pub fn activate_with_rent_transfer( program_id: &Pubkey, feature: &Pubkey, - authority: &Pubkey, payer: &Pubkey, ) -> [Instruction; 2] { let lamports = Rent::default().minimum_balance(Feature::size_of()); [ system_instruction::transfer(payer, feature, lamports), - activate(program_id, feature, authority), + activate(program_id, feature), ] } /// Creates a 'RevokePendingActivation' instruction. -pub fn revoke( - program_id: &Pubkey, - feature: &Pubkey, - destination: &Pubkey, - authority: &Pubkey, -) -> Instruction { +pub fn revoke(program_id: &Pubkey, feature: &Pubkey, destination: &Pubkey) -> Instruction { let accounts = vec![ - AccountMeta::new(*feature, false), + AccountMeta::new(*feature, true), AccountMeta::new(*destination, false), - AccountMeta::new_readonly(*authority, false), ]; let data = FeatureGateInstruction::RevokePendingActivation.pack(); diff --git a/feature-gate/program/src/processor.rs b/feature-gate/program/src/processor.rs index d214ad9818c..c9b7f09af7f 100644 --- a/feature-gate/program/src/processor.rs +++ b/feature-gate/program/src/processor.rs @@ -19,10 +19,9 @@ pub fn process_activate(program_id: &Pubkey, accounts: &[AccountInfo]) -> Progra let account_info_iter = &mut accounts.iter(); let feature_info = next_account_info(account_info_iter)?; - let authority_info = next_account_info(account_info_iter)?; let _system_program_info = next_account_info(account_info_iter)?; - if !feature_info.is_signer || !authority_info.is_signer { + if !feature_info.is_signer { return Err(ProgramError::MissingRequiredSignature); } @@ -48,9 +47,8 @@ pub fn process_revoke(program_id: &Pubkey, accounts: &[AccountInfo]) -> ProgramR let feature_info = next_account_info(account_info_iter)?; let destination_info = next_account_info(account_info_iter)?; - let authority_info = next_account_info(account_info_iter)?; - if !authority_info.is_signer { + if !feature_info.is_signer { return Err(ProgramError::MissingRequiredSignature); } diff --git a/feature-gate/program/tests/functional.rs b/feature-gate/program/tests/functional.rs index 4cf4d7ec3f3..b7ce9b5cac3 100644 --- a/feature-gate/program/tests/functional.rs +++ b/feature-gate/program/tests/functional.rs @@ -1,4 +1,4 @@ -#![cfg(feature = "test-sbf")] +// #![cfg(feature = "test-sbf")] use { solana_program::instruction::InstructionError, @@ -17,20 +17,15 @@ use { }, }; -async fn setup_feature( - context: &mut ProgramTestContext, - feature_keypair: &Keypair, - authority_keypair: &Keypair, -) { +async fn setup_feature(context: &mut ProgramTestContext, feature_keypair: &Keypair) { let transaction = Transaction::new_signed_with_payer( &activate_with_rent_transfer( &spl_feature_gate::id(), &feature_keypair.pubkey(), - &authority_keypair.pubkey(), &context.payer.pubkey(), ), Some(&context.payer.pubkey()), - &[&context.payer, feature_keypair, authority_keypair], + &[&context.payer, feature_keypair], context.last_blockhash, ); @@ -45,7 +40,6 @@ async fn setup_feature( async fn test_activate() { let mock_feature_keypair = Keypair::new(); let feature_keypair = Keypair::new(); - let authority_keypair = Keypair::new(); let mut program_test = ProgramTest::new( "spl_feature_gate", @@ -53,15 +47,6 @@ async fn test_activate() { processor!(spl_feature_gate::processor::process), ); - // Create the authority account with some lamports for transaction fees - program_test.add_account( - authority_keypair.pubkey(), - SolanaAccount { - lamports: 500_000_000, - ..SolanaAccount::default() - }, - ); - // Add a mock feature for testing later program_test.add_account( mock_feature_keypair.pubkey(), @@ -77,11 +62,7 @@ async fn test_activate() { let rent_lamports = rent.minimum_balance(Feature::size_of()); // Activate: Fail feature not signer - let mut activate_ix = activate( - &spl_feature_gate::id(), - &feature_keypair.pubkey(), - &authority_keypair.pubkey(), - ); + let mut activate_ix = activate(&spl_feature_gate::id(), &feature_keypair.pubkey()); activate_ix.accounts[0].is_signer = false; let transaction = Transaction::new_signed_with_payer( &[ @@ -93,38 +74,7 @@ async fn test_activate() { activate_ix, ], Some(&context.payer.pubkey()), - &[&context.payer, &authority_keypair], - context.last_blockhash, - ); - let error = context - .banks_client - .process_transaction(transaction) - .await - .unwrap_err() - .unwrap(); - assert_eq!( - error, - TransactionError::InstructionError(1, InstructionError::MissingRequiredSignature) - ); - - // Activate: Fail authority not signer - let mut activate_ix = activate( - &spl_feature_gate::id(), - &feature_keypair.pubkey(), - &authority_keypair.pubkey(), - ); - activate_ix.accounts[1].is_signer = false; - let transaction = Transaction::new_signed_with_payer( - &[ - system_instruction::transfer( - &context.payer.pubkey(), - &feature_keypair.pubkey(), - rent_lamports, - ), - activate_ix, - ], - Some(&context.payer.pubkey()), - &[&context.payer, &feature_keypair], + &[&context.payer], context.last_blockhash, ); let error = context @@ -143,10 +93,9 @@ async fn test_activate() { &[activate( &spl_feature_gate::id(), &mock_feature_keypair.pubkey(), - &authority_keypair.pubkey(), )], - Some(&authority_keypair.pubkey()), - &[&mock_feature_keypair, &authority_keypair], + Some(&context.payer.pubkey()), + &[&context.payer, &mock_feature_keypair], context.last_blockhash, ); let error = context @@ -171,14 +120,10 @@ async fn test_activate() { &feature_keypair.pubkey(), rent_lamports, ), - activate( - &spl_feature_gate::id(), - &feature_keypair.pubkey(), - &authority_keypair.pubkey(), - ), + activate(&spl_feature_gate::id(), &feature_keypair.pubkey()), ], Some(&context.payer.pubkey()), - &[&context.payer, &feature_keypair, &authority_keypair], + &[&context.payer, &feature_keypair], context.last_blockhash, ); @@ -201,7 +146,6 @@ async fn test_activate() { #[tokio::test] async fn test_revoke() { let feature_keypair = Keypair::new(); - let authority_keypair = Keypair::new(); let destination = Pubkey::new_unique(); let mock_active_feature_keypair = Keypair::new(); @@ -211,15 +155,6 @@ async fn test_revoke() { processor!(spl_feature_gate::processor::process), ); - // Create the authority account with some lamports for transaction fees - program_test.add_account( - authority_keypair.pubkey(), - SolanaAccount { - lamports: 500_000_000, - ..SolanaAccount::default() - }, - ); - // Add a mock feature that might be active for testing later program_test.add_account( mock_active_feature_keypair.pubkey(), @@ -238,16 +173,15 @@ async fn test_revoke() { let rent = context.banks_client.get_rent().await.unwrap(); let rent_lamports = rent.minimum_balance(Feature::size_of()); // For checking account balance later - setup_feature(&mut context, &feature_keypair, &authority_keypair).await; + setup_feature(&mut context, &feature_keypair).await; - // Revoke: Fail authority not signer + // Revoke: Fail feature not signer let mut revoke_ix = revoke( &spl_feature_gate::id(), &feature_keypair.pubkey(), &destination, - &authority_keypair.pubkey(), ); - revoke_ix.accounts[2].is_signer = false; + revoke_ix.accounts[0].is_signer = false; let transaction = Transaction::new_signed_with_payer( &[revoke_ix], Some(&context.payer.pubkey()), @@ -271,10 +205,9 @@ async fn test_revoke() { &spl_feature_gate::id(), &mock_active_feature_keypair.pubkey(), &destination, - &authority_keypair.pubkey(), )], - Some(&authority_keypair.pubkey()), - &[&authority_keypair], + Some(&context.payer.pubkey()), + &[&context.payer, &mock_active_feature_keypair], context.last_blockhash, ); let error = context @@ -297,10 +230,9 @@ async fn test_revoke() { &spl_feature_gate::id(), &feature_keypair.pubkey(), &destination, - &authority_keypair.pubkey(), )], - Some(&authority_keypair.pubkey()), - &[&authority_keypair], + Some(&context.payer.pubkey()), + &[&context.payer, &feature_keypair], context.last_blockhash, );