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

Feature - Loader built-in program v4 #30464

Merged
merged 3 commits into from
Mar 23, 2023

Conversation

Lichtso
Copy link
Contributor

@Lichtso Lichtso commented Feb 23, 2023

Problem

This is the bare bones prototype for of the loader for the program runtime v2.
It is going to be fleshed out over the course of many PRs yet to come.

Summary of Changes

Adds the prototype of the new loader, which will:

  • have no syscalls and no VM nesting
  • CPI will be replaced by dynamic loading (later)
  • run SBFv2 programs only
  • explicate the deployment status inside the account data and not use the is_executable account flag anymore (thus not require a proxy account like the upgradeable loader does)
  • support un-/re-/deployment with a cool-down, finalization and program authorities like the upgradeable loader does

New Program Management Workflow

Buffer accounts will no longer be special and instead become normal program accounts which are simply never deployed.
The use of buffer accounts will become optional, it is more expensive than not using them but also more reliable.
Explicit control of the program account size will be provided by the Truncate instruction.

Deployment

  • First, a Truncate instruction initializes the program account and allocates memory
  • Subsequent Write instructions upload the program code
  • Deploy instruction verifies and enables the program

Redeployment

  • Retract instruction disables the program
  • Optional Truncate instruction shrinks or grows the program data account if necessary
  • Write instructions change the data in the program account
  • Deploy instruction verifies and (re)enables the program

Redeployment with a buffer account

  • First, a Truncate instruction initializes a buffer account and allocates memory
  • Subsequent Write instructions upload the program code
  • Retract instruction disables the program
  • Deploy instruction verifies the buffer, replaces the program accounts data, closes the buffer account and (re)enables the program

Undeployment

  • Retract instruction disables the program
  • Truncate instruction closes the program account

Finalization

  • TransferAuthority instruction finalizes the program

@Lichtso Lichtso added the runtime Issues related to runtime, BPF, and LLVM label Feb 23, 2023
@Lichtso Lichtso requested a review from jackcmay February 23, 2023 11:42
@Lichtso Lichtso force-pushed the feature/loader-v3 branch 2 times, most recently from 6862312 to 9d96f81 Compare March 8, 2023 12:27
@dmakarov
Copy link
Contributor

dmakarov commented Mar 8, 2023

How to read this

have no syscalls, CPI and no VM nesting

is there CPI or is there no CPI?

In the absence of CPI how one on-chain program can invoke another on-chain program?
Even if you propose a special bytecode call instruction that would trigger loading a program from blockchain, supposedly without making a syscall, it's still a CPI, in the sense that the target of the call must be loaded from the blockchain database, etc.

@Lichtso
Copy link
Contributor Author

Lichtso commented Mar 8, 2023

I updated the PR description. Yes programs will be able to call other programs, but that won't be called CPI anymore and use a different mechanism (no more VM nesting).

@dmakarov
Copy link
Contributor

dmakarov commented Mar 8, 2023

I updated the PR description. Yes programs will be able to call other programs, but that won't be called CPI anymore and use a different mechanism (no more VM nesting).

Looks good. Thank you.

@Lichtso Lichtso force-pushed the feature/loader-v3 branch from 9d96f81 to a124743 Compare March 9, 2023 13:46
@codecov
Copy link

codecov bot commented Mar 9, 2023

Codecov Report

Merging #30464 (641a8a3) into master (0291e8c) will increase coverage by 0.0%.
The diff coverage is 97.6%.

@@           Coverage Diff            @@
##           master   #30464    +/-   ##
========================================
  Coverage    81.4%    81.5%            
========================================
  Files         723      726     +3     
  Lines      203812   204742   +930     
========================================
+ Hits       166064   166962   +898     
- Misses      37748    37780    +32     

@Lichtso Lichtso force-pushed the feature/loader-v3 branch from a124743 to 3075725 Compare March 9, 2023 17:29
.get_data()
.len()
.saturating_sub(LoaderV3State::program_data_offset());
if offset as usize > program_size {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to support out of order writes for the program data? Since, there are multiple transactions to write/update the program.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Random access writes are supported once the data is initialized.
We could lift that restriction and define a write beyond the end as automatically filling the gap with zeros.

&program,
authority_address,
)?;
if !state.is_deployed {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we check DEPLOYMENT_COOLDOWN_IN_SLOTS for the program before it can be retracted?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would prevent the user from upgrading a program in a single transaction.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was mostly viewing it by the comment here: https://github.com/solana-labs/solana/pull/30464/files#diff-122620648d15121d8a722c52f2dcf42e3587ad87b0c254162edc8f857d6d218fR9

/// Cooldown before a program can be un-/redeployed again
pub const DEPLOYMENT_COOLDOWN_IN_SLOTS: u64 = 750;

I am likely confused.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, you are right, I misunderstood the first comment. Yes, we should check it here as well, just not reset / restart the cooldown.

/// # Account references
/// 0. `[writable]` The program account to deploy.
/// 1. `[signer]` The authority of the program.
/// 2. `[signer]` Optional, an undeployed source program account to take data and lamports from.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

replace signer with source_program?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. It should be [writable] instead for the source_program.

let instruction_data = instruction_context.get_instruction_data();
let program_id = instruction_context.get_last_program_key(transaction_context)?;
if loader_v3::check_id(program_id) {
match limited_deserialize(instruction_data)? {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we refactor this into smaller functions?

@pgarg66
Copy link
Contributor

pgarg66 commented Mar 21, 2023

This is pretty cool. Looks good overall. There are some minor comments/questions.

@Lichtso Lichtso force-pushed the feature/loader-v3 branch from 3075725 to e26d2ff Compare March 22, 2023 16:55
@Lichtso Lichtso force-pushed the feature/loader-v3 branch from e26d2ff to 3f38812 Compare March 23, 2023 11:45
@Lichtso Lichtso force-pushed the feature/loader-v3 branch from 3f38812 to 641a8a3 Compare March 23, 2023 14:03
@Lichtso Lichtso merged commit c10f337 into solana-labs:master Mar 23, 2023
@Lichtso Lichtso deleted the feature/loader-v3 branch March 23, 2023 17:13
@Lichtso Lichtso changed the title Feature - Loader built-in program v3 Feature - Loader built-in program v4 Aug 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
runtime Issues related to runtime, BPF, and LLVM
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants