diff --git a/Makefile b/Makefile index 0651326a90..df56672f79 100644 --- a/Makefile +++ b/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) diff --git a/builtin/plugins/chainconfig/chainconfig.go b/builtin/plugins/chainconfig/chainconfig.go index 16ad579457..660ef172fd 100644 --- a/builtin/plugins/chainconfig/chainconfig.go +++ b/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,7 +46,7 @@ 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 @@ -57,12 +54,14 @@ var ( 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 { diff --git a/builtin/plugins/chainconfig/chainconfig_test.go b/builtin/plugins/chainconfig/chainconfig_test.go index 0effbf65cb..2ff7410f9c 100644 --- a/builtin/plugins/chainconfig/chainconfig_test.go +++ b/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) +} diff --git a/cmd/loom/chainconfig/cmd.go b/cmd/loom/chainconfig/cmd.go index f696f06cfb..de62e44a49 100644 --- a/cmd/loom/chainconfig/cmd.go +++ b/cmd/loom/chainconfig/cmd.go @@ -57,13 +57,14 @@ 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 ... ", - Short: "Add feature by feature name", + Short: "Add new feature", Example: addFeatureCmdExample, RunE: func(cmd *cobra.Command, args []string) error { for _, name := range args { @@ -71,7 +72,7 @@ func AddFeatureCmd() *cobra.Command { 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 @@ -79,6 +80,10 @@ func AddFeatureCmd() *cobra.Command { return nil }, } + cmdFlags := cmd.Flags() + cmdFlags.Uint64Var(&buildNumber, "build", 0, "Minimum build number that supports this feature") + cmd.MarkFlagRequired("build") + return cmd } const setParamsCmdExample = ` diff --git a/e2e/chainconfig.toml b/e2e/chainconfig.toml index a7227605d1..4d106da7b5 100644 --- a/e2e/chainconfig.toml +++ b/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'] \ No newline at end of file + 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'] \ No newline at end of file diff --git a/e2e/enable-receipts-v2-feature.toml b/e2e/enable-receipts-v2-feature.toml index 7b5ce49304..52a09a3fd3 100644 --- a/e2e/enable-receipts-v2-feature.toml +++ b/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" diff --git a/plugin/chain_config_manager.go b/plugin/chain_config_manager.go index 19f1010fac..62deaa87bb 100644 --- a/plugin/chain_config_manager.go +++ b/plugin/chain_config_manager.go @@ -1,6 +1,8 @@ package plugin import ( + "strconv" + "github.com/loomnetwork/go-loom" contract "github.com/loomnetwork/go-loom/plugin/contractpb" "github.com/loomnetwork/loomchain" @@ -18,6 +20,7 @@ var ( type ChainConfigManager struct { ctx contract.Context state loomchain.State + build uint64 } // NewChainConfigManager attempts to create an instance of ChainConfigManager. @@ -32,15 +35,26 @@ func NewChainConfigManager(pvm *PluginVM, state loomchain.State) (*ChainConfigMa } readOnly := false ctx := contract.WrapPluginContext(pvm.CreateContractContext(caller, contractAddr, readOnly)) + build, err := strconv.ParseUint(loomchain.Build, 10, 64) + if err != nil { + build = 0 + } return &ChainConfigManager{ ctx: ctx, state: state, + build: build, }, nil } func (c *ChainConfigManager) EnableFeatures(blockHeight int64) error { - features, err := chainconfig.EnableFeatures(c.ctx, uint64(blockHeight)) + features, err := chainconfig.EnableFeatures(c.ctx, uint64(blockHeight), c.build) if err != nil { + // When an unsupported feature has been activated by the rest of the chain + // panic to prevent the node from processing any further blocks until it's + // upgraded to a new build that supports the feature. + if err == chainconfig.ErrFeatureNotSupported { + panic(err) + } return err } for _, feature := range features {