Check minimum build version before enabling feature flag

Makefile

@@ -182,7 +182,7 @@ deps: $(PLUGIN_DIR) $(GO_ETHEREUM_DIR) $(SSHA3_DIR)
 		github.com/posener/wstest
 
 	# for when you want to reference a different branch of go-loom
-	# cd $(PLUGIN_DIR) && git checkout 907-feat && git pull origin 907-feat
+	#cd $(PLUGIN_DIR) && git checkout feature-flag-minimum-build && git pull origin feature-flag-minimum-build
 	cd $(GOLANG_PROTOBUF_DIR) && git checkout v1.1.0
 	cd $(GOGO_PROTOBUF_DIR) && git checkout v1.1.1
 	cd $(GO_ETHEREUM_DIR) && git checkout master && git pull && git checkout $(ETHEREUM_GIT_REV)

builtin/plugins/chainconfig/chainconfig.go

@@ -16,21 +16,18 @@ import (
 )
 
 type (
-	InitRequest          = cctypes.InitRequest
-	ListFeaturesRequest  = cctypes.ListFeaturesRequest
-	ListFeaturesResponse = cctypes.ListFeaturesResponse
-
-	GetFeatureRequest  = cctypes.GetFeatureRequest
-	GetFeatureResponse = cctypes.GetFeatureResponse
-	AddFeatureRequest  = cctypes.AddFeatureRequest
-	AddFeatureResponse = cctypes.AddFeatureResponse
-	SetParamsRequest   = cctypes.SetParamsRequest
-	GetParamsRequest   = cctypes.GetParamsRequest
-	GetParamsResponse  = cctypes.GetParamsResponse
-	Params             = cctypes.Params
-	Feature            = cctypes.Feature
-
-	UpdateFeatureRequest  = cctypes.UpdateFeatureRequest
+	InitRequest           = cctypes.InitRequest
+	ListFeaturesRequest   = cctypes.ListFeaturesRequest
+	ListFeaturesResponse  = cctypes.ListFeaturesResponse
+	GetFeatureRequest     = cctypes.GetFeatureRequest
+	GetFeatureResponse    = cctypes.GetFeatureResponse
+	AddFeatureRequest     = cctypes.AddFeatureRequest
+	AddFeatureResponse    = cctypes.AddFeatureResponse
+	SetParamsRequest      = cctypes.SetParamsRequest
+	GetParamsRequest      = cctypes.GetParamsRequest
+	GetParamsResponse     = cctypes.GetParamsResponse
+	Params                = cctypes.Params
+	Feature               = cctypes.Feature
 	EnableFeatureRequest  = cctypes.EnableFeatureRequest
 	EnableFeatureResponse = cctypes.EnableFeatureResponse
 )
@@ -49,20 +46,22 @@ const (
 )
 
 var (
-	// ErrrNotAuthorized indicates that a contract method failed because the caller didn't have
+	// ErrNotAuthorized indicates that a contract method failed because the caller didn't have
 	// the permission to execute that method.
 	ErrNotAuthorized = errors.New("[ChainConfig] not authorized")
 	// ErrInvalidRequest is a generic error that's returned when something is wrong with the
 	// request message, e.g. missing or invalid fields.
 	ErrInvalidRequest = errors.New("[ChainConfig] invalid request")
 	// ErrOwnerNotSpecified returned if init request does not have owner address
 	ErrOwnerNotSpecified = errors.New("[ChainConfig] owner not specified")
-	// ErrFeatureFound returned if an owner try to set an existing feature
+	// ErrFeatureAlreadyExists returned if an owner try to set an existing feature
 	ErrFeatureAlreadyExists = errors.New("[ChainConfig] feature already exists")
 	// ErrInvalidParams returned if parameters are invalid
 	ErrInvalidParams = errors.New("[ChainConfig] invalid params")
 	// ErrFeatureAlreadyEnabled is returned if a validator tries to enable a feature that's already enabled
 	ErrFeatureAlreadyEnabled = errors.New("[ChainConfig] feature already enabled")
+	// ErrFeatureNotSupported inidicates that an enabled feature is not supported in the current build
+	ErrFeatureNotSupported = errors.New("[ChainConfig] feature is not supported in the current build")
 )
 
 const (
@@ -175,7 +174,7 @@ func (c *ChainConfig) AddFeature(ctx contract.Context, req *AddFeatureRequest) e
 		return ErrInvalidRequest
 	}
 	for _, name := range req.Names {
-		if err := addFeature(ctx, name); err != nil {
+		if err := addFeature(ctx, name, req.BuildNumber); err != nil {
 			return err
 		}
 	}
@@ -234,7 +233,7 @@ func (c *ChainConfig) GetFeature(ctx contract.StaticContext, req *GetFeatureRequ
 //   feature reaches a certain threshold.
 // - A WAITING feature will become ENABLED after a sufficient number of block confirmations.
 // Returns a list of features whose status has changed from WAITING to ENABLED at the given height.
-func EnableFeatures(ctx contract.Context, blockHeight uint64) ([]*Feature, error) {
+func EnableFeatures(ctx contract.Context, blockHeight, buildNumber uint64) ([]*Feature, error) {
 	params, err := getParams(ctx)
 	if err != nil {
 		return nil, err
@@ -277,6 +276,9 @@ func EnableFeatures(ctx contract.Context, blockHeight uint64) ([]*Feature, error
 			}
 		case FeatureWaiting:
 			if blockHeight > (feature.BlockHeight + params.NumBlockConfirmations) {
+				if buildNumber < feature.BuildNumber {
+					return nil, ErrFeatureNotSupported
+				}
 				feature.Status = FeatureEnabled
 				if err := ctx.Set(featureKey(feature.Name), feature); err != nil {
 					return nil, err
@@ -292,6 +294,7 @@ func EnableFeatures(ctx contract.Context, blockHeight uint64) ([]*Feature, error
 				)
 			}
 		}
+
 	}
 	return enabledFeatures, nil
 }
@@ -448,7 +451,7 @@ func enableFeature(ctx contract.Context, name string) error {
 	return ctx.Set(featureKey(name), &feature)
 }
 
-func addFeature(ctx contract.Context, name string) error {
+func addFeature(ctx contract.Context, name string, buildNumber uint64) error {
 	if name == "" {
 		return ErrInvalidRequest
 	}
@@ -462,8 +465,9 @@ func addFeature(ctx contract.Context, name string) error {
 	}
 
 	feature := Feature{
-		Name:   name,
-		Status: FeaturePending,
+		Name:        name,
+		BuildNumber: buildNumber,
+		Status:      FeaturePending,
 	}
 
 	if err := ctx.Set(featureKey(name), &feature); err != nil {

builtin/plugins/chainconfig/chainconfig_test.go

@@ -370,8 +370,8 @@ func (c *ChainConfigTestSuite) TestFeatureFlagEnabledFourValidators() {
 	require.Equal(cctypes.Feature_PENDING, getFeature.Feature.Status)
 	require.Equal(uint64(75), getFeature.Feature.Percentage)
 	fmt.Println(formatJSON(getFeature))
-
-	enabledFeatures, err := EnableFeatures(ctx, 20)
+	buildNumber := uint64(1000)
+	enabledFeatures, err := EnableFeatures(ctx, 20, buildNumber)
 	require.NoError(err)
 	require.Equal(0, len(enabledFeatures))
 
@@ -384,7 +384,7 @@ func (c *ChainConfigTestSuite) TestFeatureFlagEnabledFourValidators() {
 	require.Equal(uint64(75), getFeature.Feature.Percentage)
 	fmt.Println(formatJSON(getFeature))
 
-	enabledFeatures, err = EnableFeatures(ctx, 31)
+	enabledFeatures, err = EnableFeatures(ctx, 31, buildNumber)
 	require.NoError(err)
 	require.Equal(1, len(enabledFeatures))
 
@@ -403,3 +403,118 @@ func (c *ChainConfigTestSuite) TestFeatureFlagEnabledFourValidators() {
 
 	fmt.Println(formatJSON(featureEnable))
 }
+
+func (c *ChainConfigTestSuite) TestUnsupportedFeatureEnabled() {
+	require := c.Require()
+	featureName := "hardfork"
+	featureName2 := "test-ft"
+	featureName3 := "test2-ft"
+	encoder := base64.StdEncoding
+	pubKeyB64_1, _ := encoder.DecodeString(pubKey1)
+	chainID := "default"
+	addr1 := loom.Address{ChainID: chainID, Local: loom.LocalAddressFromPublicKey(pubKeyB64_1)}
+	//setup dposv2 fake contract
+	pctx := plugin.CreateFakeContext(addr1, addr1).WithBlock(loom.BlockHeader{
+		ChainID: chainID,
+		Time:    time.Now().Unix(),
+	})
+
+	//Init fake coin contract
+	coinContract := &coin.Coin{}
+	coinAddr := pctx.CreateContract(coin.Contract)
+	coinCtx := pctx.WithAddress(coinAddr)
+	err := coinContract.Init(contractpb.WrapPluginContext(coinCtx), &coin.InitRequest{
+		Accounts: []*coin.InitialAccount{},
+	})
+	require.NoError(err)
+
+	//Init fake dposv2 contract
+	dposv2Contract := dposv2.DPOS{}
+	dposv2Addr := pctx.CreateContract(dposv2.Contract)
+	pctx = pctx.WithAddress(dposv2Addr)
+	ctx := contractpb.WrapPluginContext(pctx)
+	validators := []*dposv2.Validator{
+		&dposv2.Validator{
+			PubKey: pubKeyB64_1,
+			Power:  10,
+		},
+	}
+	err = dposv2Contract.Init(ctx, &dposv2.InitRequest{
+		Params: &dposv2.Params{
+			ValidatorCount: 21,
+		},
+		Validators: validators,
+	})
+	require.NoError(err)
+
+	//setup chainconfig contract
+	chainconfigContract := &ChainConfig{}
+	err = chainconfigContract.Init(ctx, &InitRequest{
+		Owner: addr1.MarshalPB(),
+		Params: &Params{
+			VoteThreshold:         66,
+			NumBlockConfirmations: 10,
+		},
+		Features: []*Feature{
+			&Feature{
+				Name:       featureName2,
+				Status:     FeaturePending,
+				Percentage: 0,
+			},
+			&Feature{
+				Name:       featureName3,
+				Status:     FeatureWaiting,
+				Percentage: 100,
+			},
+		},
+	})
+	require.NoError(err)
+
+	err = chainconfigContract.AddFeature(ctx, &AddFeatureRequest{
+		Names:       []string{featureName},
+		BuildNumber: 1000,
+	})
+	require.NoError(err)
+
+	getFeature, err := chainconfigContract.GetFeature(ctx, &GetFeatureRequest{
+		Name: featureName,
+	})
+	require.NoError(err)
+	require.Equal(featureName, getFeature.Feature.Name)
+	require.Equal(cctypes.Feature_PENDING, getFeature.Feature.Status)
+	require.Equal(uint64(0), getFeature.Feature.Percentage)
+
+	getFeature, err = chainconfigContract.GetFeature(ctx, &GetFeatureRequest{
+		Name: featureName2,
+	})
+	require.NoError(err)
+	require.Equal(featureName2, getFeature.Feature.Name)
+	require.Equal(cctypes.Feature_PENDING, getFeature.Feature.Status)
+	require.Equal(uint64(0), getFeature.Feature.Percentage)
+
+	getFeature, err = chainconfigContract.GetFeature(ctx, &GetFeatureRequest{
+		Name: featureName3,
+	})
+	require.NoError(err)
+	require.Equal(featureName3, getFeature.Feature.Name)
+	require.Equal(cctypes.Feature_WAITING, getFeature.Feature.Status)
+	require.Equal(uint64(100), getFeature.Feature.Percentage)
+
+	listFeatures, err := chainconfigContract.ListFeatures(ctx, &ListFeaturesRequest{})
+	require.NoError(err)
+	require.Equal(3, len(listFeatures.Features))
+
+	err = chainconfigContract.EnableFeature(ctx, &EnableFeatureRequest{
+		Names: []string{featureName},
+	})
+	require.NoError(err)
+
+	buildNumber := uint64(10)
+	_, err = EnableFeatures(ctx, 100, buildNumber)
+	_, err = EnableFeatures(ctx, 1000, buildNumber)
+	require.Equal(ErrFeatureNotSupported, err)
+
+	buildNumber = uint64(2000)
+	_, err = EnableFeatures(ctx, 1000, buildNumber)
+	require.NoError(err)
+}

cmd/loom/chainconfig/cmd.go

@@ -57,28 +57,33 @@ func EnableFeatureCmd() *cobra.Command {
 }
 
 const addFeatureCmdExample = `
-loom chain-cfg add-feature hardfork multichain
+loom chain-cfg add-feature hardfork multichain --build 866
 `
 
 func AddFeatureCmd() *cobra.Command {
-	return &cobra.Command{
+	var buildNumber uint64
+	cmd := &cobra.Command{
 		Use:     "add-feature <feature name 1> ... <feature name N>",
-		Short:   "Add feature by feature name",
+		Short:   "Add new feature",
 		Example: addFeatureCmdExample,
 		RunE: func(cmd *cobra.Command, args []string) error {
 			for _, name := range args {
 				if name == "" {
 					return fmt.Errorf("Invalid feature name")
 				}
 			}
-			req := &cctype.AddFeatureRequest{Names: args}
+			req := &cctype.AddFeatureRequest{Names: args, BuildNumber: buildNumber}
 			err := cli.CallContract(chainConfigContractName, "AddFeature", req, nil)
 			if err != nil {
 				return err
 			}
 			return nil
 		},
 	}
+	cmdFlags := cmd.Flags()
+	cmdFlags.Uint64Var(&buildNumber, "build", 0, "Minimum build number that supports this feature")
+	cmd.MarkFlagRequired("build")
+	return cmd
 }
 
 const setParamsCmdExample = `

e2e/chainconfig.toml

@@ -4,27 +4,27 @@
   Expected = ['features', 'dposv3']
 
 [[TestCases]]
-  RunCmd = "{{ $.LoomPath }} chain-cfg add-feature hardfork multichain -k {{index $.NodePrivKeyPathList 0}}"
+  RunCmd = "{{ $.LoomPath }} chain-cfg add-feature hardfork multichain --build 0 -k {{index $.NodePrivKeyPathList 0}}"
   Condition = "contains"
   Expected = ['']
 
 [[TestCases]]
-  RunCmd = "{{ $.LoomPath }} chain-cfg add-feature hardfork -k {{index $.NodePrivKeyPathList 1}}"
+  RunCmd = "{{ $.LoomPath }} chain-cfg add-feature hardfork --build 0 -k {{index $.NodePrivKeyPathList 1}}"
   Condition = "contains"
   Expected = ['not authorized']
 
 [[TestCases]]
-  RunCmd = "{{ $.LoomPath }} chain-cfg add-feature hardfork -k {{index $.NodePrivKeyPathList 2}}"
+  RunCmd = "{{ $.LoomPath }} chain-cfg add-feature hardfork --build 0 -k {{index $.NodePrivKeyPathList 2}}"
   Condition = "contains"
   Expected = ['not authorized']
 
 [[TestCases]]
-  RunCmd = "{{ $.LoomPath }} chain-cfg add-feature hardfork -k {{index $.NodePrivKeyPathList 3}}"
+  RunCmd = "{{ $.LoomPath }} chain-cfg add-feature hardfork --build 0 -k {{index $.NodePrivKeyPathList 3}}"
   Condition = "contains"
   Expected = ['not authorized']
 
 [[TestCases]]
-  RunCmd = "{{ $.LoomPath }} chain-cfg add-feature hardfork -k {{index $.NodePrivKeyPathList 0}}"
+  RunCmd = "{{ $.LoomPath }} chain-cfg add-feature hardfork --build 0 -k {{index $.NodePrivKeyPathList 0}}"
   Condition = "contains"
   Expected = ['feature already exists']
 
@@ -54,7 +54,7 @@
   Expected = ['75']
 
 [[TestCases]]
-  RunCmd = "{{ $.LoomPath }} chain-cfg add-feature newfeature -k {{index $.NodePrivKeyPathList 0}}"
+  RunCmd = "{{ $.LoomPath }} chain-cfg add-feature newfeature --build 0 -k {{index $.NodePrivKeyPathList 0}}"
   Condition = "contains"
   Expected = []
 
@@ -116,4 +116,19 @@
 [[TestCases]]
   RunCmd = "{{ $.LoomPath }} chain-cfg get-feature hardfork -k {{index $.NodePrivKeyPathList 3}}"
   Condition = "contains"
-  Expected = ['75']
+  Expected = ['75']
+
+[[TestCases]]
+  RunCmd = "{{ $.LoomPath }} chain-cfg add-feature feature1 feature2 --build 567 -k {{index $.NodePrivKeyPathList 0}}"
+  Condition = "contains"
+  Expected = ['']
+
+[[TestCases]]
+  RunCmd = "{{ $.LoomPath }} chain-cfg get-feature feature1  -k {{index $.NodePrivKeyPathList 0}}"
+  Condition = "contains"
+  Expected = ['feature1', '567']
+
+[[TestCases]]
+  RunCmd = "{{ $.LoomPath }} chain-cfg get-feature feature2  -k {{index $.NodePrivKeyPathList 0}}"
+  Condition = "contains"
+  Expected = ['feature2', '567']

e2e/enable-receipts-v2-feature.toml

@@ -17,7 +17,7 @@
   Expected = ['receipts_db']
 
 [[TestCases]]
-  RunCmd = "{{ $.LoomPath }} chain-cfg add-feature receipts:v2 -k {{index $.NodePrivKeyPathList 0}}"
+  RunCmd = "{{ $.LoomPath }} chain-cfg add-feature --build 0 receipts:v2 -k {{index $.NodePrivKeyPathList 0}}"
 
  [[TestCases]]
    RunCmd = "/bin/sleep 3s"