Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: cleanup unused globals #8804

Closed
wants to merge 2 commits into from
+11 −26
Closed

chore: cleanup unused globals #8804

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e6316ea
chore: cleanup unused globals
robert-zaremba Oct 27, 2024
1c0375f
changelog
robert-zaremba Nov 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,13 +45,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### State Breaking

* [#8682](https://github.com/osmosis-labs/osmosis/pull/8682) chore: bump cosmwasm-optimizer
* [#8732](https://github.com/osmosis-labs/osmosis/pull/8732) fix: iterate delegations continue instead of erroring
* [#8734](https://github.com/osmosis-labs/osmosis/pull/8734) chore: update cosmwasm vm
* [#8777](https://github.com/osmosis-labs/osmosis/pull/8777) fix: state export for gov module constitution
* [#8751](https://github.com/osmosis-labs/osmosis/pull/8751) fix: supply offsets for osmo token
* [#8764](https://github.com/osmosis-labs/osmosis/pull/8764) chore: add cosmwasm 1_3 feature
* [#8779](https://github.com/osmosis-labs/osmosis/pull/8779) chore: bump cometbft/cosmos-sdk versions
* [#8801](https://github.com/osmosis-labs/osmosis/pull/8801) chore: update tagged submodules for v27

### API

* [#8804](https://github.com/osmosis-labs/osmosis/pull/8804) chore: cleanup `app` package unused globals: `WasmProposalsEnabled`, `EnableSpecificWasmProposals`, `EmptyWasmOpts`.

### Config


Expand All @@ -68,10 +73,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
* [#8765](https://github.com/osmosis-labs/osmosis/pull/8765) fix concurrency issue in go test(x/lockup)
* [#8791](https://github.com/osmosis-labs/osmosis/pull/8791) fix: superfluid log for error that should be ignored

### State Machine Breaking

* [#8732](https://github.com/osmosis-labs/osmosis/pull/8732) fix: iterate delegations continue instead of erroring

Comment on lines -71 to -74
Copy link
Member

Choose a reason for hiding this comment

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

I think this was mistakenly removed?

## v26.0.1

### State Machine Breaking
Expand Down
16 changes: 0 additions & 16 deletions app/app.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,22 +175,6 @@ var (
// module accounts that are allowed to receive tokens.
allowedReceivingModAcc = map[string]bool{protorevtypes.ModuleName: true}

// TODO: Refactor wasm items into a wasm.go file
// WasmProposalsEnabled enables all x/wasm proposals when it's value is "true"
// and EnableSpecificWasmProposals is empty. Otherwise, all x/wasm proposals
// are disabled.
WasmProposalsEnabled = "true"

// EnableSpecificWasmProposals, if set, must be comma-separated list of values
// that are all a subset of "EnableAllProposals", which takes precedence over
// WasmProposalsEnabled.
//
// See: https://github.com/CosmWasm/wasmd/blob/02a54d33ff2c064f3539ae12d75d027d9c665f05/x/wasm/internal/types/proposal.go#L28-L34
EnableSpecificWasmProposals = ""

// EmptyWasmOpts defines a type alias for a list of wasm options.
EmptyWasmOpts []wasmkeeper.Option

_ runtime.AppI = (*OsmosisApp)(nil)

Upgrades = []upgrades.Upgrade{v4.Upgrade, v5.Upgrade, v7.Upgrade, v9.Upgrade, v11.Upgrade, v12.Upgrade, v13.Upgrade, v14.Upgrade, v15.Upgrade, v16.Upgrade, v17.Upgrade, v18.Upgrade, v19.Upgrade, v20.Upgrade, v21.Upgrade, v22.Upgrade, v23.Upgrade, v24.Upgrade, v25.Upgrade, v26.Upgrade, v27.Upgrade}
Expand Down
2 changes: 1 addition & 1 deletion app/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func NewAppConstructor(chainId string) network.AppConstructor {
return NewOsmosisApp(
valCtx.Logger, dbm.NewMemDB(), nil, true, make(map[int64]bool), valCtx.Config.RootDir, 0,
sims.EmptyAppOptions{},
EmptyWasmOpts,
nil,
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Inconsistent usage of EmptyWasmOpts found in cmd/osmosisd/cmd/root.go

The change from EmptyWasmOpts to nil is not consistently applied across the codebase. While test files and app/config.go correctly use nil, there's still a reference using EmptyWasmOpts in:

  • cmd/osmosisd/cmd/root.go: Still using osmosis.EmptyWasmOpts in NewOsmosisApp call
🔗 Analysis chain

LGTM! Verify consistent usage across the codebase.

The change from EmptyWasmOpts to nil aligns with the PR objectives of cleaning up unused globals.

Let's verify that this change is consistent across the codebase and that all callers properly handle nil:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that EmptyWasmOpts is completely removed and all NewOsmosisApp calls use nil

# Test 1: Ensure EmptyWasmOpts is removed
rg "EmptyWasmOpts"

# Test 2: Check all NewOsmosisApp calls to ensure they use nil
ast-grep --pattern 'NewOsmosisApp($$$)'

Length of output: 1288

baseapp.SetMinGasPrices(appConfig.MinGasPrices),
baseapp.SetChainID(chainId),
)
Expand Down
2 changes: 1 addition & 1 deletion app/modules_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import (

func TestOrderEndBlockers_Determinism(t *testing.T) {
db := dbm.NewMemDB()
app := NewOsmosisApp(log.NewNopLogger(), db, nil, true, map[int64]bool{}, DefaultNodeHome, 5, sims.EmptyAppOptions{}, EmptyWasmOpts, baseapp.SetChainID("osmosis-1"))
app := NewOsmosisApp(log.NewNopLogger(), db, nil, true, map[int64]bool{}, DefaultNodeHome, 5, sims.EmptyAppOptions{}, nil, baseapp.SetChainID("osmosis-1"))
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Inconsistent usage found in cmd/osmosisd/cmd/root.go

The change from EmptyWasmOpts to nil is not consistently applied across the codebase. While most NewOsmosisApp calls use nil, there's still one instance in cmd/osmosisd/cmd/root.go that uses EmptyWasmOpts.

  • cmd/osmosisd/cmd/root.go: Still using EmptyWasmOpts instead of nil
app := osmosis.NewOsmosisApp(logger, db, traceStore, loadLatest, map[int64]bool{}, homeDir, 0, appOpts, osmosis.EmptyWasmOpts)
🔗 Analysis chain

LGTM! Verify consistency across the codebase.

The change from EmptyWasmOpts to nil aligns with the PR objective of cleaning up unused globals.

Let's verify that this change is consistent across all NewOsmosisApp calls:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for any remaining instances of EmptyWasmOpts
# to ensure consistent replacement with nil

# Search for any remaining EmptyWasmOpts usage
rg "EmptyWasmOpts"

# Search for NewOsmosisApp calls to verify nil usage
ast-grep --pattern 'NewOsmosisApp($$$)'

Length of output: 1288


for i := 0; i < 1000; i++ {
a := OrderEndBlockers(app.mm.ModuleNames())
Expand Down
4 changes: 2 additions & 2 deletions app/test_helpers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ func SetupWithCustomHome(isCheckTx bool, dir string) *OsmosisApp {

func SetupWithCustomHomeAndChainId(isCheckTx bool, dir, chainId string) *OsmosisApp {
db := cosmosdb.NewMemDB()
app := NewOsmosisApp(log.NewNopLogger(), db, nil, true, map[int64]bool{}, dir, 0, sims.EmptyAppOptions{}, EmptyWasmOpts, baseapp.SetChainID(chainId))
app := NewOsmosisApp(log.NewNopLogger(), db, nil, true, map[int64]bool{}, dir, 0, sims.EmptyAppOptions{}, nil, baseapp.SetChainID(chainId))
if !isCheckTx {
if len(defaultGenesisStatebytes) == 0 {
var err error
Expand Down Expand Up @@ -167,7 +167,7 @@ func SetupTestingAppWithLevelDb(isCheckTx bool) (app *OsmosisApp, cleanupFn func
panic(err)
}

app = NewOsmosisApp(log.NewNopLogger(), db, nil, true, map[int64]bool{}, DefaultNodeHome, 5, sims.EmptyAppOptions{}, EmptyWasmOpts, baseapp.SetChainID("osmosis-1"))
app = NewOsmosisApp(log.NewNopLogger(), db, nil, true, map[int64]bool{}, DefaultNodeHome, 5, sims.EmptyAppOptions{}, nil, baseapp.SetChainID("osmosis-1"))
if !isCheckTx {
genesisState := GenesisStateWithValSet(app)
stateBytes, err := json.MarshalIndent(genesisState, "", " ")
Expand Down
2 changes: 1 addition & 1 deletion cmd/osmosisd/cmd/root.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ func NewRootCmd() (*cobra.Command, params.EncodingConfig) {
WithViper("OSMOSIS")

tempDir := tempDir()
tempApp := osmosis.NewOsmosisApp(log.NewNopLogger(), cosmosdb.NewMemDB(), nil, true, map[int64]bool{}, tempDir, 5, sims.EmptyAppOptions{}, osmosis.EmptyWasmOpts, baseapp.SetChainID("osmosis-1"))
tempApp := osmosis.NewOsmosisApp(log.NewNopLogger(), cosmosdb.NewMemDB(), nil, true, map[int64]bool{}, tempDir, 5, sims.EmptyAppOptions{}, nil, baseapp.SetChainID("osmosis-1"))
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Inconsistent usage found: EmptyWasmOpts still present in root.go

The verification revealed that EmptyWasmOpts is still being used in cmd/osmosisd/cmd/root.go while other instances of NewOsmosisApp consistently use nil. This inconsistency needs to be addressed.

  • cmd/osmosisd/cmd/root.go: Still uses EmptyWasmOpts in the app initialization
  • Other files (app/test_helpers.go, app/modules_test.go, app/config.go): Correctly use nil
🔗 Analysis chain

LGTM! Verify consistent usage of nil for Wasm options.

The change from EmptyWasmOpts to nil for the temporary app initialization is safe and aligns with the PR's objective of cleaning up unused Wasm globals.

Let's verify that this change is consistently applied across the codebase:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for any remaining usage of EmptyWasmOpts
rg "EmptyWasmOpts"

# Search for NewOsmosisApp calls to verify nil usage pattern
ast-grep --pattern 'NewOsmosisApp($$$, $_, $$$)'

Length of output: 1297

Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Inconsistent usage of EmptyWasmOpts found in the codebase

There's still one instance in cmd/osmosisd/cmd/root.go using osmosis.EmptyWasmOpts while other instances use nil:

  • Line 361: NewOsmosisApp(..., nil)
  • Another call in the same file: NewOsmosisApp(..., osmosis.EmptyWasmOpts)

This inconsistency should be addressed by replacing EmptyWasmOpts with nil in all instances to maintain consistency with the PR's objective of removing unused Wasm-related globals.

🔗 Analysis chain

LGTM! Verify consistency of nil usage.

The replacement of EmptyWasmOpts with nil aligns with the PR objective of removing unused Wasm-related globals. This is a cleaner approach when no specific Wasm options are needed.

Let's verify that this change is consistent across the codebase:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for any remaining usage of EmptyWasmOpts
rg "EmptyWasmOpts"

# Search for NewOsmosisApp calls to ensure consistent nil usage
rg "NewOsmosisApp.*\(" -A 1

Length of output: 1895

defer func() {
if err := tempApp.Close(); err != nil {
panic(err)
Expand Down
2 changes: 1 addition & 1 deletion tests/simulator/osmosis_helper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ func OsmosisAppCreator(logger log.Logger, db db.DB) simtypes.AppCreator {
homepath,
legacyInvariantPeriod,
emptyAppOptions{},
app.EmptyWasmOpts,
nil,
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Found inconsistency: EmptyWasmOpts is still used in cmd/osmosisd/cmd/root.go

The change to use nil instead of EmptyWasmOpts is incomplete. While most of the codebase has been updated to use nil, there's still one instance in cmd/osmosisd/cmd/root.go that uses EmptyWasmOpts, which needs to be updated for consistency.

  • cmd/osmosisd/cmd/root.go: Replace osmosis.EmptyWasmOpts with nil to maintain consistency with other changes
🔗 Analysis chain

LGTM! The change aligns with the cleanup objectives.

The replacement of app.EmptyWasmOpts with nil is consistent with the PR's goal of removing unused globals and simplifies the code by clearly indicating "no options" rather than maintaining an empty options constant.

Let's verify that this change is consistent across the codebase:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that EmptyWasmOpts is not used anywhere else in the codebase
# and that NewOsmosisApp calls consistently use nil for this parameter

# Test 1: Check for any remaining usage of EmptyWasmOpts
rg "EmptyWasmOpts"

# Test 2: Check the pattern of NewOsmosisApp usage
ast-grep --pattern 'NewOsmosisApp($$$)'

Length of output: 1288

baseappOptions...)
}
}
Expand Down