Skip to content
This repository has been archived by the owner on Jan 13, 2025. It is now read-only.

Refactor: Make program_id always last in program chain #20598

Merged
merged 3 commits into from
Oct 13, 2021
Merged

Refactor: Make program_id always last in program chain #20598

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1be1052
Replaces program_id field in InvokeContextStackFrame by index.
Lichtso Oct 12, 2021
327dad3
Swaps order of program account and programdata account.
Lichtso Oct 11, 2021
ffc6719
Removes program_id parameter from InvokeContext::push().
Lichtso Oct 12, 2021
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
13 changes: 3 additions & 10 deletions program-runtime/src/instruction_processor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -485,7 +485,7 @@ impl InstructionProcessor {
);
return Err(InstructionError::AccountNotExecutable);
}
let mut program_indices = vec![program_account_index];
let mut program_indices = vec![];
if program_account.borrow().owner() == &bpf_loader_upgradeable::id() {
if let UpgradeableLoaderState::Program {
programdata_address,
Expand All @@ -512,6 +512,7 @@ impl InstructionProcessor {
return Err(InstructionError::MissingAccount);
}
}
program_indices.push(program_account_index);

Ok((message, caller_write_privileges, program_indices))
}
Expand Down Expand Up @@ -593,22 +594,14 @@ impl InstructionProcessor {
.get(0)
.ok_or(InstructionError::GenericError)?;

let program_id = instruction.program_id(&message.account_keys);

// Verify the calling program hasn't misbehaved
invoke_context.verify_and_update(instruction, account_indices, caller_write_privileges)?;

// clear the return data
invoke_context.set_return_data(Vec::new())?;

// Invoke callee
invoke_context.push(
program_id,
message,
instruction,
program_indices,
Some(account_indices),
)?;
invoke_context.push(message, instruction, program_indices, Some(account_indices))?;

let mut instruction_processor = InstructionProcessor::default();
for (program_id, process_instruction) in invoke_context.get_programs().iter() {
Expand Down
116 changes: 55 additions & 61 deletions programs/bpf_loader/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,72 +199,71 @@ fn process_instruction_common(
use_jit: bool,
) -> Result<(), InstructionError> {
let logger = invoke_context.get_logger();
let keyed_accounts = invoke_context.get_keyed_accounts()?;
let program_id = invoke_context.get_caller()?;

let keyed_accounts = invoke_context.get_keyed_accounts()?;
let first_account = keyed_account_at_index(keyed_accounts, first_instruction_account)?;
if first_account.executable()? {
let second_account = keyed_account_at_index(keyed_accounts, first_instruction_account + 1);
let (program, next_first_instruction_account) = if first_account.unsigned_key() == program_id {
(first_account, first_instruction_account)
} else if second_account
.as_ref()
.map(|keyed_account| keyed_account.unsigned_key() == program_id)
.unwrap_or(false)
{
(second_account?, first_instruction_account + 1)
} else {
if first_account.executable()? {
ic_logger_msg!(logger, "BPF loader is executable");
return Err(InstructionError::IncorrectProgramId);
}
(first_account, first_instruction_account)
};

if program.executable()? {
debug_assert_eq!(
first_instruction_account,
1 - (invoke_context.invoke_depth() > 1) as usize,
);

let program_id = invoke_context.get_caller()?;
if first_account.unsigned_key() != program_id {
ic_logger_msg!(logger, "Program id mismatch");
if !check_loader_id(&program.owner()?) {
ic_logger_msg!(logger, "Executable account not owned by the BPF loader");
return Err(InstructionError::IncorrectProgramId);
}

let (first_instruction_account, program_data_offset) =
if bpf_loader_upgradeable::check_id(&first_account.owner()?) {
if let UpgradeableLoaderState::Program {
programdata_address,
} = first_account.state()?
{
let programdata =
keyed_account_at_index(keyed_accounts, first_instruction_account + 1)?;
if programdata_address != *programdata.unsigned_key() {
ic_logger_msg!(
logger,
"Wrong ProgramData account for this Program account"
);
return Err(InstructionError::InvalidArgument);
}
if !matches!(
programdata.state()?,
UpgradeableLoaderState::ProgramData {
slot: _,
upgrade_authority_address: _,
}
) {
ic_logger_msg!(logger, "Program has been closed");
return Err(InstructionError::InvalidAccountData);
let program_data_offset = if bpf_loader_upgradeable::check_id(&program.owner()?) {
if let UpgradeableLoaderState::Program {
programdata_address,
} = program.state()?
{
if programdata_address != *first_account.unsigned_key() {
ic_logger_msg!(logger, "Wrong ProgramData account for this Program account");
return Err(InstructionError::InvalidArgument);
}
if !matches!(
first_account.state()?,
UpgradeableLoaderState::ProgramData {
slot: _,
upgrade_authority_address: _,
}
(
first_instruction_account + 1,
UpgradeableLoaderState::programdata_data_offset()?,
)
} else {
ic_logger_msg!(logger, "Invalid Program account");
) {
ic_logger_msg!(logger, "Program has been closed");
return Err(InstructionError::InvalidAccountData);
}
UpgradeableLoaderState::programdata_data_offset()?
} else {
(first_instruction_account, 0)
};

let keyed_accounts = invoke_context.get_keyed_accounts()?;
let program = keyed_account_at_index(keyed_accounts, first_instruction_account)?;
let loader_id = &program.owner()?;

if !check_loader_id(loader_id) {
ic_logger_msg!(logger, "Executable account not owned by the BPF loader");
return Err(InstructionError::IncorrectProgramId);
}
ic_logger_msg!(logger, "Invalid Program account");
return Err(InstructionError::InvalidAccountData);
}
} else {
0
};

let executor = match invoke_context.get_executor(program_id) {
Some(executor) => executor,
None => {
let executor = create_executor(
first_instruction_account,
next_first_instruction_account,
program_data_offset,
invoke_context,
use_jit,
Expand All @@ -275,37 +274,32 @@ fn process_instruction_common(
}
};
executor.execute(
first_instruction_account,
next_first_instruction_account,
instruction_data,
invoke_context,
use_jit,
)?
)
} else {
debug_assert_eq!(first_instruction_account, 1);

let program_id = invoke_context.get_caller()?;
if !check_loader_id(program_id) {
ic_logger_msg!(logger, "Invalid BPF loader id");
return Err(InstructionError::IncorrectProgramId);
}

if bpf_loader_upgradeable::check_id(program_id) {
process_loader_upgradeable_instruction(
first_instruction_account,
instruction_data,
invoke_context,
use_jit,
)?;
} else {
)
} else if bpf_loader::check_id(program_id) || bpf_loader_deprecated::check_id(program_id) {
process_loader_instruction(
first_instruction_account,
instruction_data,
invoke_context,
use_jit,
)?;
)
} else {
ic_logger_msg!(logger, "Invalid BPF loader id");
Err(InstructionError::IncorrectProgramId)
}
}
Ok(())
}

fn process_loader_upgradeable_instruction(
Expand Down Expand Up @@ -3374,8 +3368,8 @@ mod tests {

// Try to invoke closed account
let keyed_accounts: Vec<KeyedAccountTuple> = vec![
(false, false, &program_address, &program_account),
(false, false, &programdata_address, &programdata_account),
(false, false, &program_address, &program_account),
];
assert_eq!(
Err(InstructionError::InvalidAccountData),
Expand Down
7 changes: 3 additions & 4 deletions runtime/src/accounts.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -429,7 +429,7 @@ impl Accounts {

// Add loader to chain
let program_owner = *program.owner();

account_indices.insert(0, program_account_index);
if bpf_loader_upgradeable::check_id(&program_owner) {
// The upgradeable loader requires the derived ProgramData account
if let Ok(UpgradeableLoaderState::Program {
Expand Down Expand Up @@ -457,7 +457,6 @@ impl Accounts {
}
}

account_indices.insert(0, program_account_index);
program_id = program_owner;
}
Ok(account_indices)
Expand Down Expand Up @@ -1923,8 +1922,8 @@ mod tests {
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]], accounts[5]);
assert_eq!(result.accounts[result.program_indices[0][1]], accounts[3]);
assert_eq!(result.accounts[result.program_indices[0][2]], accounts[4]);
assert_eq!(result.accounts[result.program_indices[0][1]], accounts[4]);
assert_eq!(result.accounts[result.program_indices[0][2]], accounts[3]);
}

#[test]
Expand Down
Loading