diff --git a/feature-gate/README.md b/feature-gate/README.md index c0dbaf5787c..e83c14df86a 100644 --- a/feature-gate/README.md +++ b/feature-gate/README.md @@ -2,7 +2,8 @@ This program serves to manage new features on Solana. -It serves one purpose: revoking features that are pending activation. +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 diff --git a/feature-gate/program/Cargo.toml b/feature-gate/program/Cargo.toml index 5cf6233d029..a2dbcddf026 100644 --- a/feature-gate/program/Cargo.toml +++ b/feature-gate/program/Cargo.toml @@ -17,8 +17,8 @@ solana-program = "1.16.16" spl-program-error = { version = "0.3.0", path = "../../libraries/program-error" } [dev-dependencies] -solana-program-test = "1.16.16" solana-sdk = "1.16.16" +solana-program-test = "1.16.16" [lib] crate-type = ["cdylib", "lib"] diff --git a/feature-gate/program/src/error.rs b/feature-gate/program/src/error.rs index 731c61213a4..942f5ae134f 100644 --- a/feature-gate/program/src/error.rs +++ b/feature-gate/program/src/error.rs @@ -11,4 +11,10 @@ pub enum FeatureGateError { /// Feature already activated #[error("Feature already activated")] FeatureAlreadyActivated, + /// Incorrect feature ID + #[error("Incorrect feature ID")] + IncorrectFeatureId, + /// Invalid feature account + #[error("Invalid feature account")] + InvalidFeatureAccount, } diff --git a/feature-gate/program/src/feature_id.rs b/feature-gate/program/src/feature_id.rs new file mode 100644 index 00000000000..de9699c8d58 --- /dev/null +++ b/feature-gate/program/src/feature_id.rs @@ -0,0 +1,37 @@ +//! Module for managing feature IDs + +// use { +// crate::error::FeatureGateError, +// solana_program::{program_error::ProgramError, pubkey::Pubkey}, +// solana_sdk::feature_set::FEATURE_NAMES, +// }; + +// /// Returns `true` if the feature ID exists in the current feature set. +// fn feature_exists(feature_id: &Pubkey) -> bool { +// FEATURE_NAMES.iter().any(|(id, _)| id == feature_id) +// } + +// /// Derives the feature ID from a authority's address. +// pub fn derive_feature_id(authority: &Pubkey) -> Result +// { let nonce = 0u16; // u16::MAX = 65_535 features! +// let mut feature_id = Pubkey::find_program_address(&[b"feature"], +// &crate::id()).0; while feature_exists(&feature_id) { +// nonce.checked_add(1).ok_or(FeatureGateError::Overflow)?; +// feature_id = Pubkey::find_program_address( +// &[b"feature", &nonce.to_le_bytes(), &authority.to_bytes()], +// &crate::id(), +// ) +// .0; +// } +// Ok(feature_id) +// } + +use solana_program::{program_error::ProgramError, pubkey::Pubkey}; + +/// Derives the feature ID from a authority's address. +pub fn derive_feature_id(authority: &Pubkey) -> Result<(Pubkey, u8), ProgramError> { + Ok(Pubkey::find_program_address( + &[b"feature", authority.as_ref()], + &crate::id(), + )) +} diff --git a/feature-gate/program/src/instruction.rs b/feature-gate/program/src/instruction.rs index 2e787ffb48b..b4ac5adeb31 100644 --- a/feature-gate/program/src/instruction.rs +++ b/feature-gate/program/src/instruction.rs @@ -3,9 +3,12 @@ use { num_enum::{IntoPrimitive, TryFromPrimitive}, solana_program::{ + feature::Feature, instruction::{AccountMeta, Instruction}, program_error::ProgramError, pubkey::Pubkey, + rent::Rent, + system_instruction, system_program, }, }; @@ -13,6 +16,20 @@ use { #[derive(Clone, Debug, PartialEq, IntoPrimitive, TryFromPrimitive)] #[repr(u8)] 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. `[w]` Feature account (must be a system account) + /// 1. `[s]` Feature activation authority (can be multisig) + /// 2. `[]` System program + ActivateFeature, /// Revoke a pending feature activation. /// /// A "pending" feature activation is a feature account that has been @@ -23,8 +40,8 @@ pub enum FeatureGateInstruction { /// /// Accounts expected by this instruction: /// - /// 0. `[w+s]` Feature account - /// 1. `[w]` Destination (for rent lamports) + /// 0. `[w]` Feature account + /// 1. `[w+s]` Feature activation authority (can be multisig) RevokePendingActivation, } impl FeatureGateInstruction { @@ -44,11 +61,42 @@ impl FeatureGateInstruction { } } +/// Creates an 'ActivateFeature' instruction. +pub fn activate_feature(feature_id: &Pubkey, authority: &Pubkey) -> Instruction { + let accounts = vec![ + AccountMeta::new(*feature_id, false), + AccountMeta::new_readonly(*authority, true), + AccountMeta::new_readonly(system_program::id(), false), + ]; + + let data = FeatureGateInstruction::ActivateFeature.pack(); + + Instruction { + program_id: crate::id(), + accounts, + data, + } +} + +/// Creates a set of two instructions: +/// * One to fund the feature account with rent-exempt lamports +/// * Another is the Feature Gate Program's 'ActivateFeature' instruction +pub fn activate_feature_with_rent_transfer( + feature_id: &Pubkey, + authority: &Pubkey, +) -> [Instruction; 2] { + let lamports = Rent::default().minimum_balance(Feature::size_of()); + [ + system_instruction::transfer(authority, feature_id, lamports), + activate_feature(feature_id, authority), + ] +} + /// Creates a 'RevokePendingActivation' instruction. -pub fn revoke_pending_activation(feature: &Pubkey, destination: &Pubkey) -> Instruction { +pub fn revoke_pending_activation(feature_id: &Pubkey, authority: &Pubkey) -> Instruction { let accounts = vec![ - AccountMeta::new(*feature, true), - AccountMeta::new(*destination, false), + AccountMeta::new(*feature_id, false), + AccountMeta::new(*authority, true), ]; let data = FeatureGateInstruction::RevokePendingActivation.pack(); @@ -70,6 +118,11 @@ mod test { assert_eq!(instruction, &unpacked); } + #[test] + fn test_pack_unpack_activate() { + test_pack_unpack(&FeatureGateInstruction::ActivateFeature); + } + #[test] fn test_pack_unpack_revoke() { test_pack_unpack(&FeatureGateInstruction::RevokePendingActivation); diff --git a/feature-gate/program/src/lib.rs b/feature-gate/program/src/lib.rs index e6be6428a15..100681fe714 100644 --- a/feature-gate/program/src/lib.rs +++ b/feature-gate/program/src/lib.rs @@ -6,6 +6,7 @@ #[cfg(not(feature = "no-entrypoint"))] mod entrypoint; pub mod error; +pub mod feature_id; pub mod instruction; pub mod processor; diff --git a/feature-gate/program/src/processor.rs b/feature-gate/program/src/processor.rs index c1ade8311fc..3e960b3c208 100644 --- a/feature-gate/program/src/processor.rs +++ b/feature-gate/program/src/processor.rs @@ -1,18 +1,58 @@ //! Program state processor use { - crate::{error::FeatureGateError, instruction::FeatureGateInstruction}, + crate::{ + error::FeatureGateError, feature_id::derive_feature_id, instruction::FeatureGateInstruction, + }, solana_program::{ account_info::{next_account_info, AccountInfo}, entrypoint::ProgramResult, feature::Feature, msg, + program::invoke_signed, program_error::ProgramError, pubkey::Pubkey, - system_program, + system_instruction, system_program, }, }; +/// Processes an [ActivateFeature](enum.FeatureGateInstruction.html) +/// instruction. +pub fn process_activate_feature(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 !authority_info.is_signer { + return Err(ProgramError::MissingRequiredSignature); + } + + if feature_info.owner != &system_program::id() { + return Err(FeatureGateError::InvalidFeatureAccount.into()); + } + + let (feature_id, feature_id_bump) = derive_feature_id(authority_info.key)?; + + if feature_info.key != &feature_id { + return Err(FeatureGateError::IncorrectFeatureId.into()); + } + + invoke_signed( + &system_instruction::allocate(feature_info.key, Feature::size_of() as u64), + &[feature_info.clone()], + &[&[b"feature", authority_info.key.as_ref(), &[feature_id_bump]]], + )?; + invoke_signed( + &system_instruction::assign(feature_info.key, program_id), + &[feature_info.clone()], + &[&[b"feature", authority_info.key.as_ref(), &[feature_id_bump]]], + )?; + + Ok(()) +} + /// Processes an [RevokePendingActivation](enum.FeatureGateInstruction.html) /// instruction. pub fn process_revoke_pending_activation( @@ -22,9 +62,9 @@ pub fn process_revoke_pending_activation( 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 !feature_info.is_signer { + if !authority_info.is_signer { return Err(ProgramError::MissingRequiredSignature); } @@ -39,13 +79,13 @@ pub fn process_revoke_pending_activation( return Err(FeatureGateError::FeatureAlreadyActivated.into()); } - let new_destination_lamports = feature_info + let new_authority_lamports = feature_info .lamports() - .checked_add(destination_info.lamports()) + .checked_add(authority_info.lamports()) .ok_or::(FeatureGateError::Overflow.into())?; **feature_info.try_borrow_mut_lamports()? = 0; - **destination_info.try_borrow_mut_lamports()? = new_destination_lamports; + **authority_info.try_borrow_mut_lamports()? = new_authority_lamports; feature_info.realloc(0, true)?; feature_info.assign(&system_program::id()); @@ -57,6 +97,10 @@ pub fn process_revoke_pending_activation( pub fn process(program_id: &Pubkey, accounts: &[AccountInfo], input: &[u8]) -> ProgramResult { let instruction = FeatureGateInstruction::unpack(input)?; match instruction { + FeatureGateInstruction::ActivateFeature => { + msg!("Instruction: ActivateFeature"); + process_activate_feature(program_id, accounts) + } FeatureGateInstruction::RevokePendingActivation => { msg!("Instruction: RevokePendingActivation"); process_revoke_pending_activation(program_id, accounts) diff --git a/feature-gate/program/tests/functional.rs b/feature-gate/program/tests/functional.rs index a1266c24a26..183f1a4ad25 100644 --- a/feature-gate/program/tests/functional.rs +++ b/feature-gate/program/tests/functional.rs @@ -1,31 +1,154 @@ -// #![cfg(feature = "test-sbf")] +#![cfg(feature = "test-sbf")] +#![allow(clippy::integer_arithmetic)] use { solana_program::instruction::InstructionError, solana_program_test::{processor, tokio, ProgramTest, ProgramTestContext}, solana_sdk::{ account::Account as SolanaAccount, - feature::{activate_with_lamports, Feature}, + feature::Feature, pubkey::Pubkey, signature::{Keypair, Signer}, + system_instruction, system_program, transaction::{Transaction, TransactionError}, }, - spl_feature_gate::{error::FeatureGateError, instruction::revoke_pending_activation}, + spl_feature_gate::{ + error::FeatureGateError, + feature_id::derive_feature_id, + instruction::{ + activate_feature, activate_feature_with_rent_transfer, revoke_pending_activation, + }, + }, }; -async fn setup_pending_feature( - context: &mut ProgramTestContext, - feature_keypair: &Keypair, - rent_lamports: u64, -) { +async fn setup_pending_feature(context: &mut ProgramTestContext, feature_id: &Pubkey) { let transaction = Transaction::new_signed_with_payer( - &activate_with_lamports( - &feature_keypair.pubkey(), + &activate_feature_with_rent_transfer(feature_id, &context.payer.pubkey()), + Some(&context.payer.pubkey()), + &[&context.payer], + context.last_blockhash, + ); + + context + .banks_client + .process_transaction(transaction) + .await + .unwrap(); +} + +#[tokio::test] +async fn test_activate_feature() { + let mock_invalid_feature = Pubkey::new_unique(); + let mock_invalid_signer = Keypair::new(); + + let mut program_test = ProgramTest::new( + "spl_feature_gate", + spl_feature_gate::id(), + processor!(spl_feature_gate::processor::process), + ); + + // Need to fund this account for a test transfer later + program_test.add_account( + mock_invalid_signer.pubkey(), + SolanaAccount { + lamports: 500_000_000, + owner: system_program::id(), + ..SolanaAccount::default() + }, + ); + // Add a mock account that's NOT a valid feature account for testing later + program_test.add_account( + mock_invalid_feature, + 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()); + + let (feature_id, _) = derive_feature_id(&context.payer.pubkey()).unwrap(); + + // Fail: incorrect feature ID + let incorrect_id = Pubkey::new_unique(); + let transaction = Transaction::new_signed_with_payer( + &[activate_feature(&incorrect_id, &context.payer.pubkey())], + 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::Custom(FeatureGateError::IncorrectFeatureId as u32) + ) + ); + + // Fail: authority not signer + let mut activate_ix = activate_feature(&feature_id, &context.payer.pubkey()); + activate_ix.accounts[1].is_signer = false; + let transaction = Transaction::new_signed_with_payer( + &[ + system_instruction::transfer(&mock_invalid_signer.pubkey(), &feature_id, rent_lamports), + activate_ix, + ], + Some(&mock_invalid_signer.pubkey()), + &[&mock_invalid_signer], + context.last_blockhash, + ); + let error = context + .banks_client + .process_transaction(transaction) + .await + .unwrap_err() + .unwrap(); + assert_eq!( + error, + TransactionError::InstructionError(1, InstructionError::MissingRequiredSignature) + ); + + // Fail: feature not owned by system program + let transaction = Transaction::new_signed_with_payer( + &[activate_feature( + &mock_invalid_feature, &context.payer.pubkey(), - rent_lamports, - ), + )], + 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::Custom(FeatureGateError::InvalidFeatureAccount as u32), + ) + ); + + // Success: Submit a feature for activation + let transaction = Transaction::new_signed_with_payer( + &[ + system_instruction::transfer(&context.payer.pubkey(), &feature_id, rent_lamports), + activate_feature(&feature_id, &context.payer.pubkey()), + ], Some(&context.payer.pubkey()), - &[&context.payer, feature_keypair], + &[&context.payer], context.last_blockhash, ); @@ -34,13 +157,42 @@ async fn setup_pending_feature( .process_transaction(transaction) .await .unwrap(); + + // Confirm feature account exists with proper configurations + let feature_account = context + .banks_client + .get_account(feature_id) + .await + .unwrap() + .unwrap(); + assert_eq!(feature_account.owner, spl_feature_gate::id()); + + // Cannot activate the same feature again + let transaction = Transaction::new_signed_with_payer( + &[activate_feature(&feature_id, &context.payer.pubkey())], + 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::Custom(FeatureGateError::InvalidFeatureAccount as u32) + ) + ); } #[tokio::test] async fn test_revoke_pending_activation() { - let feature_keypair = Keypair::new(); - let destination = Pubkey::new_unique(); let mock_active_feature_keypair = Keypair::new(); + let mock_invalid_signer = Keypair::new(); let mut program_test = ProgramTest::new( "spl_feature_gate", @@ -48,6 +200,15 @@ async fn test_revoke_pending_activation() { processor!(spl_feature_gate::processor::process), ); + // Need to fund this account for a test transfer later + program_test.add_account( + mock_invalid_signer.pubkey(), + SolanaAccount { + lamports: 500_000_000, + owner: system_program::id(), + ..SolanaAccount::default() + }, + ); // Add a mock _active_ feature for testing later program_test.add_account( mock_active_feature_keypair.pubkey(), @@ -66,15 +227,17 @@ async fn test_revoke_pending_activation() { let rent = context.banks_client.get_rent().await.unwrap(); let rent_lamports = rent.minimum_balance(Feature::size_of()); // For checking account balance later - setup_pending_feature(&mut context, &feature_keypair, rent_lamports).await; + let (feature_id, _) = derive_feature_id(&context.payer.pubkey()).unwrap(); + + setup_pending_feature(&mut context, &feature_id).await; - // Fail: feature not signer - let mut revoke_ix = revoke_pending_activation(&feature_keypair.pubkey(), &destination); - revoke_ix.accounts[0].is_signer = false; + // Fail: authority not signer + let mut revoke_ix = revoke_pending_activation(&feature_id, &context.payer.pubkey()); + revoke_ix.accounts[1].is_signer = false; let transaction = Transaction::new_signed_with_payer( &[revoke_ix], - Some(&context.payer.pubkey()), - &[&context.payer], + Some(&mock_invalid_signer.pubkey()), + &[&mock_invalid_signer], context.last_blockhash, ); let error = context @@ -92,10 +255,10 @@ async fn test_revoke_pending_activation() { let transaction = Transaction::new_signed_with_payer( &[revoke_pending_activation( &mock_active_feature_keypair.pubkey(), - &destination, + &context.payer.pubkey(), )], Some(&context.payer.pubkey()), - &[&context.payer, &mock_active_feature_keypair], + &[&context.payer], context.last_blockhash, ); let error = context @@ -113,13 +276,18 @@ async fn test_revoke_pending_activation() { ); // Success: Revoke a feature activation + let payer_lamports = context + .banks_client + .get_balance(context.payer.pubkey()) + .await + .unwrap(); let transaction = Transaction::new_signed_with_payer( &[revoke_pending_activation( - &feature_keypair.pubkey(), - &destination, + &feature_id, + &context.payer.pubkey(), )], Some(&context.payer.pubkey()), - &[&context.payer, &feature_keypair], + &[&context.payer], context.last_blockhash, ); @@ -130,17 +298,16 @@ async fn test_revoke_pending_activation() { .unwrap(); // Confirm feature account was closed and destination account received lamports - let feature_account = context - .banks_client - .get_account(feature_keypair.pubkey()) - .await - .unwrap(); + let feature_account = context.banks_client.get_account(feature_id).await.unwrap(); assert!(feature_account.is_none()); let destination_account = context .banks_client - .get_account(destination) + .get_account(context.payer.pubkey()) .await .unwrap() .unwrap(); - assert_eq!(destination_account.lamports, rent_lamports); + assert_eq!( + destination_account.lamports, + payer_lamports + rent_lamports - 5000 + ); // 5000 lamports for the transaction fee }