From eb65b8ec458cc451632b9075c90226ef3e65cf79 Mon Sep 17 00:00:00 2001 From: Jay Guo Date: Tue, 5 Nov 2019 17:48:15 +0800 Subject: [PATCH] Convert creation block to genesis block early on As pointed out in previous review, ReplicateChains should consistently accept channel genesis blocks and replicate channels, but today it accepts one of - channel creation block, in case channel is made aware via replaying system channel - channel genesis block, in case channel is already being tracked by inactive chain registry However, this has made ChannelCreationBlockToGenesisBlock method misleading, and this conversion should be made upfront when channel creation block is pulled from other orderers. Furthermore, since this method is a superset of `IsNewChannelBlock`, they are merged together to inspect config block and attempt to extract genesis block in one call, to avoid repeat unmarshalling. Note, several UT are removed since they are already covered by other tests, namely nil block, nil block data and malformed block data. FAB-13552 #done Change-Id: If38f2afe454e3b6cc3c75a38af75a5c0a24e87aa Signed-off-by: Jay Guo --- orderer/common/cluster/replication.go | 106 +++++++++------------ orderer/common/cluster/replication_test.go | 91 ++++-------------- 2 files changed, 62 insertions(+), 135 deletions(-) diff --git a/orderer/common/cluster/replication.go b/orderer/common/cluster/replication.go index ac29f7af748..94478a0c8de 100644 --- a/orderer/common/cluster/replication.go +++ b/orderer/common/cluster/replication.go @@ -148,12 +148,7 @@ func (r *Replicator) ReplicateChains() []string { continue } - gb, err := ChannelCreationBlockToGenesisBlock(channel.GenesisBlock) - if err != nil { - r.Logger.Panicf("Failed converting channel creation block for channel %s to genesis block: %v", - channel.ChannelName, err) - } - r.appendBlock(gb, ledger, channel.ChannelName) + r.appendBlock(channel.GenesisBlock, ledger, channel.ChannelName) } } @@ -559,23 +554,25 @@ func (ci *ChainInspector) Channels() []ChannelGenesisBlock { ci.Logger.Panicf("Failed pulling block [%d] from the system channel", seq) } ci.validateHashPointer(block, prevHash) - channel, err := IsNewChannelBlock(block) + // Set the previous hash for the next iteration + prevHash = protoutil.BlockHeaderHash(block.Header) + + channel, gb, err := ExtractGenesisBlock(ci.Logger, block) if err != nil { - // If we failed to classify a block, something is wrong in the system chain + // If we failed to inspect a block, something is wrong in the system chain // we're trying to pull, so abort. - ci.Logger.Panicf("Failed classifying block [%d]: %s", seq, err) - continue + ci.Logger.Panicf("Failed extracting channel genesis block from config block: %v", err) } - // Set the previous hash for the next iteration - prevHash = protoutil.BlockHeaderHash(block.Header) + if channel == "" { ci.Logger.Info("Block", seq, "doesn't contain a new channel") continue } + ci.Logger.Info("Block", seq, "contains channel", channel) channels[channel] = ChannelGenesisBlock{ ChannelName: channel, - GenesisBlock: block, + GenesisBlock: gb, } } // At this point, block holds reference to the last block pulled. @@ -610,83 +607,72 @@ func flattenChannelMap(m map[string]ChannelGenesisBlock) []ChannelGenesisBlock { return res } -// ChannelCreationBlockToGenesisBlock converts a channel creation block to a genesis block -func ChannelCreationBlockToGenesisBlock(block *common.Block) (*common.Block, error) { - if block == nil { - return nil, errors.New("nil block") - } - env, err := protoutil.ExtractEnvelope(block, 0) - if err != nil { - return nil, err - } - payload, err := protoutil.UnmarshalPayload(env.Payload) - if err != nil { - return nil, err - } - block.Data.Data = [][]byte{payload.Data} - block.Header.DataHash = protoutil.BlockDataHash(block.Data) - block.Header.Number = 0 - block.Header.PreviousHash = nil - metadata := &common.BlockMetadata{ - Metadata: make([][]byte, 4), - } - block.Metadata = metadata - metadata.Metadata[common.BlockMetadataIndex_LAST_CONFIG] = protoutil.MarshalOrPanic(&common.Metadata{ - Value: protoutil.MarshalOrPanic(&common.LastConfig{Index: 0}), - // This is a genesis block, peer never verify this signature because we can't bootstrap - // trust from an earlier block, hence there are no signatures here. - }) - return block, nil -} - -// IsNewChannelBlock returns a name of the channel in case -// it holds a channel create transaction, or empty string otherwise. -func IsNewChannelBlock(block *common.Block) (string, error) { +// ExtractGenesisBlock determines if a config block creates new channel, in which +// case it returns channel name, genesis block and nil error. +func ExtractGenesisBlock(logger *flogging.FabricLogger, block *common.Block) (string, *common.Block, error) { if block == nil { - return "", errors.New("nil block") + return "", nil, errors.New("nil block") } env, err := protoutil.ExtractEnvelope(block, 0) if err != nil { - return "", err + return "", nil, err } payload, err := protoutil.UnmarshalPayload(env.Payload) if err != nil { - return "", err + return "", nil, err } if payload.Header == nil { - return "", errors.New("nil header in payload") + return "", nil, errors.New("nil header in payload") } chdr, err := protoutil.UnmarshalChannelHeader(payload.Header.ChannelHeader) if err != nil { - return "", err + return "", nil, err } - // The transaction is an orderer transaction + // The transaction is not orderer transaction if common.HeaderType(chdr.Type) != common.HeaderType_ORDERER_TRANSACTION { - return "", nil + return "", nil, nil } systemChannelName := chdr.ChannelId innerEnvelope, err := protoutil.UnmarshalEnvelope(payload.Data) if err != nil { - return "", err + return "", nil, err } innerPayload, err := protoutil.UnmarshalPayload(innerEnvelope.Payload) if err != nil { - return "", err + return "", nil, err } if innerPayload.Header == nil { - return "", errors.New("inner payload's header is nil") + return "", nil, errors.New("inner payload's header is nil") } chdr, err = protoutil.UnmarshalChannelHeader(innerPayload.Header.ChannelHeader) if err != nil { - return "", err + return "", nil, err } - // The inner payload's header is a config transaction + // The inner payload's header should be a config transaction if common.HeaderType(chdr.Type) != common.HeaderType_CONFIG { - return "", nil + logger.Warnf("Expecting %s envelope in block, got %s", common.HeaderType_CONFIG, common.HeaderType(chdr.Type)) + return "", nil, nil } // In any case, exclude all system channel transactions if chdr.ChannelId == systemChannelName { - return "", nil + logger.Warnf("Expecting config envelope in %s block to target a different "+ + "channel other than system channel '%s'", common.HeaderType_ORDERER_TRANSACTION, systemChannelName) + return "", nil, nil + } + + metadata := &common.BlockMetadata{ + Metadata: make([][]byte, 4), + } + metadata.Metadata[common.BlockMetadataIndex_LAST_CONFIG] = protoutil.MarshalOrPanic(&common.Metadata{ + Value: protoutil.MarshalOrPanic(&common.LastConfig{Index: 0}), + // This is a genesis block, peer never verify this signature because we can't bootstrap + // trust from an earlier block, hence there are no signatures here. + }) + blockdata := &common.BlockData{Data: [][]byte{payload.Data}} + b := &common.Block{ + Header: &common.BlockHeader{DataHash: protoutil.BlockDataHash(blockdata)}, + Data: blockdata, + Metadata: metadata, } - return chdr.ChannelId, nil + return chdr.ChannelId, b, nil } diff --git a/orderer/common/cluster/replication_test.go b/orderer/common/cluster/replication_test.go index c13fde144ac..fc1f5d9d09a 100644 --- a/orderer/common/cluster/replication_test.go +++ b/orderer/common/cluster/replication_test.go @@ -174,31 +174,6 @@ func TestReplicateChainsFailures(t *testing.T) { ledgerFactoryError: errors.New("IO error"), expectedPanic: "Failed to create a ledger for channel channelWeAreNotPartOf: IO error", }, - { - name: "pulled genesis block is malformed", - latestBlockSeqInOrderer: 21, - channelsReturns: []cluster.ChannelGenesisBlock{ - {ChannelName: "channelWeAreNotPartOf", GenesisBlock: &common.Block{Header: &common.BlockHeader{}}}, - }, - expectedPanic: "Failed converting channel creation block for channel channelWeAreNotPartOf to genesis" + - " block: block data is nil", - }, - { - name: "pulled genesis block is malformed - bad payload", - latestBlockSeqInOrderer: 21, - channelsReturns: []cluster.ChannelGenesisBlock{ - {ChannelName: "channelWeAreNotPartOf", GenesisBlock: &common.Block{ - Header: &common.BlockHeader{}, - Data: &common.BlockData{ - Data: [][]byte{protoutil.MarshalOrPanic(&common.Envelope{ - Payload: []byte{1, 2, 3}, - })}, - }, - }}, - }, - expectedPanic: "Failed converting channel creation block for channel channelWeAreNotPartOf" + - " to genesis block: error unmarshaling Payload: proto: common.Payload: illegal tag 0 (wire type 1)", - }, } { t.Run(testCase.name, func(t *testing.T) { systemChannelBlocks := createBlockChain(0, 21) @@ -1136,12 +1111,13 @@ func injectTLSCACert(t *testing.T, block *common.Block, tlsCA []byte) { block.Data.Data[0] = protoutil.MarshalOrPanic(env) } -func TestIsNewChannelBlock(t *testing.T) { +func TestExtractGenesisBlock(t *testing.T) { for _, testCase := range []struct { - name string - expectedErr string - returnedName string - block *common.Block + name string + expectedErr string + returnedName string + block *common.Block + returnGenesisBlock bool }{ { name: "nil block", @@ -1381,16 +1357,22 @@ func TestIsNewChannelBlock(t *testing.T) { })}, }, }, + returnGenesisBlock: true, }, } { t.Run(testCase.name, func(t *testing.T) { - channelName, err := cluster.IsNewChannelBlock(testCase.block) + channelName, gb, err := cluster.ExtractGenesisBlock(flogging.MustGetLogger("test"), testCase.block) if testCase.expectedErr != "" { assert.EqualError(t, err, testCase.expectedErr) } else { assert.NoError(t, err) } assert.Equal(t, testCase.returnedName, channelName) + if testCase.returnGenesisBlock { + assert.NotNil(t, gb) + } else { + assert.Nil(t, gb) + } }) } } @@ -1494,9 +1476,9 @@ func TestChannels(t *testing.T) { systemChain[len(systemChain)-2].Data.Data = [][]byte{{1, 2, 3}} }, assertion: func(t *testing.T, ci *cluster.ChainInspector) { - panicValue := "Failed classifying block [2]: block data does not carry" + - " an envelope at index 0: error unmarshaling Envelope: " + - "proto: common.Envelope: illegal tag 0 (wire type 1)" + panicValue := "Failed extracting channel genesis block from config block: " + + "block data does not carry an envelope at index 0: error unmarshaling " + + "Envelope: proto: common.Envelope: illegal tag 0 (wire type 1)" assert.PanicsWithValue(t, panicValue, func() { ci.Channels() }) @@ -1603,47 +1585,6 @@ func simulateNonParticipantChannelPull(osn *deliverServer) { osn.blockResponses <- nil } -func TestChannelCreationBlockToGenesisBlock(t *testing.T) { - for _, testCase := range []struct { - name string - expectedErr string - block *common.Block - }{ - { - name: "nil block", - expectedErr: "nil block", - }, - { - name: "no data", - expectedErr: "block data is nil", - block: &common.Block{}, - }, - { - name: "no block data", - expectedErr: "envelope index out of bounds", - block: &common.Block{ - Data: &common.BlockData{}, - }, - }, - { - name: "bad block data", - expectedErr: "block data does not carry an envelope at index 0:" + - " error unmarshaling Envelope: proto: common.Envelope:" + - " illegal tag 0 (wire type 1)", - block: &common.Block{ - Data: &common.BlockData{ - Data: [][]byte{{1, 2, 3}}, - }, - }, - }, - } { - t.Run(testCase.name, func(t *testing.T) { - _, err := cluster.ChannelCreationBlockToGenesisBlock(testCase.block) - assert.EqualError(t, err, testCase.expectedErr) - }) - } -} - func TestFilter(t *testing.T) { logger := flogging.MustGetLogger("test") logger = logger.WithOptions(zap.Hooks(func(entry zapcore.Entry) error {