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

Reloads deployments with environments.program_runtime_v1 #33412

Merged

Conversation

Lichtso
Copy link
Contributor

@Lichtso Lichtso commented Sep 26, 2023

Problem

Quoting @pgarg66:

In deploy_program macro we are creating a new instance of runtime environment:

let program_runtime_environment = create_program_runtime_environment_v1(

This means that the cached program will get pruned here:
Also, reject_deployment_of_broken_elfs is false when the environment is created in bank.rs while it is true when it is created in deploy_program(). This is to retain some old behavior for previously deployed program (so that bank can continue to load them), but reject the broken elf for any new program deployments.
For loader-v4 we don't have the above restriction (i.e. no old deployed programs which could have broken elfs). So, I think it makes sense to reuse the runtime environment instance from the cache, instead of creating a new one.

Summary of Changes

Instead of loading the deployed program with the special deployment environment and storing it in the cache:

  • First only parse the ELF, and verify it against the special deployment environment
  • But then reload and compile it (without another verification) against the current environment and store that in the cache

@Lichtso Lichtso requested review from dmakarov and pgarg66 September 26, 2023 17:32
@Lichtso Lichtso changed the title Reloads deployments with environments.program_runtime_v1. Reloads deployments with environments.program_runtime_v1 Sep 26, 2023
@pgarg66
Copy link
Contributor

pgarg66 commented Sep 26, 2023

LGTM, will approve once CI passes.

@dmakarov
Copy link
Contributor

LGTM

@codecov
Copy link

codecov bot commented Sep 26, 2023

Codecov Report

Merging #33412 (0c4ad81) into master (ddd0297) will decrease coverage by 0.1%.
The diff coverage is 50.0%.

@@            Coverage Diff            @@
##           master   #33412     +/-   ##
=========================================
- Coverage    81.9%    81.9%   -0.1%     
=========================================
  Files         798      798             
  Lines      217101   217109      +8     
=========================================
- Hits       178015   177983     -32     
- Misses      39086    39126     +40     

@Lichtso Lichtso merged commit 01c71e7 into solana-labs:master Sep 26, 2023
@Lichtso Lichtso deleted the fix/deploy_program_runtime_environment branch September 26, 2023 18:43
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.

3 participants