Parcourir la source

Make `InstructionAccount` compatible with `#[repr(C)]` (#6829)

* Access is_signer and is_writable via functions

* Change type to u8

* Fix CI warnings

* Use != instead of ==

* Make struct #[repr(C)]
Lucas Ste il y a 4 mois
Parent
commit
a99e440241

+ 8 - 14
ledger-tool/src/program.rs

@@ -409,13 +409,7 @@ pub fn program(ledger_path: &Path, matches: &ArgMatches<'_>) {
                 pubkey,
                 AccountSharedData::new(0, allocation_size, &Pubkey::new_unique()),
             ));
-            instruction_accounts.push(InstructionAccount {
-                index_in_transaction: 0,
-                index_in_caller: 0,
-                index_in_callee: 0,
-                is_signer: false,
-                is_writable: true,
-            });
+            instruction_accounts.push(InstructionAccount::new(0, 0, 0, false, true));
             vec![]
         }
         Err(_) => {
@@ -486,13 +480,13 @@ pub fn program(ledger_path: &Path, matches: &ArgMatches<'_>) {
                         transaction_accounts.push((pubkey, account));
                         idx
                     };
-                    InstructionAccount {
-                        index_in_transaction: txn_acct_index as IndexOfAccount,
-                        index_in_caller: txn_acct_index as IndexOfAccount,
-                        index_in_callee: txn_acct_index as IndexOfAccount,
-                        is_signer: account_info.is_signer.unwrap_or(false),
-                        is_writable: account_info.is_writable.unwrap_or(false),
-                    }
+                    InstructionAccount::new(
+                        txn_acct_index as IndexOfAccount,
+                        txn_acct_index as IndexOfAccount,
+                        txn_acct_index as IndexOfAccount,
+                        account_info.is_signer.unwrap_or(false),
+                        account_info.is_writable.unwrap_or(false),
+                    )
                 })
                 .collect();
             input.instruction_data

+ 56 - 60
program-runtime/src/invoke_context.rs

@@ -357,8 +357,10 @@ impl<'a> InvokeContext<'a> {
                 let instruction_account = deduplicated_instruction_accounts
                     .get_mut(duplicate_index)
                     .ok_or(InstructionError::NotEnoughAccountKeys)?;
-                instruction_account.is_signer |= account_meta.is_signer;
-                instruction_account.is_writable |= account_meta.is_writable;
+                instruction_account
+                    .set_is_signer(instruction_account.is_signer() || account_meta.is_signer);
+                instruction_account
+                    .set_is_writable(instruction_account.is_writable() || account_meta.is_writable);
             } else {
                 let index_in_caller = instruction_context
                     .find_index_of_instruction_account(
@@ -374,13 +376,13 @@ impl<'a> InvokeContext<'a> {
                         InstructionError::MissingAccount
                     })?;
                 duplicate_indicies.push(deduplicated_instruction_accounts.len());
-                deduplicated_instruction_accounts.push(InstructionAccount {
+                deduplicated_instruction_accounts.push(InstructionAccount::new(
                     index_in_transaction,
                     index_in_caller,
-                    index_in_callee: instruction_account_index as IndexOfAccount,
-                    is_signer: account_meta.is_signer,
-                    is_writable: account_meta.is_writable,
-                });
+                    instruction_account_index as IndexOfAccount,
+                    account_meta.is_signer,
+                    account_meta.is_writable,
+                ));
             }
         }
         for instruction_account in deduplicated_instruction_accounts.iter() {
@@ -390,7 +392,7 @@ impl<'a> InvokeContext<'a> {
             )?;
 
             // Readonly in caller cannot become writable in callee
-            if instruction_account.is_writable && !borrowed_account.is_writable() {
+            if instruction_account.is_writable() && !borrowed_account.is_writable() {
                 ic_msg!(
                     self,
                     "{}'s writable privilege escalated",
@@ -401,7 +403,7 @@ impl<'a> InvokeContext<'a> {
 
             // To be signed in the callee,
             // it must be either signed in the caller or by the program
-            if instruction_account.is_signer
+            if instruction_account.is_signer()
                 && !(borrowed_account.is_signer() || signers.contains(borrowed_account.get_key()))
             {
                 ic_msg!(
@@ -838,13 +840,13 @@ pub fn mock_process_instruction_with_feature_set<
                 instruction_account.index_in_transaction == index_in_transaction
             })
             .unwrap_or(instruction_account_index) as IndexOfAccount;
-        instruction_accounts.push(InstructionAccount {
+        instruction_accounts.push(InstructionAccount::new(
+            index_in_transaction,
             index_in_transaction,
-            index_in_caller: index_in_transaction,
             index_in_callee,
-            is_signer: account_meta.is_signer,
-            is_writable: account_meta.is_writable,
-        });
+            account_meta.is_signer,
+            account_meta.is_writable,
+        ));
     }
     if program_indices.is_empty() {
         program_indices.insert(0, transaction_accounts.len() as IndexOfAccount);
@@ -959,12 +961,14 @@ mod tests {
             let instruction_data = instruction_context.get_instruction_data();
             let program_id = instruction_context.get_last_program_key(transaction_context)?;
             let instruction_accounts = (0..4)
-                .map(|instruction_account_index| InstructionAccount {
-                    index_in_transaction: instruction_account_index,
-                    index_in_caller: instruction_account_index,
-                    index_in_callee: instruction_account_index,
-                    is_signer: false,
-                    is_writable: false,
+                .map(|instruction_account_index| {
+                    InstructionAccount::new(
+                        instruction_account_index,
+                        instruction_account_index,
+                        instruction_account_index,
+                        false,
+                        false,
+                    )
                 })
                 .collect::<Vec<_>>();
             assert_eq!(
@@ -1064,26 +1068,26 @@ mod tests {
                 solana_pubkey::new_rand(),
                 AccountSharedData::new(index as u64, 1, invoke_stack.get(index).unwrap()),
             ));
-            instruction_accounts.push(InstructionAccount {
-                index_in_transaction: index as IndexOfAccount,
-                index_in_caller: index as IndexOfAccount,
-                index_in_callee: instruction_accounts.len() as IndexOfAccount,
-                is_signer: false,
-                is_writable: true,
-            });
+            instruction_accounts.push(InstructionAccount::new(
+                index as IndexOfAccount,
+                index as IndexOfAccount,
+                instruction_accounts.len() as IndexOfAccount,
+                false,
+                true,
+            ));
         }
         for (index, program_id) in invoke_stack.iter().enumerate() {
             transaction_accounts.push((
                 *program_id,
                 AccountSharedData::new(1, 1, &solana_pubkey::Pubkey::default()),
             ));
-            instruction_accounts.push(InstructionAccount {
-                index_in_transaction: index as IndexOfAccount,
-                index_in_caller: index as IndexOfAccount,
-                index_in_callee: index as IndexOfAccount,
-                is_signer: false,
-                is_writable: false,
-            });
+            instruction_accounts.push(InstructionAccount::new(
+                index as IndexOfAccount,
+                index as IndexOfAccount,
+                index as IndexOfAccount,
+                false,
+                false,
+            ));
         }
         with_mock_invoke_context!(invoke_context, transaction_context, transaction_accounts);
 
@@ -1154,12 +1158,14 @@ mod tests {
             AccountMeta::new_readonly(transaction_accounts.get(2).unwrap().0, false),
         ];
         let instruction_accounts = (0..4)
-            .map(|instruction_account_index| InstructionAccount {
-                index_in_transaction: instruction_account_index,
-                index_in_caller: instruction_account_index,
-                index_in_callee: instruction_account_index,
-                is_signer: false,
-                is_writable: instruction_account_index < 2,
+            .map(|instruction_account_index| {
+                InstructionAccount::new(
+                    instruction_account_index,
+                    instruction_account_index,
+                    instruction_account_index,
+                    false,
+                    instruction_account_index < 2,
+                )
             })
             .collect::<Vec<_>>();
         with_mock_invoke_context!(invoke_context, transaction_context, transaction_accounts);
@@ -1210,12 +1216,14 @@ mod tests {
             AccountMeta::new_readonly(transaction_accounts.get(2).unwrap().0, false),
         ];
         let instruction_accounts = (0..4)
-            .map(|instruction_account_index| InstructionAccount {
-                index_in_transaction: instruction_account_index,
-                index_in_caller: instruction_account_index,
-                index_in_callee: instruction_account_index,
-                is_signer: false,
-                is_writable: instruction_account_index < 2,
+            .map(|instruction_account_index| {
+                InstructionAccount::new(
+                    instruction_account_index,
+                    instruction_account_index,
+                    instruction_account_index,
+                    false,
+                    instruction_account_index < 2,
+                )
             })
             .collect::<Vec<_>>();
         with_mock_invoke_context!(invoke_context, transaction_context, transaction_accounts);
@@ -1307,20 +1315,8 @@ mod tests {
             (program_key, program_account),
         ];
         let instruction_accounts = [
-            InstructionAccount {
-                index_in_transaction: 0,
-                index_in_caller: 0,
-                index_in_callee: 0,
-                is_signer: false,
-                is_writable: true,
-            },
-            InstructionAccount {
-                index_in_transaction: 1,
-                index_in_caller: 1,
-                index_in_callee: 1,
-                is_signer: false,
-                is_writable: false,
-            },
+            InstructionAccount::new(0, 0, 0, false, true),
+            InstructionAccount::new(1, 1, 1, false, false),
         ];
         with_mock_invoke_context!(invoke_context, transaction_context, transaction_accounts);
         let mut program_cache_for_tx_batch = ProgramCacheForTxBatch::default();

+ 7 - 7
program-runtime/src/serialization.rs

@@ -628,13 +628,13 @@ mod tests {
                     .iter()
                     .position(|account_index| account_index == index_in_transaction)
                     .unwrap_or(index_in_instruction);
-                InstructionAccount {
-                    index_in_transaction: *index_in_transaction,
-                    index_in_caller: *index_in_transaction,
-                    index_in_callee: index_in_callee as IndexOfAccount,
-                    is_signer: false,
-                    is_writable: is_writable(index_in_instruction),
-                }
+                InstructionAccount::new(
+                    *index_in_transaction,
+                    *index_in_transaction,
+                    index_in_callee as IndexOfAccount,
+                    false,
+                    is_writable(index_in_instruction),
+                )
             })
             .collect()
     }

+ 1 - 1
program-test/src/lib.rs

@@ -324,7 +324,7 @@ impl solana_sysvar::program_stubs::SyscallStubs for SyscallStubs {
                     .set_owner(account_info.owner.as_ref())
                     .unwrap();
             }
-            if instruction_account.is_writable {
+            if instruction_account.is_writable() {
                 account_indices.push((instruction_account.index_in_caller, account_info_index));
             }
         }

+ 5 - 5
programs/bpf_loader/benches/serialization.rs

@@ -93,13 +93,13 @@ fn create_inputs(owner: Pubkey, num_instruction_accounts: usize) -> TransactionC
             .iter()
             .position(|account| account.index_in_transaction == index_in_transaction)
             .unwrap_or(instruction_account_index) as IndexOfAccount;
-        instruction_accounts.push(InstructionAccount {
-            index_in_caller: instruction_account_index as IndexOfAccount,
+        instruction_accounts.push(InstructionAccount::new(
+            instruction_account_index as IndexOfAccount,
             index_in_transaction,
             index_in_callee,
-            is_signer: false,
-            is_writable: instruction_account_index >= 4,
-        });
+            false,
+            instruction_account_index >= 4,
+        ));
     }
 
     let mut transaction_context =

+ 13 - 25
programs/bpf_loader/src/syscalls/cpi.rs

@@ -875,8 +875,8 @@ where
             accounts.push(TranslatedAccount {
                 index_in_caller: instruction_account.index_in_caller,
                 caller_account,
-                update_caller_account_region: instruction_account.is_writable || update_caller,
-                update_caller_account_info: instruction_account.is_writable,
+                update_caller_account_region: instruction_account.is_writable() || update_caller,
+                update_caller_account_info: instruction_account.is_writable(),
             });
         } else {
             ic_msg!(
@@ -1324,15 +1324,15 @@ mod tests {
             let instruction_accounts = $instruction_accounts
                 .iter()
                 .enumerate()
-                .map(
-                    |(index_in_callee, index_in_transaction)| InstructionAccount {
-                        index_in_transaction: *index_in_transaction as IndexOfAccount,
-                        index_in_caller: *index_in_transaction as IndexOfAccount,
-                        index_in_callee: index_in_callee as IndexOfAccount,
-                        is_signer: false,
-                        is_writable: $transaction_accounts[*index_in_transaction as usize].2,
-                    },
-                )
+                .map(|(index_in_callee, index_in_transaction)| {
+                    InstructionAccount::new(
+                        *index_in_transaction as IndexOfAccount,
+                        *index_in_transaction as IndexOfAccount,
+                        index_in_callee as IndexOfAccount,
+                        false,
+                        $transaction_accounts[*index_in_transaction as usize].2,
+                    )
+                })
                 .collect::<Vec<_>>();
             let transaction_accounts = $transaction_accounts
                 .into_iter()
@@ -1879,20 +1879,8 @@ mod tests {
 
         let accounts = SyscallInvokeSignedRust::translate_accounts(
             &[
-                InstructionAccount {
-                    index_in_transaction: 1,
-                    index_in_caller: 0,
-                    index_in_callee: 0,
-                    is_signer: false,
-                    is_writable: true,
-                },
-                InstructionAccount {
-                    index_in_transaction: 1,
-                    index_in_caller: 0,
-                    index_in_callee: 0,
-                    is_signer: false,
-                    is_writable: true,
-                },
+                InstructionAccount::new(1, 0, 0, false, true),
+                InstructionAccount::new(1, 0, 0, false, true),
             ],
             vm_addr,
             1,

+ 7 - 7
programs/bpf_loader/src/syscalls/mod.rs

@@ -4438,13 +4438,13 @@ mod tests {
                     .transaction_context
                     .get_instruction_context_stack_height()
             {
-                let instruction_accounts = [InstructionAccount {
-                    index_in_transaction: index_in_trace.saturating_add(1) as IndexOfAccount,
-                    index_in_caller: 0, // This is incorrect / inconsistent but not required
-                    index_in_callee: 0,
-                    is_signer: false,
-                    is_writable: false,
-                }];
+                let instruction_accounts = [InstructionAccount::new(
+                    index_in_trace.saturating_add(1) as IndexOfAccount,
+                    0, // This is incorrect / inconsistent but not required
+                    0,
+                    false,
+                    false,
+                )];
                 invoke_context
                     .transaction_context
                     .get_next_instruction_context()

+ 1 - 7
programs/sbf/benches/bpf_loader.rs

@@ -60,13 +60,7 @@ macro_rules! with_mock_invoke_context {
                 AccountSharedData::new(2, $account_size, &program_key),
             ),
         ];
-        let instruction_accounts = vec![InstructionAccount {
-            index_in_transaction: 2,
-            index_in_caller: 2,
-            index_in_callee: 0,
-            is_signer: false,
-            is_writable: true,
-        }];
+        let instruction_accounts = vec![InstructionAccount::new(2, 2, 0, false, true)];
         solana_program_runtime::with_mock_invoke_context!(
             $invoke_context,
             transaction_context,

+ 2 - 14
programs/system/src/system_instruction.rs

@@ -294,20 +294,8 @@ mod test {
                 (system_program::id(), AccountSharedData::default()),
             ];
             let $instruction_accounts = vec![
-                InstructionAccount {
-                    index_in_transaction: 0,
-                    index_in_caller: 0,
-                    index_in_callee: 0,
-                    is_signer: true,
-                    is_writable: true,
-                },
-                InstructionAccount {
-                    index_in_transaction: 1,
-                    index_in_caller: 1,
-                    index_in_callee: 1,
-                    is_signer: false,
-                    is_writable: true,
-                },
+                InstructionAccount::new(0, 0, 0, true, true),
+                InstructionAccount::new(1, 1, 1, false, true),
             ];
             with_mock_invoke_context!($invoke_context, transaction_context, transaction_accounts);
         };

+ 2 - 22
programs/vote/src/vote_state/mod.rs

@@ -1158,17 +1158,7 @@ mod tests {
             0,
         );
         let mut instruction_context = InstructionContext::default();
-        instruction_context.configure(
-            &[0],
-            &[InstructionAccount {
-                index_in_transaction: 1,
-                index_in_caller: 1,
-                index_in_callee: 0,
-                is_signer: false,
-                is_writable: true,
-            }],
-            &[],
-        );
+        instruction_context.configure(&[0], &[InstructionAccount::new(1, 1, 0, false, true)], &[]);
 
         // Get the BorrowedAccount from the InstructionContext which is what is used to manipulate and inspect account
         // state
@@ -1313,17 +1303,7 @@ mod tests {
             0,
         );
         let mut instruction_context = InstructionContext::default();
-        instruction_context.configure(
-            &[0],
-            &[InstructionAccount {
-                index_in_transaction: 1,
-                index_in_caller: 1,
-                index_in_callee: 0,
-                is_signer: false,
-                is_writable: true,
-            }],
-            &[],
-        );
+        instruction_context.configure(&[0], &[InstructionAccount::new(1, 1, 0, false, true)], &[]);
 
         // Get the BorrowedAccount from the InstructionContext which is what is used to manipulate and inspect account
         // state

+ 6 - 6
svm/src/message_processor.rs

@@ -38,13 +38,13 @@ pub(crate) fn process_message(
                 .unwrap_or(instruction_account_index)
                 as IndexOfAccount;
             let index_in_transaction = *index_in_transaction as usize;
-            instruction_accounts.push(InstructionAccount {
-                index_in_transaction: index_in_transaction as IndexOfAccount,
-                index_in_caller: index_in_transaction as IndexOfAccount,
+            instruction_accounts.push(InstructionAccount::new(
+                index_in_transaction as IndexOfAccount,
+                index_in_transaction as IndexOfAccount,
                 index_in_callee,
-                is_signer: message.is_signer(index_in_transaction),
-                is_writable: message.is_writable(index_in_transaction),
-            });
+                message.is_signer(index_in_transaction),
+                message.is_writable(index_in_transaction),
+            ));
         }
 
         let mut compute_units_consumed = 0;

+ 7 - 7
svm/tests/conformance.rs

@@ -383,13 +383,13 @@ fn execute_fixture_as_instr(
             .position(|idx| *idx == *index_txn)
             .unwrap_or(instruction_acct_idx);
 
-        instruction_accounts.push(InstructionAccount {
-            index_in_transaction: *index_txn as IndexOfAccount,
-            index_in_caller: *index_txn as IndexOfAccount,
-            index_in_callee: index_in_callee as IndexOfAccount,
-            is_signer: sanitized_message.is_signer(*index_txn as usize),
-            is_writable: sanitized_message.is_writable(*index_txn as usize),
-        });
+        instruction_accounts.push(InstructionAccount::new(
+            *index_txn as IndexOfAccount,
+            *index_txn as IndexOfAccount,
+            index_in_callee as IndexOfAccount,
+            sanitized_message.is_signer(*index_txn as usize),
+            sanitized_message.is_writable(*index_txn as usize),
+        ));
     }
 
     let mut compute_units_consumed = 0u64;

+ 40 - 5
transaction-context/src/lib.rs

@@ -55,6 +55,7 @@ pub type IndexOfAccount = u16;
 /// Contains account meta data which varies between instruction.
 ///
 /// It also contains indices to other structures for faster lookup.
+#[repr(C)]
 #[derive(Clone, Debug, Eq, PartialEq)]
 pub struct InstructionAccount {
     /// Points to the account and its key in the `TransactionContext`
@@ -68,9 +69,43 @@ pub struct InstructionAccount {
     /// This excludes the program accounts.
     pub index_in_callee: IndexOfAccount,
     /// Is this account supposed to sign
-    pub is_signer: bool,
+    is_signer: u8,
     /// Is this account allowed to become writable
-    pub is_writable: bool,
+    is_writable: u8,
+}
+
+impl InstructionAccount {
+    pub fn new(
+        index_in_transaction: IndexOfAccount,
+        index_in_caller: IndexOfAccount,
+        index_in_callee: IndexOfAccount,
+        is_signer: bool,
+        is_writable: bool,
+    ) -> InstructionAccount {
+        InstructionAccount {
+            index_in_transaction,
+            index_in_caller,
+            index_in_callee,
+            is_signer: is_signer as u8,
+            is_writable: is_writable as u8,
+        }
+    }
+
+    pub fn is_signer(&self) -> bool {
+        self.is_signer != 0
+    }
+
+    pub fn is_writable(&self) -> bool {
+        self.is_writable != 0
+    }
+
+    pub fn set_is_signer(&mut self, value: bool) {
+        self.is_signer = value as u8;
+    }
+
+    pub fn set_is_writable(&mut self, value: bool) {
+        self.is_writable = value as u8;
+    }
 }
 
 /// An account key and the matching account
@@ -797,7 +832,7 @@ impl InstructionContext {
             .instruction_accounts
             .get(instruction_account_index as usize)
             .ok_or(InstructionError::MissingAccount)?
-            .is_signer)
+            .is_signer())
     }
 
     /// Returns whether an instruction account is writable
@@ -809,7 +844,7 @@ impl InstructionContext {
             .instruction_accounts
             .get(instruction_account_index as usize)
             .ok_or(InstructionError::MissingAccount)?
-            .is_writable)
+            .is_writable())
     }
 
     /// Calculates the set of all keys of signer instruction accounts in this Instruction
@@ -819,7 +854,7 @@ impl InstructionContext {
     ) -> Result<HashSet<Pubkey>, InstructionError> {
         let mut result = HashSet::new();
         for instruction_account in self.instruction_accounts.iter() {
-            if instruction_account.is_signer {
+            if instruction_account.is_signer() {
                 result.insert(
                     *transaction_context
                         .get_key_of_account_at_index(instruction_account.index_in_transaction)?,