FAB-15817 Prefer last config in signatures metadata field (#363)

This commit updates the orderer and peer to prefer retrieving the
last config from the SIGNATURES field and fallback to retrieving
it from the LAST_CONFIG field.

Signed-off-by: Will Lahti <wtlahti@us.ibm.com>

common/genesis/genesis.go

@@ -48,5 +48,10 @@ func (f *factory) Block(channelID string) *cb.Block {
 	block.Metadata.Metadata[cb.BlockMetadataIndex_LAST_CONFIG] = protoutil.MarshalOrPanic(&cb.Metadata{
 		Value: protoutil.MarshalOrPanic(&cb.LastConfig{Index: 0}),
 	})
+	block.Metadata.Metadata[cb.BlockMetadataIndex_SIGNATURES] = protoutil.MarshalOrPanic(&cb.Metadata{
+		Value: protoutil.MarshalOrPanic(&cb.OrdererBlockMetadata{
+			LastConfig: &cb.LastConfig{Index: 0},
+		}),
+	})
 	return block
 }

common/genesis/genesis_test.go

@@ -9,20 +9,39 @@ package genesis
 import (
 	"testing"
 
+	"github.com/gogo/protobuf/proto"
+	cb "github.com/hyperledger/fabric-protos-go/common"
 	"github.com/hyperledger/fabric/protoutil"
 	"github.com/stretchr/testify/assert"
 )
 
-func TestBasicSanity(t *testing.T) {
-	impl := NewFactoryImpl(protoutil.NewConfigGroup())
-	impl.Block("testchannelid")
-}
-
-func TestForTransactionID(t *testing.T) {
+func TestFactory(t *testing.T) {
 	impl := NewFactoryImpl(protoutil.NewConfigGroup())
 	block := impl.Block("testchannelid")
-	configEnv, _ := protoutil.ExtractEnvelope(block, 0)
-	configEnvPayload, _ := protoutil.UnmarshalPayload(configEnv.Payload)
-	configEnvPayloadChannelHeader, _ := protoutil.UnmarshalChannelHeader(configEnvPayload.GetHeader().ChannelHeader)
-	assert.NotEmpty(t, configEnvPayloadChannelHeader.TxId, "tx_id of configuration transaction should not be empty")
+
+	t.Run("test for transaction id", func(t *testing.T) {
+		configEnv, _ := protoutil.ExtractEnvelope(block, 0)
+		configEnvPayload, _ := protoutil.UnmarshalPayload(configEnv.Payload)
+		configEnvPayloadChannelHeader, _ := protoutil.UnmarshalChannelHeader(configEnvPayload.GetHeader().ChannelHeader)
+		assert.NotEmpty(t, configEnvPayloadChannelHeader.TxId, "tx_id of configuration transaction should not be empty")
+	})
+	t.Run("test for last config in SIGNATURES field", func(t *testing.T) {
+		metadata := &cb.Metadata{}
+		err := proto.Unmarshal(block.Metadata.Metadata[cb.BlockMetadataIndex_SIGNATURES], metadata)
+		assert.NoError(t, err)
+		ordererBlockMetadata := &cb.OrdererBlockMetadata{}
+		err = proto.Unmarshal(metadata.Value, ordererBlockMetadata)
+		assert.NoError(t, err)
+		assert.NotNil(t, ordererBlockMetadata.LastConfig)
+		assert.Equal(t, uint64(0), ordererBlockMetadata.LastConfig.Index)
+	})
+	t.Run("test for last config in LAST_CONFIG field", func(t *testing.T) {
+		metadata := &cb.Metadata{}
+		err := proto.Unmarshal(block.Metadata.Metadata[cb.BlockMetadataIndex_LAST_CONFIG], metadata)
+		assert.NoError(t, err)
+		lastConfig := &cb.LastConfig{}
+		err = proto.Unmarshal(metadata.Value, lastConfig)
+		assert.NoError(t, err)
+		assert.Equal(t, uint64(0), lastConfig.Index)
+	})
 }

common/ledger/blockledger/util.go

@@ -44,6 +44,7 @@ func (nfei *NotFoundErrorIterator) Close() {}
 func CreateNextBlock(rl Reader, messages []*cb.Envelope) *cb.Block {
 	var nextBlockNumber uint64
 	var previousBlockHash []byte
+	var err error
 
 	if rl.Height() > 0 {
 		it, _ := rl.Iterator(&ab.SeekPosition{
@@ -63,7 +64,6 @@ func CreateNextBlock(rl Reader, messages []*cb.Envelope) *cb.Block {
 		Data: make([][]byte, len(messages)),
 	}
 
-	var err error
 	for i, msg := range messages {
 		data.Data[i], err = proto.Marshal(msg)
 		if err != nil {

internal/peer/channel/fetch_test.go

@@ -166,20 +166,18 @@ func getMockDeliverService(block *cb.Block) *mock.DeliverService {
 }
 
 func createTestBlock() *cb.Block {
-	lc := &cb.LastConfig{Index: 0}
-	lcBytes := protoutil.MarshalOrPanic(lc)
-	metadata := &cb.Metadata{
-		Value: lcBytes,
-	}
-	metadataBytes := protoutil.MarshalOrPanic(metadata)
-	blockMetadata := make([][]byte, cb.BlockMetadataIndex_LAST_CONFIG+1)
-	blockMetadata[cb.BlockMetadataIndex_LAST_CONFIG] = metadataBytes
+	metadataBytes := protoutil.MarshalOrPanic(&cb.Metadata{
+		Value: protoutil.MarshalOrPanic(&cb.OrdererBlockMetadata{
+			LastConfig: &cb.LastConfig{Index: 0},
+		}),
+	})
+
 	block := &cb.Block{
 		Header: &cb.BlockHeader{
 			Number: 0,
 		},
 		Metadata: &cb.BlockMetadata{
-			Metadata: blockMetadata,
+			Metadata: [][]byte{metadataBytes},
 		},
 	}
 

orderer/common/cluster/replication.go

@@ -479,7 +479,7 @@ func PullLastConfigBlock(puller ChainPuller) (*common.Block, error) {
 	if lastBlock == nil {
 		return nil, ErrRetryCountExhausted
 	}
-	lastConfNumber, err := lastConfigFromBlock(lastBlock)
+	lastConfNumber, err := protoutil.GetLastConfigIndexFromBlock(lastBlock)
 	if err != nil {
 		return nil, err
 	}
@@ -510,13 +510,6 @@ func latestHeightAndEndpoint(puller ChainPuller) (string, uint64, error) {
 	return mostUpToDateEndpoint, maxHeight, nil
 }
 
-func lastConfigFromBlock(block *common.Block) (uint64, error) {
-	if block.Metadata == nil || len(block.Metadata.Metadata) <= int(common.BlockMetadataIndex_LAST_CONFIG) {
-		return 0, errors.New("no metadata in block")
-	}
-	return protoutil.GetLastConfigIndexFromBlock(block)
-}
-
 // Close closes the ChainInspector
 func (ci *ChainInspector) Close() {
 	ci.Puller.Close()
@@ -663,11 +656,17 @@ func ExtractGenesisBlock(logger *flogging.FabricLogger, block *common.Block) (st
 	metadata := &common.BlockMetadata{
 		Metadata: make([][]byte, 4),
 	}
+	metadata.Metadata[common.BlockMetadataIndex_SIGNATURES] = protoutil.MarshalOrPanic(&common.OrdererBlockMetadata{
+		LastConfig: &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.
+	})
 	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)},

orderer/common/cluster/replication_test.go

@@ -724,34 +724,48 @@ func TestParticipant(t *testing.T) {
 			},
 			latestBlockSeq: uint64(99),
 			latestBlock:    &common.Block{},
-			expectedError:  "no metadata in block",
+			expectedError:  "failed to retrieve metadata: no metadata in block",
 		},
 		{
 			name: "Pulled block has no last config sequence in metadata",
 			heightsByEndpoints: map[string]uint64{
 				"orderer.example.com:7050": 100,
 			},
 			latestBlockSeq: uint64(99),
+			latestBlock: &common.Block{
+				Metadata: &common.BlockMetadata{
+					Metadata: nil,
+				},
+			},
+			expectedError: "failed to retrieve metadata: no metadata at index [SIGNATURES]",
+		},
+		{
+			name: "Pulled block's SIGNATURES metadata is malformed",
+			heightsByEndpoints: map[string]uint64{
+				"orderer.example.com:7050": 100,
+			},
+			latestBlockSeq: uint64(99),
 			latestBlock: &common.Block{
 				Metadata: &common.BlockMetadata{
 					Metadata: [][]byte{{1, 2, 3}},
 				},
 			},
-			expectedError: "no metadata in block",
+			expectedError: "failed to retrieve metadata: error unmarshaling metadata" +
+				" at index [SIGNATURES]: proto: common.Metadata: illegal tag 0 (wire type 1)",
 		},
 		{
-			name: "Pulled block's metadata is malformed",
+			name: "Pulled block's LAST_CONFIG metadata is malformed",
 			heightsByEndpoints: map[string]uint64{
 				"orderer.example.com:7050": 100,
 			},
 			latestBlockSeq: uint64(99),
 			latestBlock: &common.Block{
 				Metadata: &common.BlockMetadata{
-					Metadata: [][]byte{{1, 2, 3}, {1, 2, 3}},
+					Metadata: [][]byte{{}, {1, 2, 3}},
 				},
 			},
-			expectedError: "error unmarshaling metadata from" +
-				" block at index [LAST_CONFIG]: proto: common.Metadata: illegal tag 0 (wire type 1)",
+			expectedError: "failed to retrieve metadata: error unmarshaling metadata" +
+				" at index [LAST_CONFIG]: proto: common.Metadata: illegal tag 0 (wire type 1)",
 		},
 		{
 			name: "Pulled block's metadata is valid and has a last config",
@@ -761,9 +775,9 @@ func TestParticipant(t *testing.T) {
 			latestBlockSeq: uint64(99),
 			latestBlock: &common.Block{
 				Metadata: &common.BlockMetadata{
-					Metadata: [][]byte{{1, 2, 3}, protoutil.MarshalOrPanic(&common.Metadata{
-						Value: protoutil.MarshalOrPanic(&common.LastConfig{
-							Index: 42,
+					Metadata: [][]byte{protoutil.MarshalOrPanic(&common.Metadata{
+						Value: protoutil.MarshalOrPanic(&common.OrdererBlockMetadata{
+							LastConfig: &common.LastConfig{Index: 42},
 						}),
 					})},
 				},
@@ -790,9 +804,9 @@ func TestParticipant(t *testing.T) {
 			latestBlockSeq: uint64(99),
 			latestBlock: &common.Block{
 				Metadata: &common.BlockMetadata{
-					Metadata: [][]byte{{1, 2, 3}, protoutil.MarshalOrPanic(&common.Metadata{
-						Value: protoutil.MarshalOrPanic(&common.LastConfig{
-							Index: 42,
+					Metadata: [][]byte{protoutil.MarshalOrPanic(&common.Metadata{
+						Value: protoutil.MarshalOrPanic(&common.OrdererBlockMetadata{
+							LastConfig: &common.LastConfig{Index: 42},
 						}),
 					})},
 				},
@@ -819,7 +833,7 @@ func TestParticipant(t *testing.T) {
 				assert.Len(t, configBlocks, 0)
 			} else {
 				assert.Len(t, configBlocks, 1)
-				assert.Equal(t, err, testCase.predicateReturns)
+				assert.Equal(t, testCase.predicateReturns, err)
 			}
 		})
 	}

orderer/common/cluster/util.go

@@ -652,9 +652,6 @@ func LastConfigBlock(block *common.Block, blockRetriever BlockRetriever) (*commo
 	if blockRetriever == nil {
 		return nil, errors.New("nil blockRetriever")
 	}
-	if block.Metadata == nil || len(block.Metadata.Metadata) <= int(common.BlockMetadataIndex_LAST_CONFIG) {
-		return nil, errors.New("no metadata in block")
-	}
 	lastConfigBlockNum, err := protoutil.GetLastConfigIndexFromBlock(block)
 	if err != nil {
 		return nil, err

orderer/common/cluster/util_test.go

@@ -891,31 +891,10 @@ func TestLastConfigBlock(t *testing.T) {
 		},
 		{
 			name:           "nil metadata",
-			expectedError:  "no metadata in block",
+			expectedError:  "failed to retrieve metadata: no metadata in block",
 			blockRetriever: blockRetriever,
 			block:          &common.Block{},
 		},
-		{
-			name:           "no last config block metadata",
-			expectedError:  "no metadata in block",
-			blockRetriever: blockRetriever,
-			block: &common.Block{
-				Metadata: &common.BlockMetadata{
-					Metadata: [][]byte{{}},
-				},
-			},
-		},
-		{
-			name:           "bad metadata in block",
-			blockRetriever: blockRetriever,
-			expectedError: "error unmarshaling metadata from block at index " +
-				"[LAST_CONFIG]: proto: common.Metadata: illegal tag 0 (wire type 1)",
-			block: &common.Block{
-				Metadata: &common.BlockMetadata{
-					Metadata: [][]byte{{}, {1, 2, 3}},
-				},
-			},
-		},
 		{
 			name: "no block with index",
 			block: &common.Block{

orderer/common/multichannel/blockwriter_test.go

@@ -11,6 +11,7 @@ import (
 	"os"
 	"testing"
 
+	"github.com/golang/protobuf/proto"
 	cb "github.com/hyperledger/fabric-protos-go/common"
 	"github.com/hyperledger/fabric-protos-go/orderer"
 	"github.com/hyperledger/fabric/bccsp"
@@ -366,3 +367,21 @@ func TestRaceWriteConfig(t *testing.T) {
 	require.NoError(t, err)
 	assert.Equal(t, consenterMetadata1, omd.Value)
 }
+
+func testLastConfigBlockNumber(t *testing.T, block *cb.Block, expectedBlockNumber uint64) {
+	metadata := &cb.Metadata{}
+	err := proto.Unmarshal(block.Metadata.Metadata[cb.BlockMetadataIndex_SIGNATURES], metadata)
+	assert.NoError(t, err, "Block should carry SIGNATURES metadata item")
+	obm := &cb.OrdererBlockMetadata{}
+	err = proto.Unmarshal(metadata.Value, obm)
+	assert.NoError(t, err, "Block SIGNATURES should carry OrdererBlockMetadata")
+	assert.Equal(t, expectedBlockNumber, obm.LastConfig.Index, "SIGNATURES value should point to last config block")
+
+	metadata = &cb.Metadata{}
+	err = proto.Unmarshal(block.Metadata.Metadata[cb.BlockMetadataIndex_LAST_CONFIG], metadata)
+	assert.NoError(t, err, "Block should carry LAST_CONFIG metadata item")
+	lastConfig := &cb.LastConfig{}
+	err = proto.Unmarshal(metadata.Value, lastConfig)
+	assert.NoError(t, err, "LAST_CONFIG metadata item should carry last config value")
+	assert.Equal(t, expectedBlockNumber, lastConfig.Index, "LAST_CONFIG value should point to last config block")
+}

orderer/common/multichannel/registrar_test.go

@@ -112,12 +112,12 @@ func testMessageOrderAndRetrieval(maxMessageCount uint32, chainID string, chainS
 }
 
 func TestConfigTx(t *testing.T) {
-	//system channel
+	// system channel
 	confSys := genesisconfig.Load(genesisconfig.SampleInsecureSoloProfile, configtest.GetDevConfigDir())
 	genesisBlockSys := encoder.New(confSys).GenesisBlock()
 
-	// Tests for a normal chain which contains 3 config transactions and other normal transactions to make
-	// sure the right one returned
+	// Tests for a normal channel which contains 3 config transactions and other
+	// normal transactions to make sure the right one returned
 	t.Run("GetConfigTx - ok", func(t *testing.T) {
 		tmpdir, err := ioutil.TempDir("", "registrar_test-")
 		require.NoError(t, err)
@@ -131,8 +131,12 @@ func TestConfigTx(t *testing.T) {
 		ctx := makeConfigTx("testchannelid", 6)
 		rl.Append(blockledger.CreateNextBlock(rl, []*cb.Envelope{ctx}))
 
+		// block with LAST_CONFIG metadata in SIGNATURES field
 		block := blockledger.CreateNextBlock(rl, []*cb.Envelope{makeNormalTx("testchannelid", 7)})
-		block.Metadata.Metadata[cb.BlockMetadataIndex_LAST_CONFIG] = protoutil.MarshalOrPanic(&cb.Metadata{Value: protoutil.MarshalOrPanic(&cb.LastConfig{Index: 7})})
+		blockSignatureValue := protoutil.MarshalOrPanic(&cb.OrdererBlockMetadata{
+			LastConfig: &cb.LastConfig{Index: 7},
+		})
+		block.Metadata.Metadata[cb.BlockMetadataIndex_SIGNATURES] = protoutil.MarshalOrPanic(&cb.Metadata{Value: blockSignatureValue})
 		rl.Append(block)
 
 		pctx := configTx(rl)
@@ -262,7 +266,6 @@ func TestCreateChain(t *testing.T) {
 
 	// This test brings up the entire system, with the mock consenter, including the broadcasters etc. and creates a new chain
 	t.Run("New chain", func(t *testing.T) {
-		expectedLastConfigBlockNumber := uint64(0)
 		expectedLastConfigSeq := uint64(1)
 		newChainID := "test-new-chain"
 
@@ -332,7 +335,6 @@ func TestCreateChain(t *testing.T) {
 		if status != cb.Status_SUCCESS {
 			t.Fatalf("Could not retrieve new chain genesis block")
 		}
-		testLastConfigBlockNumber(t, block, expectedLastConfigBlockNumber)
 		if len(block.Data.Data) != 1 {
 			t.Fatalf("Should have had only one message in the new genesis block")
 		}
@@ -343,7 +345,6 @@ func TestCreateChain(t *testing.T) {
 		if status != cb.Status_SUCCESS {
 			t.Fatalf("Could not retrieve block on new chain")
 		}
-		testLastConfigBlockNumber(t, block, expectedLastConfigBlockNumber)
 		for i := 0; i < int(confSys.Orderer.BatchSize.MaxMessageCount); i++ {
 			if !proto.Equal(protoutil.ExtractEnvelopeOrPanic(block, i), messages[i]) {
 				t.Errorf("Block contents wrong at index %d in new chain", i)
@@ -357,16 +358,6 @@ func TestCreateChain(t *testing.T) {
 	})
 }
 
-func testLastConfigBlockNumber(t *testing.T, block *cb.Block, expectedBlockNumber uint64) {
-	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, expectedBlockNumber, lastConfig.Index, "LAST_CONFIG value should point to last config block")
-}
-
 func TestResourcesCheck(t *testing.T) {
 	mockOrderer := &mocks.OrdererConfig{}
 	mockOrdererCaps := &mocks.OrdererCapabilities{}

orderer/common/server/main_test.go

@@ -295,6 +295,11 @@ func TestExtractSysChanLastConfig(t *testing.T) {
 	require.NoError(t, err)
 
 	nextBlock := blockledger.CreateNextBlock(rl, []*common.Envelope{configTx})
+	nextBlock.Metadata.Metadata[common.BlockMetadataIndex_SIGNATURES] = protoutil.MarshalOrPanic(&common.Metadata{
+		Value: protoutil.MarshalOrPanic(&common.OrdererBlockMetadata{
+			LastConfig: &common.LastConfig{Index: rl.Height()},
+		}),
+	})
 	nextBlock.Metadata.Metadata[common.BlockMetadataIndex_LAST_CONFIG] = protoutil.MarshalOrPanic(&common.Metadata{
 		Value: protoutil.MarshalOrPanic(&common.LastConfig{Index: rl.Height()}),
 	})