Load account from file

[losman0s]

Problem

Programs can be dumped to file, and seamlessly loaded from file at genesis into the test validator. There is no similar feature for accounts (only dumping data of an account is supported, and no loading). This would ease testing on localnet when several accounts are necessary to run tests (manual and automated both), and for repeatability purposes (cloning leads to changing states per test run).

Summary of Changes

• Add complete account dump to file (not only data)
• Add solana-test-validator option to load accounts from file

Notes

• Serialization/deserialization is done using the formats available in the --output option of solana-test-validator, i.e. json and json-compact
• Behavior is unchanged when dumping account to file without the --output option (only data gets written)

Fixes #20279

[codecov]

Codecov Report

Merging #21812 (3b0212f) into master (02fa135) will increase coverage by 0.0%.
The diff coverage is 0.0%.

❗ Current head 3b0212f differs from pull request most recent head 4fb7ae7. Consider uploading reports for the commit 4fb7ae7 to get more accurate results

@@           Coverage Diff            @@
##           master   #21812    +/-   ##
========================================
  Coverage    81.3%    81.3%            
========================================
  Files         516      516            
  Lines      144307   144064   -243     
========================================
- Hits       117323   117150   -173     
+ Misses      26984    26914    -70     

[mvines]

Thanks for this PR. We've wanted this feature for a while!

[mvines]

How about:

Suggested change

	pub fn add_accounts_with_path(&mut self, filenames: Vec<&str>) -> &mut Self {
	pub fn add_accounts_from_json_files(&mut self, filename: &[Pubkey]) -> &mut Self {

Pubkey as the type for filename, and we don't need to assume a Vec when any slice will do

[losman0s]

Slice instead of Vec makes sense, will do.
However can you elaborate about the suggestion to enforce that the file name be the account address string? I fear it might be unnecessarily constraining, particularly when the pubkey is naturally part of the file content (coming from solana account), and that the deserialization operation would already validate that this address string makes for a valid Pubkey.

[mvines]

It just feels like better typing to require the filename to be a Pubkey here to me, we don't actually accept any other value right? So we can push the validation out to the --account-from-json-file clap.rs argument handling, and know that internally the filename will always be a valid pubkey

[losman0s]

I'd argue that we don't really care about the name of the file, actually. A standard flow would look like:

1. dump mainnet account to file

solana account -u m \
  --output json-compact \
  --output-file SRM_token.json \
  SRMuApVNdxXokk5GT7XD5cUUgXMBCoAz2LHeuAoKWRt

2. start test validator with this account, and another one dumped beforehand for instance:

solana-test-validator --clone-from-file SRM_token USD_token

Am I missing something?

[mvines]

Oh, I was thinking we'd just use the filename as the address:

$ solana-test-validator --account-from-json-file SRMuApVNdxXokk5GT7XD5cUUgXMBCoAz2LHeuAoKWRt.json

but yeah maybe that's not so great, and it doesn't match what --bpf-program already does.

So I take it all back, something like this seems great (also looking to drop the "clone" naming from this argument):

$ solana-test-validator --account SRMuApVNdxXokk5GT7XD5cUUgXMBCoAz2LHeuAoKWRt SRM_token.json

[mvines]

I like it!

As a minor tweak, we could change the address to be an optional second argument to --account will simplify the arg parsing code. Slightly inconsistent with the --bpf-program argument though, but feels more natural if the address is also now embedded in SRM_token.json.

$ solana-test-validator --account SRM_token.json
        or
$ solana-test-validator --account SRM_token.json SRMuApVNdxXokk5GT7XD5cUUgXMBCoAz2LHeuAoKWRt 

[losman0s]

1. My bad, I was not clear enough. I was actually proposing to implement one, or the other. I just looked into implementing an optional pubkey arg like you suggest, and unless I am missing something it makes the parsing quite tricky, because option values are collected "flat", even across separate occurrences of a particular option.

Example:

solana-test-validator --account serum SRMuApVNdxXokk5GT7XD5cUUgXMBCoAz2LHeuAoKWRt uscd --account raydium

would be collected by clap as the following String array: [serum, SRMuApVNdxXokk5GT7XD5cUUgXMBCoAz2LHeuAoKWRt, usdc, raydium], not [[serum, SRMuApVNdxXokk5GT7XD5cUUgXMBCoAz2LHeuAoKWRt], [usdc], [raydium]].
The grouping would then need to be inferred from a is_pubkey check succeeding or not, or something similar.
Do you think this is worth it? Or would 1 of the options above sound good enough?

2. A detail I'd rather clarify, since I mislead you a bit in the examples above: the file argument is currently without the file extension, which is automatically appended to the argument provided. I went that way because only a json file is valid, but I'm open to challenge if you think that's weird.

[mvines]

Got it. This seems best then

$ solana-test-validator --account SRMuApVNdxXokk5GT7XD5cUUgXMBCoAz2LHeuAoKWRt SRM_token.json

Consistent with --bpf-program and allows the same SRM_token.json account data to be reused at different addresses without editing the file contents

[losman0s]

All comments addressed. CI under way. I actually changed the parsing so that the whole filename (including extension) is required. Didn't make much sense to not allow the user to add it (if they're using autocompletion for instance).

The only thing a bit annoying right now is that if you specify a file that does not exist in CWD or tests/fixtures (the paths explored by solana_program_test::find_file), the error log is in test-ledger/validator.log, and not in the CLI output like it is for a wrong pubkey.
I left it that way as it is a tradeoff to be able to get the files automatically found in tests/fixtures without having to supply the whole path. Not sure if it is worth it though, or if I should just require a straight valid path which could be validated before starting the validator, and would display the error in the command output.

[mvines]

Looks great, thanks. I pushed a tiny commit to update the arg name

Pull request has been modified.