Add credit-only info to AccountMetadata (#4405)

* Add credit-only flag to AccountMeta, default to false

* Sort keys by is_credit_only within signed/unsigned groupings

* Process and de-dupe program keys along with other account keys

* Add message helper functions

* Fix test

* Improve comment

* s/is_credit_only/is_debitable

* Add InstructionKeys helper struct, and simplify program_position method

programs/config_api/src/config_processor.rs

@@ -153,7 +153,7 @@ mod tests {
         let config_pubkey = config_keypair.pubkey();
 
         let transfer_instruction =
-            system_instruction::transfer(&system_pubkey, &Pubkey::default(), 42);
+            system_instruction::transfer(&system_pubkey, &Pubkey::new_rand(), 42);
         let my_config = MyConfig::new(42);
         let mut store_instruction = config_instruction::store(&config_pubkey, &my_config);
         store_instruction.accounts[0].is_signer = false; // <----- not a signer

runtime/src/accounts.rs

@@ -626,7 +626,7 @@ mod tests {
         let accounts: Vec<(Pubkey, Account)> = Vec::new();
         let mut error_counters = ErrorCounters::default();
 
-        let instructions = vec![CompiledInstruction::new(1, &(), vec![0])];
+        let instructions = vec![CompiledInstruction::new(0, &(), vec![0])];
         let tx = Transaction::new_with_compiled_instructions::<Keypair>(
             &[],
             &[],

runtime/src/message_processor.rs

@@ -221,10 +221,15 @@ impl MessageProcessor {
         tick_height: u64,
     ) -> Result<(), TransactionError> {
         for (instruction_index, instruction) in message.instructions.iter().enumerate() {
-            let executable_accounts = &mut loaders
-                [message.program_index_in_program_ids(instruction.program_ids_index) as usize];
+            let executable_index = message
+                .program_position(instruction.program_ids_index as usize)
+                .ok_or(TransactionError::InvalidAccountIndex)?;
+            let executable_accounts = &mut loaders[executable_index];
             let mut program_accounts = get_subset_unchecked_mut(accounts, &instruction.accounts)
                 .map_err(|err| TransactionError::InstructionError(instruction_index as u8, err))?;
+            // TODO: `get_subset_unchecked_mut` panics on an index out of bounds if an executable
+            // account is also included as a regular account for an instruction, because the
+            // executable account is not passed in as part of the accounts slice
             self.execute_instruction(
                 message,
                 instruction,

sdk/src/instruction.rs

@@ -96,11 +96,25 @@ pub struct AccountMeta {
     pub pubkey: Pubkey,
     /// True if an Instruciton requires a Transaction signature matching `pubkey`.
     pub is_signer: bool,
+    /// True if the `pubkey` can be loaded as a credit-debit account.
+    pub is_debitable: bool,
 }
 
 impl AccountMeta {
     pub fn new(pubkey: Pubkey, is_signer: bool) -> Self {
-        Self { pubkey, is_signer }
+        Self {
+            pubkey,
+            is_signer,
+            is_debitable: true,
+        }
+    }
+
+    pub fn new_credit_only(pubkey: Pubkey, is_signer: bool) -> Self {
+        Self {
+            pubkey,
+            is_signer,
+            is_debitable: false,
+        }
     }
 }
 

sdk/src/message.rs

@@ -30,35 +30,88 @@ fn compile_instructions(ixs: Vec<Instruction>, keys: &[Pubkey]) -> Vec<CompiledI
         .collect()
 }
 
-/// Return pubkeys referenced by all instructions, with the ones needing signatures first.
-/// If the payer key is provided, it is always placed first in the list of signed keys.
-/// No duplicates and order is preserved.
-fn get_keys(instructions: &[Instruction], payer: Option<&Pubkey>) -> (Vec<Pubkey>, Vec<Pubkey>) {
+/// A helper struct to collect pubkeys referenced by a set of instructions and credit-only counts
+#[derive(Debug, PartialEq, Eq)]
+struct InstructionKeys {
+    pub signed_keys: Vec<Pubkey>,
+    pub unsigned_keys: Vec<Pubkey>,
+    pub num_credit_only_signed_accounts: u8,
+    pub num_credit_only_unsigned_accounts: u8,
+}
+
+impl InstructionKeys {
+    fn new(
+        signed_keys: Vec<Pubkey>,
+        unsigned_keys: Vec<Pubkey>,
+        num_credit_only_signed_accounts: u8,
+        num_credit_only_unsigned_accounts: u8,
+    ) -> Self {
+        Self {
+            signed_keys,
+            unsigned_keys,
+            num_credit_only_signed_accounts,
+            num_credit_only_unsigned_accounts,
+        }
+    }
+}
+
+/// Return pubkeys referenced by all instructions, with the ones needing signatures first. If the
+/// payer key is provided, it is always placed first in the list of signed keys. Credit-only signed
+/// accounts are placed last in the set of signed accounts. Credit-only unsigned accounts,
+/// including program ids, are placed last in the set. No duplicates and order is preserved.
+fn get_keys(instructions: &[Instruction], payer: Option<&Pubkey>) -> InstructionKeys {
+    let programs: Vec<_> = get_program_ids(instructions)
+        .iter()
+        .map(|program_id| AccountMeta {
+            pubkey: *program_id,
+            is_signer: false,
+            is_debitable: false,
+        })
+        .collect();
     let mut keys_and_signed: Vec<_> = instructions
         .iter()
         .flat_map(|ix| ix.accounts.iter())
         .collect();
-    keys_and_signed.sort_by(|x, y| y.is_signer.cmp(&x.is_signer));
+    keys_and_signed.extend(&programs);
+    keys_and_signed.sort_by(|x, y| {
+        y.is_signer
+            .cmp(&x.is_signer)
+            .then(y.is_debitable.cmp(&x.is_debitable))
+    });
 
     let payer_account_meta;
     if let Some(payer) = payer {
         payer_account_meta = AccountMeta {
             pubkey: *payer,
             is_signer: true,
+            is_debitable: true,
         };
         keys_and_signed.insert(0, &payer_account_meta);
     }
 
     let mut signed_keys = vec![];
     let mut unsigned_keys = vec![];
+    let mut num_credit_only_signed_accounts = 0;
+    let mut num_credit_only_unsigned_accounts = 0;
     for account_meta in keys_and_signed.into_iter().unique_by(|x| x.pubkey) {
         if account_meta.is_signer {
             signed_keys.push(account_meta.pubkey);
+            if !account_meta.is_debitable {
+                num_credit_only_signed_accounts += 1;
+            }
         } else {
             unsigned_keys.push(account_meta.pubkey);
+            if !account_meta.is_debitable {
+                num_credit_only_unsigned_accounts += 1;
+            }
         }
     }
-    (signed_keys, unsigned_keys)
+    InstructionKeys::new(
+        signed_keys,
+        unsigned_keys,
+        num_credit_only_signed_accounts,
+        num_credit_only_unsigned_accounts,
+    )
 }
 
 /// Return program ids referenced by all instructions.  No duplicates and order is preserved.
@@ -130,13 +183,14 @@ impl Message {
     }
 
     pub fn new_with_payer(instructions: Vec<Instruction>, payer: Option<&Pubkey>) -> Self {
-        let program_ids = get_program_ids(&instructions);
-        let (mut signed_keys, unsigned_keys) = get_keys(&instructions, payer);
+        let InstructionKeys {
+            mut signed_keys,
+            unsigned_keys,
+            num_credit_only_signed_accounts,
+            num_credit_only_unsigned_accounts,
+        } = get_keys(&instructions, payer);
         let num_required_signatures = signed_keys.len() as u8;
-        let num_credit_only_signed_accounts = 0;
-        let num_credit_only_unsigned_accounts = program_ids.len() as u8;
         signed_keys.extend(&unsigned_keys);
-        signed_keys.extend(&program_ids);
         let instructions = compile_instructions(instructions, &signed_keys);
         Self::new_with_compiled_instructions(
             num_required_signatures,
@@ -148,13 +202,39 @@ impl Message {
         )
     }
 
-    pub fn program_ids(&self) -> &[Pubkey] {
-        &self.account_keys
-            [self.account_keys.len() - self.header.num_credit_only_unsigned_accounts as usize..]
+    pub fn program_ids(&self) -> Vec<&Pubkey> {
+        self.instructions
+            .iter()
+            .map(|ix| &self.account_keys[ix.program_ids_index as usize])
+            .collect()
     }
 
-    pub fn program_index_in_program_ids(&self, index: u8) -> u8 {
-        index - (self.account_keys.len() as u8 - self.header.num_credit_only_unsigned_accounts)
+    pub fn program_position(&self, index: usize) -> Option<usize> {
+        let program_ids = self.program_ids();
+        program_ids
+            .iter()
+            .position(|&&pubkey| pubkey == self.account_keys[index])
+    }
+
+    fn is_credit_debit(&self, i: usize) -> bool {
+        i < (self.header.num_required_signatures - self.header.num_credit_only_signed_accounts)
+            as usize
+            || (i >= self.header.num_required_signatures as usize
+                && i < self.account_keys.len()
+                    - self.header.num_credit_only_unsigned_accounts as usize)
+    }
+
+    pub fn get_account_keys_by_lock_type(&self) -> (Vec<&Pubkey>, Vec<&Pubkey>) {
+        let mut credit_debit_keys = vec![];
+        let mut credit_only_keys = vec![];
+        for (i, key) in self.account_keys.iter().enumerate() {
+            if self.is_credit_debit(i) {
+                credit_debit_keys.push(key);
+            } else {
+                credit_only_keys.push(key);
+            }
+        }
+        (credit_debit_keys, credit_only_keys)
     }
 }
 
@@ -209,7 +289,7 @@ mod tests {
             ],
             None,
         );
-        assert_eq!(keys, (vec![id0], vec![]));
+        assert_eq!(keys, InstructionKeys::new(vec![id0], vec![], 0, 0));
     }
 
     #[test]
@@ -224,7 +304,7 @@ mod tests {
             )],
             Some(&id0),
         );
-        assert_eq!(keys, (vec![id0], vec![]));
+        assert_eq!(keys, InstructionKeys::new(vec![id0], vec![], 0, 0));
     }
 
     #[test]
@@ -239,7 +319,7 @@ mod tests {
             )],
             Some(&id0),
         );
-        assert_eq!(keys, (vec![id0], vec![]));
+        assert_eq!(keys, InstructionKeys::new(vec![id0], vec![], 0, 0));
     }
 
     #[test]
@@ -253,7 +333,7 @@ mod tests {
             ],
             None,
         );
-        assert_eq!(keys, (vec![id0], vec![]));
+        assert_eq!(keys, InstructionKeys::new(vec![id0], vec![], 0, 0));
     }
 
     #[test]
@@ -268,7 +348,7 @@ mod tests {
             ],
             None,
         );
-        assert_eq!(keys, (vec![], vec![id0, id1]));
+        assert_eq!(keys, InstructionKeys::new(vec![], vec![id0, id1], 0, 0));
     }
 
     #[test]
@@ -284,7 +364,7 @@ mod tests {
             ],
             None,
         );
-        assert_eq!(keys, (vec![id0], vec![id1]));
+        assert_eq!(keys, InstructionKeys::new(vec![id0], vec![id1], 0, 0));
     }
 
     #[test]
@@ -299,7 +379,7 @@ mod tests {
             ],
             None,
         );
-        assert_eq!(keys, (vec![id1], vec![id0]));
+        assert_eq!(keys, InstructionKeys::new(vec![id1], vec![id0], 0, 0));
     }
 
     #[test]
@@ -316,6 +396,36 @@ mod tests {
         assert_eq!(message.header.num_required_signatures, 1);
     }
 
+    #[test]
+    fn test_message_credit_only_keys_last() {
+        let program_id = Pubkey::default();
+        let id0 = Pubkey::default(); // Identical key/program_id should be de-duped
+        let id1 = Pubkey::new_rand();
+        let id2 = Pubkey::new_rand();
+        let id3 = Pubkey::new_rand();
+        let keys = get_keys(
+            &[
+                Instruction::new(
+                    program_id,
+                    &0,
+                    vec![AccountMeta::new_credit_only(id0, false)],
+                ),
+                Instruction::new(
+                    program_id,
+                    &0,
+                    vec![AccountMeta::new_credit_only(id1, true)],
+                ),
+                Instruction::new(program_id, &0, vec![AccountMeta::new(id2, false)]),
+                Instruction::new(program_id, &0, vec![AccountMeta::new(id3, true)]),
+            ],
+            None,
+        );
+        assert_eq!(
+            keys,
+            InstructionKeys::new(vec![id3, id1], vec![id2, id0], 1, 1)
+        );
+    }
+
     #[test]
     fn test_message_kitchen_sink() {
         let program_id0 = Pubkey::new_rand();
@@ -365,4 +475,97 @@ mod tests {
         assert_eq!(message.header.num_required_signatures, 2);
     }
 
+    #[test]
+    fn test_message_program_last() {
+        let program_id = Pubkey::default();
+        let id0 = Pubkey::new_rand();
+        let id1 = Pubkey::new_rand();
+        let keys = get_keys(
+            &[
+                Instruction::new(
+                    program_id,
+                    &0,
+                    vec![AccountMeta::new_credit_only(id0, false)],
+                ),
+                Instruction::new(
+                    program_id,
+                    &0,
+                    vec![AccountMeta::new_credit_only(id1, true)],
+                ),
+            ],
+            None,
+        );
+        assert_eq!(
+            keys,
+            InstructionKeys::new(vec![id1], vec![id0, program_id], 1, 2)
+        );
+    }
+
+    #[test]
+    fn test_program_position() {
+        let program_id0 = Pubkey::default();
+        let program_id1 = Pubkey::new_rand();
+        let id = Pubkey::new_rand();
+        let message = Message::new(vec![
+            Instruction::new(program_id0, &0, vec![AccountMeta::new(id, false)]),
+            Instruction::new(program_id1, &0, vec![AccountMeta::new(id, true)]),
+        ]);
+        assert_eq!(message.program_position(0), None);
+        assert_eq!(message.program_position(1), Some(0));
+        assert_eq!(message.program_position(2), Some(1));
+    }
+
+    #[test]
+    fn test_is_credit_debit() {
+        let key0 = Pubkey::new_rand();
+        let key1 = Pubkey::new_rand();
+        let key2 = Pubkey::new_rand();
+        let key3 = Pubkey::new_rand();
+        let key4 = Pubkey::new_rand();
+        let key5 = Pubkey::new_rand();
+
+        let message = Message {
+            header: MessageHeader {
+                num_required_signatures: 3,
+                num_credit_only_signed_accounts: 2,
+                num_credit_only_unsigned_accounts: 1,
+            },
+            account_keys: vec![key0, key1, key2, key3, key4, key5],
+            recent_blockhash: Hash::default(),
+            instructions: vec![],
+        };
+        assert_eq!(message.is_credit_debit(0), true);
+        assert_eq!(message.is_credit_debit(1), false);
+        assert_eq!(message.is_credit_debit(2), false);
+        assert_eq!(message.is_credit_debit(3), true);
+        assert_eq!(message.is_credit_debit(4), true);
+        assert_eq!(message.is_credit_debit(5), false);
+    }
+
+    #[test]
+    fn test_get_account_keys_by_lock_type() {
+        let program_id = Pubkey::default();
+        let id0 = Pubkey::new_rand();
+        let id1 = Pubkey::new_rand();
+        let id2 = Pubkey::new_rand();
+        let id3 = Pubkey::new_rand();
+        let message = Message::new(vec![
+            Instruction::new(program_id, &0, vec![AccountMeta::new(id0, false)]),
+            Instruction::new(program_id, &0, vec![AccountMeta::new(id1, true)]),
+            Instruction::new(
+                program_id,
+                &0,
+                vec![AccountMeta::new_credit_only(id2, false)],
+            ),
+            Instruction::new(
+                program_id,
+                &0,
+                vec![AccountMeta::new_credit_only(id3, true)],
+            ),
+        ]);
+        assert_eq!(
+            message.get_account_keys_by_lock_type(),
+            (vec![&id1, &id0], vec![&id3, &id2, &program_id])
+        );
+    }
 }

sdk/src/transaction.rs

@@ -246,7 +246,7 @@ mod tests {
     fn get_program_id(tx: &Transaction, instruction_index: usize) -> &Pubkey {
         let message = tx.message();
         let instruction = &message.instructions[instruction_index];
-        instruction.program_id(message.program_ids())
+        instruction.program_id(&message.account_keys)
     }
 
     #[test]
@@ -257,8 +257,8 @@ mod tests {
         let prog1 = Pubkey::new_rand();
         let prog2 = Pubkey::new_rand();
         let instructions = vec![
-            CompiledInstruction::new(0, &(), vec![0, 1]),
-            CompiledInstruction::new(1, &(), vec![0, 2]),
+            CompiledInstruction::new(3, &(), vec![0, 1]),
+            CompiledInstruction::new(4, &(), vec![0, 2]),
         ];
         let tx = Transaction::new_with_compiled_instructions(
             &[&key],
@@ -306,7 +306,7 @@ mod tests {
     #[test]
     fn test_refs_invalid_account() {
         let key = Keypair::new();
-        let instructions = vec![CompiledInstruction::new(0, &(), vec![2])];
+        let instructions = vec![CompiledInstruction::new(1, &(), vec![2])];
         let tx = Transaction::new_with_compiled_instructions(
             &[&key],
             &[],