Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactoring: Unify account_deps and accounts #17898

Merged
merged 11 commits into from
Jul 5, 2021

Conversation

Lichtso
Copy link
Contributor

@Lichtso Lichtso commented Jun 11, 2021

Problem

account_deps travel along slightly different code paths than the "normal" accounts do.
This makes mapping accounts into the VM memory harder and should thus be resolved first.

Summary of Changes

  • Changes ThisInvokeContext::get_account() to use accounts instead of pre_accounts. So now, the pre_accounts are only used in MessageProcessor::verify_and_update() and MessageProcessor::verify().
  • Adds explicit keys to the accounts as tuples, the same way as it is with account_deps.
  • Appends the account_deps at the end of the accounts slice and use the message.account_keys.len() as border between both subslices / ranges.

Fixes #

@Lichtso Lichtso force-pushed the refactor/unify_account_deps branch 2 times, most recently from d4b350c to 06119f3 Compare June 11, 2021 20:13
@codecov
Copy link

codecov bot commented Jun 12, 2021

Codecov Report

Merging #17898 (b4701c3) into master (772f9d1) will increase coverage by 0.0%.
The diff coverage is 95.5%.

@@           Coverage Diff           @@
##           master   #17898   +/-   ##
=======================================
  Coverage    82.3%    82.3%           
=======================================
  Files         433      433           
  Lines      121157   121163    +6     
=======================================
+ Hits        99813    99827   +14     
+ Misses      21344    21336    -8     

@stale
Copy link

stale bot commented Jun 22, 2021

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the stale [bot only] Added to stale content; results in auto-close after a week. label Jun 22, 2021
@Lichtso Lichtso removed the stale [bot only] Added to stale content; results in auto-close after a week. label Jun 22, 2021
@Lichtso Lichtso force-pushed the refactor/unify_account_deps branch 2 times, most recently from b69a46d to 0964dc7 Compare June 23, 2021 18:54
@Lichtso Lichtso force-pushed the refactor/unify_account_deps branch from 3fd3ec0 to 0d35872 Compare June 29, 2021 10:08
@Lichtso Lichtso force-pushed the refactor/unify_account_deps branch from 038de3d to e46ed5b Compare July 1, 2021 11:40
@Lichtso Lichtso force-pushed the refactor/unify_account_deps branch from 736faba to d619984 Compare July 2, 2021 12:09
@Lichtso Lichtso merged commit 7462c27 into solana-labs:master Jul 5, 2021
@Lichtso Lichtso deleted the refactor/unify_account_deps branch July 5, 2021 11:49
@jstarry jstarry added the v1.7 label Jul 7, 2021
mergify bot pushed a commit that referenced this pull request Jul 7, 2021
* Changes ThisInvokeContext::get_account() to use accounts instead of pre_accounts.

* Adds explicit keys to accounts to make them symmetric to account_deps.

* Appends account_deps to accounts in transaction loading and removes account_deps everywhere else.

(cherry picked from commit 7462c27)

# Conflicts:
#	program-test/src/lib.rs
#	runtime/src/bank.rs
#	runtime/src/message_processor.rs
mergify bot added a commit that referenced this pull request Jul 7, 2021
* Refactoring: Unify account_deps and accounts (#17898)

* Changes ThisInvokeContext::get_account() to use accounts instead of pre_accounts.

* Adds explicit keys to accounts to make them symmetric to account_deps.

* Appends account_deps to accounts in transaction loading and removes account_deps everywhere else.

(cherry picked from commit 7462c27)

# Conflicts:
#	program-test/src/lib.rs
#	runtime/src/bank.rs
#	runtime/src/message_processor.rs

* fix conflicts

Co-authored-by: Alexander Meißner <[email protected]>
Co-authored-by: Justin Starry <[email protected]>
@brooksprumo brooksprumo mentioned this pull request Aug 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants