blockworks-foundation
/
solana
mirror of https://github.com/blockworks-foundation/solana.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Rename verify_instruction() to verify_account_changes() (#7020)

This commit is contained in:
Greg Fitzgerald 2019-11-18 15:01:14 -07:00 committed by GitHub
parent 3acd84d9c0
commit c09469fa3a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 32 additions and 32 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
book/src/proposals/cross-program-invocation.md
View File

@ -63,7 +63,7 @@ fn pay_and_launch_missiles(keyed_accounts: &[KeyedAccount]) -> Result<()> {
} }
``` ```
where `process_instruction()` is built into Solana's runtime and responsible for routing the given instruction to the `token` program via the instruction's `program_id` field. Before invoking `pay()`, the runtime must also ensure that `acme` didn't modify any accounts owned by `token`. It does this by calling `runtime::verify_instruction()` and then afterward updating all the `pre_*` variables to tentatively commit `acme`'s account modifications. After `pay()` completes, the runtime must again ensure that `token` didn't modify any accounts owned by `acme`. It should call `verify_instruction()` again, but this time with the `token` program ID. Lastly, after `pay_and_launch_missiles()` completes, the runtime must call `verify_instruction()` one more time, where it normally would, but using all updated `pre_*` variables. If executing `pay_and_launch_missiles()` up to `pay()` made no invalid account changes, `pay()` made no invalid changes, and executing from `pay()` until `pay_and_launch_missiles()` returns made no invalid changes, then the runtime can transitively assume `pay_and_launch_missiles()` as whole made no invalid account changes, and therefore commit all account modifications. where `process_instruction()` is built into Solana's runtime and responsible for routing the given instruction to the `token` program via the instruction's `program_id` field. Before invoking `pay()`, the runtime must also ensure that `acme` didn't modify any accounts owned by `token`. It does this by calling `runtime::verify_account_changes()` and then afterward updating all the `pre_*` variables to tentatively commit `acme`'s account modifications. After `pay()` completes, the runtime must again ensure that `token` didn't modify any accounts owned by `acme`. It should call `verify_account_changes()` again, but this time with the `token` program ID. Lastly, after `pay_and_launch_missiles()` completes, the runtime must call `verify_account_changes()` one more time, where it normally would, but using all updated `pre_*` variables. If executing `pay_and_launch_missiles()` up to `pay()` made no invalid account changes, `pay()` made no invalid changes, and executing from `pay()` until `pay_and_launch_missiles()` returns made no invalid changes, then the runtime can transitively assume `pay_and_launch_missiles()` as whole made no invalid account changes, and therefore commit all account modifications.
### Setting `KeyedAccount.is_signer` ### Setting `KeyedAccount.is_signer`

8
runtime/benches/message_processor.rs
View File

@ -16,7 +16,7 @@ fn bench_has_duplicates(bencher: &mut Bencher) {
} }
#[bench] #[bench]
fn bench_verify_instruction_data(bencher: &mut Bencher) { fn bench_verify_account_changes_data(bencher: &mut Bencher) {
solana_logger::setup(); solana_logger::setup();
let owner = Pubkey::new_rand(); let owner = Pubkey::new_rand();
@ -27,11 +27,11 @@ fn bench_verify_instruction_data(bencher: &mut Bencher) {
need_account_data_checked(&owner, &owner, true), need_account_data_checked(&owner, &owner, true),
); );
let post = Account::new(0, BUFSIZE, &owner); let post = Account::new(0, BUFSIZE, &owner);
assert_eq!(verify_instruction(&owner, &pre, &post), Ok(())); assert_eq!(verify_account_changes(&owner, &pre, &post), Ok(()));
// this one should be faster // this one should be faster
bencher.iter(|| { bencher.iter(|| {
verify_instruction(&owner, &pre, &post).unwrap(); verify_account_changes(&owner, &pre, &post).unwrap();
}); });
let summary = bencher.bench(|_bencher| {}).unwrap(); let summary = bencher.bench(|_bencher| {}).unwrap();
info!("data no change by owner: {} ns/iter", summary.median); info!("data no change by owner: {} ns/iter", summary.median);
@ -48,7 +48,7 @@ fn bench_verify_instruction_data(bencher: &mut Bencher) {
let summary = bencher.bench(|_bencher| {}).unwrap(); let summary = bencher.bench(|_bencher| {}).unwrap();
info!("data compare {} ns/iter", summary.median); info!("data compare {} ns/iter", summary.median);
bencher.iter(|| { bencher.iter(|| {
verify_instruction(&non_owner, &pre, &post).unwrap(); verify_account_changes(&non_owner, &pre, &post).unwrap();
}); });
let summary = bencher.bench(|_bencher| {}).unwrap(); let summary = bencher.bench(|_bencher| {}).unwrap();
info!("data no change by non owner: {} ns/iter", summary.median); info!("data no change by non owner: {} ns/iter", summary.median);

54
runtime/src/message_processor.rs
View File

@ -91,7 +91,7 @@ pub fn need_account_data_checked(program_id: &Pubkey, owner: &Pubkey, is_writabl
// Read-only account data may not change. // Read-only account data may not change.
|| !is_writable || !is_writable
} }
pub fn verify_instruction( pub fn verify_account_changes(
program_id: &Pubkey, program_id: &Pubkey,
pre: &PreInstructionAccount, pre: &PreInstructionAccount,
post: &Account, post: &Account,
@ -321,7 +321,7 @@ impl MessageProcessor {
// Verify the instruction // Verify the instruction
for (pre_account, post_account) in pre_accounts.iter().zip(program_accounts.iter()) { for (pre_account, post_account) in pre_accounts.iter().zip(program_accounts.iter()) {
verify_instruction(&program_id, pre_account, post_account)?; verify_account_changes(&program_id, pre_account, post_account)?;
} }
// The total sum of all the lamports in all the accounts cannot change. // The total sum of all the lamports in all the accounts cannot change.
let post_total = Self::sum_account_lamports(program_accounts); let post_total = Self::sum_account_lamports(program_accounts);
@ -434,14 +434,14 @@ mod tests {
} }
#[test] #[test]
fn test_verify_instruction_change_owner() { fn test_verify_account_changes_owner() {
fn change_owner( fn change_owner(
ix: &Pubkey, ix: &Pubkey,
pre: &Pubkey, pre: &Pubkey,
post: &Pubkey, post: &Pubkey,
is_writable: bool, is_writable: bool,
) -> Result<(), InstructionError> { ) -> Result<(), InstructionError> {
verify_instruction( verify_account_changes(
&ix, &ix,
&PreInstructionAccount::new( &PreInstructionAccount::new(
&Account::new(0, 0, pre), &Account::new(0, 0, pre),
@ -499,7 +499,7 @@ mod tests {
); );
assert_eq!( assert_eq!(
verify_instruction( verify_account_changes(
&mallory_program_id, &mallory_program_id,
&PreInstructionAccount::new( &PreInstructionAccount::new(
&Account::new_data(0, &[42], &mallory_program_id).unwrap(), &Account::new_data(0, &[42], &mallory_program_id).unwrap(),
@ -512,7 +512,7 @@ mod tests {
"mallory should be able to change the account owner, if she leaves clear data" "mallory should be able to change the account owner, if she leaves clear data"
); );
assert_eq!( assert_eq!(
verify_instruction( verify_account_changes(
&mallory_program_id, &mallory_program_id,
&PreInstructionAccount::new( &PreInstructionAccount::new(
&Account::new_data(0, &[42], &mallory_program_id).unwrap(), &Account::new_data(0, &[42], &mallory_program_id).unwrap(),
@ -527,7 +527,7 @@ mod tests {
} }
#[test] #[test]
fn test_verify_instruction_change_executable() { fn test_verify_account_changes_executable() {
let owner = Pubkey::new_rand(); let owner = Pubkey::new_rand();
let change_executable = |program_id: &Pubkey, let change_executable = |program_id: &Pubkey,
is_writable: bool, is_writable: bool,
@ -549,7 +549,7 @@ mod tests {
executable: post_executable, executable: post_executable,
..Account::default() ..Account::default()
}; };
verify_instruction(&program_id, &pre, &post) verify_account_changes(&program_id, &pre, &post)
}; };
let mallory_program_id = Pubkey::new_rand(); let mallory_program_id = Pubkey::new_rand();
@ -583,9 +583,9 @@ mod tests {
} }
#[test] #[test]
fn test_verify_instruction_change_data_len() { fn test_verify_account_changes_data_len() {
assert_eq!( assert_eq!(
verify_instruction( verify_account_changes(
&system_program::id(), &system_program::id(),
&PreInstructionAccount::new( &PreInstructionAccount::new(
&Account::new_data(0, &[0], &system_program::id()).unwrap(), &Account::new_data(0, &[0], &system_program::id()).unwrap(),
@ -600,7 +600,7 @@ mod tests {
let alice_program_id = Pubkey::new_rand(); let alice_program_id = Pubkey::new_rand();
assert_eq!( assert_eq!(
verify_instruction( verify_account_changes(
&system_program::id(), &system_program::id(),
&PreInstructionAccount::new( &PreInstructionAccount::new(
&Account::new_data(0, &[0], &alice_program_id).unwrap(), &Account::new_data(0, &[0], &alice_program_id).unwrap(),
@ -615,7 +615,7 @@ mod tests {
} }
#[test] #[test]
fn test_verify_instruction_change_data() { fn test_verify_account_changes_data() {
let alice_program_id = Pubkey::new_rand(); let alice_program_id = Pubkey::new_rand();
let change_data = let change_data =
@ -626,7 +626,7 @@ mod tests {
need_account_data_checked(&alice_program_id, &program_id, is_writable), need_account_data_checked(&alice_program_id, &program_id, is_writable),
); );
let post = Account::new_data(0, &[42], &alice_program_id).unwrap(); let post = Account::new_data(0, &[42], &alice_program_id).unwrap();
verify_instruction(&program_id, &pre, &post) verify_account_changes(&program_id, &pre, &post)
}; };
let mallory_program_id = Pubkey::new_rand(); let mallory_program_id = Pubkey::new_rand();
@ -650,7 +650,7 @@ mod tests {
} }
#[test] #[test]
fn test_verify_instruction_rent_epoch() { fn test_verify_account_changes_rent_epoch() {
let alice_program_id = Pubkey::new_rand(); let alice_program_id = Pubkey::new_rand();
let pre = PreInstructionAccount::new( let pre = PreInstructionAccount::new(
&Account::new(0, 0, &alice_program_id), &Account::new(0, 0, &alice_program_id),
@ -660,21 +660,21 @@ mod tests {
let mut post = Account::new(0, 0, &alice_program_id); let mut post = Account::new(0, 0, &alice_program_id);
assert_eq!( assert_eq!(
verify_instruction(&system_program::id(), &pre, &post), verify_account_changes(&system_program::id(), &pre, &post),
Ok(()), Ok(()),
"nothing changed!" "nothing changed!"
); );
post.rent_epoch += 1; post.rent_epoch += 1;
assert_eq!( assert_eq!(
verify_instruction(&system_program::id(), &pre, &post), verify_account_changes(&system_program::id(), &pre, &post),
Err(InstructionError::RentEpochModified), Err(InstructionError::RentEpochModified),
"no one touches rent_epoch" "no one touches rent_epoch"
); );
} }
#[test] #[test]
fn test_verify_instruction_deduct_lamports_and_reassign_account() { fn test_verify_account_changes_deduct_lamports_and_reassign_account() {
let alice_program_id = Pubkey::new_rand(); let alice_program_id = Pubkey::new_rand();
let bob_program_id = Pubkey::new_rand(); let bob_program_id = Pubkey::new_rand();
let pre = PreInstructionAccount::new( let pre = PreInstructionAccount::new(
@ -686,14 +686,14 @@ mod tests {
// positive test of this capability // positive test of this capability
assert_eq!( assert_eq!(
verify_instruction(&alice_program_id, &pre, &post), verify_account_changes(&alice_program_id, &pre, &post),
Ok(()), Ok(()),
"alice should be able to deduct lamports and give the account to bob if the data is zeroed", "alice should be able to deduct lamports and give the account to bob if the data is zeroed",
); );
} }
#[test] #[test]
fn test_verify_instruction_change_lamports() { fn test_verify_account_changes_lamports() {
let alice_program_id = Pubkey::new_rand(); let alice_program_id = Pubkey::new_rand();
let pre = PreInstructionAccount::new( let pre = PreInstructionAccount::new(
&Account::new(42, 0, &alice_program_id), &Account::new(42, 0, &alice_program_id),
@ -703,7 +703,7 @@ mod tests {
let post = Account::new(0, 0, &alice_program_id); let post = Account::new(0, 0, &alice_program_id);
assert_eq!( assert_eq!(
verify_instruction(&system_program::id(), &pre, &post), verify_account_changes(&system_program::id(), &pre, &post),
Err(InstructionError::ExternalAccountLamportSpend), Err(InstructionError::ExternalAccountLamportSpend),
"debit should fail, even if system program" "debit should fail, even if system program"
); );
@ -715,7 +715,7 @@ mod tests {
); );
assert_eq!( assert_eq!(
verify_instruction(&alice_program_id, &pre, &post,), verify_account_changes(&alice_program_id, &pre, &post,),
Err(InstructionError::ReadonlyLamportChange), Err(InstructionError::ReadonlyLamportChange),
"debit should fail, even if owning program" "debit should fail, even if owning program"
); );
@ -727,7 +727,7 @@ mod tests {
); );
let post = Account::new(0, 0, &system_program::id()); let post = Account::new(0, 0, &system_program::id());
assert_eq!( assert_eq!(
verify_instruction(&system_program::id(), &pre, &post), verify_account_changes(&system_program::id(), &pre, &post),
Err(InstructionError::ModifiedProgramId), Err(InstructionError::ModifiedProgramId),
"system program can't debit the account unless it was the pre.owner" "system program can't debit the account unless it was the pre.owner"
); );
@ -739,14 +739,14 @@ mod tests {
); );
let post = Account::new(0, 0, &alice_program_id); let post = Account::new(0, 0, &alice_program_id);
assert_eq!( assert_eq!(
verify_instruction(&system_program::id(), &pre, &post), verify_account_changes(&system_program::id(), &pre, &post),
Ok(()), Ok(()),
"system can spend (and change owner)" "system can spend (and change owner)"
); );
} }
#[test] #[test]
fn test_verify_instruction_data_size_changed() { fn test_verify_account_changes_data_size_changed() {
let alice_program_id = Pubkey::new_rand(); let alice_program_id = Pubkey::new_rand();
let pre = PreInstructionAccount::new( let pre = PreInstructionAccount::new(
&Account::new_data(42, &[0], &alice_program_id).unwrap(), &Account::new_data(42, &[0], &alice_program_id).unwrap(),
@ -755,7 +755,7 @@ mod tests {
); );
let post = Account::new_data(42, &[0, 0], &alice_program_id).unwrap(); let post = Account::new_data(42, &[0, 0], &alice_program_id).unwrap();
assert_eq!( assert_eq!(
verify_instruction(&system_program::id(), &pre, &post), verify_account_changes(&system_program::id(), &pre, &post),
Err(InstructionError::AccountDataSizeChanged), Err(InstructionError::AccountDataSizeChanged),
"system program should not be able to change another program's account data size" "system program should not be able to change another program's account data size"
); );
@ -765,7 +765,7 @@ mod tests {
need_account_data_checked(&alice_program_id, &alice_program_id, true), need_account_data_checked(&alice_program_id, &alice_program_id, true),
); );
assert_eq!( assert_eq!(
verify_instruction(&alice_program_id, &pre, &post), verify_account_changes(&alice_program_id, &pre, &post),
Err(InstructionError::AccountDataSizeChanged), Err(InstructionError::AccountDataSizeChanged),
"non-system programs cannot change their data size" "non-system programs cannot change their data size"
); );
@ -775,7 +775,7 @@ mod tests {
need_account_data_checked(&system_program::id(), &system_program::id(), true), need_account_data_checked(&system_program::id(), &system_program::id(), true),
); );
assert_eq!( assert_eq!(
verify_instruction(&system_program::id(), &pre, &post), verify_account_changes(&system_program::id(), &pre, &post),
Ok(()), Ok(()),
"system program should be able to change acount data size" "system program should be able to change acount data size"
); );