From 4ab4bf194281711cbc1805aa40be1381bc885fb4 Mon Sep 17 00:00:00 2001 From: Joe Date: Wed, 9 Aug 2023 17:31:11 -0600 Subject: [PATCH 01/33] replace program account --- runtime/src/bank.rs | 64 ++++++++++++++++++++++++--------------- runtime/src/bank/tests.rs | 53 ++++++++++++++++++++------------ 2 files changed, 72 insertions(+), 45 deletions(-) diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index adc9da3a4b07bb..d052b77a29437e 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -8204,32 +8204,46 @@ impl Bank { new_address: &Pubkey, datapoint_name: &'static str, ) { - if let Some(old_account) = self.get_account_with_fixed_root(old_address) { - if let Some(new_account) = self.get_account_with_fixed_root(new_address) { - datapoint_info!(datapoint_name, ("slot", self.slot, i64)); - - // Burn lamports in the old account - self.capitalization - .fetch_sub(old_account.lamports(), Relaxed); - - // Transfer new account to old account - self.store_account(old_address, &new_account); - - // Clear new account - self.store_account(new_address, &AccountSharedData::default()); - - // Unload a program from the bank's cache - self.loaded_programs_cache - .write() - .unwrap() - .remove_programs([*old_address].into_iter()); + let program_data_address = |address: &Pubkey| { + Pubkey::find_program_address(&[address.as_ref()], &bpf_loader_upgradeable::id()).0 + }; + let maybe_replace_program_account = + |old_address: &Pubkey, new_address: &Pubkey, is_program: bool| { + if let Some(old_account) = self.get_account_with_fixed_root(old_address) { + if let Some(new_account) = self.get_account_with_fixed_root(new_address) { + datapoint_info!(datapoint_name, ("slot", self.slot, i64)); + + // Burn lamports in the old account + self.capitalization + .fetch_sub(old_account.lamports(), Relaxed); + + // Transfer new account to old account + self.store_account(old_address, &new_account); + + // Clear new account + self.store_account(new_address, &AccountSharedData::default()); + + // Unload a program from the bank's cache + if is_program { + self.loaded_programs_cache + .write() + .unwrap() + .remove_programs([*old_address].into_iter()); + } - self.calculate_and_update_accounts_data_size_delta_off_chain( - old_account.data().len(), - new_account.data().len(), - ); - } - } + self.calculate_and_update_accounts_data_size_delta_off_chain( + old_account.data().len(), + new_account.data().len(), + ); + } + } + }; + maybe_replace_program_account( + &program_data_address(old_address), + &program_data_address(new_address), + false, /* is_program */ + ); + maybe_replace_program_account(old_address, new_address, true /* is_program */); } /// Get all the accounts for this bank and calculate stats diff --git a/runtime/src/bank/tests.rs b/runtime/src/bank/tests.rs index 3263eb9c41db7c..b165dc88df8062 100644 --- a/runtime/src/bank/tests.rs +++ b/runtime/src/bank/tests.rs @@ -8003,42 +8003,55 @@ fn test_compute_active_feature_set() { assert!(feature_set.is_active(&test_feature)); } +fn set_up_account_with_bank(bank: &mut Bank, pubkey: &Pubkey, lamports: u64) -> AccountSharedData { + let new_account = AccountSharedData::from(Account { + lamports, + ..Account::default() + }); + bank.store_account_and_update_capitalization(pubkey, &new_account); + assert_eq!(bank.get_balance(pubkey), lamports); + new_account +} + #[test] fn test_program_replacement() { let mut bank = create_simple_test_bank(0); - // Setup original program account + // Setup original program accounts let old_address = Pubkey::new_unique(); + set_up_account_with_bank(&mut bank, &old_address, 100); + + let (old_data_address, _) = + Pubkey::find_program_address(&[old_address.as_ref()], &bpf_loader_upgradeable::id()); + set_up_account_with_bank(&mut bank, &old_data_address, 102); + + // Setup new program accounts let new_address = Pubkey::new_unique(); - bank.store_account_and_update_capitalization( - &old_address, - &AccountSharedData::from(Account { - lamports: 100, - ..Account::default() - }), - ); - assert_eq!(bank.get_balance(&old_address), 100); - - // Setup new program account - let new_program_account = AccountSharedData::from(Account { - lamports: 123, - ..Account::default() - }); - bank.store_account_and_update_capitalization(&new_address, &new_program_account); - assert_eq!(bank.get_balance(&new_address), 123); + let new_program_account = set_up_account_with_bank(&mut bank, &new_address, 123); + + let (new_data_address, _) = + Pubkey::find_program_address(&[new_address.as_ref()], &bpf_loader_upgradeable::id()); + let new_program_data_account = set_up_account_with_bank(&mut bank, &new_data_address, 125); let original_capitalization = bank.capitalization(); bank.replace_program_account(&old_address, &new_address, "bank-apply_program_replacement"); - // New program account is now empty + // New program accounts are now empty assert_eq!(bank.get_balance(&new_address), 0); + assert_eq!(bank.get_balance(&new_data_address), 0); // Old program account holds the new program account assert_eq!(bank.get_account(&old_address), Some(new_program_account)); - // Lamports in the old token account were burnt - assert_eq!(bank.capitalization(), original_capitalization - 100); + // Old program data account holds the new program data account + assert_eq!( + bank.get_account(&old_data_address), + Some(new_program_data_account) + ); + + // Lamports in the old token accounts were burnt + assert_eq!(bank.capitalization(), original_capitalization - 100 - 102); } fn min_rent_exempt_balance_for_sysvars(bank: &Bank, sysvar_ids: &[Pubkey]) -> u64 { From fb1d4c99ef94d037f54ac169dbd4abd1e0586ffe Mon Sep 17 00:00:00 2001 From: Joe Date: Wed, 16 Aug 2023 00:56:20 -0600 Subject: [PATCH 02/33] modify for all cases --- runtime/src/bank.rs | 141 +++++++++++++++++++++------- runtime/src/bank/tests.rs | 193 +++++++++++++++++++++++++++++++++----- 2 files changed, 274 insertions(+), 60 deletions(-) diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index d052b77a29437e..4bfba014239ae6 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -8196,6 +8196,26 @@ impl Bank { } } + fn replace_account( + &mut self, + old_address: &Pubkey, + new_address: &Pubkey, + old_account: Option<&AccountSharedData>, + new_account: &AccountSharedData, + ) { + let (old_lamports, old_len) = match old_account { + Some(old_account) => (old_account.lamports(), old_account.data().len()), + None => (0, 0), + }; + self.capitalization.fetch_sub(old_lamports, Relaxed); + self.store_account(old_address, new_account); + self.store_account(new_address, &AccountSharedData::default()); + self.calculate_and_update_accounts_data_size_delta_off_chain( + old_len, + new_account.data().len(), + ); + } + /// Use to replace programs by feature activation #[allow(dead_code)] fn replace_program_account( @@ -8204,46 +8224,95 @@ impl Bank { new_address: &Pubkey, datapoint_name: &'static str, ) { - let program_data_address = |address: &Pubkey| { - Pubkey::find_program_address(&[address.as_ref()], &bpf_loader_upgradeable::id()).0 - }; - let maybe_replace_program_account = - |old_address: &Pubkey, new_address: &Pubkey, is_program: bool| { - if let Some(old_account) = self.get_account_with_fixed_root(old_address) { - if let Some(new_account) = self.get_account_with_fixed_root(new_address) { - datapoint_info!(datapoint_name, ("slot", self.slot, i64)); - - // Burn lamports in the old account - self.capitalization - .fetch_sub(old_account.lamports(), Relaxed); - - // Transfer new account to old account - self.store_account(old_address, &new_account); - - // Clear new account - self.store_account(new_address, &AccountSharedData::default()); - - // Unload a program from the bank's cache - if is_program { - self.loaded_programs_cache - .write() - .unwrap() - .remove_programs([*old_address].into_iter()); - } + // Both program accounts must exist in order to attempt a replacement + if let Some(old_account) = self.get_account_with_fixed_root(old_address) { + if let Some(new_account) = self.get_account_with_fixed_root(new_address) { + // Both exist, so we can proceed with the replacement + datapoint_info!(datapoint_name, ("slot", self.slot, i64)); + + // Derive each program's data account address (PDA) + let (old_data_address, _) = Pubkey::find_program_address( + &[old_address.as_ref()], + &bpf_loader_upgradeable::id(), + ); + let (new_data_address, _) = Pubkey::find_program_address( + &[new_address.as_ref()], + &bpf_loader_upgradeable::id(), + ); - self.calculate_and_update_accounts_data_size_delta_off_chain( - old_account.data().len(), - new_account.data().len(), + // If a data account is also provided with this new program + // account, then we want to update the existing data account + if let Some(new_data_account) = self.get_account_with_fixed_root(&new_data_address) + { + // A data account exists for the new program + // Check if the old program account has a data account + if let Some(old_data_account) = + self.get_account_with_fixed_root(&old_data_address) + { + // It does. Replace it with the new data account + self.replace_account( + &old_data_address, + &new_data_address, + Some(&old_data_account), + &new_data_account, + ); + // The old program account will already house the PDA + // of the data account + } else { + // It does _not_. Create it with the new data account + self.replace_account( + &old_data_address, + &new_data_address, + None, + &new_data_account, ); + // Update the old program account to house the PDA of + // the data account + { + let mut account = Account::from(old_account.clone()); + account.data = old_data_address.as_ref().to_vec(); + let account_shared_data = AccountSharedData::from(account); + self.store_account(old_address, &account_shared_data); + } } + // We only swapped the data accounts, so now we need to + // clear the new program account + self.capitalization + .fetch_sub(new_account.lamports(), Relaxed); + self.store_account(new_address, &AccountSharedData::default()); + } else if let Some(old_data_account) = + self.get_account_with_fixed_root(&old_data_address) + { + // A data account exists for the old program, but not the + // new program + // Swap program accounts and delete the old data account + self.replace_account( + old_address, + new_address, + Some(&old_account), + &new_account, + ); + self.capitalization + .fetch_sub(old_data_account.lamports(), Relaxed); + self.store_account(&old_data_address, &AccountSharedData::default()); + } else { + // A data account does not exist for the new program + // Swap program accounts only + self.replace_account( + old_address, + new_address, + Some(&old_account), + &new_account, + ); } - }; - maybe_replace_program_account( - &program_data_address(old_address), - &program_data_address(new_address), - false, /* is_program */ - ); - maybe_replace_program_account(old_address, new_address, true /* is_program */); + + // Unload a program from the bank's cache + self.loaded_programs_cache + .write() + .unwrap() + .remove_programs([*old_address].into_iter()); + } + } } /// Get all the accounts for this bank and calculate stats diff --git a/runtime/src/bank/tests.rs b/runtime/src/bank/tests.rs index b165dc88df8062..8ea52a5140cf3e 100644 --- a/runtime/src/bank/tests.rs +++ b/runtime/src/bank/tests.rs @@ -8003,9 +8003,15 @@ fn test_compute_active_feature_set() { assert!(feature_set.is_active(&test_feature)); } -fn set_up_account_with_bank(bank: &mut Bank, pubkey: &Pubkey, lamports: u64) -> AccountSharedData { +fn set_up_account_with_bank( + bank: &mut Bank, + pubkey: &Pubkey, + lamports: u64, + data: Vec, +) -> AccountSharedData { let new_account = AccountSharedData::from(Account { lamports, + data, ..Account::default() }); bank.store_account_and_update_capitalization(pubkey, &new_account); @@ -8014,41 +8020,180 @@ fn set_up_account_with_bank(bank: &mut Bank, pubkey: &Pubkey, lamports: u64) -> } #[test] -fn test_program_replacement() { +fn test_non_upgradable_program_replace() { + // Non-Upgradable program + // - Old: [Old program data] + // - New: [*New program data] + // + // Should replace the old program account with the new program account: + // - Old: [*New program data] let mut bank = create_simple_test_bank(0); - // Setup original program accounts - let old_address = Pubkey::new_unique(); - set_up_account_with_bank(&mut bank, &old_address, 100); + let old = Pubkey::new_unique(); + set_up_account_with_bank(&mut bank, &old, 100, vec![0, 0, 0, 0]); - let (old_data_address, _) = - Pubkey::find_program_address(&[old_address.as_ref()], &bpf_loader_upgradeable::id()); - set_up_account_with_bank(&mut bank, &old_data_address, 102); + let new = Pubkey::new_unique(); + let new_program_bytes = vec![6, 5, 4, 3, 2, 1, 0]; + set_up_account_with_bank(&mut bank, &new, 100, new_program_bytes.clone()); - // Setup new program accounts - let new_address = Pubkey::new_unique(); - let new_program_account = set_up_account_with_bank(&mut bank, &new_address, 123); + let original_capitalization = bank.capitalization(); + + bank.replace_program_account(&old, &new, "bank-apply_program_replacement"); + + // Old program account balance is unchanged + assert_eq!(bank.get_balance(&old), 100); + + // New program account is now empty + assert_eq!(bank.get_balance(&new), 0); + + // Old program account now holds the new program data, ie: + // - Old: [*New program data] + let old_account = bank.get_account(&old).unwrap(); + assert_eq!(old_account.data(), &new_program_bytes,); + + // Lamports in the old token account was burnt + assert_eq!(bank.capitalization(), original_capitalization - 100); +} + +#[test] +fn test_non_upgradable_program_replace_with_data() { + // Non-Upgradable program + // - Old: [Old program data] + // - New: PDA(NewData) + // - NewData: [*New program data] + // + // Should replace the program account with the PDA of the data account, + // and create the data account: + // - Old: PDA(OldData) + // - OldData: [*New program data] + let bpf_id = bpf_loader_upgradeable::id(); + let mut bank = create_simple_test_bank(0); + + let old = Pubkey::new_unique(); + let (old_data, _) = Pubkey::find_program_address(&[old.as_ref()], &bpf_id); + set_up_account_with_bank(&mut bank, &old, 100, vec![0, 0, 0, 0]); - let (new_data_address, _) = - Pubkey::find_program_address(&[new_address.as_ref()], &bpf_loader_upgradeable::id()); - let new_program_data_account = set_up_account_with_bank(&mut bank, &new_data_address, 125); + let new = Pubkey::new_unique(); + let (new_data, _) = Pubkey::find_program_address(&[new.as_ref()], &bpf_id); + let new_program_bytes = vec![6, 5, 4, 3, 2, 1, 0]; + set_up_account_with_bank(&mut bank, &new, 100, new_data.to_bytes().to_vec()); + set_up_account_with_bank(&mut bank, &new_data, 102, new_program_bytes.clone()); let original_capitalization = bank.capitalization(); - bank.replace_program_account(&old_address, &new_address, "bank-apply_program_replacement"); + bank.replace_program_account(&old, &new, "bank-apply_program_replacement"); + + // Old program account balances are unchanged + assert_eq!(bank.get_balance(&old), 100); + assert_eq!(bank.get_balance(&old_data), 102); // New program accounts are now empty - assert_eq!(bank.get_balance(&new_address), 0); - assert_eq!(bank.get_balance(&new_data_address), 0); + assert_eq!(bank.get_balance(&new), 0); + assert_eq!(bank.get_balance(&new_data), 0); - // Old program account holds the new program account - assert_eq!(bank.get_account(&old_address), Some(new_program_account)); + // Old program account now holds the PDA, ie: + // - Old: PDA(OldData) + let old_account = bank.get_account(&old).unwrap(); + assert_eq!(old_account.data(), &old_data.to_bytes().to_vec(),); - // Old program data account holds the new program data account - assert_eq!( - bank.get_account(&old_data_address), - Some(new_program_data_account) - ); + // Old program data account has been created & now holds the new data, ie: + // - OldData: [*New program data] + let old_data_account = bank.get_account(&old_data).unwrap(); + assert_eq!(old_data_account.data(), &new_program_bytes,); + + // Lamports in the old token accounts were burnt + assert_eq!(bank.capitalization(), original_capitalization - 100); +} + +#[test] +fn test_upgradable_program_replace() { + // Upgradable program + // - Old: PDA(OldData) + // - OldData: [Old program data] + // - New: PDA(NewData) + // - NewData: [*New program data] + // + // Should _only_ replace the data account, not the program account: + // - Old: PDA(OldData) + // - OldData: [*New program data] + let bpf_id = bpf_loader_upgradeable::id(); + let mut bank = create_simple_test_bank(0); + + let old = Pubkey::new_unique(); + let (old_data, _) = Pubkey::find_program_address(&[old.as_ref()], &bpf_id); + set_up_account_with_bank(&mut bank, &old, 100, old_data.to_bytes().to_vec()); + set_up_account_with_bank(&mut bank, &old_data, 102, vec![0, 1, 2, 3, 4, 5, 6]); + + let new = Pubkey::new_unique(); + let (new_data, _) = Pubkey::find_program_address(&[new.as_ref()], &bpf_id); + let new_program_bytes = vec![6, 5, 4, 3, 2, 1, 0]; + set_up_account_with_bank(&mut bank, &new, 100, new_data.to_bytes().to_vec()); + set_up_account_with_bank(&mut bank, &new_data, 102, new_program_bytes.clone()); + + let original_capitalization = bank.capitalization(); + + bank.replace_program_account(&old, &new, "bank-apply_program_replacement"); + + // Old program account balances are unchanged + assert_eq!(bank.get_balance(&old), 100); + assert_eq!(bank.get_balance(&old_data), 102); + + // New program accounts are now empty + assert_eq!(bank.get_balance(&new), 0); + assert_eq!(bank.get_balance(&new_data), 0); + + // Old program account still holds the same PDA, ie: + // - Old: PDA(OldData) + let old_account = bank.get_account(&old).unwrap(); + assert_eq!(old_account.data(), &old_data.to_bytes().to_vec(),); + + // Old program data account now holds the new data, ie: + // - OldData: [*New program data] + let old_data_account = bank.get_account(&old_data).unwrap(); + assert_eq!(old_data_account.data(), &new_program_bytes,); + + // Lamports in the old token accounts were burnt + assert_eq!(bank.capitalization(), original_capitalization - 100 - 102); +} + +#[test] +fn test_upgradable_program_replace_with_no_data() { + // Upgradable program + // - Old: PDA(OldData) + // - OldData: [Old program data] + // - New: [*New program data] + // + // Should replace the program account and delete the data account: + // - Old: [*New program data] + let bpf_id = bpf_loader_upgradeable::id(); + let mut bank = create_simple_test_bank(0); + + let old = Pubkey::new_unique(); + let (old_data, _) = Pubkey::find_program_address(&[old.as_ref()], &bpf_id); + set_up_account_with_bank(&mut bank, &old, 100, old_data.to_bytes().to_vec()); + set_up_account_with_bank(&mut bank, &old_data, 102, vec![0, 1, 2, 3, 4, 5, 6]); + + let new = Pubkey::new_unique(); + let new_program_bytes = vec![6, 5, 4, 3, 2, 1, 0]; + set_up_account_with_bank(&mut bank, &new, 100, new_program_bytes.clone()); + + let original_capitalization = bank.capitalization(); + + bank.replace_program_account(&old, &new, "bank-apply_program_replacement"); + + // Old program account balance is unchanged + assert_eq!(bank.get_balance(&old), 100); + + // Old data account is now empty + assert_eq!(bank.get_balance(&old_data), 0); + + // New program account is now empty + assert_eq!(bank.get_balance(&new), 0); + + // Old program account now holds the new program bytes + // - Old: [*New program data] + let old_account = bank.get_account(&old).unwrap(); + assert_eq!(old_account.data(), &new_program_bytes,); // Lamports in the old token accounts were burnt assert_eq!(bank.capitalization(), original_capitalization - 100 - 102); From 99eb96ece86d0e6d9bc086744ae567eedc1de793 Mon Sep 17 00:00:00 2001 From: Joe Date: Wed, 16 Aug 2023 21:23:06 -0600 Subject: [PATCH 03/33] remove non-data swap --- runtime/src/bank.rs | 18 +++--------------- runtime/src/bank/tests.rs | 36 +++++++++++++++++++++--------------- 2 files changed, 24 insertions(+), 30 deletions(-) diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 4bfba014239ae6..bc120281a06b8e 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -8280,22 +8280,10 @@ impl Bank { self.capitalization .fetch_sub(new_account.lamports(), Relaxed); self.store_account(new_address, &AccountSharedData::default()); - } else if let Some(old_data_account) = - self.get_account_with_fixed_root(&old_data_address) + } else if self + .get_account_with_fixed_root(&old_data_address) + .is_none() { - // A data account exists for the old program, but not the - // new program - // Swap program accounts and delete the old data account - self.replace_account( - old_address, - new_address, - Some(&old_account), - &new_account, - ); - self.capitalization - .fetch_sub(old_data_account.lamports(), Relaxed); - self.store_account(&old_data_address, &AccountSharedData::default()); - } else { // A data account does not exist for the new program // Swap program accounts only self.replace_account( diff --git a/runtime/src/bank/tests.rs b/runtime/src/bank/tests.rs index 8ea52a5140cf3e..c89dc2219fd03b 100644 --- a/runtime/src/bank/tests.rs +++ b/runtime/src/bank/tests.rs @@ -8163,15 +8163,17 @@ fn test_upgradable_program_replace_with_no_data() { // - OldData: [Old program data] // - New: [*New program data] // - // Should replace the program account and delete the data account: - // - Old: [*New program data] + // This is NOT allowed and should leave the program unchanged: + // - Old: PDA(OldData) + // - OldData: [Old program data] let bpf_id = bpf_loader_upgradeable::id(); let mut bank = create_simple_test_bank(0); let old = Pubkey::new_unique(); let (old_data, _) = Pubkey::find_program_address(&[old.as_ref()], &bpf_id); + let old_program_bytes = vec![0, 1, 2, 3, 4, 5, 6]; set_up_account_with_bank(&mut bank, &old, 100, old_data.to_bytes().to_vec()); - set_up_account_with_bank(&mut bank, &old_data, 102, vec![0, 1, 2, 3, 4, 5, 6]); + set_up_account_with_bank(&mut bank, &old_data, 102, old_program_bytes.clone()); let new = Pubkey::new_unique(); let new_program_bytes = vec![6, 5, 4, 3, 2, 1, 0]; @@ -8181,22 +8183,26 @@ fn test_upgradable_program_replace_with_no_data() { bank.replace_program_account(&old, &new, "bank-apply_program_replacement"); - // Old program account balance is unchanged + // All balances are unchanged assert_eq!(bank.get_balance(&old), 100); + assert_eq!(bank.get_balance(&old_data), 102); + assert_eq!(bank.get_balance(&new), 100); - // Old data account is now empty - assert_eq!(bank.get_balance(&old_data), 0); - - // New program account is now empty - assert_eq!(bank.get_balance(&new), 0); - - // Old program account now holds the new program bytes - // - Old: [*New program data] + // Old program accounts' data are unchanged + // - Old: PDA(OldData) + // - OldData: [Old program data] let old_account = bank.get_account(&old).unwrap(); - assert_eq!(old_account.data(), &new_program_bytes,); + assert_eq!(old_account.data(), &old_data.to_bytes().to_vec()); + let old_data_account = bank.get_account(&old_data).unwrap(); + assert_eq!(old_data_account.data(), &old_program_bytes); - // Lamports in the old token accounts were burnt - assert_eq!(bank.capitalization(), original_capitalization - 100 - 102); + // New program account is unchanged + // - New: [*New program data] + let new_account = bank.get_account(&new).unwrap(); + assert_eq!(new_account.data(), &new_program_bytes); + + // Lamports were unchanged across the board + assert_eq!(bank.capitalization(), original_capitalization); } fn min_rent_exempt_balance_for_sysvars(bank: &Bank, sysvar_ids: &[Pubkey]) -> u64 { From acbca38d9a0a132706e51d1e531d62c364416694 Mon Sep 17 00:00:00 2001 From: Joe Date: Thu, 17 Aug 2023 19:14:06 -0600 Subject: [PATCH 04/33] address tests & conditional feedback --- runtime/src/bank.rs | 46 +++----- runtime/src/bank/tests.rs | 232 +++++++++++++++++++++++++++++++++----- 2 files changed, 217 insertions(+), 61 deletions(-) diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index bc120281a06b8e..186fbbe39c6472 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -8244,36 +8244,22 @@ impl Bank { // account, then we want to update the existing data account if let Some(new_data_account) = self.get_account_with_fixed_root(&new_data_address) { - // A data account exists for the new program - // Check if the old program account has a data account - if let Some(old_data_account) = - self.get_account_with_fixed_root(&old_data_address) - { - // It does. Replace it with the new data account - self.replace_account( - &old_data_address, - &new_data_address, - Some(&old_data_account), - &new_data_account, - ); - // The old program account will already house the PDA - // of the data account - } else { - // It does _not_. Create it with the new data account - self.replace_account( - &old_data_address, - &new_data_address, - None, - &new_data_account, - ); - // Update the old program account to house the PDA of - // the data account - { - let mut account = Account::from(old_account.clone()); - account.data = old_data_address.as_ref().to_vec(); - let account_shared_data = AccountSharedData::from(account); - self.store_account(old_address, &account_shared_data); - } + // If a data account exists for the old program, it will be + // replaced with the new data account + // If a data account does not exist for the old program, it + // will be created with the new data account + self.replace_account( + &old_data_address, + &new_data_address, + self.get_account_with_fixed_root(&old_data_address).as_ref(), // None if no old data account + &new_data_account, + ); + // If necessary, update the old program account to house + // the PDA of the data account. + if old_account.data() != old_data_address.as_ref() { + let mut account = Account::from(old_account.clone()); + account.data = old_data_address.as_ref().to_vec(); + self.store_account(old_address, &account); } // We only swapped the data accounts, so now we need to // clear the new program account diff --git a/runtime/src/bank/tests.rs b/runtime/src/bank/tests.rs index c89dc2219fd03b..bf1a5031357433 100644 --- a/runtime/src/bank/tests.rs +++ b/runtime/src/bank/tests.rs @@ -8019,8 +8019,9 @@ fn set_up_account_with_bank( new_account } +// FIXME: Use rent-real lamport values #[test] -fn test_non_upgradable_program_replace() { +fn test_program_replace_non_upgradable_base_case() { // Non-Upgradable program // - Old: [Old program data] // - New: [*New program data] @@ -8030,18 +8031,19 @@ fn test_non_upgradable_program_replace() { let mut bank = create_simple_test_bank(0); let old = Pubkey::new_unique(); - set_up_account_with_bank(&mut bank, &old, 100, vec![0, 0, 0, 0]); + let old_program_bytes = vec![0, 0, 0, 0]; + set_up_account_with_bank(&mut bank, &old, 100, old_program_bytes); let new = Pubkey::new_unique(); let new_program_bytes = vec![6, 5, 4, 3, 2, 1, 0]; - set_up_account_with_bank(&mut bank, &new, 100, new_program_bytes.clone()); + set_up_account_with_bank(&mut bank, &new, 200, new_program_bytes.clone()); let original_capitalization = bank.capitalization(); bank.replace_program_account(&old, &new, "bank-apply_program_replacement"); - // Old program account balance is unchanged - assert_eq!(bank.get_balance(&old), 100); + // Old program account balance is now the new program account's balance + assert_eq!(bank.get_balance(&old), 200); // New program account is now empty assert_eq!(bank.get_balance(&new), 0); @@ -8049,14 +8051,15 @@ fn test_non_upgradable_program_replace() { // Old program account now holds the new program data, ie: // - Old: [*New program data] let old_account = bank.get_account(&old).unwrap(); - assert_eq!(old_account.data(), &new_program_bytes,); + assert_eq!(old_account.data(), &new_program_bytes); - // Lamports in the old token account was burnt + // Lamports in the old program account were burnt assert_eq!(bank.capitalization(), original_capitalization - 100); } +// FIXME: Use rent-real lamport values #[test] -fn test_non_upgradable_program_replace_with_data() { +fn test_program_replace_non_upgradable_create_data() { // Non-Upgradable program // - Old: [Old program data] // - New: PDA(NewData) @@ -8070,22 +8073,29 @@ fn test_non_upgradable_program_replace_with_data() { let mut bank = create_simple_test_bank(0); let old = Pubkey::new_unique(); + let old_program_bytes = vec![0, 0, 0, 0]; let (old_data, _) = Pubkey::find_program_address(&[old.as_ref()], &bpf_id); - set_up_account_with_bank(&mut bank, &old, 100, vec![0, 0, 0, 0]); + set_up_account_with_bank(&mut bank, &old, 100, old_program_bytes); let new = Pubkey::new_unique(); let (new_data, _) = Pubkey::find_program_address(&[new.as_ref()], &bpf_id); let new_program_bytes = vec![6, 5, 4, 3, 2, 1, 0]; - set_up_account_with_bank(&mut bank, &new, 100, new_data.to_bytes().to_vec()); - set_up_account_with_bank(&mut bank, &new_data, 102, new_program_bytes.clone()); + set_up_account_with_bank(&mut bank, &new, 200, new_data.to_bytes().to_vec()); + set_up_account_with_bank(&mut bank, &new_data, 204, new_program_bytes.clone()); let original_capitalization = bank.capitalization(); bank.replace_program_account(&old, &new, "bank-apply_program_replacement"); - // Old program account balances are unchanged + // FIXME: We can't assume the size is unchanged + // + // Old program account's balance was unchanged + // (Data was maybe modified, but size was unchanged) assert_eq!(bank.get_balance(&old), 100); - assert_eq!(bank.get_balance(&old_data), 102); + + // Old data account's balance is now the new data account's balance + // (newly created) + assert_eq!(bank.get_balance(&old_data), 204); // New program accounts are now empty assert_eq!(bank.get_balance(&new), 0); @@ -8094,19 +8104,116 @@ fn test_non_upgradable_program_replace_with_data() { // Old program account now holds the PDA, ie: // - Old: PDA(OldData) let old_account = bank.get_account(&old).unwrap(); - assert_eq!(old_account.data(), &old_data.to_bytes().to_vec(),); + assert_eq!(old_account.data(), &old_data.to_bytes().to_vec()); - // Old program data account has been created & now holds the new data, ie: + // Old data account has been created & now holds the new data, ie: // - OldData: [*New program data] let old_data_account = bank.get_account(&old_data).unwrap(); - assert_eq!(old_data_account.data(), &new_program_bytes,); + assert_eq!(old_data_account.data(), &new_program_bytes); - // Lamports in the old token accounts were burnt - assert_eq!(bank.capitalization(), original_capitalization - 100); + // Lamports in the old program account were burnt + assert_eq!(bank.capitalization(), original_capitalization - 200); } +// FIXME: Use rent-real lamport values #[test] -fn test_upgradable_program_replace() { +fn test_program_replace_non_upgradable_does_not_exist() { + // Non-Upgradable program + // - Old: ** Does not exist! ** + // - New: PDA(NewData) + // - NewData: [*New program data] + // + // This is NOT allowed and should leave everything unchanged + let bpf_id = bpf_loader_upgradeable::id(); + let mut bank = create_simple_test_bank(0); + + let old = Pubkey::new_unique(); + let (old_data, _) = Pubkey::find_program_address(&[old.as_ref()], &bpf_id); + + let new = Pubkey::new_unique(); + let (new_data, _) = Pubkey::find_program_address(&[new.as_ref()], &bpf_id); + let new_program_bytes = vec![6, 5, 4, 3, 2, 1, 0]; + set_up_account_with_bank(&mut bank, &new, 200, new_data.to_bytes().to_vec()); + set_up_account_with_bank(&mut bank, &new_data, 204, new_program_bytes); + + let original_capitalization = bank.capitalization(); + + bank.replace_program_account(&old, &new, "bank-apply_program_replacement"); + + // Old program accounts still don't exist + assert_eq!(bank.get_balance(&old), 0); + assert_eq!(bank.get_balance(&old_data), 0); + + // New program accounts are unchanged + assert_eq!(bank.get_balance(&new), 200); + assert_eq!(bank.get_balance(&new_data), 204); + + // Lamports were unchanged across the board + assert_eq!(bank.capitalization(), original_capitalization); +} + +// FIXME: Use rent-real lamport values +#[cfg(ignore)] +#[test] +fn test_program_replace_non_upgradable_is_native_account() { + // Non-Upgradable program + // - Old: ** Native account (ie. `Feature11111111`) ** + // - New: PDA(NewData) + // - NewData: [*New program data] + // + // Should replace the native account with the PDA of the data account, + // and create the data account: + // - Old: PDA(OldData) + // - OldData: [*New program data] + let bpf_id = bpf_loader_upgradeable::id(); + let mut bank = create_simple_test_bank(0); + + let old = feature::id(); + let (old_data, _) = Pubkey::find_program_address(&[old.as_ref()], &bpf_id); + + let new = Pubkey::new_unique(); + let (new_data, _) = Pubkey::find_program_address(&[new.as_ref()], &bpf_id); + let new_program_bytes = vec![6, 5, 4, 3, 2, 1, 0]; + set_up_account_with_bank(&mut bank, &new, 200, new_data.to_bytes().to_vec()); + set_up_account_with_bank(&mut bank, &new_data, 204, new_program_bytes.clone()); + + let original_capitalization = bank.capitalization(); + + bank.replace_program_account(&old, &new, "bank-apply_program_replacement"); + + // TODO + + // FIXME: We can't assume the size is unchanged + // + // Old program account's balance was unchanged + // (Data was maybe modified, but size was unchanged) + assert_eq!(bank.get_balance(&old), 0); + + // Old data account's balance is now the new data account's balance + // (newly created) + assert_eq!(bank.get_balance(&old_data), 204); + + // New program accounts are now empty + assert_eq!(bank.get_balance(&new), 0); + assert_eq!(bank.get_balance(&new_data), 0); + + // Old program account now holds the PDA, ie: + // - Old: PDA(OldData) + let old_account = bank.get_account(&old).unwrap(); + assert_eq!(old_account.data(), &old_data.to_bytes().to_vec()); + + // Old data account has been created & now holds the new data, ie: + // - OldData: [*New program data] + let old_data_account = bank.get_account(&old_data).unwrap(); + assert_eq!(old_data_account.data(), &new_program_bytes); + + // Lamports in the old program account were burnt + assert_eq!(bank.capitalization(), original_capitalization - 200); +} + +// FIXME: Use rent-real lamport values +#[test] +fn test_program_replace_upgradable_base_case() { // Upgradable program // - Old: PDA(OldData) // - OldData: [Old program data] @@ -8127,16 +8234,21 @@ fn test_upgradable_program_replace() { let new = Pubkey::new_unique(); let (new_data, _) = Pubkey::find_program_address(&[new.as_ref()], &bpf_id); let new_program_bytes = vec![6, 5, 4, 3, 2, 1, 0]; - set_up_account_with_bank(&mut bank, &new, 100, new_data.to_bytes().to_vec()); - set_up_account_with_bank(&mut bank, &new_data, 102, new_program_bytes.clone()); + set_up_account_with_bank(&mut bank, &new, 200, new_data.to_bytes().to_vec()); + set_up_account_with_bank(&mut bank, &new_data, 204, new_program_bytes.clone()); let original_capitalization = bank.capitalization(); bank.replace_program_account(&old, &new, "bank-apply_program_replacement"); - // Old program account balances are unchanged + // FIXME: We can't assume the size is unchanged + // + // Old program account's balance was unchanged + // (Data was maybe modified, but size was unchanged) assert_eq!(bank.get_balance(&old), 100); - assert_eq!(bank.get_balance(&old_data), 102); + + // Old data account's balance is now the new data account's balance + assert_eq!(bank.get_balance(&old_data), 204); // New program accounts are now empty assert_eq!(bank.get_balance(&new), 0); @@ -8145,19 +8257,77 @@ fn test_upgradable_program_replace() { // Old program account still holds the same PDA, ie: // - Old: PDA(OldData) let old_account = bank.get_account(&old).unwrap(); - assert_eq!(old_account.data(), &old_data.to_bytes().to_vec(),); + assert_eq!(old_account.data(), &old_data.to_bytes().to_vec()); + + // Old data account now holds the new data, ie: + // - OldData: [*New program data] + let old_data_account = bank.get_account(&old_data).unwrap(); + assert_eq!(old_data_account.data(), &new_program_bytes); + + // Lamports in the old program accounts were burnt + assert_eq!(bank.capitalization(), original_capitalization - 102 - 200); +} + +// FIXME: Use rent-real lamport values +#[test] +fn test_program_replace_upgradable_not_data_pda_at_first() { + // Upgradable program + // - Old: [*Old arbitrary data] + // - OldData: [Old program data] + // - New: PDA(NewData) + // - NewData: [*New program data] + // + // Should _only_ replace the data account, not the program account: + // - Old: PDA(OldData) + // - OldData: [*New program data] + let bpf_id = bpf_loader_upgradeable::id(); + let mut bank = create_simple_test_bank(0); + + let old = Pubkey::new_unique(); + let (old_data, _) = Pubkey::find_program_address(&[old.as_ref()], &bpf_id); + set_up_account_with_bank(&mut bank, &old, 100, old_data.to_bytes().to_vec()); + set_up_account_with_bank(&mut bank, &old_data, 102, vec![0, 1, 2, 3, 4, 5, 6]); + + let new = Pubkey::new_unique(); + let (new_data, _) = Pubkey::find_program_address(&[new.as_ref()], &bpf_id); + let new_program_bytes = vec![6, 5, 4, 3, 2, 1, 0]; + set_up_account_with_bank(&mut bank, &new, 200, new_data.to_bytes().to_vec()); + set_up_account_with_bank(&mut bank, &new_data, 204, new_program_bytes.clone()); + + let original_capitalization = bank.capitalization(); + + bank.replace_program_account(&old, &new, "bank-apply_program_replacement"); + + // FIXME: We can't assume the size is unchanged + // + // Old program account's balance was unchanged + // (Data was maybe modified, but size was unchanged) + assert_eq!(bank.get_balance(&old), 100); + + // Old data account's balance is now the new data account's balance + assert_eq!(bank.get_balance(&old_data), 204); + + // New program accounts are now empty + assert_eq!(bank.get_balance(&new), 0); + assert_eq!(bank.get_balance(&new_data), 0); + + // Old program account now holds the PDA, ie: + // - Old: PDA(OldData) + let old_account = bank.get_account(&old).unwrap(); + assert_eq!(old_account.data(), &old_data.to_bytes().to_vec()); - // Old program data account now holds the new data, ie: + // Old data account now holds the new data, ie: // - OldData: [*New program data] let old_data_account = bank.get_account(&old_data).unwrap(); - assert_eq!(old_data_account.data(), &new_program_bytes,); + assert_eq!(old_data_account.data(), &new_program_bytes); - // Lamports in the old token accounts were burnt - assert_eq!(bank.capitalization(), original_capitalization - 100 - 102); + // Lamports in the old program accounts were burnt + assert_eq!(bank.capitalization(), original_capitalization - 102 - 200); } +// FIXME: Use rent-real lamport values #[test] -fn test_upgradable_program_replace_with_no_data() { +fn test_program_replace_upgradable_no_data_provided_with_replacement() { // Upgradable program // - Old: PDA(OldData) // - OldData: [Old program data] @@ -8177,7 +8347,7 @@ fn test_upgradable_program_replace_with_no_data() { let new = Pubkey::new_unique(); let new_program_bytes = vec![6, 5, 4, 3, 2, 1, 0]; - set_up_account_with_bank(&mut bank, &new, 100, new_program_bytes.clone()); + set_up_account_with_bank(&mut bank, &new, 200, new_program_bytes.clone()); let original_capitalization = bank.capitalization(); @@ -8186,7 +8356,7 @@ fn test_upgradable_program_replace_with_no_data() { // All balances are unchanged assert_eq!(bank.get_balance(&old), 100); assert_eq!(bank.get_balance(&old_data), 102); - assert_eq!(bank.get_balance(&new), 100); + assert_eq!(bank.get_balance(&new), 200); // Old program accounts' data are unchanged // - Old: PDA(OldData) From a8800b8b291b2653d895f45d3662cf6af19f6e64 Mon Sep 17 00:00:00 2001 From: Joe Date: Thu, 17 Aug 2023 20:34:17 -0600 Subject: [PATCH 05/33] get the rent involved --- runtime/src/bank.rs | 26 +++- runtime/src/bank/tests.rs | 301 ++++++++++++++++++++++++++++---------- 2 files changed, 244 insertions(+), 83 deletions(-) diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 186fbbe39c6472..f1543467fdea17 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -8251,21 +8251,33 @@ impl Bank { self.replace_account( &old_data_address, &new_data_address, - self.get_account_with_fixed_root(&old_data_address).as_ref(), // None if no old data account + self.get_account_with_fixed_root(&old_data_address).as_ref(), &new_data_account, ); // If necessary, update the old program account to house // the PDA of the data account. - if old_account.data() != old_data_address.as_ref() { - let mut account = Account::from(old_account.clone()); - account.data = old_data_address.as_ref().to_vec(); + let data = old_data_address.as_ref().to_vec(); + let change_in_cap = if old_account.data() != data { + // Determine if we need to add any lamports for rent + // exemption + let lamports = old_account + .lamports() + .max(self.get_minimum_balance_for_rent_exemption(data.len())); + let account = Account { + lamports, + data, + ..Account::from(old_account.clone()) + }; self.store_account(old_address, &account); - } + new_account.lamports() + old_account.lamports() - account.lamports + } else { + new_account.lamports() + }; // We only swapped the data accounts, so now we need to // clear the new program account - self.capitalization - .fetch_sub(new_account.lamports(), Relaxed); self.store_account(new_address, &AccountSharedData::default()); + // Update capitalization + self.capitalization.fetch_sub(change_in_cap, Relaxed); } else if self .get_account_with_fixed_root(&old_data_address) .is_none() diff --git a/runtime/src/bank/tests.rs b/runtime/src/bank/tests.rs index bf1a5031357433..f81bac1cf04ddc 100644 --- a/runtime/src/bank/tests.rs +++ b/runtime/src/bank/tests.rs @@ -8019,7 +8019,6 @@ fn set_up_account_with_bank( new_account } -// FIXME: Use rent-real lamport values #[test] fn test_program_replace_non_upgradable_base_case() { // Non-Upgradable program @@ -8031,19 +8030,21 @@ fn test_program_replace_non_upgradable_base_case() { let mut bank = create_simple_test_bank(0); let old = Pubkey::new_unique(); - let old_program_bytes = vec![0, 0, 0, 0]; - set_up_account_with_bank(&mut bank, &old, 100, old_program_bytes); + let old_bytes = vec![0, 0, 0, 0]; + let old_lamports = bank.get_minimum_balance_for_rent_exemption(old_bytes.len()); + set_up_account_with_bank(&mut bank, &old, old_lamports, old_bytes); let new = Pubkey::new_unique(); - let new_program_bytes = vec![6, 5, 4, 3, 2, 1, 0]; - set_up_account_with_bank(&mut bank, &new, 200, new_program_bytes.clone()); + let new_bytes = vec![6; 30]; + let new_lamports = bank.get_minimum_balance_for_rent_exemption(new_bytes.len()); + set_up_account_with_bank(&mut bank, &new, new_lamports, new_bytes.clone()); let original_capitalization = bank.capitalization(); bank.replace_program_account(&old, &new, "bank-apply_program_replacement"); // Old program account balance is now the new program account's balance - assert_eq!(bank.get_balance(&old), 200); + assert_eq!(bank.get_balance(&old), new_lamports); // New program account is now empty assert_eq!(bank.get_balance(&new), 0); @@ -8051,17 +8052,19 @@ fn test_program_replace_non_upgradable_base_case() { // Old program account now holds the new program data, ie: // - Old: [*New program data] let old_account = bank.get_account(&old).unwrap(); - assert_eq!(old_account.data(), &new_program_bytes); + assert_eq!(old_account.data(), &new_bytes); - // Lamports in the old program account were burnt - assert_eq!(bank.capitalization(), original_capitalization - 100); + // Lamports from the new program account were burnt + assert_eq!( + bank.capitalization(), + original_capitalization - old_lamports + ); } -// FIXME: Use rent-real lamport values #[test] -fn test_program_replace_non_upgradable_create_data() { +fn test_program_replace_non_upgradable_create_data_add_rent() { // Non-Upgradable program - // - Old: [Old program data] + // - Old: [Old program data] (smaller than minimum rent-exempt size) // - New: PDA(NewData) // - NewData: [*New program data] // @@ -8073,29 +8076,42 @@ fn test_program_replace_non_upgradable_create_data() { let mut bank = create_simple_test_bank(0); let old = Pubkey::new_unique(); - let old_program_bytes = vec![0, 0, 0, 0]; + let old_bytes = vec![0, 0, 0, 0]; // Smaller than 32 bytes + let old_lamports = bank.get_minimum_balance_for_rent_exemption(old_bytes.len()); let (old_data, _) = Pubkey::find_program_address(&[old.as_ref()], &bpf_id); - set_up_account_with_bank(&mut bank, &old, 100, old_program_bytes); + set_up_account_with_bank(&mut bank, &old, old_lamports, old_bytes); let new = Pubkey::new_unique(); let (new_data, _) = Pubkey::find_program_address(&[new.as_ref()], &bpf_id); - let new_program_bytes = vec![6, 5, 4, 3, 2, 1, 0]; - set_up_account_with_bank(&mut bank, &new, 200, new_data.to_bytes().to_vec()); - set_up_account_with_bank(&mut bank, &new_data, 204, new_program_bytes.clone()); + let new_bytes = new_data.to_bytes().to_vec(); + let new_lamports = bank.get_minimum_balance_for_rent_exemption(new_bytes.len()); + let new_data_bytes = vec![6; 30]; + let new_data_lamports = bank.get_minimum_balance_for_rent_exemption(new_data_bytes.len()); + set_up_account_with_bank(&mut bank, &new, new_lamports, new_bytes); + set_up_account_with_bank( + &mut bank, + &new_data, + new_data_lamports, + new_data_bytes.clone(), + ); let original_capitalization = bank.capitalization(); bank.replace_program_account(&old, &new, "bank-apply_program_replacement"); - // FIXME: We can't assume the size is unchanged + // Old program account's balance was adjusted to pay for minimum rent for + // the PDA (32 bytes) // - // Old program account's balance was unchanged - // (Data was maybe modified, but size was unchanged) - assert_eq!(bank.get_balance(&old), 100); + // Note: In this test, the original lamports were _less_ than required + // rent for 32 bytes, so lamports were added + assert_eq!( + bank.get_balance(&old), + bank.get_minimum_balance_for_rent_exemption(old_data.to_bytes().len()) + ); // Old data account's balance is now the new data account's balance // (newly created) - assert_eq!(bank.get_balance(&old_data), 204); + assert_eq!(bank.get_balance(&old_data), new_data_lamports); // New program accounts are now empty assert_eq!(bank.get_balance(&new), 0); @@ -8109,13 +8125,88 @@ fn test_program_replace_non_upgradable_create_data() { // Old data account has been created & now holds the new data, ie: // - OldData: [*New program data] let old_data_account = bank.get_account(&old_data).unwrap(); - assert_eq!(old_data_account.data(), &new_program_bytes); + assert_eq!(old_data_account.data(), &new_data_bytes); - // Lamports in the old program account were burnt - assert_eq!(bank.capitalization(), original_capitalization - 200); + // The remaining lamports from both program accounts minus the rent-exempt + // minimum were burnt + assert_eq!( + bank.capitalization(), + original_capitalization + - (new_lamports + old_lamports + - bank.get_minimum_balance_for_rent_exemption(old_data.to_bytes().len())) + ); +} + +#[test] +fn test_program_replace_non_upgradable_create_data_dont_add_rent() { + // Non-Upgradable program + // - Old: [Old program data] (larger than minimum rent-exempt size) + // - New: PDA(NewData) + // - NewData: [*New program data] + // + // Should replace the program account with the PDA of the data account, + // and create the data account: + // - Old: PDA(OldData) + // - OldData: [*New program data] + let bpf_id = bpf_loader_upgradeable::id(); + let mut bank = create_simple_test_bank(0); + + let old = Pubkey::new_unique(); + let old_bytes = vec![0; 44]; // Larger than 32 bytes + let old_lamports = bank.get_minimum_balance_for_rent_exemption(old_bytes.len()); + let (old_data, _) = Pubkey::find_program_address(&[old.as_ref()], &bpf_id); + set_up_account_with_bank(&mut bank, &old, old_lamports, old_bytes); + + let new = Pubkey::new_unique(); + let (new_data, _) = Pubkey::find_program_address(&[new.as_ref()], &bpf_id); + let new_bytes = new_data.to_bytes().to_vec(); + let new_lamports = bank.get_minimum_balance_for_rent_exemption(new_bytes.len()); + let new_data_bytes = vec![6; 30]; + let new_data_lamports = bank.get_minimum_balance_for_rent_exemption(new_data_bytes.len()); + set_up_account_with_bank(&mut bank, &new, new_lamports, new_bytes); + set_up_account_with_bank( + &mut bank, + &new_data, + new_data_lamports, + new_data_bytes.clone(), + ); + + let original_capitalization = bank.capitalization(); + + bank.replace_program_account(&old, &new, "bank-apply_program_replacement"); + + // Old program account's balance was _not_ adjusted to pay for minimum rent + // for the PDA + // + // Note: In this test, the original lamports were _greater_ than required + // rent for 32 bytes, so _no_ lamports were added + assert_eq!(bank.get_balance(&old), old_lamports); + + // Old data account's balance is now the new data account's balance + // (newly created) + assert_eq!(bank.get_balance(&old_data), new_data_lamports); + + // New program accounts are now empty + assert_eq!(bank.get_balance(&new), 0); + assert_eq!(bank.get_balance(&new_data), 0); + + // Old program account now holds the PDA, ie: + // - Old: PDA(OldData) + let old_account = bank.get_account(&old).unwrap(); + assert_eq!(old_account.data(), &old_data.to_bytes().to_vec()); + + // Old data account has been created & now holds the new data, ie: + // - OldData: [*New program data] + let old_data_account = bank.get_account(&old_data).unwrap(); + assert_eq!(old_data_account.data(), &new_data_bytes); + + // Lamports from the new program account were burnt + assert_eq!( + bank.capitalization(), + original_capitalization - new_lamports + ); } -// FIXME: Use rent-real lamport values #[test] fn test_program_replace_non_upgradable_does_not_exist() { // Non-Upgradable program @@ -8132,9 +8223,12 @@ fn test_program_replace_non_upgradable_does_not_exist() { let new = Pubkey::new_unique(); let (new_data, _) = Pubkey::find_program_address(&[new.as_ref()], &bpf_id); - let new_program_bytes = vec![6, 5, 4, 3, 2, 1, 0]; - set_up_account_with_bank(&mut bank, &new, 200, new_data.to_bytes().to_vec()); - set_up_account_with_bank(&mut bank, &new_data, 204, new_program_bytes); + let new_bytes = new_data.to_bytes().to_vec(); + let new_lamports = bank.get_minimum_balance_for_rent_exemption(new_bytes.len()); + let new_data_bytes = vec![6; 30]; + let new_data_lamports = bank.get_minimum_balance_for_rent_exemption(new_data_bytes.len()); + set_up_account_with_bank(&mut bank, &new, new_lamports, new_bytes); + set_up_account_with_bank(&mut bank, &new_data, new_data_lamports, new_data_bytes); let original_capitalization = bank.capitalization(); @@ -8145,8 +8239,8 @@ fn test_program_replace_non_upgradable_does_not_exist() { assert_eq!(bank.get_balance(&old_data), 0); // New program accounts are unchanged - assert_eq!(bank.get_balance(&new), 200); - assert_eq!(bank.get_balance(&new_data), 204); + assert_eq!(bank.get_balance(&new), new_lamports); + assert_eq!(bank.get_balance(&new_data), new_data_lamports); // Lamports were unchanged across the board assert_eq!(bank.capitalization(), original_capitalization); @@ -8173,9 +8267,12 @@ fn test_program_replace_non_upgradable_is_native_account() { let new = Pubkey::new_unique(); let (new_data, _) = Pubkey::find_program_address(&[new.as_ref()], &bpf_id); - let new_program_bytes = vec![6, 5, 4, 3, 2, 1, 0]; - set_up_account_with_bank(&mut bank, &new, 200, new_data.to_bytes().to_vec()); - set_up_account_with_bank(&mut bank, &new_data, 204, new_program_bytes.clone()); + let new_bytes = new_data.to_bytes().to_vec(); + let new_lamports = bank.get_minimum_balance_for_rent_exemption(new_bytes.len()); + let new_data_bytes = vec![6; 30]; + let new_data_lamports = bank.get_minimum_balance_for_rent_exemption(new_data_bytes.len()); + set_up_account_with_bank(&mut bank, &new, new_lamports, new_bytes); + set_up_account_with_bank(&mut bank, &new_data, new_data_lamports, new_data_bytes); let original_capitalization = bank.capitalization(); @@ -8191,7 +8288,7 @@ fn test_program_replace_non_upgradable_is_native_account() { // Old data account's balance is now the new data account's balance // (newly created) - assert_eq!(bank.get_balance(&old_data), 204); + assert_eq!(bank.get_balance(&old_data), new_data_lamports); // New program accounts are now empty assert_eq!(bank.get_balance(&new), 0); @@ -8205,10 +8302,13 @@ fn test_program_replace_non_upgradable_is_native_account() { // Old data account has been created & now holds the new data, ie: // - OldData: [*New program data] let old_data_account = bank.get_account(&old_data).unwrap(); - assert_eq!(old_data_account.data(), &new_program_bytes); + assert_eq!(old_data_account.data(), &new_data_bytes); - // Lamports in the old program account were burnt - assert_eq!(bank.capitalization(), original_capitalization - 200); + // Lamports from the new program account were burnt + assert_eq!( + bank.capitalization(), + original_capitalization - new_lamports + ); } // FIXME: Use rent-real lamport values @@ -8228,27 +8328,39 @@ fn test_program_replace_upgradable_base_case() { let old = Pubkey::new_unique(); let (old_data, _) = Pubkey::find_program_address(&[old.as_ref()], &bpf_id); - set_up_account_with_bank(&mut bank, &old, 100, old_data.to_bytes().to_vec()); - set_up_account_with_bank(&mut bank, &old_data, 102, vec![0, 1, 2, 3, 4, 5, 6]); + let old_bytes = old_data.to_bytes().to_vec(); + let old_lamports = bank.get_minimum_balance_for_rent_exemption(old_bytes.len()); + let old_data_bytes = vec![4; 10]; + let old_data_lamports = bank.get_minimum_balance_for_rent_exemption(old_data_bytes.len()); + set_up_account_with_bank(&mut bank, &old, old_lamports, old_bytes); + set_up_account_with_bank(&mut bank, &old_data, old_data_lamports, old_data_bytes); let new = Pubkey::new_unique(); let (new_data, _) = Pubkey::find_program_address(&[new.as_ref()], &bpf_id); - let new_program_bytes = vec![6, 5, 4, 3, 2, 1, 0]; - set_up_account_with_bank(&mut bank, &new, 200, new_data.to_bytes().to_vec()); - set_up_account_with_bank(&mut bank, &new_data, 204, new_program_bytes.clone()); + let new_bytes = new_data.to_bytes().to_vec(); + let new_lamports = bank.get_minimum_balance_for_rent_exemption(new_bytes.len()); + let new_data_bytes = vec![6; 30]; + let new_data_lamports = bank.get_minimum_balance_for_rent_exemption(new_data_bytes.len()); + set_up_account_with_bank(&mut bank, &new, new_lamports, new_bytes); + set_up_account_with_bank( + &mut bank, + &new_data, + new_data_lamports, + new_data_bytes.clone(), + ); let original_capitalization = bank.capitalization(); bank.replace_program_account(&old, &new, "bank-apply_program_replacement"); - // FIXME: We can't assume the size is unchanged + // Old program account's balance was _not_ adjusted to pay for minimum rent + // for the PDA // - // Old program account's balance was unchanged - // (Data was maybe modified, but size was unchanged) - assert_eq!(bank.get_balance(&old), 100); + // Note: In this test, the original bytes were unchanged (PDA) + assert_eq!(bank.get_balance(&old), old_lamports); // Old data account's balance is now the new data account's balance - assert_eq!(bank.get_balance(&old_data), 204); + assert_eq!(bank.get_balance(&old_data), new_data_lamports); // New program accounts are now empty assert_eq!(bank.get_balance(&new), 0); @@ -8262,17 +8374,21 @@ fn test_program_replace_upgradable_base_case() { // Old data account now holds the new data, ie: // - OldData: [*New program data] let old_data_account = bank.get_account(&old_data).unwrap(); - assert_eq!(old_data_account.data(), &new_program_bytes); + assert_eq!(old_data_account.data(), &new_data_bytes); - // Lamports in the old program accounts were burnt - assert_eq!(bank.capitalization(), original_capitalization - 102 - 200); + // Lamports from the new program account were burnt + // Lamports from the old data account were burnt + assert_eq!( + bank.capitalization(), + original_capitalization - new_lamports - old_data_lamports + ); } // FIXME: Use rent-real lamport values #[test] fn test_program_replace_upgradable_not_data_pda_at_first() { // Upgradable program - // - Old: [*Old arbitrary data] + // - Old: [*Old arbitrary data] (smaller than minimum rent-exempt size) // - OldData: [Old program data] // - New: PDA(NewData) // - NewData: [*New program data] @@ -8285,27 +8401,43 @@ fn test_program_replace_upgradable_not_data_pda_at_first() { let old = Pubkey::new_unique(); let (old_data, _) = Pubkey::find_program_address(&[old.as_ref()], &bpf_id); - set_up_account_with_bank(&mut bank, &old, 100, old_data.to_bytes().to_vec()); - set_up_account_with_bank(&mut bank, &old_data, 102, vec![0, 1, 2, 3, 4, 5, 6]); + let old_bytes = vec![1; 18]; // Smaller than 32 bytes + let old_lamports = bank.get_minimum_balance_for_rent_exemption(old_bytes.len()); + let old_data_bytes = vec![4; 10]; + let old_data_lamports = bank.get_minimum_balance_for_rent_exemption(old_data_bytes.len()); + set_up_account_with_bank(&mut bank, &old, old_lamports, old_bytes); + set_up_account_with_bank(&mut bank, &old_data, old_data_lamports, old_data_bytes); let new = Pubkey::new_unique(); let (new_data, _) = Pubkey::find_program_address(&[new.as_ref()], &bpf_id); - let new_program_bytes = vec![6, 5, 4, 3, 2, 1, 0]; - set_up_account_with_bank(&mut bank, &new, 200, new_data.to_bytes().to_vec()); - set_up_account_with_bank(&mut bank, &new_data, 204, new_program_bytes.clone()); + let new_bytes = new_data.to_bytes().to_vec(); + let new_lamports = bank.get_minimum_balance_for_rent_exemption(new_bytes.len()); + let new_data_bytes = vec![6; 30]; + let new_data_lamports = bank.get_minimum_balance_for_rent_exemption(new_data_bytes.len()); + set_up_account_with_bank(&mut bank, &new, new_lamports, new_bytes); + set_up_account_with_bank( + &mut bank, + &new_data, + new_data_lamports, + new_data_bytes.clone(), + ); let original_capitalization = bank.capitalization(); bank.replace_program_account(&old, &new, "bank-apply_program_replacement"); - // FIXME: We can't assume the size is unchanged + // Old program account's balance was adjusted to pay for minimum rent for + // the PDA (32 bytes) // - // Old program account's balance was unchanged - // (Data was maybe modified, but size was unchanged) - assert_eq!(bank.get_balance(&old), 100); + // Note: In this test, the original lamports were _less_ than required + // rent for 32 bytes, so lamports were added + assert_eq!( + bank.get_balance(&old), + bank.get_minimum_balance_for_rent_exemption(old_data.to_bytes().len()) + ); // Old data account's balance is now the new data account's balance - assert_eq!(bank.get_balance(&old_data), 204); + assert_eq!(bank.get_balance(&old_data), new_data_lamports); // New program accounts are now empty assert_eq!(bank.get_balance(&new), 0); @@ -8319,10 +8451,18 @@ fn test_program_replace_upgradable_not_data_pda_at_first() { // Old data account now holds the new data, ie: // - OldData: [*New program data] let old_data_account = bank.get_account(&old_data).unwrap(); - assert_eq!(old_data_account.data(), &new_program_bytes); + assert_eq!(old_data_account.data(), &new_data_bytes); - // Lamports in the old program accounts were burnt - assert_eq!(bank.capitalization(), original_capitalization - 102 - 200); + // The remaining lamports from both program accounts minus the rent-exempt + // minimum were burnt + // Lamports from the old data account were burnt + assert_eq!( + bank.capitalization(), + original_capitalization + - (new_lamports + old_lamports + - bank.get_minimum_balance_for_rent_exemption(old_data.to_bytes().len())) + - old_data_lamports + ); } // FIXME: Use rent-real lamport values @@ -8341,35 +8481,44 @@ fn test_program_replace_upgradable_no_data_provided_with_replacement() { let old = Pubkey::new_unique(); let (old_data, _) = Pubkey::find_program_address(&[old.as_ref()], &bpf_id); - let old_program_bytes = vec![0, 1, 2, 3, 4, 5, 6]; - set_up_account_with_bank(&mut bank, &old, 100, old_data.to_bytes().to_vec()); - set_up_account_with_bank(&mut bank, &old_data, 102, old_program_bytes.clone()); + let old_bytes = vec![5; 5]; + let old_lamports = bank.get_minimum_balance_for_rent_exemption(old_bytes.len()); + let old_data_bytes = vec![4; 10]; + let old_data_lamports = bank.get_minimum_balance_for_rent_exemption(old_data_bytes.len()); + set_up_account_with_bank(&mut bank, &old, old_lamports, old_bytes.clone()); + set_up_account_with_bank( + &mut bank, + &old_data, + old_data_lamports, + old_data_bytes.clone(), + ); let new = Pubkey::new_unique(); - let new_program_bytes = vec![6, 5, 4, 3, 2, 1, 0]; - set_up_account_with_bank(&mut bank, &new, 200, new_program_bytes.clone()); + let new_bytes = vec![2; 12]; + let new_lamports = bank.get_minimum_balance_for_rent_exemption(new_bytes.len()); + set_up_account_with_bank(&mut bank, &new, new_lamports, new_bytes.clone()); let original_capitalization = bank.capitalization(); bank.replace_program_account(&old, &new, "bank-apply_program_replacement"); // All balances are unchanged - assert_eq!(bank.get_balance(&old), 100); - assert_eq!(bank.get_balance(&old_data), 102); - assert_eq!(bank.get_balance(&new), 200); + assert_eq!(bank.get_balance(&old), old_lamports); + assert_eq!(bank.get_balance(&old_data), old_data_lamports); + assert_eq!(bank.get_balance(&new), new_lamports); // Old program accounts' data are unchanged // - Old: PDA(OldData) // - OldData: [Old program data] let old_account = bank.get_account(&old).unwrap(); - assert_eq!(old_account.data(), &old_data.to_bytes().to_vec()); + assert_eq!(old_account.data(), &old_bytes); let old_data_account = bank.get_account(&old_data).unwrap(); - assert_eq!(old_data_account.data(), &old_program_bytes); + assert_eq!(old_data_account.data(), &old_data_bytes); // New program account is unchanged // - New: [*New program data] let new_account = bank.get_account(&new).unwrap(); - assert_eq!(new_account.data(), &new_program_bytes); + assert_eq!(new_account.data(), &new_bytes); // Lamports were unchanged across the board assert_eq!(bank.capitalization(), original_capitalization); From 86833f9f5f32e5530b4518d275a4bdb925f0f67a Mon Sep 17 00:00:00 2001 From: Joe Date: Thu, 17 Aug 2023 23:07:04 -0600 Subject: [PATCH 06/33] mix in owner & executable --- runtime/src/bank.rs | 2 +- runtime/src/bank/tests.rs | 329 +++++++++++++++++++++++++++++--------- 2 files changed, 252 insertions(+), 79 deletions(-) diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index f1543467fdea17..5b5d4a778bc867 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -8266,7 +8266,7 @@ impl Bank { let account = Account { lamports, data, - ..Account::from(old_account.clone()) + ..Account::from(new_account.clone()) }; self.store_account(old_address, &account); new_account.lamports() + old_account.lamports() - account.lamports diff --git a/runtime/src/bank/tests.rs b/runtime/src/bank/tests.rs index f81bac1cf04ddc..09afb37b2c0038 100644 --- a/runtime/src/bank/tests.rs +++ b/runtime/src/bank/tests.rs @@ -8003,15 +8003,19 @@ fn test_compute_active_feature_set() { assert!(feature_set.is_active(&test_feature)); } -fn set_up_account_with_bank( +fn test_program_replace_set_up_account( bank: &mut Bank, pubkey: &Pubkey, lamports: u64, data: Vec, + owner: &Pubkey, + executable: bool, ) -> AccountSharedData { let new_account = AccountSharedData::from(Account { lamports, data, + owner: *owner, + executable, ..Account::default() }); bank.store_account_and_update_capitalization(pubkey, &new_account); @@ -8027,17 +8031,25 @@ fn test_program_replace_non_upgradable_base_case() { // // Should replace the old program account with the new program account: // - Old: [*New program data] + let bpf_id = bpf_loader::id(); let mut bank = create_simple_test_bank(0); let old = Pubkey::new_unique(); let old_bytes = vec![0, 0, 0, 0]; let old_lamports = bank.get_minimum_balance_for_rent_exemption(old_bytes.len()); - set_up_account_with_bank(&mut bank, &old, old_lamports, old_bytes); + test_program_replace_set_up_account(&mut bank, &old, old_lamports, old_bytes, &bpf_id, true); let new = Pubkey::new_unique(); let new_bytes = vec![6; 30]; let new_lamports = bank.get_minimum_balance_for_rent_exemption(new_bytes.len()); - set_up_account_with_bank(&mut bank, &new, new_lamports, new_bytes.clone()); + test_program_replace_set_up_account( + &mut bank, + &new, + new_lamports, + new_bytes.clone(), + &bpf_id, + true, + ); let original_capitalization = bank.capitalization(); @@ -8054,6 +8066,10 @@ fn test_program_replace_non_upgradable_base_case() { let old_account = bank.get_account(&old).unwrap(); assert_eq!(old_account.data(), &new_bytes); + // Ownership & executable match the new program account + assert_eq!(old_account.owner(), &bpf_id); + assert!(old_account.executable()); + // Lamports from the new program account were burnt assert_eq!( bank.capitalization(), @@ -8072,27 +8088,37 @@ fn test_program_replace_non_upgradable_create_data_add_rent() { // and create the data account: // - Old: PDA(OldData) // - OldData: [*New program data] - let bpf_id = bpf_loader_upgradeable::id(); + let bpf_id = bpf_loader::id(); + let bpf_upgradable_id = bpf_loader_upgradeable::id(); let mut bank = create_simple_test_bank(0); let old = Pubkey::new_unique(); let old_bytes = vec![0, 0, 0, 0]; // Smaller than 32 bytes let old_lamports = bank.get_minimum_balance_for_rent_exemption(old_bytes.len()); - let (old_data, _) = Pubkey::find_program_address(&[old.as_ref()], &bpf_id); - set_up_account_with_bank(&mut bank, &old, old_lamports, old_bytes); + let (old_data, _) = Pubkey::find_program_address(&[old.as_ref()], &bpf_upgradable_id); + test_program_replace_set_up_account(&mut bank, &old, old_lamports, old_bytes, &bpf_id, true); let new = Pubkey::new_unique(); - let (new_data, _) = Pubkey::find_program_address(&[new.as_ref()], &bpf_id); + let (new_data, _) = Pubkey::find_program_address(&[new.as_ref()], &bpf_upgradable_id); let new_bytes = new_data.to_bytes().to_vec(); let new_lamports = bank.get_minimum_balance_for_rent_exemption(new_bytes.len()); let new_data_bytes = vec![6; 30]; let new_data_lamports = bank.get_minimum_balance_for_rent_exemption(new_data_bytes.len()); - set_up_account_with_bank(&mut bank, &new, new_lamports, new_bytes); - set_up_account_with_bank( + test_program_replace_set_up_account( + &mut bank, + &new, + new_lamports, + new_bytes, + &bpf_upgradable_id, + true, + ); + test_program_replace_set_up_account( &mut bank, &new_data, new_data_lamports, new_data_bytes.clone(), + &bpf_upgradable_id, + false, ); let original_capitalization = bank.capitalization(); @@ -8127,6 +8153,12 @@ fn test_program_replace_non_upgradable_create_data_add_rent() { let old_data_account = bank.get_account(&old_data).unwrap(); assert_eq!(old_data_account.data(), &new_data_bytes); + // Ownership & executable match the new program accounts + assert_eq!(old_account.owner(), &bpf_upgradable_id); + assert!(old_account.executable()); + assert_eq!(old_data_account.owner(), &bpf_upgradable_id); + assert!(!old_data_account.executable()); + // The remaining lamports from both program accounts minus the rent-exempt // minimum were burnt assert_eq!( @@ -8148,27 +8180,37 @@ fn test_program_replace_non_upgradable_create_data_dont_add_rent() { // and create the data account: // - Old: PDA(OldData) // - OldData: [*New program data] - let bpf_id = bpf_loader_upgradeable::id(); + let bpf_id = bpf_loader::id(); + let bpf_upgradable_id = bpf_loader_upgradeable::id(); let mut bank = create_simple_test_bank(0); let old = Pubkey::new_unique(); let old_bytes = vec![0; 44]; // Larger than 32 bytes let old_lamports = bank.get_minimum_balance_for_rent_exemption(old_bytes.len()); - let (old_data, _) = Pubkey::find_program_address(&[old.as_ref()], &bpf_id); - set_up_account_with_bank(&mut bank, &old, old_lamports, old_bytes); + let (old_data, _) = Pubkey::find_program_address(&[old.as_ref()], &bpf_upgradable_id); + test_program_replace_set_up_account(&mut bank, &old, old_lamports, old_bytes, &bpf_id, true); let new = Pubkey::new_unique(); - let (new_data, _) = Pubkey::find_program_address(&[new.as_ref()], &bpf_id); + let (new_data, _) = Pubkey::find_program_address(&[new.as_ref()], &bpf_upgradable_id); let new_bytes = new_data.to_bytes().to_vec(); let new_lamports = bank.get_minimum_balance_for_rent_exemption(new_bytes.len()); let new_data_bytes = vec![6; 30]; let new_data_lamports = bank.get_minimum_balance_for_rent_exemption(new_data_bytes.len()); - set_up_account_with_bank(&mut bank, &new, new_lamports, new_bytes); - set_up_account_with_bank( + test_program_replace_set_up_account( + &mut bank, + &new, + new_lamports, + new_bytes, + &bpf_upgradable_id, + true, + ); + test_program_replace_set_up_account( &mut bank, &new_data, new_data_lamports, new_data_bytes.clone(), + &bpf_upgradable_id, + false, ); let original_capitalization = bank.capitalization(); @@ -8200,6 +8242,12 @@ fn test_program_replace_non_upgradable_create_data_dont_add_rent() { let old_data_account = bank.get_account(&old_data).unwrap(); assert_eq!(old_data_account.data(), &new_data_bytes); + // Ownership & executable match the new program accounts + assert_eq!(old_account.owner(), &bpf_upgradable_id); + assert!(old_account.executable()); + assert_eq!(old_data_account.owner(), &bpf_upgradable_id); + assert!(!old_data_account.executable()); + // Lamports from the new program account were burnt assert_eq!( bank.capitalization(), @@ -8207,36 +8255,88 @@ fn test_program_replace_non_upgradable_create_data_dont_add_rent() { ); } +// FIX ME: This case SHOULD BE allowed #[test] fn test_program_replace_non_upgradable_does_not_exist() { + // Non-Upgradable program + // - Old: ** Does not exist! ** + // - New: [*Arbitrary data] + // + // This is NOT allowed and should leave everything unchanged + let bpf_id = bpf_loader::id(); + let bpf_upgradable_id = bpf_loader_upgradeable::id(); + let mut bank = create_simple_test_bank(0); + + let old = Pubkey::new_unique(); + let (old_data, _) = Pubkey::find_program_address(&[old.as_ref()], &bpf_upgradable_id); + + let new = Pubkey::new_unique(); + let (new_data, _) = Pubkey::find_program_address(&[new.as_ref()], &bpf_upgradable_id); + let new_bytes = new_data.to_bytes().to_vec(); + let new_lamports = bank.get_minimum_balance_for_rent_exemption(new_bytes.len()); + test_program_replace_set_up_account(&mut bank, &new, new_lamports, new_bytes, &bpf_id, true); + + let original_capitalization = bank.capitalization(); + + bank.replace_program_account(&old, &new, "bank-apply_program_replacement"); + + // Old program accounts still don't exist + assert_eq!(bank.get_account(&old), None); + assert_eq!(bank.get_account(&old_data), None); + + // New program accounts are unchanged + assert_eq!(bank.get_balance(&new), new_lamports); + assert_eq!(bank.get_account(&new_data), None); + + // Lamports were unchanged across the board + assert_eq!(bank.capitalization(), original_capitalization); +} + +// FIX ME: This case SHOULD BE allowed +#[test] +fn test_program_replace_non_upgradable_does_not_exist_create_data() { // Non-Upgradable program // - Old: ** Does not exist! ** // - New: PDA(NewData) // - NewData: [*New program data] // // This is NOT allowed and should leave everything unchanged - let bpf_id = bpf_loader_upgradeable::id(); + let bpf_upgradable_id = bpf_loader_upgradeable::id(); let mut bank = create_simple_test_bank(0); let old = Pubkey::new_unique(); - let (old_data, _) = Pubkey::find_program_address(&[old.as_ref()], &bpf_id); + let (old_data, _) = Pubkey::find_program_address(&[old.as_ref()], &bpf_upgradable_id); let new = Pubkey::new_unique(); - let (new_data, _) = Pubkey::find_program_address(&[new.as_ref()], &bpf_id); + let (new_data, _) = Pubkey::find_program_address(&[new.as_ref()], &bpf_upgradable_id); let new_bytes = new_data.to_bytes().to_vec(); let new_lamports = bank.get_minimum_balance_for_rent_exemption(new_bytes.len()); let new_data_bytes = vec![6; 30]; let new_data_lamports = bank.get_minimum_balance_for_rent_exemption(new_data_bytes.len()); - set_up_account_with_bank(&mut bank, &new, new_lamports, new_bytes); - set_up_account_with_bank(&mut bank, &new_data, new_data_lamports, new_data_bytes); + test_program_replace_set_up_account( + &mut bank, + &new, + new_lamports, + new_bytes, + &bpf_upgradable_id, + true, + ); + test_program_replace_set_up_account( + &mut bank, + &new_data, + new_data_lamports, + new_data_bytes, + &bpf_upgradable_id, + false, + ); let original_capitalization = bank.capitalization(); bank.replace_program_account(&old, &new, "bank-apply_program_replacement"); // Old program accounts still don't exist - assert_eq!(bank.get_balance(&old), 0); - assert_eq!(bank.get_balance(&old_data), 0); + assert_eq!(bank.get_account(&old), None); + assert_eq!(bank.get_account(&old_data), None); // New program accounts are unchanged assert_eq!(bank.get_balance(&new), new_lamports); @@ -8246,8 +8346,7 @@ fn test_program_replace_non_upgradable_does_not_exist() { assert_eq!(bank.capitalization(), original_capitalization); } -// FIXME: Use rent-real lamport values -#[cfg(ignore)] +// FIX ME: This case SHOULD BE allowed #[test] fn test_program_replace_non_upgradable_is_native_account() { // Non-Upgradable program @@ -8259,56 +8358,49 @@ fn test_program_replace_non_upgradable_is_native_account() { // and create the data account: // - Old: PDA(OldData) // - OldData: [*New program data] - let bpf_id = bpf_loader_upgradeable::id(); + let bpf_upgradable_id = bpf_loader_upgradeable::id(); let mut bank = create_simple_test_bank(0); - let old = feature::id(); - let (old_data, _) = Pubkey::find_program_address(&[old.as_ref()], &bpf_id); + let old = feature::id(); // `Feature11111111` + let (old_data, _) = Pubkey::find_program_address(&[old.as_ref()], &bpf_upgradable_id); let new = Pubkey::new_unique(); - let (new_data, _) = Pubkey::find_program_address(&[new.as_ref()], &bpf_id); + let (new_data, _) = Pubkey::find_program_address(&[new.as_ref()], &bpf_upgradable_id); let new_bytes = new_data.to_bytes().to_vec(); let new_lamports = bank.get_minimum_balance_for_rent_exemption(new_bytes.len()); let new_data_bytes = vec![6; 30]; let new_data_lamports = bank.get_minimum_balance_for_rent_exemption(new_data_bytes.len()); - set_up_account_with_bank(&mut bank, &new, new_lamports, new_bytes); - set_up_account_with_bank(&mut bank, &new_data, new_data_lamports, new_data_bytes); + test_program_replace_set_up_account( + &mut bank, + &new, + new_lamports, + new_bytes, + &bpf_upgradable_id, + true, + ); + test_program_replace_set_up_account( + &mut bank, + &new_data, + new_data_lamports, + new_data_bytes, + &bpf_upgradable_id, + false, + ); let original_capitalization = bank.capitalization(); bank.replace_program_account(&old, &new, "bank-apply_program_replacement"); - // TODO - - // FIXME: We can't assume the size is unchanged - // - // Old program account's balance was unchanged - // (Data was maybe modified, but size was unchanged) - assert_eq!(bank.get_balance(&old), 0); - - // Old data account's balance is now the new data account's balance - // (newly created) - assert_eq!(bank.get_balance(&old_data), new_data_lamports); - - // New program accounts are now empty - assert_eq!(bank.get_balance(&new), 0); - assert_eq!(bank.get_balance(&new_data), 0); - - // Old program account now holds the PDA, ie: - // - Old: PDA(OldData) - let old_account = bank.get_account(&old).unwrap(); - assert_eq!(old_account.data(), &old_data.to_bytes().to_vec()); + // Old program accounts still don't exist + assert_eq!(bank.get_account(&old), None); + assert_eq!(bank.get_account(&old_data), None); - // Old data account has been created & now holds the new data, ie: - // - OldData: [*New program data] - let old_data_account = bank.get_account(&old_data).unwrap(); - assert_eq!(old_data_account.data(), &new_data_bytes); + // New program accounts are unchanged + assert_eq!(bank.get_balance(&new), new_lamports); + assert_eq!(bank.get_balance(&new_data), new_data_lamports); - // Lamports from the new program account were burnt - assert_eq!( - bank.capitalization(), - original_capitalization - new_lamports - ); + // Lamports were unchanged across the board + assert_eq!(bank.capitalization(), original_capitalization); } // FIXME: Use rent-real lamport values @@ -8323,30 +8415,53 @@ fn test_program_replace_upgradable_base_case() { // Should _only_ replace the data account, not the program account: // - Old: PDA(OldData) // - OldData: [*New program data] - let bpf_id = bpf_loader_upgradeable::id(); + let bpf_upgradable_id = bpf_loader_upgradeable::id(); let mut bank = create_simple_test_bank(0); let old = Pubkey::new_unique(); - let (old_data, _) = Pubkey::find_program_address(&[old.as_ref()], &bpf_id); + let (old_data, _) = Pubkey::find_program_address(&[old.as_ref()], &bpf_upgradable_id); let old_bytes = old_data.to_bytes().to_vec(); let old_lamports = bank.get_minimum_balance_for_rent_exemption(old_bytes.len()); let old_data_bytes = vec![4; 10]; let old_data_lamports = bank.get_minimum_balance_for_rent_exemption(old_data_bytes.len()); - set_up_account_with_bank(&mut bank, &old, old_lamports, old_bytes); - set_up_account_with_bank(&mut bank, &old_data, old_data_lamports, old_data_bytes); + test_program_replace_set_up_account( + &mut bank, + &old, + old_lamports, + old_bytes, + &bpf_upgradable_id, + true, + ); + test_program_replace_set_up_account( + &mut bank, + &old_data, + old_data_lamports, + old_data_bytes, + &bpf_upgradable_id, + false, + ); let new = Pubkey::new_unique(); - let (new_data, _) = Pubkey::find_program_address(&[new.as_ref()], &bpf_id); + let (new_data, _) = Pubkey::find_program_address(&[new.as_ref()], &bpf_upgradable_id); let new_bytes = new_data.to_bytes().to_vec(); let new_lamports = bank.get_minimum_balance_for_rent_exemption(new_bytes.len()); let new_data_bytes = vec![6; 30]; let new_data_lamports = bank.get_minimum_balance_for_rent_exemption(new_data_bytes.len()); - set_up_account_with_bank(&mut bank, &new, new_lamports, new_bytes); - set_up_account_with_bank( + test_program_replace_set_up_account( + &mut bank, + &new, + new_lamports, + new_bytes, + &bpf_upgradable_id, + true, + ); + test_program_replace_set_up_account( &mut bank, &new_data, new_data_lamports, new_data_bytes.clone(), + &bpf_upgradable_id, + false, ); let original_capitalization = bank.capitalization(); @@ -8376,6 +8491,12 @@ fn test_program_replace_upgradable_base_case() { let old_data_account = bank.get_account(&old_data).unwrap(); assert_eq!(old_data_account.data(), &new_data_bytes); + // Ownership & executable match the new program accounts + assert_eq!(old_account.owner(), &bpf_upgradable_id); + assert!(old_account.executable()); + assert_eq!(old_data_account.owner(), &bpf_upgradable_id); + assert!(!old_data_account.executable()); + // Lamports from the new program account were burnt // Lamports from the old data account were burnt assert_eq!( @@ -8396,30 +8517,53 @@ fn test_program_replace_upgradable_not_data_pda_at_first() { // Should _only_ replace the data account, not the program account: // - Old: PDA(OldData) // - OldData: [*New program data] - let bpf_id = bpf_loader_upgradeable::id(); + let bpf_upgradable_id = bpf_loader_upgradeable::id(); let mut bank = create_simple_test_bank(0); let old = Pubkey::new_unique(); - let (old_data, _) = Pubkey::find_program_address(&[old.as_ref()], &bpf_id); + let (old_data, _) = Pubkey::find_program_address(&[old.as_ref()], &bpf_upgradable_id); let old_bytes = vec![1; 18]; // Smaller than 32 bytes let old_lamports = bank.get_minimum_balance_for_rent_exemption(old_bytes.len()); let old_data_bytes = vec![4; 10]; let old_data_lamports = bank.get_minimum_balance_for_rent_exemption(old_data_bytes.len()); - set_up_account_with_bank(&mut bank, &old, old_lamports, old_bytes); - set_up_account_with_bank(&mut bank, &old_data, old_data_lamports, old_data_bytes); + test_program_replace_set_up_account( + &mut bank, + &old, + old_lamports, + old_bytes, + &bpf_upgradable_id, + true, + ); + test_program_replace_set_up_account( + &mut bank, + &old_data, + old_data_lamports, + old_data_bytes, + &bpf_upgradable_id, + false, + ); let new = Pubkey::new_unique(); - let (new_data, _) = Pubkey::find_program_address(&[new.as_ref()], &bpf_id); + let (new_data, _) = Pubkey::find_program_address(&[new.as_ref()], &bpf_upgradable_id); let new_bytes = new_data.to_bytes().to_vec(); let new_lamports = bank.get_minimum_balance_for_rent_exemption(new_bytes.len()); let new_data_bytes = vec![6; 30]; let new_data_lamports = bank.get_minimum_balance_for_rent_exemption(new_data_bytes.len()); - set_up_account_with_bank(&mut bank, &new, new_lamports, new_bytes); - set_up_account_with_bank( + test_program_replace_set_up_account( + &mut bank, + &new, + new_lamports, + new_bytes, + &bpf_upgradable_id, + true, + ); + test_program_replace_set_up_account( &mut bank, &new_data, new_data_lamports, new_data_bytes.clone(), + &bpf_upgradable_id, + false, ); let original_capitalization = bank.capitalization(); @@ -8453,6 +8597,12 @@ fn test_program_replace_upgradable_not_data_pda_at_first() { let old_data_account = bank.get_account(&old_data).unwrap(); assert_eq!(old_data_account.data(), &new_data_bytes); + // Ownership & executable match the new program accounts + assert_eq!(old_account.owner(), &bpf_upgradable_id); + assert!(old_account.executable()); + assert_eq!(old_data_account.owner(), &bpf_upgradable_id); + assert!(!old_data_account.executable()); + // The remaining lamports from both program accounts minus the rent-exempt // minimum were burnt // Lamports from the old data account were burnt @@ -8476,27 +8626,44 @@ fn test_program_replace_upgradable_no_data_provided_with_replacement() { // This is NOT allowed and should leave the program unchanged: // - Old: PDA(OldData) // - OldData: [Old program data] - let bpf_id = bpf_loader_upgradeable::id(); + let bpf_id = bpf_loader::id(); + let bpf_upgradable_id = bpf_loader_upgradeable::id(); let mut bank = create_simple_test_bank(0); let old = Pubkey::new_unique(); - let (old_data, _) = Pubkey::find_program_address(&[old.as_ref()], &bpf_id); + let (old_data, _) = Pubkey::find_program_address(&[old.as_ref()], &bpf_upgradable_id); let old_bytes = vec![5; 5]; let old_lamports = bank.get_minimum_balance_for_rent_exemption(old_bytes.len()); let old_data_bytes = vec![4; 10]; let old_data_lamports = bank.get_minimum_balance_for_rent_exemption(old_data_bytes.len()); - set_up_account_with_bank(&mut bank, &old, old_lamports, old_bytes.clone()); - set_up_account_with_bank( + test_program_replace_set_up_account( + &mut bank, + &old, + old_lamports, + old_bytes.clone(), + &bpf_upgradable_id, + true, + ); + test_program_replace_set_up_account( &mut bank, &old_data, old_data_lamports, old_data_bytes.clone(), + &bpf_upgradable_id, + false, ); let new = Pubkey::new_unique(); let new_bytes = vec![2; 12]; let new_lamports = bank.get_minimum_balance_for_rent_exemption(new_bytes.len()); - set_up_account_with_bank(&mut bank, &new, new_lamports, new_bytes.clone()); + test_program_replace_set_up_account( + &mut bank, + &new, + new_lamports, + new_bytes.clone(), + &bpf_id, + true, + ); let original_capitalization = bank.capitalization(); @@ -8520,6 +8687,12 @@ fn test_program_replace_upgradable_no_data_provided_with_replacement() { let new_account = bank.get_account(&new).unwrap(); assert_eq!(new_account.data(), &new_bytes); + // Ownership & executable are unchanged + assert_eq!(old_account.owner(), &bpf_upgradable_id); + assert!(old_account.executable()); + assert_eq!(old_data_account.owner(), &bpf_upgradable_id); + assert!(!old_data_account.executable()); + // Lamports were unchanged across the board assert_eq!(bank.capitalization(), original_capitalization); } From 637dd83e3b2c3c4f7a1b20fd3a459f668aa50da1 Mon Sep 17 00:00:00 2001 From: Joe Date: Fri, 18 Aug 2023 02:09:11 -0600 Subject: [PATCH 07/33] feature-related cases --- runtime/src/bank.rs | 142 ++++++++-------- runtime/src/bank/tests.rs | 336 ++++++++++++++++++++++++++++++++------ 2 files changed, 363 insertions(+), 115 deletions(-) diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 5b5d4a778bc867..e1524bb936dfeb 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -8224,80 +8224,90 @@ impl Bank { new_address: &Pubkey, datapoint_name: &'static str, ) { - // Both program accounts must exist in order to attempt a replacement - if let Some(old_account) = self.get_account_with_fixed_root(old_address) { - if let Some(new_account) = self.get_account_with_fixed_root(new_address) { - // Both exist, so we can proceed with the replacement - datapoint_info!(datapoint_name, ("slot", self.slot, i64)); - - // Derive each program's data account address (PDA) - let (old_data_address, _) = Pubkey::find_program_address( - &[old_address.as_ref()], - &bpf_loader_upgradeable::id(), - ); - let (new_data_address, _) = Pubkey::find_program_address( - &[new_address.as_ref()], - &bpf_loader_upgradeable::id(), - ); + if let Some(new_account) = self.get_account_with_fixed_root(new_address) { + datapoint_info!(datapoint_name, ("slot", self.slot, i64)); - // If a data account is also provided with this new program - // account, then we want to update the existing data account - if let Some(new_data_account) = self.get_account_with_fixed_root(&new_data_address) - { - // If a data account exists for the old program, it will be - // replaced with the new data account - // If a data account does not exist for the old program, it - // will be created with the new data account - self.replace_account( - &old_data_address, - &new_data_address, - self.get_account_with_fixed_root(&old_data_address).as_ref(), - &new_data_account, - ); - // If necessary, update the old program account to house - // the PDA of the data account. - let data = old_data_address.as_ref().to_vec(); - let change_in_cap = if old_account.data() != data { - // Determine if we need to add any lamports for rent - // exemption - let lamports = old_account - .lamports() - .max(self.get_minimum_balance_for_rent_exemption(data.len())); - let account = Account { + let (old_data_address, _) = Pubkey::find_program_address( + &[old_address.as_ref()], + &bpf_loader_upgradeable::id(), + ); + let (new_data_address, _) = Pubkey::find_program_address( + &[new_address.as_ref()], + &bpf_loader_upgradeable::id(), + ); + + // If a data account is also provided with this new program + // account, then we want to update the existing data account + if let Some(new_data_account) = self.get_account_with_fixed_root(&new_data_address) { + let data = old_data_address.as_ref().to_vec(); + + let change_in_cap = + if let Some(old_account) = self.get_account_with_fixed_root(old_address) { + // If the old program account exists and the data is not + // the PDA of the data account, overwrite the data with the + // PDA and fund it with any lamports for rent if necessary + if old_account.data() != data { + let lamports = old_account + .lamports() + .max(self.get_minimum_balance_for_rent_exemption(data.len())); + let overwrite = Account { + lamports, + data, + ..Account::from(new_account.clone()) + }; + self.store_account(old_address, &overwrite); + new_account.lamports() + old_account.lamports() - lamports + } else { + new_account.lamports() + } + } else { + // If the old program account does not exist, create it + // with the PDA as its data and fund it with rent-exempt + // lamports + let lamports = self.get_minimum_balance_for_rent_exemption(data.len()); + let overwrite = Account { lamports, data, ..Account::from(new_account.clone()) }; - self.store_account(old_address, &account); - new_account.lamports() + old_account.lamports() - account.lamports - } else { - new_account.lamports() + self.store_account(old_address, &overwrite); + new_account.lamports() - lamports }; - // We only swapped the data accounts, so now we need to - // clear the new program account - self.store_account(new_address, &AccountSharedData::default()); - // Update capitalization - self.capitalization.fetch_sub(change_in_cap, Relaxed); - } else if self - .get_account_with_fixed_root(&old_data_address) - .is_none() - { - // A data account does not exist for the new program - // Swap program accounts only - self.replace_account( - old_address, - new_address, - Some(&old_account), - &new_account, - ); - } - // Unload a program from the bank's cache - self.loaded_programs_cache - .write() - .unwrap() - .remove_programs([*old_address].into_iter()); + // Replace the old data account with the new data account + // If the old data account does not exist, it will be created + self.replace_account( + &old_data_address, + &new_data_address, + self.get_account_with_fixed_root(&old_data_address).as_ref(), + &new_data_account, + ); + + // Clear the new program account + self.store_account(new_address, &AccountSharedData::default()); + + // Update capitalization + self.capitalization.fetch_sub(change_in_cap, Relaxed); + } else if self + .get_account_with_fixed_root(&old_data_address) + .is_none() + { + // A data account does not exist for the new program + // Replace the old program account with the new program account + // If the old program account doesn't exist, it will be created + self.replace_account( + old_address, + new_address, + self.get_account_with_fixed_root(old_address).as_ref(), + &new_account, + ); } + + // Unload a program from the bank's cache + self.loaded_programs_cache + .write() + .unwrap() + .remove_programs([*old_address].into_iter()); } } diff --git a/runtime/src/bank/tests.rs b/runtime/src/bank/tests.rs index 09afb37b2c0038..c2c79737138484 100644 --- a/runtime/src/bank/tests.rs +++ b/runtime/src/bank/tests.rs @@ -8161,11 +8161,11 @@ fn test_program_replace_non_upgradable_create_data_add_rent() { // The remaining lamports from both program accounts minus the rent-exempt // minimum were burnt + let burnt_after_rent = new_lamports + old_lamports + - bank.get_minimum_balance_for_rent_exemption(old_data.to_bytes().len()); assert_eq!( bank.capitalization(), - original_capitalization - - (new_lamports + old_lamports - - bank.get_minimum_balance_for_rent_exemption(old_data.to_bytes().len())) + original_capitalization - burnt_after_rent ); } @@ -8255,15 +8255,74 @@ fn test_program_replace_non_upgradable_create_data_dont_add_rent() { ); } -// FIX ME: This case SHOULD BE allowed #[test] -fn test_program_replace_non_upgradable_does_not_exist() { - // Non-Upgradable program +fn test_program_replace_does_not_exist() { + // Non-Existent program // - Old: ** Does not exist! ** // - New: [*Arbitrary data] // - // This is NOT allowed and should leave everything unchanged + // Should create the program account, ie: + // - Old: [*Arbitrary data] let bpf_id = bpf_loader::id(); + let mut bank = create_simple_test_bank(0); + + let old = Pubkey::new_unique(); + + let new = Pubkey::new_unique(); + let new_bytes = vec![6; 30]; + let new_lamports = bank.get_minimum_balance_for_rent_exemption(new_bytes.len()); + test_program_replace_set_up_account( + &mut bank, + &new, + new_lamports, + new_bytes.clone(), + &bpf_id, + true, + ); + + let original_capitalization = bank.capitalization(); + + bank.replace_program_account(&old, &new, "bank-apply_program_replacement"); + + // Old program account was created and funded to pay for minimum rent + // for the arbitrary data + assert_eq!( + bank.get_balance(&old), + bank.get_minimum_balance_for_rent_exemption(new_bytes.len()) + ); + + // New program account is now empty + assert_eq!(bank.get_balance(&new), 0); + + // Old program account holds new program account bytes, ie: + // - Old: [*Arbitrary data] + let old_account = bank.get_account(&old).unwrap(); + assert_eq!(old_account.data(), &new_bytes); + + // Ownership & executable match the new program account + assert_eq!(old_account.owner(), &bpf_id); + assert!(old_account.executable()); + + // The remaining lamports from both program accounts minus the rent-exempt + // minimum were burnt + let burnt_after_rent = + new_lamports - bank.get_minimum_balance_for_rent_exemption(new_bytes.len()); + assert_eq!( + bank.capitalization(), + original_capitalization - burnt_after_rent + ); +} + +#[test] +fn test_program_replace_does_not_exist_create_data() { + // Non-Existent program + // - Old: ** Does not exist! ** + // - New: PDA(NewData) + // - NewData: [*New program data] + // + // Should create the program account and the data account, ie: + // - Old: PDA(OldData) + // - OldData: [Old program data] let bpf_upgradable_id = bpf_loader_upgradeable::id(); let mut bank = create_simple_test_bank(0); @@ -8274,33 +8333,79 @@ fn test_program_replace_non_upgradable_does_not_exist() { let (new_data, _) = Pubkey::find_program_address(&[new.as_ref()], &bpf_upgradable_id); let new_bytes = new_data.to_bytes().to_vec(); let new_lamports = bank.get_minimum_balance_for_rent_exemption(new_bytes.len()); - test_program_replace_set_up_account(&mut bank, &new, new_lamports, new_bytes, &bpf_id, true); + let new_data_bytes = vec![6; 30]; + let new_data_lamports = bank.get_minimum_balance_for_rent_exemption(new_data_bytes.len()); + test_program_replace_set_up_account( + &mut bank, + &new, + new_lamports, + new_bytes, + &bpf_upgradable_id, + true, + ); + test_program_replace_set_up_account( + &mut bank, + &new_data, + new_data_lamports, + new_data_bytes.clone(), + &bpf_upgradable_id, + false, + ); let original_capitalization = bank.capitalization(); bank.replace_program_account(&old, &new, "bank-apply_program_replacement"); - // Old program accounts still don't exist - assert_eq!(bank.get_account(&old), None); - assert_eq!(bank.get_account(&old_data), None); + // Old program account was created and funded to pay for minimum rent + // for the PDA + assert_eq!( + bank.get_balance(&old), + bank.get_minimum_balance_for_rent_exemption(old_data.to_bytes().len()) + ); - // New program accounts are unchanged - assert_eq!(bank.get_balance(&new), new_lamports); - assert_eq!(bank.get_account(&new_data), None); + // Old data account was created, now holds the new data account's balance + assert_eq!(bank.get_balance(&old_data), new_data_lamports); - // Lamports were unchanged across the board - assert_eq!(bank.capitalization(), original_capitalization); + // New program accounts are now empty + assert_eq!(bank.get_balance(&new), 0); + assert_eq!(bank.get_balance(&new_data), 0); + + // Old program account holds the PDA, ie: + // - Old: PDA(OldData) + let old_account = bank.get_account(&old).unwrap(); + assert_eq!(old_account.data(), &old_data.to_bytes().to_vec()); + + // Old data account holds the new data, ie: + // - OldData: [*New program data] + let old_data_account = bank.get_account(&old_data).unwrap(); + assert_eq!(old_data_account.data(), &new_data_bytes); + + // Ownership & executable match the new program accounts + assert_eq!(old_account.owner(), &bpf_upgradable_id); + assert!(old_account.executable()); + assert_eq!(old_data_account.owner(), &bpf_upgradable_id); + assert!(!old_data_account.executable()); + + // The remaining lamports from both program accounts minus the rent-exempt + // minimum were burnt + let burnt_after_rent = + new_lamports - bank.get_minimum_balance_for_rent_exemption(old_data.to_bytes().len()); + assert_eq!( + bank.capitalization(), + original_capitalization - burnt_after_rent + ); } -// FIX ME: This case SHOULD BE allowed #[test] -fn test_program_replace_non_upgradable_does_not_exist_create_data() { - // Non-Upgradable program - // - Old: ** Does not exist! ** +fn test_program_replace_is_native_account() { + // Native program + // - Old: ** Native account (ie. `Feature11111111`) ** // - New: PDA(NewData) // - NewData: [*New program data] // - // This is NOT allowed and should leave everything unchanged + // Should create the program account and the data account, ie: + // - Old: PDA(OldData) + // - OldData: [Old program data] let bpf_upgradable_id = bpf_loader_upgradeable::id(); let mut bank = create_simple_test_bank(0); @@ -8325,7 +8430,7 @@ fn test_program_replace_non_upgradable_does_not_exist_create_data() { &mut bank, &new_data, new_data_lamports, - new_data_bytes, + new_data_bytes.clone(), &bpf_upgradable_id, false, ); @@ -8334,35 +8439,143 @@ fn test_program_replace_non_upgradable_does_not_exist_create_data() { bank.replace_program_account(&old, &new, "bank-apply_program_replacement"); - // Old program accounts still don't exist - assert_eq!(bank.get_account(&old), None); - assert_eq!(bank.get_account(&old_data), None); + // Old program account was created and funded to pay for minimum rent + // for the PDA + assert_eq!( + bank.get_balance(&old), + bank.get_minimum_balance_for_rent_exemption(old_data.to_bytes().len()) + ); + + // Old data account was created, now holds the new data account's balance + assert_eq!(bank.get_balance(&old_data), new_data_lamports); + + // New program accounts are now empty + assert_eq!(bank.get_balance(&new), 0); + assert_eq!(bank.get_balance(&new_data), 0); + + // Old program account holds the PDA, ie: + // - Old: PDA(OldData) + let old_account = bank.get_account(&old).unwrap(); + assert_eq!(old_account.data(), &old_data.to_bytes().to_vec()); + + // Old data account holds the new data, ie: + // - OldData: [*New program data] + let old_data_account = bank.get_account(&old_data).unwrap(); + assert_eq!(old_data_account.data(), &new_data_bytes); + + // Ownership & executable match the new program accounts + assert_eq!(old_account.owner(), &bpf_upgradable_id); + assert!(old_account.executable()); + assert_eq!(old_data_account.owner(), &bpf_upgradable_id); + assert!(!old_data_account.executable()); + + // The remaining lamports from both program accounts minus the rent-exempt + // minimum were burnt + let burnt_after_rent = + new_lamports - bank.get_minimum_balance_for_rent_exemption(old_data.to_bytes().len()); + assert_eq!( + bank.capitalization(), + original_capitalization - burnt_after_rent + ); +} + +#[test] +fn test_program_replace_does_not_exist_but_data_does() { + // Non-Upgradable program + // - Old: ** Does not exist! ** + // - OldData: [Old program data] + // - New: [*Arbitrary data] + // + // This is NOT allowed and should leave the program unchanged: + // - Old: ** Does not exist! ** + // - OldData: [Old program data] + let bpf_id = bpf_loader::id(); + let bpf_upgradable_id = bpf_loader_upgradeable::id(); + let mut bank = create_simple_test_bank(0); - // New program accounts are unchanged + let old = Pubkey::new_unique(); + let (old_data, _) = Pubkey::find_program_address(&[old.as_ref()], &bpf_upgradable_id); + let old_data_bytes = vec![4; 10]; + let old_data_lamports = bank.get_minimum_balance_for_rent_exemption(old_data_bytes.len()); + test_program_replace_set_up_account( + &mut bank, + &old_data, + old_data_lamports, + old_data_bytes.clone(), + &bpf_upgradable_id, + false, + ); + + let new = Pubkey::new_unique(); + let new_bytes = vec![2; 12]; + let new_lamports = bank.get_minimum_balance_for_rent_exemption(new_bytes.len()); + test_program_replace_set_up_account( + &mut bank, + &new, + new_lamports, + new_bytes.clone(), + &bpf_id, + true, + ); + + let original_capitalization = bank.capitalization(); + + bank.replace_program_account(&old, &new, "bank-apply_program_replacement"); + + // All balances are unchanged + assert_eq!(bank.get_balance(&old), 0); + assert_eq!(bank.get_balance(&old_data), old_data_lamports); assert_eq!(bank.get_balance(&new), new_lamports); - assert_eq!(bank.get_balance(&new_data), new_data_lamports); + + // Old program accounts still does not exist + assert_eq!(bank.get_account(&old), None); + + // Old data accounts' data is unchanged + // - OldData: [Old program data] + let old_data_account = bank.get_account(&old_data).unwrap(); + assert_eq!(old_data_account.data(), &old_data_bytes); + + // New program account is unchanged + // - New: [*Arbitrary data] + let new_account = bank.get_account(&new).unwrap(); + assert_eq!(new_account.data(), &new_bytes); + + // Ownership & executable are unchanged + assert_eq!(old_data_account.owner(), &bpf_upgradable_id); + assert!(!old_data_account.executable()); + assert_eq!(new_account.owner(), &bpf_id); + assert!(new_account.executable()); // Lamports were unchanged across the board assert_eq!(bank.capitalization(), original_capitalization); } -// FIX ME: This case SHOULD BE allowed #[test] -fn test_program_replace_non_upgradable_is_native_account() { +fn test_program_replace_does_not_exist_but_data_does_with_data() { // Non-Upgradable program - // - Old: ** Native account (ie. `Feature11111111`) ** + // - Old: ** Does not exist! ** + // - OldData: [Old program data] // - New: PDA(NewData) // - NewData: [*New program data] // - // Should replace the native account with the PDA of the data account, - // and create the data account: + // Should create the program account and replace the data account: // - Old: PDA(OldData) // - OldData: [*New program data] let bpf_upgradable_id = bpf_loader_upgradeable::id(); let mut bank = create_simple_test_bank(0); - let old = feature::id(); // `Feature11111111` + let old = Pubkey::new_unique(); let (old_data, _) = Pubkey::find_program_address(&[old.as_ref()], &bpf_upgradable_id); + let old_data_bytes = vec![4; 10]; + let old_data_lamports = bank.get_minimum_balance_for_rent_exemption(old_data_bytes.len()); + test_program_replace_set_up_account( + &mut bank, + &old_data, + old_data_lamports, + old_data_bytes, + &bpf_upgradable_id, + false, + ); let new = Pubkey::new_unique(); let (new_data, _) = Pubkey::find_program_address(&[new.as_ref()], &bpf_upgradable_id); @@ -8382,7 +8595,7 @@ fn test_program_replace_non_upgradable_is_native_account() { &mut bank, &new_data, new_data_lamports, - new_data_bytes, + new_data_bytes.clone(), &bpf_upgradable_id, false, ); @@ -8391,19 +8604,47 @@ fn test_program_replace_non_upgradable_is_native_account() { bank.replace_program_account(&old, &new, "bank-apply_program_replacement"); - // Old program accounts still don't exist - assert_eq!(bank.get_account(&old), None); - assert_eq!(bank.get_account(&old_data), None); + // Old program account was created and funded to pay for minimum rent + // for the PDA + assert_eq!( + bank.get_balance(&old), + bank.get_minimum_balance_for_rent_exemption(old_data.to_bytes().len()) + ); - // New program accounts are unchanged - assert_eq!(bank.get_balance(&new), new_lamports); - assert_eq!(bank.get_balance(&new_data), new_data_lamports); + // Old data account's balance is now the new data account's balance + assert_eq!(bank.get_balance(&old_data), new_data_lamports); - // Lamports were unchanged across the board - assert_eq!(bank.capitalization(), original_capitalization); + // New program accounts are now empty + assert_eq!(bank.get_balance(&new), 0); + assert_eq!(bank.get_balance(&new_data), 0); + + // Old program account holds the PDA, ie: + // - Old: PDA(OldData) + let old_account = bank.get_account(&old).unwrap(); + assert_eq!(old_account.data(), &old_data.to_bytes().to_vec()); + + // Old data account now holds the new data, ie: + // - OldData: [*New program data] + let old_data_account = bank.get_account(&old_data).unwrap(); + assert_eq!(old_data_account.data(), &new_data_bytes); + + // Ownership & executable match the new program accounts + assert_eq!(old_account.owner(), &bpf_upgradable_id); + assert!(old_account.executable()); + assert_eq!(old_data_account.owner(), &bpf_upgradable_id); + assert!(!old_data_account.executable()); + + // The remaining lamports from both program accounts minus the rent-exempt + // minimum were burnt + // Lamports from the old data account were burnt + let burnt_after_rent = + new_lamports - bank.get_minimum_balance_for_rent_exemption(old_data.to_bytes().len()); + assert_eq!( + bank.capitalization(), + original_capitalization - burnt_after_rent - old_data_lamports + ); } -// FIXME: Use rent-real lamport values #[test] fn test_program_replace_upgradable_base_case() { // Upgradable program @@ -8505,7 +8746,6 @@ fn test_program_replace_upgradable_base_case() { ); } -// FIXME: Use rent-real lamport values #[test] fn test_program_replace_upgradable_not_data_pda_at_first() { // Upgradable program @@ -8606,16 +8846,14 @@ fn test_program_replace_upgradable_not_data_pda_at_first() { // The remaining lamports from both program accounts minus the rent-exempt // minimum were burnt // Lamports from the old data account were burnt + let burnt_after_rent = new_lamports + old_lamports + - bank.get_minimum_balance_for_rent_exemption(old_data.to_bytes().len()); assert_eq!( bank.capitalization(), - original_capitalization - - (new_lamports + old_lamports - - bank.get_minimum_balance_for_rent_exemption(old_data.to_bytes().len())) - - old_data_lamports + original_capitalization - burnt_after_rent - old_data_lamports ); } -// FIXME: Use rent-real lamport values #[test] fn test_program_replace_upgradable_no_data_provided_with_replacement() { // Upgradable program From e6bfb43751ac7473628b1781e8877d692b38d0bd Mon Sep 17 00:00:00 2001 From: Joe Date: Fri, 18 Aug 2023 10:41:19 -0600 Subject: [PATCH 08/33] stripped back to feature-specific case only --- runtime/src/bank.rs | 117 ++++--- runtime/src/bank/tests.rs | 673 +++----------------------------------- 2 files changed, 99 insertions(+), 691 deletions(-) diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index e1524bb936dfeb..cbc474697df15f 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -8196,20 +8196,30 @@ impl Bank { } } - fn replace_account( + fn replace_account( &mut self, old_address: &Pubkey, new_address: &Pubkey, - old_account: Option<&AccountSharedData>, - new_account: &AccountSharedData, - ) { + old_account: Option<&O>, + new_account: &N, + ) where + O: ReadableAccount + Sync + ZeroLamport, + N: ReadableAccount + Sync + ZeroLamport, + { let (old_lamports, old_len) = match old_account { Some(old_account) => (old_account.lamports(), old_account.data().len()), None => (0, 0), }; + + // Burn lamports in the old account self.capitalization.fetch_sub(old_lamports, Relaxed); + + // Transfer new account to old account self.store_account(old_address, new_account); + + // Clear new account self.store_account(new_address, &AccountSharedData::default()); + self.calculate_and_update_accounts_data_size_delta_off_chain( old_len, new_account.data().len(), @@ -8224,9 +8234,32 @@ impl Bank { new_address: &Pubkey, datapoint_name: &'static str, ) { - if let Some(new_account) = self.get_account_with_fixed_root(new_address) { - datapoint_info!(datapoint_name, ("slot", self.slot, i64)); + if let Some(old_account) = self.get_account_with_fixed_root(old_address) { + if let Some(new_account) = self.get_account_with_fixed_root(new_address) { + datapoint_info!(datapoint_name, ("slot", self.slot, i64)); + self.replace_account(old_address, new_address, Some(&old_account), &new_account); + + // Unload a program from the bank's cache + self.loaded_programs_cache + .write() + .unwrap() + .remove_programs([*old_address].into_iter()); + } + } + } + + /// Use to replace an empty account with a program by feature activation + #[allow(dead_code)] + fn replace_empty_account_with_upgradeable_program( + &mut self, + old_address: &Pubkey, + new_address: &Pubkey, + datapoint_name: &'static str, + ) { + // Must be attempting to replace an empty account with a program + // account _and_ data account + if let Some(new_account) = self.get_account_with_fixed_root(new_address) { let (old_data_address, _) = Pubkey::find_program_address( &[old_address.as_ref()], &bpf_loader_upgradeable::id(), @@ -8235,47 +8268,15 @@ impl Bank { &[new_address.as_ref()], &bpf_loader_upgradeable::id(), ); - - // If a data account is also provided with this new program - // account, then we want to update the existing data account if let Some(new_data_account) = self.get_account_with_fixed_root(&new_data_address) { - let data = old_data_address.as_ref().to_vec(); + datapoint_info!(datapoint_name, ("slot", self.slot, i64)); - let change_in_cap = - if let Some(old_account) = self.get_account_with_fixed_root(old_address) { - // If the old program account exists and the data is not - // the PDA of the data account, overwrite the data with the - // PDA and fund it with any lamports for rent if necessary - if old_account.data() != data { - let lamports = old_account - .lamports() - .max(self.get_minimum_balance_for_rent_exemption(data.len())); - let overwrite = Account { - lamports, - data, - ..Account::from(new_account.clone()) - }; - self.store_account(old_address, &overwrite); - new_account.lamports() + old_account.lamports() - lamports - } else { - new_account.lamports() - } - } else { - // If the old program account does not exist, create it - // with the PDA as its data and fund it with rent-exempt - // lamports - let lamports = self.get_minimum_balance_for_rent_exemption(data.len()); - let overwrite = Account { - lamports, - data, - ..Account::from(new_account.clone()) - }; - self.store_account(old_address, &overwrite); - new_account.lamports() - lamports - }; + let data = old_data_address.as_ref().to_vec(); + let lamports = self.get_minimum_balance_for_rent_exemption(data.len()); - // Replace the old data account with the new data account + // Replace the old data account with the new one // If the old data account does not exist, it will be created + // If the new data account does exist, it will be overwritten self.replace_account( &old_data_address, &new_data_address, @@ -8283,31 +8284,23 @@ impl Bank { &new_data_account, ); - // Clear the new program account - self.store_account(new_address, &AccountSharedData::default()); - - // Update capitalization - self.capitalization.fetch_sub(change_in_cap, Relaxed); - } else if self - .get_account_with_fixed_root(&old_data_address) - .is_none() - { - // A data account does not exist for the new program - // Replace the old program account with the new program account - // If the old program account doesn't exist, it will be created + // Write the data account's PDA into the program account + let created_program_account = Account { + lamports, + data, + ..new_account.clone().into() + }; self.replace_account( old_address, new_address, self.get_account_with_fixed_root(old_address).as_ref(), - &new_account, + &created_program_account, ); - } - // Unload a program from the bank's cache - self.loaded_programs_cache - .write() - .unwrap() - .remove_programs([*old_address].into_iter()); + // Any remaining lamports in the new program account are burnt + let change_in_cap = new_account.lamports() - lamports; + self.capitalization.fetch_sub(change_in_cap, Relaxed); + } } } diff --git a/runtime/src/bank/tests.rs b/runtime/src/bank/tests.rs index c2c79737138484..91e0e9c27ea089 100644 --- a/runtime/src/bank/tests.rs +++ b/runtime/src/bank/tests.rs @@ -8024,8 +8024,8 @@ fn test_program_replace_set_up_account( } #[test] -fn test_program_replace_non_upgradable_base_case() { - // Non-Upgradable program +fn test_program_replace_non_upgradeable() { + // Non-upgradeable program // - Old: [Old program data] // - New: [*New program data] // @@ -8078,243 +8078,7 @@ fn test_program_replace_non_upgradable_base_case() { } #[test] -fn test_program_replace_non_upgradable_create_data_add_rent() { - // Non-Upgradable program - // - Old: [Old program data] (smaller than minimum rent-exempt size) - // - New: PDA(NewData) - // - NewData: [*New program data] - // - // Should replace the program account with the PDA of the data account, - // and create the data account: - // - Old: PDA(OldData) - // - OldData: [*New program data] - let bpf_id = bpf_loader::id(); - let bpf_upgradable_id = bpf_loader_upgradeable::id(); - let mut bank = create_simple_test_bank(0); - - let old = Pubkey::new_unique(); - let old_bytes = vec![0, 0, 0, 0]; // Smaller than 32 bytes - let old_lamports = bank.get_minimum_balance_for_rent_exemption(old_bytes.len()); - let (old_data, _) = Pubkey::find_program_address(&[old.as_ref()], &bpf_upgradable_id); - test_program_replace_set_up_account(&mut bank, &old, old_lamports, old_bytes, &bpf_id, true); - - let new = Pubkey::new_unique(); - let (new_data, _) = Pubkey::find_program_address(&[new.as_ref()], &bpf_upgradable_id); - let new_bytes = new_data.to_bytes().to_vec(); - let new_lamports = bank.get_minimum_balance_for_rent_exemption(new_bytes.len()); - let new_data_bytes = vec![6; 30]; - let new_data_lamports = bank.get_minimum_balance_for_rent_exemption(new_data_bytes.len()); - test_program_replace_set_up_account( - &mut bank, - &new, - new_lamports, - new_bytes, - &bpf_upgradable_id, - true, - ); - test_program_replace_set_up_account( - &mut bank, - &new_data, - new_data_lamports, - new_data_bytes.clone(), - &bpf_upgradable_id, - false, - ); - - let original_capitalization = bank.capitalization(); - - bank.replace_program_account(&old, &new, "bank-apply_program_replacement"); - - // Old program account's balance was adjusted to pay for minimum rent for - // the PDA (32 bytes) - // - // Note: In this test, the original lamports were _less_ than required - // rent for 32 bytes, so lamports were added - assert_eq!( - bank.get_balance(&old), - bank.get_minimum_balance_for_rent_exemption(old_data.to_bytes().len()) - ); - - // Old data account's balance is now the new data account's balance - // (newly created) - assert_eq!(bank.get_balance(&old_data), new_data_lamports); - - // New program accounts are now empty - assert_eq!(bank.get_balance(&new), 0); - assert_eq!(bank.get_balance(&new_data), 0); - - // Old program account now holds the PDA, ie: - // - Old: PDA(OldData) - let old_account = bank.get_account(&old).unwrap(); - assert_eq!(old_account.data(), &old_data.to_bytes().to_vec()); - - // Old data account has been created & now holds the new data, ie: - // - OldData: [*New program data] - let old_data_account = bank.get_account(&old_data).unwrap(); - assert_eq!(old_data_account.data(), &new_data_bytes); - - // Ownership & executable match the new program accounts - assert_eq!(old_account.owner(), &bpf_upgradable_id); - assert!(old_account.executable()); - assert_eq!(old_data_account.owner(), &bpf_upgradable_id); - assert!(!old_data_account.executable()); - - // The remaining lamports from both program accounts minus the rent-exempt - // minimum were burnt - let burnt_after_rent = new_lamports + old_lamports - - bank.get_minimum_balance_for_rent_exemption(old_data.to_bytes().len()); - assert_eq!( - bank.capitalization(), - original_capitalization - burnt_after_rent - ); -} - -#[test] -fn test_program_replace_non_upgradable_create_data_dont_add_rent() { - // Non-Upgradable program - // - Old: [Old program data] (larger than minimum rent-exempt size) - // - New: PDA(NewData) - // - NewData: [*New program data] - // - // Should replace the program account with the PDA of the data account, - // and create the data account: - // - Old: PDA(OldData) - // - OldData: [*New program data] - let bpf_id = bpf_loader::id(); - let bpf_upgradable_id = bpf_loader_upgradeable::id(); - let mut bank = create_simple_test_bank(0); - - let old = Pubkey::new_unique(); - let old_bytes = vec![0; 44]; // Larger than 32 bytes - let old_lamports = bank.get_minimum_balance_for_rent_exemption(old_bytes.len()); - let (old_data, _) = Pubkey::find_program_address(&[old.as_ref()], &bpf_upgradable_id); - test_program_replace_set_up_account(&mut bank, &old, old_lamports, old_bytes, &bpf_id, true); - - let new = Pubkey::new_unique(); - let (new_data, _) = Pubkey::find_program_address(&[new.as_ref()], &bpf_upgradable_id); - let new_bytes = new_data.to_bytes().to_vec(); - let new_lamports = bank.get_minimum_balance_for_rent_exemption(new_bytes.len()); - let new_data_bytes = vec![6; 30]; - let new_data_lamports = bank.get_minimum_balance_for_rent_exemption(new_data_bytes.len()); - test_program_replace_set_up_account( - &mut bank, - &new, - new_lamports, - new_bytes, - &bpf_upgradable_id, - true, - ); - test_program_replace_set_up_account( - &mut bank, - &new_data, - new_data_lamports, - new_data_bytes.clone(), - &bpf_upgradable_id, - false, - ); - - let original_capitalization = bank.capitalization(); - - bank.replace_program_account(&old, &new, "bank-apply_program_replacement"); - - // Old program account's balance was _not_ adjusted to pay for minimum rent - // for the PDA - // - // Note: In this test, the original lamports were _greater_ than required - // rent for 32 bytes, so _no_ lamports were added - assert_eq!(bank.get_balance(&old), old_lamports); - - // Old data account's balance is now the new data account's balance - // (newly created) - assert_eq!(bank.get_balance(&old_data), new_data_lamports); - - // New program accounts are now empty - assert_eq!(bank.get_balance(&new), 0); - assert_eq!(bank.get_balance(&new_data), 0); - - // Old program account now holds the PDA, ie: - // - Old: PDA(OldData) - let old_account = bank.get_account(&old).unwrap(); - assert_eq!(old_account.data(), &old_data.to_bytes().to_vec()); - - // Old data account has been created & now holds the new data, ie: - // - OldData: [*New program data] - let old_data_account = bank.get_account(&old_data).unwrap(); - assert_eq!(old_data_account.data(), &new_data_bytes); - - // Ownership & executable match the new program accounts - assert_eq!(old_account.owner(), &bpf_upgradable_id); - assert!(old_account.executable()); - assert_eq!(old_data_account.owner(), &bpf_upgradable_id); - assert!(!old_data_account.executable()); - - // Lamports from the new program account were burnt - assert_eq!( - bank.capitalization(), - original_capitalization - new_lamports - ); -} - -#[test] -fn test_program_replace_does_not_exist() { - // Non-Existent program - // - Old: ** Does not exist! ** - // - New: [*Arbitrary data] - // - // Should create the program account, ie: - // - Old: [*Arbitrary data] - let bpf_id = bpf_loader::id(); - let mut bank = create_simple_test_bank(0); - - let old = Pubkey::new_unique(); - - let new = Pubkey::new_unique(); - let new_bytes = vec![6; 30]; - let new_lamports = bank.get_minimum_balance_for_rent_exemption(new_bytes.len()); - test_program_replace_set_up_account( - &mut bank, - &new, - new_lamports, - new_bytes.clone(), - &bpf_id, - true, - ); - - let original_capitalization = bank.capitalization(); - - bank.replace_program_account(&old, &new, "bank-apply_program_replacement"); - - // Old program account was created and funded to pay for minimum rent - // for the arbitrary data - assert_eq!( - bank.get_balance(&old), - bank.get_minimum_balance_for_rent_exemption(new_bytes.len()) - ); - - // New program account is now empty - assert_eq!(bank.get_balance(&new), 0); - - // Old program account holds new program account bytes, ie: - // - Old: [*Arbitrary data] - let old_account = bank.get_account(&old).unwrap(); - assert_eq!(old_account.data(), &new_bytes); - - // Ownership & executable match the new program account - assert_eq!(old_account.owner(), &bpf_id); - assert!(old_account.executable()); - - // The remaining lamports from both program accounts minus the rent-exempt - // minimum were burnt - let burnt_after_rent = - new_lamports - bank.get_minimum_balance_for_rent_exemption(new_bytes.len()); - assert_eq!( - bank.capitalization(), - original_capitalization - burnt_after_rent - ); -} - -#[test] -fn test_program_replace_does_not_exist_create_data() { +fn test_replace_empty_account_with_upgradeable_program() { // Non-Existent program // - Old: ** Does not exist! ** // - New: PDA(NewData) @@ -8323,14 +8087,14 @@ fn test_program_replace_does_not_exist_create_data() { // Should create the program account and the data account, ie: // - Old: PDA(OldData) // - OldData: [Old program data] - let bpf_upgradable_id = bpf_loader_upgradeable::id(); + let bpf_upgradeable_id = bpf_loader_upgradeable::id(); let mut bank = create_simple_test_bank(0); let old = Pubkey::new_unique(); - let (old_data, _) = Pubkey::find_program_address(&[old.as_ref()], &bpf_upgradable_id); + let (old_data, _) = Pubkey::find_program_address(&[old.as_ref()], &bpf_upgradeable_id); let new = Pubkey::new_unique(); - let (new_data, _) = Pubkey::find_program_address(&[new.as_ref()], &bpf_upgradable_id); + let (new_data, _) = Pubkey::find_program_address(&[new.as_ref()], &bpf_upgradeable_id); let new_bytes = new_data.to_bytes().to_vec(); let new_lamports = bank.get_minimum_balance_for_rent_exemption(new_bytes.len()); let new_data_bytes = vec![6; 30]; @@ -8340,7 +8104,7 @@ fn test_program_replace_does_not_exist_create_data() { &new, new_lamports, new_bytes, - &bpf_upgradable_id, + &bpf_upgradeable_id, true, ); test_program_replace_set_up_account( @@ -8348,13 +8112,17 @@ fn test_program_replace_does_not_exist_create_data() { &new_data, new_data_lamports, new_data_bytes.clone(), - &bpf_upgradable_id, + &bpf_upgradeable_id, false, ); let original_capitalization = bank.capitalization(); - bank.replace_program_account(&old, &new, "bank-apply_program_replacement"); + bank.replace_empty_account_with_upgradeable_program( + &old, + &new, + "bank-apply_program_replacement", + ); // Old program account was created and funded to pay for minimum rent // for the PDA @@ -8381,9 +8149,9 @@ fn test_program_replace_does_not_exist_create_data() { assert_eq!(old_data_account.data(), &new_data_bytes); // Ownership & executable match the new program accounts - assert_eq!(old_account.owner(), &bpf_upgradable_id); + assert_eq!(old_account.owner(), &bpf_upgradeable_id); assert!(old_account.executable()); - assert_eq!(old_data_account.owner(), &bpf_upgradable_id); + assert_eq!(old_data_account.owner(), &bpf_upgradeable_id); assert!(!old_data_account.executable()); // The remaining lamports from both program accounts minus the rent-exempt @@ -8397,7 +8165,7 @@ fn test_program_replace_does_not_exist_create_data() { } #[test] -fn test_program_replace_is_native_account() { +fn test_replace_empty_account_with_upgradeable_program_is_native_account() { // Native program // - Old: ** Native account (ie. `Feature11111111`) ** // - New: PDA(NewData) @@ -8406,14 +8174,14 @@ fn test_program_replace_is_native_account() { // Should create the program account and the data account, ie: // - Old: PDA(OldData) // - OldData: [Old program data] - let bpf_upgradable_id = bpf_loader_upgradeable::id(); + let bpf_upgradeable_id = bpf_loader_upgradeable::id(); let mut bank = create_simple_test_bank(0); - let old = Pubkey::new_unique(); - let (old_data, _) = Pubkey::find_program_address(&[old.as_ref()], &bpf_upgradable_id); + let old = feature::id(); // `Feature11111111` + let (old_data, _) = Pubkey::find_program_address(&[old.as_ref()], &bpf_upgradeable_id); let new = Pubkey::new_unique(); - let (new_data, _) = Pubkey::find_program_address(&[new.as_ref()], &bpf_upgradable_id); + let (new_data, _) = Pubkey::find_program_address(&[new.as_ref()], &bpf_upgradeable_id); let new_bytes = new_data.to_bytes().to_vec(); let new_lamports = bank.get_minimum_balance_for_rent_exemption(new_bytes.len()); let new_data_bytes = vec![6; 30]; @@ -8423,7 +8191,7 @@ fn test_program_replace_is_native_account() { &new, new_lamports, new_bytes, - &bpf_upgradable_id, + &bpf_upgradeable_id, true, ); test_program_replace_set_up_account( @@ -8431,13 +8199,17 @@ fn test_program_replace_is_native_account() { &new_data, new_data_lamports, new_data_bytes.clone(), - &bpf_upgradable_id, + &bpf_upgradeable_id, false, ); let original_capitalization = bank.capitalization(); - bank.replace_program_account(&old, &new, "bank-apply_program_replacement"); + bank.replace_empty_account_with_upgradeable_program( + &old, + &new, + "bank-apply_program_replacement", + ); // Old program account was created and funded to pay for minimum rent // for the PDA @@ -8464,9 +8236,9 @@ fn test_program_replace_is_native_account() { assert_eq!(old_data_account.data(), &new_data_bytes); // Ownership & executable match the new program accounts - assert_eq!(old_account.owner(), &bpf_upgradable_id); + assert_eq!(old_account.owner(), &bpf_upgradeable_id); assert!(old_account.executable()); - assert_eq!(old_data_account.owner(), &bpf_upgradable_id); + assert_eq!(old_data_account.owner(), &bpf_upgradeable_id); assert!(!old_data_account.executable()); // The remaining lamports from both program accounts minus the rent-exempt @@ -8480,79 +8252,8 @@ fn test_program_replace_is_native_account() { } #[test] -fn test_program_replace_does_not_exist_but_data_does() { - // Non-Upgradable program - // - Old: ** Does not exist! ** - // - OldData: [Old program data] - // - New: [*Arbitrary data] - // - // This is NOT allowed and should leave the program unchanged: - // - Old: ** Does not exist! ** - // - OldData: [Old program data] - let bpf_id = bpf_loader::id(); - let bpf_upgradable_id = bpf_loader_upgradeable::id(); - let mut bank = create_simple_test_bank(0); - - let old = Pubkey::new_unique(); - let (old_data, _) = Pubkey::find_program_address(&[old.as_ref()], &bpf_upgradable_id); - let old_data_bytes = vec![4; 10]; - let old_data_lamports = bank.get_minimum_balance_for_rent_exemption(old_data_bytes.len()); - test_program_replace_set_up_account( - &mut bank, - &old_data, - old_data_lamports, - old_data_bytes.clone(), - &bpf_upgradable_id, - false, - ); - - let new = Pubkey::new_unique(); - let new_bytes = vec![2; 12]; - let new_lamports = bank.get_minimum_balance_for_rent_exemption(new_bytes.len()); - test_program_replace_set_up_account( - &mut bank, - &new, - new_lamports, - new_bytes.clone(), - &bpf_id, - true, - ); - - let original_capitalization = bank.capitalization(); - - bank.replace_program_account(&old, &new, "bank-apply_program_replacement"); - - // All balances are unchanged - assert_eq!(bank.get_balance(&old), 0); - assert_eq!(bank.get_balance(&old_data), old_data_lamports); - assert_eq!(bank.get_balance(&new), new_lamports); - - // Old program accounts still does not exist - assert_eq!(bank.get_account(&old), None); - - // Old data accounts' data is unchanged - // - OldData: [Old program data] - let old_data_account = bank.get_account(&old_data).unwrap(); - assert_eq!(old_data_account.data(), &old_data_bytes); - - // New program account is unchanged - // - New: [*Arbitrary data] - let new_account = bank.get_account(&new).unwrap(); - assert_eq!(new_account.data(), &new_bytes); - - // Ownership & executable are unchanged - assert_eq!(old_data_account.owner(), &bpf_upgradable_id); - assert!(!old_data_account.executable()); - assert_eq!(new_account.owner(), &bpf_id); - assert!(new_account.executable()); - - // Lamports were unchanged across the board - assert_eq!(bank.capitalization(), original_capitalization); -} - -#[test] -fn test_program_replace_does_not_exist_but_data_does_with_data() { - // Non-Upgradable program +fn test_replace_empty_account_with_upgradeable_program_has_data() { + // Non-upgradeable program // - Old: ** Does not exist! ** // - OldData: [Old program data] // - New: PDA(NewData) @@ -8561,11 +8262,11 @@ fn test_program_replace_does_not_exist_but_data_does_with_data() { // Should create the program account and replace the data account: // - Old: PDA(OldData) // - OldData: [*New program data] - let bpf_upgradable_id = bpf_loader_upgradeable::id(); + let bpf_upgradeable_id = bpf_loader_upgradeable::id(); let mut bank = create_simple_test_bank(0); let old = Pubkey::new_unique(); - let (old_data, _) = Pubkey::find_program_address(&[old.as_ref()], &bpf_upgradable_id); + let (old_data, _) = Pubkey::find_program_address(&[old.as_ref()], &bpf_upgradeable_id); let old_data_bytes = vec![4; 10]; let old_data_lamports = bank.get_minimum_balance_for_rent_exemption(old_data_bytes.len()); test_program_replace_set_up_account( @@ -8573,12 +8274,12 @@ fn test_program_replace_does_not_exist_but_data_does_with_data() { &old_data, old_data_lamports, old_data_bytes, - &bpf_upgradable_id, + &bpf_upgradeable_id, false, ); let new = Pubkey::new_unique(); - let (new_data, _) = Pubkey::find_program_address(&[new.as_ref()], &bpf_upgradable_id); + let (new_data, _) = Pubkey::find_program_address(&[new.as_ref()], &bpf_upgradeable_id); let new_bytes = new_data.to_bytes().to_vec(); let new_lamports = bank.get_minimum_balance_for_rent_exemption(new_bytes.len()); let new_data_bytes = vec![6; 30]; @@ -8588,7 +8289,7 @@ fn test_program_replace_does_not_exist_but_data_does_with_data() { &new, new_lamports, new_bytes, - &bpf_upgradable_id, + &bpf_upgradeable_id, true, ); test_program_replace_set_up_account( @@ -8596,13 +8297,17 @@ fn test_program_replace_does_not_exist_but_data_does_with_data() { &new_data, new_data_lamports, new_data_bytes.clone(), - &bpf_upgradable_id, + &bpf_upgradeable_id, false, ); let original_capitalization = bank.capitalization(); - bank.replace_program_account(&old, &new, "bank-apply_program_replacement"); + bank.replace_empty_account_with_upgradeable_program( + &old, + &new, + "bank-apply_program_replacement", + ); // Old program account was created and funded to pay for minimum rent // for the PDA @@ -8629,9 +8334,9 @@ fn test_program_replace_does_not_exist_but_data_does_with_data() { assert_eq!(old_data_account.data(), &new_data_bytes); // Ownership & executable match the new program accounts - assert_eq!(old_account.owner(), &bpf_upgradable_id); + assert_eq!(old_account.owner(), &bpf_upgradeable_id); assert!(old_account.executable()); - assert_eq!(old_data_account.owner(), &bpf_upgradable_id); + assert_eq!(old_data_account.owner(), &bpf_upgradeable_id); assert!(!old_data_account.executable()); // The remaining lamports from both program accounts minus the rent-exempt @@ -8645,296 +8350,6 @@ fn test_program_replace_does_not_exist_but_data_does_with_data() { ); } -#[test] -fn test_program_replace_upgradable_base_case() { - // Upgradable program - // - Old: PDA(OldData) - // - OldData: [Old program data] - // - New: PDA(NewData) - // - NewData: [*New program data] - // - // Should _only_ replace the data account, not the program account: - // - Old: PDA(OldData) - // - OldData: [*New program data] - let bpf_upgradable_id = bpf_loader_upgradeable::id(); - let mut bank = create_simple_test_bank(0); - - let old = Pubkey::new_unique(); - let (old_data, _) = Pubkey::find_program_address(&[old.as_ref()], &bpf_upgradable_id); - let old_bytes = old_data.to_bytes().to_vec(); - let old_lamports = bank.get_minimum_balance_for_rent_exemption(old_bytes.len()); - let old_data_bytes = vec![4; 10]; - let old_data_lamports = bank.get_minimum_balance_for_rent_exemption(old_data_bytes.len()); - test_program_replace_set_up_account( - &mut bank, - &old, - old_lamports, - old_bytes, - &bpf_upgradable_id, - true, - ); - test_program_replace_set_up_account( - &mut bank, - &old_data, - old_data_lamports, - old_data_bytes, - &bpf_upgradable_id, - false, - ); - - let new = Pubkey::new_unique(); - let (new_data, _) = Pubkey::find_program_address(&[new.as_ref()], &bpf_upgradable_id); - let new_bytes = new_data.to_bytes().to_vec(); - let new_lamports = bank.get_minimum_balance_for_rent_exemption(new_bytes.len()); - let new_data_bytes = vec![6; 30]; - let new_data_lamports = bank.get_minimum_balance_for_rent_exemption(new_data_bytes.len()); - test_program_replace_set_up_account( - &mut bank, - &new, - new_lamports, - new_bytes, - &bpf_upgradable_id, - true, - ); - test_program_replace_set_up_account( - &mut bank, - &new_data, - new_data_lamports, - new_data_bytes.clone(), - &bpf_upgradable_id, - false, - ); - - let original_capitalization = bank.capitalization(); - - bank.replace_program_account(&old, &new, "bank-apply_program_replacement"); - - // Old program account's balance was _not_ adjusted to pay for minimum rent - // for the PDA - // - // Note: In this test, the original bytes were unchanged (PDA) - assert_eq!(bank.get_balance(&old), old_lamports); - - // Old data account's balance is now the new data account's balance - assert_eq!(bank.get_balance(&old_data), new_data_lamports); - - // New program accounts are now empty - assert_eq!(bank.get_balance(&new), 0); - assert_eq!(bank.get_balance(&new_data), 0); - - // Old program account still holds the same PDA, ie: - // - Old: PDA(OldData) - let old_account = bank.get_account(&old).unwrap(); - assert_eq!(old_account.data(), &old_data.to_bytes().to_vec()); - - // Old data account now holds the new data, ie: - // - OldData: [*New program data] - let old_data_account = bank.get_account(&old_data).unwrap(); - assert_eq!(old_data_account.data(), &new_data_bytes); - - // Ownership & executable match the new program accounts - assert_eq!(old_account.owner(), &bpf_upgradable_id); - assert!(old_account.executable()); - assert_eq!(old_data_account.owner(), &bpf_upgradable_id); - assert!(!old_data_account.executable()); - - // Lamports from the new program account were burnt - // Lamports from the old data account were burnt - assert_eq!( - bank.capitalization(), - original_capitalization - new_lamports - old_data_lamports - ); -} - -#[test] -fn test_program_replace_upgradable_not_data_pda_at_first() { - // Upgradable program - // - Old: [*Old arbitrary data] (smaller than minimum rent-exempt size) - // - OldData: [Old program data] - // - New: PDA(NewData) - // - NewData: [*New program data] - // - // Should _only_ replace the data account, not the program account: - // - Old: PDA(OldData) - // - OldData: [*New program data] - let bpf_upgradable_id = bpf_loader_upgradeable::id(); - let mut bank = create_simple_test_bank(0); - - let old = Pubkey::new_unique(); - let (old_data, _) = Pubkey::find_program_address(&[old.as_ref()], &bpf_upgradable_id); - let old_bytes = vec![1; 18]; // Smaller than 32 bytes - let old_lamports = bank.get_minimum_balance_for_rent_exemption(old_bytes.len()); - let old_data_bytes = vec![4; 10]; - let old_data_lamports = bank.get_minimum_balance_for_rent_exemption(old_data_bytes.len()); - test_program_replace_set_up_account( - &mut bank, - &old, - old_lamports, - old_bytes, - &bpf_upgradable_id, - true, - ); - test_program_replace_set_up_account( - &mut bank, - &old_data, - old_data_lamports, - old_data_bytes, - &bpf_upgradable_id, - false, - ); - - let new = Pubkey::new_unique(); - let (new_data, _) = Pubkey::find_program_address(&[new.as_ref()], &bpf_upgradable_id); - let new_bytes = new_data.to_bytes().to_vec(); - let new_lamports = bank.get_minimum_balance_for_rent_exemption(new_bytes.len()); - let new_data_bytes = vec![6; 30]; - let new_data_lamports = bank.get_minimum_balance_for_rent_exemption(new_data_bytes.len()); - test_program_replace_set_up_account( - &mut bank, - &new, - new_lamports, - new_bytes, - &bpf_upgradable_id, - true, - ); - test_program_replace_set_up_account( - &mut bank, - &new_data, - new_data_lamports, - new_data_bytes.clone(), - &bpf_upgradable_id, - false, - ); - - let original_capitalization = bank.capitalization(); - - bank.replace_program_account(&old, &new, "bank-apply_program_replacement"); - - // Old program account's balance was adjusted to pay for minimum rent for - // the PDA (32 bytes) - // - // Note: In this test, the original lamports were _less_ than required - // rent for 32 bytes, so lamports were added - assert_eq!( - bank.get_balance(&old), - bank.get_minimum_balance_for_rent_exemption(old_data.to_bytes().len()) - ); - - // Old data account's balance is now the new data account's balance - assert_eq!(bank.get_balance(&old_data), new_data_lamports); - - // New program accounts are now empty - assert_eq!(bank.get_balance(&new), 0); - assert_eq!(bank.get_balance(&new_data), 0); - - // Old program account now holds the PDA, ie: - // - Old: PDA(OldData) - let old_account = bank.get_account(&old).unwrap(); - assert_eq!(old_account.data(), &old_data.to_bytes().to_vec()); - - // Old data account now holds the new data, ie: - // - OldData: [*New program data] - let old_data_account = bank.get_account(&old_data).unwrap(); - assert_eq!(old_data_account.data(), &new_data_bytes); - - // Ownership & executable match the new program accounts - assert_eq!(old_account.owner(), &bpf_upgradable_id); - assert!(old_account.executable()); - assert_eq!(old_data_account.owner(), &bpf_upgradable_id); - assert!(!old_data_account.executable()); - - // The remaining lamports from both program accounts minus the rent-exempt - // minimum were burnt - // Lamports from the old data account were burnt - let burnt_after_rent = new_lamports + old_lamports - - bank.get_minimum_balance_for_rent_exemption(old_data.to_bytes().len()); - assert_eq!( - bank.capitalization(), - original_capitalization - burnt_after_rent - old_data_lamports - ); -} - -#[test] -fn test_program_replace_upgradable_no_data_provided_with_replacement() { - // Upgradable program - // - Old: PDA(OldData) - // - OldData: [Old program data] - // - New: [*New program data] - // - // This is NOT allowed and should leave the program unchanged: - // - Old: PDA(OldData) - // - OldData: [Old program data] - let bpf_id = bpf_loader::id(); - let bpf_upgradable_id = bpf_loader_upgradeable::id(); - let mut bank = create_simple_test_bank(0); - - let old = Pubkey::new_unique(); - let (old_data, _) = Pubkey::find_program_address(&[old.as_ref()], &bpf_upgradable_id); - let old_bytes = vec![5; 5]; - let old_lamports = bank.get_minimum_balance_for_rent_exemption(old_bytes.len()); - let old_data_bytes = vec![4; 10]; - let old_data_lamports = bank.get_minimum_balance_for_rent_exemption(old_data_bytes.len()); - test_program_replace_set_up_account( - &mut bank, - &old, - old_lamports, - old_bytes.clone(), - &bpf_upgradable_id, - true, - ); - test_program_replace_set_up_account( - &mut bank, - &old_data, - old_data_lamports, - old_data_bytes.clone(), - &bpf_upgradable_id, - false, - ); - - let new = Pubkey::new_unique(); - let new_bytes = vec![2; 12]; - let new_lamports = bank.get_minimum_balance_for_rent_exemption(new_bytes.len()); - test_program_replace_set_up_account( - &mut bank, - &new, - new_lamports, - new_bytes.clone(), - &bpf_id, - true, - ); - - let original_capitalization = bank.capitalization(); - - bank.replace_program_account(&old, &new, "bank-apply_program_replacement"); - - // All balances are unchanged - assert_eq!(bank.get_balance(&old), old_lamports); - assert_eq!(bank.get_balance(&old_data), old_data_lamports); - assert_eq!(bank.get_balance(&new), new_lamports); - - // Old program accounts' data are unchanged - // - Old: PDA(OldData) - // - OldData: [Old program data] - let old_account = bank.get_account(&old).unwrap(); - assert_eq!(old_account.data(), &old_bytes); - let old_data_account = bank.get_account(&old_data).unwrap(); - assert_eq!(old_data_account.data(), &old_data_bytes); - - // New program account is unchanged - // - New: [*New program data] - let new_account = bank.get_account(&new).unwrap(); - assert_eq!(new_account.data(), &new_bytes); - - // Ownership & executable are unchanged - assert_eq!(old_account.owner(), &bpf_upgradable_id); - assert!(old_account.executable()); - assert_eq!(old_data_account.owner(), &bpf_upgradable_id); - assert!(!old_data_account.executable()); - - // Lamports were unchanged across the board - assert_eq!(bank.capitalization(), original_capitalization); -} - fn min_rent_exempt_balance_for_sysvars(bank: &Bank, sysvar_ids: &[Pubkey]) -> u64 { sysvar_ids .iter() From 24e75d501098ec20a2baeb8b96664f5a275d8683 Mon Sep 17 00:00:00 2001 From: Joe Date: Fri, 18 Aug 2023 11:40:30 -0600 Subject: [PATCH 09/33] added feature --- runtime/src/bank.rs | 9 +++++++++ runtime/src/inline_feature_gate_program.rs | 5 +++++ runtime/src/lib.rs | 1 + sdk/src/feature_set.rs | 5 +++++ 4 files changed, 20 insertions(+) create mode 100644 runtime/src/inline_feature_gate_program.rs diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index cbc474697df15f..60d87621315c67 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -42,6 +42,7 @@ use { builtins::{BuiltinPrototype, BUILTINS}, epoch_rewards_hasher::hash_rewards_into_partitions, epoch_stakes::{EpochStakes, NodeVoteAccounts}, + inline_feature_gate_program, runtime_config::RuntimeConfig, serde_snapshot::BankIncrementalSnapshotPersistence, snapshot_hash::SnapshotHash, @@ -8054,6 +8055,14 @@ impl Bank { if new_feature_activations.contains(&feature_set::update_hashes_per_tick::id()) { self.apply_updated_hashes_per_tick(DEFAULT_HASHES_PER_TICK); } + + if new_feature_activations.contains(&feature_set::feature_gate_program::id()) { + self.replace_empty_account_with_upgradeable_program( + &feature::id(), + &inline_feature_gate_program::noop_program::id(), + "bank-apply_feature_gate_program", + ); + } } fn apply_updated_hashes_per_tick(&mut self, hashes_per_tick: u64) { diff --git a/runtime/src/inline_feature_gate_program.rs b/runtime/src/inline_feature_gate_program.rs new file mode 100644 index 00000000000000..84391b3054d0be --- /dev/null +++ b/runtime/src/inline_feature_gate_program.rs @@ -0,0 +1,5 @@ +solana_sdk::declare_id!("Feature111111111111111111111111111111111111"); + +pub(crate) mod noop_program { + solana_sdk::declare_id!("5Re2FUHmvSdem1KV2bU5GFvqZpP2MXj76yVhMKKbP45o"); +} diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index ff94a68c69fa1e..503d24410e8cdc 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -14,6 +14,7 @@ pub mod commitment; mod epoch_rewards_hasher; pub mod epoch_stakes; pub mod genesis_utils; +pub mod inline_feature_gate_program; pub mod inline_spl_associated_token_account; pub mod loader_utils; pub mod non_circulating_supply; diff --git a/sdk/src/feature_set.rs b/sdk/src/feature_set.rs index b414a5f6ab4551..ac8332a9eddbdc 100644 --- a/sdk/src/feature_set.rs +++ b/sdk/src/feature_set.rs @@ -700,6 +700,10 @@ pub mod better_error_codes_for_tx_lamport_check { solana_sdk::declare_id!("Ffswd3egL3tccB6Rv3XY6oqfdzn913vUcjCSnpvCKpfx"); } +pub mod feature_gate_program { + solana_sdk::declare_id!("8GdovDzVwWU5edz2G697bbB7GZjrUc6aQZLWyNNAtHdg"); +} + lazy_static! { /// Map of feature identifiers to user-visible description pub static ref FEATURE_NAMES: HashMap = [ @@ -870,6 +874,7 @@ lazy_static! { (require_rent_exempt_split_destination::id(), "Require stake split destination account to be rent exempt"), (better_error_codes_for_tx_lamport_check::id(), "better error codes for tx lamport check #33353"), (enable_alt_bn128_compression_syscall::id(), "add alt_bn128 compression syscalls"), + (feature_gate_program::id(), "control feature activations with an on-chain program"), /*************** ADD NEW FEATURES HERE ***************/ ] .iter() From a573a974d34ccc8a344ab06f4b44b50b3d6e2396 Mon Sep 17 00:00:00 2001 From: Joe Date: Mon, 21 Aug 2023 15:00:17 -0600 Subject: [PATCH 10/33] address initial feedback --- runtime/src/bank.rs | 3 +-- runtime/src/inline_feature_gate_program.rs | 2 +- sdk/src/feature_set.rs | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 60d87621315c67..39e2e30b640f25 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -8259,7 +8259,6 @@ impl Bank { } /// Use to replace an empty account with a program by feature activation - #[allow(dead_code)] fn replace_empty_account_with_upgradeable_program( &mut self, old_address: &Pubkey, @@ -8285,7 +8284,7 @@ impl Bank { // Replace the old data account with the new one // If the old data account does not exist, it will be created - // If the new data account does exist, it will be overwritten + // If it does exist, it will be overwritten self.replace_account( &old_data_address, &new_data_address, diff --git a/runtime/src/inline_feature_gate_program.rs b/runtime/src/inline_feature_gate_program.rs index 84391b3054d0be..3354c8f7e06e3e 100644 --- a/runtime/src/inline_feature_gate_program.rs +++ b/runtime/src/inline_feature_gate_program.rs @@ -1,4 +1,4 @@ -solana_sdk::declare_id!("Feature111111111111111111111111111111111111"); +//! Contains replacement program IDs for the feature gate program pub(crate) mod noop_program { solana_sdk::declare_id!("5Re2FUHmvSdem1KV2bU5GFvqZpP2MXj76yVhMKKbP45o"); diff --git a/sdk/src/feature_set.rs b/sdk/src/feature_set.rs index ac8332a9eddbdc..289e9a8d2ca7e6 100644 --- a/sdk/src/feature_set.rs +++ b/sdk/src/feature_set.rs @@ -874,7 +874,7 @@ lazy_static! { (require_rent_exempt_split_destination::id(), "Require stake split destination account to be rent exempt"), (better_error_codes_for_tx_lamport_check::id(), "better error codes for tx lamport check #33353"), (enable_alt_bn128_compression_syscall::id(), "add alt_bn128 compression syscalls"), - (feature_gate_program::id(), "control feature activations with an on-chain program"), + (feature_gate_program::id(), "control feature activations with an on-chain program #32783"), /*************** ADD NEW FEATURES HERE ***************/ ] .iter() From c2210385fe86f4595beb4d3b83f5ca788281dd70 Mon Sep 17 00:00:00 2001 From: Joe Date: Mon, 21 Aug 2023 15:28:41 -0600 Subject: [PATCH 11/33] added more lamport checks --- runtime/src/bank.rs | 70 ++++++++++++++++++++++++++------------------- 1 file changed, 41 insertions(+), 29 deletions(-) diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 39e2e30b640f25..a09a414f9c753e 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -8277,37 +8277,49 @@ impl Bank { &bpf_loader_upgradeable::id(), ); if let Some(new_data_account) = self.get_account_with_fixed_root(&new_data_address) { - datapoint_info!(datapoint_name, ("slot", self.slot, i64)); - - let data = old_data_address.as_ref().to_vec(); - let lamports = self.get_minimum_balance_for_rent_exemption(data.len()); - - // Replace the old data account with the new one - // If the old data account does not exist, it will be created - // If it does exist, it will be overwritten - self.replace_account( - &old_data_address, - &new_data_address, - self.get_account_with_fixed_root(&old_data_address).as_ref(), - &new_data_account, - ); + // Make sure the old account is empty + // We aren't going to check that there isn't a data account at + // the known program-derived address (ie. `old_data_address`), + // because if it exists, it will be overwritten + if self.get_account_with_fixed_root(old_address).is_none() { + let data = old_data_address.as_ref().to_vec(); + let lamports = self.get_minimum_balance_for_rent_exemption(data.len()); + + // Make sure the new account has enough rent-exempt + // lamports for the empty account that will now house the + // PDA + if new_account.lamports() >= lamports { + if let Some(change_in_cap) = new_account.lamports().checked_sub(lamports) { + datapoint_info!(datapoint_name, ("slot", self.slot, i64)); + + // Replace the old data account with the new one + // If the old data account does not exist, it will be created + // If it does exist, it will be overwritten + self.replace_account( + &old_data_address, + &new_data_address, + self.get_account_with_fixed_root(&old_data_address).as_ref(), + &new_data_account, + ); - // Write the data account's PDA into the program account - let created_program_account = Account { - lamports, - data, - ..new_account.clone().into() - }; - self.replace_account( - old_address, - new_address, - self.get_account_with_fixed_root(old_address).as_ref(), - &created_program_account, - ); + // Write the data account's PDA into the program account + let created_program_account = Account { + lamports, + data, + ..new_account.into() + }; + self.replace_account( + old_address, + new_address, + self.get_account_with_fixed_root(old_address).as_ref(), + &created_program_account, + ); - // Any remaining lamports in the new program account are burnt - let change_in_cap = new_account.lamports() - lamports; - self.capitalization.fetch_sub(change_in_cap, Relaxed); + // Any remaining lamports in the new program account are burnt + self.capitalization.fetch_sub(change_in_cap, Relaxed); + } + } + } } } } From b36f56a6fe27d14086bd391893abc2dcc90889c5 Mon Sep 17 00:00:00 2001 From: Joe Date: Mon, 21 Aug 2023 16:15:18 -0600 Subject: [PATCH 12/33] condense tests --- runtime/src/bank/tests.rs | 240 ++++++++++---------------------------- 1 file changed, 60 insertions(+), 180 deletions(-) diff --git a/runtime/src/bank/tests.rs b/runtime/src/bank/tests.rs index 91e0e9c27ea089..d06ffeb33097f4 100644 --- a/runtime/src/bank/tests.rs +++ b/runtime/src/bank/tests.rs @@ -8024,7 +8024,7 @@ fn test_program_replace_set_up_account( } #[test] -fn test_program_replace_non_upgradeable() { +fn test_program_replace() { // Non-upgradeable program // - Old: [Old program data] // - New: [*New program data] @@ -8077,22 +8077,20 @@ fn test_program_replace_non_upgradeable() { ); } -#[test] -fn test_replace_empty_account_with_upgradeable_program() { - // Non-Existent program - // - Old: ** Does not exist! ** - // - New: PDA(NewData) - // - NewData: [*New program data] - // - // Should create the program account and the data account, ie: +fn test_replace_empty_account_success( + old: Pubkey, + maybe_old_data_bytes: Option>, // Inner data of the old program _data_ account +) { + // Ensures a program account and data account are created when replacing an + // empty account, ie: // - Old: PDA(OldData) // - OldData: [Old program data] + // + // If the old data account exists, it will be overwritten let bpf_upgradeable_id = bpf_loader_upgradeable::id(); let mut bank = create_simple_test_bank(0); - let old = Pubkey::new_unique(); - let (old_data, _) = Pubkey::find_program_address(&[old.as_ref()], &bpf_upgradeable_id); - + // Create the test new accounts, one for program and one for data let new = Pubkey::new_unique(); let (new_data, _) = Pubkey::find_program_address(&[new.as_ref()], &bpf_upgradeable_id); let new_bytes = new_data.to_bytes().to_vec(); @@ -8116,12 +8114,31 @@ fn test_replace_empty_account_with_upgradeable_program() { false, ); + // Derive the well-known PDA address for the old data account + let (old_data, _) = Pubkey::find_program_address(&[old.as_ref()], &bpf_upgradeable_id); + let burnt_after_rent = if let Some(old_data_bytes) = maybe_old_data_bytes { + let old_data_lamports = bank.get_minimum_balance_for_rent_exemption(old_data_bytes.len()); + test_program_replace_set_up_account( + &mut bank, + &old_data, + old_data_lamports, + old_data_bytes, + &bpf_upgradeable_id, + false, + ); + old_data_lamports + new_lamports + - bank.get_minimum_balance_for_rent_exemption(old_data.to_bytes().len()) + } else { + new_lamports - bank.get_minimum_balance_for_rent_exemption(old_data.to_bytes().len()) + }; + let original_capitalization = bank.capitalization(); + // Do the replacement bank.replace_empty_account_with_upgradeable_program( &old, &new, - "bank-apply_program_replacement", + "bank-apply_empty_account_replacement_for_program", ); // Old program account was created and funded to pay for minimum rent @@ -8156,8 +8173,6 @@ fn test_replace_empty_account_with_upgradeable_program() { // The remaining lamports from both program accounts minus the rent-exempt // minimum were burnt - let burnt_after_rent = - new_lamports - bank.get_minimum_balance_for_rent_exemption(old_data.to_bytes().len()); assert_eq!( bank.capitalization(), original_capitalization - burnt_after_rent @@ -8165,188 +8180,53 @@ fn test_replace_empty_account_with_upgradeable_program() { } #[test] -fn test_replace_empty_account_with_upgradeable_program_is_native_account() { - // Native program - // - Old: ** Native account (ie. `Feature11111111`) ** +fn test_replace_empty_account() { + // Empty account _without_ corresponding data account + // - Old: ** Does not exist! ** // - New: PDA(NewData) // - NewData: [*New program data] // // Should create the program account and the data account, ie: // - Old: PDA(OldData) // - OldData: [Old program data] - let bpf_upgradeable_id = bpf_loader_upgradeable::id(); - let mut bank = create_simple_test_bank(0); - - let old = feature::id(); // `Feature11111111` - let (old_data, _) = Pubkey::find_program_address(&[old.as_ref()], &bpf_upgradeable_id); - - let new = Pubkey::new_unique(); - let (new_data, _) = Pubkey::find_program_address(&[new.as_ref()], &bpf_upgradeable_id); - let new_bytes = new_data.to_bytes().to_vec(); - let new_lamports = bank.get_minimum_balance_for_rent_exemption(new_bytes.len()); - let new_data_bytes = vec![6; 30]; - let new_data_lamports = bank.get_minimum_balance_for_rent_exemption(new_data_bytes.len()); - test_program_replace_set_up_account( - &mut bank, - &new, - new_lamports, - new_bytes, - &bpf_upgradeable_id, - true, - ); - test_program_replace_set_up_account( - &mut bank, - &new_data, - new_data_lamports, - new_data_bytes.clone(), - &bpf_upgradeable_id, - false, - ); - - let original_capitalization = bank.capitalization(); - - bank.replace_empty_account_with_upgradeable_program( - &old, - &new, - "bank-apply_program_replacement", - ); - - // Old program account was created and funded to pay for minimum rent - // for the PDA - assert_eq!( - bank.get_balance(&old), - bank.get_minimum_balance_for_rent_exemption(old_data.to_bytes().len()) - ); - - // Old data account was created, now holds the new data account's balance - assert_eq!(bank.get_balance(&old_data), new_data_lamports); - - // New program accounts are now empty - assert_eq!(bank.get_balance(&new), 0); - assert_eq!(bank.get_balance(&new_data), 0); - - // Old program account holds the PDA, ie: - // - Old: PDA(OldData) - let old_account = bank.get_account(&old).unwrap(); - assert_eq!(old_account.data(), &old_data.to_bytes().to_vec()); - - // Old data account holds the new data, ie: - // - OldData: [*New program data] - let old_data_account = bank.get_account(&old_data).unwrap(); - assert_eq!(old_data_account.data(), &new_data_bytes); - - // Ownership & executable match the new program accounts - assert_eq!(old_account.owner(), &bpf_upgradeable_id); - assert!(old_account.executable()); - assert_eq!(old_data_account.owner(), &bpf_upgradeable_id); - assert!(!old_data_account.executable()); - - // The remaining lamports from both program accounts minus the rent-exempt - // minimum were burnt - let burnt_after_rent = - new_lamports - bank.get_minimum_balance_for_rent_exemption(old_data.to_bytes().len()); - assert_eq!( - bank.capitalization(), - original_capitalization - burnt_after_rent - ); -} + test_replace_empty_account_success(Pubkey::new_unique(), None); -#[test] -fn test_replace_empty_account_with_upgradeable_program_has_data() { - // Non-upgradeable program + // Empty account _with_ corresponding data account // - Old: ** Does not exist! ** // - OldData: [Old program data] // - New: PDA(NewData) // - NewData: [*New program data] // - // Should create the program account and replace the data account: + // Should create the program account and the data account, ie: // - Old: PDA(OldData) - // - OldData: [*New program data] - let bpf_upgradeable_id = bpf_loader_upgradeable::id(); - let mut bank = create_simple_test_bank(0); - - let old = Pubkey::new_unique(); - let (old_data, _) = Pubkey::find_program_address(&[old.as_ref()], &bpf_upgradeable_id); - let old_data_bytes = vec![4; 10]; - let old_data_lamports = bank.get_minimum_balance_for_rent_exemption(old_data_bytes.len()); - test_program_replace_set_up_account( - &mut bank, - &old_data, - old_data_lamports, - old_data_bytes, - &bpf_upgradeable_id, - false, - ); - - let new = Pubkey::new_unique(); - let (new_data, _) = Pubkey::find_program_address(&[new.as_ref()], &bpf_upgradeable_id); - let new_bytes = new_data.to_bytes().to_vec(); - let new_lamports = bank.get_minimum_balance_for_rent_exemption(new_bytes.len()); - let new_data_bytes = vec![6; 30]; - let new_data_lamports = bank.get_minimum_balance_for_rent_exemption(new_data_bytes.len()); - test_program_replace_set_up_account( - &mut bank, - &new, - new_lamports, - new_bytes, - &bpf_upgradeable_id, - true, - ); - test_program_replace_set_up_account( - &mut bank, - &new_data, - new_data_lamports, - new_data_bytes.clone(), - &bpf_upgradeable_id, - false, - ); - - let original_capitalization = bank.capitalization(); - - bank.replace_empty_account_with_upgradeable_program( - &old, - &new, - "bank-apply_program_replacement", - ); + // - OldData: [Old program data] + test_replace_empty_account_success(Pubkey::new_unique(), Some(vec![4; 40])); - // Old program account was created and funded to pay for minimum rent - // for the PDA - assert_eq!( - bank.get_balance(&old), - bank.get_minimum_balance_for_rent_exemption(old_data.to_bytes().len()) + // Native account _without_ corresponding data account + // - Old: ** Native account (ie. `Feature11111111`) ** + // - New: PDA(NewData) + // - NewData: [*New program data] + // + // Should create the program account and the data account, ie: + // - Old: PDA(OldData) + // - OldData: [Old program data] + test_replace_empty_account_success( + feature::id(), // `Feature11111111` + None, ); - // Old data account's balance is now the new data account's balance - assert_eq!(bank.get_balance(&old_data), new_data_lamports); - - // New program accounts are now empty - assert_eq!(bank.get_balance(&new), 0); - assert_eq!(bank.get_balance(&new_data), 0); - - // Old program account holds the PDA, ie: + // Native account _with_ corresponding data account + // - Old: ** Native account (ie. `Feature11111111`) ** + // - OldData: [Old program data] + // - New: PDA(NewData) + // - NewData: [*New program data] + // + // Should create the program account and the data account, ie: // - Old: PDA(OldData) - let old_account = bank.get_account(&old).unwrap(); - assert_eq!(old_account.data(), &old_data.to_bytes().to_vec()); - - // Old data account now holds the new data, ie: - // - OldData: [*New program data] - let old_data_account = bank.get_account(&old_data).unwrap(); - assert_eq!(old_data_account.data(), &new_data_bytes); - - // Ownership & executable match the new program accounts - assert_eq!(old_account.owner(), &bpf_upgradeable_id); - assert!(old_account.executable()); - assert_eq!(old_data_account.owner(), &bpf_upgradeable_id); - assert!(!old_data_account.executable()); - - // The remaining lamports from both program accounts minus the rent-exempt - // minimum were burnt - // Lamports from the old data account were burnt - let burnt_after_rent = - new_lamports - bank.get_minimum_balance_for_rent_exemption(old_data.to_bytes().len()); - assert_eq!( - bank.capitalization(), - original_capitalization - burnt_after_rent - old_data_lamports + // - OldData: [Old program data] + test_replace_empty_account_success( + feature::id(), // `Feature11111111` + Some(vec![4; 40]), ); } From 9a61900badfbf93cc87de083b9f41431c5e4450d Mon Sep 17 00:00:00 2001 From: Joe Date: Mon, 21 Aug 2023 16:43:37 -0600 Subject: [PATCH 13/33] using test_case --- runtime/src/bank/tests.rs | 71 +++++++++++---------------------------- 1 file changed, 20 insertions(+), 51 deletions(-) diff --git a/runtime/src/bank/tests.rs b/runtime/src/bank/tests.rs index d06ffeb33097f4..bab876d553051a 100644 --- a/runtime/src/bank/tests.rs +++ b/runtime/src/bank/tests.rs @@ -8077,6 +8077,26 @@ fn test_program_replace() { ); } +#[test_case( + Pubkey::new_unique(), + None; + "Empty account _without_ corresponding data account" +)] +#[test_case( + Pubkey::new_unique(), + Some(vec![4; 40]); + "Empty account _with_ corresponding data account" +)] +#[test_case( + feature::id(), // `Feature11111111` + None; + "Native account _without_ corresponding data account" +)] +#[test_case( + feature::id(), // `Feature11111111` + Some(vec![4; 40]); + "Native account _with_ corresponding data account" +)] fn test_replace_empty_account_success( old: Pubkey, maybe_old_data_bytes: Option>, // Inner data of the old program _data_ account @@ -8179,57 +8199,6 @@ fn test_replace_empty_account_success( ); } -#[test] -fn test_replace_empty_account() { - // Empty account _without_ corresponding data account - // - Old: ** Does not exist! ** - // - New: PDA(NewData) - // - NewData: [*New program data] - // - // Should create the program account and the data account, ie: - // - Old: PDA(OldData) - // - OldData: [Old program data] - test_replace_empty_account_success(Pubkey::new_unique(), None); - - // Empty account _with_ corresponding data account - // - Old: ** Does not exist! ** - // - OldData: [Old program data] - // - New: PDA(NewData) - // - NewData: [*New program data] - // - // Should create the program account and the data account, ie: - // - Old: PDA(OldData) - // - OldData: [Old program data] - test_replace_empty_account_success(Pubkey::new_unique(), Some(vec![4; 40])); - - // Native account _without_ corresponding data account - // - Old: ** Native account (ie. `Feature11111111`) ** - // - New: PDA(NewData) - // - NewData: [*New program data] - // - // Should create the program account and the data account, ie: - // - Old: PDA(OldData) - // - OldData: [Old program data] - test_replace_empty_account_success( - feature::id(), // `Feature11111111` - None, - ); - - // Native account _with_ corresponding data account - // - Old: ** Native account (ie. `Feature11111111`) ** - // - OldData: [Old program data] - // - New: PDA(NewData) - // - NewData: [*New program data] - // - // Should create the program account and the data account, ie: - // - Old: PDA(OldData) - // - OldData: [Old program data] - test_replace_empty_account_success( - feature::id(), // `Feature11111111` - Some(vec![4; 40]), - ); -} - fn min_rent_exempt_balance_for_sysvars(bank: &Bank, sysvar_ids: &[Pubkey]) -> u64 { sysvar_ids .iter() From 67986b48f3669169f9bd4ce61f62adc048c86545 Mon Sep 17 00:00:00 2001 From: Joe Date: Mon, 21 Aug 2023 17:01:11 -0600 Subject: [PATCH 14/33] add fail cases to tests --- runtime/src/bank/tests.rs | 93 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 93 insertions(+) diff --git a/runtime/src/bank/tests.rs b/runtime/src/bank/tests.rs index bab876d553051a..782345997cdd38 100644 --- a/runtime/src/bank/tests.rs +++ b/runtime/src/bank/tests.rs @@ -8136,7 +8136,10 @@ fn test_replace_empty_account_success( // Derive the well-known PDA address for the old data account let (old_data, _) = Pubkey::find_program_address(&[old.as_ref()], &bpf_upgradeable_id); + + // Determine the lamports that will be burnt after the replacement let burnt_after_rent = if let Some(old_data_bytes) = maybe_old_data_bytes { + // Create the data account if necessary let old_data_lamports = bank.get_minimum_balance_for_rent_exemption(old_data_bytes.len()); test_program_replace_set_up_account( &mut bank, @@ -8199,6 +8202,96 @@ fn test_replace_empty_account_success( ); } +#[test_case( + None; + "Existing account _without_ corresponding data account" +)] +#[test_case( + Some(vec![4; 40]); + "Existing account _with_ corresponding data account" +)] +fn test_replace_empty_account_fail_when_account_exists( + maybe_old_data_bytes: Option>, // Inner data of the old program _data_ account +) { + // Should not be allowed to execute replacement + let bpf_upgradeable_id = bpf_loader_upgradeable::id(); + let mut bank = create_simple_test_bank(0); + + // Create the test old account with some arbitrary data and lamports balance + let old = Pubkey::new_unique(); + let old_bytes = vec![0, 0, 0, 0]; // Arbitrary bytes, doesn't matter + let old_lamports = bank.get_minimum_balance_for_rent_exemption(old_bytes.len()); + let old_account = test_program_replace_set_up_account( + &mut bank, + &old, + old_lamports, + old_bytes, + &bpf_upgradeable_id, + true, + ); + + // Create the test new accounts, one for program and one for data + let new = Pubkey::new_unique(); + let (new_data, _) = Pubkey::find_program_address(&[new.as_ref()], &bpf_upgradeable_id); + let new_bytes = new_data.to_bytes().to_vec(); + let new_lamports = bank.get_minimum_balance_for_rent_exemption(new_bytes.len()); + let new_data_bytes = vec![6; 30]; + let new_data_lamports = bank.get_minimum_balance_for_rent_exemption(new_data_bytes.len()); + let new_account = test_program_replace_set_up_account( + &mut bank, + &new, + new_lamports, + new_bytes, + &bpf_upgradeable_id, + true, + ); + let new_data_account = test_program_replace_set_up_account( + &mut bank, + &new_data, + new_data_lamports, + new_data_bytes, + &bpf_upgradeable_id, + false, + ); + + // Derive the well-known PDA address for the old data account + let (old_data, _) = Pubkey::find_program_address(&[old.as_ref()], &bpf_upgradeable_id); + + // Create the data account if necessary + let old_data_account = if let Some(old_data_bytes) = maybe_old_data_bytes { + let old_data_lamports = bank.get_minimum_balance_for_rent_exemption(old_data_bytes.len()); + let old_data_account = test_program_replace_set_up_account( + &mut bank, + &old_data, + old_data_lamports, + old_data_bytes, + &bpf_upgradeable_id, + false, + ); + Some(old_data_account) + } else { + None + }; + + let original_capitalization = bank.capitalization(); + + // Attempt the replacement + bank.replace_empty_account_with_upgradeable_program( + &old, + &new, + "bank-apply_empty_account_replacement_for_program", + ); + + // Everything should be unchanged + assert_eq!(bank.get_account(&old).unwrap(), old_account); + if let Some(old_data_account) = old_data_account { + assert_eq!(bank.get_account(&old_data).unwrap(), old_data_account); + } + assert_eq!(bank.get_account(&new).unwrap(), new_account); + assert_eq!(bank.get_account(&new_data).unwrap(), new_data_account); + assert_eq!(bank.capitalization(), original_capitalization); +} + fn min_rent_exempt_balance_for_sysvars(bank: &Bank, sysvar_ids: &[Pubkey]) -> u64 { sysvar_ids .iter() From 453dc8a71cae2222dadb46ca2481d6ebb775244d Mon Sep 17 00:00:00 2001 From: Joe Date: Tue, 22 Aug 2023 08:30:55 -0600 Subject: [PATCH 15/33] more cleanup --- runtime/src/bank.rs | 63 ++++++++++++++++++++++----------------------- 1 file changed, 31 insertions(+), 32 deletions(-) diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index a09a414f9c753e..b694a90ab37265 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -8205,15 +8205,15 @@ impl Bank { } } - fn replace_account( + fn replace_account( &mut self, old_address: &Pubkey, new_address: &Pubkey, - old_account: Option<&O>, - new_account: &N, + old_account: Option<&U>, + new_account: &V, ) where - O: ReadableAccount + Sync + ZeroLamport, - N: ReadableAccount + Sync + ZeroLamport, + U: ReadableAccount + Sync + ZeroLamport, + V: ReadableAccount + Sync + ZeroLamport, { let (old_lamports, old_len) = match old_account { Some(old_account) => (old_account.lamports(), old_account.data().len()), @@ -8289,35 +8289,34 @@ impl Bank { // lamports for the empty account that will now house the // PDA if new_account.lamports() >= lamports { - if let Some(change_in_cap) = new_account.lamports().checked_sub(lamports) { - datapoint_info!(datapoint_name, ("slot", self.slot, i64)); - - // Replace the old data account with the new one - // If the old data account does not exist, it will be created - // If it does exist, it will be overwritten - self.replace_account( - &old_data_address, - &new_data_address, - self.get_account_with_fixed_root(&old_data_address).as_ref(), - &new_data_account, - ); + let change_in_cap = new_account.lamports().saturating_sub(lamports); + datapoint_info!(datapoint_name, ("slot", self.slot, i64)); + + // Replace the old data account with the new one + // If the old data account does not exist, it will be created + // If it does exist, it will be overwritten + self.replace_account( + &old_data_address, + &new_data_address, + self.get_account_with_fixed_root(&old_data_address).as_ref(), + &new_data_account, + ); - // Write the data account's PDA into the program account - let created_program_account = Account { - lamports, - data, - ..new_account.into() - }; - self.replace_account( - old_address, - new_address, - self.get_account_with_fixed_root(old_address).as_ref(), - &created_program_account, - ); + // Write the data account's PDA into the program account + let created_program_account = Account { + lamports, + data, + ..new_account.into() + }; + self.replace_account( + old_address, + new_address, + None::<&AccountSharedData>, + &created_program_account, + ); - // Any remaining lamports in the new program account are burnt - self.capitalization.fetch_sub(change_in_cap, Relaxed); - } + // Any remaining lamports in the new program account are burnt + self.capitalization.fetch_sub(change_in_cap, Relaxed); } } } From 7d3f8775a0ebe053a508d36fbeb1d0cf4d5579b2 Mon Sep 17 00:00:00 2001 From: Joe Date: Tue, 22 Aug 2023 12:37:45 -0600 Subject: [PATCH 16/33] add verifiably built program --- runtime/src/inline_feature_gate_program.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/runtime/src/inline_feature_gate_program.rs b/runtime/src/inline_feature_gate_program.rs index 3354c8f7e06e3e..125dc74df243d6 100644 --- a/runtime/src/inline_feature_gate_program.rs +++ b/runtime/src/inline_feature_gate_program.rs @@ -1,5 +1,5 @@ //! Contains replacement program IDs for the feature gate program pub(crate) mod noop_program { - solana_sdk::declare_id!("5Re2FUHmvSdem1KV2bU5GFvqZpP2MXj76yVhMKKbP45o"); + solana_sdk::declare_id!("2rqZsQBbacRbuAuTSuJ7n49UQT9fzes8RLggFcmB9YuN"); } From a6dd68e2aeec64c1f1b8312df1bb0ba157a59a6a Mon Sep 17 00:00:00 2001 From: Joe Date: Tue, 22 Aug 2023 15:43:37 -0600 Subject: [PATCH 17/33] update program account state --- runtime/src/bank.rs | 77 +++++++++++++++------------ runtime/src/bank/tests.rs | 107 ++++++++++++++++++++++---------------- 2 files changed, 106 insertions(+), 78 deletions(-) diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index b694a90ab37265..4038ae0fd0a039 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -8237,7 +8237,7 @@ impl Bank { /// Use to replace programs by feature activation #[allow(dead_code)] - fn replace_program_account( + fn replace_non_upgradeable_program_account( &mut self, old_address: &Pubkey, new_address: &Pubkey, @@ -8282,41 +8282,52 @@ impl Bank { // the known program-derived address (ie. `old_data_address`), // because if it exists, it will be overwritten if self.get_account_with_fixed_root(old_address).is_none() { - let data = old_data_address.as_ref().to_vec(); - let lamports = self.get_minimum_balance_for_rent_exemption(data.len()); - - // Make sure the new account has enough rent-exempt - // lamports for the empty account that will now house the - // PDA - if new_account.lamports() >= lamports { - let change_in_cap = new_account.lamports().saturating_sub(lamports); - datapoint_info!(datapoint_name, ("slot", self.slot, i64)); - - // Replace the old data account with the new one - // If the old data account does not exist, it will be created - // If it does exist, it will be overwritten - self.replace_account( - &old_data_address, - &new_data_address, - self.get_account_with_fixed_root(&old_data_address).as_ref(), - &new_data_account, - ); - - // Write the data account's PDA into the program account - let created_program_account = Account { + let lamports = self.get_minimum_balance_for_rent_exemption( + UpgradeableLoaderState::size_of_program(), + ); + let state = UpgradeableLoaderState::Program { + programdata_address: old_data_address, + }; + if let Ok(data_len) = bincode::serialized_size(&state) { + let mut created_program_account = Account { lamports, - data, - ..new_account.into() + data: vec![0u8; data_len as usize], + owner: bpf_loader_upgradeable::id(), + executable: true, + rent_epoch: new_account.rent_epoch(), }; - self.replace_account( - old_address, - new_address, - None::<&AccountSharedData>, - &created_program_account, - ); - // Any remaining lamports in the new program account are burnt - self.capitalization.fetch_sub(change_in_cap, Relaxed); + // Make sure the new account has enough rent-exempt + // lamports for the empty account that will now house the + // PDA, and we can properly serialize the program account's + // state + if new_account.lamports() >= lamports + && created_program_account.serialize_data(&state).is_ok() + { + let change_in_cap = new_account.lamports().saturating_sub(lamports); + datapoint_info!(datapoint_name, ("slot", self.slot, i64)); + + // Replace the old data account with the new one + // If the old data account does not exist, it will be created + // If it does exist, it will be overwritten + self.replace_account( + &old_data_address, + &new_data_address, + self.get_account_with_fixed_root(&old_data_address).as_ref(), + &new_data_account, + ); + + // Write the data account's PDA into the program account + self.replace_account( + old_address, + new_address, + None::<&AccountSharedData>, + &created_program_account, + ); + + // Any remaining lamports in the new program account are burnt + self.capitalization.fetch_sub(change_in_cap, Relaxed); + } } } } diff --git a/runtime/src/bank/tests.rs b/runtime/src/bank/tests.rs index 782345997cdd38..afeda0699d24be 100644 --- a/runtime/src/bank/tests.rs +++ b/runtime/src/bank/tests.rs @@ -8003,28 +8003,30 @@ fn test_compute_active_feature_set() { assert!(feature_set.is_active(&test_feature)); } -fn test_program_replace_set_up_account( +fn test_program_replace_set_up_account( bank: &mut Bank, pubkey: &Pubkey, lamports: u64, - data: Vec, + state: &T, owner: &Pubkey, executable: bool, ) -> AccountSharedData { - let new_account = AccountSharedData::from(Account { + let data_len = bincode::serialized_size(state).unwrap() as usize; + let mut new_account = AccountSharedData::from(Account { lamports, - data, owner: *owner, executable, + data: vec![0u8; data_len], ..Account::default() }); + new_account.serialize_data(state).unwrap(); bank.store_account_and_update_capitalization(pubkey, &new_account); assert_eq!(bank.get_balance(pubkey), lamports); new_account } #[test] -fn test_program_replace() { +fn test_replace_non_upgradeable_program_account() { // Non-upgradeable program // - Old: [Old program data] // - New: [*New program data] @@ -8035,25 +8037,26 @@ fn test_program_replace() { let mut bank = create_simple_test_bank(0); let old = Pubkey::new_unique(); - let old_bytes = vec![0, 0, 0, 0]; - let old_lamports = bank.get_minimum_balance_for_rent_exemption(old_bytes.len()); - test_program_replace_set_up_account(&mut bank, &old, old_lamports, old_bytes, &bpf_id, true); + let old_state = vec![0u8; 4]; + let old_lamports = bank.get_minimum_balance_for_rent_exemption(old_state.len()); + test_program_replace_set_up_account(&mut bank, &old, old_lamports, &old_state, &bpf_id, true); let new = Pubkey::new_unique(); - let new_bytes = vec![6; 30]; - let new_lamports = bank.get_minimum_balance_for_rent_exemption(new_bytes.len()); - test_program_replace_set_up_account( + let new_state = vec![6; 30]; + let new_lamports = bank.get_minimum_balance_for_rent_exemption(new_state.len()); + let check_new_account = test_program_replace_set_up_account( &mut bank, &new, new_lamports, - new_bytes.clone(), + &new_state, &bpf_id, true, ); + let check_data_account_data = check_new_account.data().to_vec(); let original_capitalization = bank.capitalization(); - bank.replace_program_account(&old, &new, "bank-apply_program_replacement"); + bank.replace_non_upgradeable_program_account(&old, &new, "bank-apply_program_replacement"); // Old program account balance is now the new program account's balance assert_eq!(bank.get_balance(&old), new_lamports); @@ -8064,7 +8067,7 @@ fn test_program_replace() { // Old program account now holds the new program data, ie: // - Old: [*New program data] let old_account = bank.get_account(&old).unwrap(); - assert_eq!(old_account.data(), &new_bytes); + assert_eq!(old_account.data(), &check_data_account_data); // Ownership & executable match the new program account assert_eq!(old_account.owner(), &bpf_id); @@ -8097,9 +8100,9 @@ fn test_program_replace() { Some(vec![4; 40]); "Native account _with_ corresponding data account" )] -fn test_replace_empty_account_success( +fn test_replace_empty_account_with_upgradeable_program_success( old: Pubkey, - maybe_old_data_bytes: Option>, // Inner data of the old program _data_ account + maybe_old_data_state: Option>, // Inner data of the old program _data_ account ) { // Ensures a program account and data account are created when replacing an // empty account, ie: @@ -8113,46 +8116,51 @@ fn test_replace_empty_account_success( // Create the test new accounts, one for program and one for data let new = Pubkey::new_unique(); let (new_data, _) = Pubkey::find_program_address(&[new.as_ref()], &bpf_upgradeable_id); - let new_bytes = new_data.to_bytes().to_vec(); - let new_lamports = bank.get_minimum_balance_for_rent_exemption(new_bytes.len()); - let new_data_bytes = vec![6; 30]; - let new_data_lamports = bank.get_minimum_balance_for_rent_exemption(new_data_bytes.len()); + let new_state = UpgradeableLoaderState::Program { + programdata_address: new_data, + }; + let new_lamports = + bank.get_minimum_balance_for_rent_exemption(UpgradeableLoaderState::size_of_program()); + let new_data_state = vec![6; 30]; + let new_data_lamports = bank.get_minimum_balance_for_rent_exemption(new_data_state.len()); test_program_replace_set_up_account( &mut bank, &new, new_lamports, - new_bytes, + &new_state, &bpf_upgradeable_id, true, ); - test_program_replace_set_up_account( + let check_new_data_account = test_program_replace_set_up_account( &mut bank, &new_data, new_data_lamports, - new_data_bytes.clone(), + &new_data_state, &bpf_upgradeable_id, false, ); + let check_data_account_data = check_new_data_account.data().to_vec(); // Derive the well-known PDA address for the old data account let (old_data, _) = Pubkey::find_program_address(&[old.as_ref()], &bpf_upgradeable_id); // Determine the lamports that will be burnt after the replacement - let burnt_after_rent = if let Some(old_data_bytes) = maybe_old_data_bytes { + let burnt_after_rent = if let Some(old_data_state) = maybe_old_data_state { // Create the data account if necessary - let old_data_lamports = bank.get_minimum_balance_for_rent_exemption(old_data_bytes.len()); + let old_data_lamports = bank.get_minimum_balance_for_rent_exemption(old_data_state.len()); test_program_replace_set_up_account( &mut bank, &old_data, old_data_lamports, - old_data_bytes, + &old_data_state, &bpf_upgradeable_id, false, ); old_data_lamports + new_lamports - - bank.get_minimum_balance_for_rent_exemption(old_data.to_bytes().len()) + - bank.get_minimum_balance_for_rent_exemption(UpgradeableLoaderState::size_of_program()) } else { - new_lamports - bank.get_minimum_balance_for_rent_exemption(old_data.to_bytes().len()) + new_lamports + - bank.get_minimum_balance_for_rent_exemption(UpgradeableLoaderState::size_of_program()) }; let original_capitalization = bank.capitalization(); @@ -8168,7 +8176,7 @@ fn test_replace_empty_account_success( // for the PDA assert_eq!( bank.get_balance(&old), - bank.get_minimum_balance_for_rent_exemption(old_data.to_bytes().len()) + bank.get_minimum_balance_for_rent_exemption(UpgradeableLoaderState::size_of_program()), ); // Old data account was created, now holds the new data account's balance @@ -8181,12 +8189,18 @@ fn test_replace_empty_account_success( // Old program account holds the PDA, ie: // - Old: PDA(OldData) let old_account = bank.get_account(&old).unwrap(); - assert_eq!(old_account.data(), &old_data.to_bytes().to_vec()); + assert_eq!( + old_account.data(), + &bincode::serialize(&UpgradeableLoaderState::Program { + programdata_address: old_data + }) + .unwrap(), + ); // Old data account holds the new data, ie: // - OldData: [*New program data] let old_data_account = bank.get_account(&old_data).unwrap(); - assert_eq!(old_data_account.data(), &new_data_bytes); + assert_eq!(old_data_account.data(), &check_data_account_data); // Ownership & executable match the new program accounts assert_eq!(old_account.owner(), &bpf_upgradeable_id); @@ -8210,8 +8224,8 @@ fn test_replace_empty_account_success( Some(vec![4; 40]); "Existing account _with_ corresponding data account" )] -fn test_replace_empty_account_fail_when_account_exists( - maybe_old_data_bytes: Option>, // Inner data of the old program _data_ account +fn test_replace_empty_account_with_upgradeable_program_fail_when_account_exists( + maybe_old_data_state: Option>, // Inner data of the old program _data_ account ) { // Should not be allowed to execute replacement let bpf_upgradeable_id = bpf_loader_upgradeable::id(); @@ -8219,13 +8233,13 @@ fn test_replace_empty_account_fail_when_account_exists( // Create the test old account with some arbitrary data and lamports balance let old = Pubkey::new_unique(); - let old_bytes = vec![0, 0, 0, 0]; // Arbitrary bytes, doesn't matter - let old_lamports = bank.get_minimum_balance_for_rent_exemption(old_bytes.len()); + let old_state = vec![0, 0, 0, 0]; // Arbitrary bytes, doesn't matter + let old_lamports = bank.get_minimum_balance_for_rent_exemption(old_state.len()); let old_account = test_program_replace_set_up_account( &mut bank, &old, old_lamports, - old_bytes, + &old_state, &bpf_upgradeable_id, true, ); @@ -8233,15 +8247,18 @@ fn test_replace_empty_account_fail_when_account_exists( // Create the test new accounts, one for program and one for data let new = Pubkey::new_unique(); let (new_data, _) = Pubkey::find_program_address(&[new.as_ref()], &bpf_upgradeable_id); - let new_bytes = new_data.to_bytes().to_vec(); - let new_lamports = bank.get_minimum_balance_for_rent_exemption(new_bytes.len()); - let new_data_bytes = vec![6; 30]; - let new_data_lamports = bank.get_minimum_balance_for_rent_exemption(new_data_bytes.len()); + let new_state = UpgradeableLoaderState::Program { + programdata_address: new_data, + }; + let new_lamports = + bank.get_minimum_balance_for_rent_exemption(UpgradeableLoaderState::size_of_program()); + let new_data_state = vec![6; 30]; + let new_data_lamports = bank.get_minimum_balance_for_rent_exemption(new_data_state.len()); let new_account = test_program_replace_set_up_account( &mut bank, &new, new_lamports, - new_bytes, + &new_state, &bpf_upgradeable_id, true, ); @@ -8249,7 +8266,7 @@ fn test_replace_empty_account_fail_when_account_exists( &mut bank, &new_data, new_data_lamports, - new_data_bytes, + &new_data_state, &bpf_upgradeable_id, false, ); @@ -8258,13 +8275,13 @@ fn test_replace_empty_account_fail_when_account_exists( let (old_data, _) = Pubkey::find_program_address(&[old.as_ref()], &bpf_upgradeable_id); // Create the data account if necessary - let old_data_account = if let Some(old_data_bytes) = maybe_old_data_bytes { - let old_data_lamports = bank.get_minimum_balance_for_rent_exemption(old_data_bytes.len()); + let old_data_account = if let Some(old_data_state) = maybe_old_data_state { + let old_data_lamports = bank.get_minimum_balance_for_rent_exemption(old_data_state.len()); let old_data_account = test_program_replace_set_up_account( &mut bank, &old_data, old_data_lamports, - old_data_bytes, + &old_data_state, &bpf_upgradeable_id, false, ); From 59a4132321dbf87eb58e2866cdcbd8590e927cca Mon Sep 17 00:00:00 2001 From: Joe Date: Tue, 22 Aug 2023 22:28:21 -0600 Subject: [PATCH 18/33] cleaned up serializing logic --- runtime/src/bank.rs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 4038ae0fd0a039..e710ca313e8c79 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -8288,10 +8288,10 @@ impl Bank { let state = UpgradeableLoaderState::Program { programdata_address: old_data_address, }; - if let Ok(data_len) = bincode::serialized_size(&state) { - let mut created_program_account = Account { + if let Ok(data) = bincode::serialize(&state) { + let created_program_account = Account { lamports, - data: vec![0u8; data_len as usize], + data, owner: bpf_loader_upgradeable::id(), executable: true, rent_epoch: new_account.rent_epoch(), @@ -8301,11 +8301,9 @@ impl Bank { // lamports for the empty account that will now house the // PDA, and we can properly serialize the program account's // state - if new_account.lamports() >= lamports - && created_program_account.serialize_data(&state).is_ok() - { - let change_in_cap = new_account.lamports().saturating_sub(lamports); + if new_account.lamports() >= lamports { datapoint_info!(datapoint_name, ("slot", self.slot, i64)); + let change_in_cap = new_account.lamports().saturating_sub(lamports); // Replace the old data account with the new one // If the old data account does not exist, it will be created From acfa993fdf4fdde7c560078b6d2fad0e7c4ad5ab Mon Sep 17 00:00:00 2001 From: Joe Date: Fri, 15 Sep 2023 12:34:23 -0400 Subject: [PATCH 19/33] use full word capitalization --- runtime/src/bank.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index e710ca313e8c79..c550f12091c508 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -8303,7 +8303,7 @@ impl Bank { // state if new_account.lamports() >= lamports { datapoint_info!(datapoint_name, ("slot", self.slot, i64)); - let change_in_cap = new_account.lamports().saturating_sub(lamports); + let change_in_capitalization = new_account.lamports().saturating_sub(lamports); // Replace the old data account with the new one // If the old data account does not exist, it will be created @@ -8324,7 +8324,7 @@ impl Bank { ); // Any remaining lamports in the new program account are burnt - self.capitalization.fetch_sub(change_in_cap, Relaxed); + self.capitalization.fetch_sub(change_in_capitalization, Relaxed); } } } From 6ef83538224491e0b3b12ddf2a060bd6ae089f0c Mon Sep 17 00:00:00 2001 From: Joe Date: Fri, 15 Sep 2023 13:32:47 -0400 Subject: [PATCH 20/33] rename old & new to dst & src --- runtime/src/bank.rs | 103 +++++++-------- runtime/src/bank/tests.rs | 256 +++++++++++++++++++------------------- 2 files changed, 182 insertions(+), 177 deletions(-) diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index c550f12091c508..f3e7e7fb611c84 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -8205,55 +8205,60 @@ impl Bank { } } - fn replace_account( + /// Moves one account in place of another + /// `src`: the program to replace with + /// `dst`: the program to be replaced + fn move_account( &mut self, - old_address: &Pubkey, - new_address: &Pubkey, - old_account: Option<&U>, - new_account: &V, + dst_address: &Pubkey, + src_address: &Pubkey, + dst_account: Option<&U>, + src_account: &V, ) where U: ReadableAccount + Sync + ZeroLamport, V: ReadableAccount + Sync + ZeroLamport, { - let (old_lamports, old_len) = match old_account { - Some(old_account) => (old_account.lamports(), old_account.data().len()), + let (dst_lamports, dst_len) = match dst_account { + Some(dst_account) => (dst_account.lamports(), dst_account.data().len()), None => (0, 0), }; - // Burn lamports in the old account - self.capitalization.fetch_sub(old_lamports, Relaxed); + // Burn lamports in the destination account + self.capitalization.fetch_sub(dst_lamports, Relaxed); - // Transfer new account to old account - self.store_account(old_address, new_account); + // Transfer source account to destination account + self.store_account(dst_address, src_account); - // Clear new account - self.store_account(new_address, &AccountSharedData::default()); + // Clear source account + self.store_account(src_address, &AccountSharedData::default()); self.calculate_and_update_accounts_data_size_delta_off_chain( - old_len, - new_account.data().len(), + dst_len, + src_account.data().len(), ); } /// Use to replace programs by feature activation + /// `src`: the program to replace with + /// `dst`: the program to be replaced #[allow(dead_code)] fn replace_non_upgradeable_program_account( &mut self, - old_address: &Pubkey, - new_address: &Pubkey, + dst_address: &Pubkey, + src_address: &Pubkey, datapoint_name: &'static str, ) { - if let Some(old_account) = self.get_account_with_fixed_root(old_address) { - if let Some(new_account) = self.get_account_with_fixed_root(new_address) { + if let Some(dst_account) = self.get_account_with_fixed_root(dst_address) { + if let Some(src_account) = self.get_account_with_fixed_root(src_address) { datapoint_info!(datapoint_name, ("slot", self.slot, i64)); - self.replace_account(old_address, new_address, Some(&old_account), &new_account); + self.move_account(dst_address, src_address, Some(&dst_account), &src_account); // Unload a program from the bank's cache self.loaded_programs_cache .write() .unwrap() - .remove_programs([*old_address].into_iter()); + .remove_programs([*dst_address].into_iter()); } } } @@ -8261,32 +8266,32 @@ impl Bank { /// Use to replace an empty account with a program by feature activation fn replace_empty_account_with_upgradeable_program( &mut self, - old_address: &Pubkey, - new_address: &Pubkey, + dst_address: &Pubkey, + src_address: &Pubkey, datapoint_name: &'static str, ) { // Must be attempting to replace an empty account with a program // account _and_ data account - if let Some(new_account) = self.get_account_with_fixed_root(new_address) { - let (old_data_address, _) = Pubkey::find_program_address( - &[old_address.as_ref()], + if let Some(src_account) = self.get_account_with_fixed_root(src_address) { + let (dst_data_address, _) = Pubkey::find_program_address( + &[dst_address.as_ref()], &bpf_loader_upgradeable::id(), ); - let (new_data_address, _) = Pubkey::find_program_address( - &[new_address.as_ref()], + let (src_data_address, _) = Pubkey::find_program_address( + &[src_address.as_ref()], &bpf_loader_upgradeable::id(), ); - if let Some(new_data_account) = self.get_account_with_fixed_root(&new_data_address) { - // Make sure the old account is empty + if let Some(src_data_account) = self.get_account_with_fixed_root(&src_data_address) { + // Make sure the destination account is empty // We aren't going to check that there isn't a data account at - // the known program-derived address (ie. `old_data_address`), + // the known program-derived address (ie. `dst_data_address`), // because if it exists, it will be overwritten - if self.get_account_with_fixed_root(old_address).is_none() { + if self.get_account_with_fixed_root(dst_address).is_none() { let lamports = self.get_minimum_balance_for_rent_exemption( UpgradeableLoaderState::size_of_program(), ); let state = UpgradeableLoaderState::Program { - programdata_address: old_data_address, + programdata_address: dst_data_address, }; if let Ok(data) = bincode::serialize(&state) { let created_program_account = Account { @@ -8294,36 +8299,36 @@ impl Bank { data, owner: bpf_loader_upgradeable::id(), executable: true, - rent_epoch: new_account.rent_epoch(), + rent_epoch: src_account.rent_epoch(), }; - // Make sure the new account has enough rent-exempt + // Make sure the source account has enough rent-exempt // lamports for the empty account that will now house the // PDA, and we can properly serialize the program account's // state - if new_account.lamports() >= lamports { + if src_account.lamports() >= lamports { datapoint_info!(datapoint_name, ("slot", self.slot, i64)); - let change_in_capitalization = new_account.lamports().saturating_sub(lamports); + let change_in_capitalization = src_account.lamports().saturating_sub(lamports); - // Replace the old data account with the new one - // If the old data account does not exist, it will be created + // Replace the destination data account with the source one + // If the destination data account does not exist, it will be created // If it does exist, it will be overwritten - self.replace_account( - &old_data_address, - &new_data_address, - self.get_account_with_fixed_root(&old_data_address).as_ref(), - &new_data_account, + self.move_account( + &dst_data_address, + &src_data_address, + self.get_account_with_fixed_root(&dst_data_address).as_ref(), + &src_data_account, ); - // Write the data account's PDA into the program account - self.replace_account( - old_address, - new_address, + // Write the source data account's PDA into the destination program account + self.move_account( + dst_address, + src_address, None::<&AccountSharedData>, &created_program_account, ); - // Any remaining lamports in the new program account are burnt + // Any remaining lamports in the source program account are burnt self.capitalization.fetch_sub(change_in_capitalization, Relaxed); } } diff --git a/runtime/src/bank/tests.rs b/runtime/src/bank/tests.rs index afeda0699d24be..6abbd1596bb070 100644 --- a/runtime/src/bank/tests.rs +++ b/runtime/src/bank/tests.rs @@ -8012,71 +8012,71 @@ fn test_program_replace_set_up_account( executable: bool, ) -> AccountSharedData { let data_len = bincode::serialized_size(state).unwrap() as usize; - let mut new_account = AccountSharedData::from(Account { + let mut account = AccountSharedData::from(Account { lamports, owner: *owner, executable, data: vec![0u8; data_len], ..Account::default() }); - new_account.serialize_data(state).unwrap(); - bank.store_account_and_update_capitalization(pubkey, &new_account); + account.serialize_data(state).unwrap(); + bank.store_account_and_update_capitalization(pubkey, &account); assert_eq!(bank.get_balance(pubkey), lamports); - new_account + account } #[test] fn test_replace_non_upgradeable_program_account() { // Non-upgradeable program - // - Old: [Old program data] - // - New: [*New program data] + // - Destination: [Destination program data] + // - Source: [*Source program data] // - // Should replace the old program account with the new program account: - // - Old: [*New program data] + // Should replace the destination program account with the source program account: + // - Destination: [*Source program data] let bpf_id = bpf_loader::id(); let mut bank = create_simple_test_bank(0); - let old = Pubkey::new_unique(); - let old_state = vec![0u8; 4]; - let old_lamports = bank.get_minimum_balance_for_rent_exemption(old_state.len()); - test_program_replace_set_up_account(&mut bank, &old, old_lamports, &old_state, &bpf_id, true); + let dst = Pubkey::new_unique(); + let dst_state = vec![0u8; 4]; + let dst_lamports = bank.get_minimum_balance_for_rent_exemption(dst_state.len()); + test_program_replace_set_up_account(&mut bank, &dst, dst_lamports, &dst_state, &bpf_id, true); - let new = Pubkey::new_unique(); - let new_state = vec![6; 30]; - let new_lamports = bank.get_minimum_balance_for_rent_exemption(new_state.len()); - let check_new_account = test_program_replace_set_up_account( + let src = Pubkey::new_unique(); + let src_state = vec![6; 30]; + let src_lamports = bank.get_minimum_balance_for_rent_exemption(src_state.len()); + let check_src_account = test_program_replace_set_up_account( &mut bank, - &new, - new_lamports, - &new_state, + &src, + src_lamports, + &src_state, &bpf_id, true, ); - let check_data_account_data = check_new_account.data().to_vec(); + let check_data_account_data = check_src_account.data().to_vec(); let original_capitalization = bank.capitalization(); - bank.replace_non_upgradeable_program_account(&old, &new, "bank-apply_program_replacement"); + bank.replace_non_upgradeable_program_account(&dst, &src, "bank-apply_program_replacement"); - // Old program account balance is now the new program account's balance - assert_eq!(bank.get_balance(&old), new_lamports); + // Destination program account balance is now the source program account's balance + assert_eq!(bank.get_balance(&dst), src_lamports); - // New program account is now empty - assert_eq!(bank.get_balance(&new), 0); + // Source program account is now empty + assert_eq!(bank.get_balance(&src), 0); - // Old program account now holds the new program data, ie: - // - Old: [*New program data] - let old_account = bank.get_account(&old).unwrap(); - assert_eq!(old_account.data(), &check_data_account_data); + // Destination program account now holds the source program data, ie: + // - Destination: [*Source program data] + let dst_account = bank.get_account(&dst).unwrap(); + assert_eq!(dst_account.data(), &check_data_account_data); - // Ownership & executable match the new program account - assert_eq!(old_account.owner(), &bpf_id); - assert!(old_account.executable()); + // Ownership & executable match the source program account + assert_eq!(dst_account.owner(), &bpf_id); + assert!(dst_account.executable()); - // Lamports from the new program account were burnt + // Lamports from the source program account were burnt assert_eq!( bank.capitalization(), - original_capitalization - old_lamports + original_capitalization - dst_lamports ); } @@ -8101,65 +8101,65 @@ fn test_replace_non_upgradeable_program_account() { "Native account _with_ corresponding data account" )] fn test_replace_empty_account_with_upgradeable_program_success( - old: Pubkey, - maybe_old_data_state: Option>, // Inner data of the old program _data_ account + dst: Pubkey, + maybe_dst_data_state: Option>, // Inner data of the destination program _data_ account ) { // Ensures a program account and data account are created when replacing an // empty account, ie: - // - Old: PDA(OldData) - // - OldData: [Old program data] + // - Destination: PDA(DestinationData) + // - DestinationData: [Destination program data] // - // If the old data account exists, it will be overwritten + // If the destination data account exists, it will be overwritten let bpf_upgradeable_id = bpf_loader_upgradeable::id(); let mut bank = create_simple_test_bank(0); - // Create the test new accounts, one for program and one for data - let new = Pubkey::new_unique(); - let (new_data, _) = Pubkey::find_program_address(&[new.as_ref()], &bpf_upgradeable_id); - let new_state = UpgradeableLoaderState::Program { - programdata_address: new_data, + // Create the test source accounts, one for program and one for data + let src = Pubkey::new_unique(); + let (src_data, _) = Pubkey::find_program_address(&[src.as_ref()], &bpf_upgradeable_id); + let src_state = UpgradeableLoaderState::Program { + programdata_address: src_data, }; - let new_lamports = + let src_lamports = bank.get_minimum_balance_for_rent_exemption(UpgradeableLoaderState::size_of_program()); - let new_data_state = vec![6; 30]; - let new_data_lamports = bank.get_minimum_balance_for_rent_exemption(new_data_state.len()); + let src_data_state = vec![6; 30]; + let src_data_lamports = bank.get_minimum_balance_for_rent_exemption(src_data_state.len()); test_program_replace_set_up_account( &mut bank, - &new, - new_lamports, - &new_state, + &src, + src_lamports, + &src_state, &bpf_upgradeable_id, true, ); - let check_new_data_account = test_program_replace_set_up_account( + let check_src_data_account = test_program_replace_set_up_account( &mut bank, - &new_data, - new_data_lamports, - &new_data_state, + &src_data, + src_data_lamports, + &src_data_state, &bpf_upgradeable_id, false, ); - let check_data_account_data = check_new_data_account.data().to_vec(); + let check_data_account_data = check_src_data_account.data().to_vec(); - // Derive the well-known PDA address for the old data account - let (old_data, _) = Pubkey::find_program_address(&[old.as_ref()], &bpf_upgradeable_id); + // Derive the well-known PDA address for the destination data account + let (dst_data, _) = Pubkey::find_program_address(&[dst.as_ref()], &bpf_upgradeable_id); // Determine the lamports that will be burnt after the replacement - let burnt_after_rent = if let Some(old_data_state) = maybe_old_data_state { + let burnt_after_rent = if let Some(dst_data_state) = maybe_dst_data_state { // Create the data account if necessary - let old_data_lamports = bank.get_minimum_balance_for_rent_exemption(old_data_state.len()); + let dst_data_lamports = bank.get_minimum_balance_for_rent_exemption(dst_data_state.len()); test_program_replace_set_up_account( &mut bank, - &old_data, - old_data_lamports, - &old_data_state, + &dst_data, + dst_data_lamports, + &dst_data_state, &bpf_upgradeable_id, false, ); - old_data_lamports + new_lamports + dst_data_lamports + src_lamports - bank.get_minimum_balance_for_rent_exemption(UpgradeableLoaderState::size_of_program()) } else { - new_lamports + src_lamports - bank.get_minimum_balance_for_rent_exemption(UpgradeableLoaderState::size_of_program()) }; @@ -8167,46 +8167,46 @@ fn test_replace_empty_account_with_upgradeable_program_success( // Do the replacement bank.replace_empty_account_with_upgradeable_program( - &old, - &new, + &dst, + &src, "bank-apply_empty_account_replacement_for_program", ); - // Old program account was created and funded to pay for minimum rent + // Destination program account was created and funded to pay for minimum rent // for the PDA assert_eq!( - bank.get_balance(&old), + bank.get_balance(&dst), bank.get_minimum_balance_for_rent_exemption(UpgradeableLoaderState::size_of_program()), ); - // Old data account was created, now holds the new data account's balance - assert_eq!(bank.get_balance(&old_data), new_data_lamports); + // Destination data account was created, now holds the source data account's balance + assert_eq!(bank.get_balance(&dst_data), src_data_lamports); - // New program accounts are now empty - assert_eq!(bank.get_balance(&new), 0); - assert_eq!(bank.get_balance(&new_data), 0); + // Source program accounts are now empty + assert_eq!(bank.get_balance(&src), 0); + assert_eq!(bank.get_balance(&src_data), 0); - // Old program account holds the PDA, ie: - // - Old: PDA(OldData) - let old_account = bank.get_account(&old).unwrap(); + // Destination program account holds the PDA, ie: + // - Destination: PDA(DestinationData) + let dst_account = bank.get_account(&dst).unwrap(); assert_eq!( - old_account.data(), + dst_account.data(), &bincode::serialize(&UpgradeableLoaderState::Program { - programdata_address: old_data + programdata_address: dst_data }) .unwrap(), ); - // Old data account holds the new data, ie: - // - OldData: [*New program data] - let old_data_account = bank.get_account(&old_data).unwrap(); - assert_eq!(old_data_account.data(), &check_data_account_data); + // Destination data account holds the source data, ie: + // - DestinationData: [*Source program data] + let dst_data_account = bank.get_account(&dst_data).unwrap(); + assert_eq!(dst_data_account.data(), &check_data_account_data); - // Ownership & executable match the new program accounts - assert_eq!(old_account.owner(), &bpf_upgradeable_id); - assert!(old_account.executable()); - assert_eq!(old_data_account.owner(), &bpf_upgradeable_id); - assert!(!old_data_account.executable()); + // Ownership & executable match the source program accounts + assert_eq!(dst_account.owner(), &bpf_upgradeable_id); + assert!(dst_account.executable()); + assert_eq!(dst_data_account.owner(), &bpf_upgradeable_id); + assert!(!dst_data_account.executable()); // The remaining lamports from both program accounts minus the rent-exempt // minimum were burnt @@ -8225,67 +8225,67 @@ fn test_replace_empty_account_with_upgradeable_program_success( "Existing account _with_ corresponding data account" )] fn test_replace_empty_account_with_upgradeable_program_fail_when_account_exists( - maybe_old_data_state: Option>, // Inner data of the old program _data_ account + maybe_dst_data_state: Option>, // Inner data of the destination program _data_ account ) { // Should not be allowed to execute replacement let bpf_upgradeable_id = bpf_loader_upgradeable::id(); let mut bank = create_simple_test_bank(0); - // Create the test old account with some arbitrary data and lamports balance - let old = Pubkey::new_unique(); - let old_state = vec![0, 0, 0, 0]; // Arbitrary bytes, doesn't matter - let old_lamports = bank.get_minimum_balance_for_rent_exemption(old_state.len()); - let old_account = test_program_replace_set_up_account( + // Create the test destination account with some arbitrary data and lamports balance + let dst = Pubkey::new_unique(); + let dst_state = vec![0, 0, 0, 0]; // Arbitrary bytes, doesn't matter + let dst_lamports = bank.get_minimum_balance_for_rent_exemption(dst_state.len()); + let dst_account = test_program_replace_set_up_account( &mut bank, - &old, - old_lamports, - &old_state, + &dst, + dst_lamports, + &dst_state, &bpf_upgradeable_id, true, ); - // Create the test new accounts, one for program and one for data - let new = Pubkey::new_unique(); - let (new_data, _) = Pubkey::find_program_address(&[new.as_ref()], &bpf_upgradeable_id); - let new_state = UpgradeableLoaderState::Program { - programdata_address: new_data, + // Create the test source accounts, one for program and one for data + let src = Pubkey::new_unique(); + let (src_data, _) = Pubkey::find_program_address(&[src.as_ref()], &bpf_upgradeable_id); + let src_state = UpgradeableLoaderState::Program { + programdata_address: src_data, }; - let new_lamports = + let src_lamports = bank.get_minimum_balance_for_rent_exemption(UpgradeableLoaderState::size_of_program()); - let new_data_state = vec![6; 30]; - let new_data_lamports = bank.get_minimum_balance_for_rent_exemption(new_data_state.len()); - let new_account = test_program_replace_set_up_account( + let src_data_state = vec![6; 30]; + let src_data_lamports = bank.get_minimum_balance_for_rent_exemption(src_data_state.len()); + let src_account = test_program_replace_set_up_account( &mut bank, - &new, - new_lamports, - &new_state, + &src, + src_lamports, + &src_state, &bpf_upgradeable_id, true, ); - let new_data_account = test_program_replace_set_up_account( + let src_data_account = test_program_replace_set_up_account( &mut bank, - &new_data, - new_data_lamports, - &new_data_state, + &src_data, + src_data_lamports, + &src_data_state, &bpf_upgradeable_id, false, ); - // Derive the well-known PDA address for the old data account - let (old_data, _) = Pubkey::find_program_address(&[old.as_ref()], &bpf_upgradeable_id); + // Derive the well-known PDA address for the destination data account + let (dst_data, _) = Pubkey::find_program_address(&[dst.as_ref()], &bpf_upgradeable_id); // Create the data account if necessary - let old_data_account = if let Some(old_data_state) = maybe_old_data_state { - let old_data_lamports = bank.get_minimum_balance_for_rent_exemption(old_data_state.len()); - let old_data_account = test_program_replace_set_up_account( + let dst_data_account = if let Some(dst_data_state) = maybe_dst_data_state { + let dst_data_lamports = bank.get_minimum_balance_for_rent_exemption(dst_data_state.len()); + let dst_data_account = test_program_replace_set_up_account( &mut bank, - &old_data, - old_data_lamports, - &old_data_state, + &dst_data, + dst_data_lamports, + &dst_data_state, &bpf_upgradeable_id, false, ); - Some(old_data_account) + Some(dst_data_account) } else { None }; @@ -8294,18 +8294,18 @@ fn test_replace_empty_account_with_upgradeable_program_fail_when_account_exists( // Attempt the replacement bank.replace_empty_account_with_upgradeable_program( - &old, - &new, + &dst, + &src, "bank-apply_empty_account_replacement_for_program", ); // Everything should be unchanged - assert_eq!(bank.get_account(&old).unwrap(), old_account); - if let Some(old_data_account) = old_data_account { - assert_eq!(bank.get_account(&old_data).unwrap(), old_data_account); + assert_eq!(bank.get_account(&dst).unwrap(), dst_account); + if let Some(dst_data_account) = dst_data_account { + assert_eq!(bank.get_account(&dst_data).unwrap(), dst_data_account); } - assert_eq!(bank.get_account(&new).unwrap(), new_account); - assert_eq!(bank.get_account(&new_data).unwrap(), new_data_account); + assert_eq!(bank.get_account(&src).unwrap(), src_account); + assert_eq!(bank.get_account(&src_data).unwrap(), src_data_account); assert_eq!(bank.capitalization(), original_capitalization); } From 069dd538ba391a2b72e5d5ca511540d204c5203c Mon Sep 17 00:00:00 2001 From: Joe Date: Fri, 15 Sep 2023 13:35:31 -0400 Subject: [PATCH 21/33] swap src and dst in parameters --- runtime/src/bank.rs | 18 +++++++++--------- runtime/src/bank/tests.rs | 6 +++--- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index f3e7e7fb611c84..bdb81722cb9b59 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -8210,10 +8210,10 @@ impl Bank { /// `dst`: the program to be replaced fn move_account( &mut self, - dst_address: &Pubkey, src_address: &Pubkey, - dst_account: Option<&U>, src_account: &V, + dst_address: &Pubkey, + dst_account: Option<&U>, ) where U: ReadableAccount + Sync + ZeroLamport, V: ReadableAccount + Sync + ZeroLamport, @@ -8244,15 +8244,15 @@ impl Bank { #[allow(dead_code)] fn replace_non_upgradeable_program_account( &mut self, - dst_address: &Pubkey, src_address: &Pubkey, + dst_address: &Pubkey, datapoint_name: &'static str, ) { if let Some(dst_account) = self.get_account_with_fixed_root(dst_address) { if let Some(src_account) = self.get_account_with_fixed_root(src_address) { datapoint_info!(datapoint_name, ("slot", self.slot, i64)); - self.move_account(dst_address, src_address, Some(&dst_account), &src_account); + self.move_account(src_address, &src_account, dst_address, Some(&dst_account)); // Unload a program from the bank's cache self.loaded_programs_cache @@ -8266,8 +8266,8 @@ impl Bank { /// Use to replace an empty account with a program by feature activation fn replace_empty_account_with_upgradeable_program( &mut self, - dst_address: &Pubkey, src_address: &Pubkey, + dst_address: &Pubkey, datapoint_name: &'static str, ) { // Must be attempting to replace an empty account with a program @@ -8314,18 +8314,18 @@ impl Bank { // If the destination data account does not exist, it will be created // If it does exist, it will be overwritten self.move_account( - &dst_data_address, &src_data_address, - self.get_account_with_fixed_root(&dst_data_address).as_ref(), &src_data_account, + &dst_data_address, + self.get_account_with_fixed_root(&dst_data_address).as_ref(), ); // Write the source data account's PDA into the destination program account self.move_account( - dst_address, src_address, - None::<&AccountSharedData>, &created_program_account, + dst_address, + None::<&AccountSharedData>, ); // Any remaining lamports in the source program account are burnt diff --git a/runtime/src/bank/tests.rs b/runtime/src/bank/tests.rs index 6abbd1596bb070..a021bb7ab7c853 100644 --- a/runtime/src/bank/tests.rs +++ b/runtime/src/bank/tests.rs @@ -8056,7 +8056,7 @@ fn test_replace_non_upgradeable_program_account() { let original_capitalization = bank.capitalization(); - bank.replace_non_upgradeable_program_account(&dst, &src, "bank-apply_program_replacement"); + bank.replace_non_upgradeable_program_account(&src, &dst, "bank-apply_program_replacement"); // Destination program account balance is now the source program account's balance assert_eq!(bank.get_balance(&dst), src_lamports); @@ -8167,8 +8167,8 @@ fn test_replace_empty_account_with_upgradeable_program_success( // Do the replacement bank.replace_empty_account_with_upgradeable_program( - &dst, &src, + &dst, "bank-apply_empty_account_replacement_for_program", ); @@ -8294,8 +8294,8 @@ fn test_replace_empty_account_with_upgradeable_program_fail_when_account_exists( // Attempt the replacement bank.replace_empty_account_with_upgradeable_program( - &dst, &src, + &dst, "bank-apply_empty_account_replacement_for_program", ); From f04489050521e66f70cb884c7afc68eebbd49985 Mon Sep 17 00:00:00 2001 From: Joe Date: Fri, 15 Sep 2023 13:54:52 -0400 Subject: [PATCH 22/33] add warnings and errors --- runtime/src/bank.rs | 96 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 94 insertions(+), 2 deletions(-) diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index bdb81722cb9b59..139222ef6ce005 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -8259,7 +8259,30 @@ impl Bank { .write() .unwrap() .remove_programs([*dst_address].into_iter()); + } else { + warn!("Unable to find source program {}", src_address); + datapoint_warn!( + datapoint_name, + ("slot", self.slot(), i64), + ("source program pubkey", src_address.to_string(), String), + ( + "destination program pubkey", + dst_address.to_string(), + String + ), + ); } + } else { + warn!("Unable to find destination program {}", dst_address); + datapoint_warn!( + datapoint_name, + ("slot", self.slot(), i64), + ( + "destination program pubkey", + dst_address.to_string(), + String + ), + ); } } @@ -8308,7 +8331,8 @@ impl Bank { // state if src_account.lamports() >= lamports { datapoint_info!(datapoint_name, ("slot", self.slot, i64)); - let change_in_capitalization = src_account.lamports().saturating_sub(lamports); + let change_in_capitalization = + src_account.lamports().saturating_sub(lamports); // Replace the destination data account with the source one // If the destination data account does not exist, it will be created @@ -8329,11 +8353,79 @@ impl Bank { ); // Any remaining lamports in the source program account are burnt - self.capitalization.fetch_sub(change_in_capitalization, Relaxed); + self.capitalization + .fetch_sub(change_in_capitalization, Relaxed); } + } else { + error!("Unable to serialize program account state"); + datapoint_error!( + datapoint_name, + ("slot", self.slot(), i64), + ("source program pubkey", src_address.to_string(), String), + ( + "source data account pubkey", + src_data_address.to_string(), + String + ), + ( + "destination program pubkey", + dst_address.to_string(), + String + ), + ( + "destination data account pubkey", + dst_data_address.to_string(), + String + ), + ( + "destination data account pubkey", + format!( + "UpgradeableLoaderState::Program with data address {}", + dst_data_address + ), + String + ), + ); } } + } else { + warn!( + "Unable to find data account for source program {}", + src_address + ); + datapoint_warn!( + datapoint_name, + ("slot", self.slot(), i64), + ("source program pubkey", src_address.to_string(), String), + ( + "source data account pubkey", + src_data_address.to_string(), + String + ), + ( + "destination program pubkey", + dst_address.to_string(), + String + ), + ( + "destination data account pubkey", + dst_data_address.to_string(), + String + ), + ); } + } else { + warn!("Unable to find source program {}", src_address); + datapoint_warn!( + datapoint_name, + ("slot", self.slot(), i64), + ("source program pubkey", src_address.to_string(), String), + ( + "destination program pubkey", + dst_address.to_string(), + String + ), + ); } } From 1e7b4e5fd1e23ea44e442bb7ddd813df5f74161d Mon Sep 17 00:00:00 2001 From: Joe Date: Wed, 20 Sep 2023 18:11:37 +0200 Subject: [PATCH 23/33] rename feature to programify --- runtime/src/bank.rs | 2 +- sdk/src/feature_set.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 139222ef6ce005..39703bbd1dc1fd 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -8056,7 +8056,7 @@ impl Bank { self.apply_updated_hashes_per_tick(DEFAULT_HASHES_PER_TICK); } - if new_feature_activations.contains(&feature_set::feature_gate_program::id()) { + if new_feature_activations.contains(&feature_set::programify_feature_gate_program::id()) { self.replace_empty_account_with_upgradeable_program( &feature::id(), &inline_feature_gate_program::noop_program::id(), diff --git a/sdk/src/feature_set.rs b/sdk/src/feature_set.rs index 289e9a8d2ca7e6..9ec56b03e0e3bf 100644 --- a/sdk/src/feature_set.rs +++ b/sdk/src/feature_set.rs @@ -700,7 +700,7 @@ pub mod better_error_codes_for_tx_lamport_check { solana_sdk::declare_id!("Ffswd3egL3tccB6Rv3XY6oqfdzn913vUcjCSnpvCKpfx"); } -pub mod feature_gate_program { +pub mod programify_feature_gate_program { solana_sdk::declare_id!("8GdovDzVwWU5edz2G697bbB7GZjrUc6aQZLWyNNAtHdg"); } @@ -874,7 +874,7 @@ lazy_static! { (require_rent_exempt_split_destination::id(), "Require stake split destination account to be rent exempt"), (better_error_codes_for_tx_lamport_check::id(), "better error codes for tx lamport check #33353"), (enable_alt_bn128_compression_syscall::id(), "add alt_bn128 compression syscalls"), - (feature_gate_program::id(), "control feature activations with an on-chain program #32783"), + (programify_feature_gate_program::id(), "move feature gate activation logic to an on-chain program #32783"), /*************** ADD NEW FEATURES HERE ***************/ ] .iter() From 251b9a93da6bda95e72d3fa0754f728f2be2bd7c Mon Sep 17 00:00:00 2001 From: Joe Date: Wed, 20 Sep 2023 18:14:42 +0200 Subject: [PATCH 24/33] test suite description clarity --- runtime/src/bank/tests.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/runtime/src/bank/tests.rs b/runtime/src/bank/tests.rs index a021bb7ab7c853..2c46b63149d639 100644 --- a/runtime/src/bank/tests.rs +++ b/runtime/src/bank/tests.rs @@ -8083,22 +8083,22 @@ fn test_replace_non_upgradeable_program_account() { #[test_case( Pubkey::new_unique(), None; - "Empty account _without_ corresponding data account" + "Empty destination account _without_ corresponding data account" )] #[test_case( Pubkey::new_unique(), Some(vec![4; 40]); - "Empty account _with_ corresponding data account" + "Empty destination account _with_ corresponding data account" )] #[test_case( feature::id(), // `Feature11111111` None; - "Native account _without_ corresponding data account" + "Native destination account _without_ corresponding data account" )] #[test_case( feature::id(), // `Feature11111111` Some(vec![4; 40]); - "Native account _with_ corresponding data account" + "Native destination account _with_ corresponding data account" )] fn test_replace_empty_account_with_upgradeable_program_success( dst: Pubkey, @@ -8218,11 +8218,11 @@ fn test_replace_empty_account_with_upgradeable_program_success( #[test_case( None; - "Existing account _without_ corresponding data account" + "Existing destination account _without_ corresponding data account" )] #[test_case( Some(vec![4; 40]); - "Existing account _with_ corresponding data account" + "Existing destination account _with_ corresponding data account" )] fn test_replace_empty_account_with_upgradeable_program_fail_when_account_exists( maybe_dst_data_state: Option>, // Inner data of the destination program _data_ account From 1b37cdc55bf58e0dc14d30a959243cd52f2c1514 Mon Sep 17 00:00:00 2001 From: Joe Date: Wed, 20 Sep 2023 18:56:43 +0200 Subject: [PATCH 25/33] remove strings from datapoints --- runtime/src/bank.rs | 99 +++++++++------------------------------------ 1 file changed, 20 insertions(+), 79 deletions(-) diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 39703bbd1dc1fd..0ce35d805df69f 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -8260,29 +8260,15 @@ impl Bank { .unwrap() .remove_programs([*dst_address].into_iter()); } else { - warn!("Unable to find source program {}", src_address); - datapoint_warn!( - datapoint_name, - ("slot", self.slot(), i64), - ("source program pubkey", src_address.to_string(), String), - ( - "destination program pubkey", - dst_address.to_string(), - String - ), + warn!( + "Unable to find source program {}. Destination: {}", + src_address, dst_address ); + datapoint_warn!(datapoint_name, ("slot", self.slot(), i64),); } } else { warn!("Unable to find destination program {}", dst_address); - datapoint_warn!( - datapoint_name, - ("slot", self.slot(), i64), - ( - "destination program pubkey", - dst_address.to_string(), - String - ), - ); + datapoint_warn!(datapoint_name, ("slot", self.slot(), i64),); } } @@ -8357,75 +8343,30 @@ impl Bank { .fetch_sub(change_in_capitalization, Relaxed); } } else { - error!("Unable to serialize program account state"); - datapoint_error!( - datapoint_name, - ("slot", self.slot(), i64), - ("source program pubkey", src_address.to_string(), String), - ( - "source data account pubkey", - src_data_address.to_string(), - String - ), - ( - "destination program pubkey", - dst_address.to_string(), - String - ), - ( - "destination data account pubkey", - dst_data_address.to_string(), - String - ), - ( - "destination data account pubkey", - format!( - "UpgradeableLoaderState::Program with data address {}", - dst_data_address - ), - String - ), + error!( + "Unable to serialize program account state. \ + Source program: {} Source data account: {} \ + Destination program: {} Destination data account: {}", + src_address, src_data_address, dst_address, dst_data_address, ); + datapoint_error!(datapoint_name, ("slot", self.slot(), i64),); } } } else { warn!( - "Unable to find data account for source program {}", - src_address - ); - datapoint_warn!( - datapoint_name, - ("slot", self.slot(), i64), - ("source program pubkey", src_address.to_string(), String), - ( - "source data account pubkey", - src_data_address.to_string(), - String - ), - ( - "destination program pubkey", - dst_address.to_string(), - String - ), - ( - "destination data account pubkey", - dst_data_address.to_string(), - String - ), + "Unable to find data account for source program. \ + Source program: {} Source data account: {} \ + Destination program: {} Destination data account: {}", + src_address, src_data_address, dst_address, dst_data_address, ); + datapoint_warn!(datapoint_name, ("slot", self.slot(), i64),); } } else { - warn!("Unable to find source program {}", src_address); - datapoint_warn!( - datapoint_name, - ("slot", self.slot(), i64), - ("source program pubkey", src_address.to_string(), String), - ( - "destination program pubkey", - dst_address.to_string(), - String - ), + warn!( + "Unable to find source program {}. Destination: {}", + src_address, dst_address ); + datapoint_warn!(datapoint_name, ("slot", self.slot(), i64),); } } From 5b280fde5cefd4bcb8d98d98709bb975709870e0 Mon Sep 17 00:00:00 2001 From: Joe Date: Fri, 29 Sep 2023 20:32:49 +0200 Subject: [PATCH 26/33] spell out source and destination --- runtime/src/bank.rs | 112 ++++++++++-------- runtime/src/bank/tests.rs | 231 +++++++++++++++++++++----------------- 2 files changed, 191 insertions(+), 152 deletions(-) diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 0ce35d805df69f..48b63c4f620aaf 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -8206,68 +8206,76 @@ impl Bank { } /// Moves one account in place of another - /// `src`: the program to replace with - /// `dst`: the program to be replaced + /// `source`: the program to replace with + /// `destination`: the program to be replaced fn move_account( &mut self, - src_address: &Pubkey, - src_account: &V, - dst_address: &Pubkey, - dst_account: Option<&U>, + source_address: &Pubkey, + source_account: &V, + destination_address: &Pubkey, + destination_account: Option<&U>, ) where U: ReadableAccount + Sync + ZeroLamport, V: ReadableAccount + Sync + ZeroLamport, { - let (dst_lamports, dst_len) = match dst_account { - Some(dst_account) => (dst_account.lamports(), dst_account.data().len()), + let (destination_lamports, destination_len) = match destination_account { + Some(destination_account) => ( + destination_account.lamports(), + destination_account.data().len(), + ), None => (0, 0), }; // Burn lamports in the destination account - self.capitalization.fetch_sub(dst_lamports, Relaxed); + self.capitalization.fetch_sub(destination_lamports, Relaxed); // Transfer source account to destination account - self.store_account(dst_address, src_account); + self.store_account(destination_address, source_account); // Clear source account - self.store_account(src_address, &AccountSharedData::default()); + self.store_account(source_address, &AccountSharedData::default()); self.calculate_and_update_accounts_data_size_delta_off_chain( - dst_len, - src_account.data().len(), + destination_len, + source_account.data().len(), ); } /// Use to replace programs by feature activation - /// `src`: the program to replace with - /// `dst`: the program to be replaced + /// `source`: the program to replace with + /// `destination`: the program to be replaced #[allow(dead_code)] fn replace_non_upgradeable_program_account( &mut self, - src_address: &Pubkey, - dst_address: &Pubkey, + source_address: &Pubkey, + destination_address: &Pubkey, datapoint_name: &'static str, ) { - if let Some(dst_account) = self.get_account_with_fixed_root(dst_address) { - if let Some(src_account) = self.get_account_with_fixed_root(src_address) { + if let Some(destination_account) = self.get_account_with_fixed_root(destination_address) { + if let Some(source_account) = self.get_account_with_fixed_root(source_address) { datapoint_info!(datapoint_name, ("slot", self.slot, i64)); - self.move_account(src_address, &src_account, dst_address, Some(&dst_account)); + self.move_account( + source_address, + &source_account, + destination_address, + Some(&destination_account), + ); // Unload a program from the bank's cache self.loaded_programs_cache .write() .unwrap() - .remove_programs([*dst_address].into_iter()); + .remove_programs([*destination_address].into_iter()); } else { warn!( "Unable to find source program {}. Destination: {}", - src_address, dst_address + source_address, destination_address ); datapoint_warn!(datapoint_name, ("slot", self.slot(), i64),); } } else { - warn!("Unable to find destination program {}", dst_address); + warn!("Unable to find destination program {}", destination_address); datapoint_warn!(datapoint_name, ("slot", self.slot(), i64),); } } @@ -8275,32 +8283,37 @@ impl Bank { /// Use to replace an empty account with a program by feature activation fn replace_empty_account_with_upgradeable_program( &mut self, - src_address: &Pubkey, - dst_address: &Pubkey, + source_address: &Pubkey, + destination_address: &Pubkey, datapoint_name: &'static str, ) { // Must be attempting to replace an empty account with a program // account _and_ data account - if let Some(src_account) = self.get_account_with_fixed_root(src_address) { - let (dst_data_address, _) = Pubkey::find_program_address( - &[dst_address.as_ref()], + if let Some(source_account) = self.get_account_with_fixed_root(source_address) { + let (destination_data_address, _) = Pubkey::find_program_address( + &[destination_address.as_ref()], &bpf_loader_upgradeable::id(), ); - let (src_data_address, _) = Pubkey::find_program_address( - &[src_address.as_ref()], + let (source_data_address, _) = Pubkey::find_program_address( + &[source_address.as_ref()], &bpf_loader_upgradeable::id(), ); - if let Some(src_data_account) = self.get_account_with_fixed_root(&src_data_address) { + if let Some(source_data_account) = + self.get_account_with_fixed_root(&source_data_address) + { // Make sure the destination account is empty // We aren't going to check that there isn't a data account at - // the known program-derived address (ie. `dst_data_address`), + // the known program-derived address (ie. `destination_data_address`), // because if it exists, it will be overwritten - if self.get_account_with_fixed_root(dst_address).is_none() { + if self + .get_account_with_fixed_root(destination_address) + .is_none() + { let lamports = self.get_minimum_balance_for_rent_exemption( UpgradeableLoaderState::size_of_program(), ); let state = UpgradeableLoaderState::Program { - programdata_address: dst_data_address, + programdata_address: destination_data_address, }; if let Ok(data) = bincode::serialize(&state) { let created_program_account = Account { @@ -8308,33 +8321,34 @@ impl Bank { data, owner: bpf_loader_upgradeable::id(), executable: true, - rent_epoch: src_account.rent_epoch(), + rent_epoch: source_account.rent_epoch(), }; // Make sure the source account has enough rent-exempt // lamports for the empty account that will now house the // PDA, and we can properly serialize the program account's // state - if src_account.lamports() >= lamports { + if source_account.lamports() >= lamports { datapoint_info!(datapoint_name, ("slot", self.slot, i64)); let change_in_capitalization = - src_account.lamports().saturating_sub(lamports); + source_account.lamports().saturating_sub(lamports); // Replace the destination data account with the source one // If the destination data account does not exist, it will be created // If it does exist, it will be overwritten self.move_account( - &src_data_address, - &src_data_account, - &dst_data_address, - self.get_account_with_fixed_root(&dst_data_address).as_ref(), + &source_data_address, + &source_data_account, + &destination_data_address, + self.get_account_with_fixed_root(&destination_data_address) + .as_ref(), ); // Write the source data account's PDA into the destination program account self.move_account( - src_address, + source_address, &created_program_account, - dst_address, + destination_address, None::<&AccountSharedData>, ); @@ -8347,7 +8361,10 @@ impl Bank { "Unable to serialize program account state. \ Source program: {} Source data account: {} \ Destination program: {} Destination data account: {}", - src_address, src_data_address, dst_address, dst_data_address, + source_address, + source_data_address, + destination_address, + destination_data_address, ); datapoint_error!(datapoint_name, ("slot", self.slot(), i64),); } @@ -8357,14 +8374,17 @@ impl Bank { "Unable to find data account for source program. \ Source program: {} Source data account: {} \ Destination program: {} Destination data account: {}", - src_address, src_data_address, dst_address, dst_data_address, + source_address, + source_data_address, + destination_address, + destination_data_address, ); datapoint_warn!(datapoint_name, ("slot", self.slot(), i64),); } } else { warn!( "Unable to find source program {}. Destination: {}", - src_address, dst_address + source_address, destination_address ); datapoint_warn!(datapoint_name, ("slot", self.slot(), i64),); } diff --git a/runtime/src/bank/tests.rs b/runtime/src/bank/tests.rs index 2c46b63149d639..4134f75e9fdbb1 100644 --- a/runtime/src/bank/tests.rs +++ b/runtime/src/bank/tests.rs @@ -8036,47 +8036,58 @@ fn test_replace_non_upgradeable_program_account() { let bpf_id = bpf_loader::id(); let mut bank = create_simple_test_bank(0); - let dst = Pubkey::new_unique(); - let dst_state = vec![0u8; 4]; - let dst_lamports = bank.get_minimum_balance_for_rent_exemption(dst_state.len()); - test_program_replace_set_up_account(&mut bank, &dst, dst_lamports, &dst_state, &bpf_id, true); - - let src = Pubkey::new_unique(); - let src_state = vec![6; 30]; - let src_lamports = bank.get_minimum_balance_for_rent_exemption(src_state.len()); - let check_src_account = test_program_replace_set_up_account( + let destination = Pubkey::new_unique(); + let destination_state = vec![0u8; 4]; + let destination_lamports = bank.get_minimum_balance_for_rent_exemption(destination_state.len()); + test_program_replace_set_up_account( + &mut bank, + &destination, + destination_lamports, + &destination_state, + &bpf_id, + true, + ); + + let source = Pubkey::new_unique(); + let source_state = vec![6; 30]; + let source_lamports = bank.get_minimum_balance_for_rent_exemption(source_state.len()); + let check_source_account = test_program_replace_set_up_account( &mut bank, - &src, - src_lamports, - &src_state, + &source, + source_lamports, + &source_state, &bpf_id, true, ); - let check_data_account_data = check_src_account.data().to_vec(); + let check_data_account_data = check_source_account.data().to_vec(); let original_capitalization = bank.capitalization(); - bank.replace_non_upgradeable_program_account(&src, &dst, "bank-apply_program_replacement"); + bank.replace_non_upgradeable_program_account( + &source, + &destination, + "bank-apply_program_replacement", + ); // Destination program account balance is now the source program account's balance - assert_eq!(bank.get_balance(&dst), src_lamports); + assert_eq!(bank.get_balance(&destination), source_lamports); // Source program account is now empty - assert_eq!(bank.get_balance(&src), 0); + assert_eq!(bank.get_balance(&source), 0); // Destination program account now holds the source program data, ie: // - Destination: [*Source program data] - let dst_account = bank.get_account(&dst).unwrap(); - assert_eq!(dst_account.data(), &check_data_account_data); + let destination_account = bank.get_account(&destination).unwrap(); + assert_eq!(destination_account.data(), &check_data_account_data); // Ownership & executable match the source program account - assert_eq!(dst_account.owner(), &bpf_id); - assert!(dst_account.executable()); + assert_eq!(destination_account.owner(), &bpf_id); + assert!(destination_account.executable()); // Lamports from the source program account were burnt assert_eq!( bank.capitalization(), - original_capitalization - dst_lamports + original_capitalization - destination_lamports ); } @@ -8101,8 +8112,8 @@ fn test_replace_non_upgradeable_program_account() { "Native destination account _with_ corresponding data account" )] fn test_replace_empty_account_with_upgradeable_program_success( - dst: Pubkey, - maybe_dst_data_state: Option>, // Inner data of the destination program _data_ account + destination: Pubkey, + maybe_destination_data_state: Option>, // Inner data of the destination program _data_ account ) { // Ensures a program account and data account are created when replacing an // empty account, ie: @@ -8114,52 +8125,54 @@ fn test_replace_empty_account_with_upgradeable_program_success( let mut bank = create_simple_test_bank(0); // Create the test source accounts, one for program and one for data - let src = Pubkey::new_unique(); - let (src_data, _) = Pubkey::find_program_address(&[src.as_ref()], &bpf_upgradeable_id); - let src_state = UpgradeableLoaderState::Program { - programdata_address: src_data, + let source = Pubkey::new_unique(); + let (source_data, _) = Pubkey::find_program_address(&[source.as_ref()], &bpf_upgradeable_id); + let source_state = UpgradeableLoaderState::Program { + programdata_address: source_data, }; - let src_lamports = + let source_lamports = bank.get_minimum_balance_for_rent_exemption(UpgradeableLoaderState::size_of_program()); - let src_data_state = vec![6; 30]; - let src_data_lamports = bank.get_minimum_balance_for_rent_exemption(src_data_state.len()); + let source_data_state = vec![6; 30]; + let source_data_lamports = bank.get_minimum_balance_for_rent_exemption(source_data_state.len()); test_program_replace_set_up_account( &mut bank, - &src, - src_lamports, - &src_state, + &source, + source_lamports, + &source_state, &bpf_upgradeable_id, true, ); - let check_src_data_account = test_program_replace_set_up_account( + let check_source_data_account = test_program_replace_set_up_account( &mut bank, - &src_data, - src_data_lamports, - &src_data_state, + &source_data, + source_data_lamports, + &source_data_state, &bpf_upgradeable_id, false, ); - let check_data_account_data = check_src_data_account.data().to_vec(); + let check_data_account_data = check_source_data_account.data().to_vec(); // Derive the well-known PDA address for the destination data account - let (dst_data, _) = Pubkey::find_program_address(&[dst.as_ref()], &bpf_upgradeable_id); + let (destination_data, _) = + Pubkey::find_program_address(&[destination.as_ref()], &bpf_upgradeable_id); // Determine the lamports that will be burnt after the replacement - let burnt_after_rent = if let Some(dst_data_state) = maybe_dst_data_state { + let burnt_after_rent = if let Some(destination_data_state) = maybe_destination_data_state { // Create the data account if necessary - let dst_data_lamports = bank.get_minimum_balance_for_rent_exemption(dst_data_state.len()); + let destination_data_lamports = + bank.get_minimum_balance_for_rent_exemption(destination_data_state.len()); test_program_replace_set_up_account( &mut bank, - &dst_data, - dst_data_lamports, - &dst_data_state, + &destination_data, + destination_data_lamports, + &destination_data_state, &bpf_upgradeable_id, false, ); - dst_data_lamports + src_lamports + destination_data_lamports + source_lamports - bank.get_minimum_balance_for_rent_exemption(UpgradeableLoaderState::size_of_program()) } else { - src_lamports + source_lamports - bank.get_minimum_balance_for_rent_exemption(UpgradeableLoaderState::size_of_program()) }; @@ -8167,46 +8180,46 @@ fn test_replace_empty_account_with_upgradeable_program_success( // Do the replacement bank.replace_empty_account_with_upgradeable_program( - &src, - &dst, + &source, + &destination, "bank-apply_empty_account_replacement_for_program", ); // Destination program account was created and funded to pay for minimum rent // for the PDA assert_eq!( - bank.get_balance(&dst), + bank.get_balance(&destination), bank.get_minimum_balance_for_rent_exemption(UpgradeableLoaderState::size_of_program()), ); // Destination data account was created, now holds the source data account's balance - assert_eq!(bank.get_balance(&dst_data), src_data_lamports); + assert_eq!(bank.get_balance(&destination_data), source_data_lamports); // Source program accounts are now empty - assert_eq!(bank.get_balance(&src), 0); - assert_eq!(bank.get_balance(&src_data), 0); + assert_eq!(bank.get_balance(&source), 0); + assert_eq!(bank.get_balance(&source_data), 0); // Destination program account holds the PDA, ie: // - Destination: PDA(DestinationData) - let dst_account = bank.get_account(&dst).unwrap(); + let destination_account = bank.get_account(&destination).unwrap(); assert_eq!( - dst_account.data(), + destination_account.data(), &bincode::serialize(&UpgradeableLoaderState::Program { - programdata_address: dst_data + programdata_address: destination_data }) .unwrap(), ); // Destination data account holds the source data, ie: // - DestinationData: [*Source program data] - let dst_data_account = bank.get_account(&dst_data).unwrap(); - assert_eq!(dst_data_account.data(), &check_data_account_data); + let destination_data_account = bank.get_account(&destination_data).unwrap(); + assert_eq!(destination_data_account.data(), &check_data_account_data); // Ownership & executable match the source program accounts - assert_eq!(dst_account.owner(), &bpf_upgradeable_id); - assert!(dst_account.executable()); - assert_eq!(dst_data_account.owner(), &bpf_upgradeable_id); - assert!(!dst_data_account.executable()); + assert_eq!(destination_account.owner(), &bpf_upgradeable_id); + assert!(destination_account.executable()); + assert_eq!(destination_data_account.owner(), &bpf_upgradeable_id); + assert!(!destination_data_account.executable()); // The remaining lamports from both program accounts minus the rent-exempt // minimum were burnt @@ -8225,87 +8238,93 @@ fn test_replace_empty_account_with_upgradeable_program_success( "Existing destination account _with_ corresponding data account" )] fn test_replace_empty_account_with_upgradeable_program_fail_when_account_exists( - maybe_dst_data_state: Option>, // Inner data of the destination program _data_ account + maybe_destination_data_state: Option>, // Inner data of the destination program _data_ account ) { // Should not be allowed to execute replacement let bpf_upgradeable_id = bpf_loader_upgradeable::id(); let mut bank = create_simple_test_bank(0); // Create the test destination account with some arbitrary data and lamports balance - let dst = Pubkey::new_unique(); - let dst_state = vec![0, 0, 0, 0]; // Arbitrary bytes, doesn't matter - let dst_lamports = bank.get_minimum_balance_for_rent_exemption(dst_state.len()); - let dst_account = test_program_replace_set_up_account( + let destination = Pubkey::new_unique(); + let destination_state = vec![0, 0, 0, 0]; // Arbitrary bytes, doesn't matter + let destination_lamports = bank.get_minimum_balance_for_rent_exemption(destination_state.len()); + let destination_account = test_program_replace_set_up_account( &mut bank, - &dst, - dst_lamports, - &dst_state, + &destination, + destination_lamports, + &destination_state, &bpf_upgradeable_id, true, ); // Create the test source accounts, one for program and one for data - let src = Pubkey::new_unique(); - let (src_data, _) = Pubkey::find_program_address(&[src.as_ref()], &bpf_upgradeable_id); - let src_state = UpgradeableLoaderState::Program { - programdata_address: src_data, + let source = Pubkey::new_unique(); + let (source_data, _) = Pubkey::find_program_address(&[source.as_ref()], &bpf_upgradeable_id); + let source_state = UpgradeableLoaderState::Program { + programdata_address: source_data, }; - let src_lamports = + let source_lamports = bank.get_minimum_balance_for_rent_exemption(UpgradeableLoaderState::size_of_program()); - let src_data_state = vec![6; 30]; - let src_data_lamports = bank.get_minimum_balance_for_rent_exemption(src_data_state.len()); - let src_account = test_program_replace_set_up_account( + let source_data_state = vec![6; 30]; + let source_data_lamports = bank.get_minimum_balance_for_rent_exemption(source_data_state.len()); + let source_account = test_program_replace_set_up_account( &mut bank, - &src, - src_lamports, - &src_state, + &source, + source_lamports, + &source_state, &bpf_upgradeable_id, true, ); - let src_data_account = test_program_replace_set_up_account( + let source_data_account = test_program_replace_set_up_account( &mut bank, - &src_data, - src_data_lamports, - &src_data_state, + &source_data, + source_data_lamports, + &source_data_state, &bpf_upgradeable_id, false, ); // Derive the well-known PDA address for the destination data account - let (dst_data, _) = Pubkey::find_program_address(&[dst.as_ref()], &bpf_upgradeable_id); + let (destination_data, _) = + Pubkey::find_program_address(&[destination.as_ref()], &bpf_upgradeable_id); // Create the data account if necessary - let dst_data_account = if let Some(dst_data_state) = maybe_dst_data_state { - let dst_data_lamports = bank.get_minimum_balance_for_rent_exemption(dst_data_state.len()); - let dst_data_account = test_program_replace_set_up_account( - &mut bank, - &dst_data, - dst_data_lamports, - &dst_data_state, - &bpf_upgradeable_id, - false, - ); - Some(dst_data_account) - } else { - None - }; + let destination_data_account = + if let Some(destination_data_state) = maybe_destination_data_state { + let destination_data_lamports = + bank.get_minimum_balance_for_rent_exemption(destination_data_state.len()); + let destination_data_account = test_program_replace_set_up_account( + &mut bank, + &destination_data, + destination_data_lamports, + &destination_data_state, + &bpf_upgradeable_id, + false, + ); + Some(destination_data_account) + } else { + None + }; let original_capitalization = bank.capitalization(); // Attempt the replacement bank.replace_empty_account_with_upgradeable_program( - &src, - &dst, + &source, + &destination, "bank-apply_empty_account_replacement_for_program", ); // Everything should be unchanged - assert_eq!(bank.get_account(&dst).unwrap(), dst_account); - if let Some(dst_data_account) = dst_data_account { - assert_eq!(bank.get_account(&dst_data).unwrap(), dst_data_account); + assert_eq!(bank.get_account(&destination).unwrap(), destination_account); + if let Some(destination_data_account) = destination_data_account { + assert_eq!( + bank.get_account(&destination_data).unwrap(), + destination_data_account + ); } - assert_eq!(bank.get_account(&src).unwrap(), src_account); - assert_eq!(bank.get_account(&src_data).unwrap(), src_data_account); + assert_eq!(bank.get_account(&source).unwrap(), source_account); + assert_eq!(bank.get_account(&source_data).unwrap(), source_data_account); assert_eq!(bank.capitalization(), original_capitalization); } From 6ccf0f13d947ebc42797de55f1d01f2cac69ae60 Mon Sep 17 00:00:00 2001 From: Joe Date: Fri, 29 Sep 2023 20:42:54 +0200 Subject: [PATCH 27/33] more verbose comments in account replace functions --- runtime/src/bank.rs | 15 ++++++++++----- runtime/src/bank/tests.rs | 2 +- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 48b63c4f620aaf..2fe44d134a98c8 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -8206,8 +8206,8 @@ impl Bank { } /// Moves one account in place of another - /// `source`: the program to replace with - /// `destination`: the program to be replaced + /// `source`: the account to replace with + /// `destination`: the account to be replaced fn move_account( &mut self, source_address: &Pubkey, @@ -8241,9 +8241,9 @@ impl Bank { ); } - /// Use to replace programs by feature activation - /// `source`: the program to replace with - /// `destination`: the program to be replaced + /// Use to replace non-upgradeable programs by feature activation + /// `source`: the non-upgradeable program account to replace with + /// `destination`: the non-upgradeable program account to be replaced #[allow(dead_code)] fn replace_non_upgradeable_program_account( &mut self, @@ -8281,6 +8281,11 @@ impl Bank { } /// Use to replace an empty account with a program by feature activation + /// Note: The upgradeable program should have both: + /// - Program account + /// - Program data account + /// `source`: the upgradeable program account to replace with + /// `destination`: the empty account to be replaced fn replace_empty_account_with_upgradeable_program( &mut self, source_address: &Pubkey, diff --git a/runtime/src/bank/tests.rs b/runtime/src/bank/tests.rs index 4134f75e9fdbb1..f23d621bbff3ea 100644 --- a/runtime/src/bank/tests.rs +++ b/runtime/src/bank/tests.rs @@ -8084,7 +8084,7 @@ fn test_replace_non_upgradeable_program_account() { assert_eq!(destination_account.owner(), &bpf_id); assert!(destination_account.executable()); - // Lamports from the source program account were burnt + // The destination account's original lamports balance was burnt assert_eq!( bank.capitalization(), original_capitalization - destination_lamports From 0392ad852b0d78463997cb765ac82b61420f1898 Mon Sep 17 00:00:00 2001 From: Joe Date: Fri, 29 Sep 2023 21:07:43 +0200 Subject: [PATCH 28/33] move lamport calculation --- runtime/src/bank.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 2fe44d134a98c8..e00213824c920f 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -8314,13 +8314,11 @@ impl Bank { .get_account_with_fixed_root(destination_address) .is_none() { - let lamports = self.get_minimum_balance_for_rent_exemption( - UpgradeableLoaderState::size_of_program(), - ); let state = UpgradeableLoaderState::Program { programdata_address: destination_data_address, }; if let Ok(data) = bincode::serialize(&state) { + let lamports = self.get_minimum_balance_for_rent_exemption(data.len()); let created_program_account = Account { lamports, data, From 41c9c40b5926774bfbbef50e16295d6b1245f92a Mon Sep 17 00:00:00 2001 From: Joe Date: Fri, 29 Sep 2023 21:20:45 +0200 Subject: [PATCH 29/33] swap lamport check for state check --- runtime/src/bank.rs | 154 +++++++++++++++++++++++++------------------- 1 file changed, 88 insertions(+), 66 deletions(-) diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index e00213824c920f..2e4f4b8479e1b4 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -8303,65 +8303,97 @@ impl Bank { &[source_address.as_ref()], &bpf_loader_upgradeable::id(), ); - if let Some(source_data_account) = - self.get_account_with_fixed_root(&source_data_address) - { - // Make sure the destination account is empty - // We aren't going to check that there isn't a data account at - // the known program-derived address (ie. `destination_data_address`), - // because if it exists, it will be overwritten - if self - .get_account_with_fixed_root(destination_address) - .is_none() - { - let state = UpgradeableLoaderState::Program { - programdata_address: destination_data_address, - }; - if let Ok(data) = bincode::serialize(&state) { - let lamports = self.get_minimum_balance_for_rent_exemption(data.len()); - let created_program_account = Account { - lamports, - data, - owner: bpf_loader_upgradeable::id(), - executable: true, - rent_epoch: source_account.rent_epoch(), - }; - - // Make sure the source account has enough rent-exempt - // lamports for the empty account that will now house the - // PDA, and we can properly serialize the program account's - // state - if source_account.lamports() >= lamports { - datapoint_info!(datapoint_name, ("slot", self.slot, i64)); - let change_in_capitalization = - source_account.lamports().saturating_sub(lamports); - - // Replace the destination data account with the source one - // If the destination data account does not exist, it will be created - // If it does exist, it will be overwritten - self.move_account( - &source_data_address, - &source_data_account, - &destination_data_address, - self.get_account_with_fixed_root(&destination_data_address) - .as_ref(), - ); - // Write the source data account's PDA into the destination program account - self.move_account( + // Make sure the data within the source account is the PDA of its + // data account. This also means it has at least the necessary + // lamports for rent. + if let Ok(source_state) = + bincode::deserialize::(source_account.data()) + { + match source_state { + UpgradeableLoaderState::Program { + programdata_address: _, + } => { + if let Some(source_data_account) = + self.get_account_with_fixed_root(&source_data_address) + { + // Make sure the destination account is empty + // We aren't going to check that there isn't a data account at + // the known program-derived address (ie. `destination_data_address`), + // because if it exists, it will be overwritten + if self + .get_account_with_fixed_root(destination_address) + .is_none() + { + let state = UpgradeableLoaderState::Program { + programdata_address: destination_data_address, + }; + if let Ok(data) = bincode::serialize(&state) { + let lamports = + self.get_minimum_balance_for_rent_exemption(data.len()); + let created_program_account = Account { + lamports, + data, + owner: bpf_loader_upgradeable::id(), + executable: true, + rent_epoch: source_account.rent_epoch(), + }; + + datapoint_info!(datapoint_name, ("slot", self.slot, i64)); + let change_in_capitalization = + source_account.lamports().saturating_sub(lamports); + + // Replace the destination data account with the source one + // If the destination data account does not exist, it will be created + // If it does exist, it will be overwritten + self.move_account( + &source_data_address, + &source_data_account, + &destination_data_address, + self.get_account_with_fixed_root(&destination_data_address) + .as_ref(), + ); + + // Write the source data account's PDA into the destination program account + self.move_account( + source_address, + &created_program_account, + destination_address, + None::<&AccountSharedData>, + ); + + // Any remaining lamports in the source program account are burnt + self.capitalization + .fetch_sub(change_in_capitalization, Relaxed); + } else { + error!( + "Unable to serialize program account state. \ + Source program: {} Source data account: {} \ + Destination program: {} Destination data account: {}", + source_address, + source_data_address, + destination_address, + destination_data_address, + ); + datapoint_error!(datapoint_name, ("slot", self.slot(), i64),); + } + } + } else { + warn!( + "Unable to find data account for source program. \ + Source program: {} Source data account: {} \ + Destination program: {} Destination data account: {}", source_address, - &created_program_account, + source_data_address, destination_address, - None::<&AccountSharedData>, + destination_data_address, ); - - // Any remaining lamports in the source program account are burnt - self.capitalization - .fetch_sub(change_in_capitalization, Relaxed); + datapoint_warn!(datapoint_name, ("slot", self.slot(), i64),); } - } else { - error!( - "Unable to serialize program account state. \ + } + _ => { + warn!( + "Source program account is not an upgradeable program. \ Source program: {} Source data account: {} \ Destination program: {} Destination data account: {}", source_address, @@ -8369,20 +8401,10 @@ impl Bank { destination_address, destination_data_address, ); - datapoint_error!(datapoint_name, ("slot", self.slot(), i64),); + datapoint_warn!(datapoint_name, ("slot", self.slot(), i64),); + return; } } - } else { - warn!( - "Unable to find data account for source program. \ - Source program: {} Source data account: {} \ - Destination program: {} Destination data account: {}", - source_address, - source_data_address, - destination_address, - destination_data_address, - ); - datapoint_warn!(datapoint_name, ("slot", self.slot(), i64),); } } else { warn!( From 5cadf5b439d6be715ee35b83e91f794428f29359 Mon Sep 17 00:00:00 2001 From: Joe Date: Fri, 29 Sep 2023 22:03:28 +0200 Subject: [PATCH 30/33] move replace functions to helper module --- runtime/src/bank.rs | 214 +-------------------------- runtime/src/bank/replace_account.rs | 222 ++++++++++++++++++++++++++++ runtime/src/bank/tests.rs | 12 +- 3 files changed, 234 insertions(+), 214 deletions(-) create mode 100644 runtime/src/bank/replace_account.rs diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 2e4f4b8479e1b4..98ce1d5f39816f 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -216,6 +216,7 @@ pub mod bank_hash_details; mod builtin_programs; pub mod epoch_accounts_hash_utils; mod metrics; +mod replace_account; mod serde_snapshot; mod sysvar_cache; #[cfg(test)] @@ -8057,7 +8058,8 @@ impl Bank { } if new_feature_activations.contains(&feature_set::programify_feature_gate_program::id()) { - self.replace_empty_account_with_upgradeable_program( + replace_account::replace_empty_account_with_upgradeable_program( + self, &feature::id(), &inline_feature_gate_program::noop_program::id(), "bank-apply_feature_gate_program", @@ -8205,216 +8207,6 @@ impl Bank { } } - /// Moves one account in place of another - /// `source`: the account to replace with - /// `destination`: the account to be replaced - fn move_account( - &mut self, - source_address: &Pubkey, - source_account: &V, - destination_address: &Pubkey, - destination_account: Option<&U>, - ) where - U: ReadableAccount + Sync + ZeroLamport, - V: ReadableAccount + Sync + ZeroLamport, - { - let (destination_lamports, destination_len) = match destination_account { - Some(destination_account) => ( - destination_account.lamports(), - destination_account.data().len(), - ), - None => (0, 0), - }; - - // Burn lamports in the destination account - self.capitalization.fetch_sub(destination_lamports, Relaxed); - - // Transfer source account to destination account - self.store_account(destination_address, source_account); - - // Clear source account - self.store_account(source_address, &AccountSharedData::default()); - - self.calculate_and_update_accounts_data_size_delta_off_chain( - destination_len, - source_account.data().len(), - ); - } - - /// Use to replace non-upgradeable programs by feature activation - /// `source`: the non-upgradeable program account to replace with - /// `destination`: the non-upgradeable program account to be replaced - #[allow(dead_code)] - fn replace_non_upgradeable_program_account( - &mut self, - source_address: &Pubkey, - destination_address: &Pubkey, - datapoint_name: &'static str, - ) { - if let Some(destination_account) = self.get_account_with_fixed_root(destination_address) { - if let Some(source_account) = self.get_account_with_fixed_root(source_address) { - datapoint_info!(datapoint_name, ("slot", self.slot, i64)); - - self.move_account( - source_address, - &source_account, - destination_address, - Some(&destination_account), - ); - - // Unload a program from the bank's cache - self.loaded_programs_cache - .write() - .unwrap() - .remove_programs([*destination_address].into_iter()); - } else { - warn!( - "Unable to find source program {}. Destination: {}", - source_address, destination_address - ); - datapoint_warn!(datapoint_name, ("slot", self.slot(), i64),); - } - } else { - warn!("Unable to find destination program {}", destination_address); - datapoint_warn!(datapoint_name, ("slot", self.slot(), i64),); - } - } - - /// Use to replace an empty account with a program by feature activation - /// Note: The upgradeable program should have both: - /// - Program account - /// - Program data account - /// `source`: the upgradeable program account to replace with - /// `destination`: the empty account to be replaced - fn replace_empty_account_with_upgradeable_program( - &mut self, - source_address: &Pubkey, - destination_address: &Pubkey, - datapoint_name: &'static str, - ) { - // Must be attempting to replace an empty account with a program - // account _and_ data account - if let Some(source_account) = self.get_account_with_fixed_root(source_address) { - let (destination_data_address, _) = Pubkey::find_program_address( - &[destination_address.as_ref()], - &bpf_loader_upgradeable::id(), - ); - let (source_data_address, _) = Pubkey::find_program_address( - &[source_address.as_ref()], - &bpf_loader_upgradeable::id(), - ); - - // Make sure the data within the source account is the PDA of its - // data account. This also means it has at least the necessary - // lamports for rent. - if let Ok(source_state) = - bincode::deserialize::(source_account.data()) - { - match source_state { - UpgradeableLoaderState::Program { - programdata_address: _, - } => { - if let Some(source_data_account) = - self.get_account_with_fixed_root(&source_data_address) - { - // Make sure the destination account is empty - // We aren't going to check that there isn't a data account at - // the known program-derived address (ie. `destination_data_address`), - // because if it exists, it will be overwritten - if self - .get_account_with_fixed_root(destination_address) - .is_none() - { - let state = UpgradeableLoaderState::Program { - programdata_address: destination_data_address, - }; - if let Ok(data) = bincode::serialize(&state) { - let lamports = - self.get_minimum_balance_for_rent_exemption(data.len()); - let created_program_account = Account { - lamports, - data, - owner: bpf_loader_upgradeable::id(), - executable: true, - rent_epoch: source_account.rent_epoch(), - }; - - datapoint_info!(datapoint_name, ("slot", self.slot, i64)); - let change_in_capitalization = - source_account.lamports().saturating_sub(lamports); - - // Replace the destination data account with the source one - // If the destination data account does not exist, it will be created - // If it does exist, it will be overwritten - self.move_account( - &source_data_address, - &source_data_account, - &destination_data_address, - self.get_account_with_fixed_root(&destination_data_address) - .as_ref(), - ); - - // Write the source data account's PDA into the destination program account - self.move_account( - source_address, - &created_program_account, - destination_address, - None::<&AccountSharedData>, - ); - - // Any remaining lamports in the source program account are burnt - self.capitalization - .fetch_sub(change_in_capitalization, Relaxed); - } else { - error!( - "Unable to serialize program account state. \ - Source program: {} Source data account: {} \ - Destination program: {} Destination data account: {}", - source_address, - source_data_address, - destination_address, - destination_data_address, - ); - datapoint_error!(datapoint_name, ("slot", self.slot(), i64),); - } - } - } else { - warn!( - "Unable to find data account for source program. \ - Source program: {} Source data account: {} \ - Destination program: {} Destination data account: {}", - source_address, - source_data_address, - destination_address, - destination_data_address, - ); - datapoint_warn!(datapoint_name, ("slot", self.slot(), i64),); - } - } - _ => { - warn!( - "Source program account is not an upgradeable program. \ - Source program: {} Source data account: {} \ - Destination program: {} Destination data account: {}", - source_address, - source_data_address, - destination_address, - destination_data_address, - ); - datapoint_warn!(datapoint_name, ("slot", self.slot(), i64),); - return; - } - } - } - } else { - warn!( - "Unable to find source program {}. Destination: {}", - source_address, destination_address - ); - datapoint_warn!(datapoint_name, ("slot", self.slot(), i64),); - } - } - /// Get all the accounts for this bank and calculate stats pub fn get_total_accounts_stats(&self) -> ScanResult { let accounts = self.get_all_accounts()?; diff --git a/runtime/src/bank/replace_account.rs b/runtime/src/bank/replace_account.rs new file mode 100644 index 00000000000000..e5bf6855c1b12d --- /dev/null +++ b/runtime/src/bank/replace_account.rs @@ -0,0 +1,222 @@ +use { + super::Bank, + log::*, + solana_accounts_db::accounts_index::ZeroLamport, + solana_sdk::{ + account::{Account, AccountSharedData, ReadableAccount}, + bpf_loader_upgradeable::{self, UpgradeableLoaderState}, + pubkey::Pubkey, + }, + std::sync::atomic::Ordering::Relaxed, +}; + +/// Moves one account in place of another +/// `source`: the account to replace with +/// `destination`: the account to be replaced +fn move_account( + bank: &mut Bank, + source_address: &Pubkey, + source_account: &V, + destination_address: &Pubkey, + destination_account: Option<&U>, +) where + U: ReadableAccount + Sync + ZeroLamport, + V: ReadableAccount + Sync + ZeroLamport, +{ + let (destination_lamports, destination_len) = match destination_account { + Some(destination_account) => ( + destination_account.lamports(), + destination_account.data().len(), + ), + None => (0, 0), + }; + + // Burn lamports in the destination account + bank.capitalization.fetch_sub(destination_lamports, Relaxed); + + // Transfer source account to destination account + bank.store_account(destination_address, source_account); + + // Clear source account + bank.store_account(source_address, &AccountSharedData::default()); + + bank.calculate_and_update_accounts_data_size_delta_off_chain( + destination_len, + source_account.data().len(), + ); +} + +/// Use to replace non-upgradeable programs by feature activation +/// `source`: the non-upgradeable program account to replace with +/// `destination`: the non-upgradeable program account to be replaced +#[allow(dead_code)] +pub(crate) fn replace_non_upgradeable_program_account( + bank: &mut Bank, + source_address: &Pubkey, + destination_address: &Pubkey, + datapoint_name: &'static str, +) { + if let Some(destination_account) = bank.get_account_with_fixed_root(destination_address) { + if let Some(source_account) = bank.get_account_with_fixed_root(source_address) { + datapoint_info!(datapoint_name, ("slot", bank.slot, i64)); + + move_account( + bank, + source_address, + &source_account, + destination_address, + Some(&destination_account), + ); + + // Unload a program from the bank's cache + bank.loaded_programs_cache + .write() + .unwrap() + .remove_programs([*destination_address].into_iter()); + } else { + warn!( + "Unable to find source program {}. Destination: {}", + source_address, destination_address + ); + datapoint_warn!(datapoint_name, ("slot", bank.slot(), i64),); + } + } else { + warn!("Unable to find destination program {}", destination_address); + datapoint_warn!(datapoint_name, ("slot", bank.slot(), i64),); + } +} + +/// Use to replace an empty account with a program by feature activation +/// Note: The upgradeable program should have both: +/// - Program account +/// - Program data account +/// `source`: the upgradeable program account to replace with +/// `destination`: the empty account to be replaced +pub(crate) fn replace_empty_account_with_upgradeable_program( + bank: &mut Bank, + source_address: &Pubkey, + destination_address: &Pubkey, + datapoint_name: &'static str, +) { + // Must be attempting to replace an empty account with a program + // account _and_ data account + if let Some(source_account) = bank.get_account_with_fixed_root(source_address) { + let (destination_data_address, _) = Pubkey::find_program_address( + &[destination_address.as_ref()], + &bpf_loader_upgradeable::id(), + ); + let (source_data_address, _) = + Pubkey::find_program_address(&[source_address.as_ref()], &bpf_loader_upgradeable::id()); + + // Make sure the data within the source account is the PDA of its + // data account. This also means it has at least the necessary + // lamports for rent. + if let Ok(source_state) = + bincode::deserialize::(source_account.data()) + { + match source_state { + UpgradeableLoaderState::Program { + programdata_address: _, + } => { + if let Some(source_data_account) = + bank.get_account_with_fixed_root(&source_data_address) + { + // Make sure the destination account is empty + // We aren't going to check that there isn't a data account at + // the known program-derived address (ie. `destination_data_address`), + // because if it exists, it will be overwritten + if bank + .get_account_with_fixed_root(destination_address) + .is_none() + { + let state = UpgradeableLoaderState::Program { + programdata_address: destination_data_address, + }; + if let Ok(data) = bincode::serialize(&state) { + let lamports = + bank.get_minimum_balance_for_rent_exemption(data.len()); + let created_program_account = Account { + lamports, + data, + owner: bpf_loader_upgradeable::id(), + executable: true, + rent_epoch: source_account.rent_epoch(), + }; + + datapoint_info!(datapoint_name, ("slot", bank.slot, i64)); + let change_in_capitalization = + source_account.lamports().saturating_sub(lamports); + + // Replace the destination data account with the source one + // If the destination data account does not exist, it will be created + // If it does exist, it will be overwritten + move_account( + bank, + &source_data_address, + &source_data_account, + &destination_data_address, + bank.get_account_with_fixed_root(&destination_data_address) + .as_ref(), + ); + + // Write the source data account's PDA into the destination program account + move_account( + bank, + source_address, + &created_program_account, + destination_address, + None::<&AccountSharedData>, + ); + + // Any remaining lamports in the source program account are burnt + bank.capitalization + .fetch_sub(change_in_capitalization, Relaxed); + } else { + error!( + "Unable to serialize program account state. \ + Source program: {} Source data account: {} \ + Destination program: {} Destination data account: {}", + source_address, + source_data_address, + destination_address, + destination_data_address, + ); + datapoint_error!(datapoint_name, ("slot", bank.slot(), i64),); + } + } + } else { + warn!( + "Unable to find data account for source program. \ + Source program: {} Source data account: {} \ + Destination program: {} Destination data account: {}", + source_address, + source_data_address, + destination_address, + destination_data_address, + ); + datapoint_warn!(datapoint_name, ("slot", bank.slot(), i64),); + } + } + _ => { + warn!( + "Source program account is not an upgradeable program. \ + Source program: {} Source data account: {} \ + Destination program: {} Destination data account: {}", + source_address, + source_data_address, + destination_address, + destination_data_address, + ); + datapoint_warn!(datapoint_name, ("slot", bank.slot(), i64),); + return; + } + } + } + } else { + warn!( + "Unable to find source program {}. Destination: {}", + source_address, destination_address + ); + datapoint_warn!(datapoint_name, ("slot", bank.slot(), i64),); + } +} diff --git a/runtime/src/bank/tests.rs b/runtime/src/bank/tests.rs index f23d621bbff3ea..a2672e809d2a2a 100644 --- a/runtime/src/bank/tests.rs +++ b/runtime/src/bank/tests.rs @@ -8,6 +8,9 @@ use { }, crate::{ accounts_background_service::{PrunedBanksRequestHandler, SendDroppedBankCallback}, + bank::replace_account::{ + replace_empty_account_with_upgradeable_program, replace_non_upgradeable_program_account, + }, bank_client::BankClient, epoch_rewards_hasher::hash_rewards_into_partitions, genesis_utils::{ @@ -8063,7 +8066,8 @@ fn test_replace_non_upgradeable_program_account() { let original_capitalization = bank.capitalization(); - bank.replace_non_upgradeable_program_account( + replace_non_upgradeable_program_account( + &mut bank, &source, &destination, "bank-apply_program_replacement", @@ -8179,7 +8183,8 @@ fn test_replace_empty_account_with_upgradeable_program_success( let original_capitalization = bank.capitalization(); // Do the replacement - bank.replace_empty_account_with_upgradeable_program( + replace_empty_account_with_upgradeable_program( + &mut bank, &source, &destination, "bank-apply_empty_account_replacement_for_program", @@ -8309,7 +8314,8 @@ fn test_replace_empty_account_with_upgradeable_program_fail_when_account_exists( let original_capitalization = bank.capitalization(); // Attempt the replacement - bank.replace_empty_account_with_upgradeable_program( + replace_empty_account_with_upgradeable_program( + &mut bank, &source, &destination, "bank-apply_empty_account_replacement_for_program", From 3e7dc305e53151d6fac338ef8dcd29b32a2daeca Mon Sep 17 00:00:00 2001 From: Joe Date: Tue, 3 Oct 2023 08:54:18 +0200 Subject: [PATCH 31/33] make replace_account methods fallible --- runtime/src/bank.rs | 15 ++++- runtime/src/bank/replace_account.rs | 87 +++++++++++++---------------- runtime/src/bank/tests.rs | 23 +++++--- 3 files changed, 67 insertions(+), 58 deletions(-) diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 98ce1d5f39816f..68e5492186ff9d 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -8058,12 +8058,21 @@ impl Bank { } if new_feature_activations.contains(&feature_set::programify_feature_gate_program::id()) { - replace_account::replace_empty_account_with_upgradeable_program( + let datapoint_name = "bank-progamify_feature_gate_program"; + if let Err(e) = replace_account::replace_empty_account_with_upgradeable_program( self, &feature::id(), &inline_feature_gate_program::noop_program::id(), - "bank-apply_feature_gate_program", - ); + datapoint_name, + ) { + warn!( + "{}: Failed to replace empty account {} with upgradeable program: {}", + datapoint_name, + feature::id(), + e + ); + datapoint_warn!(datapoint_name, ("slot", self.slot(), i64),); + } } } diff --git a/runtime/src/bank/replace_account.rs b/runtime/src/bank/replace_account.rs index e5bf6855c1b12d..1835e769413686 100644 --- a/runtime/src/bank/replace_account.rs +++ b/runtime/src/bank/replace_account.rs @@ -8,8 +8,32 @@ use { pubkey::Pubkey, }, std::sync::atomic::Ordering::Relaxed, + thiserror::Error, }; +/// Errors returned by `replace_account` methods +#[derive(Debug, Error, PartialEq)] +pub enum ReplaceAccountError { + /// Source account not found + #[error("Source account not found")] + SourceAccountNotFound, + /// Source data account not found + #[error("Source data account not found")] + SourceDataAccountNotFound, + /// Destination account not found + #[error("Destination account not found")] + DestinationAccountNotFound, + /// Destination account exists + #[error("Destination account exists")] + DestinationAccountExists, + /// Unable to serialize program account state + #[error("Unable to serialize program account state")] + UnableToSerializeProgramAccountState, + /// Not an upgradeable program + #[error("Not an upgradeable program")] + NotAnUpgradeableProgram, +} + /// Moves one account in place of another /// `source`: the account to replace with /// `destination`: the account to be replaced @@ -55,7 +79,7 @@ pub(crate) fn replace_non_upgradeable_program_account( source_address: &Pubkey, destination_address: &Pubkey, datapoint_name: &'static str, -) { +) -> Result<(), ReplaceAccountError> { if let Some(destination_account) = bank.get_account_with_fixed_root(destination_address) { if let Some(source_account) = bank.get_account_with_fixed_root(source_address) { datapoint_info!(datapoint_name, ("slot", bank.slot, i64)); @@ -73,16 +97,13 @@ pub(crate) fn replace_non_upgradeable_program_account( .write() .unwrap() .remove_programs([*destination_address].into_iter()); + + Ok(()) } else { - warn!( - "Unable to find source program {}. Destination: {}", - source_address, destination_address - ); - datapoint_warn!(datapoint_name, ("slot", bank.slot(), i64),); + Err(ReplaceAccountError::SourceAccountNotFound) } } else { - warn!("Unable to find destination program {}", destination_address); - datapoint_warn!(datapoint_name, ("slot", bank.slot(), i64),); + Err(ReplaceAccountError::DestinationAccountNotFound) } } @@ -97,7 +118,7 @@ pub(crate) fn replace_empty_account_with_upgradeable_program( source_address: &Pubkey, destination_address: &Pubkey, datapoint_name: &'static str, -) { +) -> Result<(), ReplaceAccountError> { // Must be attempting to replace an empty account with a program // account _and_ data account if let Some(source_account) = bank.get_account_with_fixed_root(source_address) { @@ -171,52 +192,24 @@ pub(crate) fn replace_empty_account_with_upgradeable_program( // Any remaining lamports in the source program account are burnt bank.capitalization .fetch_sub(change_in_capitalization, Relaxed); + + Ok(()) } else { - error!( - "Unable to serialize program account state. \ - Source program: {} Source data account: {} \ - Destination program: {} Destination data account: {}", - source_address, - source_data_address, - destination_address, - destination_data_address, - ); - datapoint_error!(datapoint_name, ("slot", bank.slot(), i64),); + Err(ReplaceAccountError::UnableToSerializeProgramAccountState) } + } else { + Err(ReplaceAccountError::DestinationAccountExists) } } else { - warn!( - "Unable to find data account for source program. \ - Source program: {} Source data account: {} \ - Destination program: {} Destination data account: {}", - source_address, - source_data_address, - destination_address, - destination_data_address, - ); - datapoint_warn!(datapoint_name, ("slot", bank.slot(), i64),); + Err(ReplaceAccountError::SourceDataAccountNotFound) } } - _ => { - warn!( - "Source program account is not an upgradeable program. \ - Source program: {} Source data account: {} \ - Destination program: {} Destination data account: {}", - source_address, - source_data_address, - destination_address, - destination_data_address, - ); - datapoint_warn!(datapoint_name, ("slot", bank.slot(), i64),); - return; - } + _ => Err(ReplaceAccountError::NotAnUpgradeableProgram), } + } else { + Err(ReplaceAccountError::NotAnUpgradeableProgram) } } else { - warn!( - "Unable to find source program {}. Destination: {}", - source_address, destination_address - ); - datapoint_warn!(datapoint_name, ("slot", bank.slot(), i64),); + Err(ReplaceAccountError::SourceAccountNotFound) } } diff --git a/runtime/src/bank/tests.rs b/runtime/src/bank/tests.rs index a2672e809d2a2a..f4c37780fd3508 100644 --- a/runtime/src/bank/tests.rs +++ b/runtime/src/bank/tests.rs @@ -9,7 +9,8 @@ use { crate::{ accounts_background_service::{PrunedBanksRequestHandler, SendDroppedBankCallback}, bank::replace_account::{ - replace_empty_account_with_upgradeable_program, replace_non_upgradeable_program_account, + replace_empty_account_with_upgradeable_program, + replace_non_upgradeable_program_account, ReplaceAccountError, }, bank_client::BankClient, epoch_rewards_hasher::hash_rewards_into_partitions, @@ -8071,7 +8072,8 @@ fn test_replace_non_upgradeable_program_account() { &source, &destination, "bank-apply_program_replacement", - ); + ) + .unwrap(); // Destination program account balance is now the source program account's balance assert_eq!(bank.get_balance(&destination), source_lamports); @@ -8188,7 +8190,8 @@ fn test_replace_empty_account_with_upgradeable_program_success( &source, &destination, "bank-apply_empty_account_replacement_for_program", - ); + ) + .unwrap(); // Destination program account was created and funded to pay for minimum rent // for the PDA @@ -8314,11 +8317,15 @@ fn test_replace_empty_account_with_upgradeable_program_fail_when_account_exists( let original_capitalization = bank.capitalization(); // Attempt the replacement - replace_empty_account_with_upgradeable_program( - &mut bank, - &source, - &destination, - "bank-apply_empty_account_replacement_for_program", + assert_eq!( + replace_empty_account_with_upgradeable_program( + &mut bank, + &source, + &destination, + "bank-apply_empty_account_replacement_for_program", + ) + .unwrap_err(), + ReplaceAccountError::DestinationAccountExists ); // Everything should be unchanged From 19b35de595901fd414d7eaf39a02f31ad85e4bcd Mon Sep 17 00:00:00 2001 From: Joe Date: Wed, 4 Oct 2023 14:57:13 +0200 Subject: [PATCH 32/33] refactor error handling --- runtime/src/bank/replace_account.rs | 244 +++++++++++++--------------- runtime/src/bank/tests.rs | 36 ++-- 2 files changed, 128 insertions(+), 152 deletions(-) diff --git a/runtime/src/bank/replace_account.rs b/runtime/src/bank/replace_account.rs index 1835e769413686..9fe91c3b0bddf0 100644 --- a/runtime/src/bank/replace_account.rs +++ b/runtime/src/bank/replace_account.rs @@ -12,23 +12,16 @@ use { }; /// Errors returned by `replace_account` methods -#[derive(Debug, Error, PartialEq)] +#[derive(Debug, Error)] pub enum ReplaceAccountError { - /// Source account not found - #[error("Source account not found")] - SourceAccountNotFound, - /// Source data account not found - #[error("Source data account not found")] - SourceDataAccountNotFound, - /// Destination account not found - #[error("Destination account not found")] - DestinationAccountNotFound, - /// Destination account exists - #[error("Destination account exists")] - DestinationAccountExists, - /// Unable to serialize program account state - #[error("Unable to serialize program account state")] - UnableToSerializeProgramAccountState, + /// Account not found + #[error("Account not found: {0:?}")] + AccountNotFound(Pubkey), + /// Account exists + #[error("Account exists: {0:?}")] + AccountExists(Pubkey), + #[error("Bincode Error: {0}")] + BincodeError(#[from] bincode::Error), /// Not an upgradeable program #[error("Not an upgradeable program")] NotAnUpgradeableProgram, @@ -38,7 +31,7 @@ pub enum ReplaceAccountError { /// `source`: the account to replace with /// `destination`: the account to be replaced fn move_account( - bank: &mut Bank, + bank: &Bank, source_address: &Pubkey, source_account: &V, destination_address: &Pubkey, @@ -75,36 +68,35 @@ fn move_account( /// `destination`: the non-upgradeable program account to be replaced #[allow(dead_code)] pub(crate) fn replace_non_upgradeable_program_account( - bank: &mut Bank, + bank: &Bank, source_address: &Pubkey, destination_address: &Pubkey, datapoint_name: &'static str, ) -> Result<(), ReplaceAccountError> { - if let Some(destination_account) = bank.get_account_with_fixed_root(destination_address) { - if let Some(source_account) = bank.get_account_with_fixed_root(source_address) { - datapoint_info!(datapoint_name, ("slot", bank.slot, i64)); - - move_account( - bank, - source_address, - &source_account, - destination_address, - Some(&destination_account), - ); - - // Unload a program from the bank's cache - bank.loaded_programs_cache - .write() - .unwrap() - .remove_programs([*destination_address].into_iter()); - - Ok(()) - } else { - Err(ReplaceAccountError::SourceAccountNotFound) - } - } else { - Err(ReplaceAccountError::DestinationAccountNotFound) - } + let destination_account = bank + .get_account_with_fixed_root(destination_address) + .ok_or(ReplaceAccountError::AccountNotFound(*destination_address))?; + let source_account = bank + .get_account_with_fixed_root(source_address) + .ok_or(ReplaceAccountError::AccountNotFound(*source_address))?; + + datapoint_info!(datapoint_name, ("slot", bank.slot, i64)); + + move_account( + bank, + source_address, + &source_account, + destination_address, + Some(&destination_account), + ); + + // Unload a program from the bank's cache + bank.loaded_programs_cache + .write() + .unwrap() + .remove_programs([*destination_address].into_iter()); + + Ok(()) } /// Use to replace an empty account with a program by feature activation @@ -114,102 +106,86 @@ pub(crate) fn replace_non_upgradeable_program_account( /// `source`: the upgradeable program account to replace with /// `destination`: the empty account to be replaced pub(crate) fn replace_empty_account_with_upgradeable_program( - bank: &mut Bank, + bank: &Bank, source_address: &Pubkey, destination_address: &Pubkey, datapoint_name: &'static str, ) -> Result<(), ReplaceAccountError> { // Must be attempting to replace an empty account with a program // account _and_ data account - if let Some(source_account) = bank.get_account_with_fixed_root(source_address) { - let (destination_data_address, _) = Pubkey::find_program_address( - &[destination_address.as_ref()], - &bpf_loader_upgradeable::id(), - ); - let (source_data_address, _) = - Pubkey::find_program_address(&[source_address.as_ref()], &bpf_loader_upgradeable::id()); - - // Make sure the data within the source account is the PDA of its - // data account. This also means it has at least the necessary - // lamports for rent. - if let Ok(source_state) = - bincode::deserialize::(source_account.data()) - { - match source_state { - UpgradeableLoaderState::Program { - programdata_address: _, - } => { - if let Some(source_data_account) = - bank.get_account_with_fixed_root(&source_data_address) - { - // Make sure the destination account is empty - // We aren't going to check that there isn't a data account at - // the known program-derived address (ie. `destination_data_address`), - // because if it exists, it will be overwritten - if bank - .get_account_with_fixed_root(destination_address) - .is_none() - { - let state = UpgradeableLoaderState::Program { - programdata_address: destination_data_address, - }; - if let Ok(data) = bincode::serialize(&state) { - let lamports = - bank.get_minimum_balance_for_rent_exemption(data.len()); - let created_program_account = Account { - lamports, - data, - owner: bpf_loader_upgradeable::id(), - executable: true, - rent_epoch: source_account.rent_epoch(), - }; - - datapoint_info!(datapoint_name, ("slot", bank.slot, i64)); - let change_in_capitalization = - source_account.lamports().saturating_sub(lamports); - - // Replace the destination data account with the source one - // If the destination data account does not exist, it will be created - // If it does exist, it will be overwritten - move_account( - bank, - &source_data_address, - &source_data_account, - &destination_data_address, - bank.get_account_with_fixed_root(&destination_data_address) - .as_ref(), - ); - - // Write the source data account's PDA into the destination program account - move_account( - bank, - source_address, - &created_program_account, - destination_address, - None::<&AccountSharedData>, - ); - - // Any remaining lamports in the source program account are burnt - bank.capitalization - .fetch_sub(change_in_capitalization, Relaxed); - - Ok(()) - } else { - Err(ReplaceAccountError::UnableToSerializeProgramAccountState) - } - } else { - Err(ReplaceAccountError::DestinationAccountExists) - } - } else { - Err(ReplaceAccountError::SourceDataAccountNotFound) - } - } - _ => Err(ReplaceAccountError::NotAnUpgradeableProgram), - } - } else { - Err(ReplaceAccountError::NotAnUpgradeableProgram) - } - } else { - Err(ReplaceAccountError::SourceAccountNotFound) + let source_account = bank + .get_account_with_fixed_root(source_address) + .ok_or(ReplaceAccountError::AccountNotFound(*source_address))?; + + let (destination_data_address, _) = Pubkey::find_program_address( + &[destination_address.as_ref()], + &bpf_loader_upgradeable::id(), + ); + let (source_data_address, _) = + Pubkey::find_program_address(&[source_address.as_ref()], &bpf_loader_upgradeable::id()); + + // Make sure the data within the source account is the PDA of its + // data account. This also means it has at least the necessary + // lamports for rent. + let source_state = bincode::deserialize::(source_account.data())?; + if !matches!(source_state, UpgradeableLoaderState::Program { .. }) { + return Err(ReplaceAccountError::NotAnUpgradeableProgram); + } + + let source_data_account = bank + .get_account_with_fixed_root(&source_data_address) + .ok_or(ReplaceAccountError::AccountNotFound(source_data_address))?; + + // Make sure the destination account is empty + // We aren't going to check that there isn't a data account at + // the known program-derived address (ie. `destination_data_address`), + // because if it exists, it will be overwritten + if bank + .get_account_with_fixed_root(destination_address) + .is_some() + { + return Err(ReplaceAccountError::AccountExists(*destination_address)); } + let state = UpgradeableLoaderState::Program { + programdata_address: destination_data_address, + }; + let data = bincode::serialize(&state)?; + let lamports = bank.get_minimum_balance_for_rent_exemption(data.len()); + let created_program_account = Account { + lamports, + data, + owner: bpf_loader_upgradeable::id(), + executable: true, + rent_epoch: source_account.rent_epoch(), + }; + + datapoint_info!(datapoint_name, ("slot", bank.slot, i64)); + let change_in_capitalization = source_account.lamports().saturating_sub(lamports); + + // Replace the destination data account with the source one + // If the destination data account does not exist, it will be created + // If it does exist, it will be overwritten + move_account( + bank, + &source_data_address, + &source_data_account, + &destination_data_address, + bank.get_account_with_fixed_root(&destination_data_address) + .as_ref(), + ); + + // Write the source data account's PDA into the destination program account + move_account( + bank, + source_address, + &created_program_account, + destination_address, + None::<&AccountSharedData>, + ); + + // Any remaining lamports in the source program account are burnt + bank.capitalization + .fetch_sub(change_in_capitalization, Relaxed); + + Ok(()) } diff --git a/runtime/src/bank/tests.rs b/runtime/src/bank/tests.rs index f4c37780fd3508..9732b79231f278 100644 --- a/runtime/src/bank/tests.rs +++ b/runtime/src/bank/tests.rs @@ -8008,7 +8008,7 @@ fn test_compute_active_feature_set() { } fn test_program_replace_set_up_account( - bank: &mut Bank, + bank: &Bank, pubkey: &Pubkey, lamports: u64, state: &T, @@ -8038,13 +8038,13 @@ fn test_replace_non_upgradeable_program_account() { // Should replace the destination program account with the source program account: // - Destination: [*Source program data] let bpf_id = bpf_loader::id(); - let mut bank = create_simple_test_bank(0); + let bank = create_simple_test_bank(0); let destination = Pubkey::new_unique(); let destination_state = vec![0u8; 4]; let destination_lamports = bank.get_minimum_balance_for_rent_exemption(destination_state.len()); test_program_replace_set_up_account( - &mut bank, + &bank, &destination, destination_lamports, &destination_state, @@ -8056,7 +8056,7 @@ fn test_replace_non_upgradeable_program_account() { let source_state = vec![6; 30]; let source_lamports = bank.get_minimum_balance_for_rent_exemption(source_state.len()); let check_source_account = test_program_replace_set_up_account( - &mut bank, + &bank, &source, source_lamports, &source_state, @@ -8068,7 +8068,7 @@ fn test_replace_non_upgradeable_program_account() { let original_capitalization = bank.capitalization(); replace_non_upgradeable_program_account( - &mut bank, + &bank, &source, &destination, "bank-apply_program_replacement", @@ -8128,7 +8128,7 @@ fn test_replace_empty_account_with_upgradeable_program_success( // // If the destination data account exists, it will be overwritten let bpf_upgradeable_id = bpf_loader_upgradeable::id(); - let mut bank = create_simple_test_bank(0); + let bank = create_simple_test_bank(0); // Create the test source accounts, one for program and one for data let source = Pubkey::new_unique(); @@ -8141,7 +8141,7 @@ fn test_replace_empty_account_with_upgradeable_program_success( let source_data_state = vec![6; 30]; let source_data_lamports = bank.get_minimum_balance_for_rent_exemption(source_data_state.len()); test_program_replace_set_up_account( - &mut bank, + &bank, &source, source_lamports, &source_state, @@ -8149,7 +8149,7 @@ fn test_replace_empty_account_with_upgradeable_program_success( true, ); let check_source_data_account = test_program_replace_set_up_account( - &mut bank, + &bank, &source_data, source_data_lamports, &source_data_state, @@ -8168,7 +8168,7 @@ fn test_replace_empty_account_with_upgradeable_program_success( let destination_data_lamports = bank.get_minimum_balance_for_rent_exemption(destination_data_state.len()); test_program_replace_set_up_account( - &mut bank, + &bank, &destination_data, destination_data_lamports, &destination_data_state, @@ -8186,7 +8186,7 @@ fn test_replace_empty_account_with_upgradeable_program_success( // Do the replacement replace_empty_account_with_upgradeable_program( - &mut bank, + &bank, &source, &destination, "bank-apply_empty_account_replacement_for_program", @@ -8250,14 +8250,14 @@ fn test_replace_empty_account_with_upgradeable_program_fail_when_account_exists( ) { // Should not be allowed to execute replacement let bpf_upgradeable_id = bpf_loader_upgradeable::id(); - let mut bank = create_simple_test_bank(0); + let bank = create_simple_test_bank(0); // Create the test destination account with some arbitrary data and lamports balance let destination = Pubkey::new_unique(); let destination_state = vec![0, 0, 0, 0]; // Arbitrary bytes, doesn't matter let destination_lamports = bank.get_minimum_balance_for_rent_exemption(destination_state.len()); let destination_account = test_program_replace_set_up_account( - &mut bank, + &bank, &destination, destination_lamports, &destination_state, @@ -8276,7 +8276,7 @@ fn test_replace_empty_account_with_upgradeable_program_fail_when_account_exists( let source_data_state = vec![6; 30]; let source_data_lamports = bank.get_minimum_balance_for_rent_exemption(source_data_state.len()); let source_account = test_program_replace_set_up_account( - &mut bank, + &bank, &source, source_lamports, &source_state, @@ -8284,7 +8284,7 @@ fn test_replace_empty_account_with_upgradeable_program_fail_when_account_exists( true, ); let source_data_account = test_program_replace_set_up_account( - &mut bank, + &bank, &source_data, source_data_lamports, &source_data_state, @@ -8302,7 +8302,7 @@ fn test_replace_empty_account_with_upgradeable_program_fail_when_account_exists( let destination_data_lamports = bank.get_minimum_balance_for_rent_exemption(destination_data_state.len()); let destination_data_account = test_program_replace_set_up_account( - &mut bank, + &bank, &destination_data, destination_data_lamports, &destination_data_state, @@ -8317,15 +8317,15 @@ fn test_replace_empty_account_with_upgradeable_program_fail_when_account_exists( let original_capitalization = bank.capitalization(); // Attempt the replacement - assert_eq!( + assert_matches!( replace_empty_account_with_upgradeable_program( - &mut bank, + &bank, &source, &destination, "bank-apply_empty_account_replacement_for_program", ) .unwrap_err(), - ReplaceAccountError::DestinationAccountExists + ReplaceAccountError::AccountExists(..) ); // Everything should be unchanged From 91bfec7a3e207811f395aac155388f96fcc358e7 Mon Sep 17 00:00:00 2001 From: Joe Date: Wed, 4 Oct 2023 15:23:31 +0200 Subject: [PATCH 33/33] add test for source program state --- runtime/src/bank/replace_account.rs | 2 +- runtime/src/bank/tests.rs | 65 +++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 1 deletion(-) diff --git a/runtime/src/bank/replace_account.rs b/runtime/src/bank/replace_account.rs index 9fe91c3b0bddf0..8d650aeebe7e87 100644 --- a/runtime/src/bank/replace_account.rs +++ b/runtime/src/bank/replace_account.rs @@ -116,7 +116,7 @@ pub(crate) fn replace_empty_account_with_upgradeable_program( let source_account = bank .get_account_with_fixed_root(source_address) .ok_or(ReplaceAccountError::AccountNotFound(*source_address))?; - + let (destination_data_address, _) = Pubkey::find_program_address( &[destination_address.as_ref()], &bpf_loader_upgradeable::id(), diff --git a/runtime/src/bank/tests.rs b/runtime/src/bank/tests.rs index 9732b79231f278..58ce790d43d0d4 100644 --- a/runtime/src/bank/tests.rs +++ b/runtime/src/bank/tests.rs @@ -8341,6 +8341,71 @@ fn test_replace_empty_account_with_upgradeable_program_fail_when_account_exists( assert_eq!(bank.capitalization(), original_capitalization); } +#[test] +fn test_replace_empty_account_with_upgradeable_program_fail_when_not_upgradeable_program() { + // Should not be allowed to execute replacement + let bpf_upgradeable_id = bpf_loader_upgradeable::id(); + let bank = create_simple_test_bank(0); + + // Create the test destination account with some arbitrary data and lamports balance + let destination = Pubkey::new_unique(); + let destination_state = vec![0, 0, 0, 0]; // Arbitrary bytes, doesn't matter + let destination_lamports = bank.get_minimum_balance_for_rent_exemption(destination_state.len()); + let destination_account = test_program_replace_set_up_account( + &bank, + &destination, + destination_lamports, + &destination_state, + &bpf_upgradeable_id, + true, + ); + + // Create the test source accounts, one for program and one for data + let source = Pubkey::new_unique(); + let (source_data, _) = Pubkey::find_program_address(&[source.as_ref()], &bpf_upgradeable_id); + let source_state = [0, 0, 0, 0]; // Arbitrary bytes, NOT an upgradeable program + let source_lamports = + bank.get_minimum_balance_for_rent_exemption(UpgradeableLoaderState::size_of_program()); + let source_data_state = vec![6; 30]; + let source_data_lamports = bank.get_minimum_balance_for_rent_exemption(source_data_state.len()); + let source_account = test_program_replace_set_up_account( + &bank, + &source, + source_lamports, + &source_state, + &bpf_upgradeable_id, + true, + ); + let source_data_account = test_program_replace_set_up_account( + &bank, + &source_data, + source_data_lamports, + &source_data_state, + &bpf_upgradeable_id, + false, + ); + + let original_capitalization = bank.capitalization(); + + // Attempt the replacement + assert_matches!( + replace_empty_account_with_upgradeable_program( + &bank, + &source, + &destination, + "bank-apply_empty_account_replacement_for_program", + ) + .unwrap_err(), + ReplaceAccountError::NotAnUpgradeableProgram + ); + + // Everything should be unchanged + assert_eq!(bank.get_account(&destination).unwrap(), destination_account); + assert_eq!(bank.get_account(&source).unwrap(), source_account); + assert_eq!(bank.get_account(&source_data).unwrap(), source_data_account); + assert_eq!(bank.capitalization(), original_capitalization); +} + fn min_rent_exempt_balance_for_sysvars(bank: &Bank, sysvar_ids: &[Pubkey]) -> u64 { sysvar_ids .iter()