Skip to content

feat(amm): namespace all PDAs to host many AMM instances per deployment - #357

Open
0x-r4bbit wants to merge 1 commit into
mainfrom
feat/amm-namespaces
Open

feat(amm): namespace all PDAs to host many AMM instances per deployment#357
0x-r4bbit wants to merge 1 commit into
mainfrom
feat/amm-namespaces

Conversation

@0x-r4bbit

Copy link
Copy Markdown
Collaborator

A LEZ program can only be deployed once, so a single AMM deployment previously had one singleton config and one pool per token pair. Namespace the whole system so independent teams can run isolated AMMs under one deployment.

Only the two root PDAs carry the namespace; everything downstream inherits isolation via pool_id:

namespace = (owner, nonce)
config_id = PDA(amm, hash("CONFIG" || owner || nonce))
pool_id   = PDA(amm, config_id, sorted(token_a, token_b))
vault / lp_token / lp_lock / twap   (unchanged; derived from pool_id)

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Several core AMM operations (swap/add/remove/sync) do not yet explicitly bind pool.account_id to the passed config.account_id namespace, which can undermine instance isolation.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR updates the AMM program to support multiple isolated AMM instances under a single deployment by namespacing the root PDAs with (owner, nonce), and then deriving all per-pair pool PDAs under the instance’s config_id.

Changes:

  • Namespaced AMM config PDA derivation via (owner, nonce) and updated pool PDA derivation to include config_id.
  • Updated AMM instruction handlers (and chained-call PDA seeds) plus guest/IDL to reflect the new Initialize ABI.
  • Updated integration tests and FFI helpers to derive and operate on per-namespace pools/configs.
File summaries
File Description
programs/integration_tests/tests/amm.rs Adds multi-namespace integration coverage and updated PDA derivations
programs/amm/src/update_config.rs Removes singleton config-PDA assumption; gates by AMM ownership
programs/amm/src/tests.rs Updates unit fixtures to use namespaced config/pool PDAs
programs/amm/src/sync.rs Uses config_id in pool PDA seed for TWAP updates
programs/amm/src/swap.rs Uses config_id in pool PDA seed for TWAP updates
programs/amm/src/remove.rs Uses config_id in pool PDA seed for TWAP updates
programs/amm/src/new_definition.rs Derives/claims pool PDA under config namespace
programs/amm/src/initialize.rs Adds (owner, nonce) initialization + owner signature requirement
programs/amm/src/create_price_observations.rs Derives pool PDA under config namespace for oracle feeds
programs/amm/src/create_oracle_price_account.rs Derives pool PDA under config namespace for oracle price accounts
programs/amm/src/add.rs Uses config_id in pool PDA seed for TWAP updates
programs/amm/methods/guest/src/bin/amm.rs Updates guest Initialize signature to include owner+nonce
programs/amm/examples/amm_pdas.rs CLI example updated to accept owner and derive namespaced PDAs
programs/amm/core/src/lib.rs Implements namespaced config PDA + config-rooted pool PDA scheme
modules/amm/ffi/tests/public_api.rs Updates config-id API test to include owner/nonce
modules/amm/ffi/src/api/token_holdings.rs Adapts to load_config returning (id, config)
modules/amm/ffi/src/api/tests.rs Updates FFI test derivations for namespaced config/pool
modules/amm/ffi/src/api/swap.rs Makes pool_id derivation depend on passed config id
modules/amm/ffi/src/api/request.rs Extends request models (ConfigIdRequest, PoolIdRequest)
modules/amm/ffi/src/api/pair.rs Derives pool PDA under decoded config id
modules/amm/ffi/src/api/liquidity.rs Updates plan tests to use namespaced config/pool ids
modules/amm/ffi/src/api/context.rs Adapts to updated load_config return type
modules/amm/ffi/src/api/config.rs Implements namespaced config_id API + revised load_config contract
modules/amm/ffi/src/api/admin.rs Uses decoded config id for UpdateConfig planning
artifacts/amm-idl.json Updates IDL for Initialize(owner, nonce, ...)
Review details

Suppressed comments (1)

programs/amm/src/swap.rs:376

  • Same as swap_exact_input: swap should bind pool to the config namespace with an explicit compute_pool_pda(…, config.account_id, …) check. Without it, a caller can potentially operate on a pool from a different AMM instance by supplying any AMM-owned config that decodes (and pointing it at a permissive oracle).
    assert_eq!(
        config.account.program_owner, amm_program_id,
        "Swap exact output: AMM config account must be owned by the AMM Program"
    );
  • Files reviewed: 25/25 changed files
  • Comments generated: 5
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread programs/amm/src/add.rs
Comment on lines 34 to +38
// The program IDs are taken from the config account, not trusted from a caller-supplied
// holding. Validating the config PDA is also the Program's initialization gate.
assert_eq!(
config.account_id,
compute_config_pda(amm_program_id),
"Add liquidity: AMM config Account ID does not match PDA"
config.account.program_owner, amm_program_id,
"Add liquidity: AMM config account must be owned by the AMM Program"
Comment on lines 39 to 42
assert_eq!(
config.account_id,
compute_config_pda(amm_program_id),
"Remove liquidity: AMM config Account ID does not match PDA"
config.account.program_owner, amm_program_id,
"Remove liquidity: AMM config account must be owned by the AMM Program"
);
Comment thread programs/amm/src/swap.rs
Comment on lines 160 to 163
assert_eq!(
config.account_id,
compute_config_pda(amm_program_id),
"Swap exact input: AMM config Account ID does not match PDA"
config.account.program_owner, amm_program_id,
"Swap exact input: AMM config account must be owned by the AMM Program"
);
Comment thread programs/amm/src/sync.rs
Comment on lines 27 to 30
assert_eq!(
config.account_id,
compute_config_pda(amm_program_id),
"Sync reserves: AMM config Account ID does not match PDA"
config.account.program_owner, amm_program_id,
"Sync reserves: AMM config account must be owned by the AMM Program"
);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checking only that config is AMM-owned does not prove that the supplied pool was derived under that config. A caller can create config B with the real token program and a permissive oracle, then invoke SyncReserves with config B and pool A. The handler accepts B, mutates pool A, and the permissive oracle can ignore the mismatched pool authorization, allowing runtime to commit pool A's new reserves without updating A's canonical TWAP account. I reproduced this through the zkVM: pool A changed from (10, 10) to (20, 20) using config B. Please assert pool.account_id == compute_pool_pda(amm_program_id, config.account_id, token_a, token_b) before mutation in sync, add, remove, and both swap paths.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same issue in add.rs:38, remove.rs:42, and both swap handlers

Comment on lines 33 to 37
ctx: ProgramContext,
#[account(signer)]
owner: AccountWithMetadata,
#[account(init)]
config: AccountWithMetadata,
Comment thread programs/amm/src/sync.rs
Comment on lines 27 to 30
assert_eq!(
config.account_id,
compute_config_pda(amm_program_id),
"Sync reserves: AMM config Account ID does not match PDA"
config.account.program_owner, amm_program_id,
"Sync reserves: AMM config account must be owned by the AMM Program"
);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checking only that config is AMM-owned does not prove that the supplied pool was derived under that config. A caller can create config B with the real token program and a permissive oracle, then invoke SyncReserves with config B and pool A. The handler accepts B, mutates pool A, and the permissive oracle can ignore the mismatched pool authorization, allowing runtime to commit pool A's new reserves without updating A's canonical TWAP account. I reproduced this through the zkVM: pool A changed from (10, 10) to (20, 20) using config B. Please assert pool.account_id == compute_pool_pda(amm_program_id, config.account_id, token_a, token_b) before mutation in sync, add, remove, and both swap paths.

Comment thread programs/amm/src/sync.rs
Comment on lines 27 to 30
assert_eq!(
config.account_id,
compute_config_pda(amm_program_id),
"Sync reserves: AMM config Account ID does not match PDA"
config.account.program_owner, amm_program_id,
"Sync reserves: AMM config account must be owned by the AMM Program"
);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same issue in add.rs:38, remove.rs:42, and both swap handlers

#[serde(rename_all = "camelCase")]
pub struct ConfigIdRequest {
pub amm_program_id: String,
pub owner: String,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The existing C++ callers still send the old request shapes. AmmModuleImpl::readConfig() calls amm_config_id with only ammProgramId, so making owner required causes every config read to fail with ``invalid request JSON: missing field owner```. Likewise, swapExactInQuote()` and `swapExactOutQuote()` call `amm_pool_id` without the newly required `config`, so quotes fail before reading a pool. Pool creation, swaps, liquidity operations, and admin operations also use these calls. Please wire the selected instance's owner/nonce or config ID through the module and update both sets of JSON callers in this PR. The Rust tests construct the new request fields directly, so they do not exercise these production callers.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also lines 134–136

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is addressed in #358

};
let amm = parse_pid(amm_s);
let config = compute_config_pda(amm);
let owner = AccountId::from_str(owner_s).expect("owner must be base58");

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

apps/amm/tests/testnet/setup-amm-testnet.sh:421–422 still invokes this example as <amm_pid> <twap_pid> <defA> <defB>. The second argument now gets parsed as an owner account ID, but the script supplies eight comma-separated program-ID limbs, so setup panics at owner must be base58 after deploying programs and creating tokens. The later spel initialize call also omits the new owner account and nonce. Please add a dedicated namespace owner to the setup and pass the same owner/nonce to PDA derivation and initialization so the existing testnet setup remains usable.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is addressed in #358

A LEZ program can only be deployed once, so a single AMM deployment
previously had one singleton config and one pool per token pair. Namespace
the whole system so independent teams can run isolated AMMs under one
deployment.

Only the two root PDAs carry the namespace; everything downstream inherits
isolation via pool_id:

    namespace = (owner, nonce)
    config_id = PDA(amm, hash("CONFIG" || owner || nonce))
    pool_id   = PDA(amm, config_id, sorted(token_a, token_b))
    vault / lp_token / lp_lock / twap   (unchanged; derived from pool_id)

BREAKING CHANGES:

* Initialize gains an `owner` account and a `nonce` argument. `owner`
  must sign (this squat-proofs the namespace) and is now WRITABLE — the
  AMM claims it on first use, so it must be a fresh, dedicated account,
  not a reused wallet. `nonce` selects the instance under that owner
  (all-zero = the owner's default instance). There is no longer a single
  fixed config address.

* All AMM PDA addresses change. Both root PDAs are re-derived and every
  downstream account (vaults, LP token, LP lock, TWAP tick / price
  observations / oracle price) moves with the new pool_id. Any persisted
  or hardcoded AMM account addresses are invalid and must be re-derived.

* amm_core PDA helper signatures changed:
    compute_config_pda(amm)
      -> compute_config_pda(amm, owner, nonce)
    compute_pool_pda(amm, token_a, token_b)
      -> compute_pool_pda(amm, config_id, token_a, token_b)
    compute_pool_pda_seed likewise gains config_id
  Anything linking amm_core must update call sites.

* config and pool must now belong to the same namespace. Every pool-
  consuming instruction (new_definition, add/remove liquidity, swap,
  sync_reserves, and the oracle keeper ops) asserts
  pool.account_id == compute_pool_pda(amm, config.account_id,
  token_a, token_b), so a config and pool from different instances can
  no longer be combined.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants