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

fix: upgrade fw & vuln #107

Merged
merged 2 commits into from
Nov 25, 2024
Merged

fix: upgrade fw & vuln #107

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
4074bc6
fix: upgrade fw
fmorency Nov 25, 2024
af83190
fix: GO-2024-3279
fmorency Nov 25, 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
8 changes: 2 additions & 6 deletions app/upgrades.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,14 @@ import (

"github.com/liftedinit/manifest-ledger/app/upgrades"
"github.com/liftedinit/manifest-ledger/app/upgrades/next"
"github.com/liftedinit/manifest-ledger/app/upgrades/noop"
)

// Upgrades list of chain upgrades
var Upgrades = []upgrades.Upgrade{next.NewUpgrade()}
var Upgrades []upgrades.Upgrade

// RegisterUpgradeHandlers registers the chain upgrade handlers
func (app *ManifestApp) RegisterUpgradeHandlers() {
if len(Upgrades) == 0 {
// always have a unique upgrade registered for the current version to test in system tests
Upgrades = append(Upgrades, noop.NewUpgrade(app.Version()))
}
Upgrades = append(Upgrades, next.NewUpgrade(app.Version()))
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

⚠️ Potential issue

Potential duplicate upgrades and version handling concerns

The unconditional append of upgrades could lead to duplicate entries if RegisterUpgradeHandlers is called multiple times. Additionally, using app.Version() directly for upgrade naming needs careful consideration.

Consider these improvements:

  1. Add a check to prevent duplicate upgrades
  2. Validate version format before using it as an upgrade name

Consider applying this improvement:

-	Upgrades = append(Upgrades, next.NewUpgrade(app.Version()))
+	version := app.Version()
+	// Check for existing upgrade with this version
+	for _, upgrade := range Upgrades {
+		if upgrade.UpgradeName == version {
+			return
+		}
+	}
+	Upgrades = append(Upgrades, next.NewUpgrade(version))
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
Upgrades = append(Upgrades, next.NewUpgrade(app.Version()))
version := app.Version()
// Check for existing upgrade with this version
for _, upgrade := range Upgrades {
if upgrade.UpgradeName == version {
return
}
}
Upgrades = append(Upgrades, next.NewUpgrade(version))


keepers := upgrades.AppKeepers{AccountKeeper: app.AccountKeeper, BankKeeper: app.BankKeeper}
// register all upgrade handlers
Expand Down
30 changes: 3 additions & 27 deletions app/upgrades/next/upgrades.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,13 @@ import (
upgradetypes "cosmossdk.io/x/upgrade/types"

"github.com/cosmos/cosmos-sdk/types/module"
banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"

"github.com/liftedinit/manifest-ledger/app/upgrades"
)

func NewUpgrade() upgrades.Upgrade {
func NewUpgrade(name string) upgrades.Upgrade {
return upgrades.Upgrade{
UpgradeName: "umfx-denom-metadata",
UpgradeName: name,
CreateUpgradeHandler: CreateUpgradeHandler,
StoreUpgrades: storetypes.StoreUpgrades{
Added: []string{},
Expand All @@ -26,32 +25,9 @@ func NewUpgrade() upgrades.Upgrade {
func CreateUpgradeHandler(
mm *module.Manager,
configurator module.Configurator,
keepers *upgrades.AppKeepers,
_ *upgrades.AppKeepers,
) upgradetypes.UpgradeHandler {
return func(ctx context.Context, _ upgradetypes.Plan, fromVM module.VersionMap) (module.VersionMap, error) {
metadata := banktypes.Metadata{
Description: "The Manifest Network token",
DenomUnits: []*banktypes.DenomUnit{
{
Denom: "umfx",
Exponent: 0,
Aliases: []string{},
},
{
Denom: "mfx",
Exponent: 6,
Aliases: []string{},
},
},
Base: "umfx",
Display: "MFX",
Symbol: "MFX",
Name: "Manifest Network Token",
}

// Set the new metadata in the bank keeper
keepers.BankKeeper.SetDenomMetaData(ctx, metadata)

return mm.RunMigrations(ctx, configurator, fromVM)
}
}
34 changes: 0 additions & 34 deletions app/upgrades/noop/upgrades.go
View file
Open in desktop

This file was deleted.

2 changes: 1 addition & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ require (
cosmossdk.io/depinject v1.0.0
cosmossdk.io/errors v1.0.1
cosmossdk.io/log v1.3.1
cosmossdk.io/math v1.3.0
cosmossdk.io/math v1.4.0
cosmossdk.io/store v1.1.0
cosmossdk.io/tools/confix v0.1.1
cosmossdk.io/x/circuit v0.1.1
Expand Down
4 changes: 2 additions & 2 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,8 +200,8 @@ cosmossdk.io/errors v1.0.1 h1:bzu+Kcr0kS/1DuPBtUFdWjzLqyUuCiyHjyJB6srBV/0=
cosmossdk.io/errors v1.0.1/go.mod h1:MeelVSZThMi4bEakzhhhE/CKqVv3nOJDA25bIqRDu/U=
cosmossdk.io/log v1.3.1 h1:UZx8nWIkfbbNEWusZqzAx3ZGvu54TZacWib3EzUYmGI=
cosmossdk.io/log v1.3.1/go.mod h1:2/dIomt8mKdk6vl3OWJcPk2be3pGOS8OQaLUM/3/tCM=
cosmossdk.io/math v1.3.0 h1:RC+jryuKeytIiictDslBP9i1fhkVm6ZDmZEoNP316zE=
cosmossdk.io/math v1.3.0/go.mod h1:vnRTxewy+M7BtXBNFybkuhSH4WfedVAAnERHgVFhp3k=
cosmossdk.io/math v1.4.0 h1:XbgExXFnXmF/CccPPEto40gOO7FpWu9yWNAZPN3nkNQ=
cosmossdk.io/math v1.4.0/go.mod h1:O5PkD4apz2jZs4zqFdTr16e1dcaQCc5z6lkEnrrppuk=
cosmossdk.io/store v1.1.0 h1:LnKwgYMc9BInn9PhpTFEQVbL9UK475G2H911CGGnWHk=
cosmossdk.io/store v1.1.0/go.mod h1:oZfW/4Fc/zYqu3JmQcQdUJ3fqu5vnYTn3LZFFy8P8ng=
cosmossdk.io/tools/confix v0.1.1 h1:aexyRv9+y15veH3Qw16lxQwo+ki7r2I+g0yNTEFEQM8=
Expand Down
2 changes: 1 addition & 1 deletion interchaintest/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ replace (
)

require (
cosmossdk.io/math v1.3.0
cosmossdk.io/math v1.4.0
cosmossdk.io/x/upgrade v0.1.2
github.com/cockroachdb/errors v1.11.3
github.com/cosmos/cosmos-sdk v0.50.8
Expand Down
4 changes: 2 additions & 2 deletions interchaintest/go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,8 +200,8 @@ cosmossdk.io/errors v1.0.1 h1:bzu+Kcr0kS/1DuPBtUFdWjzLqyUuCiyHjyJB6srBV/0=
cosmossdk.io/errors v1.0.1/go.mod h1:MeelVSZThMi4bEakzhhhE/CKqVv3nOJDA25bIqRDu/U=
cosmossdk.io/log v1.3.1 h1:UZx8nWIkfbbNEWusZqzAx3ZGvu54TZacWib3EzUYmGI=
cosmossdk.io/log v1.3.1/go.mod h1:2/dIomt8mKdk6vl3OWJcPk2be3pGOS8OQaLUM/3/tCM=
cosmossdk.io/math v1.3.0 h1:RC+jryuKeytIiictDslBP9i1fhkVm6ZDmZEoNP316zE=
cosmossdk.io/math v1.3.0/go.mod h1:vnRTxewy+M7BtXBNFybkuhSH4WfedVAAnERHgVFhp3k=
cosmossdk.io/math v1.4.0 h1:XbgExXFnXmF/CccPPEto40gOO7FpWu9yWNAZPN3nkNQ=
cosmossdk.io/math v1.4.0/go.mod h1:O5PkD4apz2jZs4zqFdTr16e1dcaQCc5z6lkEnrrppuk=
cosmossdk.io/store v1.1.0 h1:LnKwgYMc9BInn9PhpTFEQVbL9UK475G2H911CGGnWHk=
cosmossdk.io/store v1.1.0/go.mod h1:oZfW/4Fc/zYqu3JmQcQdUJ3fqu5vnYTn3LZFFy8P8ng=
cosmossdk.io/x/circuit v0.1.1 h1:KPJCnLChWrxD4jLwUiuQaf5mFD/1m7Omyo7oooefBVQ=
Expand Down
Loading