gaspersic/solana
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Allow feature builtins to overwrite existing builtins (bp #13403) (#13419)

Browse Source 
* Allow feature builtins to overwrite existing builtins (#13403)

* Allow feature builtins to overwrite existing builtins

* Add feature_builtin ActivationType

* Correctly retain idempotent for replacing case

* Fix test

Co-authored-by: Ryo Onodera <ryoqun@gmail.com>
(cherry picked from commit bc62313c66)

# Conflicts:
#	ledger/src/builtins.rs
#	runtime/src/bank.rs

* Fix conflicts

Co-authored-by: Tyera Eulberg <teulberg@gmail.com>
Co-authored-by: Tyera Eulberg <tyera@solana.com>
This commit is contained in:
mergify[bot]
2020-11-05 17:53:03 +00:00
committed by GitHub
parent a899d75d2d
commit ef776c0a0e
6 changed files with 156 additions and 39 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
ledger/src/builtins.rs
View File

@ -1,5 +1,6 @@
use solana_runtime::{
bank::{Builtin, Builtins, Entrypoint},
builtins::ActivationType,
feature_set,
};
use solana_sdk::{genesis_config::ClusterType, pubkey::Pubkey};
@ -24,15 +25,16 @@ fn genesis_builtins(cluster_type: ClusterType) -> Vec<Builtin> {
}
/// Builtin programs activated dynamically by feature
fn feature_builtins() -> Vec<(Builtin, Pubkey)> {
fn feature_builtins() -> Vec<(Builtin, Pubkey, ActivationType)> {
let builtins = vec![(
solana_bpf_loader_program!(),
feature_set::bpf_loader2_program::id(),
ActivationType::NewProgram,
)];
builtins
.into_iter()
.map(|(b, p)| (Builtin::new(&b.0, b.1, Entrypoint::Loader(b.2)), p))
.map(|(b, p, t)| (Builtin::new(&b.0, b.1, Entrypoint::Loader(b.2)), p, t))
.collect()
}

2
programs/failure/tests/failure.rs
View File

@ -12,7 +12,7 @@ fn test_program_native_failure() {
let (genesis_config, alice_keypair) = create_genesis_config(50);
let program_id = solana_sdk::pubkey::new_rand();
let bank = Bank::new(&genesis_config);
bank.add_native_program("solana_failure_program", &program_id);
bank.add_native_program("solana_failure_program", &program_id, false);
// Call user program
let instruction = create_invoke_instruction(alice_keypair.pubkey(), program_id, &1u8);

2
runtime/benches/bank.rs
View File

@ -128,7 +128,7 @@ fn do_bench_transactions(
Pubkey::new(&BUILTIN_PROGRAM_ID),
process_instruction,
);
bank.add_native_program("solana_noop_program", &Pubkey::new(&NOOP_PROGRAM_ID));
bank.add_native_program("solana_noop_program", &Pubkey::new(&NOOP_PROGRAM_ID), false);
let bank = Arc::new(bank);
let bank_client = BankClient::new_shared(&bank);
let transactions = create_transactions(&bank_client, &mint_keypair);

174
runtime/src/bank.rs
View File

@ -10,7 +10,7 @@ use crate::{
accounts_db::{ErrorCounters, SnapshotStorages},
accounts_index::Ancestors,
blockhash_queue::BlockhashQueue,
builtins,
builtins::{self, ActivationType},
epoch_stakes::{EpochStakes, NodeVoteAccounts},
feature::Feature,
feature_set::{self, FeatureSet},
@ -232,7 +232,7 @@ pub struct Builtins {
pub genesis_builtins: Vec<Builtin>,
/// Builtin programs activated dynamically by feature
pub feature_builtins: Vec<(Builtin, Pubkey)>,
pub feature_builtins: Vec<(Builtin, Pubkey, ActivationType)>,
}
const MAX_CACHED_EXECUTORS: usize = 100; // 10 MB assuming programs are around 100k
@ -691,7 +691,7 @@ pub struct Bank {
message_processor: MessageProcessor,
/// Builtin programs activated dynamically by feature
feature_builtins: Arc<Vec<(Builtin, Pubkey)>>,
feature_builtins: Arc<Vec<(Builtin, Pubkey, ActivationType)>>,
/// Last time when the cluster info vote listener has synced with this bank
pub last_vote_sync: AtomicU64,
@ -1603,16 +1603,15 @@ impl Bank {
// Add additional native programs specified in the genesis config
for (name, program_id) in &genesis_config.native_instruction_processors {
self.add_native_program(name, program_id);
self.add_native_program(name, program_id, false);
}
}
pub fn add_native_program(&self, name: &str, program_id: &Pubkey) {
let mut already_genuine_program_exists = false;
if let Some(mut account) = self.get_account(&program_id) {
already_genuine_program_exists = native_loader::check_id(&account.owner);
if !already_genuine_program_exists {
// NOTE: must hold idempotent for the same set of arguments
pub fn add_native_program(&self, name: &str, program_id: &Pubkey, must_replace: bool) {
let mut existing_genuine_program = self.get_account(&program_id);
if let Some(account) = &mut existing_genuine_program {
if !native_loader::check_id(&account.owner) {
// malicious account is pre-occupying at program_id
// forcibly burn and purge it
@ -1625,19 +1624,54 @@ impl Bank {
}
}
if !already_genuine_program_exists {
assert!(
!self.is_frozen(),
"Can't change frozen bank by adding not-existing new native program ({}, {}). \
Maybe, inconsistent program activation is detected on snapshot restore?",
name,
program_id
);
if must_replace {
// updating native program
// Add a bogus executable native account, which will be loaded and ignored.
let account = native_loader::create_loadable_account(name);
self.store_account(&program_id, &account);
match existing_genuine_program {
None => panic!(
"There is no account to replace with native program ({}, {}).",
name, program_id
),
Some(account) => {
if *name == String::from_utf8_lossy(&account.data) {
// nop; it seems that already AccountsDB is updated.
return;
}
// continue to replace account
}
}
} else {
// introducing native program
match existing_genuine_program {
None => (), // continue to add account
Some(_account) => {
// nop; it seems that we already have account
// before returning here to retain idempotent just make sure
// the existing native program name is same with what we're
// supposed to add here (but skipping) But I can't:
// following assertion already catches several different names for same
// program_id
// depending on clusters...
// assert_eq!(name.to_owned(), String::from_utf8_lossy(&account.data));
return;
}
}
}
assert!(
!self.is_frozen(),
"Can't change frozen bank by adding not-existing new native program ({}, {}). \
Maybe, inconsistent program activation is detected on snapshot restore?",
name,
program_id
);
// Add a bogus executable native account, which will be loaded and ignored.
let account = native_loader::create_loadable_account(name);
self.store_account(&program_id, &account);
debug!("Added native program {} under {:?}", name, program_id);
}
@ -3765,17 +3799,34 @@ impl Bank {
/// Add an instruction processor to intercept instructions before the dynamic loader.
pub fn add_builtin(&mut self, name: &str, program_id: Pubkey, entrypoint: Entrypoint) {
self.add_native_program(name, &program_id);
self.add_native_program(name, &program_id, false);
match entrypoint {
Entrypoint::Program(process_instruction) => {
self.message_processor
.add_program(program_id, process_instruction);
debug!("Added builtin program {} under {:?}", name, program_id);
debug!("Adding builtin program {} under {:?}", name, program_id);
}
Entrypoint::Loader(process_instruction_with_context) => {
self.message_processor
.add_loader(program_id, process_instruction_with_context);
debug!("Added builtin loader {} under {:?}", name, program_id);
debug!("Adding builtin loader {} under {:?}", name, program_id);
}
}
}
/// Replace a builtin instruction processor if it already exists
pub fn replace_builtin(&mut self, name: &str, program_id: Pubkey, entrypoint: Entrypoint) {
self.add_native_program(name, &program_id, true);
match entrypoint {
Entrypoint::Program(process_instruction) => {
self.message_processor
.add_program(program_id, process_instruction);
debug!("Replacing builtin program {} under {:?}", name, program_id);
}
Entrypoint::Loader(process_instruction_with_context) => {
self.message_processor
.add_loader(program_id, process_instruction_with_context);
debug!("Replacing builtin loader {} under {:?}", name, program_id);
}
}
}
@ -3904,11 +3955,18 @@ impl Bank {
new_feature_activations: &HashSet<Pubkey>,
) {
let feature_builtins = self.feature_builtins.clone();
for (builtin, feature) in feature_builtins.iter() {
for (builtin, feature, activation_type) in feature_builtins.iter() {
let should_populate = init_or_warp && self.feature_set.is_active(&feature)
|| !init_or_warp && new_feature_activations.contains(&feature);
if should_populate {
self.add_builtin(&builtin.name, builtin.id, builtin.entrypoint);
match activation_type {
ActivationType::NewProgram => {
self.add_builtin(&builtin.name, builtin.id, builtin.entrypoint)
}
ActivationType::NewVersion => {
self.replace_builtin(&builtin.name, builtin.id, builtin.entrypoint)
}
}
}
}
}
@ -9092,6 +9150,16 @@ mod tests {
mock_ix_processor,
);
assert_eq!(bank.get_account_modified_slot(&program_id).unwrap().1, slot);
Arc::get_mut(&mut bank).unwrap().replace_builtin(
"mock_program v2",
program_id,
Entrypoint::Program(mock_ix_processor),
);
assert_eq!(
bank.get_account_modified_slot(&program_id).unwrap().1,
bank.slot()
);
}
#[test]
@ -9149,14 +9217,35 @@ mod tests {
Arc::get_mut(&mut bank)
.unwrap()
.add_native_program("mock_program", &program_id);
.add_native_program("mock_program", &program_id, false);
assert_eq!(bank.get_account_modified_slot(&program_id).unwrap().1, slot);
let mut bank = Arc::new(new_from_parent(&bank));
Arc::get_mut(&mut bank)
.unwrap()
.add_native_program("mock_program", &program_id);
.add_native_program("mock_program", &program_id, false);
assert_eq!(bank.get_account_modified_slot(&program_id).unwrap().1, slot);
let mut bank = Arc::new(new_from_parent(&bank));
// When replacing native_program, name must change to disambiguate from repeated
// invocations.
Arc::get_mut(&mut bank)
.unwrap()
.add_native_program("mock_program v2", &program_id, true);
assert_eq!(
bank.get_account_modified_slot(&program_id).unwrap().1,
bank.slot()
);
let mut bank = Arc::new(new_from_parent(&bank));
Arc::get_mut(&mut bank)
.unwrap()
.add_native_program("mock_program v2", &program_id, true);
// replacing with same name shouldn't update account
assert_eq!(
bank.get_account_modified_slot(&program_id).unwrap().1,
bank.parent_slot()
);
}
#[test]
@ -9172,16 +9261,35 @@ mod tests {
let slot = 123;
let program_id = Pubkey::from_str("CiXgo2KHKSDmDnV1F6B69eWFgNAPiSBjjYvfB4cvRNre").unwrap();
let mut bank = Arc::new(Bank::new_from_parent(
let bank = Bank::new_from_parent(
&Arc::new(Bank::new(&genesis_config)),
&Pubkey::default(),
slot,
));
);
bank.freeze();
Arc::get_mut(&mut bank)
.unwrap()
.add_native_program("mock_program", &program_id);
bank.add_native_program("mock_program", &program_id, false);
}
#[test]
#[should_panic(
expected = "There is no account to replace with native program (mock_program, \
CiXgo2KHKSDmDnV1F6B69eWFgNAPiSBjjYvfB4cvRNre)."
)]
fn test_add_native_program_replace_none() {
use std::str::FromStr;
let (genesis_config, _mint_keypair) = create_genesis_config(100_000);
let slot = 123;
let program_id = Pubkey::from_str("CiXgo2KHKSDmDnV1F6B69eWFgNAPiSBjjYvfB4cvRNre").unwrap();
let bank = Bank::new_from_parent(
&Arc::new(Bank::new(&genesis_config)),
&Pubkey::default(),
slot,
);
bank.add_native_program("mock_program", &program_id, true);
}
#[test]

9
runtime/src/builtins.rs
View File

@ -30,8 +30,14 @@ fn genesis_builtins() -> Vec<Builtin> {
]
}
#[derive(AbiExample, Debug, Clone)]
pub enum ActivationType {
NewProgram,
NewVersion,
}
/// Builtin programs activated dynamically by feature
fn feature_builtins() -> Vec<(Builtin, Pubkey)> {
fn feature_builtins() -> Vec<(Builtin, Pubkey, ActivationType)> {
vec![(
Builtin::new(
"secp256k1_program",
@ -39,6 +45,7 @@ fn feature_builtins() -> Vec<(Builtin, Pubkey)> {
Entrypoint::Program(solana_secp256k1_program::process_instruction),
),
feature_set::secp256k1_program_enabled::id(),
ActivationType::NewProgram,
)]
}

2
runtime/tests/noop.rs
View File

@ -10,7 +10,7 @@ fn test_program_native_noop() {
let (genesis_config, alice_keypair) = create_genesis_config(50);
let program_id = solana_sdk::pubkey::new_rand();
let bank = Bank::new(&genesis_config);
bank.add_native_program("solana_noop_program", &program_id);
bank.add_native_program("solana_noop_program", &program_id, false);
// Call user program
let instruction = create_invoke_instruction(alice_keypair.pubkey(), program_id, &1u8);