solana: multi-tenant deployment

[kcsongor]

No description provided.

[johnsaigle]

@claude

[github-actions]

Claude finished @johnsaigle's task in 3m 35s —— View job

---

Review: PR #875 — Solana Multi-Tenant Deployment

• Read repository guidelines
• Gather context on key changes
• Review Solana program changes
• Review CLI changes
• Review SDK/TS changes
• Review test changes
• Post review summary

---

Overview

This is a well-architected PR that transforms the Solana NTT program from a singleton-per-program model (v3) into a multi-instance/multi-tenant architecture (v4). Each program can now host multiple independent NTT deployments, where each deployment is identified by a caller-provided Config keypair rather than the program ID.

Key architectural changes:

• Config account changes from a global PDA (seeds = [b"config"]) to a caller-provided keypair
• All PDAs are now scoped by config.key() as a seed prefix for cross-instance isolation
• On-the-wire manager identity changes from program ID → Config pubkey
• Ownership transfer is decoupled from BPF loader upgrade authority (pure data mutations)
• New OutboxItem.manager field binds outbox items to their source instance

The PR is ~15k lines with changes across 89 files. Here are my findings:

---

Solana Program — Positive Findings

Cross-instance isolation is thorough. Every critical PDA (outbox rate limit, inbox rate limit, peers, registered transceivers, token authority, session authority, emitter, inbox items, LUT) correctly incorporates config.key().as_ref() in its seeds. I verified this across all instruction files.

OutboxItem binding is correct. The new outbox_item.manager == config.key() constraint in both mark_outbox_item_as_released.rs:23 and release_outbound.rs:22 prevents transceivers from one instance from releasing outbox items belonging to another — this is critical for multi-instance security.

Redeem properly validates the recipient manager identity against config.key() at redeem.rs:43, preventing cross-instance message replay.

Ownership transfer simplification is clean. Removing the BPF loader CPI (~90 lines) and replacing it with a pure data mutation in transfer_ownership.rs is a significant reduction in complexity and attack surface.

---

Issues & Suggestions

1. set-mint-authority.ts — Missing instance awareness on undeployed v4 path (Medium)

cli/src/commands/set-mint-authority.ts:152:

const tokenAuthority = NTT.pdas(managerKey).tokenAuthority();

When solanaNtt is undefined (undeployed program path, major === -1), and the user intends to deploy a v4 program, NTT.pdas(managerKey) computes the v3 token authority PDA (no instance scope). This would be incorrect for v4 deployments — the authority should be NTT.pdas(managerKey, instancePubkey).tokenAuthority().

The deployed path correctly uses solanaNtt.pdas (which has the instance built in), but the undeployed path doesn't accept an --instance flag. Consider either:

• Adding --instance to set-mint-authority for undeployed v4 programs, or
• Documenting that this command's --manager/--token mode is v3-only

Fix this →

2. Silent error swallowing in addSolanaInstance (Low-Medium)

cli/src/solana/deploy.ts:677-688:

try {
  await signSendWait(ch, initTxs, signer.signer);
} catch (e: any) {
  console.error(e.logs);
}

Both the initialize and transceiver registration steps catch errors and only log e.logs. If e is not a Solana SendTransactionError, .logs will be undefined, and the function returns "success" with a partially initialized instance. The same pattern exists in the legacy deploySvm (lines 517-528) — but introducing it in new code is an opportunity to do better.

Consider at minimum logging the full error message:

} catch (e: any) {
  console.error("Initialize failed:", e.logs ?? e.message ?? e);
}

Or better: distinguish between recoverable failures (e.g., LUT initialization when wormhole bridge isn't loaded) and non-recoverable ones (initialization itself failing).

Fix this →

3. Instance keypair written to relative path (Low)

cli/src/solana/deploy.ts:573:

const generatedPath = `${ch.chain}-instance.json`;

When no --instance-key is provided, the generated keypair is written to the current working directory with a name like Solana-instance.json. This is fragile if the CLI is invoked from different directories. Consider writing to the deployment config directory instead, or at least using an absolute path in the log message.

4. OutboxRateLimit comment is now inaccurate (Nit)

solana/programs/example-native-token-transfers/src/queue/outbox.rs:54-55:

/// Global rate limit for all outbound transfers to all chains.
/// NOTE: only one of this account can exist, so we don't need to check the PDA.

In v4, this is no longer true — each instance has its own outbox rate limit (scoped by config.key()). Multiple such accounts can exist. The comment should be updated.

Fix this →

5. Consider documenting the migration path more explicitly (Suggestion)

The upgradeBarriers.ts correctly blocks v3→v4 in-place upgrades with a helpful message. However, the operational steps for migrating an existing v3 deployment to v4 (fresh deploy, re-peer, migrate liquidity) might warrant a brief note in the barrier's reason string or a linked doc.

---

Tests Assessment

The new test coverage is solid:

• multi_instance.test.ts — Tests two independent instances under the same program, verifies PDA isolation, state isolation (pausing one doesn't affect the other), and cross-instance VAA replay protection. This is the most important new test.
• upgrade_authority_decoupling.test.ts — Verifies that ownership transfer no longer modifies the BPF loader's upgrade authority.
• e2e-solana.test.ts — End-to-end CLI test covering full deployment lifecycle, --instance-of flow, v3→v4 upgrade rejection, and solana token-authority --instance command.

Minor test gaps:

• No explicit test for keypair mismatch at initialization (SDK validation exists but isn't tested)
• No test verifying per-instance rate limit independence under actual transfer load

---

Architecture Summary

Aspect	v3	v4
Instances per program	1	Multiple
Instance identity	Program ID	Config keypair
PDA scoping	Global	Per-instance (config.key())
Ownership	Tied to BPF upgrade authority	Independent
Manager identity (on-wire)	Program ID bytes	Config pubkey bytes
Ownership transfer	BPF loader CPI (~90 LOC)	Pure data mutation (~3 LOC)

Overall this is a well-executed architectural change with strong cross-instance isolation. The issues noted above are relatively minor — the most actionable one is the set-mint-authority instance awareness gap.

---

[johnsaigle]

Needs rebase

[johnsaigle]

Two potential issues that I'd appreciate clarification on. 🙏🏻

I have a few design questions as well -- maybe they would be best recorded as GitHub issues or else in a design document somewhere.

• Have you considered whether these changes a) are compatible with Fogo or b) how this change impacts SVM <-> SVM interactions? I haven't considered this in my review because I wanted to check whether it's worth spending time on first.

[johnsaigle]

My understanding is that the inbox_item should also be validated against the instance (config) key. Otherwise, it seems possible to pass an InboxItem on Instance A and pass it through the release flow on Instance B. The recipient encoded in the message would not be altered, but the funds would be minted/unlocked out of the wrong instance. (This relies on both instances using the same token mint also -- maybe that disqualifies this scenario? If so, we should document explicitly)

It looks like OutboxItem was updated so I think we just need something similar for the InboxItem.

Here's a test I was playing with -- if useful, we could modify the logic slightly to ensure that the inbox item can't be used on the wrong instance.

(test under the multi_instance.test.ts file)

  it("demonstrates cross-instance release_inbound_mint uses an inbox item from another instance", async () => {
    const vaaForB = makeInboundVaa(
      "xinst-release-bug",
      instanceB.publicKey,
      payer.publicKey
    );

    // Prepare a valid, approved InboxItem for instance B, but do not release it.
    await redeemWithoutRelease(nttB, vaaForB, sender, signer);

    const recipientA = spl.getAssociatedTokenAddressSync(
      testMintA.address,
      payer.publicKey,
      true,
      TOKEN_PROGRAM
    );
    await spl.getOrCreateAssociatedTokenAccount(
      $.connection,
      payer,
      testMintA.address,
      payer.publicKey,
      true,
      undefined,
      undefined,
      TOKEN_PROGRAM,
      spl.ASSOCIATED_TOKEN_PROGRAM_ID
    );
    await assert.tokenBalance($.connection, recipientA).equal(0);

    // This should be rejected: config/mint/token_authority/custody are from A,
    // while inbox_item is the approved PDA for B. The current program accepts it.
    const maliciousReleaseIx = await nttA.program.methods
      .releaseInboundMint({ revertOnDelay: false, revertWhenNotReady: false })
      .accounts({
        common: {
          payer: payer.publicKey,
          config: { config: nttA.pdas.configAccount() },
          inboxItem: nttB.pdas.inboxItemAccount(
            "Ethereum",
            vaaForB.payload.nttManagerPayload
          ),
          recipient: recipientA,
          mint: testMintA.address,
          tokenAuthority: nttA.pdas.tokenAuthority(),
          tokenProgram: TOKEN_PROGRAM,
          custody: await NTT.custodyAccountAddress(
            nttA.pdas,
            await nttA.getConfig()
          ),
        },
        multisigTokenAuthority: multisigTokenAuthorityA,
      })
      .instruction();

    await $.sendAndConfirm(maliciousReleaseIx, payer);

    // 25_000 at 8 decimals released to a 9-decimal mint becomes 250_000 units.
    await assert.tokenBalance($.connection, recipientA).equal(250_000);
  }); 

[kcsongor]

this is valid, added a check for this!

[johnsaigle]

I think we ought to add a check here such that the recipient_ntt_manager matches the instance key. Otherwise I think you can take a VAA intended for an Instance A and create the transceiver message for Instance B. That incorrectly creates the transceiver message

I believe the impact here is low-to-nil for a few reasons:

• The exist seeds include the emitter chain and ID as seeds
• Solana only allows one peer per Wormhole emitter chain
• The IDs increase monotically on the sender side

This should suffice to make the account resulting from the seeds truly unique. However if all of the above weren't true, it would be possible to e.g. relay messages sent from EVM peers intended for given SVM instances to the wrong SVM instance, and potentially front-run transceiver message accounts and DoS them via the init constraint.

Let me know if you agree with this read. IMO this is definitely worth a comment because understanding the security here relies on keeping a lot of NTT subtlety in your head.

[johnsaigle]

This should be applied to both files:

solana/programs/example-native-token-transfers/src/transceivers/wormhole/instructions/receive_message.rs
solana/programs/ntt-transceiver/src/wormhole/instructions/receive_message.rs

[johnsaigle]

There are no README changes in this PR. Maybe the design document could be included in this PR? Easy to do in a follow-up PR though.

[johnsaigle]

@claude Verify whether the TS files in the SDK are correctly deriving the token and mint authorities near e.g. cli/src/commands/set-mint-authority.ts:152.

This change makes it so the instance (config) key is part of the seeds. The relevant code is not obviously shown in the diff so when proving/disproving this claim, quote the relevant code directly and include line numbers.

[johnsaigle]

Lol Claude emoji reacted but didn't complete I guess?

[johnsaigle]

Further down in this file, the multisig is initialized with the token_authority based on the manager key. Should this include the instance key?

[mdulin2]

I think we need an additional seed check here.

Previously, when a transceiver could have only a single instance associated with it, this was safe. Now, I think a maliciously crafted transceiver_message from another deployment on the same transceiver could be used. The id, recipient manager, source manager, and destination chain are all controllable via a crafted VAA, since this data is just in the body.

[mdulin2]

MarkOutboxItemAsReleased already checks that outbox_item.manager == config.key(). Could we add this here just for a sanity check? It's in the standalone version already.

[mdulin2]

Could we add a seed derivation for this? It's already checked in the MarkOutboxItemAsReleased on the CPI. But, it'd be good to add a second layer as a sanity check; it's already done in the standalone version of this.