Save cloning program account data (#14251)

This commit is contained in:
Jack May
2020-12-23 10:18:14 -08:00
committed by GitHub
parent 29221904b9
commit 5945305b1d

View File

@ -8,7 +8,6 @@ use solana_sdk::{
account::Account, account::Account,
account_utils::StateMut, account_utils::StateMut,
bpf_loader_upgradeable::{self, UpgradeableLoaderState}, bpf_loader_upgradeable::{self, UpgradeableLoaderState},
clock::Epoch,
feature_set::{instructions_sysvar_enabled, FeatureSet}, feature_set::{instructions_sysvar_enabled, FeatureSet},
instruction::{CompiledInstruction, Instruction, InstructionError}, instruction::{CompiledInstruction, Instruction, InstructionError},
keyed_account::{create_keyed_readonly_accounts, KeyedAccount}, keyed_account::{create_keyed_readonly_accounts, KeyedAccount},
@ -54,11 +53,7 @@ pub struct PreAccount {
key: Pubkey, key: Pubkey,
is_signer: bool, is_signer: bool,
is_writable: bool, is_writable: bool,
is_executable: bool, account: RefCell<Account>,
lamports: u64,
data: Vec<u8>,
owner: Pubkey,
rent_epoch: Epoch,
} }
impl PreAccount { impl PreAccount {
pub fn new(key: &Pubkey, account: &Account, is_signer: bool, is_writable: bool) -> Self { pub fn new(key: &Pubkey, account: &Account, is_signer: bool, is_writable: bool) -> Self {
@ -66,11 +61,7 @@ impl PreAccount {
key: *key, key: *key,
is_signer, is_signer,
is_writable, is_writable,
lamports: account.lamports, account: RefCell::new(account.clone()),
data: account.data.clone(),
owner: account.owner,
is_executable: account.executable,
rent_epoch: account.rent_epoch,
} }
} }
@ -80,41 +71,43 @@ impl PreAccount {
rent: &Rent, rent: &Rent,
post: &Account, post: &Account,
) -> Result<(), InstructionError> { ) -> Result<(), InstructionError> {
let pre = self.account.borrow();
// Only the owner of the account may change owner and // Only the owner of the account may change owner and
// only if the account is writable and // only if the account is writable and
// only if the account is not executable and // only if the account is not executable and
// only if the data is zero-initialized or empty // only if the data is zero-initialized or empty
if self.owner != post.owner if pre.owner != post.owner
&& (!self.is_writable // line coverage used to get branch coverage && (!self.is_writable // line coverage used to get branch coverage
|| self.is_executable || pre.executable
|| *program_id != self.owner || *program_id != pre.owner
|| !Self::is_zeroed(&post.data)) || !Self::is_zeroed(&post.data))
{ {
return Err(InstructionError::ModifiedProgramId); return Err(InstructionError::ModifiedProgramId);
} }
// An account not assigned to the program cannot have its balance decrease. // An account not assigned to the program cannot have its balance decrease.
if *program_id != self.owner // line coverage used to get branch coverage if *program_id != pre.owner // line coverage used to get branch coverage
&& self.lamports > post.lamports && pre.lamports > post.lamports
{ {
return Err(InstructionError::ExternalAccountLamportSpend); return Err(InstructionError::ExternalAccountLamportSpend);
} }
// The balance of read-only and executable accounts may not change // The balance of read-only and executable accounts may not change
if self.lamports != post.lamports { if pre.lamports != post.lamports {
if !self.is_writable { if !self.is_writable {
return Err(InstructionError::ReadonlyLamportChange); return Err(InstructionError::ReadonlyLamportChange);
} }
if self.is_executable { if pre.executable {
return Err(InstructionError::ExecutableLamportChange); return Err(InstructionError::ExecutableLamportChange);
} }
} }
// Only the system program can change the size of the data // Only the system program can change the size of the data
// and only if the system program owns the account // and only if the system program owns the account
if self.data.len() != post.data.len() if pre.data.len() != post.data.len()
&& (!system_program::check_id(program_id) // line coverage used to get branch coverage && (!system_program::check_id(program_id) // line coverage used to get branch coverage
|| !system_program::check_id(&self.owner)) || !system_program::check_id(&pre.owner))
{ {
return Err(InstructionError::AccountDataSizeChanged); return Err(InstructionError::AccountDataSizeChanged);
} }
@ -122,12 +115,12 @@ impl PreAccount {
// Only the owner may change account data // Only the owner may change account data
// and if the account is writable // and if the account is writable
// and if the account is not executable // and if the account is not executable
if !(*program_id == self.owner if !(*program_id == pre.owner
&& self.is_writable // line coverage used to get branch coverage && self.is_writable // line coverage used to get branch coverage
&& !self.is_executable) && !pre.executable)
&& self.data != post.data && pre.data != post.data
{ {
if self.is_executable { if pre.executable {
return Err(InstructionError::ExecutableDataModified); return Err(InstructionError::ExecutableDataModified);
} else if self.is_writable { } else if self.is_writable {
return Err(InstructionError::ExternalAccountDataModified); return Err(InstructionError::ExternalAccountDataModified);
@ -137,20 +130,20 @@ impl PreAccount {
} }
// executable is one-way (false->true) and only the account owner may set it. // executable is one-way (false->true) and only the account owner may set it.
if self.is_executable != post.executable { if pre.executable != post.executable {
if !rent.is_exempt(post.lamports, post.data.len()) { if !rent.is_exempt(post.lamports, post.data.len()) {
return Err(InstructionError::ExecutableAccountNotRentExempt); return Err(InstructionError::ExecutableAccountNotRentExempt);
} }
if !self.is_writable // line coverage used to get branch coverage if !self.is_writable // line coverage used to get branch coverage
|| self.is_executable || pre.executable
|| *program_id != self.owner || *program_id != pre.owner
{ {
return Err(InstructionError::ExecutableModified); return Err(InstructionError::ExecutableModified);
} }
} }
// No one modifies rent_epoch (yet). // No one modifies rent_epoch (yet).
if self.rent_epoch != post.rent_epoch { if pre.rent_epoch != post.rent_epoch {
return Err(InstructionError::RentEpochModified); return Err(InstructionError::RentEpochModified);
} }
@ -158,14 +151,16 @@ impl PreAccount {
} }
pub fn update(&mut self, account: &Account) { pub fn update(&mut self, account: &Account) {
self.lamports = account.lamports; let mut pre = self.account.borrow_mut();
self.owner = account.owner;
if self.data.len() != account.data.len() { pre.lamports = account.lamports;
pre.owner = account.owner;
if pre.data.len() != account.data.len() {
// Only system account can change data size, copy with alloc // Only system account can change data size, copy with alloc
self.data = account.data.clone(); pre.data = account.data.clone();
} else { } else {
// Copy without allocate // Copy without allocate
self.data.clone_from_slice(&account.data); pre.data.clone_from_slice(&account.data);
} }
} }
@ -174,7 +169,7 @@ impl PreAccount {
} }
pub fn lamports(&self) -> u64 { pub fn lamports(&self) -> u64 {
self.lamports self.account.borrow().lamports
} }
pub fn is_zeroed(buf: &[u8]) -> bool { pub fn is_zeroed(buf: &[u8]) -> bool {
@ -317,24 +312,18 @@ impl<'a> InvokeContext for ThisInvokeContext<'a> {
self.feature_set.is_active(feature_id) self.feature_set.is_active(feature_id)
} }
fn get_account(&self, pubkey: &Pubkey) -> Option<RefCell<Account>> { fn get_account(&self, pubkey: &Pubkey) -> Option<RefCell<Account>> {
if let Some(account) = self.account_deps.iter().find_map(|(key, account)| { if let Some(account) = self.pre_accounts.iter().find_map(|pre| {
if key == pubkey { if pre.key == *pubkey {
Some(account.clone()) Some(pre.account.clone())
} else { } else {
None None
} }
}) { }) {
return Some(account); return Some(account);
} }
self.pre_accounts.iter().find_map(|pre| { self.account_deps.iter().find_map(|(key, account)| {
if pre.key == *pubkey { if key == pubkey {
Some(RefCell::new(Account { Some(account.clone())
lamports: pre.lamports,
data: pre.data.clone(),
owner: pre.owner,
executable: pre.is_executable,
rent_epoch: pre.rent_epoch,
}))
} else { } else {
None None
} }
@ -1049,7 +1038,10 @@ mod tests {
.verify_and_update(&message, &message.instructions[0], &these_accounts) .verify_and_update(&message, &message.instructions[0], &these_accounts)
.unwrap(); .unwrap();
assert_eq!( assert_eq!(
invoke_context.pre_accounts[owned_index].data[0], invoke_context.pre_accounts[owned_index]
.account
.borrow()
.data[0],
(MAX_DEPTH + owned_index) as u8 (MAX_DEPTH + owned_index) as u8
); );
@ -1064,7 +1056,13 @@ mod tests {
), ),
Err(InstructionError::ExternalAccountDataModified) Err(InstructionError::ExternalAccountDataModified)
); );
assert_eq!(invoke_context.pre_accounts[not_owned_index].data[0], data); assert_eq!(
invoke_context.pre_accounts[not_owned_index]
.account
.borrow()
.data[0],
data
);
accounts[not_owned_index].borrow_mut().data[0] = data; accounts[not_owned_index].borrow_mut().data[0] = data;
invoke_context.pop(); invoke_context.pop();
@ -1143,12 +1141,12 @@ mod tests {
self self
} }
pub fn executable(mut self, pre: bool, post: bool) -> Self { pub fn executable(mut self, pre: bool, post: bool) -> Self {
self.pre.is_executable = pre; self.pre.account.borrow_mut().executable = pre;
self.post.executable = post; self.post.executable = post;
self self
} }
pub fn lamports(mut self, pre: u64, post: u64) -> Self { pub fn lamports(mut self, pre: u64, post: u64) -> Self {
self.pre.lamports = pre; self.pre.account.borrow_mut().lamports = pre;
self.post.lamports = post; self.post.lamports = post;
self self
} }
@ -1157,12 +1155,12 @@ mod tests {
self self
} }
pub fn data(mut self, pre: Vec<u8>, post: Vec<u8>) -> Self { pub fn data(mut self, pre: Vec<u8>, post: Vec<u8>) -> Self {
self.pre.data = pre; self.pre.account.borrow_mut().data = pre;
self.post.data = post; self.post.data = post;
self self
} }
pub fn rent_epoch(mut self, pre: u64, post: u64) -> Self { pub fn rent_epoch(mut self, pre: u64, post: u64) -> Self {
self.pre.rent_epoch = pre; self.pre.account.borrow_mut().rent_epoch = pre;
self.post.rent_epoch = post; self.post.rent_epoch = post;
self self
} }