cleanup old runtime features (#15787)

This commit is contained in:
Jack May 2021-03-10 09:48:41 -08:00 committed by GitHub
parent c5a5d7457e
commit 369e13b111
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 60 additions and 149 deletions

View File

@ -30,7 +30,6 @@ use solana_sdk::{
bpf_loader_upgradeable::{self, UpgradeableLoaderState},
clock::Clock,
entrypoint::SUCCESS,
feature_set::matching_buffer_upgrade_authorities,
ic_logger_msg, ic_msg,
instruction::InstructionError,
keyed_account::{from_keyed_account, next_keyed_account, KeyedAccount},
@ -303,27 +302,18 @@ fn process_loader_upgradeable_instruction(
return Err(InstructionError::AccountAlreadyInitialized);
}
if invoke_context.is_feature_active(&matching_buffer_upgrade_authorities::id()) {
let authority = next_keyed_account(account_iter)?;
let authority = next_keyed_account(account_iter)?;
buffer.set_state(&UpgradeableLoaderState::Buffer {
authority_address: Some(*authority.unsigned_key()),
})?;
} else {
let authority = next_keyed_account(account_iter)
.ok()
.map(|account| account.unsigned_key());
buffer.set_state(&UpgradeableLoaderState::Buffer {
authority_address: authority.cloned(),
})?;
}
buffer.set_state(&UpgradeableLoaderState::Buffer {
authority_address: Some(*authority.unsigned_key()),
})?;
}
UpgradeableLoaderInstruction::Write { offset, bytes } => {
let buffer = next_keyed_account(account_iter)?;
let authority = next_keyed_account(account_iter)?;
if let UpgradeableLoaderState::Buffer { authority_address } = buffer.state()? {
if authority_address == None {
if authority_address.is_none() {
ic_logger_msg!(logger, "Buffer is immutable");
return Err(InstructionError::Immutable); // TODO better error code
}
@ -354,19 +344,9 @@ fn process_loader_upgradeable_instruction(
let rent = from_keyed_account::<Rent>(next_keyed_account(account_iter)?)?;
let clock = from_keyed_account::<Clock>(next_keyed_account(account_iter)?)?;
let system = next_keyed_account(account_iter)?;
let (upgrade_authority_address, upgrade_authority_signer) =
if invoke_context.is_feature_active(&matching_buffer_upgrade_authorities::id()) {
let authority = next_keyed_account(account_iter)?;
(
Some(*authority.unsigned_key()),
authority.signer_key().is_none(),
)
} else {
let authority = next_keyed_account(account_iter)
.ok()
.map(|account| account.unsigned_key());
(authority.cloned(), false)
};
let authority = next_keyed_account(account_iter)?;
let upgrade_authority_address = Some(*authority.unsigned_key());
let upgrade_authority_signer = authority.signer_key().is_none();
// Verify Program account
@ -386,15 +366,13 @@ fn process_loader_upgradeable_instruction(
// Verify Buffer account
if let UpgradeableLoaderState::Buffer { authority_address } = buffer.state()? {
if invoke_context.is_feature_active(&matching_buffer_upgrade_authorities::id()) {
if authority_address != upgrade_authority_address {
ic_logger_msg!(logger, "Buffer and upgrade authority don't match");
return Err(InstructionError::IncorrectAuthority);
}
if upgrade_authority_signer {
ic_logger_msg!(logger, "Upgrade authority did not sign");
return Err(InstructionError::MissingRequiredSignature);
}
if authority_address != upgrade_authority_address {
ic_logger_msg!(logger, "Buffer and upgrade authority don't match");
return Err(InstructionError::IncorrectAuthority);
}
if upgrade_authority_signer {
ic_logger_msg!(logger, "Upgrade authority did not sign");
return Err(InstructionError::MissingRequiredSignature);
}
} else {
ic_logger_msg!(logger, "Invalid Buffer account");
@ -511,15 +489,13 @@ fn process_loader_upgradeable_instruction(
// Verify Buffer account
if let UpgradeableLoaderState::Buffer { authority_address } = buffer.state()? {
if invoke_context.is_feature_active(&matching_buffer_upgrade_authorities::id()) {
if authority_address != Some(*authority.unsigned_key()) {
ic_logger_msg!(logger, "Buffer and upgrade authority don't match");
return Err(InstructionError::IncorrectAuthority);
}
if authority.signer_key().is_none() {
ic_logger_msg!(logger, "Upgrade authority did not sign");
return Err(InstructionError::MissingRequiredSignature);
}
if authority_address != Some(*authority.unsigned_key()) {
ic_logger_msg!(logger, "Buffer and upgrade authority don't match");
return Err(InstructionError::IncorrectAuthority);
}
if authority.signer_key().is_none() {
ic_logger_msg!(logger, "Upgrade authority did not sign");
return Err(InstructionError::MissingRequiredSignature);
}
} else {
ic_logger_msg!(logger, "Invalid Buffer account");
@ -553,7 +529,7 @@ fn process_loader_upgradeable_instruction(
upgrade_authority_address,
} = programdata.state()?
{
if upgrade_authority_address == None {
if upgrade_authority_address.is_none() {
ic_logger_msg!(logger, "Program not upgradeable");
return Err(InstructionError::Immutable);
}
@ -614,13 +590,11 @@ fn process_loader_upgradeable_instruction(
match account.state()? {
UpgradeableLoaderState::Buffer { authority_address } => {
if invoke_context.is_feature_active(&matching_buffer_upgrade_authorities::id())
&& new_authority == None
{
if new_authority.is_none() {
ic_logger_msg!(logger, "Buffer authority is not optional");
return Err(InstructionError::IncorrectAuthority);
}
if authority_address == None {
if authority_address.is_none() {
ic_logger_msg!(logger, "Buffer is immutable");
return Err(InstructionError::Immutable);
}
@ -640,7 +614,7 @@ fn process_loader_upgradeable_instruction(
slot,
upgrade_authority_address,
} => {
if upgrade_authority_address == None {
if upgrade_authority_address.is_none() {
ic_logger_msg!(logger, "Program not upgradeable");
return Err(InstructionError::Immutable);
}

View File

@ -16,7 +16,7 @@ use solana_sdk::{
bpf_loader, bpf_loader_deprecated,
bpf_loader_upgradeable::{self, UpgradeableLoaderState},
entrypoint::{MAX_PERMITTED_DATA_INCREASE, SUCCESS},
feature_set::{per_byte_logging_cost, ristretto_mul_syscall_enabled, use_loaded_executables},
feature_set::ristretto_mul_syscall_enabled,
hash::{Hasher, HASH_BYTES},
ic_msg,
instruction::{AccountMeta, Instruction, InstructionError},
@ -159,19 +159,13 @@ pub fn bind_syscall_context_objects<'a>(
vm.bind_syscall_context_object(Box::new(SyscallAbort {}), None)?;
vm.bind_syscall_context_object(
Box::new(SyscallPanic {
compute_meter: if invoke_context.is_feature_active(&per_byte_logging_cost::id()) {
Some(invoke_context.get_compute_meter())
} else {
None
},
compute_meter: invoke_context.get_compute_meter(),
loader_id,
}),
None,
)?;
vm.bind_syscall_context_object(
Box::new(SyscallLog {
per_byte_cost: invoke_context.is_feature_active(&per_byte_logging_cost::id()),
cost: bpf_compute_budget.log_units,
compute_meter: invoke_context.get_compute_meter(),
logger: invoke_context.get_logger(),
loader_id,
@ -413,7 +407,7 @@ impl SyscallObject<BpfError> for SyscallAbort {
/// Causes the BPF program to be halted immediately
/// Log a user's info message
pub struct SyscallPanic<'a> {
compute_meter: Option<Rc<RefCell<dyn ComputeMeter>>>,
compute_meter: Rc<RefCell<dyn ComputeMeter>>,
loader_id: &'a Pubkey,
}
impl<'a> SyscallObject<BpfError> for SyscallPanic<'a> {
@ -427,9 +421,7 @@ impl<'a> SyscallObject<BpfError> for SyscallPanic<'a> {
memory_mapping: &MemoryMapping,
result: &mut Result<u64, EbpfError<BpfError>>,
) {
if let Some(ref mut compute_meter) = self.compute_meter {
question_mark!(compute_meter.consume(len), result);
}
question_mark!(self.compute_meter.consume(len), result);
*result = translate_string_and_do(
memory_mapping,
file,
@ -442,8 +434,6 @@ impl<'a> SyscallObject<BpfError> for SyscallPanic<'a> {
/// Log a user's info message
pub struct SyscallLog<'a> {
per_byte_cost: bool,
cost: u64,
compute_meter: Rc<RefCell<dyn ComputeMeter>>,
logger: Rc<RefCell<dyn Logger>>,
loader_id: &'a Pubkey,
@ -459,11 +449,7 @@ impl<'a> SyscallObject<BpfError> for SyscallLog<'a> {
memory_mapping: &MemoryMapping,
result: &mut Result<u64, EbpfError<BpfError>>,
) {
if self.per_byte_cost {
question_mark!(self.compute_meter.consume(len), result);
} else {
question_mark!(self.compute_meter.consume(self.cost), result);
}
question_mark!(self.compute_meter.consume(len), result);
question_mark!(
translate_string_and_do(
memory_mapping,
@ -1430,10 +1416,7 @@ where
SyscallError::InstructionError(InstructionError::MissingAccount)
})?;
if i == program_account_index
|| (invoke_context.is_feature_active(&use_loaded_executables::id())
&& account.borrow().executable)
{
if i == program_account_index || account.borrow().executable {
// Use the known executable
accounts.push(Rc::new(account));
refs.push(None);
@ -1616,7 +1599,7 @@ fn call<'a>(
// Construct executables
let program_account = (**accounts.get(callee_program_id_index).ok_or_else(|| {
ic_msg!(invoke_context, "Unknown program {}", callee_program_id,);
ic_msg!(invoke_context, "Unknown program {}", callee_program_id);
SyscallError::InstructionError(InstructionError::MissingAccount)
})?)
.clone();
@ -1768,11 +1751,11 @@ mod tests {
for (ok, start, length, value) in cases {
if ok {
assert_eq!(
translate(&memory_mapping, AccessType::Load, start, length,).unwrap(),
translate(&memory_mapping, AccessType::Load, start, length).unwrap(),
value
)
} else {
assert!(translate(&memory_mapping, AccessType::Load, start, length,).is_err())
assert!(translate(&memory_mapping, AccessType::Load, start, length).is_err())
}
}
}
@ -1897,7 +1880,7 @@ mod tests {
assert_eq!(data, translated_data);
data[0] = 10;
assert_eq!(data, translated_data);
assert!(translate_slice::<u64>(&memory_mapping, 96, u64::MAX, &bpf_loader::id(),).is_err());
assert!(translate_slice::<u64>(&memory_mapping, 96, u64::MAX, &bpf_loader::id()).is_err());
// Pubkeys
let mut data = vec![solana_sdk::pubkey::new_rand(); 5];
@ -1989,7 +1972,7 @@ mod tests {
remaining: string.len() as u64 - 1,
}));
let mut syscall_panic = SyscallPanic {
compute_meter: Some(compute_meter),
compute_meter,
loader_id: &bpf_loader::id(),
};
let mut result: Result<u64, EbpfError<BpfError>> = Ok(0);
@ -2009,8 +1992,12 @@ mod tests {
result
);
let compute_meter: Rc<RefCell<dyn ComputeMeter>> =
Rc::new(RefCell::new(MockComputeMeter {
remaining: string.len() as u64,
}));
let mut syscall_panic = SyscallPanic {
compute_meter: None,
compute_meter,
loader_id: &bpf_loader::id(),
};
let mut result: Result<u64, EbpfError<BpfError>> = Ok(0);
@ -2032,13 +2019,11 @@ mod tests {
let addr = string.as_ptr() as *const _ as u64;
let compute_meter: Rc<RefCell<dyn ComputeMeter>> =
Rc::new(RefCell::new(MockComputeMeter { remaining: 3 }));
Rc::new(RefCell::new(MockComputeMeter { remaining: 1000000 }));
let log = Rc::new(RefCell::new(vec![]));
let logger: Rc<RefCell<dyn Logger>> =
Rc::new(RefCell::new(MockLogger { log: log.clone() }));
let mut syscall_sol_log = SyscallLog {
per_byte_cost: false,
cost: 1,
compute_meter,
logger,
loader_id: &bpf_loader::id(),
@ -2100,12 +2085,6 @@ mod tests {
&memory_mapping,
&mut result,
);
assert_eq!(
Err(EbpfError::UserError(BpfError::SyscallError(
SyscallError::InstructionError(InstructionError::ComputationalBudgetExceeded)
))),
result
);
let compute_meter: Rc<RefCell<dyn ComputeMeter>> =
Rc::new(RefCell::new(MockComputeMeter {
@ -2113,8 +2092,6 @@ mod tests {
}));
let logger: Rc<RefCell<dyn Logger>> = Rc::new(RefCell::new(MockLogger { log }));
let mut syscall_sol_log = SyscallLog {
per_byte_cost: true,
cost: 1,
compute_meter,
logger,
loader_id: &bpf_loader::id(),

View File

@ -16,13 +16,12 @@ fn bench_verify_account_changes_data(bencher: &mut Bencher) {
let pre = PreAccount::new(
&pubkey::new_rand(),
&AccountSharedData::new(0, BUFSIZE, &owner),
false,
);
let post = AccountSharedData::new(0, BUFSIZE, &owner);
assert_eq!(
pre.verify(
&owner,
Some(false),
false,
&Rent::default(),
&post,
&mut ExecuteDetailsTimings::default()
@ -34,7 +33,7 @@ fn bench_verify_account_changes_data(bencher: &mut Bencher) {
bencher.iter(|| {
pre.verify(
&owner,
Some(false),
false,
&Rent::default(),
&post,
&mut ExecuteDetailsTimings::default(),
@ -53,12 +52,11 @@ fn bench_verify_account_changes_data(bencher: &mut Bencher) {
let pre = PreAccount::new(
&pubkey::new_rand(),
&AccountSharedData::new(0, BUFSIZE, &owner),
false,
);
bencher.iter(|| {
pre.verify(
&non_owner,
Some(false),
false,
&Rent::default(),
&post,
&mut ExecuteDetailsTimings::default(),

View File

@ -8,7 +8,7 @@ use solana_sdk::{
account::{AccountSharedData, ReadableAccount},
account_utils::StateMut,
bpf_loader_upgradeable::{self, UpgradeableLoaderState},
feature_set::{instructions_sysvar_enabled, track_writable_deescalation, FeatureSet},
feature_set::{instructions_sysvar_enabled, FeatureSet},
ic_msg,
instruction::{CompiledInstruction, Instruction, InstructionError},
keyed_account::{create_keyed_readonly_accounts, KeyedAccount},
@ -82,15 +82,13 @@ impl ExecuteDetailsTimings {
#[derive(Clone, Debug, Default)]
pub struct PreAccount {
key: Pubkey,
is_writable: bool,
account: RefCell<AccountSharedData>,
changed: bool,
}
impl PreAccount {
pub fn new(key: &Pubkey, account: &AccountSharedData, is_writable: bool) -> Self {
pub fn new(key: &Pubkey, account: &AccountSharedData) -> Self {
Self {
key: *key,
is_writable,
account: RefCell::new(account.clone()),
changed: false,
}
@ -99,19 +97,13 @@ impl PreAccount {
pub fn verify(
&self,
program_id: &Pubkey,
is_writable: Option<bool>,
is_writable: bool,
rent: &Rent,
post: &AccountSharedData,
timings: &mut ExecuteDetailsTimings,
) -> Result<(), InstructionError> {
let pre = self.account.borrow();
let is_writable = if let Some(is_writable) = is_writable {
is_writable
} else {
self.is_writable
};
// Only the owner of the account may change owner and
// only if the account is writable and
// only if the account is not executable and
@ -331,8 +323,6 @@ impl<'a> InvokeContext for ThisInvokeContext<'a> {
accounts: &[Rc<RefCell<AccountSharedData>>],
caller_privileges: Option<&[bool]>,
) -> Result<(), InstructionError> {
let track_writable_deescalation =
self.is_feature_active(&track_writable_deescalation::id());
match self.program_ids.last() {
Some(program_id) => MessageProcessor::verify_and_update(
message,
@ -341,7 +331,6 @@ impl<'a> InvokeContext for ThisInvokeContext<'a> {
accounts,
program_id,
&self.rent,
track_writable_deescalation,
caller_privileges,
&mut self.timings,
),
@ -870,9 +859,8 @@ impl MessageProcessor {
{
let mut work = |_unique_index: usize, account_index: usize| {
let key = &message.account_keys[account_index];
let is_writable = message.is_writable(account_index);
let account = accounts[account_index].borrow();
pre_accounts.push(PreAccount::new(key, &account, is_writable));
pre_accounts.push(PreAccount::new(key, &account));
Ok(())
};
let _ = instruction.visit_each_account(&mut work);
@ -916,7 +904,7 @@ impl MessageProcessor {
.map_err(|_| InstructionError::AccountBorrowOutstanding)?;
pre_accounts[unique_index].verify(
&program_id,
Some(message.is_writable(account_index)),
message.is_writable(account_index),
rent,
&account,
timings,
@ -943,7 +931,6 @@ impl MessageProcessor {
accounts: &[Rc<RefCell<AccountSharedData>>],
program_id: &Pubkey,
rent: &Rent,
track_writable_deescalation: bool,
caller_privileges: Option<&[bool]>,
timings: &mut ExecuteDetailsTimings,
) -> Result<(), InstructionError> {
@ -953,14 +940,10 @@ impl MessageProcessor {
if account_index < message.account_keys.len() && account_index < accounts.len() {
let key = &message.account_keys[account_index];
let account = &accounts[account_index];
let is_writable = if track_writable_deescalation {
Some(if let Some(caller_privileges) = caller_privileges {
caller_privileges[account_index]
} else {
message.is_writable(account_index)
})
let is_writable = if let Some(caller_privileges) = caller_privileges {
caller_privileges[account_index]
} else {
None
message.is_writable(account_index)
};
// Find the matching PreAccount
for pre_account in pre_accounts.iter_mut() {
@ -1132,11 +1115,11 @@ mod tests {
1,
&program_ids[i],
))));
pre_accounts.push(PreAccount::new(&keys[i], &accounts[i].borrow(), false))
pre_accounts.push(PreAccount::new(&keys[i], &accounts[i].borrow()))
}
let account = AccountSharedData::new(1, 1, &solana_sdk::pubkey::Pubkey::default());
for program_id in program_ids.iter() {
pre_accounts.push(PreAccount::new(program_id, &account.clone(), false));
pre_accounts.push(PreAccount::new(program_id, &account.clone()));
}
let mut invoke_context = ThisInvokeContext::new(
@ -1282,7 +1265,6 @@ mod tests {
data: vec![],
..AccountSharedData::default()
},
false,
),
post: AccountSharedData {
owner: *owner,
@ -1322,7 +1304,7 @@ mod tests {
pub fn verify(&self) -> Result<(), InstructionError> {
self.pre.verify(
&self.program_id,
Some(self.is_writable),
self.is_writable,
&self.rent,
&self.post,
&mut ExecuteDetailsTimings::default(),
@ -2004,16 +1986,16 @@ mod tests {
let mut program_account = AccountSharedData::new(1, 0, &native_loader::id());
program_account.executable = true;
let executable_preaccount = PreAccount::new(&callee_program_id, &program_account, true);
let executable_preaccount = PreAccount::new(&callee_program_id, &program_account);
let executable_accounts = vec![(callee_program_id, RefCell::new(program_account.clone()))];
let owned_key = solana_sdk::pubkey::new_rand();
let owned_account = AccountSharedData::new(42, 1, &callee_program_id);
let owned_preaccount = PreAccount::new(&owned_key, &owned_account, true);
let owned_preaccount = PreAccount::new(&owned_key, &owned_account);
let not_owned_key = solana_sdk::pubkey::new_rand();
let not_owned_account = AccountSharedData::new(84, 1, &solana_sdk::pubkey::new_rand());
let not_owned_preaccount = PreAccount::new(&not_owned_key, &not_owned_account, true);
let not_owned_preaccount = PreAccount::new(&not_owned_key, &not_owned_account);
#[allow(unused_mut)]
let mut accounts = vec![

View File

@ -83,18 +83,10 @@ pub mod stake_program_v3 {
solana_sdk::declare_id!("Ego6nTu7WsBcZBvVqJQKp6Yku2N3mrfG8oYCfaLZkAeK");
}
pub mod use_loaded_executables {
solana_sdk::declare_id!("3Jq7mE2chDpf6oeEDsuGK7orTYEgyQjCPvaRppTNdVGK");
}
pub mod turbine_retransmit_peers_patch {
solana_sdk::declare_id!("5Lu3JnWSFwRYpXzwDMkanWSk6XqSuF2i5fpnVhzB5CTc");
}
pub mod track_writable_deescalation {
solana_sdk::declare_id!("HVPSxqskEtRLRT2ZeEMmkmt9FWqoFX4vrN6f5VaadLED");
}
pub mod require_custodian_for_locked_stake_authorize {
solana_sdk::declare_id!("D4jsDcXaqdW8tDAWn8H4R25Cdns2YwLneujSL1zvjW6R");
}
@ -103,18 +95,10 @@ pub mod spl_token_v2_self_transfer_fix {
solana_sdk::declare_id!("BL99GYhdjjcv6ys22C9wPgn2aTVERDbPHHo4NbS3hgp7");
}
pub mod matching_buffer_upgrade_authorities {
solana_sdk::declare_id!("B5PSjDEJvKJEUQSL7q94N7XCEoWJCYum8XfUg7yuugUU");
}
pub mod warp_timestamp_again {
solana_sdk::declare_id!("GvDsGDkH5gyzwpDhxNixx8vtx1kwYHH13RiNAPw27zXb");
}
pub mod per_byte_logging_cost {
solana_sdk::declare_id!("59dM4SV6dPEKXPfkrkhFkRdn4K6xwKxdNAPMyXG7J1wT");
}
pub mod check_init_vote_data {
solana_sdk::declare_id!("3ccR6QpxGYsAbWyfevEtBNGfWV4xBffxRj2tD6A9i39F");
}
@ -142,16 +126,12 @@ lazy_static! {
(simple_capitalization::id(), "simple capitalization"),
(bpf_loader_upgradeable_program::id(), "upgradeable bpf loader"),
(stake_program_v3::id(), "solana_stake_program v3"),
(use_loaded_executables::id(), "use loaded executable accounts"),
(turbine_retransmit_peers_patch::id(), "turbine retransmit peers patch #14631"),
(track_writable_deescalation::id(), "track account writable deescalation"),
(require_custodian_for_locked_stake_authorize::id(), "require custodian to authorize withdrawer change for locked stake"),
(spl_token_v2_self_transfer_fix::id(), "spl-token self-transfer fix"),
(matching_buffer_upgrade_authorities::id(), "Upgradeable buffer and program authorities must match"),
(full_inflation::mainnet::certusone::enable::id(), "Full inflation enabled by Certus One"),
(full_inflation::mainnet::certusone::vote::id(), "Community vote allowing Certus One to enable full inflation"),
(warp_timestamp_again::id(), "warp timestamp again, adjust bounding to 25% fast 80% slow #15204"),
(per_byte_logging_cost::id(), "charge the compute budget per byte for logging"),
(check_init_vote_data::id(), "check initialized Vote data"),
(check_program_owner::id(), "limit programs to operating on accounts owned by itself")
/*************** ADD NEW FEATURES HERE ***************/