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

Fix ProgramTestContext::set_account not updating programs #34780

Conversation

andreisilviudragnea
Copy link

@andreisilviudragnea andreisilviudragnea commented Jan 14, 2024

Problem

ProgramTestContext::warp_to_slot ignores ProgramTestContext::set_account program updates.

Summary of Changes

This PR tries to fix this problem by using BankForks::insert_from_ledger (introduced in #31331) instead of BankForks::insert everywhere inside ProgramTestContext. From my investigations, ProgramTestContext::set_account was never able to update program accounts, not even before #31331.

I also added relevant unit tests which verify the bug fix. I did not know how to write them without using precompiled Solana programs. The 3 precompiled Solana programs are variations of the program from here.

Fixes #34728

@andreisilviudragnea
Copy link
Author

@pgarg66 @Lichtso I pinged you because you are the author and reviewer of #31331.

@andreisilviudragnea andreisilviudragnea force-pushed the fix-34728-program-test-update-program branch from f28d2ba to d941dfe Compare January 15, 2024 10:16
@andreisilviudragnea
Copy link
Author

@tiago18c Please have a look at this PR, updating program accounts never worked for me before.

@buffalojoec
Copy link
Contributor

@andreisilviudragnea I might be missing something here, but have you tried ProgramTest::add_program(..)? Or do you specifically need to add this program using the context post-warp?

I've run into a similar issue before when trying to add an upgradeable program using ProgramTestContext::add_account(..) to add both the program and program data accounts, where the program was stuck at LoadedProgramType::DelayVisibility in the program runtime.

ProgramTest::add_program(..) specifically adds it using one single program account under the BpfLoader, which seems to shake the delayed visibility issue.

@andreisilviudragnea
Copy link
Author

@buffalojoec I am trying to re-use the same ProgramTestContext with multiple versions of the same Solana program.

I want to be able to simulate Solana transactions on multiple versions of the same program as part of a Tokio web app. And it's not clear to me if ProgramTestContext can be cheaply created and destroyed for each transaction simulation request, so I want to re-use an existing ProgramTestContext where I update the program account to a new version.

@buffalojoec
Copy link
Contributor

@buffalojoec I am trying to re-use the same ProgramTestContext with multiple versions of the same Solana program.

I want to be able to simulate Solana transactions on multiple versions of the same program as part of a Tokio web app. And it's not clear to me if ProgramTestContext can be cheaply created and destroyed for each transaction simulation request, so I want to re-use an existing ProgramTestContext where I update the program account to a new version.

I would say unless you specifically need to test the program in state A, then do an upgrade, then test it in state B, all within the same blockstore, then you should use separate ProgramTestContext instances.

I can't advise exactly how cheap they are to spin up, but we do this with all of SPL. Here's some examples.

You can see in each test (here, here, and the rest), we're spinning up a new ProgramTestContext using this function here.

In your case, you'd just add a different program processor with add_program(..) on each ProgramTest initialization.

@Lichtso
Copy link
Contributor

Lichtso commented Jan 15, 2024

As explained in #34728 (comment), you have to use the deployment instruction like this test does:

fn test_program_sbf_upgrade() {

That ProgramTestContext::set_account() was ever able to upgrade a program was not intended and got removed in v1.16.

@Lichtso Lichtso closed this Jan 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community Community contribution need:merge-assist
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ProgramTestContext::set_account does not update programs
3 participants