Skip to content

fix(dex): reject same-token (base==quote) pools on create and remove (v1.1.9)#479

Open
AntonAndell wants to merge 3 commits into
mainfrom
fix/dex-same-token-pool
Open

fix(dex): reject same-token (base==quote) pools on create and remove (v1.1.9)#479
AntonAndell wants to merge 3 commits into
mainfrom
fix/dex-same-token-pool

Conversation

@AntonAndell

Copy link
Copy Markdown
Collaborator

Summary

Reject AMM pools where base == quote.

The create-time guard in addInternal used Java reference equality on score.Address (_baseToken != _quoteToken), which is always true for freshly-deserialized parameters — so it never fired, and a same-token pool could be created. When base == quote, the per-token storage keys (poolTotal, deposit) collide, and _remove's sequential read-modify-write double-credits the deposit ledger, letting a withdrawal exceed the pool's backing.

Changes

  • DexImpl.addInternal: _baseToken != _quoteToken!_baseToken.equals(_quoteToken) (value equality) — prevents creating same-token pools.
  • DexImpl._remove: add require(!baseToken.equals(quoteToken), ...) — prevents removing from an already-created same-token pool (the create-time guard alone does not cover pre-existing pools).
  • Bump DEX version v1.1.8 → v1.1.9.

Test plan

  • add/xAdd with base == quote reverts ("Pool must contain two token contracts").
  • remove on a pre-existing base == quote pool reverts ("Invalid same-token pool, removal blocked").
  • Normal (base != quote) add/remove/swap unaffected (regression).

Deployment

Deploy under governance shutdown, then re-enable.

🤖 Generated with Claude Code

addInternal used Java reference equality (!=) on freshly-deserialized
score.Address params, so the base!=quote guard was a silent no-op and
base==quote pools could be created. Switch to value equality (.equals).
Add the same guard to _remove so any pre-existing same-token pool cannot
be drained via the DictDB key-collision double-credit. Bump DEX to v1.1.9.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@codecov

codecov Bot commented Jul 13, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.97%. Comparing base (10ab2f4) to head (b539792).

Files with missing lines Patch % Lines
.../java/network/balanced/score/core/dex/DexImpl.java 0.00% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main     #479      +/-   ##
============================================
- Coverage     72.98%   72.97%   -0.01%     
  Complexity     1639     1639              
============================================
  Files           120      120              
  Lines          7839     7840       +1     
  Branches        944      945       +1     
============================================
  Hits           5721     5721              
  Misses         1708     1708              
- Partials        410      411       +1     
Flag Coverage Δ
java 72.97% <0.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

58-action Governance execute() batch used alongside the DEX v1.1.9 fix:
block bnUSD/BALN cross-chain transfers (setSpokeLimit 0) and lock down
Loans (setRedemptionExemption true / setDebtCeiling 0) to neutralize the
redemption decimals bug and new borrows config-side. See recovery/README.md.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@github-actions github-actions Bot added the documentation Improvements or additions to documentation label Jul 13, 2026
ArbitraryCallManager parses Boolean params via JsonValue.asBoolean, which
requires a JSON boolean literal. The value was a quoted string ("true"),
causing execute() to revert with UnknownFailure on the first exemption call
(atomic batch -> whole tx failed). Verified fixed via debug_estimateStep.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants