FAB-16158 Store chaincodeID as string in handler

The handler presently never references any field of its chaincodeID
member other than .Name, which is in fact, the chaincode id.  So, it
makes no sense to be storing this as a confusing nested structure,
removing.

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

Author: Jason Yellick

core/chaincode/chaincode_support_test.go

@@ -1128,7 +1128,7 @@ func TestCCFramework(t *testing.T) {
 	initializeCC(t, chainID, ccname, ccSide, chaincodeSupport)
 
 	//chaincode support should not allow dups
-	handler := &Handler{chaincodeID: &pb.ChaincodeID{Name: ccname + ":0"}, BuiltinSCCs: chaincodeSupport.BuiltinSCCs}
+	handler := &Handler{chaincodeID: ccname + ":0", BuiltinSCCs: chaincodeSupport.BuiltinSCCs}
 	if err := chaincodeSupport.HandlerRegistry.Register(handler); err == nil {
 		t.Fatalf("expected re-register to fail")
 	}

core/chaincode/handler.go

@@ -127,7 +127,7 @@ type Handler struct {
 	// ready.
 	state State
 	// chaincodeID holds the ID of the chaincode that registered with the peer.
-	chaincodeID *pb.ChaincodeID
+	chaincodeID string
 
 	// serialLock is used to serialize sends across the grpc chat stream.
 	serialLock sync.Mutex
@@ -229,11 +229,10 @@ func (h *Handler) HandleTransaction(msg *pb.ChaincodeMessage, delegate handleFun
 		txContext, err = h.isValidTxSim(msg.ChannelId, msg.Txid, "no ledger context")
 	}
 
-	chaincodeName := h.chaincodeID.Name + ":" + h.chaincodeID.Version
 	meterLabels := []string{
 		"type", msg.Type.String(),
 		"channel", msg.ChannelId,
-		"chaincode", chaincodeName,
+		"chaincode", h.chaincodeID,
 	}
 	h.Metrics.ShimRequestsReceived.With(meterLabels...).Add(1)
 
@@ -341,11 +340,8 @@ func (h *Handler) checkACL(signedProp *pb.SignedProposal, proposal *pb.Proposal,
 }
 
 func (h *Handler) deregister() {
-	if h.chaincodeID != nil {
-		// FIXME: this works once again because h.chaincodeID.Name
-		// is not the chaincode name but the concatenation of name
-		// and version (FAB-14630)
-		h.Registry.Deregister(h.chaincodeID.Name)
+	if h.chaincodeID != "" {
+		h.Registry.Deregister(h.chaincodeID)
 	}
 }
 
@@ -417,18 +413,18 @@ func (h *Handler) ProcessStream(stream ccintf.ChaincodeStream) error {
 
 // sendReady sends READY to chaincode serially (just like REGISTER)
 func (h *Handler) sendReady() error {
-	chaincodeLogger.Debugf("sending READY for chaincode %+v", h.chaincodeID)
+	chaincodeLogger.Debugf("sending READY for chaincode %s", h.chaincodeID)
 	ccMsg := &pb.ChaincodeMessage{Type: pb.ChaincodeMessage_READY}
 
 	// if error in sending tear down the h
 	if err := h.serialSend(ccMsg); err != nil {
-		chaincodeLogger.Errorf("error sending READY (%s) for chaincode %+v", err, h.chaincodeID)
+		chaincodeLogger.Errorf("error sending READY (%s) for chaincode %s", err, h.chaincodeID)
 		return err
 	}
 
 	h.state = Ready
 
-	chaincodeLogger.Debugf("Changed to state ready for chaincode %+v", h.chaincodeID)
+	chaincodeLogger.Debugf("Changed to state ready for chaincode %s", h.chaincodeID)
 
 	return nil
 }
@@ -441,12 +437,12 @@ func (h *Handler) notifyRegistry(err error) {
 	}
 
 	if err != nil {
-		h.Registry.Failed(h.chaincodeID.Name, err)
+		h.Registry.Failed(h.chaincodeID, err)
 		chaincodeLogger.Errorf("failed to start %s -- %s", h.chaincodeID, err)
 		return
 	}
 
-	h.Registry.Ready(h.chaincodeID.Name)
+	h.Registry.Ready(h.chaincodeID)
 }
 
 // handleRegister is invoked when chaincode tries to register.
@@ -460,14 +456,17 @@ func (h *Handler) HandleRegister(msg *pb.ChaincodeMessage) {
 	}
 
 	// Now register with the chaincodeSupport
-	h.chaincodeID = chaincodeID
+	// Note: chaincodeID.Name is actually of the form name:version for older chaincodes, and
+	// of the form label:hash for newer chaincodes.  Either way, it is the handle by which
+	// we track the chaincode's registration.
+	h.chaincodeID = chaincodeID.Name
 	err = h.Registry.Register(h)
 	if err != nil {
 		h.notifyRegistry(err)
 		return
 	}
 
-	chaincodeLogger.Debugf("Got %s for chaincodeID = %s, sending back %s", pb.ChaincodeMessage_REGISTER, chaincodeID, pb.ChaincodeMessage_REGISTERED)
+	chaincodeLogger.Debugf("Got %s for chaincodeID = %s, sending back %s", pb.ChaincodeMessage_REGISTER, h.chaincodeID, pb.ChaincodeMessage_REGISTERED)
 	if err := h.serialSend(&pb.ChaincodeMessage{Type: pb.ChaincodeMessage_REGISTERED}); err != nil {
 		chaincodeLogger.Errorf("error sending %s: %s", pb.ChaincodeMessage_REGISTERED, err)
 		h.notifyRegistry(err)
@@ -476,7 +475,7 @@ func (h *Handler) HandleRegister(msg *pb.ChaincodeMessage) {
 
 	h.state = Established
 
-	chaincodeLogger.Debugf("Changed state to established for %+v", h.chaincodeID)
+	chaincodeLogger.Debugf("Changed state to established for %s", h.chaincodeID)
 
 	// for dev mode this will also move to ready automatically
 	h.notifyRegistry(nil)
@@ -513,11 +512,11 @@ func (h *Handler) registerTxid(msg *pb.ChaincodeMessage) bool {
 	}
 
 	// Log the issue and drop the request
-	chaincodeName := "unknown"
-	if h.chaincodeID != nil {
-		chaincodeName = h.chaincodeID.Name
+	chaincodeID := "unknown"
+	if h.chaincodeID != "" {
+		chaincodeID = h.chaincodeID
 	}
-	chaincodeLogger.Errorf("[%s] Another request pending for this CC: %s, Txid: %s, ChannelID: %s. Cannot process.", shorttxid(msg.Txid), chaincodeName, msg.Txid, msg.ChannelId)
+	chaincodeLogger.Errorf("[%s] Another request pending for this CC: %s, Txid: %s, ChannelID: %s. Cannot process.", shorttxid(msg.Txid), chaincodeID, msg.Txid, msg.ChannelId)
 	return false
 }
 

core/chaincode/handler_internal_test.go

@@ -8,12 +8,11 @@ package chaincode
 
 import (
 	"github.com/hyperledger/fabric/core/container/ccintf"
-	pb "github.com/hyperledger/fabric/protos/peer"
 )
 
 // Helpers to access unexported state.
 
-func SetHandlerChaincodeID(h *Handler, chaincodeID *pb.ChaincodeID) {
+func SetHandlerChaincodeID(h *Handler, chaincodeID string) {
 	h.chaincodeID = chaincodeID
 }
 

core/chaincode/handler_registry.go

@@ -130,28 +130,20 @@ func (r *HandlerRegistry) Register(h *Handler) error {
 	r.mutex.Lock()
 	defer r.mutex.Unlock()
 
-	// FIXME: the ccid must be its own field in the handler
-	// This hack works because currently h.chaincodeID.Name is
-	// actually set as the concatenation of chaincode name and
-	// chaincode ID, which is set at build time. While it's ok
-	// for the chaincode to communicate back its packageID, the
-	// usage of the chaincodeID field is misleading (FAB-14630)
-	ccid := h.chaincodeID.Name
-
-	if r.handlers[ccid] != nil {
-		chaincodeLogger.Debugf("duplicate registered handler(key:%s) return error", ccid)
-		return errors.Errorf("duplicate chaincodeID: %s", h.chaincodeID.Name)
+	if r.handlers[h.chaincodeID] != nil {
+		chaincodeLogger.Debugf("duplicate registered handler(key:%s) return error", h.chaincodeID)
+		return errors.Errorf("duplicate chaincodeID: %s", h.chaincodeID)
 	}
 
 	// This chaincode was not launched by the peer but is attempting
 	// to register. Only allowed in development mode.
-	if r.launching[ccid] == nil && !r.allowUnsolicitedRegistration {
-		return errors.Errorf("peer will not accept external chaincode connection %v (except in dev mode)", h.chaincodeID.Name)
+	if r.launching[h.chaincodeID] == nil && !r.allowUnsolicitedRegistration {
+		return errors.Errorf("peer will not accept external chaincode connection %s (except in dev mode)", h.chaincodeID)
 	}
 
-	r.handlers[ccid] = h
+	r.handlers[h.chaincodeID] = h
 
-	chaincodeLogger.Debugf("registered handler complete for chaincode %s", ccid)
+	chaincodeLogger.Debugf("registered handler complete for chaincode %s", h.chaincodeID)
 	return nil
 }
 

core/chaincode/handler_registry_test.go

@@ -10,7 +10,6 @@ import (
 	"github.com/hyperledger/fabric/core/chaincode"
 	"github.com/hyperledger/fabric/core/chaincode/mock"
 	"github.com/hyperledger/fabric/core/common/ccprovider"
-	pb "github.com/hyperledger/fabric/protos/peer"
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 	"github.com/pkg/errors"
@@ -23,7 +22,7 @@ var _ = Describe("HandlerRegistry", func() {
 	BeforeEach(func() {
 		hr = chaincode.NewHandlerRegistry(true)
 		handler = &chaincode.Handler{}
-		chaincode.SetHandlerChaincodeID(handler, &pb.ChaincodeID{Name: "chaincode-name"})
+		chaincode.SetHandlerChaincodeID(handler, "chaincode-name")
 	})
 
 	Describe("Launching", func() {
@@ -310,7 +309,7 @@ var _ = Describe("TxSimulatorGetter", func() {
 
 	When("No TxContext is created", func() {
 		BeforeEach(func() {
-			chaincode.SetHandlerChaincodeID(handler, &pb.ChaincodeID{Name: "package-ID"})
+			chaincode.SetHandlerChaincodeID(handler, "package-ID")
 			hr.Register(handler)
 		})
 
@@ -322,7 +321,7 @@ var _ = Describe("TxSimulatorGetter", func() {
 
 	When("Handler is created for package-ID and TxContext is created for channel-ID/tx-ID", func() {
 		BeforeEach(func() {
-			chaincode.SetHandlerChaincodeID(handler, &pb.ChaincodeID{Name: "package-ID"})
+			chaincode.SetHandlerChaincodeID(handler, "package-ID")
 			hr.Register(handler)
 			handler.TXContexts.Create(&ccprovider.TransactionParams{
 				ChannelID:   "channel-ID",

core/chaincode/handler_test.go

@@ -126,7 +126,7 @@ var _ = Describe("Handler", func() {
 			Metrics:   chaincodeMetrics,
 		}
 		chaincode.SetHandlerChatStream(handler, fakeChatStream)
-		chaincode.SetHandlerChaincodeID(handler, &pb.ChaincodeID{Name: "test-handler-name", Version: "1.0"})
+		chaincode.SetHandlerChaincodeID(handler, "test-handler-name:1.0")
 	})
 
 	Describe("HandleTransaction", func() {