BFT: orderer deliver headers on cache (hyperledger#4455)

The orderer delivers blocks with block.Data=nil when asked for SeekInfo_HEADER_WITH_SIG. However, one needs to avoid mutating the block received from the block iterator, as those are from a cache. Change-Id: I0fa4ba510a68ce54c37b8efe9d4c405ec55177e2 Signed-off-by: Yoav Tock <[email protected]>
MayRosenbaum · Oct 2, 2023 · dcf07a8 · dcf07a8
1 parent e74253a
commit dcf07a8
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 3 deletions.
diff --git a/common/deliver/deliver.go b/common/deliver/deliver.go
@@ -328,12 +328,18 @@ func (h *Handler) deliverBlocks(ctx context.Context, srv *Server, envelope *cb.E
 
 		logger.Debugf("[channel: %s] Delivering block [%d] for (%p) for %s", chdr.ChannelId, block.Header.Number, seekInfo, addr)
 
+		// Data blocks carry nil data for block attestations.
+		// Never mutate the block received from the iterator as it is from a cache.
+		block2send := block
 		if seekInfo.ContentType == ab.SeekInfo_HEADER_WITH_SIG && !protoutil.IsConfigBlock(block) {
-			block.Data = nil
+			block2send = &cb.Block{
+				Header:   block.Header,
+				Metadata: block.Metadata,
+			}
 		}
 
 		signedData := &protoutil.SignedData{Data: envelope.Payload, Identity: shdr.Creator, Signature: envelope.Signature}
-		if err := srv.SendBlockResponse(block, chdr.ChannelId, chain, signedData); err != nil {
+		if err := srv.SendBlockResponse(block2send, chdr.ChannelId, chain, signedData); err != nil {
 			logger.Warningf("[channel: %s] Error sending to %s: %s", chdr.ChannelId, addr, err)
 			return cb.Status_INTERNAL_SERVER_ERROR, err
 		}

diff --git a/common/deliver/deliver_test.go b/common/deliver/deliver_test.go
@@ -502,6 +502,7 @@ var _ = Describe("Deliver", func() {
 		})
 
 		Context("when seek info is configured to header with sig content type", func() {
+			var cachedBlocks []*cb.Block
 			BeforeEach(func() {
 				seekInfo = &ab.SeekInfo{
 					Start:       &ab.SeekPosition{},
@@ -516,11 +517,12 @@ var _ = Describe("Deliver", func() {
 						Data:     &cb.BlockData{Data: [][]byte{{1}, {2}}},
 						Metadata: &cb.BlockMetadata{Metadata: [][]byte{{3}, {4}}},
 					}
+					cachedBlocks = append(cachedBlocks, blk)
 					return blk, cb.Status_SUCCESS
 				}
 			})
 
-			It("sends blocks with nil Data", func() {
+			It("sends blocks with nil Data, but does not mutate cached blocks", func() {
 				err := handler.Handle(context.Background(), server)
 				Expect(err).NotTo(HaveOccurred())
 
@@ -538,6 +540,10 @@ var _ = Describe("Deliver", func() {
 						Metadata: &cb.BlockMetadata{Metadata: [][]byte{{3}, {4}}},
 					}))
 				}
+
+				for _, b := range cachedBlocks {
+					Expect(b.Data).ToNot(BeNil())
+				}
 			})
 		})