[AMM] feat: add sync reserves and recover surplus functionality by 3esmit · Pull Request #12 · logos-blockchain/lez-programs

3esmit · 2026-03-31T15:05:16Z

🎯 Purpose

Fixes #6.
Migrate the AMM safety work from logos-execution-zone PR 371 into lez-programs, adapted to this repository's current structure and guest program model.

Adds explicit reserve sync and surplus recovery flows so direct vault donations no longer skew pricing silently.
Adds permanent minimum-liquidity locking so LP supply cannot be fully drained to zero.
Updates the guest entrypoint, checked-in IDL, and AMM tests to match the new behavior.

⚙️ What Changed

Add SyncReserves and RecoverSurplus { InactiveOrZeroSupplyOnly } to amm_core, wire them through amm/methods/guest/src/bin/amm.rs, and regenerate amm/amm-idl.json.
Add MINIMUM_LIQUIDITY, stable LP lock PDA helpers/domain separators, and mint locked LP before user LP in new_definition.
Tighten remove_liquidity so it rejects invalid LP holdings, over-burns, and attempts to remove the permanently locked liquidity.
Read live vault balances through shared vault_utils in add, swap, sync, and recover, with explicit under-collateralization and token-definition mismatch checks.
Extend AMM unit coverage for donation sync, surplus recovery, under-collateralized vaults, LP lock behavior, and reinitialization paths.
Update integration_tests/tests/amm.rs so pool initialization expects user LP to equal sqrt(a * b) - MINIMUM_LIQUIDITY.

🔎 Migration Notes

Ports the intent of logos-execution-zone PR 371, but adapts it to spel_framework #[instruction] guests and the per-program methods/guest layout used in this repo.
recover_surplus now transfers only vault_balance - reserve, so reserves remain unchanged and no follow-up sync_reserves call is required after recovery.
The final diff from main is scoped to AMM program/core/guest/IDL files plus integration_tests/tests/amm.rs.

🔎 Review Focus

new_definition now mints locked LP first and uses the post-lock LP definition state for the user mint.
sync_reserves and recover_surplus both fail if a vault is under-collateralized instead of silently saturating.
recover_surplus is restricted to inactive or zero-supply pools and validates vault/recipient token definitions before transferring.

🧪 How to Test

RISC0_SKIP_BUILD=1 cargo +1.94.0 clippy --workspace --all-targets -- -D warnings
RISC0_DEV_MODE=1 cargo +1.94.0 test -p amm_program
RISC0_DEV_MODE=1 cargo +1.94.0 test -p integration_tests --test amm
spel-cli generate-idl amm/methods/guest/src/bin/amm.rs > amm/amm-idl.json

🔗 Dependencies

None.

🔜 Future Work

Add caller-facing support for sync_reserves and recover_surplus in external tooling or clients that invoke AMM instructions.

📋 PR Completion Checklist

Port donation-handling and minimum-liquidity protections into lez-programs
Wire the new AMM instructions through the guest and checked-in IDL
Add or update AMM unit and integration coverage
Keep the final diff scoped to AMM behavior plus AMM integration artifacts

- Introduced `SyncReserves` instruction to synchronize pool reserves with current vault balances. - Added `RecoverSurplus` instruction to recover surplus balances not backed by reserves. - Implemented utility functions for reading vault balances and fungible holdings. - Updated liquidity management to ensure minimum liquidity is maintained during operations. - Enhanced tests to cover new functionalities and edge cases for surplus recovery and reserve synchronization.

Copilot

Pull request overview

This PR migrates AMM “safety” functionality into lez-programs by adding explicit reserve reconciliation and surplus recovery flows, plus a minimum-liquidity LP lock to prevent pools from being fully drained to zero supply.

Changes:

Adds sync_reserves and recover_surplus { mode } instructions (core enum + guest entrypoint + IDL), plus shared vault parsing helpers.
Introduces MINIMUM_LIQUIDITY = 1 and an LP lock holding PDA; updates new_definition and remove_liquidity to enforce the permanent minimum-liquidity lock.
Extends unit/integration tests to cover donation syncing, surplus recovery, and minimum-liquidity behavior.

Reviewed changes

Copilot reviewed 12 out of 13 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
integration_tests/tests/amm.rs	Adjusts expected initial user LP to account for permanently locked `MINIMUM_LIQUIDITY`.
amm/src/vault_utils.rs	Adds shared helpers to parse fungible token holdings / vault balances with contextual panic messages.
amm/src/sync.rs	Implements `sync_reserves` to update pool reserves from current vault balances.
amm/src/recover.rs	Implements `recover_surplus` to transfer only `vault_balance - reserve` (with under-collateralization checks).
amm/src/new_definition.rs	Mints locked LP first, then mints user LP; adds minimum-liquidity lock PDA usage and related checks.
amm/src/remove.rs	Prevents burning locked minimum liquidity and adds explicit “amount exceeds user balance” assertion.
amm/src/add.rs	Refactors vault holding parsing to shared `vault_utils` helpers.
amm/src/swap.rs	Refactors vault holding parsing to shared `vault_utils` helpers.
amm/src/lib.rs	Wires in new modules (`recover`, `sync`) and shared `vault_utils`.
amm/methods/guest/src/bin/amm.rs	Exposes `sync_reserves` / `recover_surplus` as `#[instruction]` guest entrypoints.
amm/core/src/lib.rs	Adds new instruction variants/types, `MINIMUM_LIQUIDITY`, LP lock PDA derivation, and PDA domain separator constants.
amm/src/tests.rs	Updates/extends unit tests for min-liquidity lock, sync, and surplus recovery scenarios.
amm/amm-idl.json	Regenerates IDL to include the new instructions and args.

Comments suppressed due to low confidence (1)

amm/src/new_definition.rs:79

new_definition currently allows initializing any pool with active == false, even if it already has nonzero liquidity_pool_supply and/or the LP TokenDefinition already has a nonzero total_supply. In that state, the instruction will mint MINIMUM_LIQUIDITY + user_lp again, inflating LP token supply and desynchronizing it from pool_post_definition.liquidity_pool_supply. Recommend adding an explicit guard when !is_new_pool (e.g., require pool_account_data.liquidity_pool_supply == 0 and the existing LP token definition total_supply == 0) before proceeding with the re-initialization mint path.

    let is_new_pool = pool.account == Account::default();
    let pool_account_data = if is_new_pool {
        PoolDefinition::default()
    } else {
        PoolDefinition::try_from(&pool.account.data)
            .expect("AMM program expects a valid Pool account")
    };

    assert!(
        !pool_account_data.active,
        "Cannot initialize an active Pool Definition"
    );

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

amm/src/sync.rs

amm/src/recover.rs

…ction

…s function

… and add panic test for vault A definition mismatch

Copilot

Pull request overview

Copilot reviewed 12 out of 13 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

0x-r4bbit · 2026-04-02T11:50:44Z

amm/core/src/lib.rs

+    InactiveOrZeroSupplyOnly,
 }

+pub const MINIMUM_LIQUIDITY: u128 = 1;


Let's increase this to 1000 to make this stronger (similar to how it's done in Uniswap)

0x-r4bbit · 2026-04-02T12:00:55Z

amm/core/src/lib.rs

+    ///
+    /// This transfers only balances above the tracked reserves, so pool reserves remain
+    /// unchanged and no follow-up `SyncReserves` call is required.
+    RecoverSurplus { mode: RecoverSurplusMode },


I think if we have SyncReserves, there's little point in having RecoverSurplus. Recovering funds can easily be prevent by syncing reserves.

0x-r4bbit · 2026-04-02T12:41:26Z

amm/src/vault_utils.rs

@@ -0,0 +1,29 @@
+use nssa_core::account::{AccountId, AccountWithMetadata};


Can we move this stuff into core/src (maybe just in a utils.rs file?
Let's keep the convention to only have program function files in /src

0x-r4bbit · 2026-04-02T12:59:45Z

amm/core/src/lib.rs


+// These separators are part of the PDA derivation scheme and must stay stable for compatibility.
+const LIQUIDITY_TOKEN_PDA_DOMAIN_SEPARATOR: [u8; 32] = [0; 32];
+const LP_LOCK_HOLDING_PDA_DOMAIN_SEPARATOR: [u8; 32] = [1; 32];


These are actually seeds so let's rename them to

Suggested change

const LP_LOCK_HOLDING_PDA_DOMAIN_SEPARATOR: [u8; 32] = [1; 32];

const LP_LOCK_HOLDING_PDA_SEED: [u8; 32] = [1; 32];

Maybe, we can also consider changing the seeds to something like:

const LIQUIDITY_TOKEN_PDA_SEED: &[u8] = b"amm:liquidity_token"; const LP_LOCK_HOLDING_PDA_SEED: &[u8] = b"amm:lp_lock_holding";

Let's explore best practices here.

0x-r4bbit · 2026-04-02T13:10:06Z

amm/src/remove.rs

        panic!(
            "Remove liquidity: AMM Program expects a valid Fungible Token Holding Account for liquidity token"
        );
    };


Shouldn't this also use the new utilities in vault_utils.rs?

0x-r4bbit · 2026-04-02T13:12:31Z

amm/src/remove.rs

+    assert!(
+        remove_liquidity_amount <= user_lp_balance,
+        "Cannot remove more liquidity than owned"
+    );


This check is not needed because this will fail in Transfer if the user tries to transfer more than what they have.

0x-r4bbit · 2026-04-02T13:16:27Z

amm/src/remove.rs

+    assert!(
+        pool_def_data.liquidity_pool_supply > MINIMUM_LIQUIDITY,
+        "Pool only contains locked liquidity"
+    );


This check seems unnecessary as well.
If lp pool supply is <= MINIMUM_SUPPLY, that means there are no additional LP tokens. So the user won't have any balance to burn in the first place

0x-r4bbit · 2026-04-02T13:17:37Z

amm/src/remove.rs

+    assert!(
+        remove_liquidity_amount <= pool_def_data.liquidity_pool_supply - MINIMUM_LIQUIDITY,
+        "Cannot remove locked minimum liquidity"
+    );


This as well. I don't think there's a scenario where this will ever be true.

0x-r4bbit · 2026-04-02T13:49:30Z

amm/core/src/lib.rs

+
+#[derive(Clone, Copy, Serialize, Deserialize)]
+pub enum RecoverSurplusMode {
+    InactiveOrZeroSupplyOnly,


This is already cancelling itself out with MINIMUM_LIQUIDITY.

When a pool is initialized, it will have a supply (MINIMUM_LIQUIDITY) which also makes it active.

So with this , surplus can never be recovered.
I created #25 to make this more clear.

In any case, either we remove this mode, or we drop recover surplus entirely.

Copilot AI review requested due to automatic review settings March 31, 2026 15:05

Copilot started reviewing on behalf of 3esmit March 31, 2026 15:06 View session

Copilot AI reviewed Mar 31, 2026

View reviewed changes

amm/src/sync.rs Show resolved Hide resolved

amm/src/recover.rs Outdated Show resolved Hide resolved

3esmit added 5 commits March 31, 2026 12:41

chore: fix lint blockers before PR

b090a74

fix: update import for Sha256 in compute_lp_lock_holding_pda_seed fun…

69ed296

…ction

test: add panic tests for under-collateralized vaults in sync_reserve…

16f073f

…s function

fix: update recover_surplus function to read vault balances correctly…

3cf864a

… and add panic test for vault A definition mismatch

fix: revert test configuration directive to ata/tests.rs

82d3b24

3esmit requested a review from Copilot March 31, 2026 18:06

Copilot started reviewing on behalf of 3esmit March 31, 2026 18:06 View session

Copilot AI reviewed Mar 31, 2026

View reviewed changes

3esmit mentioned this pull request Mar 31, 2026

[AMM] add sync reserves and recover surplus functionality logos-blockchain/logos-execution-zone#371

Closed

14 tasks

3esmit changed the title ~~[AMM] add sync reserves and recover surplus functionality~~ [AMM] feat: add sync reserves and recover surplus functionality Mar 31, 2026

0x-r4bbit requested review from 0x-r4bbit and gravityblast April 1, 2026 10:19

0x-r4bbit requested changes Apr 2, 2026

View reviewed changes

0x-r4bbit reviewed Apr 2, 2026

View reviewed changes

This was referenced Apr 2, 2026

Remove dead re-initialization code path from new_definition #26

Open

feat: enhance remove liquidity function to handle surplus balances #16

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[AMM] feat: add sync reserves and recover surplus functionality#12

[AMM] feat: add sync reserves and recover surplus functionality#12
3esmit wants to merge 6 commits intologos-blockchain:mainfrom
3esmit:fix/amm-surplus-sync-recover-min-lp

3esmit commented Mar 31, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

0x-r4bbit Apr 2, 2026

Uh oh!

0x-r4bbit Apr 2, 2026

Uh oh!

0x-r4bbit Apr 2, 2026

Uh oh!

0x-r4bbit Apr 2, 2026

Uh oh!

0x-r4bbit Apr 2, 2026

Uh oh!

0x-r4bbit Apr 2, 2026

Uh oh!

0x-r4bbit Apr 2, 2026

Uh oh!

0x-r4bbit Apr 2, 2026

Uh oh!

0x-r4bbit Apr 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		@@ -0,0 +1,29 @@
		use nssa_core::account::{AccountId, AccountWithMetadata};

	const LP_LOCK_HOLDING_PDA_DOMAIN_SEPARATOR: [u8; 32] = [1; 32];
	const LP_LOCK_HOLDING_PDA_SEED: [u8; 32] = [1; 32];

Conversation

3esmit commented Mar 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🎯 Purpose

⚙️ What Changed

🔎 Migration Notes

🔎 Review Focus

🧪 How to Test

🔗 Dependencies

🔜 Future Work

📋 PR Completion Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

3esmit commented Mar 31, 2026 •

edited

Loading