diff --git a/CHANGELOG.md b/CHANGELOG.md index a563d3695d..a188c7c7fb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -49,6 +49,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * Add upgrade handler for 1.18 [#1756](https://github.com/provenance-io/provenance/pull/1756). * Updated documentation for each module to work with docusaurus [PR 1763](https://github.com/provenance-io/provenance/pull/1763). * Create a default market in `make run`, `localnet`, `devnet` and the `provenanced testnet` command [#1757](https://github.com/provenance-io/provenance/issues/1757). +* Remove the rust upgrade handlers [PR 1774](https://github.com/provenance-io/provenance/pull/1774). ### Dependencies diff --git a/app/upgrades.go b/app/upgrades.go index 2ddebe28ae..0568160020 100644 --- a/app/upgrades.go +++ b/app/upgrades.go @@ -11,22 +11,16 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/module" banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" - govtypesv1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1" - govtypesv1beta1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1beta1" upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types" icqtypes "github.com/cosmos/ibc-apps/modules/async-icq/v6/types" transfertypes "github.com/cosmos/ibc-go/v6/modules/apps/transfer/types" - attributekeeper "github.com/provenance-io/provenance/x/attribute/keeper" - attributetypes "github.com/provenance-io/provenance/x/attribute/types" "github.com/provenance-io/provenance/x/exchange" "github.com/provenance-io/provenance/x/hold" ibchookstypes "github.com/provenance-io/provenance/x/ibchooks/types" ibcratelimit "github.com/provenance-io/provenance/x/ibcratelimit" markertypes "github.com/provenance-io/provenance/x/marker/types" - msgfeetypes "github.com/provenance-io/provenance/x/msgfees/types" oracletypes "github.com/provenance-io/provenance/x/oracle/types" - triggertypes "github.com/provenance-io/provenance/x/trigger/types" ) // appUpgrade is an internal structure for defining all things for an upgrade. @@ -54,57 +48,6 @@ type appUpgrade struct { // If something is happening in the rc upgrade(s) that isn't being applied in the non-rc, // or vice versa, please add comments explaining why in both entries. var upgrades = map[string]appUpgrade{ - "rust-rc1": { // upgrade for v1.16.0-rc1 - Handler: func(ctx sdk.Context, app *App, vm module.VersionMap) (module.VersionMap, error) { - var err error - vm, err = runModuleMigrations(ctx, app, vm) - if err != nil { - return nil, err - } - - removeInactiveValidatorDelegations(ctx, app) - - err = setAccountDataNameRecord(ctx, app.AccountKeeper, &app.NameKeeper) - if err != nil { - return nil, err - } - - // We only need to call addGovV1SubmitFee on testnet. - addGovV1SubmitFee(ctx, app) - - removeP8eMemorializeContractFee(ctx, app) - - fixNameIndexEntries(ctx, app) - - return vm, nil - }, - Added: []string{triggertypes.ModuleName}, - }, - "rust": { // upgrade for v1.16.0 - Handler: func(ctx sdk.Context, app *App, vm module.VersionMap) (module.VersionMap, error) { - var err error - vm, err = runModuleMigrations(ctx, app, vm) - if err != nil { - return nil, err - } - - removeInactiveValidatorDelegations(ctx, app) - - err = setAccountDataNameRecord(ctx, app.AccountKeeper, &app.NameKeeper) - if err != nil { - return nil, err - } - - // No need to call addGovV1SubmitFee in here as mainnet already has it defined. - - removeP8eMemorializeContractFee(ctx, app) - - fixNameIndexEntries(ctx, app) - - return vm, nil - }, - Added: []string{triggertypes.ModuleName}, - }, "saffron-rc1": { // upgrade for v1.17.0-rc1 Handler: func(ctx sdk.Context, app *App, vm module.VersionMap) (module.VersionMap, error) { var err error @@ -183,6 +126,8 @@ var upgrades = map[string]appUpgrade{ return nil, err } + removeInactiveValidatorDelegations(ctx, app) + return vm, nil }, }, @@ -195,6 +140,8 @@ var upgrades = map[string]appUpgrade{ return nil, err } + removeInactiveValidatorDelegations(ctx, app) + return vm, nil }, }, @@ -281,62 +228,6 @@ func runModuleMigrations(ctx sdk.Context, app *App, vm module.VersionMap) (modul // nor complains about a nolint:unused directive that isn't needed because the function is used. var _ = runModuleMigrations -// addGovV1SubmitFee adds a msg-fee for the gov v1 MsgSubmitProposal if there isn't one yet. -// TODO: Remove with the rust handlers. -func addGovV1SubmitFee(ctx sdk.Context, app *App) { - typeURL := sdk.MsgTypeURL(&govtypesv1.MsgSubmitProposal{}) - - ctx.Logger().Info(fmt.Sprintf("Creating message fee for %q if it doesn't already exist.", typeURL)) - // At the time of writing this, the only way GetMsgFee returns an error is if it can't unmarshall state. - // If that's the case for the v1 entry, we want to fix it anyway, so we just ignore any error here. - fee, _ := app.MsgFeesKeeper.GetMsgFee(ctx, typeURL) - // If there's already a fee for it, do nothing. - if fee != nil { - ctx.Logger().Info(fmt.Sprintf("Message fee for %q already exists with amount %q. Nothing to do.", fee.MsgTypeUrl, fee.AdditionalFee.String())) - return - } - - // Copy the fee from the beta entry if it exists, otherwise, just make it fresh. - betaTypeURL := sdk.MsgTypeURL(&govtypesv1beta1.MsgSubmitProposal{}) - // Here too, if there's an error getting the beta fee, just ignore it. - betaFee, _ := app.MsgFeesKeeper.GetMsgFee(ctx, betaTypeURL) - if betaFee != nil { - fee = betaFee - fee.MsgTypeUrl = typeURL - ctx.Logger().Info(fmt.Sprintf("Copying %q fee to %q.", betaTypeURL, fee.MsgTypeUrl)) - } else { - fee = &msgfeetypes.MsgFee{ - MsgTypeUrl: typeURL, - AdditionalFee: sdk.NewInt64Coin("nhash", 100_000_000_000), // 100 hash - Recipient: "", - RecipientBasisPoints: 0, - } - ctx.Logger().Info(fmt.Sprintf("Creating %q fee.", fee.MsgTypeUrl)) - } - - // At the time of writing this, SetMsgFee always returns nil. - _ = app.MsgFeesKeeper.SetMsgFee(ctx, *fee) - ctx.Logger().Info(fmt.Sprintf("Successfully set fee for %q with amount %q.", fee.MsgTypeUrl, fee.AdditionalFee.String())) -} - -// removeP8eMemorializeContractFee removes the message fee for the now-non-existent MsgP8eMemorializeContractRequest. -// TODO: Remove with the rust handlers. -func removeP8eMemorializeContractFee(ctx sdk.Context, app *App) { - typeURL := "/provenance.metadata.v1.MsgP8eMemorializeContractRequest" - - ctx.Logger().Info(fmt.Sprintf("Removing message fee for %q if one exists.", typeURL)) - // Get the existing fee for log output, but ignore any errors so we try to delete the entry either way. - fee, _ := app.MsgFeesKeeper.GetMsgFee(ctx, typeURL) - // At the time of writing this, the only error that RemoveMsgFee can return is ErrMsgFeeDoesNotExist. - // So ignore any error here and just use fee != nil for the different log messages. - _ = app.MsgFeesKeeper.RemoveMsgFee(ctx, typeURL) - if fee == nil { - ctx.Logger().Info(fmt.Sprintf("Message fee for %q already does not exist. Nothing to do.", typeURL)) - } else { - ctx.Logger().Info(fmt.Sprintf("Successfully removed message fee for %q with amount %q.", fee.MsgTypeUrl, fee.AdditionalFee.String())) - } -} - // removeInactiveValidatorDelegations unbonds all delegations from inactive validators, triggering their removal from the validator set. // This should be applied in most upgrades. func removeInactiveValidatorDelegations(ctx sdk.Context, app *App) { @@ -370,21 +261,6 @@ func removeInactiveValidatorDelegations(ctx sdk.Context, app *App) { ctx.Logger().Info(fmt.Sprintf("a total of %d inactive (unbonded) validators have had all their delegators removed", removalCount)) } -// fixNameIndexEntries fixes the name module's address to name index entries. -// TODO: Remove with the rust handlers. -func fixNameIndexEntries(ctx sdk.Context, app *App) { - ctx.Logger().Info("Fixing name module store index entries.") - app.NameKeeper.DeleteInvalidAddressIndexEntries(ctx) - ctx.Logger().Info("Done fixing name module store index entries.") -} - -// setAccountDataNameRecord makes sure the account data name record exists, is restricted, -// and is owned by the attribute module. An error is returned if it fails to make it so. -// TODO: Remove with the rust handlers. -func setAccountDataNameRecord(ctx sdk.Context, accountK attributetypes.AccountKeeper, nameK attributetypes.NameKeeper) (err error) { - return attributekeeper.EnsureModuleAccountAndAccountDataNameRecord(ctx, accountK, nameK) -} - // setupICQ sets the correct default values for ICQKeeper. // TODO: Remove with the saffron handlers. func setupICQ(ctx sdk.Context, app *App) { @@ -405,6 +281,7 @@ func updateMaxSupply(ctx sdk.Context, app *App) { } // addMarkerNavs adds navs to existing markers +// TODO: Remove with the saffron handlers. func addMarkerNavs(ctx sdk.Context, app *App, denomToNav map[string]markertypes.NetAssetValue) { ctx.Logger().Info("Adding marker net asset values") for denom, nav := range denomToNav { diff --git a/app/upgrades_test.go b/app/upgrades_test.go index 868ae306a4..c82ee3f658 100644 --- a/app/upgrades_test.go +++ b/app/upgrades_test.go @@ -2,7 +2,6 @@ package app import ( "bytes" - "errors" "fmt" "regexp" "sort" @@ -26,7 +25,6 @@ import ( "github.com/provenance-io/provenance/x/exchange" markertypes "github.com/provenance-io/provenance/x/marker/types" - msgfeetypes "github.com/provenance-io/provenance/x/msgfees/types" ) type UpgradeTestSuite struct { @@ -382,41 +380,6 @@ func (s *UpgradeTestSuite) TestKeysInHandlersMap() { }) } -func (s *UpgradeTestSuite) TestRustRC1() { - // Each part is (hopefully) tested thoroughly on its own. - // So for this test, just make sure there's log entries for each part being done. - - expInLog := []string{ - "INF Starting module migrations. This may take a significant amount of time to complete. Do not restart node.", - "INF removing all delegations from validators that have been inactive (unbonded) for 21 days", - `INF Setting "accountdata" name record.`, - `INF Creating message fee for "/cosmos.gov.v1.MsgSubmitProposal" if it doesn't already exist.`, - `INF Removing message fee for "/provenance.metadata.v1.MsgP8eMemorializeContractRequest" if one exists.`, - "INF Fixing name module store index entries.", - } - - s.AssertUpgradeHandlerLogs("rust-rc1", expInLog, nil) -} - -func (s *UpgradeTestSuite) TestRust() { - // Each part is (hopefully) tested thoroughly on its own. - // So for this test, just make sure there's log entries for each part being done. - // And not a log entry for stuff done in rust-rc1 but not this one. - - expInLog := []string{ - "INF Starting module migrations. This may take a significant amount of time to complete. Do not restart node.", - "INF removing all delegations from validators that have been inactive (unbonded) for 21 days", - `INF Setting "accountdata" name record.`, - `INF Removing message fee for "/provenance.metadata.v1.MsgP8eMemorializeContractRequest" if one exists.`, - "INF Fixing name module store index entries.", - } - expNotInLog := []string{ - `Creating message fee for "/cosmos.gov.v1.MsgSubmitProposal" if it doesn't already exist.`, - } - - s.AssertUpgradeHandlerLogs("rust", expInLog, expNotInLog) -} - func (s *UpgradeTestSuite) TestSaffronRC1() { // Each part is (hopefully) tested thoroughly on its own. // So for this test, just make sure there's log entries for each part being done. @@ -688,201 +651,6 @@ func (s *UpgradeTestSuite) TestRemoveInactiveValidatorDelegations() { }) } -func (s *UpgradeTestSuite) TestAddGovV1SubmitFee() { - v1TypeURL := "/cosmos.gov.v1.MsgSubmitProposal" - v1B1TypeURL := "/cosmos.gov.v1beta1.MsgSubmitProposal" - - startingMsg := `INF Creating message fee for "` + v1TypeURL + `" if it doesn't already exist.` - successMsg := func(amt string) string { - return `INF Successfully set fee for "` + v1TypeURL + `" with amount "` + amt + `".` - } - - coin := func(denom string, amt int64) *sdk.Coin { - rv := sdk.NewInt64Coin(denom, amt) - return &rv - } - - tests := []struct { - name string - v1Amt *sdk.Coin - v1B1Amt *sdk.Coin - expInLog []string - expAmt sdk.Coin - }{ - { - name: "v1 fee already exists", - v1Amt: coin("foocoin", 88), - v1B1Amt: coin("betacoin", 99), - expInLog: []string{ - startingMsg, - `INF Message fee for "` + v1TypeURL + `" already exists with amount "88foocoin". Nothing to do.`, - }, - expAmt: *coin("foocoin", 88), - }, - { - name: "v1beta1 exists", - v1B1Amt: coin("betacoin", 99), - expInLog: []string{ - startingMsg, - `INF Copying "` + v1B1TypeURL + `" fee to "` + v1TypeURL + `".`, - successMsg("99betacoin"), - }, - expAmt: *coin("betacoin", 99), - }, - { - name: "brand new", - expInLog: []string{ - startingMsg, - `INF Creating "` + v1TypeURL + `" fee.`, - successMsg("100000000000nhash"), - }, - expAmt: *coin("nhash", 100_000_000_000), - }, - } - - for _, tc := range tests { - s.Run(tc.name, func() { - // Set/unset the v1 fee. - if tc.v1Amt != nil { - fee := msgfeetypes.NewMsgFee(v1TypeURL, *tc.v1Amt, "", 0) - s.Require().NoError(s.app.MsgFeesKeeper.SetMsgFee(s.ctx, fee), "SetMsgFee v1") - } else { - err := s.app.MsgFeesKeeper.RemoveMsgFee(s.ctx, v1TypeURL) - if err != nil && !errors.Is(err, msgfeetypes.ErrMsgFeeDoesNotExist) { - s.Require().NoError(err, "RemoveMsgFee v1") - } - } - - // Set/unset the v1beta1 fee. - if tc.v1B1Amt != nil { - fee := msgfeetypes.NewMsgFee(v1B1TypeURL, *tc.v1B1Amt, "", 0) - s.Require().NoError(s.app.MsgFeesKeeper.SetMsgFee(s.ctx, fee), "SetMsgFee v1beta1") - } else { - err := s.app.MsgFeesKeeper.RemoveMsgFee(s.ctx, v1B1TypeURL) - if err != nil && !errors.Is(err, msgfeetypes.ErrMsgFeeDoesNotExist) { - s.Require().NoError(err, "RemoveMsgFee v1") - } - } - - // Reset the log buffer to clear out unrelated entries. - s.logBuffer.Reset() - // Call addGovV1SubmitFee and relog its output (to help if things fail). - testFunc := func() { - addGovV1SubmitFee(s.ctx, s.app) - } - didNotPanic := s.Assert().NotPanics(testFunc, "addGovV1SubmitFee") - logOutput := s.GetLogOutput("addGovV1SubmitFee") - if !didNotPanic { - return - } - - // Make sure the log has the expected lines. - s.AssertLogContents(logOutput, tc.expInLog, nil, true, "addGovV1SubmitFee") - - // Get the fee and make sure it's now as expected. - fee, err := s.app.MsgFeesKeeper.GetMsgFee(s.ctx, v1TypeURL) - s.Require().NoError(err, "GetMsgFee(%q) error", v1TypeURL) - s.Require().NotNil(fee, "GetMsgFee(%q) value", v1TypeURL) - actFeeAmt := fee.AdditionalFee - s.Assert().Equal(tc.expAmt.String(), actFeeAmt.String(), "final %s fee amount", v1TypeURL) - }) - } -} - -func (s *UpgradeTestSuite) TestRemoveP8eMemorializeContractFee() { - typeURL := "/provenance.metadata.v1.MsgP8eMemorializeContractRequest" - startingMsg := `INF Removing message fee for "` + typeURL + `" if one exists.` - - coin := func(denom string, amt int64) *sdk.Coin { - rv := sdk.NewInt64Coin(denom, amt) - return &rv - } - - tests := []struct { - name string - amt *sdk.Coin - expInLog []string - }{ - { - name: "does not exist", - expInLog: []string{ - startingMsg, - `INF Message fee for "` + typeURL + `" already does not exist. Nothing to do.`, - }, - }, - { - name: "exists", - amt: coin("p8ecoin", 808), - expInLog: []string{ - startingMsg, - `INF Successfully removed message fee for "` + typeURL + `" with amount "808p8ecoin".`, - }, - }, - } - - for _, tc := range tests { - s.Run(tc.name, func() { - // set/unset the fee - if tc.amt != nil { - fee := msgfeetypes.NewMsgFee(typeURL, *tc.amt, "", 0) - s.Require().NoError(s.app.MsgFeesKeeper.SetMsgFee(s.ctx, fee), "Setup: SetMsgFee(%q)", typeURL) - } else { - err := s.app.MsgFeesKeeper.RemoveMsgFee(s.ctx, typeURL) - if err != nil && !errors.Is(err, msgfeetypes.ErrMsgFeeDoesNotExist) { - s.Require().NoError(err, "Setup: RemoveMsgFee(%q)", typeURL) - } - } - - // Reset the log buffer to clear out unrelated entries. - s.logBuffer.Reset() - // Call removeP8eMemorializeContractFee and relog its output (to help if things fail). - testFunc := func() { - removeP8eMemorializeContractFee(s.ctx, s.app) - } - didNotPanic := s.Assert().NotPanics(testFunc, "removeP8eMemorializeContractFee") - logOutput := s.GetLogOutput("removeP8eMemorializeContractFee") - if !didNotPanic { - return - } - - s.AssertLogContents(logOutput, tc.expInLog, nil, true, "removeP8eMemorializeContractFee") - - // Make sure there isn't a fee anymore. - fee, err := s.app.MsgFeesKeeper.GetMsgFee(s.ctx, typeURL) - s.Require().NoError(err, "GetMsgFee(%q) error", typeURL) - s.Require().Nil(fee, "GetMsgFee(%q) value", typeURL) - }) - } -} - -func (s *UpgradeTestSuite) TestSetAccountDataNameRecord() { - // Most of the testing should (hopefully) be done in the name module. So this is - // just a superficial test that makes sure it's doing something. - // Since the name is also created during InitGenesis, it should already be set up as needed. - // So in this unit test, the logs will indicate that. - expInLog := []string{ - `INF Setting "accountdata" name record.`, - `INF The "accountdata" name record already exists as needed. Nothing to do.`, - } - // During an actual upgrade, that last line would instead be this: - // `INF Successfully set "accountdata" name record.` - - // Reset the log buffer to clear out unrelated entries. - s.logBuffer.Reset() - // Call setAccountDataNameRecord and relog its output (to help if things fail). - var err error - testFunc := func() { - err = setAccountDataNameRecord(s.ctx, s.app.AccountKeeper, &s.app.NameKeeper) - } - didNotPanic := s.Assert().NotPanics(testFunc, "setAccountDataNameRecord") - logOutput := s.GetLogOutput("setAccountDataNameRecord") - if !didNotPanic { - return - } - s.Require().NoError(err, "setAccountDataNameRecord") - s.AssertLogContents(logOutput, expInLog, nil, true, "setAccountDataNameRecord") -} - func (s *UpgradeTestSuite) TestAddMarkerNavs() { address1 := sdk.AccAddress("address1") testcoin := markertypes.NewEmptyMarkerAccount("testcoin",