Skip to content
This repository was archived by the owner on Jan 22, 2025. It is now read-only.

Feature: TransactionContext, InstructionContext and BorrowedAccount#21706

Merged
Lichtso merged 3 commits intosolana-labs:masterfrom
Lichtso:feature/account_context
Dec 27, 2021
Merged

Feature: TransactionContext, InstructionContext and BorrowedAccount#21706
Lichtso merged 3 commits intosolana-labs:masterfrom
Lichtso:feature/account_context

Conversation

@Lichtso
Copy link
Copy Markdown
Contributor

@Lichtso Lichtso commented Dec 8, 2021

#21882 and #21927 are preparations for this PR to remove the Rc from Refcell<AccountSharedData> in the program-runtime because Rc can not be serialized.

Problem

  • KeyedAccount does not validate self.is_writable() before modification
  • As the name suggests, KeyedAccount works by using Pubkey instead of an index (in instruction and in transaction)
  • KeyedAccount and AccountInfo are two redundant implementations of nearly the same interface

Summary of Changes

As proposed for ABIv2 (#19191):

  • Adds TransactionContext to outsource the accounts, invoke_stack and return_data properties of InvokeContext
  • Adds InstructionContext which will replace invoke_context::StackFrame
  • Adds BorrowedAccount which will replace only KeyedAccount at first and later also AccountInfo
  • Redirects the usage of accounts in InvokeContext through TransactionContext.
  • Moves the types TransactionAccount and InstructionAccount into transaction_context.rs.

Replacement of other runtime structs and the encoding and the new BPF loader will come later.
This is only about the interfaces for the runtime and program tests for now, so that we can start migrating them.

Open issues / up to discussion:

  • BorrowedAccount is a RefMut, so the user won't be able to borrow the same account multiple times even if they don't want to modify any. But of course, the user can have multiple references to one BorrowedAccount and also wrap it in a RefCell. I don't think that introducing a BorrowedAccountReadonly is necessary.
  • Account reallocation interface: Copy the one from AccountInfo?

Fixes #

@Lichtso Lichtso added the work in progress This isn't quite right yet label Dec 8, 2021
@codecov
Copy link
Copy Markdown

codecov Bot commented Dec 8, 2021

Codecov Report

Merging #21706 (5de6b8c) into master (cc947ca) will decrease coverage by 0.0%.
The diff coverage is 74.7%.

@@            Coverage Diff            @@
##           master   #21706     +/-   ##
=========================================
- Coverage    81.2%    81.2%   -0.1%     
=========================================
  Files         519      520      +1     
  Lines      145734   145929    +195     
=========================================
+ Hits       118448   118534     +86     
- Misses      27286    27395    +109     

Comment thread sdk/src/account_context.rs Outdated
Comment thread sdk/src/account_context.rs Outdated
Comment thread sdk/src/account_context.rs Outdated
Comment thread sdk/src/account_context.rs Outdated
Comment thread sdk/src/account_context.rs Outdated
Comment thread sdk/src/account_context.rs Outdated
Comment thread sdk/src/account_context.rs Outdated
Comment thread sdk/src/lib.rs Outdated
Comment thread program-runtime/src/invoke_context.rs
@Lichtso Lichtso force-pushed the feature/account_context branch from 920c7eb to ea7ab2a Compare December 25, 2021 18:12
@Lichtso Lichtso force-pushed the feature/account_context branch from ea7ab2a to 5de6b8c Compare December 25, 2021 18:31
@Lichtso Lichtso merged commit a066466 into solana-labs:master Dec 27, 2021
@Lichtso Lichtso deleted the feature/account_context branch December 27, 2021 17:49
@Lichtso Lichtso removed the work in progress This isn't quite right yet label Jan 3, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants