feat(amm): point the module, UI app, and testnet setup at namespaced … - #358
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Several code paths pass a possibly-null config read into amm_pool_id (breaking FFI request deserialization) and there are multiple redundant config reads plus stale error logs that should be fixed before merge.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR updates the AMM client stack (C++ module + QML UI + testnet setup) to target a specific namespaced AMM instance by its config-PDA account id (ammConfigId), matching the program-side change where pools are namespaced under the config PDA rather than a singleton config.
Changes:
- Module: adds
setConfigId/ammConfigId()and switches config reads to use the configured config-PDA id (withAMM_CONFIG_IDenv fallback). - Module: threads the config account read into
amm_pool_idcalls (so pool derivation is namespaced by config). - UI + testnet tooling: loads/publishes
ammConfigIdfrom the registry and updates testnet setup/initialize flow to include(owner, nonce).
File summaries
| File | Description |
|---|---|
| modules/amm/src/amm_module_impl.h | Adds config-id selection API and updates config-read contract to be instance-based. |
| modules/amm/src/amm_module_impl.cpp | Implements config id selection + instance config reads; updates pool-id derivations to include config. |
| apps/amm/tests/testnet/setup-amm-testnet.sh | Updates testnet setup to create a namespaced instance and publish ammConfigId into the registry. |
| apps/amm/src/RegistryLoader.h | Adds storage/accessor for active network’s ammConfigId. |
| apps/amm/src/RegistryLoader.cpp | Loads ammConfigId from the selected network entry. |
| apps/amm/src/AmmUiBackend.cpp | Pushes ammConfigId into the module via setConfigId on registry changes. |
Review details
Suppressed comments (8)
modules/amm/src/amm_module_impl.cpp:684
- readConfig() can return null when no config id is configured, but amm_pool_id deserializes PoolIdRequest::config as a required AccountRead. Passing a null config here will fail request JSON deserialization and likely surface as backend_error rather than a stable config error.
const FfiResult poolId = call(amm_pool_id, json{
{"ammProgramId", amm_program_id},
{"config", readConfig()},
{"tokenInId", token_in},
{"tokenOutId", token_out},
});
modules/amm/src/amm_module_impl.cpp:1054
- addLiquidityQuote() forwards readConfig() directly into amm_pool_id, but readConfig() returns null when no config id is configured. PoolIdRequest requires a non-null config AccountRead, so this will fail JSON deserialization and surface as an opaque backend_error.
const FfiResult poolId = call(amm_pool_id, json{
{"ammProgramId", amm_program_id},
{"config", readConfig()},
{"tokenInId", token_a},
{"tokenOutId", token_b},
});
modules/amm/src/amm_module_impl.cpp:1207
- removeLiquidityQuote() forwards readConfig() directly into amm_pool_id, but readConfig() returns null when no config id is configured. PoolIdRequest requires a non-null config AccountRead, so this will fail JSON deserialization and surface as an opaque backend_error.
const FfiResult poolId = call(amm_pool_id, json{
{"ammProgramId", amm_program_id},
{"config", readConfig()},
{"tokenInId", token_a},
{"tokenOutId", token_b},
});
modules/amm/src/amm_module_impl.cpp:842
- swapExactOutput() already read the config into a local
configvariable, but then calls readConfig() again when deriving the pool id. This adds an unnecessary wallet read and risks using a different config for pool derivation than for the plan.
{"config", readConfig()},
modules/amm/src/amm_module_impl.cpp:1133
- addLiquidity() already fetched
config(and validated it non-null) but then calls readConfig() again when deriving the pool id, causing an extra wallet read and bypassing the earlier validation.
{"config", readConfig()},
modules/amm/src/amm_module_impl.cpp:1284
- removeLiquidity() already fetched
config(and validated it non-null) but then calls readConfig() again when deriving the pool id, causing an extra wallet read and bypassing the earlier validation.
{"config", readConfig()},
modules/amm/src/amm_module_impl.cpp:1359
- syncReserves() already fetched
config(and validated it non-null) but then calls readConfig() again when deriving the pool id, causing an extra wallet read and bypassing the earlier validation.
{"config", readConfig()},
modules/amm/src/amm_module_impl.cpp:827
- The failure log message still says the config_id op failed, but readConfig() now returns null when no config id is configured (setConfigId / AMM_CONFIG_ID). Updating this message will make debugging configuration issues much clearer.
AMM_TRACE("swapExactOutput: FAIL config_id op failed");
- Files reviewed: 6/6 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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.
e7b6eac to
1ed20e3
Compare
…instances Follow-up to the program-side namespacing (e7b6eac): wire the client stack to a specific AMM instance by its config-PDA id, the same way the registry already publishes pool ids. Module (modules/amm): - readConfig() now reads the account at a configured config-PDA id (ammConfigId, via setConfigId or the AMM_CONFIG_ID env fallback) instead of deriving the old singleton config; drop the amm_config_id call from the app path. - Add setConfigId / ammConfigId; every amm_pool_id call now carries `config` (pools are namespaced by their instance's config), and the inline config derivations in the plan/quote paths reuse readConfig(). - INTERFACE.md: config discovery is namespaced (registry-provided config account), no longer a singleton. No owner/nonce anywhere in the runtime. instance); amm_pdas derives the config PDA from (owner, nonce); the emitted registry carries the resulting ammConfigId. The owner/nonce live only in the deploy tooling; the module and app never see them — they read the published config id and derive everything from it.
575b419 to
cd4f41c
Compare
…instances
Follow-up to the program-side namespacing (e7b6eac): wire the client stack to a specific AMM instance by its config-PDA id, the same way the registry already publishes pool ids.
Module (modules/amm):
readConfig() now reads the account at a configured config-PDA id (ammConfigId, via setConfigId or the AMM_CONFIG_ID env fallback) instead of deriving the old singleton config; drop the amm_config_id call from the app path.
Add setConfigId / ammConfigId; every amm_pool_id call now carries
config(pools are namespaced by their instance's config), and the inline config derivations in the plan/quote paths reuse readConfig().config discovery is namespaced (registry-provided config account), no longer a singleton. No owner/nonce anywhere in the runtime.
instance); amm_pdas derives the config PDA from (owner, nonce); the emitted registry carries the resulting ammConfigId.
The owner/nonce live only in the deploy tooling; the module and app never see them — they read the published config id and derive everything from it.