Skip to content

Conversation

@StrathCole
Copy link
Member

Summary of changes (Copilot)

This pull request introduces the v13 upgrade to the Terra Classic blockchain application and refines the handling of tax distribution logic, especially regarding market redirection. It also updates several module integrations and improves test coverage for new tax behaviors. The most important changes are grouped below:

Upgrade Integration

  • Added the new v13 upgrade, including its constants and handler, which restricts allowed swap denominations for the market module to uusd. (app/app.go, app/upgrades/v13/constants.go, app/upgrades/v13/upgrades.go) [1] [2] [3] [4]

Tax Distribution and Market Redirect Logic

  • Refactored tax distribution logic in tests to support market redirect rates, ensuring correct allocation to fee collector, oracle, market accumulator, and community pool. Tests now explicitly set market redirect rates and validate expected balances. (custom/auth/ante/fee_test.go) [1] [2] [3] [4] [5] [6] [7] [8] [9]

Module and Keeper Integration

  • Updated module account permissions and allowed receiving accounts for the market and its accumulator, reflecting the new tax redirect logic. (app/modules.go) [1] [2]
  • Integrated custom staking and custom wasm modules in place of their standard counterparts, allowing for extended functionality. (app/modules.go) [1] [2] [3]

API and Service Changes

  • Modified the transaction service registration to include the TaxExemptionKeeper, ensuring exemption logic is properly available in transaction handling. (app/app.go, custom/auth/tx/service.go) [1] [2] [3]

Codebase Cleanup

  • Removed unused code and improved clarity in test setup and assertions, especially around fee collector and supply tracking. (custom/auth/ante/fee_test.go) [1] [2]

These updates ensure the application supports the new v13 upgrade, correctly handles tax and fee distribution (including market redirects), and maintains robust test coverage for these changes.

@StrathCole StrathCole requested a review from Copilot August 19, 2025 09:24
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces the v13 upgrade to the Terra Classic blockchain application, implementing significant changes to tax distribution and market functionality. The upgrade adds a new tax redirect mechanism that directs a portion of collected taxes to a market accumulator module, while also integrating oracle-based USD pricing for enhanced market operations.

  • Implements v13 upgrade with restricted swap denomination support to only uusd
  • Introduces tax redirect rate parameter allowing configurable percentage of taxes to flow to market accumulator
  • Adds USD pricing functionality to oracle module using meta-denom for USTC/USD conversion
  • Implements market epoch processing with burn/refill cycles for pool management

Reviewed Changes

Copilot reviewed 59 out of 61 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
app/app.go Integrates v13 upgrade handler and updates transaction service registration
app/modules.go Updates module permissions and replaces standard modules with custom implementations
app/upgrades/v13/ Defines v13 upgrade constants and logic restricting swap denoms to uusd
x/treasury/types/ Adds TaxRedirectRate parameter for market redirect functionality
x/tax/keeper/ Implements tax splitting logic with market redirect support and event emission
x/oracle/types/ Adds USD pricing queries and meta-denom support for USTC/USD rates
x/market/types/ Adds epoch processing parameters and accumulator module support
custom/auth/ Updates transaction service and fee test coverage for new tax logic

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

func validateTaxRedirectRate(i interface{}) error {
v, ok := i.(sdk.Dec)
if !ok {
return fmt.Errorf("invalid paramater type: %T", i)
Copy link

Copilot AI Aug 19, 2025

Choose a reason for hiding this comment

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

The word "paramater" is misspelled. It should be "parameter".

Suggested change
return fmt.Errorf("invalid paramater type: %T", i)
return fmt.Errorf("invalid parameter type: %T", i)

Copilot uses AI. Check for mistakes.
for ; iter.Valid(); iter.Next() {
denom := string(iter.Key()[len(types.ExchangeRateKey):])
// Skip duplicate entry if denom is uluna (already pushed)
if denom == core.MicroLunaDenom || denom == types.MetaUSDDenom || len(denom) == 0 || denom[0] != 'u' {
Copy link

Copilot AI Aug 19, 2025

Choose a reason for hiding this comment

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

The hardcoded check for denom[0] != 'u' is fragile and could fail for valid denoms. Consider using a more robust validation method or making this configurable.

Suggested change
if denom == core.MicroLunaDenom || denom == types.MetaUSDDenom || len(denom) == 0 || denom[0] != 'u' {
if denom == core.MicroLunaDenom || denom == types.MetaUSDDenom || len(denom) == 0 || sdk.ValidateDenom(denom) != nil {

Copilot uses AI. Check for mistakes.
}
}
// Deduct redirected portion from taxes before any other splits
taxes = taxes.Sub(marketSplitCoins...)
Copy link

Copilot AI Aug 19, 2025

Choose a reason for hiding this comment

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

The Sub operation could panic if marketSplitCoins contains denoms not present in taxes or if amounts are insufficient. Consider adding validation to ensure safe subtraction.

Copilot uses AI. Check for mistakes.
StrathCole and others added 24 commits August 19, 2025 11:28
…ssic-terra#588)

Co-authored-by: Tuan Tran <tropicaldog17@gmail.com>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Mergify <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: Khanh Hoa <hoa@notional.ventures>
Co-authored-by: Kien Trinh <51135161+kien6034@users.noreply.github.com>
Co-authored-by: StrathCole <strathcole@gmail.com>
Co-authored-by: DevOrbitlabs <hoank@protonmail.com>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
- 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
StrathCole and others added 30 commits September 14, 2025 11:58
fix ibc hooks ic test
change to cosmos interchaintest v10
* refactor app

* update lint

* fix lint

* fix lint

* fix lint

* update cl
* longer timeout

* fix test

* lint

* adjust

* bet
- twap and oracle vote age checks
Co-authored-by: Tuan Tran <tropicaldog17@gmail.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: DevOrbitlabs <hoa@notional.ventures>
# Conflicts:
#	go.mod
#	go.sum
#	scripts/run-node-legacy.sh
#	tests/interchaintest/go.mod
#	tests/interchaintest/go.sum
#	wasmbinding/query_plugin.go
#	wasmbinding/wasm.go
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.

5 participants