feat: index clamm pair on dex to support skip api#243
Conversation
WalkthroughAdds a public Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Dex as DEX Module
participant Clamm as ClammTokenPairs (Storage)
participant Pool as Pool Storage
participant Flash as FlashSwapLock
Client->>Dex: create_pair_script(request with clamm token)
Dex->>Flash: acquire FlashSwapLock
Dex->>Pool: acquire Pool
Dex->>Clamm: acquire & populate ClammTokenPairs (store pair tokens)
Dex->>Dex: destroy one coin (clamm-specific handling)
Dex-->>Client: return pair info (denoms via address_to_denom)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@precompile/modules/initia_stdlib/sources/dex.move`:
- Around line 469-487: The code borrows from ClammTokenPairs.table without
verifying the key exists, causing a generic table abort if pair_denom is neither
a Config nor a registered clamm pair; before calling
clamm_pair_to_tokens.pair_to_tokens.borrow(addr) (and after
borrow_global<ClammTokenPairs>(`@initia_std`)), add an existence check (e.g., use
the table contains/has-key API or ensure the pair key exists in pair_to_tokens)
and abort with a clear error if the pair is not registered so the function (the
PoolInfoResponse path using denom_to_address, ClammTokenPairs, and
pair_to_tokens.borrow) fails with a descriptive message instead of a generic
table error.
- Around line 1364-1399: The clamm branch currently calls
fungible_asset::destroy_zero(coin_b) which will abort if coin_b has non-zero
balance; update the clamm branch in the create-pair flow (the block that checks
signer::address_of(creator) == `@dex_clamm_package`) to either (A) validate coin_b
is zero up front (e.g., require coin::value(&coin_b) == 0 or check the original
coin_b_amount) and abort with a clear error, or (B) if non-zero coin_b should be
preserved, deposit/return coin_b to the creator (use fungible_asset::deposit or
the existing withdraw/deposit helpers) instead of destroy_zero; additionally
ensure this clamm branch runs the same validations and side-effects as the
normal branch by calling/asserting assert_weights(weights), enforcing the
swap_fee_rate max validation, and emitting the CreatePairEvent (or invoking the
same event emission code used in the non-clamm path) so PairResponse creation,
validations, and event emission are consistent.
🧹 Nitpick comments (1)
precompile/modules/initia_stdlib/sources/dex.move (1)
2109-2111: Test coverage for clamm-specific paths is missing.The existing tests have been updated with the new
ClammTokenPairsacquires clause, but there are no new tests covering:
init_clamm_pair_indexinitializationcreate_paircalled by@dex_clamm_package(clamm branch)get_pool_info_by_denomfor clamm pairs- Edge cases for
denom_to_addressandaddress_to_denomConsider adding dedicated tests for the new clamm pair indexing functionality.
Do you want me to help generate test cases for the clamm pair functionality?
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@precompile/modules/initia_stdlib/sources/dex.move`:
- Around line 2110-2113: Tests fail because create_pair unconditionally borrows
ClammTokenPairs; update create_pair to use a conditional borrow (e.g.,
borrow_global_opt/exists check) instead of unconditionally borrowing
ClammTokenPairs so it won't abort when the index isn't initialized, referencing
the create_pair function and ClammTokenPairs resource; alternatively, if
unconditional presence is intended, add a call to init_clamm_pair_index(&chain,
<owner_address>) from test setup paths such as end_to_end (which currently only
calls init_module) to ensure init_clamm_pair_index is executed before
create_pair is called.
- Around line 1358-1361: The code unconditionally calls
borrow_global_mut<ClammTokenPairs>(`@initia_std`) in create_pair which causes
aborts if init_clamm_pair_index wasn't run; change the logic so ClammTokenPairs
is only borrowed when needed: perform the clamm-specific check first (e.g.,
detect clamm package/address or token pair type), and only then call
borrow_global_mut<ClammTokenPairs>(`@initia_std`) and use it; adjust create_pair
(and callers create_pair_script / create_lbp_pair_script) to avoid the
unconditional borrow and to fall back to non-clamm flow when ClammTokenPairs is
absent, or use a conditional exists/borrow_global_mut check tied to
init_clamm_pair_index.
🧹 Nitpick comments (1)
precompile/modules/initia_stdlib/sources/dex.move (1)
786-789: Consider adding an existence check before initialization.If
init_clamm_pair_indexis called twice,move_towill abort with a genericRESOURCE_ALREADY_EXISTSerror. Adding an explicit check would provide a clearer error message.🔧 Proposed fix
public entry fun init_clamm_pair_index(chain: &signer, dex_clamm_package_addr: address) { check_sudo(chain); + assert!( + !exists<ClammTokenPairs>(`@initia_std`), + error::already_exists(EUNAUTHORIZED) // or define a new error constant + ); move_to(chain, ClammTokenPairs { dex_clamm_package_addr, pair_to_tokens: table::new() }); }
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@precompile/modules/initia_stdlib/sources/dex.move`:
- Around line 468-492: In get_pool_info_by_denom, guard the
borrow_global<ClammTokenPairs>(`@initia_std`) to avoid a missing-resource abort by
first checking exists<ClammTokenPairs>(`@initia_std`) (or otherwise handling the
absent resource) and return/assert a clear not-found error (e.g.,
error::not_found(ECOIN_TYPE) or a new specific error) before attempting to
borrow; locate the borrow in get_pool_info_by_denom and replace the
unconditional borrow_global call with an existence check followed by a
controlled error path, then proceed to borrow and read pair_to_tokens only when
the resource exists.
- Around line 1859-1880: denom_to_address currently returns
coin::metadata_address(`@initia_std`, denom) for non-"move/" denoms without
checking that Metadata exists, while address_to_denom treats addresses without
Metadata as encoded "move/..." strings via fungible_asset::is_fungible_asset;
make them symmetric by adding an existence check in denom_to_address (verify
exists<Metadata>(coin::metadata_address(...)) and abort/return an encoded
"move/<hex>" in the same way as address_to_denom), or alternatively have
address_to_denom stop treating missing Metadata as a "move/" encoding and
instead mirror coin::metadata_address behavior; update the logic in
denom_to_address and/or address_to_denom so every address produced by
denom_to_address is handled deterministically by address_to_denom (refer to
functions denom_to_address, address_to_denom, coin::metadata_address,
fungible_asset::is_fungible_asset).
🧹 Nitpick comments (1)
precompile/modules/initia_stdlib/sources/dex.move (1)
790-799: Consider idempotent init or a clearer error on re‑init.
move_towill abort if the resource already exists; a guard or explicit error would make this safer for upgrades/ops.
Description
Closes: #XXXX
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!in the type prefix if API or client breaking changeReviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...
Summary by CodeRabbit
New Features
Updates