Upgradeable programs called same as non-upgradeable (#14239)

* Upgradeable programs called same as non-upgradeable

* nudge
This commit is contained in:
Jack May
2020-12-22 09:26:55 -08:00
committed by GitHub
parent 1b30155dc3
commit ab205b682a
8 changed files with 212 additions and 76 deletions

View File

@@ -61,11 +61,17 @@ pub struct Accounts {
// for the load instructions
pub type TransactionAccounts = Vec<Account>;
pub type TransactionAccountDeps = Vec<(Pubkey, Account)>;
pub type TransactionRent = u64;
pub type TransactionLoaders = Vec<Vec<(Pubkey, Account)>>;
pub type TransactionLoadResult = (
Result<(TransactionAccounts, TransactionLoaders, TransactionRent)>,
Result<(
TransactionAccounts,
TransactionAccountDeps,
TransactionLoaders,
TransactionRent,
)>,
Option<NonceRollbackFull>,
);
@@ -137,7 +143,7 @@ impl Accounts {
error_counters: &mut ErrorCounters,
rent_collector: &RentCollector,
feature_set: &FeatureSet,
) -> Result<(TransactionAccounts, TransactionRent)> {
) -> Result<(TransactionAccounts, TransactionAccountDeps, TransactionRent)> {
// Copy all the accounts
let message = tx.message();
if tx.signatures.is_empty() && fee != 0 {
@@ -148,6 +154,7 @@ impl Accounts {
let mut payer_index = None;
let mut tx_rent: TransactionRent = 0;
let mut accounts = Vec::with_capacity(message.account_keys.len());
let mut account_deps = Vec::with_capacity(message.account_keys.len());
let rent_fix_enabled = feature_set.cumulative_rent_related_fixes_enabled();
for (i, key) in message.account_keys.iter().enumerate() {
@@ -181,6 +188,28 @@ impl Accounts {
})
.unwrap_or_default();
if account.executable && bpf_loader_upgradeable::check_id(&account.owner) {
// The upgradeable loader requires the derived ProgramData account
if let Ok(UpgradeableLoaderState::Program {
programdata_address,
}) = account.state()
{
if let Some(account) = self
.accounts_db
.load(ancestors, &programdata_address)
.map(|(account, _)| account)
{
account_deps.push((programdata_address, account));
} else {
error_counters.account_not_found += 1;
return Err(TransactionError::ProgramAccountNotFound);
}
} else {
error_counters.invalid_program_for_execution += 1;
return Err(TransactionError::InvalidProgramForExecution);
}
}
tx_rent += rent;
account
}
@@ -216,7 +245,7 @@ impl Accounts {
Err(TransactionError::InsufficientFundsForFee)
} else {
accounts[payer_index].lamports -= fee;
Ok((accounts, tx_rent))
Ok((accounts, account_deps, tx_rent))
}
}
} else {
@@ -357,8 +386,8 @@ impl Accounts {
rent_collector,
feature_set,
);
let (accounts, rents) = match load_res {
Ok((a, r)) => (a, r),
let (accounts, account_deps, rents) = match load_res {
Ok((a, d, r)) => (a, d, r),
Err(e) => return (Err(e), None),
};
@@ -382,7 +411,7 @@ impl Accounts {
None
};
(Ok((accounts, loaders, rents)), nonce_rollback)
(Ok((accounts, account_deps, loaders, rents)), nonce_rollback)
}
(_, (Err(e), _nonce_rollback)) => (Err(e), None),
})
@@ -885,7 +914,7 @@ impl Accounts {
}
}
if account.rent_epoch == 0 {
acc.2 += rent_collector.collect_from_created_account(
acc.3 += rent_collector.collect_from_created_account(
&key,
account,
rent_fix_enabled,
@@ -1226,7 +1255,7 @@ mod tests {
);
assert_eq!(loaded_accounts.len(), 1);
let (load_res, _nonce_rollback) = &loaded_accounts[0];
let (tx_accounts, _loaders, _rents) = load_res.as_ref().unwrap();
let (tx_accounts, _account_deps, _loaders, _rents) = load_res.as_ref().unwrap();
assert_eq!(tx_accounts[0].lamports, min_balance);
// Fee leaves zero balance fails
@@ -1288,7 +1317,12 @@ mod tests {
assert_eq!(loaded_accounts.len(), 1);
match &loaded_accounts[0] {
(
Ok((transaction_accounts, transaction_loaders, _transaction_rents)),
Ok((
transaction_accounts,
_transaction_account_deps,
transaction_loaders,
_transaction_rents,
)),
_nonce_rollback,
) => {
assert_eq!(transaction_accounts.len(), 3);
@@ -1514,7 +1548,12 @@ mod tests {
assert_eq!(loaded_accounts.len(), 1);
match &loaded_accounts[0] {
(
Ok((transaction_accounts, transaction_loaders, _transaction_rents)),
Ok((
transaction_accounts,
_transaction_account_deps,
transaction_loaders,
_transaction_rents,
)),
_nonce_rollback,
) => {
assert_eq!(transaction_accounts.len(), 3);
@@ -1811,6 +1850,7 @@ mod tests {
let loaded0 = (
Ok((
transaction_accounts0,
vec![],
transaction_loaders0,
transaction_rent0,
)),
@@ -1823,6 +1863,7 @@ mod tests {
let loaded1 = (
Ok((
transaction_accounts1,
vec![],
transaction_loaders1,
transaction_rent1,
)),
@@ -2193,7 +2234,12 @@ mod tests {
let transaction_loaders = vec![];
let transaction_rent = 0;
let loaded = (
Ok((transaction_accounts, transaction_loaders, transaction_rent)),
Ok((
transaction_accounts,
vec![],
transaction_loaders,
transaction_rent,
)),
nonce_rollback,
);
@@ -2298,7 +2344,12 @@ mod tests {
let transaction_loaders = vec![];
let transaction_rent = 0;
let loaded = (
Ok((transaction_accounts, transaction_loaders, transaction_rent)),
Ok((
transaction_accounts,
vec![],
transaction_loaders,
transaction_rent,
)),
nonce_rollback,
);

View File

@@ -4,8 +4,8 @@
//! already been signed and verified.
use crate::{
accounts::{
AccountAddressFilter, Accounts, TransactionAccounts, TransactionLoadResult,
TransactionLoaders,
AccountAddressFilter, Accounts, TransactionAccountDeps, TransactionAccounts,
TransactionLoadResult, TransactionLoaders,
},
accounts_db::{ErrorCounters, SnapshotStorages},
accounts_index::Ancestors,
@@ -117,6 +117,7 @@ type BankStatusCache = StatusCache<Result<()>>;
#[frozen_abi(digest = "GSPuprru1pomsgvopKG7XRWiXdqdXJdLPkgJ2arPbkXM")]
pub type BankSlotDelta = SlotDelta<Result<()>>;
type TransactionAccountRefCells = Vec<Rc<RefCell<Account>>>;
type TransactionAccountDepRefCells = Vec<(Pubkey, RefCell<Account>)>;
type TransactionLoaderRefCells = Vec<Vec<(Pubkey, RefCell<Account>)>>;
// Eager rent collection repeats in cyclic manner.
@@ -2699,12 +2700,21 @@ impl Bank {
/// ownership by draining the source
fn accounts_to_refcells(
accounts: &mut TransactionAccounts,
account_deps: &mut TransactionAccountDeps,
loaders: &mut TransactionLoaders,
) -> (TransactionAccountRefCells, TransactionLoaderRefCells) {
) -> (
TransactionAccountRefCells,
TransactionAccountDepRefCells,
TransactionLoaderRefCells,
) {
let account_refcells: Vec<_> = accounts
.drain(..)
.map(|account| Rc::new(RefCell::new(account)))
.collect();
let account_dep_refcells: Vec<_> = account_deps
.drain(..)
.map(|(pubkey, account_dep)| (pubkey, RefCell::new(account_dep)))
.collect();
let loader_refcells: Vec<Vec<_>> = loaders
.iter_mut()
.map(|v| {
@@ -2713,7 +2723,7 @@ impl Bank {
.collect()
})
.collect();
(account_refcells, loader_refcells)
(account_refcells, account_dep_refcells, loader_refcells)
}
/// Converts back from RefCell<Account> to Account, this involves moving
@@ -2865,13 +2875,13 @@ impl Bank {
.zip(OrderedIterator::new(txs, batch.iteration_order()))
.map(|(accs, (_, tx))| match accs {
(Err(e), _nonce_rollback) => (Err(e.clone()), None),
(Ok((accounts, loaders, _rents)), nonce_rollback) => {
(Ok((accounts, account_deps, loaders, _rents)), nonce_rollback) => {
signature_count += u64::from(tx.message().header.num_required_signatures);
let executors = self.get_executors(&tx.message, &loaders);
let (account_refcells, loader_refcells) =
Self::accounts_to_refcells(accounts, loaders);
let (account_refcells, account_dep_refcells, loader_refcells) =
Self::accounts_to_refcells(accounts, account_deps, loaders);
let instruction_recorders = if enable_cpi_recording {
let ix_count = tx.message.instructions.len();
@@ -2892,6 +2902,7 @@ impl Bank {
tx.message(),
&loader_refcells,
&account_refcells,
&account_dep_refcells,
&self.rent_collector,
log_collector.clone(),
executors.clone(),
@@ -3308,7 +3319,7 @@ impl Bank {
let acc = raccs.as_ref().unwrap();
collected_rent += acc.2;
collected_rent += acc.3;
}
self.collected_rent.fetch_add(collected_rent, Relaxed);

View File

@@ -207,6 +207,7 @@ pub struct ThisInvokeContext<'a> {
program_ids: Vec<Pubkey>,
rent: Rent,
pre_accounts: Vec<PreAccount>,
account_deps: &'a [(Pubkey, RefCell<Account>)],
programs: &'a [(Pubkey, ProcessInstructionWithContext)],
logger: Rc<RefCell<dyn Logger>>,
bpf_compute_budget: BpfComputeBudget,
@@ -216,10 +217,12 @@ pub struct ThisInvokeContext<'a> {
feature_set: Arc<FeatureSet>,
}
impl<'a> ThisInvokeContext<'a> {
#[allow(clippy::too_many_arguments)]
pub fn new(
program_id: &Pubkey,
rent: Rent,
pre_accounts: Vec<PreAccount>,
account_deps: &'a [(Pubkey, RefCell<Account>)],
programs: &'a [(Pubkey, ProcessInstructionWithContext)],
log_collector: Option<Rc<LogCollector>>,
bpf_compute_budget: BpfComputeBudget,
@@ -233,6 +236,7 @@ impl<'a> ThisInvokeContext<'a> {
program_ids,
rent,
pre_accounts,
account_deps,
programs,
logger: Rc::new(RefCell::new(ThisLogger { log_collector })),
bpf_compute_budget,
@@ -312,16 +316,25 @@ impl<'a> InvokeContext for ThisInvokeContext<'a> {
fn is_feature_active(&self, feature_id: &Pubkey) -> bool {
self.feature_set.is_active(feature_id)
}
fn get_account(&self, pubkey: &Pubkey) -> Option<Account> {
fn get_account(&self, pubkey: &Pubkey) -> Option<RefCell<Account>> {
if let Some(account) = self.account_deps.iter().find_map(|(key, account)| {
if key == pubkey {
Some(account.clone())
} else {
None
}
}) {
return Some(account);
}
self.pre_accounts.iter().find_map(|pre| {
if pre.key == *pubkey {
Some(Account {
Some(RefCell::new(Account {
lamports: pre.lamports,
data: pre.data.clone(),
owner: pre.owner,
executable: pre.is_executable,
rent_epoch: pre.rent_epoch,
})
}))
} else {
None
}
@@ -641,26 +654,27 @@ impl MessageProcessor {
let program_account = invoke_context
.get_account(&callee_program_id)
.ok_or(InstructionError::MissingAccount)?;
if !program_account.executable {
if !program_account.borrow().executable {
return Err(InstructionError::AccountNotExecutable);
}
let programdata_executable = if program_account.owner == bpf_loader_upgradeable::id() {
if let UpgradeableLoaderState::Program {
programdata_address,
} = program_account.state()?
{
if let Some(account) = invoke_context.get_account(&programdata_address) {
Some((programdata_address, RefCell::new(account)))
let programdata_executable =
if program_account.borrow().owner == bpf_loader_upgradeable::id() {
if let UpgradeableLoaderState::Program {
programdata_address,
} = program_account.borrow().state()?
{
if let Some(account) = invoke_context.get_account(&programdata_address) {
Some((programdata_address, account))
} else {
return Err(InstructionError::MissingAccount);
}
} else {
return Err(InstructionError::MissingAccount);
}
} else {
return Err(InstructionError::MissingAccount);
}
} else {
None
};
let mut executable_accounts = vec![(callee_program_id, RefCell::new(program_account))];
None
};
let mut executable_accounts = vec![(callee_program_id, program_account)];
if let Some(programdata) = programdata_executable {
executable_accounts.push(programdata);
}
@@ -859,6 +873,7 @@ impl MessageProcessor {
instruction: &CompiledInstruction,
executable_accounts: &[(Pubkey, RefCell<Account>)],
accounts: &[Rc<RefCell<Account>>],
account_deps: &[(Pubkey, RefCell<Account>)],
rent_collector: &RentCollector,
log_collector: Option<Rc<LogCollector>>,
executors: Rc<RefCell<Executors>>,
@@ -887,6 +902,7 @@ impl MessageProcessor {
instruction.program_id(&message.account_keys),
rent_collector.rent,
pre_accounts,
account_deps,
&self.programs,
log_collector,
bpf_compute_budget,
@@ -917,6 +933,7 @@ impl MessageProcessor {
message: &Message,
loaders: &[Vec<(Pubkey, RefCell<Account>)>],
accounts: &[Rc<RefCell<Account>>],
account_deps: &[(Pubkey, RefCell<Account>)],
rent_collector: &RentCollector,
log_collector: Option<Rc<LogCollector>>,
executors: Rc<RefCell<Executors>>,
@@ -933,6 +950,7 @@ impl MessageProcessor {
instruction,
&loaders[instruction_index],
accounts,
account_deps,
rent_collector,
log_collector.clone(),
executors.clone(),
@@ -988,6 +1006,7 @@ mod tests {
Rent::default(),
pre_accounts,
&[],
&[],
None,
BpfComputeBudget::default(),
Rc::new(RefCell::new(Executors::default())),
@@ -1540,6 +1559,7 @@ mod tests {
&message,
&loaders,
&accounts,
&[],
&rent_collector,
None,
executors.clone(),
@@ -1564,6 +1584,7 @@ mod tests {
&message,
&loaders,
&accounts,
&[],
&rent_collector,
None,
executors.clone(),
@@ -1592,6 +1613,7 @@ mod tests {
&message,
&loaders,
&accounts,
&[],
&rent_collector,
None,
executors,
@@ -1704,6 +1726,7 @@ mod tests {
&message,
&loaders,
&accounts,
&[],
&rent_collector,
None,
executors.clone(),
@@ -1732,6 +1755,7 @@ mod tests {
&message,
&loaders,
&accounts,
&[],
&rent_collector,
None,
executors.clone(),
@@ -1757,6 +1781,7 @@ mod tests {
&message,
&loaders,
&accounts,
&[],
&rent_collector,
None,
executors,
@@ -1842,6 +1867,7 @@ mod tests {
not_owned_preaccount,
executable_preaccount,
],
&[],
programs.as_slice(),
None,
BpfComputeBudget::default(),