Refactor - Simplify program accounts in transaction loading #29728
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Lichtso
force-pushed
the
refactor/simplify_program_accounts_in_transaction_loading
branch
from
e48aedd
to
57283cd
Compare
Lichtso
force-pushed
the
refactor/simplify_program_accounts_in_transaction_loading
branch
from
57283cd
to
95f3a2e
Compare
|
Can this PR be backported to 1.14? The function on top of this (to limit loaded data to a fixed size) will be backported, that'd bring it to 1.14, would you think so? |
tao-stones
reviewed
Jan 19, 2023
Lichtso
force-pushed
the
refactor/simplify_program_accounts_in_transaction_loading
branch
2 times, most recently
from
0c6c32e
to
cc6df59
Compare
tao-stones
previously approved these changes
Jan 24, 2023
Lichtso
force-pushed
the
refactor/simplify_program_accounts_in_transaction_loading
branch
from
cc6df59
to
6e67274
Compare
mergify
bot
dismissed
tao-stones’s stale review
Pull request has been modified.
Lichtso
deleted the
refactor/simplify_program_accounts_in_transaction_loading
branch
mergify bot
pushed a commit
that referenced
this pull request
Feb 1, 2023
* Refactors the "!validated_fee_payer" case from an "else" branch to an early "return". * Moves the early return upward. * Removes empty entries. * Adds account_found_and_dep_index. * cargo fmt. * Replaces call site of load_executable_accounts(). * Adjusts number of total loaded accounts in test_load_accounts_multiple_loaders(). * Removes test_accounts_account_not_found(). * Removes load_executable_accounts(). * Refactor back to built-in loader ownership chain loop. (cherry picked from commit aa2e348) # Conflicts: # runtime/src/accounts.rs
mergify bot
pushed a commit
that referenced
this pull request
Feb 1, 2023
* Refactors the "!validated_fee_payer" case from an "else" branch to an early "return". * Moves the early return upward. * Removes empty entries. * Adds account_found_and_dep_index. * cargo fmt. * Replaces call site of load_executable_accounts(). * Adjusts number of total loaded accounts in test_load_accounts_multiple_loaders(). * Removes test_accounts_account_not_found(). * Removes load_executable_accounts(). * Refactor back to built-in loader ownership chain loop. (cherry picked from commit aa2e348) # Conflicts: # runtime/src/accounts.rs
pgarg66
reviewed
Feb 18, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
Currently we skip the loading of program accounts (of top level instructions) by filling them with an empty entry and later loading them separately in
load_executable_accounts(). This not only causes redundant loading logic for the upgradeable loaders programdata special case, but also duplication of all program accounts, even multiple times if a program is used more than once.Summary of Changes
load_executable_accounts()and uses the normal account loading instead.test_accounts_account_not_found()as theProgramAccountNotFoundcase is now covered bytest_program_sbf_invoke_in_same_tx_as_deployment()(see Adds symmetric tests for all cases of un-/re-/deployment inside the same transaction #29725).test_load_accounts_multiple_loaders().