Skip to content

!fix(move): route dex swaps through stdaddr#509

Open
beer-1 wants to merge 2 commits intomainfrom
feat/dex-stdaddr-swap-routing
Open

!fix(move): route dex swaps through stdaddr#509
beer-1 wants to merge 2 commits intomainfrom
feat/dex-stdaddr-swap-routing

Conversation

@beer-1
Copy link
Copy Markdown
Member

@beer-1 beer-1 commented Apr 7, 2026

Description

Closes: N/A

Route DEX swap execution for blocked recipients through StdAddr in the dex layer instead of calling stableswap or CLAMM directly from the original recipient.

This change does three things:

  • applies the existing StdAddr routing pattern to stableswap-backed dex swaps
  • extracts the shared routing flow into a helper used by both stableswap and CLAMM paths in DexKeeper
  • adds regression tests showing direct keeper swaps fail for blocked recipients like the fee collector, while dex-level swaps succeed through the routed path

Critical files to review:

  • x/move/keeper/dex.go
  • x/move/keeper/dex_test.go
  • x/move/keeper/stableswap_test.go
  • x/move/keeper/clamm_test.go

Author Checklist

I have...

  • included the correct type prefix in the PR title
  • confirmed ! in the type prefix if API or client breaking change
  • targeted the correct branch
  • provided a link to the relevant issue or specification
  • reviewed "Files changed" and left comments if necessary
  • included the necessary unit and integration tests
  • updated the relevant documentation or specification, including comments for documenting Go code
  • confirmed all CI checks have passed

Notes:

  • breaking change: not applicable
  • issue/spec link: N/A
  • docs/spec update: not applicable for this internal keeper fix
  • CI checks: local validation completed; GitHub CI pending on the PR

Reviewers Checklist

I have...

  • confirmed the correct type prefix in the PR title
  • confirmed all author checklist items have been addressed
  • reviewed state machine logic, API design and naming, documentation is accurate, tests and test coverage

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 7, 2026

📝 Walkthrough

Walkthrough

The pull request introduces blocked recipient validation for CLAMM and StableSwap swap operations, refactors the SwapToBase flow to route both paths through a shared helper function, updates corresponding tests to use the fee collector module account, and adds an operational note regarding potential stableswap-backed gas-price swap failures.

Changes

Cohort / File(s) Summary
Blocked Recipient Validation Tests
x/move/keeper/clamm_test.go, x/move/keeper/stableswap_test.go
Added new test cases (Test_CLAMM_SwapToBase_BlockedRecipient and Test_StableSwap_SwapToBase_BlockedRecipient) that verify SwapToBase returns an error when the fee collector module address is used as the recipient.
SwapToBase Refactoring
x/move/keeper/dex.go
Introduced swapToBaseViaStdAddr helper function to centralize balance tracking and transfer routing. Both StableSwap and CLAMM paths now route through this helper, which manages quote coin transfers to StdAddr, captures base balance deltas, and forwards only positive deltas back to the original caller address.
Test Updates
x/move/keeper/dex_test.go
Updated TestDex_SwapToBase_StableSwap to fund and use the fee collector module account instead of a test account, added assertions to verify standard address balance remains unchanged during the swap.
Operational Guidance
x/move/keeper/whitelist.go
Added multi-line NOTE comment in the stableswap-backed WhitelistGasPrice method documenting potential execution failures and repeated log emissions prior to the next upgrade, with assurance that chain liveness is not affected.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A swap flows true through helper's gate,
Where base and quote decide their fate,
Fee collectors blocked from swapping ways,
Notes guide ops through uncertain days,
Balance tracked at standard's door.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description check ✅ Passed The description is directly related to the changeset, explaining the routing pattern change, helper extraction, and the regression tests added across multiple files.
Title check ✅ Passed The title 'fix(move): route dex swaps through stdaddr' directly describes the main change: routing DEX swaps through StdAddr instead of invoking stableswap/CLAMM directly from blocked recipients.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/dex-stdaddr-swap-routing

Comment @coderabbitai help to get the list of available commands and usage tips.

@beer-1 beer-1 force-pushed the feat/dex-stdaddr-swap-routing branch from da125df to 8eda438 Compare April 7, 2026 04:43
@beer-1 beer-1 changed the title [codex] route dex stableswap swaps via stdaddr fix(move): route dex swaps through stdaddr Apr 7, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 7, 2026

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Snapshot Warnings

⚠️: No snapshots were found for the head SHA 1f47d3d.
Ensure that dependencies are being submitted on PR branches. Re-running this action after a short time may resolve the issue. See the documentation for more information and troubleshooting advice.

Scanned Files

None

@beer-1 beer-1 marked this pull request as ready for review April 7, 2026 04:47
@beer-1 beer-1 requested a review from a team as a code owner April 7, 2026 04:47
@beer-1 beer-1 changed the title fix(move): route dex swaps through stdaddr !fix(move): route dex swaps through stdaddr Apr 7, 2026
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@x/move/keeper/dex.go`:
- Around line 324-338: The route is not atomic: k.moveBankKeeper.SendCoin debits
addr before swap() and later GetBalance/SendCoin to refund can fail, leaving
funds stranded on types.StdAddr; wrap the entire sequence (the initial SendCoin,
swap(), balance check and possible payout using
prevBaseBalance/baseDenom/baseBalanceDiff) inside a cached context/transaction
and only commit if swap() and the payout succeed, or implement an explicit
rollback/refund path that returns the quote/base from types.StdAddr back to addr
on any error—ensure the swap() caller (the higher-order call site suggested
around the former lines 279-300) accepts and uses the provided cached context so
the whole operation is committed atomically.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: d87c5514-8695-48c1-b62f-137ed8d02392

📥 Commits

Reviewing files that changed from the base of the PR and between b5de972 and 1f47d3d.

📒 Files selected for processing (5)
  • x/move/keeper/clamm_test.go
  • x/move/keeper/dex.go
  • x/move/keeper/dex_test.go
  • x/move/keeper/stableswap_test.go
  • x/move/keeper/whitelist.go

Comment thread x/move/keeper/dex.go
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 7, 2026

Codecov Report

❌ Patch coverage is 52.77778% with 17 lines in your changes missing coverage. Please review.
✅ Project coverage is 38.11%. Comparing base (b5de972) to head (1f47d3d).

Files with missing lines Patch % Lines
x/move/keeper/dex.go 46.87% 11 Missing and 6 partials ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #509      +/-   ##
==========================================
+ Coverage   38.09%   38.11%   +0.01%     
==========================================
  Files         325      325              
  Lines       30819    30832      +13     
==========================================
+ Hits        11742    11752      +10     
- Misses      17184    17186       +2     
- Partials     1893     1894       +1     
Files with missing lines Coverage Δ
x/move/keeper/whitelist.go 38.75% <100.00%> (+1.03%) ⬆️
x/move/keeper/dex.go 43.78% <46.87%> (+1.07%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

1 participant