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

litesvm does not enforce rent minimum balance for non-payer accounts #98

Open
OliverNChalk opened this issue Oct 11, 2024 · 6 comments
Open
Labels
bug Something isn't working

Comments

@OliverNChalk
Copy link
Contributor

Checking the source, it looks like only the payer account has its post TX rent balance checked. So the question is:

  • Do we want to build out the feature to check all accounts post execution.
  • Should this feature be optional or just hard-coded.
  • If optional, should it be on by default?

I imagine this was avoided for simplicity/dev experience. However, I've had a snag in that tests that should fail are now passing (i.e. im not properly pre-funding the accounts im allocating and litesvm isnt catching this for me).

I'd be down to take a first swing at such a PR if it's of interest.

@Aursen Aursen added the bug Something isn't working label Oct 16, 2024
@Aursen
Copy link
Collaborator

Aursen commented Oct 16, 2024

Hi, do you have a code snippet that normally wouldn't work?

@OliverNChalk
Copy link
Contributor Author

The opposite, I am able to intialize accounts without paying sufficient rent. This is usually fine but it makes testing deployment scripts impossible (because behavior is not 1to1 with mainnet).

@kevinheavey
Copy link
Collaborator

Hmm, I think this should be optional and on by default. When it's on, we should also make it impossible to add an account that has insufficient rent

m30m added a commit to pyth-network/per that referenced this issue Dec 6, 2024
m30m added a commit to pyth-network/per that referenced this issue Dec 6, 2024
* fix: check rent exemptions manually after the simulation

This is in progress on litesvm package:
LiteSVM/litesvm#98

* More profiling + better docs
@Aursen
Copy link
Collaborator

Aursen commented Dec 23, 2024

The opposite, I am able to intialize accounts without paying sufficient rent. This is usually fine but it makes testing deployment scripts impossible (because behavior is not 1to1 with mainnet).

How did you do? I cannot reproduce the behavior

@OliverNChalk
Copy link
Contributor Author

#[test]
fn repro() {
    let mut svm: Svm<HashMap<Pubkey, Account>> = Svm::new(HashMap::default());

    let payer = Keypair::new();
    svm.set(payer.pubkey(), Account { lamports: 10u64.pow(9), ..Default::default() });

    let target = Keypair::new();
    let allocate = system_instruction::allocate(&target.pubkey(), 100);

    assert!(svm
        .simulate_transaction(Transaction::new_signed_with_payer(
            &[allocate],
            Some(&payer.pubkey()),
            &[&payer, &target],
            svm.blockhash(),
        ))
        .is_err());
}

The Svm type is my wrapper around LiteSVM - if you can copy this into your workspace and just adjust the API slightly you should also see this assertion fail

@Aursen
Copy link
Collaborator

Aursen commented Dec 30, 2024

I will fix it in the 0.4.1 version of LiteSVM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants