From 4b65e32f22ef0da600606a41bba0624e1f1cb2dd Mon Sep 17 00:00:00 2001 From: Michael Vines Date: Fri, 30 Oct 2020 13:40:55 -0700 Subject: [PATCH] Move Feature struct to solana-program --- cli/src/feature.rs | 19 +++---- runtime/src/accounts.rs | 2 +- runtime/src/bank.rs | 25 ++++++---- runtime/src/builtins.rs | 4 +- runtime/src/genesis_utils.rs | 16 +++--- runtime/src/lib.rs | 4 -- runtime/src/message_processor.rs | 8 ++- sdk/program/src/feature.rs | 86 ++++++++++++++++++++++++++++++++ sdk/program/src/lib.rs | 1 + sdk/src/feature.rs | 80 +++++++---------------------- 10 files changed, 140 insertions(+), 105 deletions(-) create mode 100644 sdk/program/src/feature.rs diff --git a/cli/src/feature.rs b/cli/src/feature.rs index c47e731978..508e7cd241 100644 --- a/cli/src/feature.rs +++ b/cli/src/feature.rs @@ -15,7 +15,6 @@ use solana_sdk::{ feature_set::FEATURE_NAMES, message::Message, pubkey::Pubkey, - system_instruction, transaction::Transaction, }; use std::{collections::HashMap, fmt, sync::Arc}; @@ -281,7 +280,7 @@ fn process_status( let feature_id = &feature_ids[i]; let feature_name = FEATURE_NAMES.get(feature_id).unwrap(); if let Some(account) = account { - if let Some(feature) = Feature::from_account(&account) { + if let Some(feature) = feature::from_account(&account) { let feature_status = match feature.activated_at { None => CliFeatureStatus::Pending, Some(activation_slot) => CliFeatureStatus::Active(activation_slot), @@ -322,7 +321,7 @@ fn process_activate( .next() .unwrap(); if let Some(account) = account { - if Feature::from_account(&account).is_some() { + if feature::from_account(&account).is_some() { return Err(format!("{} has already been activated", feature_id).into()); } } @@ -342,15 +341,11 @@ fn process_activate( &config.signers[0].pubkey(), |lamports| { Message::new( - &[ - system_instruction::transfer( - &config.signers[0].pubkey(), - &feature_id, - lamports, - ), - system_instruction::allocate(&feature_id, Feature::size_of() as u64), - system_instruction::assign(&feature_id, &feature::id()), - ], + &feature::activate_with_lamports( + &feature_id, + &config.signers[0].pubkey(), + lamports, + ), Some(&config.signers[0].pubkey()), ) }, diff --git a/runtime/src/accounts.rs b/runtime/src/accounts.rs index 89b651e137..53e9b0d43c 100644 --- a/runtime/src/accounts.rs +++ b/runtime/src/accounts.rs @@ -6,7 +6,6 @@ use crate::{ append_vec::StoredAccount, bank::{HashAgeKind, TransactionProcessResult}, blockhash_queue::BlockhashQueue, - feature_set::{self, FeatureSet}, rent_collector::RentCollector, system_instruction_processor::{get_system_account_kind, SystemAccountKind}, transaction_utils::OrderedIterator, @@ -18,6 +17,7 @@ use solana_sdk::{ account::Account, account_utils::StateMut, clock::{Epoch, Slot}, + feature_set::{self, FeatureSet}, fee_calculator::{FeeCalculator, FeeConfig}, genesis_config::ClusterType, hash::Hash, diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index ab8d2fb0f3..fa1aa477fe 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -12,8 +12,6 @@ use crate::{ blockhash_queue::BlockhashQueue, builtins, epoch_stakes::{EpochStakes, NodeVoteAccounts}, - feature::Feature, - feature_set::{self, FeatureSet}, instruction_recorder::InstructionRecorder, log_collector::LogCollector, message_processor::{Executors, MessageProcessor}, @@ -40,6 +38,8 @@ use solana_sdk::{ }, epoch_info::EpochInfo, epoch_schedule::EpochSchedule, + feature, + feature_set::{self, FeatureSet}, fee_calculator::{FeeCalculator, FeeConfig, FeeRateGovernor}, genesis_config::{ClusterType, GenesisConfig}, hard_forks::HardForks, @@ -3919,13 +3919,13 @@ impl Bank { for feature_id in &self.feature_set.inactive { let mut activated = None; if let Some(mut account) = self.get_account(feature_id) { - if let Some(mut feature) = Feature::from_account(&account) { + if let Some(mut feature) = feature::from_account(&account) { match feature.activated_at { None => { if allow_new_activations { // Feature has been requested, activate it now feature.activated_at = Some(slot); - if feature.to_account(&mut account).is_some() { + if feature::to_account(&feature, &mut account).is_some() { self.store_account(feature_id, &account); } newly_activated.insert(*feature_id); @@ -4103,6 +4103,7 @@ mod tests { account_utils::StateMut, clock::{DEFAULT_SLOTS_PER_EPOCH, DEFAULT_TICKS_PER_SLOT}, epoch_schedule::MINIMUM_SLOTS_PER_EPOCH, + feature::Feature, genesis_config::create_genesis_config, instruction::{AccountMeta, CompiledInstruction, Instruction, InstructionError}, keyed_account::KeyedAccount, @@ -9571,13 +9572,13 @@ mod tests { // Request `test_feature` activation let feature = Feature::default(); assert_eq!(feature.activated_at, None); - bank.store_account(&test_feature, &feature.create_account(42)); + bank.store_account(&test_feature, &feature::create_account(&feature, 42)); // Run `compute_active_feature_set` disallowing new activations let new_activations = bank.compute_active_feature_set(false); assert!(new_activations.is_empty()); assert!(!bank.feature_set.is_active(&test_feature)); - let feature = Feature::from_account(&bank.get_account(&test_feature).expect("get_account")) + let feature = feature::from_account(&bank.get_account(&test_feature).expect("get_account")) .expect("from_account"); assert_eq!(feature.activated_at, None); @@ -9585,7 +9586,7 @@ mod tests { let new_activations = bank.compute_active_feature_set(true); assert_eq!(new_activations.len(), 1); assert!(bank.feature_set.is_active(&test_feature)); - let feature = Feature::from_account(&bank.get_account(&test_feature).expect("get_account")) + let feature = feature::from_account(&bank.get_account(&test_feature).expect("get_account")) .expect("from_account"); assert_eq!(feature.activated_at, Some(1)); @@ -9720,12 +9721,14 @@ mod tests { assert_eq!(clock.unix_timestamp, bank.unix_timestamp_from_genesis()); // Request `timestamp_correction` activation - let feature = Feature { - activated_at: Some(bank.slot), - }; bank.store_account( &feature_set::timestamp_correction::id(), - &feature.create_account(42), + &feature::create_account( + &Feature { + activated_at: Some(bank.slot), + }, + 42, + ), ); bank.compute_active_feature_set(true); diff --git a/runtime/src/builtins.rs b/runtime/src/builtins.rs index 845438fc1a..8afbe1b88c 100644 --- a/runtime/src/builtins.rs +++ b/runtime/src/builtins.rs @@ -1,8 +1,8 @@ use crate::{ bank::{Builtin, Builtins}, - feature_set, system_instruction_processor, + system_instruction_processor, }; -use solana_sdk::{pubkey::Pubkey, system_program}; +use solana_sdk::{feature_set, pubkey::Pubkey, system_program}; /// Builtin programs that are always available fn genesis_builtins() -> Vec { diff --git a/runtime/src/genesis_utils.rs b/runtime/src/genesis_utils.rs index 9cb84259dd..083db2d70e 100644 --- a/runtime/src/genesis_utils.rs +++ b/runtime/src/genesis_utils.rs @@ -1,6 +1,7 @@ -use crate::{feature::Feature, feature_set::FeatureSet}; use solana_sdk::{ account::Account, + feature::{self, Feature}, + feature_set::FeatureSet, fee_calculator::FeeRateGovernor, genesis_config::{ClusterType, GenesisConfig}, pubkey::Pubkey, @@ -127,15 +128,14 @@ pub fn create_genesis_config_with_leader( pub fn activate_all_features(genesis_config: &mut GenesisConfig) { // Activate all features at genesis in development mode for feature_id in FeatureSet::default().inactive { - let feature = Feature { - activated_at: Some(0), - }; genesis_config.accounts.insert( feature_id, - feature.create_account(std::cmp::max( - genesis_config.rent.minimum_balance(Feature::size_of()), - 1, - )), + feature::create_account( + &Feature { + activated_at: Some(0), + }, + std::cmp::max(genesis_config.rent.minimum_balance(Feature::size_of()), 1), + ), ); } } diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index 0b994e3549..35bc6c730d 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -31,10 +31,6 @@ pub mod transaction_batch; pub mod transaction_utils; pub mod vote_sender_types; -// TODO: Refactor all feature users to reference the solana_sdk definitions directly and remove the -// next line -use solana_sdk::{feature, feature_set}; - extern crate solana_config_program; extern crate solana_stake_program; extern crate solana_vote_program; diff --git a/runtime/src/message_processor.rs b/runtime/src/message_processor.rs index 00e049a57e..13deecf374 100644 --- a/runtime/src/message_processor.rs +++ b/runtime/src/message_processor.rs @@ -1,15 +1,13 @@ use crate::{ - feature_set::{instructions_sysvar_enabled, FeatureSet}, - instruction_recorder::InstructionRecorder, - log_collector::LogCollector, - native_loader::NativeLoader, - rent_collector::RentCollector, + instruction_recorder::InstructionRecorder, log_collector::LogCollector, + native_loader::NativeLoader, rent_collector::RentCollector, }; use log::*; use serde::{Deserialize, Serialize}; use solana_sdk::{ account::Account, clock::Epoch, + feature_set::{instructions_sysvar_enabled, FeatureSet}, instruction::{CompiledInstruction, Instruction, InstructionError}, keyed_account::{create_keyed_readonly_accounts, KeyedAccount}, message::Message, diff --git a/sdk/program/src/feature.rs b/sdk/program/src/feature.rs new file mode 100644 index 0000000000..7586157c50 --- /dev/null +++ b/sdk/program/src/feature.rs @@ -0,0 +1,86 @@ +/// Runtime features. +/// +/// Feature activation is accomplished by: +/// 1. Activation is requested by the feature authority, who issues a transaction to create the +/// feature account. The newly created feature account will have the value of +/// `Feature::default()` +/// 2. When the next epoch is entered the runtime will check for new activation requests and +/// active them. When this occurs, the activation slot is recorded in the feature account +/// +use crate::{ + account_info::AccountInfo, clock::Slot, instruction::Instruction, program_error::ProgramError, + pubkey::Pubkey, rent::Rent, system_instruction, +}; + +crate::declare_id!("Feature111111111111111111111111111111111111"); + +#[derive(Default, Debug, Serialize, Deserialize, PartialEq)] +pub struct Feature { + pub activated_at: Option, +} + +impl Feature { + pub fn size_of() -> usize { + bincode::serialized_size(&Feature { + activated_at: Some(0), + }) + .unwrap() as usize + } + + pub fn from_account_info(account_info: &AccountInfo) -> Result { + if *account_info.owner != id() { + return Err(ProgramError::InvalidArgument); + } + bincode::deserialize(&account_info.data.borrow()).map_err(|_| ProgramError::InvalidArgument) + } +} + +/// Activate a feature +pub fn activate(feature_id: &Pubkey, funding_address: &Pubkey, rent: &Rent) -> Vec { + activate_with_lamports( + feature_id, + funding_address, + rent.minimum_balance(Feature::size_of()), + ) +} + +pub fn activate_with_lamports( + feature_id: &Pubkey, + funding_address: &Pubkey, + lamports: u64, +) -> Vec { + vec![ + system_instruction::transfer(funding_address, feature_id, lamports), + system_instruction::allocate(feature_id, Feature::size_of() as u64), + system_instruction::assign(feature_id, &id()), + ] +} + +#[cfg(test)] +mod test { + use super::*; + use solana_program::clock::Slot; + + #[test] + fn feature_sizeof() { + assert!( + Feature::size_of() >= bincode::serialized_size(&Feature::default()).unwrap() as usize + ); + assert_eq!(Feature::default(), Feature { activated_at: None }); + + let features = [ + Feature { + activated_at: Some(0), + }, + Feature { + activated_at: Some(Slot::MAX), + }, + ]; + for feature in &features { + assert_eq!( + Feature::size_of(), + bincode::serialized_size(feature).unwrap() as usize + ); + } + } +} diff --git a/sdk/program/src/lib.rs b/sdk/program/src/lib.rs index c31a019922..de8a79cce1 100644 --- a/sdk/program/src/lib.rs +++ b/sdk/program/src/lib.rs @@ -12,6 +12,7 @@ pub mod decode_error; pub mod entrypoint; pub mod entrypoint_deprecated; pub mod epoch_schedule; +pub mod feature; pub mod fee_calculator; pub mod hash; pub mod incinerator; diff --git a/sdk/src/feature.rs b/sdk/src/feature.rs index 6339865360..61a1fdaa93 100644 --- a/sdk/src/feature.rs +++ b/sdk/src/feature.rs @@ -1,44 +1,23 @@ -use solana_sdk::{account::Account, clock::Slot}; +use crate::account::Account; +pub use solana_program::feature::*; -solana_sdk::declare_id!("Feature111111111111111111111111111111111111"); - -/// The `Feature` struct is the on-chain representation of a runtime feature. -/// -/// Feature activation is accomplished by: -/// 1. Activation is requested by the feature authority, who issues a transaction to create the -/// feature account. The newly created feature account will have the value of -/// `Feature::default()` -/// 2. When the next epoch is entered the runtime will check for new activation requests and -/// active them. When this occurs, the activation slot is recorded in the feature account -/// -#[derive(Default, Debug, Serialize, Deserialize, PartialEq)] -pub struct Feature { - pub activated_at: Option, +pub fn from_account(account: &Account) -> Option { + if account.owner != id() { + None + } else { + bincode::deserialize(&account.data).ok() + } } -impl Feature { - pub fn size_of() -> usize { - bincode::serialized_size(&Self { - activated_at: Some(0), - }) - .unwrap() as usize - } - pub fn from_account(account: &Account) -> Option { - if account.owner != id() { - None - } else { - bincode::deserialize(&account.data).ok() - } - } - pub fn to_account(&self, account: &mut Account) -> Option<()> { - bincode::serialize_into(&mut account.data[..], self).ok() - } - pub fn create_account(&self, lamports: u64) -> Account { - let data_len = Self::size_of().max(bincode::serialized_size(self).unwrap() as usize); - let mut account = Account::new(lamports, data_len, &id()); - self.to_account(&mut account).unwrap(); - account - } +pub fn to_account(feature: &Feature, account: &mut Account) -> Option<()> { + bincode::serialize_into(&mut account.data[..], feature).ok() +} + +pub fn create_account(feature: &Feature, lamports: u64) -> Account { + let data_len = Feature::size_of().max(bincode::serialized_size(feature).unwrap() as usize); + let mut account = Account::new(lamports, data_len, &id()); + to_account(feature, &mut account).unwrap(); + account } #[cfg(test)] @@ -49,31 +28,8 @@ mod test { fn feature_deserialize_none() { let just_initialized = Account::new(42, Feature::size_of(), &id()); assert_eq!( - Feature::from_account(&just_initialized), + from_account(&just_initialized), Some(Feature { activated_at: None }) ); } - - #[test] - fn feature_sizeof() { - assert!( - Feature::size_of() >= bincode::serialized_size(&Feature::default()).unwrap() as usize - ); - assert_eq!(Feature::default(), Feature { activated_at: None }); - - let features = [ - Feature { - activated_at: Some(0), - }, - Feature { - activated_at: Some(Slot::MAX), - }, - ]; - for feature in &features { - assert_eq!( - Feature::size_of(), - bincode::serialized_size(feature).unwrap() as usize - ); - } - } }