Market Module 2.0 Implementation

[StrathCole]

Summary of changes (Copilot)

This pull request introduces the v15 network upgrade and implements a new tax redirect mechanism for the market module, along with associated test coverage and module/account configuration changes. The upgrade restricts allowed swap denoms to uusd by default, ensures the oracle meta denom is present, and refactors the burn/tax split logic to support redirecting a portion of taxes to the market accumulator. Test logic is updated to cover these new behaviors.

Network Upgrade and Module Changes:

• Added the v15 upgrade handler, which restricts allowed market swap denoms to uusd and ensures the oracle meta denom is included in vote targets. The upgrade is registered in the app and includes no store migrations. (app/upgrades/v15/constants.go, app/upgrades/v15/upgrades.go, app/app.go) [1] [2] [3] [4]
• Updated the market keeper initialization to include the distribution keeper and set market hooks on the oracle keeper to track tally events for TWAP and freshness. (app/keepers/keepers.go)

Tax Redirect and Module Account Handling:

• Modified module account permissions and allowed receiving accounts to add support for the new market accumulator module account. The market module can now receive funds, and the accumulator account is registered. (app/modules.go) [1] [2]
• Updated the burn/tax split logic in tests to support the new tax redirect rate, ensuring correct distribution between fee collector, oracle, market accumulator, and community pool. The test logic now sets all relevant parameters atomically and validates the new distribution. (custom/auth/ante/fee_test.go) [1] [2] [3] [4] [5]

Test Improvements:

• Enhanced ante handler tests to explicitly set the tax redirect rate to zero in legacy test cases, ensuring backward compatibility and focused test coverage for the new redirect logic. (custom/auth/ante/fee_test.go) [1] [2] [3]

Minor/Housekeeping:

• Added import and formatting tweaks for clarity and to support new modules. (app/app.go, app/modules.go) [1] [2] [3]
• Removed a redundant consensus node prefix setting in the CLI root command. (cmd/terrad/root.go)

[Copilot]

Pull request overview

This PR ships "Market Module 2.0" together with a v15 network upgrade. It restructures the swap pipeline (allowed-denom guard, oracle freshness, TWAP deviation, daily cap, epoch burn/refill), redirects a configurable share of taxes to a new market_accumulator module account, adds oracle USD-price queries with a UST meta-denom, and wires market hooks into the oracle tally. It touches consensus-critical paths across app/, x/market, x/oracle, x/tax, x/treasury, plus extensive ante/e2e test changes.

Changes:

• v15 upgrade: pin allowed swap denom to uusd, register UST meta-denom in oracle whitelist/Tobin-tax, add market_accumulator module account.
• Market 2.0: in-memory allowed denoms, swap fee burn/community split, TWAP store + deviation check, daily cap, end-of-epoch burn-and-refill from accumulator, oracle freshness guard.
• Tax/treasury: new TaxRedirectRate (default 0.6) routes a slice of taxes to the market accumulator before the existing oracle/community/burn split, with per-leg events.

Reviewed changes

Copilot reviewed 61 out of 63 changed files in this pull request and generated 17 comments.

Show a summary per file

File	Description
app/app.go, app/keepers/keepers.go, app/modules.go	Register v15 upgrade, wire distribution keeper into market, add market_accumulator module account, set oracle MarketHooks
app/upgrades/v15/{constants,upgrades}.go	New upgrade handler: set allowed denom to uusd, ensure UST meta denom in whitelist + Tobin tax store
x/market/keeper/keeper.go, msg_server.go, abci.go	Allowed-denom map, freshness/TWAP/daily-cap safeguards, epoch burn+refill, swap fee split
x/market/types/{keys.go,params.pb.go,market.pb.go} & proto	New params (epoch length, fee splits, oracle age, TWAP window/deviation, daily-cap factor), accumulator module name, store keys
x/market/keeper/{epoch_test.go,safeguards_test.go,msg_server_test.go,test_utils.go}	Unit tests for new safeguards + epoch; market account perms updated in test setup
x/market/simulation/*	Minor genesis/param additions (does not yet cover new safeguards)
x/oracle/keeper/{keeper.go,querier.go,ballot.go}, types/params.go, abci.go, client/cli, proto	Add MetaUSDDenom = "UST", GetUSDPrice/IterateUSDPrices, USDPrice/USDPrices RPCs, MarketHooks tally callback
x/tax/keeper/tax_split.go, types/events.go, tax_split_test.go, handlers/market_msg_server.go	Market-redirect-first split with per-leg events; new Swap reverse-charge handler; focused test
x/treasury/{types/params.go, keeper/params.go, types/params_test.go, keeper/test_utils.go}	Add TaxRedirectRate param, getter/setter, validation, default 0.6
custom/auth/ante/fee_test.go	Updated burn/tax-split test harness to account for redirect; legacy cases pin redirect to 0
cmd/terrad/root.go	Removes SetBech32PrefixForConsensusNode call
tests/e2e/*	New end-to-end coverage for upgrade, safeguards, redirect; config knobs lowered for fast epochs
tests/interchaintest/{setup.go,go.sum}	Pin tax_redirect_rate to 0; regenerated go.sum with some suspicious entries
scripts/protocgen.sh, scripts/upgrade-test.sh	Minor script tweaks
proto/terra/{market,treasury,oracle}/v1beta1/*.proto + generated pb.go	Schema additions for new params and USD price RPCs

---

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

[hoank101]

maccPerms registers AccumulatorModuleName for new chains (via InitChainer), but for an existing chain upgrading, the account doesn't exist in the auth store. The upgrade handler must create it

[StrathCole]

It is only a virtual address, not a module. That normally is lazily created on usage.

[Farrell23]

@StrathCole the review states that the simulation does not yet cover the new safeguards (TWAP, daily cap). Will this be taken into account/covered in the future? Or is this just a test gap?

[Farrell23]

Good day everyone! I tried to explore the code, but there were some difficulties with the launch and real testing.
I did a code review of this MM2.0 implementation and pushed a small set of safe-hardening fixes as a companion PR: Market-Module-2-0#2.

It hardens IsReverseCharge against a missing context value (panic), makes the oracle-freshness guard fail closed before the first tally, fixes the copy-by-value SetAllowedSwapDenoms setter, and corrects the stale market spec. The full review — architecture assessment and all findings, including policy items deliberately left as proposals — is in docs/reviews/mm2.0-review.md.

Please read my ideas and the approach that I implemented through Claude.
I would be grateful for feedback and comments.

Happy to discuss any of the findings.
@StrathCole @fragwuerdig

[lunctoken]

Hi @StrathCole and reviewers,

I used this PR's current head (c5bf7ed) as the baseline for an extended local implementation and validation campaign. The resulting companion work is available in these draft PRs:

• Core changes and test report: feat(market): complete MM2 activation, safeguards, and validation Market-Module-2-0/core#3
• Related StrathCole feeder correction: fix(feeder): publish USD per USTC for MM2 StrathCole/oracle-go#1

The companion core PR addresses and validates several points found while exercising the implementation end to end:

• deterministic market_accumulator creation and an idempotent v15 migration from pre-MM2 state;
• Market starting disabled, then activating only after a complete collection epoch, funded reserves, and healthy Oracle quorum;
• the UST meta-denom contract as USD per USTC while the bank denom remains uusd;
• adaptive liquidity using the proposal's 7% factor, with the 10% daily cap kept as an independent hard limit;
• persistent Oracle quorum halt/recovery, a duration-weighted 45-block TWAP, and governance closure/reopening through the historical spread;
• additional export, invariant, fuzz, regression, and multi-validator E2E coverage.

The validation includes the full Go suite, four-validator unequal-power Oracle and governance scenarios, bidirectional LUNC/USTC swaps, and a process-level disk-state upgrade from the official pre-v15/v14_2 binary at 9a5ee56 to the modified MM2 binary. The legacy binary stopped at upgrade height 30, the new binary resumed on the same database, real tax routing and deferred activation completed, and both swap directions committed without a chain panic.

The detailed English evidence, findings, implemented corrections, and remaining limitations are in:
https://github.com/lunctoken/terra-classic-mm2/blob/mm2-development/docs/MM2_TEST_REPORT.md

Could you please review the differences and test evidence before #664 is finalized? Both companion PRs remain drafts and this is presented as local technical validation, not as a mainnet-ready release.

[GeoffreySHD]

MM2 (v15 market) review — head c5bf7ed

Reviewed the market module rework, the tax-redirect wiring, and the oracle hook end to end. Overall the architecture is coherent and the fund paths conserve value: offer coins enter the pool before payout, the liquidity check covers payout + fee, the fee split is floored (remainder to oracle), the daily cap is net-drainage based so round-trips net out, and the epoch burn/refill is a clean governance-funded reset. The time-based oracle freshness gate with the hook firing only on successful tallies is a real improvement over main's block-based design.

A few suggestions, roughly in priority order:

1. SwapFeeBurnRate + SwapFeeCommunityRate are not jointly validated (x/market/types/params.go). Each uses validateFraction ([0,1]), but the sum is unbounded. A governance proposal setting both to 1.0 (each individually valid) would make every swap fail at FundCommunityPool after the burn — the module account is empty at that point. Recoverable via another proposal, but it effectively disables the market, same class as the div-by-zero in ProcessTaxSplits (see #670). Recommend a cross-param check in Params.Validate() that SwapFeeBurnRate.Add(SwapFeeCommunityRate).LTE(OneDec()).

2. ComputeTWAP is a simple average, not time-weighted (x/market/keeper/keeper.go, comment in code agrees). Snapshots are per-tally, so a stretch of failed quorum (no snapshots) is silently underweighted and a burst of tallies overweights. You already store Height per snapshot — weighting each snapshot by the block interval to the next one would make the deviation guard robust to irregular tallies and is a small change.

3. Swap-rejection paths are silent. ErrTWAPDeviation, ErrDailyCapExceeded, ErrOraclePriceStale return without any event or telemetry. The tax-split path got events in this PR; the swap brakes would benefit from module-level telemetry counters so governance can tune MaxTwapDeviation / DailyCapFactor from data instead of guessing.

4. ResetDailyCapIfNeeded treats 14,400 blocks as a day (x/market/keeper/keeper.go). That assumption (3s blocks) drifts with LUNC's actual block time; MaxOracleAgeSeconds already went time-based. Either reset on wall-clock or document the block-based assumption explicitly.

5. Coordination with open PRs touching the same code:

• #670 guards a governance-reachable div-by-zero in exactly this file's sibling x/tax/keeper/tax_split.go (community-tax adjustment when communityTax=1.0, oracleSplit=0). The redirect block here lands above it without touching the formula — on rebase, keep the guarded helper so the halt path stays closed.
• #668 modifies the oracle EndBlocker where the AfterOracleTally hook call is inserted — trivial, but rebase order matters for clean CI.

6. Minor hardening: the constant-product askPool.Sub(cp.Quo(offerPool.Add(offerAmt))) in ComputeSwap (x/market/keeper/swap.go) divides by offerPool + offerAmt; with governance-settable BasePool and a deeply negative TerraPoolDelta this can reach zero. Pre-existing from main, but with pool-backed liquidity it's worth a defensive fail-swap guard rather than relying on the tx-level recover.

No fund-theft or chain-halt paths found beyond the above. Happy to turn any of 1–3 into a PR if useful.