draft: loop-counter bug

Author: buffalojoec

program/src/processor.rs

@@ -3,10 +3,7 @@
 use {
     crate::state::ConfigKeys,
     solana_program::{
-        account_info::{next_account_info, AccountInfo},
-        entrypoint::ProgramResult,
-        msg,
-        program_error::ProgramError,
+        account_info::AccountInfo, entrypoint::ProgramResult, msg, program_error::ProgramError,
         pubkey::Pubkey,
     },
     std::collections::BTreeSet,
@@ -28,8 +25,7 @@ where
 pub fn process(program_id: &Pubkey, accounts: &[AccountInfo], input: &[u8]) -> ProgramResult {
     let key_list: ConfigKeys = limited_deserialize(input)?;
 
-    let mut accounts_iter = accounts.iter();
-    let config_account = next_account_info(&mut accounts_iter)?;
+    let config_account = accounts.first().ok_or(ProgramError::NotEnoughAccountKeys)?;
 
     if config_account.owner != program_id {
         msg!("Config account is not owned by the config program");
@@ -61,7 +57,7 @@ pub fn process(program_id: &Pubkey, accounts: &[AccountInfo], input: &[u8]) -> P
     for (signer, _) in key_list.keys.iter().filter(|(_, is_signer)| *is_signer) {
         counter = counter.saturating_add(1);
         if signer != config_account.key {
-            let signer_account = next_account_info(&mut accounts_iter).map_err(|_| {
+            let signer_account = accounts.get(counter).ok_or({
                 msg!("account {:?} is not in account list", signer);
                 ProgramError::MissingRequiredSignature
             })?;

program/tests/functional.rs

@@ -314,6 +314,22 @@ fn test_config_updates() {
     let updated_config_account = result.get_account(&config).unwrap();
 
     // Attempt update with incomplete signatures.
+    let keys = vec![
+        (pubkey, false),
+        (signer1, true), // Missing signer0.
+    ];
+    let instruction = config_instruction::store(&config, false, keys, &my_config);
+    mollusk.process_and_validate_instruction(
+        &instruction,
+        &[
+            (config, updated_config_account.clone()),
+            // Missing signer0.
+            (signer1, AccountSharedData::default()),
+        ],
+        &[Check::err(ProgramError::MissingRequiredSignature)],
+    );
+
+    // Do it again, this time missing signer1.
     let keys = vec![
         (pubkey, false),
         (signer0, true), // Missing signer1.
@@ -323,7 +339,8 @@ fn test_config_updates() {
         &instruction,
         &[
             (config, updated_config_account.clone()),
-            (signer0, AccountSharedData::default()), // Missing signer1.
+            (signer0, AccountSharedData::default()),
+            // Missing signer1.
         ],
         &[Check::err(ProgramError::MissingRequiredSignature)],
     );
@@ -346,6 +363,55 @@ fn test_config_updates() {
     );
 }
 
+#[test]
+fn test_config_updates_empty_current_keys() {
+    let mollusk = setup();
+
+    let config = Pubkey::new_unique();
+
+    // New keys contains the config account, as well as another signer.
+    let signer = Pubkey::new_unique();
+    let new_keys = vec![(config, true), (signer, true)];
+    let my_config = MyConfig::new(42);
+
+    let config_account = {
+        // Allocate enough space for they `new_keys`, but leave the account
+        // uninitalized.
+        let space = get_config_space(new_keys.len());
+        let lamports = mollusk.sysvars.rent.minimum_balance(space);
+        AccountSharedData::new(lamports, space, &solana_config_program::id())
+    };
+
+    let mut instruction = config_instruction::store(&config, true, new_keys, &my_config);
+    mollusk.process_and_validate_instruction(
+        &instruction,
+        &[
+            (config, config_account.clone()),
+            (signer, AccountSharedData::default()),
+        ],
+        &[Check::err(ProgramError::MissingRequiredSignature)],
+    );
+
+    // This is kind of strange, since the `store` helper was taken directly
+    // from the builtin crate, and it's designed to only add the config account
+    // once, even if it's a signer.
+    // However, if you include it in the instruction twice, the loop-counter
+    // mechanism of the processor actually works...
+    instruction.accounts = vec![
+        AccountMeta::new(config, true),
+        AccountMeta::new(config, true),
+        AccountMeta::new(signer, true),
+    ];
+    mollusk.process_and_validate_instruction(
+        &instruction,
+        &[
+            (config, config_account),
+            (signer, AccountSharedData::default()),
+        ],
+        &[Check::success()],
+    );
+}
+
 #[test]
 fn test_config_initialize_contains_duplicates_fails() {
     let mollusk = setup();