Support Account Data for Seeds in Account Resolution

[buffalojoec]

This PR adds support for account data as a seed configuration for a PDA stored
within validation data.

A dynamic program-derived address may depend on one or more seeds that are
actually taken from the inner data of some account in the list of accounts
provided to the program.

Reviewing this PR

• Commit 1: Changes made to account resolution
• Commit 2: Re-working tests and docs
• Commit 3: Fixing the closure provided to a transfer hook test to return None for accounts that don't exist

[buffalojoec]

Finally circled back to this bad boy. It's hilarious (and painful) how simple this change appears despite the tribulations it went through.

In any case, it's worth mentioning that there are a few things I dislike about the design.

RPC Fetch Abuse

Because not every address is known at the time add_to_instruction is invoked, it's not possible to use the RPC's get_multiple_accounts method over get_account.

I don't think there's anything we can do about this, but we are going to have a problem if people need to resolve a large number of accounts off-chain.

I propose three solutions:

• Provide a fetch_account_data: bool switch (not the best).
• Insert a new TLV entry in front of the ExtraAccountMetaList with length 0 that simply uses a discriminator to declare whether or not the extra account meta configuration requires the fetching of account data.
• Leverage the TLV entry above to actually describe specifically which accounts must be fetched. For example, it could be a vector of u8 values describing each index of an account that must be fetched. We could also provide a helper for building ExtraAccountMetaList data that does this automatically based on seed configs (my favorite).

#[discriminator_hash_input("extra_account_metas:fetch_configs")]
pub struct FetchConfiigs {
    indices: Vec<u8>,
}

impl SplVariableLenPack for FetchConfigs {
    ...
}

[buffalojoec]

In conclusion, I think the points raised above should become issues and solutions can be implemented later, but as long as nobody presses on their RPC endpoint rate limits with their list of extra metas, we should be OK for merging this.

As it stands right now, the most restrictive is https://api.devnet.solana.com at 100 requests per second. I don't think anyone using transfer hook will near 100 accounts for a while. Once they do, we will integrate a more robust solution.

[joncinque]

Looks really great overall! Nothing big here, mostly nits, so this should be ready to go soon

[joncinque]

Really great work! Just one little question, but otherwise this is ready to go from my side

[buffalojoec]

Really great work! Just one little question, but otherwise this is ready to go from my side

Thanks Jon, appreciate the review & feedback.

I still have to address the Rust doc comment you had, which unfortunately may change a few of the future-based types.

Basically, I feel like RPC client is going to be one of the more popular off-chain provided "get account data" closures, so I'd better make sure it isn't a pain to package up for the resolution call.

That will be just one more commit. Also will rebase, which will (maybe?) fix audit 💪🏼

[joncinque]

I still have to address the Rust doc comment you had, which unfortunately may change a few of the future-based types.

Ah right, my bad, this is why I shouldn't do reviews past a certain hour

[joncinque]

Also will rebase, which will (maybe?) fix audit 💪🏼

And yeah we should be good

Pull request has been modified.

[buffalojoec]

@joncinque In my last commit "enable doctest", I set up a client with RPC very similar to the test client I use in state.rs. This configuration will work for anyone who uses ExtraAccountMetaList::add_to_instruction(..).

However, if they try to use a closure and just call an RPC client object directly, like below, the static lifetime requirements of futures will not allow you to pass an object and then return a future that depends on a reference to said object, even if you use move all over the place.

|address: Pubkey| {
   client
      .get_account(&address) // Temporary value
      .map_ok(|acct| Some(acct.data))
      .map_err(|e| Box::new(e) as AccountFetchError)
}

If the configuration I've written in the Rustdoc looks good to you, this is ready to roll.

[joncinque]

Looks great! Just one last micro-nit and then this is good to go. I couldn't come up with anything better than your custom client

Pull request has been modified.

[buffalojoec]

This should be good to go

[joncinque]

Really great work, thanks!