Allow extending upgradeable program data account length (#26386)

* Allow extending upgradeable program data account length

* Add is_writable check

* Fix cargo version

* System program fix, comment, and test

* Switch to u32 for serialized bytes value in ix
This commit is contained in:
Justin Starry 2022-07-11 22:46:32 +01:00 committed by GitHub
parent 1c2f6a4c41
commit 8e8c6e60ff
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 690 additions and 3 deletions

11
Cargo.lock generated
View File

@ -4736,6 +4736,17 @@ dependencies = [
"thiserror",
]
[[package]]
name = "solana-bpf-loader-program-tests"
version = "1.11.3"
dependencies = [
"assert_matches",
"bincode",
"solana-bpf-loader-program",
"solana-program-test",
"solana-sdk 1.11.3",
]
[[package]]
name = "solana-bucket-map"
version = "1.11.3"

View File

@ -49,6 +49,7 @@ members = [
"program-test",
"programs/address-lookup-table",
"programs/address-lookup-table-tests",
"programs/bpf-loader-tests",
"programs/bpf_loader",
"programs/bpf_loader/gen-syscall-list",
"programs/compute-budget",

View File

@ -1,5 +1,5 @@
# This package only exists to avoid circular dependencies during cargo publish:
# solana-runtime -> solana-address-program-runtime -> solana-program-test -> solana-runtime
# solana-runtime -> solana-address-program -> solana-program-test -> solana-runtime
[package]
name = "solana-address-lookup-table-program-tests"

View File

@ -0,0 +1,22 @@
# This package only exists to avoid circular dependencies during cargo publish:
# solana-bpf-loader-program -> solana-program-test -> solana-bpf-loader-program
[package]
name = "solana-bpf-loader-program-tests"
version = "1.11.3"
authors = ["Solana Maintainers <maintainers@solana.foundation>"]
repository = "https://github.com/solana-labs/solana"
license = "Apache-2.0"
homepage = "https://solana.com/"
edition = "2021"
publish = false
[dev-dependencies]
assert_matches = "1.5.0"
bincode = "1.3.3"
solana-bpf-loader-program = { path = "../bpf_loader", version = "=1.11.3" }
solana-program-test = { path = "../../program-test", version = "=1.11.3" }
solana-sdk = { path = "../../sdk", version = "=1.11.3" }
[package.metadata.docs.rs]
targets = ["x86_64-unknown-linux-gnu"]

View File

@ -0,0 +1,73 @@
#![allow(dead_code)]
use {
solana_bpf_loader_program::{process_instruction, upgradeable::id},
solana_program_test::*,
solana_sdk::{
account::AccountSharedData,
account_utils::StateMut,
bpf_loader_upgradeable::UpgradeableLoaderState,
instruction::{Instruction, InstructionError},
pubkey::Pubkey,
signature::{Keypair, Signer},
transaction::{Transaction, TransactionError},
},
};
pub async fn setup_test_context() -> ProgramTestContext {
let program_test = ProgramTest::new("", id(), Some(process_instruction));
program_test.start_with_context().await
}
pub async fn assert_ix_error(
context: &mut ProgramTestContext,
ix: Instruction,
additional_payer_keypair: Option<&Keypair>,
expected_err: InstructionError,
assertion_failed_msg: &str,
) {
let client = &mut context.banks_client;
let fee_payer = &context.payer;
let recent_blockhash = context.last_blockhash;
let mut signers = vec![fee_payer];
if let Some(additional_payer) = additional_payer_keypair {
signers.push(additional_payer);
}
let transaction = Transaction::new_signed_with_payer(
&[ix],
Some(&fee_payer.pubkey()),
&signers,
recent_blockhash,
);
assert_eq!(
client
.process_transaction(transaction)
.await
.unwrap_err()
.unwrap(),
TransactionError::InstructionError(0, expected_err),
"{}",
assertion_failed_msg,
);
}
pub async fn add_upgradeable_loader_account(
context: &mut ProgramTestContext,
account_address: &Pubkey,
account_state: &UpgradeableLoaderState,
account_data_len: usize,
) {
let rent = context.banks_client.get_rent().await.unwrap();
let mut account = AccountSharedData::new(
rent.minimum_balance(account_data_len),
account_data_len,
&id(),
);
account
.set_state(account_state)
.expect("state failed to serialize into account data");
context.set_account(account_address, &account);
}

View File

@ -0,0 +1,422 @@
use {
assert_matches::assert_matches,
common::{add_upgradeable_loader_account, assert_ix_error, setup_test_context},
solana_program_test::*,
solana_sdk::{
account::{AccountSharedData, ReadableAccount},
account_utils::StateMut,
bpf_loader_upgradeable::{extend_program_data, id, UpgradeableLoaderState},
instruction::InstructionError,
pubkey::Pubkey,
signature::{Keypair, Signer},
system_instruction::{self, SystemError, MAX_PERMITTED_DATA_LENGTH},
system_program,
transaction::Transaction,
},
};
mod common;
#[tokio::test]
async fn test_extend_program_data() {
let mut context = setup_test_context().await;
let program_data_address = Pubkey::new_unique();
let program_data_len = 100;
add_upgradeable_loader_account(
&mut context,
&program_data_address,
&UpgradeableLoaderState::ProgramData {
slot: 0,
upgrade_authority_address: Some(Pubkey::new_unique()),
},
program_data_len,
)
.await;
let client = &mut context.banks_client;
let payer = &context.payer;
let recent_blockhash = context.last_blockhash;
const ADDITIONAL_BYTES: u32 = 42;
let transaction = Transaction::new_signed_with_payer(
&[extend_program_data(
&program_data_address,
Some(&payer.pubkey()),
ADDITIONAL_BYTES,
)],
Some(&payer.pubkey()),
&[payer],
recent_blockhash,
);
assert_matches!(client.process_transaction(transaction).await, Ok(()));
let updated_program_data_account = client
.get_account(program_data_address)
.await
.unwrap()
.unwrap();
assert_eq!(
updated_program_data_account.data().len(),
program_data_len + ADDITIONAL_BYTES as usize
);
}
#[tokio::test]
async fn test_extend_program_data_not_upgradeable() {
let mut context = setup_test_context().await;
let program_data_address = Pubkey::new_unique();
add_upgradeable_loader_account(
&mut context,
&program_data_address,
&UpgradeableLoaderState::ProgramData {
slot: 0,
upgrade_authority_address: None,
},
100,
)
.await;
let payer_address = context.payer.pubkey();
assert_ix_error(
&mut context,
extend_program_data(&program_data_address, Some(&payer_address), 42),
None,
InstructionError::Immutable,
"should fail because the program data account isn't upgradeable",
)
.await;
}
#[tokio::test]
async fn test_extend_program_data_by_zero_bytes() {
let mut context = setup_test_context().await;
let program_data_address = Pubkey::new_unique();
add_upgradeable_loader_account(
&mut context,
&program_data_address,
&UpgradeableLoaderState::ProgramData {
slot: 0,
upgrade_authority_address: Some(Pubkey::new_unique()),
},
100,
)
.await;
let payer_address = context.payer.pubkey();
assert_ix_error(
&mut context,
extend_program_data(&program_data_address, Some(&payer_address), 0),
None,
InstructionError::InvalidInstructionData,
"should fail because the program data account must be extended by more than 0 bytes",
)
.await;
}
#[tokio::test]
async fn test_extend_program_data_past_max_size() {
let mut context = setup_test_context().await;
let program_data_address = Pubkey::new_unique();
add_upgradeable_loader_account(
&mut context,
&program_data_address,
&UpgradeableLoaderState::ProgramData {
slot: 0,
upgrade_authority_address: Some(Pubkey::new_unique()),
},
MAX_PERMITTED_DATA_LENGTH as usize,
)
.await;
let payer_address = context.payer.pubkey();
assert_ix_error(
&mut context,
extend_program_data(&program_data_address, Some(&payer_address), 1),
None,
InstructionError::InvalidRealloc,
"should fail because the program data account cannot be extended past the max data size",
)
.await;
}
#[tokio::test]
async fn test_extend_program_data_with_invalid_payer() {
let mut context = setup_test_context().await;
let rent = context.banks_client.get_rent().await.unwrap();
let program_data_address = Pubkey::new_unique();
add_upgradeable_loader_account(
&mut context,
&program_data_address,
&UpgradeableLoaderState::ProgramData {
slot: 0,
upgrade_authority_address: Some(Pubkey::new_unique()),
},
100,
)
.await;
let payer_with_sufficient_funds = Keypair::new();
context.set_account(
&payer_with_sufficient_funds.pubkey(),
&AccountSharedData::new(10_000_000_000, 0, &system_program::id()),
);
let payer_with_insufficient_funds = Keypair::new();
context.set_account(
&payer_with_insufficient_funds.pubkey(),
&AccountSharedData::new(rent.minimum_balance(0), 0, &system_program::id()),
);
let payer_with_invalid_owner = Keypair::new();
context.set_account(
&payer_with_invalid_owner.pubkey(),
&AccountSharedData::new(rent.minimum_balance(0), 0, &id()),
);
assert_ix_error(
&mut context,
extend_program_data(
&program_data_address,
Some(&payer_with_insufficient_funds.pubkey()),
1024,
),
Some(&payer_with_insufficient_funds),
InstructionError::from(SystemError::ResultWithNegativeLamports),
"should fail because the payer has insufficient funds to cover program data account rent",
)
.await;
assert_ix_error(
&mut context,
extend_program_data(
&program_data_address,
Some(&payer_with_invalid_owner.pubkey()),
1,
),
Some(&payer_with_invalid_owner),
InstructionError::ExternalAccountLamportSpend,
"should fail because the payer is not a system account",
)
.await;
let mut ix = extend_program_data(
&program_data_address,
Some(&payer_with_sufficient_funds.pubkey()),
1,
);
// Demote payer account meta to non-signer so that transaction signing succeeds
{
let payer_meta = ix
.accounts
.iter_mut()
.find(|meta| meta.pubkey == payer_with_sufficient_funds.pubkey())
.expect("expected to find payer account meta");
payer_meta.is_signer = false;
}
assert_ix_error(
&mut context,
ix,
None,
InstructionError::PrivilegeEscalation,
"should fail because the payer did not sign",
)
.await;
}
#[tokio::test]
async fn test_extend_program_data_without_payer() {
let mut context = setup_test_context().await;
let rent = context.banks_client.get_rent().await.unwrap();
let program_data_address = Pubkey::new_unique();
let program_data_len = 100;
add_upgradeable_loader_account(
&mut context,
&program_data_address,
&UpgradeableLoaderState::ProgramData {
slot: 0,
upgrade_authority_address: Some(Pubkey::new_unique()),
},
program_data_len,
)
.await;
assert_ix_error(
&mut context,
extend_program_data(&program_data_address, None, 1024),
None,
InstructionError::NotEnoughAccountKeys,
"should fail because program data has insufficient funds to cover rent",
)
.await;
let client = &mut context.banks_client;
let payer = &context.payer;
let recent_blockhash = context.last_blockhash;
const ADDITIONAL_BYTES: u32 = 42;
let min_balance_increase_for_extend = rent
.minimum_balance(ADDITIONAL_BYTES as usize)
.saturating_sub(rent.minimum_balance(0));
let transaction = Transaction::new_signed_with_payer(
&[
system_instruction::transfer(
&payer.pubkey(),
&program_data_address,
min_balance_increase_for_extend,
),
extend_program_data(&program_data_address, None, ADDITIONAL_BYTES),
],
Some(&payer.pubkey()),
&[payer],
recent_blockhash,
);
assert_matches!(client.process_transaction(transaction).await, Ok(()));
let updated_program_data_account = client
.get_account(program_data_address)
.await
.unwrap()
.unwrap();
assert_eq!(
updated_program_data_account.data().len(),
program_data_len + ADDITIONAL_BYTES as usize
);
}
#[tokio::test]
async fn test_extend_program_data_with_invalid_system_program() {
let mut context = setup_test_context().await;
let program_data_address = Pubkey::new_unique();
let program_data_len = 100;
add_upgradeable_loader_account(
&mut context,
&program_data_address,
&UpgradeableLoaderState::ProgramData {
slot: 0,
upgrade_authority_address: Some(Pubkey::new_unique()),
},
program_data_len,
)
.await;
let payer_address = context.payer.pubkey();
let mut ix = extend_program_data(&program_data_address, Some(&payer_address), 1);
// Change system program to an invalid key
{
let system_program_meta = ix
.accounts
.iter_mut()
.find(|meta| meta.pubkey == crate::system_program::ID)
.expect("expected to find system program account meta");
system_program_meta.pubkey = Pubkey::new_unique();
}
assert_ix_error(
&mut context,
ix,
None,
InstructionError::MissingAccount,
"should fail because the system program is missing",
)
.await;
}
#[tokio::test]
async fn test_extend_program_data_with_invalid_program_data() {
let mut context = setup_test_context().await;
let rent = context.banks_client.get_rent().await.unwrap();
let payer_address = context.payer.pubkey();
let program_data_address = Pubkey::new_unique();
add_upgradeable_loader_account(
&mut context,
&program_data_address,
&UpgradeableLoaderState::ProgramData {
slot: 0,
upgrade_authority_address: Some(Pubkey::new_unique()),
},
100,
)
.await;
let program_data_address_with_invalid_state = Pubkey::new_unique();
{
let mut account = AccountSharedData::new(rent.minimum_balance(100), 100, &id());
account
.set_state(&UpgradeableLoaderState::Buffer {
authority_address: Some(payer_address),
})
.expect("serialization failed");
context.set_account(&program_data_address_with_invalid_state, &account);
}
let program_data_address_with_invalid_owner = Pubkey::new_unique();
{
let invalid_owner = Pubkey::new_unique();
let mut account = AccountSharedData::new(rent.minimum_balance(100), 100, &invalid_owner);
account
.set_state(&UpgradeableLoaderState::ProgramData {
slot: 0,
upgrade_authority_address: Some(payer_address),
})
.expect("serialization failed");
context.set_account(&program_data_address_with_invalid_owner, &account);
}
assert_ix_error(
&mut context,
extend_program_data(
&program_data_address_with_invalid_state,
Some(&payer_address),
1024,
),
None,
InstructionError::InvalidAccountData,
"should fail because the program data account state isn't valid",
)
.await;
assert_ix_error(
&mut context,
extend_program_data(
&program_data_address_with_invalid_owner,
Some(&payer_address),
1024,
),
None,
InstructionError::InvalidAccountOwner,
"should fail because the program data account owner isn't valid",
)
.await;
let mut ix = extend_program_data(&program_data_address, Some(&payer_address), 1);
// Demote ProgramData account meta to read-only
{
let program_data_meta = ix
.accounts
.iter_mut()
.find(|meta| meta.pubkey == program_data_address)
.expect("expected to find program data account meta");
program_data_meta.is_writable = false;
}
assert_ix_error(
&mut context,
ix,
None,
InstructionError::InvalidArgument,
"should fail because the program data account is not writable",
)
.await;
}

View File

@ -43,8 +43,9 @@ use {
feature_set::{
cap_accounts_data_len, disable_bpf_deprecated_load_instructions,
disable_bpf_unresolved_symbols_at_runtime, disable_deploy_of_alloc_free_syscall,
disable_deprecated_loader, error_on_syscall_bpf_function_hash_collisions,
reduce_required_deploy_balance, reject_callx_r10,
disable_deprecated_loader, enable_bpf_loader_extend_program_data_ix,
error_on_syscall_bpf_function_hash_collisions, reduce_required_deploy_balance,
reject_callx_r10,
},
instruction::{AccountMeta, InstructionError},
loader_instruction::LoaderInstruction,
@ -1058,6 +1059,102 @@ fn process_loader_upgradeable_instruction(
}
}
}
UpgradeableLoaderInstruction::ExtendProgramData { additional_bytes } => {
if !invoke_context
.feature_set
.is_active(&enable_bpf_loader_extend_program_data_ix::ID)
{
return Err(InstructionError::InvalidInstructionData);
}
if additional_bytes == 0 {
ic_logger_msg!(log_collector, "Additional bytes must be greater than 0");
return Err(InstructionError::InvalidInstructionData);
}
const PROGRAM_DATA_ACCOUNT_INDEX: usize = 0;
#[allow(dead_code)]
// System program is only required when a CPI is performed
const OPTIONAL_SYSTEM_PROGRAM_ACCOUNT_INDEX: usize = 1;
const OPTIONAL_PAYER_ACCOUNT_INDEX: usize = 2;
let programdata_account = instruction_context
.try_borrow_instruction_account(transaction_context, PROGRAM_DATA_ACCOUNT_INDEX)?;
let programdata_key = *programdata_account.get_key();
if program_id != programdata_account.get_owner() {
ic_logger_msg!(log_collector, "ProgramData owner is invalid");
return Err(InstructionError::InvalidAccountOwner);
}
if !programdata_account.is_writable() {
ic_logger_msg!(log_collector, "ProgramData is not writable");
return Err(InstructionError::InvalidArgument);
}
let old_len = programdata_account.get_data().len();
let new_len = old_len.saturating_add(additional_bytes as usize);
if new_len > MAX_PERMITTED_DATA_LENGTH as usize {
ic_logger_msg!(
log_collector,
"Extended ProgramData length of {} bytes exceeds max account data length of {} bytes",
new_len,
MAX_PERMITTED_DATA_LENGTH
);
return Err(InstructionError::InvalidRealloc);
}
if let UpgradeableLoaderState::ProgramData {
slot: _,
upgrade_authority_address,
} = programdata_account.get_state()?
{
if upgrade_authority_address.is_none() {
ic_logger_msg!(
log_collector,
"Cannot extend ProgramData accounts that are not upgradeable"
);
return Err(InstructionError::Immutable);
}
} else {
ic_logger_msg!(log_collector, "ProgramData state is invalid");
return Err(InstructionError::InvalidAccountData);
}
let required_payment = {
let balance = programdata_account.get_lamports();
let rent = invoke_context.get_sysvar_cache().get_rent()?;
let min_balance = rent.minimum_balance(new_len).max(1);
min_balance.saturating_sub(balance)
};
// Borrowed accounts need to be dropped before native_invoke
drop(programdata_account);
if required_payment > 0 {
let payer_key = *transaction_context.get_key_of_account_at_index(
instruction_context.get_index_of_instruction_account_in_transaction(
OPTIONAL_PAYER_ACCOUNT_INDEX,
)?,
)?;
invoke_context.native_invoke(
system_instruction::transfer(&payer_key, &programdata_key, required_payment),
&[],
)?;
}
let transaction_context = &invoke_context.transaction_context;
let instruction_context = transaction_context.get_current_instruction_context()?;
let mut programdata_account = instruction_context
.try_borrow_instruction_account(transaction_context, PROGRAM_DATA_ACCOUNT_INDEX)?;
programdata_account.set_data_length(new_len)?;
ic_logger_msg!(
log_collector,
"Extended ProgramData account by {} bytes",
additional_bytes
);
}
}
Ok(())

View File

@ -300,6 +300,27 @@ pub fn close_any(
Instruction::new_with_bincode(id(), &UpgradeableLoaderInstruction::Close, metas)
}
/// Returns the instruction required to extend the size of a program data account
pub fn extend_program_data(
program_data_address: &Pubkey,
payer_address: Option<&Pubkey>,
additional_bytes: u32,
) -> Instruction {
let mut metas = vec![AccountMeta::new(*program_data_address, false)];
if let Some(payer_address) = payer_address {
metas.push(AccountMeta::new_readonly(
crate::system_program::id(),
false,
));
metas.push(AccountMeta::new(*payer_address, true));
}
Instruction::new_with_bincode(
id(),
&UpgradeableLoaderInstruction::ExtendProgramData { additional_bytes },
metas,
)
}
#[cfg(test)]
mod tests {
use {super::*, bincode::serialized_size};

View File

@ -127,4 +127,23 @@ pub enum UpgradeableLoaderInstruction {
/// 3. `[writable]` The associated Program account if the account to close
/// is a ProgramData account.
Close,
/// Extend a ProgramData account by the specified number of bytes.
/// Only upgradeable program's can be extended.
///
/// The payer account must contain sufficient lamports to fund the
/// ProgramData account to be rent-exempt. If the ProgramData account
/// balance is already sufficient to cover the rent exemption cost
/// for the extended bytes, the payer account is not required.
///
/// # Account references
/// 0. `[writable]` The ProgramData account.
/// 1. `[]` System program (`solana_sdk::system_program::id()`), optional, used to transfer
/// lamports from the payer to the ProgramData account.
/// 2. `[signer]` The payer account, optional, that will pay necessary rent exemption costs
/// for the increased storage size.
ExtendProgramData {
/// Number of bytes to extend the program data.
additional_bytes: u32,
},
}

View File

@ -452,6 +452,10 @@ pub mod preserve_rent_epoch_for_rent_exempt_accounts {
solana_sdk::declare_id!("HH3MUYReL2BvqqA3oEcAa7txju5GY6G4nxJ51zvsEjEZ");
}
pub mod enable_bpf_loader_extend_program_data_ix {
solana_sdk::declare_id!("8Zs9W7D9MpSEtUWSQdGniZk2cNmV22y6FLJwCx53asme");
}
lazy_static! {
/// Map of feature identifiers to user-visible description
pub static ref FEATURE_NAMES: HashMap<Pubkey, &'static str> = [
@ -559,6 +563,7 @@ lazy_static! {
(vote_authorize_with_seed::id(), "An instruction you can use to change a vote accounts authority when the current authority is a derived key #25860"),
(cap_accounts_data_size_per_block::id(), "cap the accounts data size per block #25517"),
(preserve_rent_epoch_for_rent_exempt_accounts::id(), "preserve rent epoch for rent exempt accounts #26479"),
(enable_bpf_loader_extend_program_data_ix::id(), "enable bpf upgradeable loader ExtendProgramData instruction #25234"),
/*************** ADD NEW FEATURES HERE ***************/
]
.iter()

View File

@ -150,6 +150,22 @@ pub fn parse_bpf_upgradeable_loader(
}),
})
}
UpgradeableLoaderInstruction::ExtendProgramData { additional_bytes } => {
check_num_bpf_upgradeable_loader_accounts(&instruction.accounts, 2)?;
Ok(ParsedInstructionEnum {
instruction_type: "extendProgramData".to_string(),
info: json!({
"additionalBytes": additional_bytes,
"programDataAccount": account_keys[instruction.accounts[0] as usize].to_string(),
"systemProgram": account_keys[instruction.accounts[1] as usize].to_string(),
"payerAccount": if instruction.accounts.len() > 2 {
Some(account_keys[instruction.accounts[2] as usize].to_string())
} else {
None
},
}),
})
}
}
}