New crate: mollusk-svm-on-demand
#171
Conversation
buffalojoec
left a comment
buffalojoec
left a comment
There was a problem hiding this comment.
This is pretty nifty!
My biggest concern is that it reimplements a lot of the harness's API. I would rather figure out how we can get this to play nice with the AccountStore trait and then it can be supported by the generic framework. I left a comment about it.
Thanks for putting this up, though! Seems very useful.
| @@ -0,0 +1,20 @@ | |||
| [package] | |||
| name = "mollusk-svm-on-demand" | |||
There was a problem hiding this comment.
I think we need to bikeshed the name. 😅
This library tends to use more descriptive (sometimes verbose) naming conventions, so it's clear what a given crate does. "On demand" is pretty vague in my opinion.
What about something like one of these?
mollusk-account-store-on-demandmollusk-account-store-rpc-on-demandmollusk-account-store-rpc
| @@ -0,0 +1,20 @@ | |||
| [package] | |||
| name = "mollusk-svm-on-demand" | |||
| description = "Automatically fetch and use mainnet accounts with the Mollusk SVM harness." | |||
There was a problem hiding this comment.
Is it only mainnet-beta?
There was a problem hiding this comment.
Good catch, it's not. Fixing
| //! RPC account store for fetching accounts from Solana RPC endpoints. | ||
| //! | ||
| //! This module provides the `RpcAccountStore` type for automatically fetching | ||
| //! accounts from mainnet and managing them for use with Mollusk testing. |
There was a problem hiding this comment.
It doesn't appear the tool is limited to mainnet. Why should it be, anyway?
| self.fetch_accounts(&pubkeys.into_iter().collect::<Vec<_>>()) | ||
| .await?; |
There was a problem hiding this comment.
Since fetch_accounts is internal, you could just make it accept a pubkey iterator (impl Iterator<Item = Pubkey>). You're also deduping in the fetch_accounts as well, so no need for the hash map above this. Just pipe the mapped iterator over the instruction accounts directly into fetch_accounts!
| return Ok(()); | ||
| } | ||
|
|
||
| let accounts = self.client.get_multiple_accounts(&missing_pubkeys).await?; |
There was a problem hiding this comment.
Eh, I suppose you maybe do have to do one allocation, but perhaps you can filter by cache.contains_key and collect that into a vec, then pass to the RPC call?
| pub async fn process_instruction_with_context( | ||
| rpc_url: &str, | ||
| mut mollusk: Mollusk, | ||
| instruction: &Instruction, | ||
| accounts: &[(Pubkey, Account)], | ||
| ) -> Result<InstructionResult> { | ||
| let cache = RpcAccountStore::new(rpc_url) | ||
| .allow_missing_accounts() | ||
| .skip_program_validation() | ||
| .with_accounts(accounts) | ||
| .from_instruction(instruction) | ||
| .await? | ||
| .add_programs(&mut mollusk) | ||
| .await? | ||
| .cache; | ||
|
|
||
| let context = mollusk.with_context(cache); | ||
| Ok(context.process_instruction(instruction)) | ||
| } |
There was a problem hiding this comment.
Why not just let developers create an RpcAccountStore and then do this?
let store = RpcAccountStore::new(rpc_url)
.allow_missing_accounts()
.skip_program_validation();
let mollusk = Mollusk::default().with_context(&store);
mollusk.process_instruction(
&instruction,
&accounts,
);You just need to implement AccountStore for RpcAccountStore.
mollusk/harness/src/account_store.rs
Line 7 in fc77245
Is it because your fetcher is async? Can you use the blocking RPC client instead?
There was a problem hiding this comment.
Yes I maybe should have made it more clear but this is currently possible.
You just need to add an .await() at the end of the store implementation and you can then blast it in the normal harness
| /// This function fetches the program data accounts for all programs that are | ||
| /// stored in the cache and adds them to the Mollusk environment. | ||
| /// | ||
| /// Note: This is needed because mollusk-svm doesn't load the programs for CPIs directly from the accounts. |
There was a problem hiding this comment.
Hm, maybe it should (?)
Agree, I don't love that as well. Tbh all current methods works without having to reimplement the harness, I added that after just to make it easier to use for people that don't really know how to do so. But probably it's just more confusing and just midcurved the entire implementation. I'm down to nuke the reimplementation of the harness |
|
@L0STE what do you think about something like this? I just went with the blocking RPC client and implemented What you could do it configure it to either |
2 participants
This crate simplifies testing by fetching and storing mainnet accounts directly in the
AccountStore.Key Features
The new
RpcAccountStoreallows you to:with_accountsmethodfrom_instructionmethod to automatically dump all accounts referenced in an instructionallow_missing_accountsmethod, which stores them with default valuesadd_programsmethod, which fetches program data from the account store and loads it viamollusk.add_program_with_elf_and_loader(supports both loader V2 and V3)with_synced_slotfor oracles that require the current mainnet slot to function correctlyNew Harness
Added a new test harness variant with
*_with_contextmethods that implement all the above helpers. These methods support both strict and non-strict modes depending on how they're called, while maintaining the same interface as the original harness.