From c09469fa3a4e96dac975b0f30e7b76b39eb3136b Mon Sep 17 00:00:00 2001 From: Greg Fitzgerald Date: Mon, 18 Nov 2019 15:01:14 -0700 Subject: [PATCH] Rename verify_instruction() to verify_account_changes() (#7020) --- .../src/proposals/cross-program-invocation.md | 2 +- runtime/benches/message_processor.rs | 8 +-- runtime/src/message_processor.rs | 54 +++++++++---------- 3 files changed, 32 insertions(+), 32 deletions(-) diff --git a/book/src/proposals/cross-program-invocation.md b/book/src/proposals/cross-program-invocation.md index 64853c78a1..9dd1c2167d 100644 --- a/book/src/proposals/cross-program-invocation.md +++ b/book/src/proposals/cross-program-invocation.md @@ -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` diff --git a/runtime/benches/message_processor.rs b/runtime/benches/message_processor.rs index 017ed41ff0..2858ee47f4 100644 --- a/runtime/benches/message_processor.rs +++ b/runtime/benches/message_processor.rs @@ -16,7 +16,7 @@ fn bench_has_duplicates(bencher: &mut Bencher) { } #[bench] -fn bench_verify_instruction_data(bencher: &mut Bencher) { +fn bench_verify_account_changes_data(bencher: &mut Bencher) { solana_logger::setup(); let owner = Pubkey::new_rand(); @@ -27,11 +27,11 @@ fn bench_verify_instruction_data(bencher: &mut Bencher) { need_account_data_checked(&owner, &owner, true), ); 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 bencher.iter(|| { - verify_instruction(&owner, &pre, &post).unwrap(); + verify_account_changes(&owner, &pre, &post).unwrap(); }); let summary = bencher.bench(|_bencher| {}).unwrap(); 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(); info!("data compare {} ns/iter", summary.median); bencher.iter(|| { - verify_instruction(&non_owner, &pre, &post).unwrap(); + verify_account_changes(&non_owner, &pre, &post).unwrap(); }); let summary = bencher.bench(|_bencher| {}).unwrap(); info!("data no change by non owner: {} ns/iter", summary.median); diff --git a/runtime/src/message_processor.rs b/runtime/src/message_processor.rs index ebed004983..580d4afc64 100644 --- a/runtime/src/message_processor.rs +++ b/runtime/src/message_processor.rs @@ -91,7 +91,7 @@ pub fn need_account_data_checked(program_id: &Pubkey, owner: &Pubkey, is_writabl // Read-only account data may not change. || !is_writable } -pub fn verify_instruction( +pub fn verify_account_changes( program_id: &Pubkey, pre: &PreInstructionAccount, post: &Account, @@ -321,7 +321,7 @@ impl MessageProcessor { // Verify the instruction 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. let post_total = Self::sum_account_lamports(program_accounts); @@ -434,14 +434,14 @@ mod tests { } #[test] - fn test_verify_instruction_change_owner() { + fn test_verify_account_changes_owner() { fn change_owner( ix: &Pubkey, pre: &Pubkey, post: &Pubkey, is_writable: bool, ) -> Result<(), InstructionError> { - verify_instruction( + verify_account_changes( &ix, &PreInstructionAccount::new( &Account::new(0, 0, pre), @@ -499,7 +499,7 @@ mod tests { ); assert_eq!( - verify_instruction( + verify_account_changes( &mallory_program_id, &PreInstructionAccount::new( &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" ); assert_eq!( - verify_instruction( + verify_account_changes( &mallory_program_id, &PreInstructionAccount::new( &Account::new_data(0, &[42], &mallory_program_id).unwrap(), @@ -527,7 +527,7 @@ mod tests { } #[test] - fn test_verify_instruction_change_executable() { + fn test_verify_account_changes_executable() { let owner = Pubkey::new_rand(); let change_executable = |program_id: &Pubkey, is_writable: bool, @@ -549,7 +549,7 @@ mod tests { executable: post_executable, ..Account::default() }; - verify_instruction(&program_id, &pre, &post) + verify_account_changes(&program_id, &pre, &post) }; let mallory_program_id = Pubkey::new_rand(); @@ -583,9 +583,9 @@ mod tests { } #[test] - fn test_verify_instruction_change_data_len() { + fn test_verify_account_changes_data_len() { assert_eq!( - verify_instruction( + verify_account_changes( &system_program::id(), &PreInstructionAccount::new( &Account::new_data(0, &[0], &system_program::id()).unwrap(), @@ -600,7 +600,7 @@ mod tests { let alice_program_id = Pubkey::new_rand(); assert_eq!( - verify_instruction( + verify_account_changes( &system_program::id(), &PreInstructionAccount::new( &Account::new_data(0, &[0], &alice_program_id).unwrap(), @@ -615,7 +615,7 @@ mod tests { } #[test] - fn test_verify_instruction_change_data() { + fn test_verify_account_changes_data() { let alice_program_id = Pubkey::new_rand(); let change_data = @@ -626,7 +626,7 @@ mod tests { need_account_data_checked(&alice_program_id, &program_id, is_writable), ); 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(); @@ -650,7 +650,7 @@ mod tests { } #[test] - fn test_verify_instruction_rent_epoch() { + fn test_verify_account_changes_rent_epoch() { let alice_program_id = Pubkey::new_rand(); let pre = PreInstructionAccount::new( &Account::new(0, 0, &alice_program_id), @@ -660,21 +660,21 @@ mod tests { let mut post = Account::new(0, 0, &alice_program_id); assert_eq!( - verify_instruction(&system_program::id(), &pre, &post), + verify_account_changes(&system_program::id(), &pre, &post), Ok(()), "nothing changed!" ); post.rent_epoch += 1; assert_eq!( - verify_instruction(&system_program::id(), &pre, &post), + verify_account_changes(&system_program::id(), &pre, &post), Err(InstructionError::RentEpochModified), "no one touches rent_epoch" ); } #[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 bob_program_id = Pubkey::new_rand(); let pre = PreInstructionAccount::new( @@ -686,14 +686,14 @@ mod tests { // positive test of this capability assert_eq!( - verify_instruction(&alice_program_id, &pre, &post), + verify_account_changes(&alice_program_id, &pre, &post), Ok(()), "alice should be able to deduct lamports and give the account to bob if the data is zeroed", ); } #[test] - fn test_verify_instruction_change_lamports() { + fn test_verify_account_changes_lamports() { let alice_program_id = Pubkey::new_rand(); let pre = PreInstructionAccount::new( &Account::new(42, 0, &alice_program_id), @@ -703,7 +703,7 @@ mod tests { let post = Account::new(0, 0, &alice_program_id); assert_eq!( - verify_instruction(&system_program::id(), &pre, &post), + verify_account_changes(&system_program::id(), &pre, &post), Err(InstructionError::ExternalAccountLamportSpend), "debit should fail, even if system program" ); @@ -715,7 +715,7 @@ mod tests { ); assert_eq!( - verify_instruction(&alice_program_id, &pre, &post,), + verify_account_changes(&alice_program_id, &pre, &post,), Err(InstructionError::ReadonlyLamportChange), "debit should fail, even if owning program" ); @@ -727,7 +727,7 @@ mod tests { ); let post = Account::new(0, 0, &system_program::id()); assert_eq!( - verify_instruction(&system_program::id(), &pre, &post), + verify_account_changes(&system_program::id(), &pre, &post), Err(InstructionError::ModifiedProgramId), "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); assert_eq!( - verify_instruction(&system_program::id(), &pre, &post), + verify_account_changes(&system_program::id(), &pre, &post), Ok(()), "system can spend (and change owner)" ); } #[test] - fn test_verify_instruction_data_size_changed() { + fn test_verify_account_changes_data_size_changed() { let alice_program_id = Pubkey::new_rand(); let pre = PreInstructionAccount::new( &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(); assert_eq!( - verify_instruction(&system_program::id(), &pre, &post), + verify_account_changes(&system_program::id(), &pre, &post), Err(InstructionError::AccountDataSizeChanged), "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), ); assert_eq!( - verify_instruction(&alice_program_id, &pre, &post), + verify_account_changes(&alice_program_id, &pre, &post), Err(InstructionError::AccountDataSizeChanged), "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), ); assert_eq!( - verify_instruction(&system_program::id(), &pre, &post), + verify_account_changes(&system_program::id(), &pre, &post), Ok(()), "system program should be able to change acount data size" );