Remove index_in_caller from InstructionAccount#7220
Remove index_in_caller from InstructionAccount#7220LucasSte merged 5 commits intoanza-xyz:masterfrom
index_in_caller from InstructionAccount#7220Conversation
|
The Firedancer team maintains a line-for-line reimplementation of the |
| } | ||
|
|
||
| /// Get the index of account in instruction from the index in transaction | ||
| pub fn get_index_of_account_in_instruction( |
There was a problem hiding this comment.
This reverse search is O(n) on the number of instruction accounts. It is called at most twice during a CPI. Once in prepare_next_instruction to verify account permissions, and once in translate_and_update_accounts. The number of accounts is low, so the performance impact should not be representative. We are also avoiding the pub key comparisons, which was the previous method we used to find the index_in_transaction.
I still need to remove index_in_calle from InstructionAccounts, and that gave me an idea. When we are building the InstructionAccounts vector we create the index_in_transaction <-> index_in_callee map. Instead of discarding this map, we could store it in InstructionContext to detect if an account is duplicated and retrieve the index_in_instruction from index_in_transaction.
Although it may waste some space, since not all the 256 entries will be occupied, lookups in a vector are faster than those in a hash map. Also, consider that it has only 256 bytes, so impact in memory should be low.
PS: That is for a next PR to avoid cluttering this one.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #7220 +/- ##
=======================================
Coverage 83.1% 83.1%
=======================================
Files 849 849
Lines 369954 369935 -19
=======================================
- Hits 307735 307728 -7
+ Misses 62219 62207 -12 🚀 New features to boost your workflow:
|
|
I split the changes in three commits for easier review. |
| } | ||
|
|
||
| /// Retrieves an instruction account using the index in transaction | ||
| pub fn try_borrow_instruction_account_with_transaction_index<'a, 'b: 'a>( |
There was a problem hiding this comment.
I would say we inline this at its three callsites. That way it is clear where the expensive get_index_of_account_in_instruction() happens.
| instruction_context: &'a InstructionContext, | ||
| index_in_transaction: IndexOfAccount, | ||
| index_in_instruction: IndexOfAccount, | ||
| // Program accounts are not part of the instruction_accounts vector |
3 participants
Problem
According to SIMD-0177, the struct
InstructionAccountis supposed to be shared with programs. It should not contain theindex_in_callermember, since it is irrelevant to programs and unused in ABIv2.That parameter is only used in CPI.
Summary of Changes
index_in_transactionfromBorrowedAccountto the meaning used inInstructionContextto avoid confusion.InstructionAccount.