Market Module 2.0 Implementation - #664
Conversation
- e2e testing adjustments - bump to newer wasmd 0.54 - fix tests
- fix upgrade test - change legacy height
update upgrade height of multi test
- update go version to 1.24.7
There was a problem hiding this comment.
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, registerUSTmeta-denom in oracle whitelist/Tobin-tax, addmarket_accumulatormodule 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.
| oracletypes.ModuleName: true, | ||
| treasurytypes.BurnModuleName: true, | ||
| markettypes.ModuleName: true, | ||
| markettypes.AccumulatorModuleName: true, |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
It is only a virtual address, not a module. That normally is lazily created on usage.
non-determinism in map usage fixed missing tests added missing proto added missing initializations added
|
@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? |
|
Good day everyone! I tried to explore the code, but there were some difficulties with the launch and real testing. 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. Happy to discuss any of the findings. |
|
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:
The companion core PR addresses and validates several points found while exercising the implementation end to end:
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: 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. |
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
uusdby 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:
uusdand 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]app/keepers/keepers.go)Tax Redirect and Module Account Handling:
app/modules.go) [1] [2]custom/auth/ante/fee_test.go) [1] [2] [3] [4] [5]Test Improvements:
custom/auth/ante/fee_test.go) [1] [2] [3]Minor/Housekeeping:
app/app.go,app/modules.go) [1] [2] [3]cmd/terrad/root.go)