Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bank: Add function to replace empty account with upgradeable program on feature activation #32783

Merged
merged 33 commits into from
Oct 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
4ab4bf1
replace program account
buffalojoec Aug 9, 2023
fb1d4c9
modify for all cases
buffalojoec Aug 16, 2023
99eb96e
remove non-data swap
buffalojoec Aug 17, 2023
acbca38
address tests & conditional feedback
buffalojoec Aug 18, 2023
a8800b8
get the rent involved
buffalojoec Aug 18, 2023
86833f9
mix in owner & executable
buffalojoec Aug 18, 2023
637dd83
feature-related cases
buffalojoec Aug 18, 2023
e6bfb43
stripped back to feature-specific case only
buffalojoec Aug 18, 2023
24e75d5
added feature
buffalojoec Aug 18, 2023
a573a97
address initial feedback
buffalojoec Aug 21, 2023
c221038
added more lamport checks
buffalojoec Aug 21, 2023
b36f56a
condense tests
buffalojoec Aug 21, 2023
9a61900
using test_case
buffalojoec Aug 21, 2023
67986b4
add fail cases to tests
buffalojoec Aug 21, 2023
453dc8a
more cleanup
buffalojoec Aug 22, 2023
7d3f877
add verifiably built program
buffalojoec Aug 22, 2023
a6dd68e
update program account state
buffalojoec Aug 22, 2023
59a4132
cleaned up serializing logic
buffalojoec Aug 23, 2023
acfa993
use full word capitalization
buffalojoec Sep 15, 2023
6ef8353
rename old & new to dst & src
buffalojoec Sep 15, 2023
069dd53
swap src and dst in parameters
buffalojoec Sep 15, 2023
f044890
add warnings and errors
buffalojoec Sep 15, 2023
1e7b4e5
rename feature to programify
buffalojoec Sep 20, 2023
251b9a9
test suite description clarity
buffalojoec Sep 20, 2023
1b37cdc
remove strings from datapoints
buffalojoec Sep 20, 2023
5b280fd
spell out source and destination
buffalojoec Sep 29, 2023
6ccf0f1
more verbose comments in account replace functions
buffalojoec Sep 29, 2023
0392ad8
move lamport calculation
buffalojoec Sep 29, 2023
41c9c40
swap lamport check for state check
buffalojoec Sep 29, 2023
5cadf5b
move replace functions to helper module
buffalojoec Sep 29, 2023
3e7dc30
make replace_account methods fallible
buffalojoec Oct 3, 2023
19b35de
refactor error handling
buffalojoec Oct 4, 2023
91bfec7
add test for source program state
buffalojoec Oct 4, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 20 additions & 36 deletions runtime/src/bank.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -215,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)]
Expand Down Expand Up @@ -8054,6 +8056,24 @@ 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::programify_feature_gate_program::id()) {
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(),
Comment on lines +8064 to +8065
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these arguments swapped? My impression is that this code is trying to copy code from somewhere into the account with id of feature::id() but this code uses feature_id() as the source instead of the destination.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They do appear to be swapped. Great catch, thanks! 😅

#33894

datapoint_name,
) {
warn!(
"{}: Failed to replace empty account {} with upgradeable program: {}",
datapoint_name,
feature::id(),
e
);
datapoint_warn!(datapoint_name, ("slot", self.slot(), i64),);
}
}
}

fn apply_updated_hashes_per_tick(&mut self, hashes_per_tick: u64) {
Expand Down Expand Up @@ -8196,42 +8216,6 @@ impl Bank {
}
}

/// Use to replace programs by feature activation
#[allow(dead_code)]
fn replace_program_account(
&mut self,
old_address: &Pubkey,
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());

self.calculate_and_update_accounts_data_size_delta_off_chain(
old_account.data().len(),
new_account.data().len(),
);
}
}
}

/// Get all the accounts for this bank and calculate stats
pub fn get_total_accounts_stats(&self) -> ScanResult<TotalAccountsStats> {
let accounts = self.get_all_accounts()?;
Expand Down
191 changes: 191 additions & 0 deletions runtime/src/bank/replace_account.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,191 @@
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,
thiserror::Error,
};

/// Errors returned by `replace_account` methods
#[derive(Debug, Error)]
pub enum ReplaceAccountError {
/// 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,
}
buffalojoec marked this conversation as resolved.
Show resolved Hide resolved

/// Moves one account in place of another
/// `source`: the account to replace with
/// `destination`: the account to be replaced
fn move_account<U, V>(
bank: &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: &Bank,
source_address: &Pubkey,
destination_address: &Pubkey,
datapoint_name: &'static str,
) -> Result<(), ReplaceAccountError> {
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
/// 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: &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
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::<UpgradeableLoaderState>(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(())
}
Loading