Revert [FAB-3493] Fix LAST_CONFIG on new channels

This reverts commit 150d17e.

A simpler fix is introduced in a follow-up changeset.

Change-Id: I5c63edbf3064b4f637f71becf6ac57a6f5d8d1d3
Signed-off-by: Kostas Christidis <kostas@christidis.io>

common/configtx/manager.go

@@ -175,24 +175,15 @@ func (cm *configManager) proposeConfigUpdate(configtx *cb.Envelope) (*cb.ConfigE
 
 func (cm *configManager) prepareApply(configEnv *cb.ConfigEnvelope) (map[string]comparable, *configResult, error) {
 	if configEnv == nil {
-		return nil, nil, fmt.Errorf("attempted to apply config with nil envelope")
+		return nil, nil, fmt.Errorf("Attempted to apply config with nil envelope")
 	}
 
 	if configEnv.Config == nil {
-		return nil, nil, fmt.Errorf("config cannot be nil")
+		return nil, nil, fmt.Errorf("Config cannot be nil")
 	}
 
-	var expectedSequence uint64
-	if configEnv.Config.NewChannel {
-		expectedSequence = FixNewChannelConfig(configEnv).Config.Sequence
-		if configEnv.Config.Sequence != expectedSequence {
-			return nil, nil, fmt.Errorf("should prepare to update to %d (new channel) got %d instead", expectedSequence, configEnv.Config.Sequence)
-		}
-	} else {
-		expectedSequence = cm.current.sequence + 1
-		if configEnv.Config.Sequence != expectedSequence {
-			return nil, nil, fmt.Errorf("expected sequence %d (config at: %d), cannot prepare to update to %d", expectedSequence, cm.current.sequence, configEnv.Config.Sequence)
-		}
+	if configEnv.Config.Sequence != cm.current.sequence+1 {
+		return nil, nil, fmt.Errorf("Config at sequence %d, cannot prepare to update to %d", cm.current.sequence, configEnv.Config.Sequence)
 	}
 
 	configUpdateEnv, err := envelopeToConfigUpdate(configEnv.LastUpdate)
@@ -207,11 +198,11 @@ func (cm *configManager) prepareApply(configEnv *cb.ConfigEnvelope) (map[string]
 
 	channelGroup, err := configMapToConfig(configMap)
 	if err != nil {
-		return nil, nil, fmt.Errorf("could not turn configMap back to channelGroup: %s", err)
+		return nil, nil, fmt.Errorf("Could not turn configMap back to channelGroup: %s", err)
 	}
 
 	if !reflect.DeepEqual(channelGroup, configEnv.Config.ChannelGroup) {
-		return nil, nil, fmt.Errorf("configEnvelope LastUpdate did not produce the supplied config result")
+		return nil, nil, fmt.Errorf("ConfigEnvelope LastUpdate did not produce the supplied config result")
 	}
 
 	result, err := cm.processConfig(channelGroup)

common/configtx/util.go

@@ -97,18 +97,3 @@ func UnmarshalConfigEnvelopeOrPanic(data []byte) *cb.ConfigEnvelope {
 	}
 	return result
 }
-
-// FixNewChannelConfig is to be applied on messages of type ConfigEnvelope that
-// are used to create a new channel. It returns a ConfigEnvelope whose
-// Config.Sequence and Config.NewChannel fields are set so that the config
-// manager of the new channel starts at the right sequence number.
-func FixNewChannelConfig(configEnv *cb.ConfigEnvelope) *cb.ConfigEnvelope {
-	properSequence := uint64(0)
-	if configEnv.Config.Sequence != properSequence {
-		logger.Debugf("Received a new channel config env whose sequence is incorrectly set to %d, setting to %d instead",
-			configEnv.Config.Sequence, properSequence)
-	}
-	configEnv.Config.Sequence = properSequence
-	configEnv.Config.NewChannel = true
-	return configEnv
-}

common/configtx/util_test.go

@@ -19,10 +19,6 @@ package configtx
 import (
 	"math/rand"
 	"testing"
-
-	cb "github.com/hyperledger/fabric/protos/common"
-
-	"github.com/stretchr/testify/assert"
 )
 
 // TestValidchainID checks that the constraints on chain IDs are enforced properly
@@ -63,20 +59,6 @@ func TestValidChainID(t *testing.T) {
 	})
 }
 
-// TestFixNewChannelConfig checks that returned config envelope has its Sequence
-// and NewChannel fields set appropriately.
-func TestFixNewChannelConfig(t *testing.T) {
-	expectedSequenceValue := uint64(0)
-	expectedNewChannelValue := true
-
-	sampleConfigEnvelope := &cb.ConfigEnvelope{Config: &cb.Config{Sequence: uint64(rand.Uint32())}}
-	returnedConfigEnvelope := FixNewChannelConfig(sampleConfigEnvelope)
-
-	assert.Equal(t, expectedSequenceValue, returnedConfigEnvelope.Config.Sequence, "Sequence should be zero %d", expectedSequenceValue)
-	assert.NotNil(t, returnedConfigEnvelope.Config.NewChannel, "NewChannel field should be set")
-	assert.Equal(t, expectedNewChannelValue, returnedConfigEnvelope.Config.NewChannel, "NewChannel field should be set to %t", expectedNewChannelValue)
-}
-
 // Helper functions
 
 func randomAlphaString(size int) string {

common/mocks/configtx/configtx.go

@@ -177,7 +177,7 @@ type Manager struct {
 	// ProposeConfigUpdateError is returned as the error value for ProposeConfigUpdate
 	ProposeConfigUpdateError error
 
-	// ProposeConfigUpdateVal returns as the value for ProposeConfigUpdate
+	// ProposeConfigUpdateVal is returns as the value for ProposeConfigUpdate
 	ProposeConfigUpdateVal *cb.ConfigEnvelope
 
 	// ConfigEnvelopeVal is returned as the value for ConfigEnvelope()

orderer/common/broadcast/broadcast.go

@@ -162,7 +162,7 @@ func (bh *handlerImpl) Handle(srv ab.AtomicBroadcast_BroadcastServer) error {
 		}
 
 		if logger.IsEnabledFor(logging.DEBUG) {
-			logger.Debugf("Broadcast has successfully enqueued message of type %s for chain %s", cb.HeaderType_name[chdr.Type], chdr.ChannelId)
+			logger.Debugf("Broadcast has successfully enqueued message of type %d for chain %s", chdr.Type, chdr.ChannelId)
 		}
 
 		err = srv.Send(&ab.BroadcastResponse{Status: cb.Status_SUCCESS})

orderer/configupdate/configupdate.go

@@ -23,7 +23,6 @@ package configupdate
 import (
 	"fmt"
 
-	"github.com/hyperledger/fabric/common/configtx"
 	configtxapi "github.com/hyperledger/fabric/common/configtx/api"
 	"github.com/hyperledger/fabric/common/crypto"
 	cb "github.com/hyperledger/fabric/protos/common"
@@ -142,7 +141,7 @@ func (p *Processor) newChannelConfig(channelID string, envConfigUpdate *cb.Envel
 		return nil, err
 	}
 
-	newChannelEnvConfig, err := utils.CreateSignedEnvelope(cb.HeaderType_CONFIG, channelID, p.signer, configtx.FixNewChannelConfig(newChannelConfigEnv), msgVersion, epoch)
+	newChannelEnvConfig, err := utils.CreateSignedEnvelope(cb.HeaderType_CONFIG, channelID, p.signer, newChannelConfigEnv, msgVersion, epoch)
 	if err != nil {
 		return nil, err
 	}

orderer/configupdate/configupdate_test.go

@@ -58,9 +58,6 @@ func (msm *mockSupportManager) GetChain(chainID string) (Support, bool) {
 func (msm *mockSupportManager) NewChannelConfig(env *cb.Envelope) (configtxapi.Manager, error) {
 	return &mockconfigtx.Manager{
 		ProposeConfigUpdateVal: &cb.ConfigEnvelope{
-			Config: &cb.Config{
-				Sequence: 0,
-			},
 			LastUpdate: env,
 		},
 	}, nil

orderer/multichain/chainsupport.go

@@ -231,7 +231,6 @@ func (cs *chainSupport) addLastConfigSignature(block *cb.Block) {
 	}
 
 	lastConfigValue := utils.MarshalOrPanic(&cb.LastConfig{Index: cs.lastConfig})
-	logger.Debugf("[channel: %s] About to write block, setting its LAST_CONFIG to %d", cs.ChainID(), cs.lastConfig)
 
 	lastConfigSignature.Signature = utils.SignOrPanic(cs.signer, util.ConcatenateBytes(lastConfigValue, lastConfigSignature.SignatureHeader, block.Header.Bytes()))
 
@@ -258,8 +257,6 @@ func (cs *chainSupport) WriteBlock(block *cb.Block, committers []filter.Committe
 	if err != nil {
 		logger.Panicf("[channel: %s] Could not append block: %s", cs.ChainID(), err)
 	}
-
-	logger.Debugf("[channel: %s] Wrote block %d", cs.ChainID(), block.GetHeader().Number)
 	return block
 }
 

orderer/multichain/manager.go

@@ -290,7 +290,7 @@ func (ml *multiLedger) NewChannelConfig(envConfigUpdate *cb.Envelope) (configtxa
 	channelGroup.Groups[config.ApplicationGroupKey] = applicationGroup
 	channelGroup.Values[config.ConsortiumKey] = config.TemplateConsortium(consortium.Name).Values[config.ConsortiumKey]
 
-	templateConfig, _ := utils.CreateSignedEnvelope(cb.HeaderType_CONFIG, configUpdate.ChannelId, ml.signer, &cb.ConfigEnvelope{
+	templateConfig, err := utils.CreateSignedEnvelope(cb.HeaderType_CONFIG, configUpdate.ChannelId, ml.signer, &cb.ConfigEnvelope{
 		Config: &cb.Config{
 			ChannelGroup: channelGroup,
 		},

orderer/multichain/manager_test.go

@@ -21,7 +21,6 @@ import (
 	"testing"
 	"time"
 
-	"github.com/golang/protobuf/proto"
 	"github.com/hyperledger/fabric/common/configtx"
 	genesisconfig "github.com/hyperledger/fabric/common/configtx/tool/localconfig"
 	"github.com/hyperledger/fabric/common/configtx/tool/provisional"
@@ -254,7 +253,7 @@ func TestNewChain(t *testing.T) {
 	configEnv, err := cm.ProposeConfigUpdate(envConfigUpdate)
 	assert.NoError(t, err, "Proposing initial update")
 
-	ingressTx, err := utils.CreateSignedEnvelope(cb.HeaderType_CONFIG, newChainID, mockCrypto(), configtx.FixNewChannelConfig(configEnv), msgVersion, epoch)
+	ingressTx, err := utils.CreateSignedEnvelope(cb.HeaderType_CONFIG, newChainID, mockCrypto(), configEnv, msgVersion, epoch)
 	assert.NoError(t, err, "Creating ingresstx")
 
 	wrapped := wrapConfigTx(ingressTx)
@@ -316,16 +315,6 @@ func TestNewChain(t *testing.T) {
 		if status != cb.Status_SUCCESS {
 			t.Fatalf("Could not retrieve block on new chain")
 		}
-
-		// Inspect LAST_CONFIG value
-		metadataItem := &cb.Metadata{}
-		err := proto.Unmarshal(block.Metadata.Metadata[cb.BlockMetadataIndex_LAST_CONFIG], metadataItem)
-		assert.NoError(t, err, "Block should carry LAST_CONFIG metadata item")
-		lastConfig := &cb.LastConfig{}
-		err = proto.Unmarshal(metadataItem.Value, lastConfig)
-		assert.NoError(t, err, "LAST_CONFIG metadata item should carry last config value")
-		assert.Equal(t, uint64(0), lastConfig.Index, "LAST_CONFIG value should point to genesis block")
-
 		for i := 0; i < int(conf.Orderer.BatchSize.MaxMessageCount); i++ {
 			if !reflect.DeepEqual(utils.ExtractEnvelopeOrPanic(block, i), messages[i]) {
 				t.Errorf("Block contents wrong at index %d in new chain", i)

orderer/multichain/systemchain.go

@@ -129,7 +129,7 @@ func (scf *systemChainFilter) authorize(configEnvelope *cb.ConfigEnvelope) (conf
 		return nil, err
 	}
 
-	err = configManager.Apply(configtx.FixNewChannelConfig(newChannelConfigEnv))
+	err = configManager.Apply(newChannelConfigEnv)
 	if err != nil {
 		return nil, err
 	}

orderer/multichain/systemchain_test.go

@@ -32,8 +32,6 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-var newChannelID = "newChannel"
-
 type mockSupport struct {
 	msc *mockconfig.Orderer
 }
@@ -73,94 +71,96 @@ func (mcc *mockChainCreator) NewChannelConfig(envConfigUpdate *cb.Envelope) (con
 	if mcc.NewChannelConfigErr != nil {
 		return nil, mcc.NewChannelConfigErr
 	}
-
-	pldConfigUpdate := utils.UnmarshalPayloadOrPanic(envConfigUpdate.Payload)
-	configUpdateEnv := configtx.UnmarshalConfigUpdateEnvelopeOrPanic(pldConfigUpdate.Data)
-	configUpdate := configtx.UnmarshalConfigUpdateOrPanic(configUpdateEnv.ConfigUpdate)
-
-	configEnvelopeVal := &cb.ConfigEnvelope{
-		Config: &cb.Config{ChannelGroup: configUpdate.WriteSet},
-	}
-
-	proposeConfigUpdateVal := configEnvelopeVal
-	proposeConfigUpdateVal.Config.Sequence = 1
-	proposeConfigUpdateVal.LastUpdate = envConfigUpdate
-
-	ctxm := &mockconfigtx.Manager{
-		ConfigEnvelopeVal:      configEnvelopeVal,
-		ProposeConfigUpdateVal: proposeConfigUpdateVal,
-	}
-
-	return ctxm, nil
+	confUpdate := configtx.UnmarshalConfigUpdateOrPanic(configtx.UnmarshalConfigUpdateEnvelopeOrPanic(utils.UnmarshalPayloadOrPanic(envConfigUpdate.Payload).Data).ConfigUpdate)
+	return &mockconfigtx.Manager{
+		ConfigEnvelopeVal: &cb.ConfigEnvelope{
+			Config:     &cb.Config{Sequence: 1, ChannelGroup: confUpdate.WriteSet},
+			LastUpdate: envConfigUpdate,
+		},
+	}, nil
 }
 
 func TestGoodProposal(t *testing.T) {
+	newChainID := "NewChainID"
+
 	mcc := newMockChainCreator()
 
-	configUpdateEnv, _ := configtx.NewCompositeTemplate(
+	configEnv, err := configtx.NewCompositeTemplate(
 		configtx.NewSimpleTemplate(
 			config.DefaultHashingAlgorithm(),
 			config.DefaultBlockDataHashingStructure(),
 			config.TemplateOrdererAddresses([]string{"foo"}),
 		),
 		configtx.NewChainCreationTemplate("SampleConsortium", []string{}),
-	).Envelope(newChannelID)
-
-	ingressTx := makeConfigTxFromConfigUpdateEnvelope(newChannelID, configUpdateEnv, true)
-	ordererTx := wrapConfigTx(ingressTx)
+	).Envelope(newChainID)
+	if err != nil {
+		t.Fatalf("Error constructing configtx")
+	}
+	ingressTx := makeConfigTxFromConfigUpdateEnvelope(newChainID, configEnv)
+	wrapped := wrapConfigTx(ingressTx)
 
 	sysFilter := newSystemChainFilter(mcc.ms, mcc)
-	action, committer := sysFilter.Apply(ordererTx)
+	action, committer := sysFilter.Apply(wrapped)
+
 	assert.EqualValues(t, action, filter.Accept, "Did not accept valid transaction")
 	assert.True(t, committer.Isolated(), "Channel creation belong in its own block")
 
 	committer.Commit()
 	assert.Len(t, mcc.newChains, 1, "Proposal should only have created 1 new chain")
+
 	assert.Equal(t, ingressTx, mcc.newChains[0], "New chain should have been created with ingressTx")
 }
 
 func TestProposalRejectedByConfig(t *testing.T) {
+	newChainID := "NewChainID"
+
 	mcc := newMockChainCreator()
 	mcc.NewChannelConfigErr = fmt.Errorf("Error creating channel")
 
-	configUpdateEnv, _ := configtx.NewCompositeTemplate(
+	configEnv, err := configtx.NewCompositeTemplate(
 		configtx.NewSimpleTemplate(
 			config.DefaultHashingAlgorithm(),
 			config.DefaultBlockDataHashingStructure(),
 			config.TemplateOrdererAddresses([]string{"foo"}),
 		),
 		configtx.NewChainCreationTemplate("SampleConsortium", []string{}),
-	).Envelope(newChannelID)
-
-	ingressTx := makeConfigTxFromConfigUpdateEnvelope(newChannelID, configUpdateEnv, true)
-	ordererTx := wrapConfigTx(ingressTx)
+	).Envelope(newChainID)
+	if err != nil {
+		t.Fatalf("Error constructing configtx")
+	}
+	ingressTx := makeConfigTxFromConfigUpdateEnvelope(newChainID, configEnv)
+	wrapped := wrapConfigTx(ingressTx)
 
 	sysFilter := newSystemChainFilter(mcc.ms, mcc)
-	action, _ := sysFilter.Apply(ordererTx)
+	action, _ := sysFilter.Apply(wrapped)
 
-	assert.EqualValues(t, action, filter.Reject, "Did not reject invalid transaction")
+	assert.EqualValues(t, action, filter.Reject, "Did not accept valid transaction")
 	assert.Len(t, mcc.newChains, 0, "Proposal should not have created a new chain")
 }
 
 func TestNumChainsExceeded(t *testing.T) {
+	newChainID := "NewChainID"
+
 	mcc := newMockChainCreator()
 	mcc.ms.msc.MaxChannelsCountVal = 1
 	mcc.newChains = make([]*cb.Envelope, 2)
 
-	configUpdateEnv, _ := configtx.NewCompositeTemplate(
+	configEnv, err := configtx.NewCompositeTemplate(
 		configtx.NewSimpleTemplate(
 			config.DefaultHashingAlgorithm(),
 			config.DefaultBlockDataHashingStructure(),
 			config.TemplateOrdererAddresses([]string{"foo"}),
 		),
 		configtx.NewChainCreationTemplate("SampleConsortium", []string{}),
-	).Envelope(newChannelID)
-
-	ingressTx := makeConfigTxFromConfigUpdateEnvelope(newChannelID, configUpdateEnv, true)
-	ordererTx := wrapConfigTx(ingressTx)
+	).Envelope(newChainID)
+	if err != nil {
+		t.Fatalf("Error constructing configtx")
+	}
+	ingressTx := makeConfigTxFromConfigUpdateEnvelope(newChainID, configEnv)
+	wrapped := wrapConfigTx(ingressTx)
 
 	sysFilter := newSystemChainFilter(mcc.ms, mcc)
-	action, _ := sysFilter.Apply(ordererTx)
+	action, _ := sysFilter.Apply(wrapped)
 
 	assert.EqualValues(t, filter.Reject, action, "Transaction had created too many channels")
 }

orderer/multichain/util_test.go

@@ -93,7 +93,7 @@ func makeConfigTx(chainID string, i int) *cb.Envelope {
 	if err != nil {
 		panic(err)
 	}
-	return makeConfigTxFromConfigUpdateEnvelope(chainID, configEnv, false)
+	return makeConfigTxFromConfigUpdateEnvelope(chainID, configEnv)
 }
 
 func wrapConfigTx(env *cb.Envelope) *cb.Envelope {
@@ -104,30 +104,18 @@ func wrapConfigTx(env *cb.Envelope) *cb.Envelope {
 	return result
 }
 
-func makeConfigTxFromConfigUpdateEnvelope(chainID string, configUpdateEnv *cb.ConfigUpdateEnvelope, newChannel bool) *cb.Envelope {
+func makeConfigTxFromConfigUpdateEnvelope(chainID string, configUpdateEnv *cb.ConfigUpdateEnvelope) *cb.Envelope {
 	configUpdateTx, err := utils.CreateSignedEnvelope(cb.HeaderType_CONFIG_UPDATE, chainID, mockCrypto(), configUpdateEnv, msgVersion, epoch)
 	if err != nil {
 		panic(err)
 	}
-
-	configEnv := &cb.ConfigEnvelope{
-		Config: &cb.Config{
-			Sequence:     1,
-			ChannelGroup: configtx.UnmarshalConfigUpdateOrPanic(configUpdateEnv.ConfigUpdate).WriteSet,
-		},
-		LastUpdate: configUpdateTx,
-	}
-
-	if newChannel {
-		configEnv = configtx.FixNewChannelConfig(configEnv)
-	}
-
-	configTx, err := utils.CreateSignedEnvelope(cb.HeaderType_CONFIG, chainID, mockCrypto(), configEnv,
+	configTx, err := utils.CreateSignedEnvelope(cb.HeaderType_CONFIG, chainID, mockCrypto(), &cb.ConfigEnvelope{
+		Config:     &cb.Config{Sequence: 1, ChannelGroup: configtx.UnmarshalConfigUpdateOrPanic(configUpdateEnv.ConfigUpdate).WriteSet},
+		LastUpdate: configUpdateTx},
 		msgVersion, epoch)
 	if err != nil {
 		panic(err)
 	}
-
 	return configTx
 }