requested changes

app/upgrades/types.go

@@ -1,19 +1,24 @@
 package upgrades
 
 import (
-	"time"
-
 	storetypes "cosmossdk.io/store/types"
 	upgradetypes "cosmossdk.io/x/upgrade/types"
-	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/cosmos/cosmos-sdk/types/module"
-	"github.com/cosmos/cosmos-sdk/x/authz"
 
 	"github.com/pokt-network/poktroll/app/keepers"
 )
 
-var testNetPnfAddress = "pokt1r6ja6rz6rpae58njfrsgs5n5sp3r36r2q9j04h"
-var testNetAuthorityAddress = "pokt10d07y265gmmuvt4z0w9aw880jnsr700j8yv32t"
+// TODO_CONSIDER: different networks should have the same gov module address, but might have different DAO addresses,
+// unless we specifically write in these addresses in the genesis file.
+// Should we use the same address/wallet for DAO or find a way to detect the network the upgrade is being applied to,
+// to pick different addresses depending on the name of the network? (e.g chain-id)
+
+const (
+	// The default PNF/DAO address in the genesis file for Alpha TestNet. Used to create new authz authorizations.
+	AlphaTestNetPnfAddress = "pokt1r6ja6rz6rpae58njfrsgs5n5sp3r36r2q9j04h"
+	// Authority address. Defaults to gov module address. Used to create new authz authorizations.
+	AlphaTestNetAuthorityAddress = "pokt10d07y265gmmuvt4z0w9aw880jnsr700j8yv32t"
+)
 
 // Upgrade represents a protocol upgrade in code.
 // Once a `MsgSoftwareUpgrade` is submitted on-chain, and `Upgrade.PlanName` matches the `Plan.Name`,
@@ -29,28 +34,3 @@ type Upgrade struct {
 	// StoreUpgrades adds, renames and deletes KVStores in the state to prepare for a protocol upgrade.
 	StoreUpgrades storetypes.StoreUpgrades
 }
-
-type grantAuthorization struct {
-	grantee       sdk.AccAddress
-	granter       sdk.AccAddress
-	authorization authz.Authorization
-	expiration    *time.Time
-}
-
-func newTestNetGrantAuthorization(msg string) grantAuthorization {
-	authorization := authz.NewGenericAuthorization(msg)
-	expiration, err := time.Parse(time.RFC3339, "2500-01-01T00:00:00Z")
-	if err != nil {
-		panic(err)
-	}
-	err = authorization.ValidateBasic()
-	if err != nil {
-		panic(err)
-	}
-	return grantAuthorization{
-		grantee:       sdk.MustAccAddressFromBech32(testNetAuthorityAddress),
-		granter:       sdk.MustAccAddressFromBech32(testNetPnfAddress),
-		authorization: authorization,
-		expiration:    &expiration,
-	}
-}

app/upgrades/v0.0.10.go

@@ -2,38 +2,41 @@ package upgrades
 
 import (
 	"context"
+	"fmt"
+	"time"
 
 	"cosmossdk.io/math"
 	storetypes "cosmossdk.io/store/types"
 	upgradetypes "cosmossdk.io/x/upgrade/types"
 	cosmosTypes "github.com/cosmos/cosmos-sdk/types"
 	"github.com/cosmos/cosmos-sdk/types/module"
+	"github.com/cosmos/cosmos-sdk/x/authz"
 	"github.com/pokt-network/poktroll/app/keepers"
 	prooftypes "github.com/pokt-network/poktroll/x/proof/types"
 )
 
 // Upgrade_0_0_10 is the upgrade handler for v0.0.10 Alpha TestNet upgrade
-// Before/after validations should be done using the correct version (e.g. before - v0.0.9, after - v0.0.10)
+// Before/after validations should be done using the correct version, mimiching real-world scenario:
+//   - Before: v0.0.9
+//   - After: v0.0.10
 var Upgrade_0_0_10 = Upgrade{
 	PlanName: "v0.0.10",
 	CreateUpgradeHandler: func(mm *module.Manager,
 		keepers *keepers.Keepers,
 		configurator module.Configurator,
 	) upgradetypes.UpgradeHandler {
-		return func(ctx context.Context, plan upgradetypes.Plan, vm module.VersionMap) (module.VersionMap, error) {
-			//
-			// Add missing parameters and changes from `config.yml`
-			// https://github.com/pokt-network/poktroll/compare/v0.0.9-3...ff76430
-			//
 
+		// Adds new parameters using ignite's config.yml as a reference. Assuming we don't need any other parameters.
+		// https://github.com/pokt-network/poktroll/compare/v0.0.9-3...ff76430
+		applyNewParameters := func(ctx context.Context) (err error) {
 			// Add application min stake
 			// Validate with: `poktrolld q application params --node=https://testnet-validated-validator-rpc.poktroll.com/`
 			appParams := keepers.ApplicationKeeper.GetParams(ctx)
 			newMinStakeApp := cosmosTypes.NewCoin("upokt", math.NewInt(100000000))
 			appParams.MinStake = &newMinStakeApp
-			err := keepers.ApplicationKeeper.SetParams(ctx, appParams)
+			err = keepers.ApplicationKeeper.SetParams(ctx, appParams)
 			if err != nil {
-				return vm, err
+				return err
 			}
 
 			// Add supplier min stake
@@ -43,7 +46,7 @@ var Upgrade_0_0_10 = Upgrade{
 			supplierParams.MinStake = &newMinStakeSupplier
 			err = keepers.SupplierKeeper.SetParams(ctx, supplierParams)
 			if err != nil {
-				return vm, err
+				return err
 			}
 
 			// Add gateway min stake
@@ -53,16 +56,11 @@ var Upgrade_0_0_10 = Upgrade{
 			gatewayParams.MinStake = &newMinStakeGW
 			err = keepers.GatewayKeeper.SetParams(ctx, gatewayParams)
 			if err != nil {
-				return vm, err
+				return err
 			}
 
 			// Adjust proof module parameters
 			// Validate with: `poktrolld q proof params --node=https://testnet-validated-validator-rpc.poktroll.com/`
-			//
-			// !!!! THE UPGRADE CURRENTLY BREAKS HERE !!!!
-			// https://gist.github.com/okdas/535bc3c0282483fef27454c70d889aa6
-			// proofParams := keepers.ProofKeeper.GetParams(ctx)
-			// Need to construct params manually due to protobuf type changes:
 			newProofRequirementThreshold := cosmosTypes.NewCoin("upokt", math.NewInt(20000000))
 			newProofMissingPenalty := cosmosTypes.NewCoin("upokt", math.NewInt(320000000))
 			newProofSubmissionFee := cosmosTypes.NewCoin("upokt", math.NewInt(1000000))
@@ -75,7 +73,7 @@ var Upgrade_0_0_10 = Upgrade{
 
 			err = keepers.ProofKeeper.SetParams(ctx, proofParams)
 			if err != nil {
-				return vm, err
+				return err
 			}
 
 			// Add new shared module params
@@ -86,37 +84,58 @@ var Upgrade_0_0_10 = Upgrade{
 			sharedParams.ComputeUnitsToTokensMultiplier = uint64(42)
 			err = keepers.SharedKeeper.SetParams(ctx, sharedParams)
 			if err != nil {
-				return vm, err
+				return err
 			}
+			return
+		}
 
-			//
-			// Add new authz authorizations:
-			// https://github.com/pokt-network/poktroll/compare/v0.0.9-3...ff76430
-			//
-
-			// Validate before after with:
+		// Adds new authz authorizations from the diff:
+		// https://github.com/pokt-network/poktroll/compare/v0.0.9-3...ff76430
+		applyNewAuthorizations := func(ctx context.Context) (err error) {
+			// Validate before/after with:
 			// `poktrolld q authz grants-by-granter pokt10d07y265gmmuvt4z0w9aw880jnsr700j8yv32t --node=https://testnet-validated-validator-rpc.poktroll.com/`
-			newAuthorizations := []grantAuthorization{
-				newTestNetGrantAuthorization("/poktroll.gateway.MsgUpdateParam"),
-				newTestNetGrantAuthorization("/poktroll.application.MsgUpdateParam"),
-				newTestNetGrantAuthorization("/poktroll.supplier.MsgUpdateParam"),
+			grantAuthorizationMessages := []string{
+				"/poktroll.gateway.MsgUpdateParam",
+				"/poktroll.application.MsgUpdateParam",
+				"/poktroll.supplier.MsgUpdateParam",
+			}
+
+			expiration, err := time.Parse(time.RFC3339, "2500-01-01T00:00:00Z")
+			if err != nil {
+				return fmt.Errorf("failed to parse time: %w", err)
 			}
-			for _, authorization := range newAuthorizations {
+
+			for _, msg := range grantAuthorizationMessages {
 				err = keepers.AuthzKeeper.SaveGrant(
 					ctx,
-					authorization.grantee,
-					authorization.granter,
-					authorization.authorization,
-					authorization.expiration,
+					cosmosTypes.AccAddress(AlphaTestNetPnfAddress),
+					cosmosTypes.AccAddress(AlphaTestNetAuthorityAddress),
+					authz.NewGenericAuthorization(msg),
+					&expiration,
 				)
 				if err != nil {
-					return vm, err
+					return fmt.Errorf("failed to save grant for message %s: %w", msg, err)
 				}
 			}
+			return
+		}
+
+		// Returns the upgrade handler for v0.0.10
+		return func(ctx context.Context, plan upgradetypes.Plan, vm module.VersionMap) (module.VersionMap, error) {
+			err := applyNewParameters(ctx)
+			if err != nil {
+				return vm, err
+			}
+
+			err = applyNewAuthorizations(ctx)
+			if err != nil {
+				return vm, err
+			}
 
-			// Seems like RelayMiningDifficulty have been moved from `tokenomics` to `services`.
-			// In the ideal scenario, we should have migrated the data before removing query/msg types in `tokenomics`
-			// module. It would be hard to do that now. We know that new RelayMiningDifficulty will be created,
+			// RelayMiningDifficulty have been moved from `tokenomics` to `services` in this diff.
+			// Ideally (in prod), we should have migrated the data before removing query/msg types in `tokenomics` module.
+			// In practice (development), we don't want to spend time on it.
+			// Since we know that new RelayMiningDifficulty will be re-created and real tokens are not in danger,
 			// we can skip this step now.
 
 			return mm.RunMigrations(ctx, configurator, vm)

tools/scripts/upgrades/local_test_v0.0.10.json

@@ -7,7 +7,7 @@
                 "plan": {
                     "name": "v0.0.10",
                     "height": "20",
-                    "info": "NOT NEEDED FOR LOCAL TESTING"
+                    "info": "This field is not used for local testing"
                 }
             }
         ]