Add and update tests (#9566)

programs/bpf_loader/src/helpers.rs

@@ -71,13 +71,13 @@ pub fn register_helpers<'a>(
 #[macro_export]
 macro_rules! translate {
     ($vm_addr:expr, $len:expr, $regions:expr) => {
-        translate_addr(
+        translate_addr::<BPFError>(
             $vm_addr as u64,
             $len as usize,
             file!(),
             line!() as usize - ELF_INSN_DUMP_OFFSET + 1,
             $regions,
-        )?
+        )
     };
 }
 
@@ -85,40 +85,51 @@ macro_rules! translate {
 macro_rules! translate_type_mut {
     ($t:ty, $vm_addr:expr, $regions:expr) => {
         unsafe {
-            &mut *(translate_addr(
+            match translate_addr::<BPFError>(
                 $vm_addr as u64,
                 size_of::<$t>(),
                 file!(),
                 line!() as usize - ELF_INSN_DUMP_OFFSET + 1,
                 $regions,
-            )? as *mut $t)
+            ) {
+                Ok(value) => Ok(&mut *(value as *mut $t)),
+                Err(e) => Err(e),
+            }
         }
     };
 }
 #[macro_export]
 macro_rules! translate_type {
     ($t:ty, $vm_addr:expr, $regions:expr) => {
-        &*translate_type_mut!($t, $vm_addr, $regions)
+        match translate_type_mut!($t, $vm_addr, $regions) {
+            Ok(value) => Ok(&*value),
+            Err(e) => Err(e),
+        }
     };
 }
 
 #[macro_export]
 macro_rules! translate_slice_mut {
-    ($t:ty, $vm_addr:expr, $len: expr, $regions:expr) => {{
+    ($t:ty, $vm_addr:expr, $len: expr, $regions:expr) => {
-        let host_addr = translate_addr(
+        match translate_addr::<BPFError>(
             $vm_addr as u64,
             $len as usize * size_of::<$t>(),
             file!(),
             line!() as usize - ELF_INSN_DUMP_OFFSET + 1,
             $regions,
-        )? as *mut $t;
+        ) {
-        unsafe { from_raw_parts_mut(host_addr, $len as usize) }
+            Ok(value) => Ok(unsafe { from_raw_parts_mut(value as *mut $t, $len as usize) }),
-    }};
+            Err(e) => Err(e),
+        }
+    };
 }
 #[macro_export]
 macro_rules! translate_slice {
     ($t:ty, $vm_addr:expr, $len: expr, $regions:expr) => {
-        &*translate_slice_mut!($t, $vm_addr, $len, $regions)
+        match translate_slice_mut!($t, $vm_addr, $len, $regions) {
+            Ok(value) => Ok(&*value),
+            Err(e) => Err(e),
+        }
     };
 }
 
@@ -130,7 +141,7 @@ fn translate_string_and_do(
     regions: &[MemoryRegion],
     work: &dyn Fn(&str) -> Result<u64, EbpfError<BPFError>>,
 ) -> Result<u64, EbpfError<BPFError>> {
-    let buf = translate_slice!(u8, addr, len, regions);
+    let buf = translate_slice!(u8, addr, len, regions)?;
     let i = match buf.iter().position(|byte| *byte == 0) {
         Some(i) => i,
         None => len as usize,
@@ -243,3 +254,232 @@ impl HelperObject<BPFError> for HelperSolAllocFree {
         }
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use solana_sdk::{
+        instruction::{AccountMeta, Instruction},
+        pubkey::Pubkey,
+    };
+
+    #[test]
+    fn test_translate() {
+        const START: u64 = 100;
+        const LENGTH: u64 = 1000;
+        let data = vec![0u8; LENGTH as usize];
+        let addr = data.as_ptr() as u64;
+        let regions = vec![MemoryRegion::new_from_slice(&data, START)];
+
+        let cases = vec![
+            (true, START, 0, addr),
+            (true, START, 1, addr),
+            (true, START, LENGTH, addr),
+            (true, START + 1, LENGTH - 1, addr + 1),
+            (false, START + 1, LENGTH, 0),
+            (true, START + LENGTH - 1, 1, addr + LENGTH - 1),
+            (true, START + LENGTH, 0, addr + LENGTH),
+            (false, START + LENGTH, 1, 0),
+            (false, START, LENGTH + 1, 0),
+            (false, 0, 0, 0),
+            (false, 0, 1, 0),
+            (false, START - 1, 0, 0),
+            (false, START - 1, 1, 0),
+            (true, START + LENGTH / 2, LENGTH / 2, addr + LENGTH / 2),
+        ];
+        for (ok, start, length, value) in cases {
+            match ok {
+                true => assert_eq!(translate!(start, length, &regions).unwrap(), value),
+                false => assert!(translate!(start, length, &regions).is_err()),
+            }
+        }
+    }
+
+    #[test]
+    fn test_translate_type() {
+        // Pubkey
+        let pubkey = Pubkey::new_rand();
+        let addr = &pubkey as *const _ as u64;
+        let regions = vec![MemoryRegion {
+            addr_host: addr,
+            addr_vm: 100,
+            len: std::mem::size_of::<Pubkey>() as u64,
+        }];
+        let translated_pubkey = translate_type!(Pubkey, 100, &regions).unwrap();
+        assert_eq!(pubkey, *translated_pubkey);
+
+        // Instruction
+        let instruction = Instruction::new(
+            Pubkey::new_rand(),
+            &"foobar",
+            vec![AccountMeta::new(Pubkey::new_rand(), false)],
+        );
+        let addr = &instruction as *const _ as u64;
+        let regions = vec![MemoryRegion {
+            addr_host: addr,
+            addr_vm: 100,
+            len: std::mem::size_of::<Instruction>() as u64,
+        }];
+        let translated_instruction = translate_type!(Instruction, 100, &regions).unwrap();
+        assert_eq!(instruction, *translated_instruction);
+    }
+
+    #[test]
+    fn test_translate_slice() {
+        // u8
+        let mut data = vec![1u8, 2, 3, 4, 5];
+        let addr = data.as_ptr() as *const _ as u64;
+        let regions = vec![MemoryRegion {
+            addr_host: addr,
+            addr_vm: 100,
+            len: data.len() as u64,
+        }];
+        let translated_data = translate_slice!(u8, 100, data.len(), &regions).unwrap();
+        assert_eq!(data, translated_data);
+        data[0] = 10;
+        assert_eq!(data, translated_data);
+
+        // Pubkeys
+        let mut data = vec![Pubkey::new_rand(); 5];
+        let addr = data.as_ptr() as *const _ as u64;
+        let regions = vec![MemoryRegion {
+            addr_host: addr,
+            addr_vm: 100,
+            len: (data.len() * std::mem::size_of::<Pubkey>()) as u64,
+        }];
+        let translated_data = translate_slice!(Pubkey, 100, data.len(), &regions).unwrap();
+        assert_eq!(data, translated_data);
+        data[0] = Pubkey::new_rand(); // Both should point to same place
+        assert_eq!(data, translated_data);
+    }
+
+    #[test]
+    fn test_translate_string_and_do() {
+        let string = "Gaggablaghblagh!";
+        let addr = string.as_ptr() as *const _ as u64;
+        let regions = vec![MemoryRegion {
+            addr_host: addr,
+            addr_vm: 100,
+            len: string.len() as u64,
+        }];
+        assert_eq!(
+            42,
+            translate_string_and_do(100, string.len() as u64, &regions, &|string: &str| {
+                assert_eq!(string, "Gaggablaghblagh!");
+                Ok(42)
+            })
+            .unwrap()
+        );
+    }
+
+    #[test]
+    #[should_panic(expected = "UserError(HelperError(Abort))")]
+    fn test_helper_abort() {
+        let ro_region = MemoryRegion::default();
+        let rw_region = MemoryRegion::default();
+        helper_abort(0, 0, 0, 0, 0, &[ro_region], &[rw_region]).unwrap();
+    }
+
+    #[test]
+    #[should_panic(expected = "UserError(HelperError(Panic(\"Gaggablaghblagh!\", 42, 84)))")]
+    fn test_helper_sol_panic() {
+        let string = "Gaggablaghblagh!";
+        let addr = string.as_ptr() as *const _ as u64;
+        let ro_region = MemoryRegion {
+            addr_host: addr,
+            addr_vm: 100,
+            len: string.len() as u64,
+        };
+        let rw_region = MemoryRegion::default();
+        helper_sol_panic(
+            100,
+            string.len() as u64,
+            42,
+            84,
+            0,
+            &[ro_region],
+            &[rw_region],
+        )
+        .unwrap();
+    }
+
+    #[test]
+    fn test_helper_sol_log() {
+        let string = "Gaggablaghblagh!";
+        let addr = string.as_ptr() as *const _ as u64;
+        let ro_regions = &[MemoryRegion {
+            addr_host: addr,
+            addr_vm: 100,
+            len: string.len() as u64,
+        }];
+        let rw_regions = &[MemoryRegion::default()];
+        solana_logger::setup_with_default("solana=info");
+        helper_sol_log(100, string.len() as u64, 0, 0, 0, ro_regions, rw_regions).unwrap();
+        solana_logger::setup_with_default("solana=info");
+        helper_sol_log(
+            100,
+            string.len() as u64 * 2,
+            0,
+            0,
+            0,
+            ro_regions,
+            rw_regions,
+        )
+        .unwrap_err();
+    }
+
+    #[test]
+    fn test_helper_sol_log_u64() {
+        solana_logger::setup_with_default("solana=info");
+
+        let ro_regions = &[MemoryRegion::default()];
+        let rw_regions = &[MemoryRegion::default()];
+        helper_sol_log_u64(1, 2, 3, 4, 5, ro_regions, rw_regions).unwrap();
+    }
+
+    #[test]
+    fn test_helper_sol_alloc_free() {
+        // large alloc
+        {
+            let heap = vec![0_u8; 100];
+            let ro_regions = &[MemoryRegion::default()];
+            let rw_regions = &[MemoryRegion::new_from_slice(&heap, MM_HEAP_START)];
+            let mut helper = HelperSolAllocFree {
+                allocator: BPFAllocator::new(heap, MM_HEAP_START),
+            };
+            assert_ne!(
+                helper
+                    .call(100, 0, 0, 0, 0, ro_regions, rw_regions)
+                    .unwrap(),
+                0
+            );
+            assert_eq!(
+                helper
+                    .call(100, 0, 0, 0, 0, ro_regions, rw_regions)
+                    .unwrap(),
+                0
+            );
+        }
+        // many small allocs
+        {
+            let heap = vec![0_u8; 100];
+            let ro_regions = &[MemoryRegion::default()];
+            let rw_regions = &[MemoryRegion::new_from_slice(&heap, MM_HEAP_START)];
+            let mut helper = HelperSolAllocFree {
+                allocator: BPFAllocator::new(heap, MM_HEAP_START),
+            };
+            for _ in 0..100 {
+                assert_ne!(
+                    helper.call(1, 0, 0, 0, 0, ro_regions, rw_regions).unwrap(),
+                    0
+                );
+            }
+            assert_eq!(
+                helper
+                    .call(100, 0, 0, 0, 0, ro_regions, rw_regions)
+                    .unwrap(),
+                0
+            );
+        }
+    }
+}

runtime/benches/message_processor.rs

@@ -13,7 +13,7 @@ fn bench_verify_account_changes_data(bencher: &mut Bencher) {
 
     let owner = Pubkey::new_rand();
     let non_owner = Pubkey::new_rand();
-    let pre = PreAccount::new(&Account::new(0, BUFSIZE, &owner), true, &owner);
+    let pre = PreAccount::new(&Account::new(0, BUFSIZE, &owner), &owner, true);
     let post = Account::new(0, BUFSIZE, &owner);
     assert_eq!(pre.verify(&owner, &RentCollector::default(), &post), Ok(()));
 
@@ -25,7 +25,7 @@ fn bench_verify_account_changes_data(bencher: &mut Bencher) {
     let summary = bencher.bench(|_bencher| {}).unwrap();
     info!("data no change by owner: {} ns/iter", summary.median);
 
-    let pre = PreAccount::new(&Account::new(0, BUFSIZE, &owner), true, &non_owner);
+    let pre = PreAccount::new(&Account::new(0, BUFSIZE, &owner), &non_owner, true);
     match pre.data {
         Some(ref data) => bencher.iter(|| *data == post.data),
         None => panic!("No data!"),

runtime/src/message_processor.rs

@@ -23,11 +23,11 @@ pub struct PreAccount {
     pub data_len: usize,
     pub data: Option<Vec<u8>>,
     pub owner: Pubkey,
-    pub executable: bool,
+    pub is_executable: bool,
     pub rent_epoch: Epoch,
 }
 impl PreAccount {
-    pub fn new(account: &Account, is_writable: bool, program_id: &Pubkey) -> Self {
+    pub fn new(account: &Account, program_id: &Pubkey, is_writable: bool) -> Self {
         Self {
             is_writable,
             lamports: account.lamports,
@@ -43,7 +43,7 @@ impl PreAccount {
                 None
             },
             owner: account.owner,
-            executable: account.executable,
+            is_executable: account.executable,
             rent_epoch: account.rent_epoch,
         }
     }
@@ -91,7 +91,7 @@ impl PreAccount {
             if !self.is_writable {
                 return Err(InstructionError::ReadonlyLamportChange);
             }
-            if self.executable {
+            if self.is_executable {
                 return Err(InstructionError::ExecutableLamportChange);
             }
         }
@@ -105,11 +105,16 @@ impl PreAccount {
             return Err(InstructionError::AccountDataSizeChanged);
         }
 
-        if Self::should_verify_data(&self.owner, program_id, self.is_writable, self.executable) {
+        if Self::should_verify_data(
+            &self.owner,
+            program_id,
+            self.is_writable,
+            self.is_executable,
+        ) {
             match &self.data {
                 Some(data) if *data == post.data => (),
                 _ => {
-                    if self.executable {
+                    if self.is_executable {
                         return Err(InstructionError::ExecutableDataModified);
                     } else if self.is_writable {
                         return Err(InstructionError::ExternalAccountDataModified);
@@ -121,7 +126,7 @@ impl PreAccount {
         }
 
         // executable is one-way (false->true) and only the account owner may set it.
-        if self.executable != post.executable {
+        if self.is_executable != post.executable {
             if !rent_collector
                 .rent
                 .is_exempt(post.lamports, post.data.len())
@@ -129,7 +134,7 @@ impl PreAccount {
                 return Err(InstructionError::ExecutableAccountNotRentExempt);
             }
             if !self.is_writable // line coverage used to get branch coverage
-                || self.executable // line coverage used to get branch coverage
+                || self.is_executable // line coverage used to get branch coverage
                 || *program_id != self.owner
             {
                 return Err(InstructionError::ExecutableModified);
@@ -250,7 +255,7 @@ impl MessageProcessor {
             let mut work = |_unique_index: usize, account_index: usize| {
                 let is_writable = message.is_writable(account_index);
                 let account = accounts[account_index].borrow();
-                pre_accounts.push(PreAccount::new(&account, is_writable, program_id));
+                pre_accounts.push(PreAccount::new(&account, program_id, is_writable));
                 Ok(())
             };
             let _ = instruction.visit_each_account(&mut work);
@@ -420,90 +425,119 @@ mod tests {
         );
     }
 
-    #[test]
+    struct Change {
-    fn test_verify_account_changes_owner() {
+        // key: Pubkey,
-        fn change_owner(
+        program_id: Pubkey,
-            ix: &Pubkey,
+        rent_collector: RentCollector,
-            pre: &Pubkey,
+        pre: PreAccount,
-            post: &Pubkey,
+        post: Account,
-            is_writable: bool,
+    }
-        ) -> Result<(), InstructionError> {
+    impl Change {
-            PreAccount::new(&Account::new(0, 0, pre), is_writable, ix).verify(
+        pub fn new(owner: &Pubkey, program_id: &Pubkey) -> Self {
-                ix,
+            Self {
-                &RentCollector::default(),
+                // key: Pubkey::new_rand(),
-                &Account::new(0, 0, post),
+                program_id: *program_id,
-            )
+                rent_collector: RentCollector::default(),
+                pre: PreAccount::new(
+                    &Account {
+                        owner: *owner,
+                        lamports: std::u64::MAX,
+                        data: vec![],
+                        ..Account::default()
+                    },
+                    &Pubkey::new_rand(),
+                    true,
+                ),
+                post: Account {
+                    owner: *owner,
+                    lamports: std::u64::MAX,
+                    ..Account::default()
+                },
+            }
+        }
+        pub fn read_only(mut self) -> Self {
+            self.pre.is_writable = false;
+            self
+        }
+        pub fn executable(mut self, pre: bool, post: bool) -> Self {
+            self.pre.is_executable = pre;
+            self.post.executable = post;
+            self
+        }
+        pub fn lamports(mut self, pre: u64, post: u64) -> Self {
+            self.pre.lamports = pre;
+            self.post.lamports = post;
+            self
+        }
+        pub fn owner(mut self, post: &Pubkey) -> Self {
+            self.post.owner = *post;
+            self
+        }
+        pub fn data(mut self, pre: Vec<u8>, post: Vec<u8>) -> Self {
+            self.pre.data_len = pre.len();
+            self.pre.data = Some(pre);
+            self.post.data = post;
+            self
+        }
+        pub fn rent_epoch(mut self, pre: u64, post: u64) -> Self {
+            self.pre.rent_epoch = pre;
+            self.post.rent_epoch = post;
+            self
+        }
+        pub fn verify(&self) -> Result<(), InstructionError> {
+            self.pre
+                .verify(&self.program_id, &self.rent_collector, &self.post)
+        }
     }
 
+    #[test]
+    fn test_verify_account_changes_owner() {
         let system_program_id = system_program::id();
         let alice_program_id = Pubkey::new_rand();
         let mallory_program_id = Pubkey::new_rand();
 
         assert_eq!(
-            change_owner(
+            Change::new(&system_program_id, &system_program_id)
-                &system_program_id,
+                .owner(&alice_program_id)
-                &system_program_id,
+                .verify(),
-                &alice_program_id,
-                true
-            ),
             Ok(()),
             "system program should be able to change the account owner"
         );
         assert_eq!(
-            change_owner(
+            Change::new(&system_program_id, &system_program_id)
-                &system_program_id,
+                .owner(&alice_program_id)
-                &system_program_id,
+                .read_only()
-                &alice_program_id,
+                .verify(),
-                false
-            ),
             Err(InstructionError::ModifiedProgramId),
             "system program should not be able to change the account owner of a read-only account"
         );
         assert_eq!(
-            change_owner(
+            Change::new(&mallory_program_id, &system_program_id)
-                &system_program_id,
+                .owner(&alice_program_id)
-                &mallory_program_id,
+                .verify(),
-                &alice_program_id,
-                true
-            ),
             Err(InstructionError::ModifiedProgramId),
             "system program should not be able to change the account owner of a non-system account"
         );
         assert_eq!(
-            change_owner(
+            Change::new(&mallory_program_id, &mallory_program_id)
-                &mallory_program_id,
+                .owner(&alice_program_id)
-                &mallory_program_id,
+                .verify(),
-                &alice_program_id,
-                true
-            ),
             Ok(()),
             "mallory should be able to change the account owner, if she leaves clear data"
         );
         assert_eq!(
-            PreAccount::new(
+            Change::new(&mallory_program_id, &mallory_program_id)
-                &Account::new_data(0, &[42], &mallory_program_id).unwrap(),
+                .owner(&alice_program_id)
-                true,
+                .data(vec![42], vec![0])
-                &mallory_program_id,
+                .verify(),
-            )
-            .verify(
-                &mallory_program_id,
-                &RentCollector::default(),
-                &Account::new_data(0, &[0], &alice_program_id).unwrap(),
-            ),
             Ok(()),
             "mallory should be able to change the account owner, if she leaves clear data"
         );
         assert_eq!(
-            PreAccount::new(
+            Change::new(&mallory_program_id, &mallory_program_id)
-                &Account::new_data(0, &[42], &mallory_program_id).unwrap(),
+                .owner(&alice_program_id)
-                true,
+                .data(vec![42], vec![42])
-                &mallory_program_id,
+                .verify(),
-            )
-            .verify(
-                &mallory_program_id,
-                &RentCollector::default(),
-                &Account::new_data(0, &[42], &alice_program_id).unwrap(),
-            ),
             Err(InstructionError::ModifiedProgramId),
             "mallory should not be able to inject data into the alice program"
         );
@@ -515,58 +549,6 @@ mod tests {
         let mallory_program_id = Pubkey::new_rand();
         let system_program_id = system_program::id();
 
-        struct Change {
-            pre: PreAccount,
-            post: Account,
-            program_id: Pubkey,
-        }
-        impl Change {
-            pub fn new(owner: &Pubkey, program_id: &Pubkey) -> Self {
-                Self {
-                    pre: PreAccount::new(
-                        &Account {
-                            owner: *owner,
-                            lamports: std::u64::MAX,
-                            data: vec![],
-                            ..Account::default()
-                        },
-                        true,
-                        &Pubkey::new_rand(), // Force some data, ignored if not needed
-                    ),
-                    post: Account {
-                        owner: *owner,
-                        lamports: std::u64::MAX,
-                        ..Account::default()
-                    },
-                    program_id: *program_id,
-                }
-            }
-            pub fn read_only(mut self) -> Self {
-                self.pre.is_writable = false;
-                self
-            }
-            pub fn executable(mut self, pre: bool, post: bool) -> Self {
-                self.pre.executable = pre;
-                self.post.executable = post;
-                self
-            }
-            pub fn lamports(mut self, pre: u64, post: u64) -> Self {
-                self.pre.lamports = pre;
-                self.post.lamports = post;
-                self
-            }
-            pub fn data(mut self, pre: Vec<u8>, post: Vec<u8>) -> Self {
-                self.pre.data_len = pre.len();
-                self.pre.data = Some(pre);
-                self.post.data = post;
-                self
-            }
-            pub fn verify(self) -> Result<(), InstructionError> {
-                self.pre
-                    .verify(&self.program_id, &RentCollector::default(), &self.post)
-            }
-        }
-
         assert_eq!(
             Change::new(&owner, &system_program_id)
                 .executable(false, true)
@@ -670,31 +652,19 @@ mod tests {
 
     #[test]
     fn test_verify_account_changes_data_len() {
-        assert_eq!(
-            PreAccount::new(
-                &Account::new_data(0, &[0], &system_program::id()).unwrap(),
-                true,
-                &system_program::id()
-            )
-            .verify(
-                &system_program::id(),
-                &RentCollector::default(),
-                &Account::new_data(0, &[0, 0], &system_program::id()).unwrap()
-            ),
-            Ok(()),
-            "system program should be able to change the data len"
-        );
         let alice_program_id = Pubkey::new_rand();
 
         assert_eq!(
-                PreAccount::new(
+            Change::new(&system_program::id(), &system_program::id())
-                    &Account::new_data(0, &[0], &alice_program_id).unwrap(),
+                .data(vec![0], vec![0, 0])
-                    true,
+                .verify(),
-                    &system_program::id(),
+            Ok(()),
-                ).verify(
+            "system program should be able to change the data len"
-                &system_program::id(), &RentCollector::default(),
+        );
-                &Account::new_data(0, &[0, 0], &alice_program_id).unwrap(),
+        assert_eq!(
-            ),
+            Change::new(&alice_program_id, &system_program::id())
+            .data(vec![0], vec![0,0])
+            .verify(),
             Err(InstructionError::AccountDataSizeChanged),
             "system program should not be able to change the data length of accounts it does not own"
         );
@@ -703,32 +673,27 @@ mod tests {
     #[test]
     fn test_verify_account_changes_data() {
         let alice_program_id = Pubkey::new_rand();
-        let change_data =
-            |program_id: &Pubkey, is_writable: bool| -> Result<(), InstructionError> {
-                let pre = PreAccount::new(
-                    &Account::new_data(0, &[0], &alice_program_id).unwrap(),
-                    is_writable,
-                    &program_id,
-                );
-                let post = Account::new_data(0, &[42], &alice_program_id).unwrap();
-                pre.verify(&program_id, &RentCollector::default(), &post)
-            };
-
         let mallory_program_id = Pubkey::new_rand();
 
         assert_eq!(
-            change_data(&alice_program_id, true),
+            Change::new(&alice_program_id, &alice_program_id)
+                .data(vec![0], vec![42])
+                .verify(),
             Ok(()),
             "alice program should be able to change the data"
         );
         assert_eq!(
-            change_data(&mallory_program_id, true),
+            Change::new(&mallory_program_id, &alice_program_id)
+                .data(vec![0], vec![42])
+                .verify(),
             Err(InstructionError::ExternalAccountDataModified),
             "non-owner mallory should not be able to change the account data"
         );
-
         assert_eq!(
-            change_data(&alice_program_id, false),
+            Change::new(&alice_program_id, &alice_program_id)
+                .data(vec![0], vec![42])
+                .read_only()
+                .verify(),
             Err(InstructionError::ReadonlyDataModified),
             "alice isn't allowed to touch a CO account"
         );
@@ -737,23 +702,16 @@ mod tests {
     #[test]
     fn test_verify_account_changes_rent_epoch() {
         let alice_program_id = Pubkey::new_rand();
-        let rent_collector = RentCollector::default();
-        let pre = PreAccount::new(
-            &Account::new(0, 0, &alice_program_id),
-            false,
-            &system_program::id(),
-        );
-        let mut post = Account::new(0, 0, &alice_program_id);
 
         assert_eq!(
-            pre.verify(&system_program::id(), &rent_collector, &post),
+            Change::new(&alice_program_id, &system_program::id()).verify(),
             Ok(()),
             "nothing changed!"
         );
-
-        post.rent_epoch += 1;
         assert_eq!(
-            pre.verify(&system_program::id(), &rent_collector, &post),
+            Change::new(&alice_program_id, &system_program::id())
+                .rent_epoch(0, 1)
+                .verify(),
             Err(InstructionError::RentEpochModified),
             "no one touches rent_epoch"
         );
@@ -763,16 +721,14 @@ mod tests {
     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 = PreAccount::new(
-            &Account::new_data(42, &[42], &alice_program_id).unwrap(),
-            true,
-            &alice_program_id,
-        );
-        let post = Account::new_data(1, &[0], &bob_program_id).unwrap();
 
         // positive test of this capability
         assert_eq!(
-            pre.verify(&alice_program_id, &RentCollector::default(), &post),
+            Change::new(&alice_program_id, &alice_program_id)
+            .owner(&bob_program_id)
+            .lamports(42, 1)
+            .data(vec![42], vec![0])
+            .verify(),
             Ok(()),
             "alice should be able to deduct lamports and give the account to bob if the data is zeroed",
         );
@@ -781,52 +737,36 @@ mod tests {
     #[test]
     fn test_verify_account_changes_lamports() {
         let alice_program_id = Pubkey::new_rand();
-        let rent_collector = RentCollector::default();
-        let pre = PreAccount::new(
-            &Account::new(42, 0, &alice_program_id),
-            false,
-            &system_program::id(),
-        );
-        let post = Account::new(0, 0, &alice_program_id);
 
         assert_eq!(
-            pre.verify(&system_program::id(), &rent_collector, &post),
+            Change::new(&alice_program_id, &system_program::id())
+                .lamports(42, 0)
+                .read_only()
+                .verify(),
             Err(InstructionError::ExternalAccountLamportSpend),
             "debit should fail, even if system program"
         );
-
-        let pre = PreAccount::new(
-            &Account::new(42, 0, &alice_program_id),
-            false,
-            &alice_program_id,
-        );
-
         assert_eq!(
-            pre.verify(&alice_program_id, &rent_collector, &post),
+            Change::new(&alice_program_id, &alice_program_id)
+                .lamports(42, 0)
+                .read_only()
+                .verify(),
             Err(InstructionError::ReadonlyLamportChange),
             "debit should fail, even if owning program"
         );
-
-        let pre = PreAccount::new(
-            &Account::new(42, 0, &alice_program_id),
-            true,
-            &system_program::id(),
-        );
-        let post = Account::new(0, 0, &system_program::id());
         assert_eq!(
-            pre.verify(&system_program::id(), &rent_collector, &post),
+            Change::new(&alice_program_id, &system_program::id())
+                .lamports(42, 0)
+                .owner(&system_program::id())
+                .verify(),
             Err(InstructionError::ModifiedProgramId),
             "system program can't debit the account unless it was the pre.owner"
         );
-
-        let pre = PreAccount::new(
-            &Account::new(42, 0, &system_program::id()),
-            true,
-            &system_program::id(),
-        );
-        let post = Account::new(0, 0, &alice_program_id);
         assert_eq!(
-            pre.verify(&system_program::id(), &rent_collector, &post),
+            Change::new(&system_program::id(), &system_program::id())
+                .lamports(42, 0)
+                .owner(&alice_program_id)
+                .verify(),
             Ok(()),
             "system can spend (and change owner)"
         );
@@ -834,36 +774,26 @@ mod tests {
 
     #[test]
     fn test_verify_account_changes_data_size_changed() {
-        let rent_collector = RentCollector::default();
         let alice_program_id = Pubkey::new_rand();
-        let pre = PreAccount::new(
+
-            &Account::new_data(42, &[0], &alice_program_id).unwrap(),
-            true,
-            &system_program::id(),
-        );
-        let post = Account::new_data(42, &[0, 0], &alice_program_id).unwrap();
         assert_eq!(
-            pre.verify(&system_program::id(), &rent_collector, &post),
+            Change::new(&alice_program_id, &system_program::id())
+                .data(vec![0], vec![0, 0])
+                .verify(),
             Err(InstructionError::AccountDataSizeChanged),
             "system program should not be able to change another program's account data size"
         );
-        let pre = PreAccount::new(
-            &Account::new_data(42, &[0], &alice_program_id).unwrap(),
-            true,
-            &alice_program_id,
-        );
         assert_eq!(
-            pre.verify(&alice_program_id, &rent_collector, &post),
+            Change::new(&alice_program_id, &alice_program_id)
+                .data(vec![0], vec![0, 0])
+                .verify(),
             Err(InstructionError::AccountDataSizeChanged),
             "non-system programs cannot change their data size"
         );
-        let pre = PreAccount::new(
-            &Account::new_data(42, &[0], &system_program::id()).unwrap(),
-            true,
-            &system_program::id(),
-        );
         assert_eq!(
-            pre.verify(&system_program::id(), &rent_collector, &post),
+            Change::new(&system_program::id(), &system_program::id())
+                .data(vec![0], vec![0, 0])
+                .verify(),
             Ok(()),
             "system program should be able to change acount data size"
         );