fix: reject duplicate accounts between static keys and ALT

[lgalabru]

Summary

• Detect when a V0 transaction has the same account in both static keys and Address Lookup Table (ALT) loaded addresses, and reject pre-execution with AccountLoadedTwice — matching Agave behavior
• Route AccountLoadedTwice errors through the SimulationFailure path to produce Agave-compatible JSON-RPC error format ({"err":"AccountLoadedTwice","logs":[],"unitsConsumed":0,...})
• Add integration test covering the duplicate account rejection

Context

When a V0 transaction references system_program (or any account) in both its static keys and an ALT, Agave rejects it pre-execution with AccountLoadedTwice (0 CU consumed). Surfpool was letting it through to execution, where it failed with MissingAccount (~13k CU consumed) — a different error, at a different stage, with different CU cost.

Root cause: get_pubkeys_from_message() concatenated static keys with ALT-loaded addresses without deduplication.

Test plan

• Unit tests pass (3 variants: no db, in-memory sqlite, on-disk sqlite)
• End-to-end Go reproducer passes (solana-anchorage-collab repo) — confirms Agave-compatible JSON-RPC error format
• Fix-path transaction (no duplicate accounts) still succeeds

[MicaiahReid]

Are we sure SimulationFailure is the correct error here? What if simulation is disabled? (skip_preflight = true)

[lgalabru]

enum ProcessTransactionResult {
    Success(TransactionMetadata),
    SimulationFailure(FailedTransactionMetadata),
    ExecutionFailure(FailedTransactionMetadata),
}

Not success, and the transaction is not executing at this point. We could introduce a new enum but it seems overkill.

[MicaiahReid]

Yeah I'm saying I think to better match with what a node would return, if skip_preflight = true we return an ExecutionFailure - because we're not simulating the node would (I believe) return with the same error but while actually executing the transaction. Vs if skip_preflight = false, on a real node you'd be simulating and you'd return a SimulationFailure

[lgalabru]

This check is pre-execution / pre-simulation, and since the transaction is not executed (0 CU), simulation seems to be the least worst to me.
Neither of these variant are semantically correct: with the approach you're suggesting (ExecutionFailure), if we use the simulation rpc endpoint and hit that code-path, we'd be returning an execution error.

I honestly don't think it's a big deal as this enum is not exposed anyway in the RPC response.

[MicaiahReid]

this enum is not exposed anyway in the RPC response

That's not true. The send_transaction RPC method recives this TransactionStatusEvent and reacts differently according to the variant. VerificationFailure has no data on the final error and just has a message. SimulationError returns a RpcSimulateTransactionResult on the data of the error. ExecutionFailure is completely ignored - it's processed by the VM and the user would need to fetch the transaction by signature to see the error.

So perhaps your point is shining light on that we're further deviating from how mainnet would actually handle - if you skip the simulation on mainnet do you get the same error contents ("Account loaded twice") but during execution? Why isn't LiteSVM returning the correct error in the first place.

This is feeling more like a bandaid rather than a fix -perhaps this should be at the LiteSVM level.

if we use the simulation rpc endpoint and hit that code-path
Simulation RPC doesn't use this code path, I don't think that's relevant

[MicaiahReid]

This check is pre-execution / pre-simulation

Agreed - what I'm saying is that on mainnet this check isn't pre-execution/pre-simulation - so how can we have surfpool either behave the same way or appear to behave the same way

[lgalabru]

on mainnet this check isn't pre-execution/pre-simulation

Wdym? it has nothing to do with mainnet / devnet, it's a client implementation detail.

With this patch, behavior Agave and Surfpool are identical - as explained in the description, the PR was built with a test vector Surfpool / Agave.

[MicaiahReid]

I just noticed this in the PR description:

Root cause: get_pubkeys_from_message() concatenated static keys with ALT-loaded addresses without deduplication.

Would fixing that root cause allow LiteSVM to return the correct error?

[lgalabru]

LiteSVM should not be involved at all.