Address post-merge review for #7353 (#7378)

automerge

runtime/src/nonce_utils.rs

@@ -161,7 +161,7 @@ mod tests {
             assert_eq!(state, NonceState::Uninitialized);
             let recent_blockhashes = create_test_recent_blockhashes(0);
             nonce_account
-                .initialize(&recent_blockhashes, &Rent::default(), &signers)
+                .initialize(&recent_blockhashes, &Rent::free())
                 .unwrap();
             assert!(verify_nonce(&nonce_account.account, &recent_blockhashes[0]));
         });
@@ -184,7 +184,7 @@ mod tests {
             assert_eq!(state, NonceState::Uninitialized);
             let recent_blockhashes = create_test_recent_blockhashes(0);
             nonce_account
-                .initialize(&recent_blockhashes, &Rent::default(), &signers)
+                .initialize(&recent_blockhashes, &Rent::free())
                 .unwrap();
             assert!(!verify_nonce(
                 &nonce_account.account,

sdk/src/nonce_instruction.rs

@@ -41,7 +41,6 @@ pub enum NonceInstruction {
     /// Expects 3 Accounts:
     ///     0 - A NonceAccount
     ///     1 - RecentBlockhashes sysvar
-    ///     2 - Rent sysvar
     ///
     Nonce,
 
@@ -84,7 +83,7 @@ pub fn create_nonce_account(
     ]
 }
 
-pub fn initialize(nonce_pubkey: &Pubkey) -> Instruction {
+fn initialize(nonce_pubkey: &Pubkey) -> Instruction {
     Instruction::new(
         id(),
         &NonceInstruction::Initialize,
@@ -103,7 +102,6 @@ pub fn nonce(nonce_pubkey: &Pubkey) -> Instruction {
         vec![
             AccountMeta::new(*nonce_pubkey, true),
             AccountMeta::new_readonly(recent_blockhashes::id(), false),
-            AccountMeta::new_readonly(rent::id(), false),
         ],
     )
 }
@@ -149,7 +147,6 @@ pub fn process_instruction(
         NonceInstruction::Initialize => me.initialize(
             &RecentBlockhashes::from_keyed_account(next_keyed_account(keyed_accounts)?)?,
             &Rent::from_keyed_account(next_keyed_account(keyed_accounts)?)?,
-            &signers,
         ),
     }
 }
@@ -361,6 +358,34 @@ mod tests {
         );
     }
 
+    #[test]
+    fn test_process_withdraw_ix_bad_rent_state_fail() {
+        assert_eq!(
+            super::process_instruction(
+                &Pubkey::default(),
+                &mut [
+                    KeyedAccount::new(
+                        &Pubkey::default(),
+                        true,
+                        &mut nonce_state::create_account(1_000_000),
+                    ),
+                    KeyedAccount::new(&Pubkey::default(), true, &mut Account::default(),),
+                    KeyedAccount::new(
+                        &sysvar::recent_blockhashes::id(),
+                        false,
+                        &mut sysvar::recent_blockhashes::create_account_with_data(
+                            1,
+                            vec![(0u64, &Hash::default()); 32].into_iter(),
+                        ),
+                    ),
+                    KeyedAccount::new(&sysvar::rent::id(), false, &mut Account::default(),),
+                ],
+                &serialize(&NonceInstruction::Withdraw(42)).unwrap(),
+            ),
+            Err(InstructionError::InvalidArgument),
+        );
+    }
+
     #[test]
     fn test_process_withdraw_ix_ok() {
         assert_eq!(
@@ -393,6 +418,123 @@ mod tests {
         );
     }
 
+    #[test]
+    fn test_process_initialize_ix_invalid_acc_data_fail() {
+        assert_eq!(
+            process_instruction(&initialize(&Pubkey::default())),
+            Err(InstructionError::InvalidAccountData),
+        );
+    }
+
+    #[test]
+    fn test_process_initialize_ix_no_keyed_accs_fail() {
+        assert_eq!(
+            super::process_instruction(
+                &Pubkey::default(),
+                &mut [],
+                &serialize(&NonceInstruction::Initialize).unwrap(),
+            ),
+            Err(InstructionError::NotEnoughAccountKeys),
+        );
+    }
+
+    #[test]
+    fn test_process_initialize_ix_only_nonce_acc_fail() {
+        assert_eq!(
+            super::process_instruction(
+                &Pubkey::default(),
+                &mut [KeyedAccount::new(
+                    &Pubkey::default(),
+                    true,
+                    &mut nonce_state::create_account(1_000_000),
+                ),],
+                &serialize(&NonceInstruction::Initialize).unwrap(),
+            ),
+            Err(InstructionError::NotEnoughAccountKeys),
+        );
+    }
+
+    #[test]
+    fn test_process_initialize_bad_recent_blockhash_state_fail() {
+        assert_eq!(
+            super::process_instruction(
+                &Pubkey::default(),
+                &mut [
+                    KeyedAccount::new(
+                        &Pubkey::default(),
+                        true,
+                        &mut nonce_state::create_account(1_000_000),
+                    ),
+                    KeyedAccount::new(
+                        &sysvar::recent_blockhashes::id(),
+                        false,
+                        &mut Account::default(),
+                    ),
+                ],
+                &serialize(&NonceInstruction::Initialize).unwrap(),
+            ),
+            Err(InstructionError::InvalidArgument),
+        );
+    }
+
+    #[test]
+    fn test_process_initialize_ix_bad_rent_state_fail() {
+        assert_eq!(
+            super::process_instruction(
+                &Pubkey::default(),
+                &mut [
+                    KeyedAccount::new(
+                        &Pubkey::default(),
+                        true,
+                        &mut nonce_state::create_account(1_000_000),
+                    ),
+                    KeyedAccount::new(
+                        &sysvar::recent_blockhashes::id(),
+                        false,
+                        &mut sysvar::recent_blockhashes::create_account_with_data(
+                            1,
+                            vec![(0u64, &Hash::default()); 32].into_iter(),
+                        ),
+                    ),
+                    KeyedAccount::new(&sysvar::rent::id(), false, &mut Account::default(),),
+                ],
+                &serialize(&NonceInstruction::Initialize).unwrap(),
+            ),
+            Err(InstructionError::InvalidArgument),
+        );
+    }
+
+    #[test]
+    fn test_process_initialize_ix_ok() {
+        assert_eq!(
+            super::process_instruction(
+                &Pubkey::default(),
+                &mut [
+                    KeyedAccount::new(
+                        &Pubkey::default(),
+                        true,
+                        &mut nonce_state::create_account(1_000_000),
+                    ),
+                    KeyedAccount::new(
+                        &sysvar::recent_blockhashes::id(),
+                        false,
+                        &mut sysvar::recent_blockhashes::create_account_with_data(
+                            1,
+                            vec![(0u64, &Hash::default()); 32].into_iter(),
+                        ),
+                    ),
+                    KeyedAccount::new(
+                        &sysvar::rent::id(),
+                        false,
+                        &mut sysvar::rent::create_account(1, &Rent::default())
+                    ),
+                ],
+                &serialize(&NonceInstruction::Initialize).unwrap(),
+            ),
+            Ok(()),
+        );
+    }
+
     #[test]
     fn test_custom_error_decode() {
         use num_traits::FromPrimitive;

sdk/src/nonce_state.rs

@@ -58,7 +58,6 @@ pub trait NonceAccount {
         &mut self,
         recent_blockhashes: &RecentBlockhashes,
         rent: &Rent,
-        signers: &HashSet<Pubkey>,
     ) -> Result<(), InstructionError>;
 }
 
@@ -112,7 +111,6 @@ impl<'a> NonceAccount for KeyedAccount<'a> {
                     if *hash == recent_blockhashes[0] {
                         return Err(NonceError::NotExpired.into());
                     }
-                    self.set_state(&NonceState::Uninitialized)?;
                 } else {
                     let min_balance = rent.minimum_balance(self.account.data.len());
                     if lamports + min_balance > self.account.lamports {
@@ -132,16 +130,11 @@ impl<'a> NonceAccount for KeyedAccount<'a> {
         &mut self,
         recent_blockhashes: &RecentBlockhashes,
         rent: &Rent,
-        signers: &HashSet<Pubkey>,
     ) -> Result<(), InstructionError> {
         if recent_blockhashes.is_empty() {
             return Err(NonceError::NoRecentBlockhashes.into());
         }
 
-        if !signers.contains(self.unsigned_key()) {
-            return Err(InstructionError::MissingRequiredSignature);
-        }
-
         let meta = match self.state()? {
             NonceState::Uninitialized => {
                 let min_balance = rent.minimum_balance(self.account.data.len());
@@ -215,7 +208,7 @@ mod test {
             assert_eq!(state, NonceState::Uninitialized);
             let recent_blockhashes = create_test_recent_blockhashes(95);
             keyed_account
-                .initialize(&recent_blockhashes, &rent, &signers)
+                .initialize(&recent_blockhashes, &rent)
                 .unwrap();
             let state: NonceState = keyed_account.state().unwrap();
             let stored = recent_blockhashes[0];
@@ -247,10 +240,6 @@ mod test {
                         &signers,
                     )
                     .unwrap();
-                let state: NonceState = keyed_account.state().unwrap();
-                // Withdraw instruction...
-                // Deinitializes NonceAccount state
-                assert_eq!(state, NonceState::Uninitialized);
                 // Empties NonceAccount balance
                 assert_eq!(keyed_account.account.lamports, expect_nonce_lamports);
                 // NonceAccount balance goes to `to`
@@ -268,12 +257,10 @@ mod test {
         let min_lamports = rent.minimum_balance(NonceState::size());
         let meta = Meta::new();
         with_test_keyed_account(min_lamports + 42, true, |nonce_account| {
-            let mut signers = HashSet::new();
-            signers.insert(nonce_account.signer_key().unwrap().clone());
             let recent_blockhashes = create_test_recent_blockhashes(31);
             let stored = recent_blockhashes[0];
             nonce_account
-                .initialize(&recent_blockhashes, &rent, &signers)
+                .initialize(&recent_blockhashes, &rent)
                 .unwrap();
             let pubkey = nonce_account.account.owner.clone();
             let mut nonce_account = KeyedAccount::new(&pubkey, false, nonce_account.account);
@@ -298,7 +285,7 @@ mod test {
             signers.insert(keyed_account.signer_key().unwrap().clone());
             let recent_blockhashes = create_test_recent_blockhashes(0);
             keyed_account
-                .initialize(&recent_blockhashes, &rent, &signers)
+                .initialize(&recent_blockhashes, &rent)
                 .unwrap();
             let recent_blockhashes = RecentBlockhashes::from_iter(vec![].into_iter());
             let result = keyed_account.nonce(&recent_blockhashes, &signers);
@@ -318,7 +305,7 @@ mod test {
             signers.insert(keyed_account.signer_key().unwrap().clone());
             let recent_blockhashes = create_test_recent_blockhashes(63);
             keyed_account
-                .initialize(&recent_blockhashes, &rent, &signers)
+                .initialize(&recent_blockhashes, &rent)
                 .unwrap();
             let result = keyed_account.nonce(&recent_blockhashes, &signers);
             assert_eq!(result, Err(NonceError::NotExpired.into()));
@@ -490,9 +477,7 @@ mod test {
             let mut signers = HashSet::new();
             signers.insert(nonce_keyed.signer_key().unwrap().clone());
             let recent_blockhashes = create_test_recent_blockhashes(31);
-            nonce_keyed
+            nonce_keyed.initialize(&recent_blockhashes, &rent).unwrap();
-                .initialize(&recent_blockhashes, &rent, &signers)
-                .unwrap();
             let state: NonceState = nonce_keyed.state().unwrap();
             let stored = recent_blockhashes[0];
             assert_eq!(state, NonceState::Initialized(meta, stored));
@@ -527,8 +512,6 @@ mod test {
                         &signers,
                     )
                     .unwrap();
-                let state: NonceState = nonce_keyed.state().unwrap();
-                assert_eq!(state, NonceState::Uninitialized);
                 assert_eq!(nonce_keyed.account.lamports, nonce_expect_lamports);
                 assert_eq!(to_keyed.account.lamports, to_expect_lamports);
             })
@@ -543,12 +526,8 @@ mod test {
         };
         let min_lamports = rent.minimum_balance(NonceState::size());
         with_test_keyed_account(min_lamports + 42, true, |nonce_keyed| {
-            let mut signers = HashSet::new();
-            signers.insert(nonce_keyed.signer_key().unwrap().clone());
             let recent_blockhashes = create_test_recent_blockhashes(0);
-            nonce_keyed
+            nonce_keyed.initialize(&recent_blockhashes, &rent).unwrap();
-                .initialize(&recent_blockhashes, &rent, &signers)
-                .unwrap();
             with_test_keyed_account(42, false, |mut to_keyed| {
                 let mut signers = HashSet::new();
                 signers.insert(nonce_keyed.signer_key().unwrap().clone());
@@ -573,12 +552,8 @@ mod test {
         };
         let min_lamports = rent.minimum_balance(NonceState::size());
         with_test_keyed_account(min_lamports + 42, true, |nonce_keyed| {
-            let mut signers = HashSet::new();
-            signers.insert(nonce_keyed.signer_key().unwrap().clone());
             let recent_blockhashes = create_test_recent_blockhashes(95);
-            nonce_keyed
+            nonce_keyed.initialize(&recent_blockhashes, &rent).unwrap();
-                .initialize(&recent_blockhashes, &rent, &signers)
-                .unwrap();
             with_test_keyed_account(42, false, |mut to_keyed| {
                 let recent_blockhashes = create_test_recent_blockhashes(63);
                 let mut signers = HashSet::new();
@@ -604,12 +579,8 @@ mod test {
         };
         let min_lamports = rent.minimum_balance(NonceState::size());
         with_test_keyed_account(min_lamports + 42, true, |nonce_keyed| {
-            let mut signers = HashSet::new();
-            signers.insert(nonce_keyed.signer_key().unwrap().clone());
             let recent_blockhashes = create_test_recent_blockhashes(95);
-            nonce_keyed
+            nonce_keyed.initialize(&recent_blockhashes, &rent).unwrap();
-                .initialize(&recent_blockhashes, &rent, &signers)
-                .unwrap();
             with_test_keyed_account(42, false, |mut to_keyed| {
                 let recent_blockhashes = create_test_recent_blockhashes(63);
                 let mut signers = HashSet::new();
@@ -641,7 +612,7 @@ mod test {
             signers.insert(keyed_account.signer_key().unwrap().clone());
             let recent_blockhashes = create_test_recent_blockhashes(0);
             let stored = recent_blockhashes[0];
-            let result = keyed_account.initialize(&recent_blockhashes, &rent, &signers);
+            let result = keyed_account.initialize(&recent_blockhashes, &rent);
             assert_eq!(result, Ok(()));
             let state: NonceState = keyed_account.state().unwrap();
             assert_eq!(state, NonceState::Initialized(Meta::new(), stored));
@@ -659,26 +630,11 @@ mod test {
             let mut signers = HashSet::new();
             signers.insert(keyed_account.signer_key().unwrap().clone());
             let recent_blockhashes = RecentBlockhashes::from_iter(vec![].into_iter());
-            let result = keyed_account.initialize(&recent_blockhashes, &rent, &signers);
+            let result = keyed_account.initialize(&recent_blockhashes, &rent);
             assert_eq!(result, Err(NonceError::NoRecentBlockhashes.into()));
         })
     }
 
-    #[test]
-    fn initialize_inx_not_signer_fail() {
-        let rent = Rent {
-            lamports_per_byte_year: 42,
-            ..Rent::default()
-        };
-        let min_lamports = rent.minimum_balance(NonceState::size());
-        with_test_keyed_account(min_lamports + 42, false, |keyed_account| {
-            let signers = HashSet::new();
-            let recent_blockhashes = create_test_recent_blockhashes(0);
-            let result = keyed_account.initialize(&recent_blockhashes, &rent, &signers);
-            assert_eq!(result, Err(InstructionError::MissingRequiredSignature));
-        })
-    }
-
     #[test]
     fn initialize_inx_initialized_account_fail() {
         let rent = Rent {
@@ -687,14 +643,12 @@ mod test {
         };
         let min_lamports = rent.minimum_balance(NonceState::size());
         with_test_keyed_account(min_lamports + 42, true, |keyed_account| {
-            let mut signers = HashSet::new();
-            signers.insert(keyed_account.signer_key().unwrap().clone());
             let recent_blockhashes = create_test_recent_blockhashes(31);
             keyed_account
-                .initialize(&recent_blockhashes, &rent, &signers)
+                .initialize(&recent_blockhashes, &rent)
                 .unwrap();
             let recent_blockhashes = create_test_recent_blockhashes(0);
-            let result = keyed_account.initialize(&recent_blockhashes, &rent, &signers);
+            let result = keyed_account.initialize(&recent_blockhashes, &rent);
             assert_eq!(result, Err(NonceError::BadAccountState.into()));
         })
     }
@@ -707,10 +661,8 @@ mod test {
         };
         let min_lamports = rent.minimum_balance(NonceState::size());
         with_test_keyed_account(min_lamports - 42, true, |keyed_account| {
-            let mut signers = HashSet::new();
-            signers.insert(keyed_account.signer_key().unwrap().clone());
             let recent_blockhashes = create_test_recent_blockhashes(63);
-            let result = keyed_account.initialize(&recent_blockhashes, &rent, &signers);
+            let result = keyed_account.initialize(&recent_blockhashes, &rent);
             assert_eq!(result, Err(InstructionError::InsufficientFunds));
         })
     }