From e1d05629646aa6ac46df6fc6388bfc5291141767 Mon Sep 17 00:00:00 2001 From: jeff washington Date: Wed, 7 Dec 2022 10:26:40 -0600 Subject: [PATCH] tweaks --- programs/sbf/c/src/invoked/invoked.c | 6 +- programs/sbf/rust/invoked/src/processor.rs | 12 +- runtime/src/accounts.rs | 797 +++++++++++---------- runtime/src/bank.rs | 309 ++++---- runtime/src/rent_collector.rs | 4 +- 5 files changed, 601 insertions(+), 527 deletions(-) diff --git a/programs/sbf/c/src/invoked/invoked.c b/programs/sbf/c/src/invoked/invoked.c index 3798b170ddf0b1..9a38bf73806c6d 100644 --- a/programs/sbf/c/src/invoked/invoked.c +++ b/programs/sbf/c/src/invoked/invoked.c @@ -45,7 +45,7 @@ extern uint64_t entrypoint(const uint8_t *input) { sol_assert(accounts[ARGUMENT_INDEX].data_len == 100); sol_assert(accounts[ARGUMENT_INDEX].is_signer); sol_assert(accounts[ARGUMENT_INDEX].is_writable); - sol_assert(accounts[ARGUMENT_INDEX].rent_epoch == 18446744073709551615ul); + sol_assert(accounts[ARGUMENT_INDEX].rent_epoch == ULLONG_MAX); sol_assert(!accounts[ARGUMENT_INDEX].executable); for (int i = 0; i < accounts[ARGUMENT_INDEX].data_len; i++) { sol_assert(accounts[ARGUMENT_INDEX].data[i] == i); @@ -57,7 +57,7 @@ extern uint64_t entrypoint(const uint8_t *input) { sol_assert(accounts[INVOKED_ARGUMENT_INDEX].data_len == 10); sol_assert(accounts[INVOKED_ARGUMENT_INDEX].is_signer); sol_assert(accounts[INVOKED_ARGUMENT_INDEX].is_writable); - sol_assert(accounts[INVOKED_ARGUMENT_INDEX].rent_epoch == 18446744073709551615ul); + sol_assert(accounts[INVOKED_ARGUMENT_INDEX].rent_epoch == ULLONG_MAX); sol_assert(!accounts[INVOKED_ARGUMENT_INDEX].executable); sol_assert( @@ -66,7 +66,7 @@ extern uint64_t entrypoint(const uint8_t *input) { &sbf_loader_id)); sol_assert(!accounts[INVOKED_PROGRAM_INDEX].is_signer); sol_assert(!accounts[INVOKED_PROGRAM_INDEX].is_writable); - sol_assert(accounts[INVOKED_PROGRAM_INDEX].rent_epoch == 18446744073709551615ul); + sol_assert(accounts[INVOKED_PROGRAM_INDEX].rent_epoch == ULLONG_MAX); sol_assert(accounts[INVOKED_PROGRAM_INDEX].executable); sol_assert(SolPubkey_same(accounts[INVOKED_PROGRAM_INDEX].key, diff --git a/programs/sbf/rust/invoked/src/processor.rs b/programs/sbf/rust/invoked/src/processor.rs index 27cd91b32493d7..aa4d2fe801730d 100644 --- a/programs/sbf/rust/invoked/src/processor.rs +++ b/programs/sbf/rust/invoked/src/processor.rs @@ -48,7 +48,7 @@ fn process_instruction( assert_eq!(accounts[ARGUMENT_INDEX].data_len(), 100); assert!(accounts[ARGUMENT_INDEX].is_signer); assert!(accounts[ARGUMENT_INDEX].is_writable); - assert_eq!(accounts[ARGUMENT_INDEX].rent_epoch, 18446744073709551615); + assert_eq!(accounts[ARGUMENT_INDEX].rent_epoch, u64::MAX); assert!(!accounts[ARGUMENT_INDEX].executable); { let data = accounts[ARGUMENT_INDEX].try_borrow_data()?; @@ -65,20 +65,14 @@ fn process_instruction( assert_eq!(accounts[INVOKED_ARGUMENT_INDEX].data_len(), 10); assert!(accounts[INVOKED_ARGUMENT_INDEX].is_signer); assert!(accounts[INVOKED_ARGUMENT_INDEX].is_writable); - assert_eq!( - accounts[INVOKED_ARGUMENT_INDEX].rent_epoch, - 18446744073709551615 - ); + assert_eq!(accounts[INVOKED_ARGUMENT_INDEX].rent_epoch, u64::MAX); assert!(!accounts[INVOKED_ARGUMENT_INDEX].executable); assert_eq!(accounts[INVOKED_PROGRAM_INDEX].key, program_id); assert_eq!(accounts[INVOKED_PROGRAM_INDEX].owner, &bpf_loader::id()); assert!(!accounts[INVOKED_PROGRAM_INDEX].is_signer); assert!(!accounts[INVOKED_PROGRAM_INDEX].is_writable); - assert_eq!( - accounts[INVOKED_PROGRAM_INDEX].rent_epoch, - 18446744073709551615 - ); + assert_eq!(accounts[INVOKED_PROGRAM_INDEX].rent_epoch, u64::MAX); assert!(accounts[INVOKED_PROGRAM_INDEX].executable); assert_eq!( diff --git a/runtime/src/accounts.rs b/runtime/src/accounts.rs index 01319d7b74bb76..d3d891377faaf1 100644 --- a/runtime/src/accounts.rs +++ b/runtime/src/accounts.rs @@ -1508,7 +1508,7 @@ mod tests { super::*, crate::{ bank::{DurableNonceFee, TransactionExecutionDetails}, - rent_collector::RentCollector, + rent_collector::{RentCollector, RENT_EXEMPT_RENT_EPOCH}, }, assert_matches::assert_matches, solana_address_lookup_table_program::state::LookupTableMeta, @@ -1634,7 +1634,7 @@ mod tests { ka: &[TransactionAccount], error_counters: &mut TransactionErrorMetrics, ) -> Vec { - load_accounts_with_excluded_features(tx, ka, error_counters, None) + load_accounts_with_fee(tx, ka, 0, error_counters, None) } fn load_accounts_with_excluded_features( @@ -1923,47 +1923,55 @@ mod tests { #[test] fn test_load_accounts_no_loaders() { - let mut accounts: Vec = Vec::new(); - let mut error_counters = TransactionErrorMetrics::default(); - - let keypair = Keypair::new(); - let key0 = keypair.pubkey(); - let key1 = Pubkey::new(&[5u8; 32]); - - let mut account = AccountSharedData::new(1, 0, &Pubkey::default()); - account.set_rent_epoch(1); - accounts.push((key0, account)); - - let mut account = AccountSharedData::new(2, 1, &Pubkey::default()); - account.set_rent_epoch(1); - accounts.push((key1, account)); - - let instructions = vec![CompiledInstruction::new(2, &(), vec![0, 1])]; - let tx = Transaction::new_with_compiled_instructions( - &[&keypair], - &[key1], - Hash::default(), - vec![native_loader::id()], - instructions, - ); + let initial_rent_epoch = 1; + for (expected_rent_epoch, set_exempt_rent_epoch_max) in + [(RENT_EXEMPT_RENT_EPOCH, true), (initial_rent_epoch, false)] + { + let mut accounts: Vec = Vec::new(); + let mut error_counters = TransactionErrorMetrics::default(); + + let keypair = Keypair::new(); + let key0 = keypair.pubkey(); + let key1 = Pubkey::new(&[5u8; 32]); + + let mut account = AccountSharedData::new(1, 0, &Pubkey::default()); + account.set_rent_epoch(initial_rent_epoch); + let mut expected_account = account.clone(); + expected_account.set_rent_epoch(expected_rent_epoch); + accounts.push((key0, account)); + + let mut account = AccountSharedData::new(2, 1, &Pubkey::default()); + account.set_rent_epoch(initial_rent_epoch); + accounts.push((key1, account)); + + let instructions = vec![CompiledInstruction::new(2, &(), vec![0, 1])]; + let tx = Transaction::new_with_compiled_instructions( + &[&keypair], + &[key1], + Hash::default(), + vec![native_loader::id()], + instructions, + ); - let loaded_accounts = load_accounts_with_excluded_features( - tx, - &accounts, - &mut error_counters, - Some(&[solana_sdk::feature_set::set_exempt_rent_epoch_max::id()]), - ); + let loaded_accounts = load_accounts_with_excluded_features( + tx, + &accounts, + &mut error_counters, + (!set_exempt_rent_epoch_max) + .then_some(&[solana_sdk::feature_set::set_exempt_rent_epoch_max::id()]), + ); - assert_eq!(error_counters.account_not_found, 0); - assert_eq!(loaded_accounts.len(), 1); - match &loaded_accounts[0] { - (Ok(loaded_transaction), _nonce) => { - assert_eq!(loaded_transaction.accounts.len(), 3); - assert_eq!(loaded_transaction.accounts[0].1, accounts[0].1); - assert_eq!(loaded_transaction.program_indices.len(), 1); - assert_eq!(loaded_transaction.program_indices[0].len(), 0); + assert_eq!(error_counters.account_not_found, 0); + assert_eq!(loaded_accounts.len(), 1); + match &loaded_accounts[0] { + (Ok(loaded_transaction), _nonce) => { + assert_eq!(loaded_transaction.accounts.len(), 3); + assert_eq!(loaded_transaction.accounts[0].1, expected_account); + assert_eq!(loaded_transaction.program_indices.len(), 1); + assert_eq!(loaded_transaction.program_indices[0].len(), 0); + } + (Err(e), _nonce) => Err(e).unwrap(), } - (Err(e), _nonce) => Err(e).unwrap(), } } @@ -2104,73 +2112,81 @@ mod tests { #[test] fn test_load_accounts_multiple_loaders() { - let mut accounts: Vec = Vec::new(); - let mut error_counters = TransactionErrorMetrics::default(); - - let keypair = Keypair::new(); - let key0 = keypair.pubkey(); - let key1 = Pubkey::new(&[5u8; 32]); - let key2 = Pubkey::new(&[6u8; 32]); - - let mut account = AccountSharedData::new(1, 0, &Pubkey::default()); - account.set_rent_epoch(1); - accounts.push((key0, account)); - - let mut account = AccountSharedData::new(40, 1, &Pubkey::default()); - account.set_executable(true); - account.set_rent_epoch(1); - account.set_owner(native_loader::id()); - accounts.push((key1, account)); - - let mut account = AccountSharedData::new(41, 1, &Pubkey::default()); - account.set_executable(true); - account.set_rent_epoch(1); - account.set_owner(key1); - accounts.push((key2, account)); - - let instructions = vec![ - CompiledInstruction::new(1, &(), vec![0]), - CompiledInstruction::new(2, &(), vec![0]), - ]; - let tx = Transaction::new_with_compiled_instructions( - &[&keypair], - &[], - Hash::default(), - vec![key1, key2], - instructions, - ); + let initial_rent_epoch = 1; + for (expected_rent_epoch, set_exempt_rent_epoch_max) in + [(RENT_EXEMPT_RENT_EPOCH, true), (initial_rent_epoch, false)] + { + let mut accounts: Vec = Vec::new(); + let mut error_counters = TransactionErrorMetrics::default(); + + let keypair = Keypair::new(); + let key0 = keypair.pubkey(); + let key1 = Pubkey::new(&[5u8; 32]); + let key2 = Pubkey::new(&[6u8; 32]); + + let mut account = AccountSharedData::new(1, 0, &Pubkey::default()); + account.set_rent_epoch(initial_rent_epoch); + let mut expected_account = account.clone(); + expected_account.set_rent_epoch(expected_rent_epoch); + accounts.push((key0, account)); + + let mut account = AccountSharedData::new(40, 1, &Pubkey::default()); + account.set_executable(true); + account.set_rent_epoch(initial_rent_epoch); + account.set_owner(native_loader::id()); + accounts.push((key1, account)); + + let mut account = AccountSharedData::new(41, 1, &Pubkey::default()); + account.set_executable(true); + account.set_rent_epoch(initial_rent_epoch); + account.set_owner(key1); + accounts.push((key2, account)); + + let instructions = vec![ + CompiledInstruction::new(1, &(), vec![0]), + CompiledInstruction::new(2, &(), vec![0]), + ]; + let tx = Transaction::new_with_compiled_instructions( + &[&keypair], + &[], + Hash::default(), + vec![key1, key2], + instructions, + ); - let loaded_accounts = load_accounts_with_excluded_features( - tx, - &accounts, - &mut error_counters, - Some(&[solana_sdk::feature_set::set_exempt_rent_epoch_max::id()]), - ); + let loaded_accounts = load_accounts_with_excluded_features( + tx, + &accounts, + &mut error_counters, + (!set_exempt_rent_epoch_max) + .then_some(&[solana_sdk::feature_set::set_exempt_rent_epoch_max::id()]), + ); - assert_eq!(error_counters.account_not_found, 0); - assert_eq!(loaded_accounts.len(), 1); - match &loaded_accounts[0] { - (Ok(loaded_transaction), _nonce) => { - assert_eq!(loaded_transaction.accounts.len(), 6); - assert_eq!(loaded_transaction.accounts[0].1, accounts[0].1); - assert_eq!(loaded_transaction.program_indices.len(), 2); - assert_eq!(loaded_transaction.program_indices[0].len(), 1); - assert_eq!(loaded_transaction.program_indices[1].len(), 2); - for program_indices in loaded_transaction.program_indices.iter() { - for (i, program_index) in program_indices.iter().enumerate() { - // +1 to skip first not loader account - assert_eq!( - loaded_transaction.accounts[*program_index as usize].0, - accounts[i + 1].0 - ); - assert_eq!( - loaded_transaction.accounts[*program_index as usize].1, - accounts[i + 1].1 - ); + assert_eq!(error_counters.account_not_found, 0); + assert_eq!(loaded_accounts.len(), 1); + match &loaded_accounts[0] { + (Ok(loaded_transaction), _nonce) => { + assert_eq!(loaded_transaction.accounts.len(), 6); + assert_eq!(loaded_transaction.accounts[0].1, expected_account); + assert_eq!(loaded_transaction.program_indices.len(), 2); + assert_eq!(loaded_transaction.program_indices[0].len(), 1); + assert_eq!(loaded_transaction.program_indices[1].len(), 2); + for program_indices in loaded_transaction.program_indices.iter() { + for (i, program_index) in program_indices.iter().enumerate() { + // +1 to skip first not loader account + assert_eq!( + loaded_transaction.accounts[*program_index as usize].0, + accounts[i + 1].0 + ); + assert_eq!( + loaded_transaction.accounts[*program_index as usize].1, + accounts[i + 1].1 + ); + } } } + (Err(e), _nonce) => Err(e).unwrap(), } - (Err(e), _nonce) => Err(e).unwrap(), } } @@ -2344,295 +2360,344 @@ mod tests { #[test] fn test_load_accounts_executable_with_write_lock() { - let mut accounts: Vec = Vec::new(); - let mut error_counters = TransactionErrorMetrics::default(); - - let keypair = Keypair::new(); - let key0 = keypair.pubkey(); - let key1 = Pubkey::new(&[5u8; 32]); - let key2 = Pubkey::new(&[6u8; 32]); - - let mut account = AccountSharedData::new(1, 0, &Pubkey::default()); - account.set_rent_epoch(1); - accounts.push((key0, account)); - - let mut account = AccountSharedData::new(40, 1, &native_loader::id()); - account.set_executable(true); - account.set_rent_epoch(1); - accounts.push((key1, account)); - - let mut account = AccountSharedData::new(40, 1, &native_loader::id()); - account.set_executable(true); - account.set_rent_epoch(1); - accounts.push((key2, account)); - - let instructions = vec![CompiledInstruction::new(2, &(), vec![0, 1])]; - let mut message = Message::new_with_compiled_instructions( - 1, - 0, - 1, // only one executable marked as readonly - vec![key0, key1, key2], - Hash::default(), - instructions, - ); - let tx = Transaction::new(&[&keypair], message.clone(), Hash::default()); - let loaded_accounts = load_accounts_with_excluded_features( - tx, - &accounts, - &mut error_counters, - Some(&[solana_sdk::feature_set::set_exempt_rent_epoch_max::id()]), - ); + let initial_rent_epoch = 1; + for (expected_rent_epoch, set_exempt_rent_epoch_max) in + [(RENT_EXEMPT_RENT_EPOCH, true), (initial_rent_epoch, false)] + { + let mut accounts: Vec = Vec::new(); + let mut error_counters = TransactionErrorMetrics::default(); + + let keypair = Keypair::new(); + let key0 = keypair.pubkey(); + let key1 = Pubkey::new(&[5u8; 32]); + let key2 = Pubkey::new(&[6u8; 32]); + + let mut account = AccountSharedData::new(1, 0, &Pubkey::default()); + account.set_rent_epoch(initial_rent_epoch); + accounts.push((key0, account)); + + let mut account = AccountSharedData::new(40, 1, &native_loader::id()); + account.set_executable(true); + account.set_rent_epoch(initial_rent_epoch); + accounts.push((key1, account)); + + let mut account = AccountSharedData::new(40, 1, &native_loader::id()); + account.set_executable(true); + account.set_rent_epoch(initial_rent_epoch); + accounts.push((key2, account)); + + let mut expected_accounts = accounts.clone(); + expected_accounts[0].1.set_rent_epoch(expected_rent_epoch); + // expected_accounts[1] is native loader. Its rent_epoch doesn't change + + let instructions = vec![CompiledInstruction::new(2, &(), vec![0, 1])]; + let mut message = Message::new_with_compiled_instructions( + 1, + 0, + 1, // only one executable marked as readonly + vec![key0, key1, key2], + Hash::default(), + instructions, + ); + let tx = Transaction::new(&[&keypair], message.clone(), Hash::default()); + let loaded_accounts = load_accounts_with_excluded_features( + tx, + &accounts, + &mut error_counters, + (!set_exempt_rent_epoch_max) + .then_some(&[solana_sdk::feature_set::set_exempt_rent_epoch_max::id()]), + ); - assert_eq!(error_counters.invalid_writable_account, 1); - assert_eq!(loaded_accounts.len(), 1); - assert_eq!( - loaded_accounts[0], - (Err(TransactionError::InvalidWritableAccount), None) - ); + assert_eq!(error_counters.invalid_writable_account, 1); + assert_eq!(loaded_accounts.len(), 1); + assert_eq!( + loaded_accounts[0], + (Err(TransactionError::InvalidWritableAccount), None) + ); - // Mark executables as readonly - message.account_keys = vec![key0, key1, key2]; // revert key change - message.header.num_readonly_unsigned_accounts = 2; // mark both executables as readonly - let tx = Transaction::new(&[&keypair], message, Hash::default()); - let loaded_accounts = load_accounts_with_excluded_features( - tx, - &accounts, - &mut error_counters, - Some(&[solana_sdk::feature_set::set_exempt_rent_epoch_max::id()]), - ); + // Mark executables as readonly + message.account_keys = vec![key0, key1, key2]; // revert key change + message.header.num_readonly_unsigned_accounts = 2; // mark both executables as readonly + let tx = Transaction::new(&[&keypair], message, Hash::default()); + let loaded_accounts = load_accounts_with_excluded_features( + tx, + &accounts, + &mut error_counters, + (!set_exempt_rent_epoch_max) + .then_some(&[solana_sdk::feature_set::set_exempt_rent_epoch_max::id()]), + ); - assert_eq!(error_counters.invalid_writable_account, 1); - assert_eq!(loaded_accounts.len(), 1); - let result = loaded_accounts[0].0.as_ref().unwrap(); - assert_eq!(result.accounts[..2], accounts[..2]); - assert_eq!( - result.accounts[result.program_indices[0][0] as usize], - accounts[2] - ); + assert_eq!(error_counters.invalid_writable_account, 1); + assert_eq!(loaded_accounts.len(), 1); + let result = loaded_accounts[0].0.as_ref().unwrap(); + assert_eq!( + result.accounts[..2], + expected_accounts[..2], + "{}, {}", + set_exempt_rent_epoch_max, + expected_rent_epoch + ); + assert_eq!( + result.accounts[result.program_indices[0][0] as usize], + accounts[2] + ); + } } #[test] fn test_load_accounts_upgradeable_with_write_lock() { - let mut accounts: Vec = Vec::new(); - let mut error_counters = TransactionErrorMetrics::default(); - - let keypair = Keypair::new(); - let key0 = keypair.pubkey(); - let key1 = Pubkey::new(&[5u8; 32]); - let key2 = Pubkey::new(&[6u8; 32]); - let programdata_key1 = Pubkey::new(&[7u8; 32]); - let programdata_key2 = Pubkey::new(&[8u8; 32]); - - let mut account = AccountSharedData::new(1, 0, &Pubkey::default()); - account.set_rent_epoch(1); - accounts.push((key0, account)); - - let program_data = UpgradeableLoaderState::ProgramData { - slot: 42, - upgrade_authority_address: None, - }; - - let program = UpgradeableLoaderState::Program { - programdata_address: programdata_key1, - }; - let mut account = - AccountSharedData::new_data(40, &program, &bpf_loader_upgradeable::id()).unwrap(); - account.set_executable(true); - account.set_rent_epoch(1); - accounts.push((key1, account)); - let mut account = - AccountSharedData::new_data(40, &program_data, &bpf_loader_upgradeable::id()).unwrap(); - account.set_rent_epoch(1); - accounts.push((programdata_key1, account)); - - let program = UpgradeableLoaderState::Program { - programdata_address: programdata_key2, - }; - let mut account = - AccountSharedData::new_data(40, &program, &bpf_loader_upgradeable::id()).unwrap(); - account.set_executable(true); - account.set_rent_epoch(1); - accounts.push((key2, account)); - let mut account = - AccountSharedData::new_data(40, &program_data, &bpf_loader_upgradeable::id()).unwrap(); - account.set_rent_epoch(1); - accounts.push((programdata_key2, account)); - - let mut account = AccountSharedData::new(40, 1, &native_loader::id()); // create mock bpf_loader_upgradeable - account.set_executable(true); - account.set_rent_epoch(1); - accounts.push((bpf_loader_upgradeable::id(), account)); + let initial_rent_epoch = 1; + for (expected_rent_epoch, set_exempt_rent_epoch_max) in + [(RENT_EXEMPT_RENT_EPOCH, true), (initial_rent_epoch, false)] + { + let mut accounts: Vec = Vec::new(); + let mut error_counters = TransactionErrorMetrics::default(); + + let keypair = Keypair::new(); + let key0 = keypair.pubkey(); + let key1 = Pubkey::new(&[5u8; 32]); + let key2 = Pubkey::new(&[6u8; 32]); + let programdata_key1 = Pubkey::new(&[7u8; 32]); + let programdata_key2 = Pubkey::new(&[8u8; 32]); + + let mut account = AccountSharedData::new(1, 0, &Pubkey::default()); + account.set_rent_epoch(initial_rent_epoch); + accounts.push((key0, account)); + + let program_data = UpgradeableLoaderState::ProgramData { + slot: 42, + upgrade_authority_address: None, + }; - let instructions = vec![CompiledInstruction::new(2, &(), vec![0, 1])]; - let mut message = Message::new_with_compiled_instructions( - 1, - 0, - 1, // only one executable marked as readonly - vec![key0, key1, key2], - Hash::default(), - instructions, - ); - let tx = Transaction::new(&[&keypair], message.clone(), Hash::default()); - let loaded_accounts = load_accounts_with_excluded_features( - tx, - &accounts, - &mut error_counters, - Some(&[solana_sdk::feature_set::set_exempt_rent_epoch_max::id()]), - ); + let program = UpgradeableLoaderState::Program { + programdata_address: programdata_key1, + }; + let mut account = + AccountSharedData::new_data(40, &program, &bpf_loader_upgradeable::id()).unwrap(); + account.set_executable(true); + account.set_rent_epoch(initial_rent_epoch); + accounts.push((key1, account)); + let mut account = + AccountSharedData::new_data(40, &program_data, &bpf_loader_upgradeable::id()) + .unwrap(); + account.set_rent_epoch(initial_rent_epoch); + accounts.push((programdata_key1, account)); + let mut expected_accounts = accounts.clone(); + expected_accounts[0].1.set_rent_epoch(expected_rent_epoch); + expected_accounts[1].1.set_rent_epoch(expected_rent_epoch); + let program = UpgradeableLoaderState::Program { + programdata_address: programdata_key2, + }; + let mut account = + AccountSharedData::new_data(40, &program, &bpf_loader_upgradeable::id()).unwrap(); + account.set_executable(true); + account.set_rent_epoch(initial_rent_epoch); + accounts.push((key2, account)); + let mut account = + AccountSharedData::new_data(40, &program_data, &bpf_loader_upgradeable::id()) + .unwrap(); + account.set_rent_epoch(initial_rent_epoch); + accounts.push((programdata_key2, account)); + + let mut account = AccountSharedData::new(40, 1, &native_loader::id()); // create mock bpf_loader_upgradeable + account.set_executable(true); + account.set_rent_epoch(initial_rent_epoch); + accounts.push((bpf_loader_upgradeable::id(), account)); + + let instructions = vec![CompiledInstruction::new(2, &(), vec![0, 1])]; + let mut message = Message::new_with_compiled_instructions( + 1, + 0, + 1, // only one executable marked as readonly + vec![key0, key1, key2], + Hash::default(), + instructions, + ); + let tx = Transaction::new(&[&keypair], message.clone(), Hash::default()); + let loaded_accounts = + load_accounts_with_excluded_features(tx, &accounts, &mut error_counters, None); - assert_eq!(error_counters.invalid_writable_account, 1); - assert_eq!(loaded_accounts.len(), 1); - assert_eq!( - loaded_accounts[0], - (Err(TransactionError::InvalidWritableAccount), None) - ); + assert_eq!(error_counters.invalid_writable_account, 1); + assert_eq!(loaded_accounts.len(), 1); + assert_eq!( + loaded_accounts[0], + (Err(TransactionError::InvalidWritableAccount), None) + ); - // Solution 1: include bpf_loader_upgradeable account - message.account_keys = vec![key0, key1, bpf_loader_upgradeable::id()]; - let tx = Transaction::new(&[&keypair], message.clone(), Hash::default()); - let loaded_accounts = load_accounts_with_excluded_features( - tx, - &accounts, - &mut error_counters, - Some(&[solana_sdk::feature_set::set_exempt_rent_epoch_max::id()]), - ); + // Solution 1: include bpf_loader_upgradeable account + message.account_keys = vec![key0, key1, bpf_loader_upgradeable::id()]; + let tx = Transaction::new(&[&keypair], message.clone(), Hash::default()); + let loaded_accounts = load_accounts_with_excluded_features( + tx, + &accounts, + &mut error_counters, + (!set_exempt_rent_epoch_max) + .then_some(&[solana_sdk::feature_set::set_exempt_rent_epoch_max::id()]), + ); - assert_eq!(error_counters.invalid_writable_account, 1); - assert_eq!(loaded_accounts.len(), 1); - let result = loaded_accounts[0].0.as_ref().unwrap(); - assert_eq!(result.accounts[..2], accounts[..2]); - assert_eq!( - result.accounts[result.program_indices[0][0] as usize], - accounts[5] - ); + assert_eq!(error_counters.invalid_writable_account, 1); + assert_eq!(loaded_accounts.len(), 1); + let result = loaded_accounts[0].0.as_ref().unwrap(); + assert_eq!(result.accounts[..2], expected_accounts[..2]); + assert_eq!( + result.accounts[result.program_indices[0][0] as usize], + accounts[5] + ); - // Solution 2: mark programdata as readonly - message.account_keys = vec![key0, key1, key2]; // revert key change - message.header.num_readonly_unsigned_accounts = 2; // mark both executables as readonly - let tx = Transaction::new(&[&keypair], message, Hash::default()); - let loaded_accounts = load_accounts_with_excluded_features( - tx, - &accounts, - &mut error_counters, - Some(&[solana_sdk::feature_set::set_exempt_rent_epoch_max::id()]), - ); + // Solution 2: mark programdata as readonly + message.account_keys = vec![key0, key1, key2]; // revert key change + message.header.num_readonly_unsigned_accounts = 2; // mark both executables as readonly + let tx = Transaction::new(&[&keypair], message, Hash::default()); + let loaded_accounts = load_accounts_with_excluded_features( + tx, + &accounts, + &mut error_counters, + (!set_exempt_rent_epoch_max) + .then_some(&[solana_sdk::feature_set::set_exempt_rent_epoch_max::id()]), + ); - assert_eq!(error_counters.invalid_writable_account, 1); - assert_eq!(loaded_accounts.len(), 1); - let result = loaded_accounts[0].0.as_ref().unwrap(); - assert_eq!(result.accounts[..2], accounts[..2]); - assert_eq!( - result.accounts[result.program_indices[0][0] as usize], - accounts[5] - ); - assert_eq!( - result.accounts[result.program_indices[0][1] as usize], - accounts[4] - ); - assert_eq!( - result.accounts[result.program_indices[0][2] as usize], - accounts[3] - ); + assert_eq!(error_counters.invalid_writable_account, 1); + assert_eq!(loaded_accounts.len(), 1); + let result = loaded_accounts[0].0.as_ref().unwrap(); + assert_eq!(result.accounts[..2], expected_accounts[..2]); + assert_eq!( + result.accounts[result.program_indices[0][0] as usize], + accounts[5] + ); + assert_eq!( + result.accounts[result.program_indices[0][1] as usize], + accounts[4] + ); + assert_eq!( + result.accounts[result.program_indices[0][2] as usize], + accounts[3] + ); + } } #[test] fn test_load_accounts_programdata_with_write_lock() { - let mut accounts: Vec = Vec::new(); - let mut error_counters = TransactionErrorMetrics::default(); - - let keypair = Keypair::new(); - let key0 = keypair.pubkey(); - let key1 = Pubkey::new(&[5u8; 32]); - let key2 = Pubkey::new(&[6u8; 32]); - - let mut account = AccountSharedData::new(1, 0, &Pubkey::default()); - account.set_rent_epoch(1); - accounts.push((key0, account)); + let initial_rent_epoch = 1; + for (expected_rent_epoch, set_exempt_rent_epoch_max) in + [(RENT_EXEMPT_RENT_EPOCH, true), (initial_rent_epoch, false)] + { + let mut accounts: Vec = Vec::new(); + let mut error_counters = TransactionErrorMetrics::default(); - let program_data = UpgradeableLoaderState::ProgramData { - slot: 42, - upgrade_authority_address: None, - }; - let mut account = - AccountSharedData::new_data(40, &program_data, &bpf_loader_upgradeable::id()).unwrap(); - account.set_rent_epoch(1); - accounts.push((key1, account)); + let keypair = Keypair::new(); + let key0 = keypair.pubkey(); + let key1 = Pubkey::new(&[5u8; 32]); + let key2 = Pubkey::new(&[6u8; 32]); - let mut account = AccountSharedData::new(40, 1, &native_loader::id()); - account.set_executable(true); - account.set_rent_epoch(1); - accounts.push((key2, account)); + let mut account = AccountSharedData::new(1, 0, &Pubkey::default()); + account.set_rent_epoch(initial_rent_epoch); + accounts.push((key0, account)); - let instructions = vec![CompiledInstruction::new(2, &(), vec![0, 1])]; - let mut message = Message::new_with_compiled_instructions( - 1, - 0, - 1, // only the program marked as readonly - vec![key0, key1, key2], - Hash::default(), - instructions, - ); - let tx = Transaction::new(&[&keypair], message.clone(), Hash::default()); - let loaded_accounts = load_accounts_with_excluded_features( - tx, - &accounts, - &mut error_counters, - Some(&[solana_sdk::feature_set::set_exempt_rent_epoch_max::id()]), - ); + let program_data = UpgradeableLoaderState::ProgramData { + slot: 42, + upgrade_authority_address: None, + }; + let mut account = + AccountSharedData::new_data(40, &program_data, &bpf_loader_upgradeable::id()) + .unwrap(); + account.set_rent_epoch(initial_rent_epoch); + accounts.push((key1, account)); + let mut expected_accounts = accounts.clone(); + + expected_accounts[0].1.set_rent_epoch(expected_rent_epoch); + + let mut account = AccountSharedData::new(40, 1, &native_loader::id()); + account.set_executable(true); + account.set_rent_epoch(initial_rent_epoch); + accounts.push((key2, account)); + + let instructions = vec![CompiledInstruction::new(2, &(), vec![0, 1])]; + let mut message = Message::new_with_compiled_instructions( + 1, + 0, + 1, // only the program marked as readonly + vec![key0, key1, key2], + Hash::default(), + instructions, + ); + let tx = Transaction::new(&[&keypair], message.clone(), Hash::default()); + let loaded_accounts = load_accounts_with_excluded_features( + tx, + &accounts, + &mut error_counters, + (!set_exempt_rent_epoch_max) + .then_some(&[solana_sdk::feature_set::set_exempt_rent_epoch_max::id()]), + ); - assert_eq!(error_counters.invalid_writable_account, 1); - assert_eq!(loaded_accounts.len(), 1); - assert_eq!( - loaded_accounts[0], - (Err(TransactionError::InvalidWritableAccount), None) - ); + assert_eq!(error_counters.invalid_writable_account, 1); + assert_eq!(loaded_accounts.len(), 1); + assert_eq!( + loaded_accounts[0], + (Err(TransactionError::InvalidWritableAccount), None) + ); - // Solution 1: include bpf_loader_upgradeable account - let mut account = AccountSharedData::new(40, 1, &native_loader::id()); // create mock bpf_loader_upgradeable - account.set_executable(true); - account.set_rent_epoch(1); - let accounts_with_upgradeable_loader = vec![ - accounts[0].clone(), - accounts[1].clone(), - (bpf_loader_upgradeable::id(), account), - ]; - message.account_keys = vec![key0, key1, bpf_loader_upgradeable::id()]; - let tx = Transaction::new(&[&keypair], message.clone(), Hash::default()); - let loaded_accounts = load_accounts_with_excluded_features( - tx, - &accounts_with_upgradeable_loader, - &mut error_counters, - Some(&[solana_sdk::feature_set::set_exempt_rent_epoch_max::id()]), - ); + // Solution 1: include bpf_loader_upgradeable account + let mut account = AccountSharedData::new(40, 1, &native_loader::id()); // create mock bpf_loader_upgradeable + account.set_executable(true); + account.set_rent_epoch(initial_rent_epoch); + let accounts_with_upgradeable_loader = vec![ + accounts[0].clone(), + accounts[1].clone(), + (bpf_loader_upgradeable::id(), account), + ]; + let mut accounts_with_upgradeable_loader_expected = + accounts_with_upgradeable_loader.clone(); + accounts_with_upgradeable_loader_expected + .iter_mut() + .for_each(|account| account.1.set_rent_epoch(expected_rent_epoch)); + message.account_keys = vec![key0, key1, bpf_loader_upgradeable::id()]; + let tx = Transaction::new(&[&keypair], message.clone(), Hash::default()); + let loaded_accounts = load_accounts_with_excluded_features( + tx, + &accounts_with_upgradeable_loader, + &mut error_counters, + (!set_exempt_rent_epoch_max) + .then_some(&[solana_sdk::feature_set::set_exempt_rent_epoch_max::id()]), + ); - assert_eq!(error_counters.invalid_writable_account, 1); - assert_eq!(loaded_accounts.len(), 1); - let result = loaded_accounts[0].0.as_ref().unwrap(); - assert_eq!(result.accounts[..2], accounts_with_upgradeable_loader[..2]); - assert_eq!( - result.accounts[result.program_indices[0][0] as usize], - accounts_with_upgradeable_loader[2] - ); + assert_eq!(error_counters.invalid_writable_account, 1); + assert_eq!(loaded_accounts.len(), 1); + let result = loaded_accounts[0].0.as_ref().unwrap(); + assert_eq!( + result.accounts[..2], + accounts_with_upgradeable_loader_expected[..2] + ); + assert_eq!( + result.accounts[result.program_indices[0][0] as usize], + accounts_with_upgradeable_loader[2] + ); - // Solution 2: mark programdata as readonly - message.account_keys = vec![key0, key1, key2]; // revert key change - message.header.num_readonly_unsigned_accounts = 2; // extend readonly set to include programdata - let tx = Transaction::new(&[&keypair], message, Hash::default()); - let loaded_accounts = load_accounts_with_excluded_features( - tx, - &accounts, - &mut error_counters, - Some(&[solana_sdk::feature_set::set_exempt_rent_epoch_max::id()]), - ); + // Solution 2: mark programdata as readonly + message.account_keys = vec![key0, key1, key2]; // revert key change + message.header.num_readonly_unsigned_accounts = 2; // extend readonly set to include programdata + let tx = Transaction::new(&[&keypair], message, Hash::default()); + let loaded_accounts = load_accounts_with_excluded_features( + tx, + &accounts, + &mut error_counters, + (!set_exempt_rent_epoch_max) + .then_some(&[solana_sdk::feature_set::set_exempt_rent_epoch_max::id()]), + ); - assert_eq!(error_counters.invalid_writable_account, 1); - assert_eq!(loaded_accounts.len(), 1); - let result = loaded_accounts[0].0.as_ref().unwrap(); - assert_eq!(result.accounts[..2], accounts[..2]); - assert_eq!( - result.accounts[result.program_indices[0][0] as usize], - accounts[2] - ); + assert_eq!(error_counters.invalid_writable_account, 1); + assert_eq!(loaded_accounts.len(), 1); + let result = loaded_accounts[0].0.as_ref().unwrap(); + assert_eq!( + result.accounts[..2], + expected_accounts[..2], + "{}", + set_exempt_rent_epoch_max + ); + assert_eq!( + result.accounts[result.program_indices[0][0] as usize], + accounts[2] + ); + } } #[test] diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 571d730aca5cf1..6659999ff8cf6b 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -8058,7 +8058,7 @@ pub(crate) mod tests { genesis_sysvar_and_builtin_program_lamports, GenesisConfigInfo, ValidatorVoteKeypairs, }, - rent_collector::TEST_SET_EXEMPT_RENT_EPOCH_MAX, + rent_collector::{RENT_EXEMPT_RENT_EPOCH, TEST_SET_EXEMPT_RENT_EPOCH_MAX}, rent_paying_accounts_by_partition::RentPayingAccountsByPartition, status_cache::MAX_CACHE_ENTRIES, }, @@ -12040,159 +12040,174 @@ pub(crate) mod tests { #[test] fn test_bank_update_sysvar_account() { - solana_logger::setup(); - // flushing the write cache is destructive, so test has to restart each time we flush and want to do 'illegal' operations once flushed - for pass in 0..5 { - use sysvar::clock::Clock; + let initial_rent_epoch = 1; + for (expected_rent_epoch, set_exempt_rent_epoch_max) in + [(RENT_EXEMPT_RENT_EPOCH, true), (initial_rent_epoch, false)] + { + solana_logger::setup(); + // flushing the write cache is destructive, so test has to restart each time we flush and want to do 'illegal' operations once flushed + for pass in 0..5 { + use sysvar::clock::Clock; + + let dummy_clock_id = solana_sdk::pubkey::new_rand(); + let dummy_rent_epoch = 44; + let (mut genesis_config, _mint_keypair) = create_genesis_config(500); + + let expected_previous_slot = 3; + let mut expected_next_slot = expected_previous_slot + 1; + + // First, initialize the clock sysvar + for feature_id in FeatureSet::default().inactive { + if !set_exempt_rent_epoch_max { + if feature_id == solana_sdk::feature_set::set_exempt_rent_epoch_max::id() { + continue; + } + } + activate_feature(&mut genesis_config, feature_id); + } + let bank1 = Arc::new(Bank::new_for_tests_with_config( + &genesis_config, + bank_test_config_caching_enabled(), + )); + if pass == 0 { + add_root_and_flush_write_cache(&bank1); + assert_eq!(bank1.calculate_capitalization(true), bank1.capitalization()); + continue; + } - let dummy_clock_id = solana_sdk::pubkey::new_rand(); - let dummy_rent_epoch = 44; - let (mut genesis_config, _mint_keypair) = create_genesis_config(500); + assert_capitalization_diff( + &bank1, + || { + bank1.update_sysvar_account(&dummy_clock_id, |optional_account| { + assert!(optional_account.is_none()); + + let mut account = create_account( + &Clock { + slot: expected_previous_slot, + ..Clock::default() + }, + bank1.inherit_specially_retained_account_fields(optional_account), + ); + account.set_rent_epoch(dummy_rent_epoch); + account + }); + let current_account = bank1.get_account(&dummy_clock_id).unwrap(); + assert_eq!( + expected_previous_slot, + from_account::(¤t_account).unwrap().slot + ); + assert_eq!(dummy_rent_epoch, current_account.rent_epoch()); + }, + |old, new| { + assert_eq!( + old + min_rent_exempt_balance_for_sysvars( + &bank1, + &[sysvar::clock::id()] + ), + new + ); + pass == 1 + }, + ); + if pass == 1 { + continue; + } - let expected_previous_slot = 3; - let mut expected_next_slot = expected_previous_slot + 1; + assert_capitalization_diff( + &bank1, + || { + bank1.update_sysvar_account(&dummy_clock_id, |optional_account| { + assert!(optional_account.is_some()); + + create_account( + &Clock { + slot: expected_previous_slot, + ..Clock::default() + }, + bank1.inherit_specially_retained_account_fields(optional_account), + ) + }) + }, + |old, new| { + // creating new sysvar twice in a slot shouldn't increment capitalization twice + assert_eq!(old, new); + pass == 2 + }, + ); + if pass == 2 { + continue; + } - // First, initialize the clock sysvar - activate_all_features(&mut genesis_config); - let bank1 = Arc::new(Bank::new_for_tests_with_config( - &genesis_config, - bank_test_config_caching_enabled(), - )); - if pass == 0 { + // Updating should increment the clock's slot + let bank2 = Arc::new(Bank::new_from_parent(&bank1, &Pubkey::default(), 1)); add_root_and_flush_write_cache(&bank1); - assert_eq!(bank1.calculate_capitalization(true), bank1.capitalization()); - continue; - } - - assert_capitalization_diff( - &bank1, - || { - bank1.update_sysvar_account(&dummy_clock_id, |optional_account| { - assert!(optional_account.is_none()); - - let mut account = create_account( - &Clock { - slot: expected_previous_slot, - ..Clock::default() - }, - bank1.inherit_specially_retained_account_fields(optional_account), + assert_capitalization_diff( + &bank2, + || { + bank2.update_sysvar_account(&dummy_clock_id, |optional_account| { + let slot = from_account::(optional_account.as_ref().unwrap()) + .unwrap() + .slot + + 1; + + create_account( + &Clock { + slot, + ..Clock::default() + }, + bank2.inherit_specially_retained_account_fields(optional_account), + ) + }); + let current_account = bank2.get_account(&dummy_clock_id).unwrap(); + assert_eq!( + expected_next_slot, + from_account::(¤t_account).unwrap().slot ); - account.set_rent_epoch(dummy_rent_epoch); - account - }); - let current_account = bank1.get_account(&dummy_clock_id).unwrap(); - assert_eq!( - expected_previous_slot, - from_account::(¤t_account).unwrap().slot - ); - assert_eq!(dummy_rent_epoch, current_account.rent_epoch()); - }, - |old, new| { - assert_eq!( - old + min_rent_exempt_balance_for_sysvars(&bank1, &[sysvar::clock::id()]), - new - ); - pass == 1 - }, - ); - if pass == 1 { - continue; - } - - assert_capitalization_diff( - &bank1, - || { - bank1.update_sysvar_account(&dummy_clock_id, |optional_account| { - assert!(optional_account.is_some()); - - create_account( - &Clock { - slot: expected_previous_slot, - ..Clock::default() - }, - bank1.inherit_specially_retained_account_fields(optional_account), - ) - }) - }, - |old, new| { - // creating new sysvar twice in a slot shouldn't increment capitalization twice - assert_eq!(old, new); - pass == 2 - }, - ); - if pass == 2 { - continue; - } + assert_eq!(/*expected_rent_epoch*/dummy_rent_epoch, current_account.rent_epoch()); + }, + |old, new| { + // if existing, capitalization shouldn't change + assert_eq!(old, new); + pass == 3 + }, + ); + if pass == 3 { + continue; + } - // Updating should increment the clock's slot - let bank2 = Arc::new(Bank::new_from_parent(&bank1, &Pubkey::default(), 1)); - add_root_and_flush_write_cache(&bank1); - assert_capitalization_diff( - &bank2, - || { - bank2.update_sysvar_account(&dummy_clock_id, |optional_account| { - let slot = from_account::(optional_account.as_ref().unwrap()) - .unwrap() - .slot - + 1; - - create_account( - &Clock { - slot, - ..Clock::default() - }, - bank2.inherit_specially_retained_account_fields(optional_account), - ) - }); - let current_account = bank2.get_account(&dummy_clock_id).unwrap(); - assert_eq!( - expected_next_slot, - from_account::(¤t_account).unwrap().slot - ); - assert_eq!(dummy_rent_epoch, current_account.rent_epoch()); - }, - |old, new| { - // if existing, capitalization shouldn't change - assert_eq!(old, new); - pass == 3 - }, - ); - if pass == 3 { - continue; + // Updating again should give bank2's sysvar to the closure not bank1's. + // Thus, increment expected_next_slot accordingly + expected_next_slot += 1; + assert_capitalization_diff( + &bank2, + || { + bank2.update_sysvar_account(&dummy_clock_id, |optional_account| { + let slot = from_account::(optional_account.as_ref().unwrap()) + .unwrap() + .slot + + 1; + + create_account( + &Clock { + slot, + ..Clock::default() + }, + bank2.inherit_specially_retained_account_fields(optional_account), + ) + }); + let current_account = bank2.get_account(&dummy_clock_id).unwrap(); + assert_eq!( + expected_next_slot, + from_account::(¤t_account).unwrap().slot + ); + }, + |old, new| { + // updating twice in a slot shouldn't increment capitalization twice + assert_eq!(old, new); + true + }, + ); } - - // Updating again should give bank2's sysvar to the closure not bank1's. - // Thus, increment expected_next_slot accordingly - expected_next_slot += 1; - assert_capitalization_diff( - &bank2, - || { - bank2.update_sysvar_account(&dummy_clock_id, |optional_account| { - let slot = from_account::(optional_account.as_ref().unwrap()) - .unwrap() - .slot - + 1; - - create_account( - &Clock { - slot, - ..Clock::default() - }, - bank2.inherit_specially_retained_account_fields(optional_account), - ) - }); - let current_account = bank2.get_account(&dummy_clock_id).unwrap(); - assert_eq!( - expected_next_slot, - from_account::(¤t_account).unwrap().slot - ); - }, - |old, new| { - // updating twice in a slot shouldn't increment capitalization twice - assert_eq!(old, new); - true - }, - ); } } diff --git a/runtime/src/rent_collector.rs b/runtime/src/rent_collector.rs index a6273b580fe2fd..53fb63b2895e0a 100644 --- a/runtime/src/rent_collector.rs +++ b/runtime/src/rent_collector.rs @@ -35,7 +35,7 @@ impl Default for RentCollector { /// When rent is collected from an exempt account, rent_epoch is set to this /// value. The idea is to have a fixed, consistent value for rent_epoch for all accounts that do not collect rent. /// This enables us to get rid of the field completely. -const RENT_EXEMPT_RENT_EPOCH: Epoch = Epoch::MAX; +pub const RENT_EXEMPT_RENT_EPOCH: Epoch = Epoch::MAX; pub const TEST_SET_EXEMPT_RENT_EPOCH_MAX: bool = false; @@ -108,7 +108,7 @@ impl RentCollector { let due = self.rent.due_amount(account.data().len(), years_elapsed); // we expect rent_epoch to always be one of: {0, self.epoch-1, self.epoch, self.epoch+1} - if account_rent_epoch != 0 + if account_rent_epoch != 0 && account_rent_epoch != RENT_EXEMPT_RENT_EPOCH && (account_rent_epoch + 1 < self.epoch || account_rent_epoch > self.epoch + 1) { // this should not occur in a running validator