Add SyncReserves Instruction For AMM Pools#36
Add SyncReserves Instruction For AMM Pools#363esmit wants to merge 2 commits intologos-blockchain:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new SyncReserves AMM instruction that refreshes a pool’s stored reserves from the current vault balances, enabling explicit reconciliation (e.g., after token donations) while rejecting under-collateralized vault states.
Changes:
- Introduces
SyncReservesinamm_core::Instructionand wires a guest entrypoint. - Implements reserve syncing logic and shared vault-balance parsing helpers.
- Adds unit tests covering donation syncing and under-collateralized vault failures; updates the checked-in AMM IDL.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| amm/src/vault_utils.rs | Adds shared helpers to read fungible token holding balances from vault accounts. |
| amm/src/sync.rs | Implements sync_reserves to update pool reserve bookkeeping from vault balances (with under-collateralization checks). |
| amm/src/lib.rs | Exposes the new sync module and adds internal vault_utils module. |
| amm/src/tests.rs | Adds unit tests for donation-driven reserve syncing and failure cases. |
| amm/methods/guest/src/bin/amm.rs | Adds guest entrypoint for the new sync_reserves instruction. |
| amm/core/src/lib.rs | Extends the AMM instruction enum with SyncReserves. |
| amm/amm-idl.json | Updates the IDL to include the new sync_reserves instruction. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…vault_fungible_balances functions
652e932 to
079a3f8
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 7 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "accounts": [ | ||
| { | ||
| "name": "pool", | ||
| "writable": false, |
There was a problem hiding this comment.
In sync_reserves, the pool account is mutated (reserves are rewritten), but the IDL marks the pool account as "writable": false. If IDL consumers use this flag to build transactions, they may pass the pool as read-only and the instruction will fail when trying to write. Consider setting pool.writable to true for sync_reserves (and aligning other instructions similarly if the writable field is intended to be authoritative).
| "writable": false, | |
| "writable": true, |
There was a problem hiding this comment.
This looks directionally correct: sync_reserves does rewrite the pool, so writable: false is misleading for that instruction. One nuance is that this does not appear isolated to sync_reserves; the checked-in generated IDLs currently mark every account as non-writable, including other mutating instructions, so this is likely a broader IDL-generation issue rather than a one-off JSON typo.
| let (_, vault_b_balance) = read_fungible_holding(vault_b, &vault_b_context); | ||
|
|
||
| (vault_a_balance, vault_b_balance) | ||
| } |
There was a problem hiding this comment.
As already mentioned in the other PR, let's not put this here, and instead move it into core and maybe have a utils.rs there. src/ is for program functions.
|
Landed via #40 |
Summary
This PR adds a
SyncReservesinstruction to the AMM so pool reserves can be explicitly refreshed from current vault balances.The new path makes reserve reconciliation an intentional AMM action and covers the donation case where vault balances grow beyond the reserves currently stored in pool state.
Changes
SyncReservesto the AMM instruction enumsync_reservesTesting
cargo +nightly fmt --all -- --checktaplo fmt --check .RISC0_SKIP_BUILD=1 cargo +1.94.0 clippy --workspace --all-targets -- -D warningsRISC0_DEV_MODE=1 cargo +1.94.0 test --workspace --exclude integration_testsRISC0_DEV_MODE=1 cargo +1.94.0 test -p integration_testsNotes
SyncReservesupdates only AMM reserve bookkeeping and does not transfer tokens.