[FAB-6839] configtx to directly utilize cb.Config

The configtx package has changed considerably over time, and no longer
needs to be initialized with an envelope message.  The only places which
actually instantiate the configtx manager are creating an artificial
Envelope message and mashaling the config inside it, purely to satisfy
the constructor.  This CR modifies the constructor to take the
ultimately necessary type.

Change-Id: I3e4909e45118694b309f733a5179efb7d9a235fd
Signed-off-by: Jason Yellick <jyellick@us.ibm.com>

common/channelconfig/bundle.go

@@ -217,12 +217,7 @@ func NewBundle(channelID string, config *cb.Config) (*Bundle, error) {
 		return nil, errors.Wrap(err, "initializing policymanager failed")
 	}
 
-	env, err := utils.CreateSignedEnvelope(cb.HeaderType_CONFIG, channelID, nil, &cb.ConfigEnvelope{Config: config}, 0, 0)
-	if err != nil {
-		return nil, errors.Wrap(err, "creating envelope for configtx manager failed")
-	}
-
-	configtxManager, err := configtx.NewManagerImpl(env, simpleProposer{
+	configtxManager, err := configtx.NewManagerImpl(channelID, config, simpleProposer{
 		policyManager: policyManager,
 	})
 	if err != nil {

common/configtx/api/api.go

@@ -22,8 +22,8 @@ type Manager interface {
 	// ChainID retrieves the chain ID associated with this manager
 	ChainID() string
 
-	// ConfigEnvelope returns the current config envelope
-	ConfigEnvelope() *cb.ConfigEnvelope
+	// ConfigProto returns the current config as a proto
+	ConfigProto() *cb.Config
 
 	// Sequence returns the current sequence number of the config
 	Sequence() uint64

common/configtx/manager.go

@@ -1,17 +1,7 @@
 /*
-Copyright IBM Corp. 2017 All Rights Reserved.
+Copyright IBM Corp. All Rights Reserved.
 
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-                 http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
+SPDX-License-Identifier: Apache-2.0
 */
 
 package configtx
@@ -23,7 +13,6 @@ import (
 	"github.com/hyperledger/fabric/common/configtx/api"
 	"github.com/hyperledger/fabric/common/flogging"
 	cb "github.com/hyperledger/fabric/protos/common"
-	"github.com/hyperledger/fabric/protos/utils"
 
 	"github.com/golang/protobuf/proto"
 )
@@ -42,10 +31,10 @@ var (
 )
 
 type configSet struct {
-	channelID string
-	sequence  uint64
-	configMap map[string]comparable
-	configEnv *cb.ConfigEnvelope
+	channelID   string
+	sequence    uint64
+	configMap   map[string]comparable
+	configProto *cb.Config
 }
 
 type configManager struct {
@@ -109,41 +98,31 @@ func validateChannelID(channelID string) error {
 	return nil
 }
 
-func NewManagerImpl(envConfig *cb.Envelope, initializer api.Proposer) (api.Manager, error) {
-	if envConfig == nil {
-		return nil, fmt.Errorf("Nil envelope")
-	}
-
-	configEnv := &cb.ConfigEnvelope{}
-	header, err := utils.UnmarshalEnvelopeOfType(envConfig, cb.HeaderType_CONFIG, configEnv)
-	if err != nil {
-		return nil, fmt.Errorf("Bad envelope: %s", err)
-	}
-
-	if configEnv.Config == nil {
+func NewManagerImpl(channelID string, config *cb.Config, initializer api.Proposer) (api.Manager, error) {
+	if config == nil {
 		return nil, fmt.Errorf("Nil config envelope Config")
 	}
 
-	if configEnv.Config.ChannelGroup == nil {
+	if config.ChannelGroup == nil {
 		return nil, fmt.Errorf("nil channel group")
 	}
 
-	if err := validateChannelID(header.ChannelId); err != nil {
+	if err := validateChannelID(channelID); err != nil {
 		return nil, fmt.Errorf("Bad channel id: %s", err)
 	}
 
-	configMap, err := MapConfig(configEnv.Config.ChannelGroup, initializer.RootGroupKey())
+	configMap, err := MapConfig(config.ChannelGroup, initializer.RootGroupKey())
 	if err != nil {
 		return nil, fmt.Errorf("Error converting config to map: %s", err)
 	}
 
 	return &configManager{
 		initializer: initializer,
 		current: &configSet{
-			sequence:  configEnv.Config.Sequence,
-			configMap: configMap,
-			channelID: header.ChannelId,
-			configEnv: configEnv,
+			sequence:    config.Sequence,
+			configMap:   configMap,
+			channelID:   channelID,
+			configProto: config,
 		},
 	}, nil
 }
@@ -227,6 +206,6 @@ func (cm *configManager) Sequence() uint64 {
 }
 
 // ConfigEnvelope returns the current config envelope
-func (cm *configManager) ConfigEnvelope() *cb.ConfigEnvelope {
-	return cm.current.configEnv
+func (cm *configManager) ConfigProto() *cb.Config {
+	return cm.current.configProto
 }

common/configtx/manager_test.go

@@ -46,12 +46,18 @@ func makeConfigPair(id, modificationPolicy string, lastModified uint64, data []b
 	}
 }
 
-func makeEnvelopeConfig(channelID string, configPairs ...*configPair) *cb.Envelope {
+func makeConfig(configPairs ...*configPair) *cb.Config {
 	channelGroup := cb.NewConfigGroup()
 	for _, pair := range configPairs {
 		channelGroup.Values[pair.key] = pair.value
 	}
 
+	return &cb.Config{
+		ChannelGroup: channelGroup,
+	}
+}
+
+func makeEnvelopeConfig(channelID string, configPairs ...*configPair) *cb.Envelope {
 	return &cb.Envelope{
 		Payload: utils.MarshalOrPanic(&cb.Payload{
 			Header: &cb.Header{
@@ -60,11 +66,7 @@ func makeEnvelopeConfig(channelID string, configPairs ...*configPair) *cb.Envelo
 					ChannelId: channelID,
 				}),
 			},
-			Data: utils.MarshalOrPanic(&cb.ConfigEnvelope{
-				Config: &cb.Config{
-					ChannelGroup: channelGroup,
-				},
-			}),
+			Data: utils.MarshalOrPanic(makeConfig(configPairs...)),
 		}),
 	}
 }
@@ -97,26 +99,15 @@ func makeConfigUpdateEnvelope(chainID string, readSet, writeSet *cb.ConfigGroup)
 }
 
 func TestEmptyChannel(t *testing.T) {
-	_, err := NewManagerImpl(&cb.Envelope{
-		Payload: utils.MarshalOrPanic(&cb.Payload{
-			Header: &cb.Header{
-				ChannelHeader: utils.MarshalOrPanic(&cb.ChannelHeader{
-					Type:      int32(cb.HeaderType_CONFIG),
-					ChannelId: "foo",
-				}),
-			},
-			Data: utils.MarshalOrPanic(&cb.ConfigEnvelope{
-				Config: &cb.Config{},
-			}),
-		}),
-	}, defaultInitializer())
+	_, err := NewManagerImpl("foo", &cb.Config{}, defaultInitializer())
 	assert.Error(t, err)
 }
 
 // TestDifferentChainID tests that a config update for a different chain ID fails
 func TestDifferentChainID(t *testing.T) {
 	cm, err := NewManagerImpl(
-		makeEnvelopeConfig(defaultChain, makeConfigPair("foo", "foo", 0, []byte("foo"))),
+		defaultChain,
+		makeConfig(makeConfigPair("foo", "foo", 0, []byte("foo"))),
 		defaultInitializer())
 
 	if err != nil {
@@ -134,7 +125,8 @@ func TestDifferentChainID(t *testing.T) {
 // TestOldConfigReplay tests that resubmitting a config for a sequence number which is not newer is ignored
 func TestOldConfigReplay(t *testing.T) {
 	cm, err := NewManagerImpl(
-		makeEnvelopeConfig(defaultChain, makeConfigPair("foo", "foo", 0, []byte("foo"))),
+		defaultChain,
+		makeConfig(makeConfigPair("foo", "foo", 0, []byte("foo"))),
 		defaultInitializer())
 
 	if err != nil {
@@ -152,7 +144,8 @@ func TestOldConfigReplay(t *testing.T) {
 // TestValidConfigChange tests the happy path of updating a config value with no defaultModificationPolicy
 func TestValidConfigChange(t *testing.T) {
 	cm, err := NewManagerImpl(
-		makeEnvelopeConfig(defaultChain, makeConfigPair("foo", "foo", 0, []byte("foo"))),
+		defaultChain,
+		makeConfig(makeConfigPair("foo", "foo", 0, []byte("foo"))),
 		defaultInitializer())
 
 	if err != nil {
@@ -176,7 +169,8 @@ func TestValidConfigChange(t *testing.T) {
 // config values while advancing another
 func TestConfigChangeRegressedSequence(t *testing.T) {
 	cm, err := NewManagerImpl(
-		makeEnvelopeConfig(defaultChain, makeConfigPair("foo", "foo", 1, []byte("foo"))),
+		defaultChain,
+		makeConfig(makeConfigPair("foo", "foo", 1, []byte("foo"))),
 		defaultInitializer())
 
 	if err != nil {
@@ -199,7 +193,8 @@ func TestConfigChangeRegressedSequence(t *testing.T) {
 // config values while advancing another
 func TestConfigChangeOldSequence(t *testing.T) {
 	cm, err := NewManagerImpl(
-		makeEnvelopeConfig(defaultChain, makeConfigPair("foo", "foo", 1, []byte("foo"))),
+		defaultChain,
+		makeConfig(makeConfigPair("foo", "foo", 1, []byte("foo"))),
 		defaultInitializer())
 
 	if err != nil {
@@ -225,8 +220,8 @@ func TestConfigChangeOldSequence(t *testing.T) {
 // of the config and still be accepted
 func TestConfigPartialUpdate(t *testing.T) {
 	cm, err := NewManagerImpl(
-		makeEnvelopeConfig(
-			defaultChain,
+		defaultChain,
+		makeConfig(
 			makeConfigPair("foo", "foo", 0, []byte("foo")),
 			makeConfigPair("bar", "bar", 0, []byte("bar")),
 		),
@@ -249,7 +244,8 @@ func TestConfigPartialUpdate(t *testing.T) {
 // TestEmptyConfigUpdate tests to make sure that an empty config is rejected as an update
 func TestEmptyConfigUpdate(t *testing.T) {
 	cm, err := NewManagerImpl(
-		makeEnvelopeConfig(defaultChain, makeConfigPair("foo", "foo", 0, []byte("foo"))),
+		defaultChain,
+		makeConfig(makeConfigPair("foo", "foo", 0, []byte("foo"))),
 		defaultInitializer())
 
 	if err != nil {
@@ -269,8 +265,8 @@ func TestEmptyConfigUpdate(t *testing.T) {
 // increasing the config item's LastModified
 func TestSilentConfigModification(t *testing.T) {
 	cm, err := NewManagerImpl(
-		makeEnvelopeConfig(
-			defaultChain,
+		defaultChain,
+		makeConfig(
 			makeConfigPair("foo", "foo", 0, []byte("foo")),
 			makeConfigPair("bar", "bar", 0, []byte("bar")),
 		),
@@ -300,7 +296,8 @@ func TestSilentConfigModification(t *testing.T) {
 func TestConfigChangeViolatesPolicy(t *testing.T) {
 	initializer := defaultInitializer()
 	cm, err := NewManagerImpl(
-		makeEnvelopeConfig(defaultChain, makeConfigPair("foo", "foo", 0, []byte("foo"))),
+		defaultChain,
+		makeConfig(makeConfigPair("foo", "foo", 0, []byte("foo"))),
 		initializer)
 
 	if err != nil {
@@ -322,7 +319,8 @@ func TestConfigChangeViolatesPolicy(t *testing.T) {
 func TestUnchangedConfigViolatesPolicy(t *testing.T) {
 	initializer := defaultInitializer()
 	cm, err := NewManagerImpl(
-		makeEnvelopeConfig(defaultChain, makeConfigPair("foo", "foo", 0, []byte("foo"))),
+		defaultChain,
+		makeConfig(makeConfigPair("foo", "foo", 0, []byte("foo"))),
 		initializer)
 
 	if err != nil {
@@ -355,7 +353,8 @@ func TestUnchangedConfigViolatesPolicy(t *testing.T) {
 func TestInvalidProposal(t *testing.T) {
 	initializer := defaultInitializer()
 	cm, err := NewManagerImpl(
-		makeEnvelopeConfig(defaultChain, makeConfigPair("foo", "foo", 0, []byte("foo"))),
+		defaultChain,
+		makeConfig(makeConfigPair("foo", "foo", 0, []byte("foo"))),
 		initializer)
 
 	if err != nil {
@@ -371,27 +370,3 @@ func TestInvalidProposal(t *testing.T) {
 		t.Error("Should have errored proposing config because the handler rejected it")
 	}
 }
-
-// TestMissingHeader checks that a config envelope with a missing header causes the config to be rejected
-func TestMissingHeader(t *testing.T) {
-	group := cb.NewConfigGroup()
-	group.Values["foo"] = &cb.ConfigValue{}
-	_, err := NewManagerImpl(
-		&cb.Envelope{Payload: utils.MarshalOrPanic(&cb.Payload{Data: utils.MarshalOrPanic(&cb.ConfigEnvelope{Config: &cb.Config{ChannelGroup: group}})})},
-		defaultInitializer())
-
-	if err == nil {
-		t.Error("Should have errored creating the config manager because of the missing header")
-	}
-}
-
-// TestMissingChainID checks that a config item with a missing chainID causes the config to be rejected
-func TestMissingChainID(t *testing.T) {
-	_, err := NewManagerImpl(
-		makeEnvelopeConfig("", makeConfigPair("foo", "foo", 0, []byte("foo"))),
-		defaultInitializer())
-
-	if err == nil {
-		t.Error("Should have errored creating the config manager because of the missing header")
-	}
-}

common/mocks/configtx/configtx.go

@@ -78,13 +78,13 @@ type Manager struct {
 	// ProposeConfigUpdateVal is returns as the value for ProposeConfigUpdate
 	ProposeConfigUpdateVal *cb.ConfigEnvelope
 
-	// ConfigEnvelopeVal is returned as the value for ConfigEnvelope()
-	ConfigEnvelopeVal *cb.ConfigEnvelope
+	// ConfigProtoVal is returned as the value for ConfigProtoVal()
+	ConfigProtoVal *cb.Config
 }
 
-// ConfigEnvelope returns the ConfigEnvelopeVal
-func (cm *Manager) ConfigEnvelope() *cb.ConfigEnvelope {
-	return cm.ConfigEnvelopeVal
+// ConfigProto returns the ConfigProtoVal
+func (cm *Manager) ConfigProto() *cb.Config {
+	return cm.ConfigProtoVal
 }
 
 // ConsensusType returns the ConsensusTypeVal

common/resourcesconfig/bundle.go

@@ -153,12 +153,7 @@ func (b *Bundle) NewFromChannelConfig(chanConf channelconfig.Resources) (*Bundle
 		resConf: b.resConf,
 	}
 
-	env, err := utils.CreateSignedEnvelope(cb.HeaderType_CONFIG, b.channelID, nil, &cb.ConfigEnvelope{Config: b.resConf}, 0, 0)
-	if err != nil {
-		return nil, errors.Wrap(err, "creating envelope for configtx manager failed")
-	}
-
-	result.cm, err = configtx.NewManagerImpl(env, b)
+	result.cm, err = configtx.NewManagerImpl(b.channelID, b.resConf, b)
 	if err != nil {
 		return nil, err
 	}

orderer/common/msgprocessor/systemchannel.go

@@ -277,7 +277,7 @@ func (dt *DefaultTemplator) NewChannelConfig(envConfigUpdate *cb.Envelope) (chan
 	applicationGroup.ModPolicy = channelconfig.ChannelCreationPolicyKey
 
 	// Get the current system channel config
-	systemChannelGroup := dt.support.ConfigtxManager().ConfigEnvelope().Config.ChannelGroup
+	systemChannelGroup := dt.support.ConfigtxManager().ConfigProto().ChannelGroup
 
 	// If the consortium group has no members, allow the source request to have no members.  However,
 	// if the consortium group has any members, there must be at least one member in the source request

orderer/common/msgprocessor/systemchannel_test.go

@@ -674,6 +674,6 @@ func TestNewChannelConfig(t *testing.T) {
 		assert.Nil(t, err)
 		res, err := templator.NewChannelConfig(createTx)
 		assert.Nil(t, err)
-		assert.NotEmpty(t, res.ConfigtxManager().ConfigEnvelope().Config.ChannelGroup.ModPolicy)
+		assert.NotEmpty(t, res.ConfigtxManager().ConfigProto().ChannelGroup.ModPolicy)
 	})
 }

orderer/common/multichannel/chainsupport.go

@@ -121,9 +121,9 @@ func (cs *ChainSupport) ChainID() string {
 	return cs.ConfigtxManager().ChainID()
 }
 
-// ConfigEnvelope passes through to the underlying configtxapi.Manager
-func (cs *ChainSupport) ConfigEnvelope() *cb.ConfigEnvelope {
-	return cs.ConfigtxManager().ConfigEnvelope()
+// ConfigProto passes through to the underlying configtxapi.Manager
+func (cs *ChainSupport) ConfigProto() *cb.Config {
+	return cs.ConfigtxManager().ConfigProto()
 }
 
 // Sequence passes through to the underlying configtxapi.Manager