#13 Update docs, fix tests

Makefile

@@ -5,7 +5,7 @@ install:
 	go install ./cmd/xrncli
 
 test:
-	go test ./... -godog.strict
+	go test ./...
 
 lint:
 	go get -u golang.org/x/lint/golint

app.go

@@ -160,7 +160,7 @@ func NewXrnApp(logger log.Logger, db dbm.DB, postgresUrl string) *xrnApp {
 
 	app.upgradeKeeper = upgrade.NewKeeper(app.upgradeStoreKey, cdc)
 	app.upgradeKeeper.SetDoShutdowner(app.shutdownOnUpgrade)
-	app.upgradeKeeper.SetUpgradeHandler("test3", func(ctx sdk.Context, plan upgrade.UpgradePlan) {
+	app.upgradeKeeper.SetUpgradeHandler("test3", func(ctx sdk.Context, plan upgrade.Plan) {
 		ctx.Logger().Info("In upgrade 3 handler!")
 	})
 
@@ -252,12 +252,12 @@ func (app *xrnApp) initChainer(ctx sdk.Context, req abci.RequestInitChain) abci.
 	return abci.ResponseInitChain{}
 }
 
-func (app *xrnApp) shutdownOnUpgrade(ctx sdk.Context, plan upgrade.UpgradePlan) {
-	if len(plan.Memo) != 0 {
+func (app *xrnApp) shutdownOnUpgrade(ctx sdk.Context, plan upgrade.Plan) {
+	if len(plan.Info) != 0 {
 		home := viper.GetString(cli.HomeFlag)
-		_ = ioutil.WriteFile(filepath.Join(home, "data", "upgrade-info"), []byte(plan.Memo), 0644)
+		_ = ioutil.WriteFile(filepath.Join(home, "data", "upgrade-info"), []byte(plan.Info), 0644)
 	}
-	ctx.Logger().Error(fmt.Sprintf("UPGRADE \"%s\" NEEDED needed at height %d: %s", plan.Name, ctx.BlockHeight(), plan.Memo))
+	ctx.Logger().Error(fmt.Sprintf("UPGRADE \"%s\" NEEDED needed at height %d: %s", plan.Name, ctx.BlockHeight(), plan.Info))
 	os.Exit(1)
 }
 

util/godog.go

@@ -11,6 +11,7 @@ import (
 var opt = godog.Options{
 	Output: colors.Colored(os.Stdout),
 	Format: "progress", // can define default values
+	Strict: true,
 }
 
 func init() {

x/consortium/actions.go

@@ -8,7 +8,7 @@ import (
 )
 
 type ActionScheduleUpgrade struct {
-	Plan upgrade.UpgradePlan `json:"upgrade_plan"`
+	Plan upgrade.Plan `json:"upgrade_plan"`
 }
 
 type ActionCancelUpgrade struct {

x/consortium/client/cli/tx.go

@@ -1,6 +1,8 @@
 package cli
 
 import (
+	"encoding/json"
+	"fmt"
 	"github.com/regen-network/regen-ledger/x/consortium"
 	"github.com/regen-network/regen-ledger/x/proposal"
 	proposalcli "github.com/regen-network/regen-ledger/x/proposal/client/cli"
@@ -14,7 +16,7 @@ import (
 func GetCmdProposeUpgrade(cdc *codec.Codec) *cobra.Command {
 	var timeStr string
 	var height int64
-	var memo string
+	var commit string
 
 	cmd := proposalcli.GetCmdPropose(cdc, func(cmd *cobra.Command, args []string) (action proposal.ProposalAction, e error) {
 		name := args[0]
@@ -24,25 +26,36 @@ func GetCmdProposeUpgrade(cdc *codec.Codec) *cobra.Command {
 		if len(timeStr) != 0 {
 			t, err = time.Parse(time.RFC3339, timeStr)
 			if err != nil {
-				panic(err)
+				return nil, fmt.Errorf("error parsing time: %+v", err)
+			}
+
+			if height != 0 {
+				return nil, fmt.Errorf("only one of --time or --height should be specified")
 			}
 		}
 
+		info := make(map[string]interface{})
+		if len(commit) != 0 {
+			info["commit"] = commit
+		}
+
+		jsonInfo, err := json.Marshal(info)
+
 		return consortium.ActionScheduleUpgrade{
-			Plan: upgrade.UpgradePlan{
+			Plan: upgrade.Plan{
 				Name:   name,
 				Time:   t,
 				Height: height,
-				Memo:   memo,
+				Info:   string(jsonInfo),
 			},
 		}, nil
 	})
 
 	cmd.Args = cobra.ExactArgs(1)
-	cmd.Use = "propose-upgrade <name> [--upgrade-time <time> | --upgrade-height <height>] [--upgrade-memo <memo>]"
+	cmd.Use = "propose-upgrade <name> [--time <time> | --height <height>] [--commit <commit-hash>]"
 	cmd.Short = "Propose an ESP version"
-	cmd.Flags().StringVar(&timeStr, "upgrade-time", "", "The time after which the upgrade must happen in ISO8601/RFC3339 format (omit if using --upgrade-height)")
-	cmd.Flags().Int64Var(&height, "upgrade-height", 0, "The height at which the upgrade must happen (omit if using --upgrade-time)")
-	cmd.Flags().StringVar(&memo, "upgrade-memo", "", "Any memo to attach to the upgrade plan such as a git commit hash")
+	cmd.Flags().StringVar(&timeStr, "time", "", "The time after which the upgrade must happen in ISO8601/RFC3339 format (omit if using --upgrade-height)")
+	cmd.Flags().Int64Var(&height, "height", 0, "The height at which the upgrade must happen (omit if using --upgrade-time)")
+	cmd.Flags().StringVar(&commit, "commit", "", "The git commit hash of the version of the software to upgrade to")
 	return cmd
 }

x/group/types.go

@@ -17,7 +17,7 @@ type Group struct {
 	// A big integer is used here to avoid any potential vulnerabilities from overflow errors
 	// where large weight and threshold values are used.
 	DecisionThreshold sdk.Int `json:"decision_threshold"`
-	// TODO maybe make this something more specific to a domain name or a claim on identity? or Memo leave it generic
+	// TODO maybe make this something more specific to a domain name or a claim on identity? or Info leave it generic
 	Memo string `json:"memo,omitempty"`
 }
 

x/upgrade/codec.go

@@ -6,5 +6,5 @@ import (
 
 // RegisterCodec registers concrete types on the Amino codec
 func RegisterCodec(cdc *codec.Codec) {
-	cdc.RegisterConcrete(UpgradePlan{}, "upgrade/UpgradePlan", nil)
+	cdc.RegisterConcrete(Plan{}, "upgrade/Plan", nil)
 }

x/upgrade/keeper.go

@@ -7,33 +7,36 @@ import (
 	abci "github.com/tendermint/tendermint/abci/types"
 )
 
+// Keeper of the upgrade module
 type Keeper struct {
 	storeKey        sdk.StoreKey
 	cdc             *codec.Codec
-	doShutdowner    func(sdk.Context, UpgradePlan)
-	upgradeHandlers map[string]UpgradeHandler
+	doShutdowner    func(sdk.Context, Plan)
+	upgradeHandlers map[string]Handler
 }
 
 const (
 	planKey = "plan"
 )
 
+// NewKeeper constructs an upgrade keeper
 func NewKeeper(storeKey sdk.StoreKey, cdc *codec.Codec) Keeper {
 	return Keeper{
 		storeKey:        storeKey,
 		cdc:             cdc,
-		upgradeHandlers: map[string]UpgradeHandler{},
+		upgradeHandlers: map[string]Handler{},
 	}
 }
 
-// Sets an upgrade handler for the upgrade specified by name. This handler will be called when the upgrade
+// SetUpgradeHandler sets an UpgradeHandler for the upgrade specified by name. This handler will be called when the upgrade
 // with this name is applied. In order for an upgrade with the given name to proceed, a handler for this upgrade
 // must be set even if it is a no-op function.
-func (keeper Keeper) SetUpgradeHandler(name string, upgradeHandler UpgradeHandler) {
+func (keeper Keeper) SetUpgradeHandler(name string, upgradeHandler Handler) {
 	keeper.upgradeHandlers[name] = upgradeHandler
 }
 
-func (keeper Keeper) ScheduleUpgrade(ctx sdk.Context, plan UpgradePlan) sdk.Error {
+// ScheduleUpgrade schedules an upgrade based on the specified plan
+func (keeper Keeper) ScheduleUpgrade(ctx sdk.Context, plan Plan) sdk.Error {
 	err := plan.ValidateBasic()
 	if err != nil {
 		return err
@@ -56,43 +59,51 @@ func (keeper Keeper) ScheduleUpgrade(ctx sdk.Context, plan UpgradePlan) sdk.Erro
 	return nil
 }
 
+// ClearUpgradePlan clears any schedule upgrade
 func (keeper Keeper) ClearUpgradePlan(ctx sdk.Context) {
 	store := ctx.KVStore(keeper.storeKey)
 	store.Delete([]byte(planKey))
 }
 
-func (plan UpgradePlan) ValidateBasic() sdk.Error {
+// ValidateBasic does basic validation of an Plan
+func (plan Plan) ValidateBasic() sdk.Error {
 	if len(plan.Name) == 0 {
 		return sdk.ErrUnknownRequest("Name cannot be empty")
 
 	}
 	return nil
 }
 
-func (keeper Keeper) GetUpgradeInfo(ctx sdk.Context) (plan UpgradePlan, err sdk.Error) {
+// GetUpgradePlan returns the currently scheduled Plan if any, setting havePlan to true if there is a scheduled
+// upgrade or false if there is none
+func (keeper Keeper) GetUpgradePlan(ctx sdk.Context) (plan Plan, havePlan bool) {
 	store := ctx.KVStore(keeper.storeKey)
 	bz := store.Get([]byte(planKey))
 	if bz == nil {
-		return plan, sdk.ErrUnknownRequest("Not found")
+		return plan, false
 	}
 	keeper.cdc.MustUnmarshalBinaryBare(bz, &plan)
-	return plan, nil
+	return plan, true
 }
 
-func (keeper *Keeper) SetDoShutdowner(doShutdowner func(ctx sdk.Context, plan UpgradePlan)) {
+// SetDoShutdowner sets a custom shutdown function for the upgrade module. This shutdown
+// function will be called during the BeginBlock method when an upgrade is required
+// instead of panic'ing which is the default behavior
+func (keeper *Keeper) SetDoShutdowner(doShutdowner func(ctx sdk.Context, plan Plan)) {
 	keeper.doShutdowner = doShutdowner
 }
 
 func upgradeDoneKey(name string) []byte {
 	return []byte(fmt.Sprintf("done/%s", name))
 }
 
+// BeginBlocker should be called inside the BeginBlocker method of any app using the upgrade module
 func (keeper *Keeper) BeginBlocker(ctx sdk.Context, req abci.RequestBeginBlock) {
 	blockTime := ctx.BlockHeader().Time
 	blockHeight := ctx.BlockHeight()
 
-	plan, err := keeper.GetUpgradeInfo(ctx)
-	if err != nil {
+	plan, havePlan := keeper.GetUpgradePlan(ctx)
+	if !havePlan {
 		return
 	}
 
@@ -110,7 +121,7 @@ func (keeper *Keeper) BeginBlocker(ctx sdk.Context, req abci.RequestBeginBlock)
 			store.Set(upgradeDoneKey(plan.Name), []byte("1"))
 		} else {
 			// We don't have an upgrade handler for this upgrade name, meaning this software is out of date so shutdown
-			ctx.Logger().Error(fmt.Sprintf("UPGRADE \"%s\" NEEDED needed at height %d: %s", plan.Name, blockHeight, plan.Memo))
+			ctx.Logger().Error(fmt.Sprintf("UPGRADE \"%s\" NEEDED needed at height %d: %s", plan.Name, blockHeight, plan.Info))
 			doShutdowner := keeper.doShutdowner
 			if doShutdowner != nil {
 				doShutdowner(ctx, plan)

x/upgrade/keeper_test.go

@@ -32,34 +32,34 @@ func (s *TestSuite) SetupTest() {
 }
 
 func (s *TestSuite) TestRequireName() {
-	err := s.keeper.ScheduleUpgrade(s.ctx, UpgradePlan{})
+	err := s.keeper.ScheduleUpgrade(s.ctx, Plan{})
 	s.Require().NotNil(err)
 	s.Require().Equal(sdk.CodeUnknownRequest, err.Code())
 }
 
 func (s *TestSuite) TestRequireFutureTime() {
-	err := s.keeper.ScheduleUpgrade(s.ctx, UpgradePlan{Name: "test", Time: s.ctx.BlockHeader().Time})
+	err := s.keeper.ScheduleUpgrade(s.ctx, Plan{Name: "test", Time: s.ctx.BlockHeader().Time})
 	s.Require().NotNil(err)
 	s.Require().Equal(sdk.CodeUnknownRequest, err.Code())
 }
 
 func (s *TestSuite) TestRequireFutureBlock() {
-	err := s.keeper.ScheduleUpgrade(s.ctx, UpgradePlan{Name: "test", Height: s.ctx.BlockHeight()})
+	err := s.keeper.ScheduleUpgrade(s.ctx, Plan{Name: "test", Height: s.ctx.BlockHeight()})
 	s.Require().NotNil(err)
 	s.Require().Equal(sdk.CodeUnknownRequest, err.Code())
 }
 
 func (s *TestSuite) TestDoTimeUpgrade() {
 	s.T().Log("Verify can schedule an upgrade")
-	err := s.keeper.ScheduleUpgrade(s.ctx, UpgradePlan{Name: "test", Time: time.Now()})
+	err := s.keeper.ScheduleUpgrade(s.ctx, Plan{Name: "test", Time: time.Now()})
 	s.Require().Nil(err)
 
 	s.VerifyDoUpgrade()
 }
 
 func (s *TestSuite) TestDoHeightUpgrade() {
 	s.T().Log("Verify can schedule an upgrade")
-	err := s.keeper.ScheduleUpgrade(s.ctx, UpgradePlan{Name: "test", Height: s.ctx.BlockHeight() + 1})
+	err := s.keeper.ScheduleUpgrade(s.ctx, Plan{Name: "test", Height: s.ctx.BlockHeight() + 1})
 	s.Require().Nil(err)
 
 	s.VerifyDoUpgrade()
@@ -74,7 +74,7 @@ func (s *TestSuite) VerifyDoUpgrade() {
 	})
 
 	s.T().Log("Verify that the upgrade can be successfully applied with a handler")
-	s.keeper.SetUpgradeHandler("test", func(ctx sdk.Context, plan UpgradePlan) {})
+	s.keeper.SetUpgradeHandler("test", func(ctx sdk.Context, plan Plan) {})
 	s.Require().NotPanics(func() {
 		s.keeper.BeginBlocker(newCtx, req)
 	})
@@ -84,14 +84,13 @@ func (s *TestSuite) VerifyDoUpgrade() {
 
 func (s *TestSuite) VerifyCleared(newCtx sdk.Context) {
 	s.T().Log("Verify that the upgrade plan has been cleared")
-	_, err := s.keeper.GetUpgradeInfo(newCtx)
-	s.Require().NotNil(err)
-	s.Require().Equal(sdk.CodeUnknownRequest, err.Code())
+	_, havePlan := s.keeper.GetUpgradePlan(newCtx)
+	s.Require().False(havePlan)
 }
 
 func (s *TestSuite) TestCanClear() {
 	s.T().Log("Verify upgrade is scheduled")
-	err := s.keeper.ScheduleUpgrade(s.ctx, UpgradePlan{Name: "test", Time: time.Now()})
+	err := s.keeper.ScheduleUpgrade(s.ctx, Plan{Name: "test", Time: time.Now()})
 	s.Require().Nil(err)
 
 	s.keeper.ClearUpgradePlan(s.ctx)
@@ -102,20 +101,20 @@ func (s *TestSuite) TestCanClear() {
 func (s *TestSuite) TestCantApplySameUpgradeTwice() {
 	s.TestDoTimeUpgrade()
 	s.T().Log("Verify an upgrade named \"test\" can't be scheduled twice")
-	err := s.keeper.ScheduleUpgrade(s.ctx, UpgradePlan{Name: "test", Time: time.Now()})
+	err := s.keeper.ScheduleUpgrade(s.ctx, Plan{Name: "test", Time: time.Now()})
 	s.Require().NotNil(err)
 	s.Require().Equal(sdk.CodeUnknownRequest, err.Code())
 }
 
 func (s *TestSuite) TestDoShutdowner() {
 	s.T().Log("Set a custom DoShutdowner")
 	shutdownerCalled := false
-	s.keeper.SetDoShutdowner(func(ctx sdk.Context, plan UpgradePlan) {
+	s.keeper.SetDoShutdowner(func(ctx sdk.Context, plan Plan) {
 		shutdownerCalled = true
 	})
 
 	s.T().Log("Run an upgrade and verify that the custom shutdowner was called and no panic happened")
-	err := s.keeper.ScheduleUpgrade(s.ctx, UpgradePlan{Name: "test", Time: time.Now()})
+	err := s.keeper.ScheduleUpgrade(s.ctx, Plan{Name: "test", Time: time.Now()})
 	s.Require().Nil(err)
 
 	header := abci.Header{Height: s.ctx.BlockHeight() + 1, Time: time.Now()}

x/upgrade/types.go

@@ -3,7 +3,8 @@ package upgrade
 import "time"
 import sdk "github.com/cosmos/cosmos-sdk/types"
 
-type UpgradePlan struct {
+// Plan specifies information about a planned upgrade and when it should occur
+type Plan struct {
 	// Sets the name for the upgrade. This name will be used by the upgraded version of the software to apply any
 	// special "on-upgrade" commands during the first BeginBlock method after the upgrade is applied. It is also used
 	// to detect whether a software version can handle a given upgrade. If no upgrade handler with this name has been
@@ -21,8 +22,8 @@ type UpgradePlan struct {
 
 	// Any application specific upgrade info to be included on-chain
 	// such as a git commit that validators could automatically upgrade to
-	Memo string `json:"memo,omitempty"`
+	Info string `json:"info,omitempty"`
 }
 
-// The type of function that is called when an upgrade is applied
-type UpgradeHandler func(ctx sdk.Context, plan UpgradePlan)
+// UpgradeHandler specifies the type of function that is called when an upgrade is applied
+type Handler func(ctx sdk.Context, plan Plan)