FAB-16353 Move proposal parsing out of eplugin

Presently, the endorser plugin contract does some static parsing and
error handling for the proposal which is duplicative in some ways of
what the endorser.go validation does.  Because this logic is not per
plugin, there is no reason it needs to be tied with the plugin provider.
So, this CR moves it back into endorser.go so that the common logic can
be factored out.

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

Author: Jason Yellick

core/endorser/endorser.go

@@ -67,7 +67,7 @@ type Support interface {
 	CheckACL(channelID string, signedProp *pb.SignedProposal) error
 
 	// EndorseWithPlugin endorses the response with a plugin
-	EndorseWithPlugin(ctx Context) (*pb.ProposalResponse, error)
+	EndorseWithPlugin(pluginName, channnelID string, prpBytes []byte, signedProposal *pb.SignedProposal) (*pb.Endorsement, []byte, error)
 
 	// GetLedgerHeight returns ledger height for given channelID
 	GetLedgerHeight(channelID string) (uint64, error)
@@ -230,26 +230,48 @@ func (e *Endorser) endorseProposal(up *UnpackedProposal, response *pb.Response,
 	endorserLogger.Debugf("[%s][%s] Entry chaincode: %s", up.ChannelHeader.ChannelId, shorttxid(up.ChannelHeader.TxId), up.ChaincodeName)
 	defer endorserLogger.Debugf("[%s][%s] Exit", up.ChannelHeader.ChannelId, shorttxid(up.ChannelHeader.TxId))
 
+	if response.Status >= shim.ERRORTHRESHOLD {
+		return &pb.ProposalResponse{Response: response}, nil
+	}
+
+	hdr, err := protoutil.UnmarshalHeader(up.Proposal.Header)
+	if err != nil {
+		endorserLogger.Warning("Failed parsing header", err)
+		return nil, errors.Wrap(err, "failed parsing header")
+	}
+
+	pHashBytes, err := protoutil.GetProposalHash1(hdr, up.Proposal.Payload)
+	if err != nil {
+		endorserLogger.Warning("Failed computing proposal hash", err)
+		return nil, errors.Wrap(err, "could not compute proposal hash")
+	}
+
+	prpBytes, err := protoutil.GetBytesProposalResponsePayload(pHashBytes, response, simRes, eventBytes, &pb.ChaincodeID{
+		Name:    up.ChaincodeName,
+		Version: cd.CCVersion(),
+	})
+	if err != nil {
+		endorserLogger.Warning("Failed marshaling the proposal response payload to bytes", err)
+		return nil, errors.New("failure while marshaling the ProposalResponsePayload")
+	}
+
 	escc := cd.Endorsement()
 
 	endorserLogger.Debugf("[%s][%s] escc for chaincode %s is %s", up.ChannelHeader.ChannelId, shorttxid(up.ChannelHeader.TxId), up.ChaincodeName, escc)
 
-	ctx := Context{
-		PluginName:     escc,
-		Channel:        up.ChannelHeader.ChannelId,
-		SignedProposal: up.SignedProposal,
-		ChaincodeID: &pb.ChaincodeID{
-			Name:    up.ChaincodeName,
-			Version: cd.CCVersion(),
-		},
-		Event:    eventBytes,
-		SimRes:   simRes,
-		Response: response,
-		// Visibility is checked to be nil at the beginning of the flow, so it is safe not to set
-		Proposal: up.Proposal,
-		TxID:     up.ChannelHeader.TxId,
-	}
-	return e.s.EndorseWithPlugin(ctx)
+	endorsement, prpBytes, err := e.s.EndorseWithPlugin(escc, up.ChannelHeader.ChannelId, prpBytes, up.SignedProposal)
+	if err != nil {
+		return nil, errors.WithMessage(err, "endorsing with plugin failed")
+	}
+
+	resp := &pb.ProposalResponse{
+		Version:     1,
+		Endorsement: endorsement,
+		Payload:     prpBytes,
+		Response:    response,
+	}
+
+	return resp, nil
 }
 
 // preProcess checks the tx proposal headers, uniqueness and ACL

core/endorser/plugin_endorser.go

@@ -10,12 +10,10 @@ import (
 	"fmt"
 	"sync"
 
-	"github.com/hyperledger/fabric/core/chaincode/shim"
 	endorsement "github.com/hyperledger/fabric/core/handlers/endorsement/api"
 	endorsement3 "github.com/hyperledger/fabric/core/handlers/endorsement/api/identities"
 	"github.com/hyperledger/fabric/core/transientstore"
 	pb "github.com/hyperledger/fabric/protos/peer"
-	"github.com/hyperledger/fabric/protoutil"
 	"github.com/pkg/errors"
 )
 
@@ -165,43 +163,13 @@ type PluginEndorser struct {
 }
 
 // EndorseWithPlugin endorses the response with a plugin
-func (pe *PluginEndorser) EndorseWithPlugin(ctx Context) (*pb.ProposalResponse, error) {
-	endorserLogger.Debug("Entering endorsement for", ctx)
-
-	if ctx.Response == nil {
-		return nil, errors.New("response is nil")
-	}
-
-	if ctx.Response.Status >= shim.ERRORTHRESHOLD {
-		return &pb.ProposalResponse{Response: ctx.Response}, nil
-	}
-
-	plugin, err := pe.getOrCreatePlugin(PluginName(ctx.PluginName), ctx.Channel)
-	if err != nil {
-		endorserLogger.Warning("Endorsement with plugin for", ctx, " failed:", err)
-		return nil, errors.Errorf("plugin with name %s could not be used: %v", ctx.PluginName, err)
-	}
-
-	prpBytes, err := proposalResponsePayloadFromContext(ctx)
-	if err != nil {
-		endorserLogger.Warning("Endorsement with plugin for", ctx, " failed:", err)
-		return nil, errors.Wrap(err, "failed assembling proposal response payload")
-	}
-
-	endorsement, prpBytes, err := plugin.Endorse(prpBytes, ctx.SignedProposal)
+func (pe *PluginEndorser) EndorseWithPlugin(pluginName, channelID string, prpBytes []byte, signedProposal *pb.SignedProposal) (*pb.Endorsement, []byte, error) {
+	plugin, err := pe.getOrCreatePlugin(PluginName(pluginName), channelID)
 	if err != nil {
-		endorserLogger.Warning("Endorsement with plugin for", ctx, " failed:", err)
-		return nil, errors.WithStack(err)
+		return nil, nil, errors.WithMessagef(err, "plugin with name %s could not be used", pluginName)
 	}
 
-	resp := &pb.ProposalResponse{
-		Version:     1,
-		Endorsement: endorsement,
-		Payload:     prpBytes,
-		Response:    ctx.Response,
-	}
-	endorserLogger.Debug("Exiting", ctx)
-	return resp, nil
+	return plugin.Endorse(prpBytes, signedProposal)
 }
 
 // getOrCreatePlugin returns a plugin instance for the given plugin name and channel
@@ -229,24 +197,3 @@ func (pe *PluginEndorser) getOrCreatePluginChannelMapping(plugin PluginName, pf
 	}
 	return endorserChannelMapping
 }
-
-func proposalResponsePayloadFromContext(ctx Context) ([]byte, error) {
-	hdr, err := protoutil.UnmarshalHeader(ctx.Proposal.Header)
-	if err != nil {
-		endorserLogger.Warning("Failed parsing header", err)
-		return nil, errors.Wrap(err, "failed parsing header")
-	}
-
-	pHashBytes, err := protoutil.GetProposalHash1(hdr, ctx.Proposal.Payload)
-	if err != nil {
-		endorserLogger.Warning("Failed computing proposal hash", err)
-		return nil, errors.Wrap(err, "could not compute proposal hash")
-	}
-
-	prpBytes, err := protoutil.GetBytesProposalResponsePayload(pHashBytes, ctx.Response, ctx.SimRes, ctx.Event, ctx.ChaincodeID)
-	if err != nil {
-		endorserLogger.Warning("Failed marshaling the proposal response payload to bytes", err)
-		return nil, errors.New("failure while marshaling the ProposalResponsePayload")
-	}
-	return prpBytes, nil
-}

core/endorser/plugin_endorser_test.go

@@ -11,17 +11,14 @@ import (
 
 	"github.com/golang/protobuf/proto"
 	"github.com/hyperledger/fabric/common/mocks/ledger"
-	"github.com/hyperledger/fabric/core/chaincode/shim"
 	"github.com/hyperledger/fabric/core/endorser"
 	"github.com/hyperledger/fabric/core/endorser/mocks"
 	endorsement "github.com/hyperledger/fabric/core/handlers/endorsement/api"
 	. "github.com/hyperledger/fabric/core/handlers/endorsement/api/state"
 	"github.com/hyperledger/fabric/core/transientstore"
-	"github.com/hyperledger/fabric/protos/common"
 	"github.com/hyperledger/fabric/protos/ledger/rwset"
 	"github.com/hyperledger/fabric/protos/peer"
 	transientstore2 "github.com/hyperledger/fabric/protos/transientstore"
-	"github.com/hyperledger/fabric/protoutil"
 	"github.com/pkg/errors"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
@@ -38,21 +35,13 @@ func TestPluginEndorserNotFound(t *testing.T) {
 	pluginEndorser := endorser.NewPluginEndorser(&endorser.PluginSupport{
 		PluginMapper: pluginMapper,
 	})
-	resp, err := pluginEndorser.EndorseWithPlugin(endorser.Context{
-		Response:   &peer.Response{},
-		PluginName: "notfound",
-	})
-	assert.Nil(t, resp)
+	endorsement, prpBytes, err := pluginEndorser.EndorseWithPlugin("notfound", "", nil, nil)
+	assert.Nil(t, endorsement)
+	assert.Nil(t, prpBytes)
 	assert.Contains(t, err.Error(), "plugin with name notfound wasn't found")
 }
 
 func TestPluginEndorserGreenPath(t *testing.T) {
-	proposal, _, err := protoutil.CreateChaincodeProposal(common.HeaderType_ENDORSER_TRANSACTION, "mychannel", &peer.ChaincodeInvocationSpec{
-		ChaincodeSpec: &peer.ChaincodeSpec{
-			ChaincodeId: &peer.ChaincodeID{Name: "mycc"},
-		},
-	}, []byte{1, 2, 3})
-	assert.NoError(t, err)
 	expectedSignature := []byte{5, 4, 3, 2, 1}
 	expectedProposalResponsePayload := []byte{1, 2, 3}
 	pluginMapper := &mocks.PluginMapper{}
@@ -73,48 +62,35 @@ func TestPluginEndorserGreenPath(t *testing.T) {
 		PluginMapper:            pluginMapper,
 		TransientStoreRetriever: mockTransientStoreRetriever,
 	})
-	ctx := endorser.Context{
-		Response:   &peer.Response{},
-		PluginName: "plugin",
-		Proposal:   proposal,
-		ChaincodeID: &peer.ChaincodeID{
-			Name: "mycc",
-		},
-		Channel: "mychannel",
-	}
 
 	// Scenario I: Call the endorsement for the first time
-	resp, err := pluginEndorser.EndorseWithPlugin(ctx)
+	endorsement, prpBytes, err := pluginEndorser.EndorseWithPlugin("plugin", "mychannel", nil, nil)
 	assert.NoError(t, err)
-	assert.NotNil(t, resp)
-	assert.Equal(t, expectedSignature, resp.Endorsement.Signature)
-	assert.Equal(t, expectedProposalResponsePayload, resp.Payload)
+	assert.Equal(t, expectedSignature, endorsement.Signature)
+	assert.Equal(t, expectedProposalResponsePayload, prpBytes)
 	// Ensure both state and SigningIdentityFetcher were passed to Init()
 	plugin.AssertCalled(t, "Init", &endorser.ChannelState{QueryCreator: queryCreator, Store: mockTransientStore}, sif)
 
 	// Scenario II: Call the endorsement again a second time.
 	// Ensure the plugin wasn't instantiated again - which means the same instance
 	// was used to service the request.
 	// Also - check that the Init() wasn't called more than once on the plugin.
-	resp, err = pluginEndorser.EndorseWithPlugin(ctx)
+	endorsement, prpBytes, err = pluginEndorser.EndorseWithPlugin("plugin", "mychannel", nil, nil)
 	assert.NoError(t, err)
-	assert.NotNil(t, resp)
-	assert.Equal(t, expectedSignature, resp.Endorsement.Signature)
-	assert.Equal(t, expectedProposalResponsePayload, resp.Payload)
+	assert.Equal(t, expectedSignature, endorsement.Signature)
+	assert.Equal(t, expectedProposalResponsePayload, prpBytes)
 	pluginFactory.AssertNumberOfCalls(t, "New", 1)
 	plugin.AssertNumberOfCalls(t, "Init", 1)
 
 	// Scenario III: Call the endorsement with a channel-less context.
 	// The init method should be called again, but this time - a channel state object
 	// should not be passed into the init.
-	ctx.Channel = ""
 	pluginFactory.On("New").Return(plugin).Once()
 	plugin.On("Init", mock.Anything).Return(nil).Once()
-	resp, err = pluginEndorser.EndorseWithPlugin(ctx)
+	endorsement, prpBytes, err = pluginEndorser.EndorseWithPlugin("plugin", "", nil, nil)
 	assert.NoError(t, err)
-	assert.NotNil(t, resp)
-	assert.Equal(t, expectedSignature, resp.Endorsement.Signature)
-	assert.Equal(t, expectedProposalResponsePayload, resp.Payload)
+	assert.Equal(t, expectedSignature, endorsement.Signature)
+	assert.Equal(t, expectedProposalResponsePayload, prpBytes)
 	plugin.AssertCalled(t, "Init", sif)
 }
 
@@ -136,73 +112,15 @@ func TestPluginEndorserErrors(t *testing.T) {
 		TransientStoreRetriever: mockTransientStoreRetriever,
 	})
 
-	// Scenario I: Failed initializing plugin
+	// Failed initializing plugin
 	t.Run("PluginInitializationFailure", func(t *testing.T) {
 		plugin.On("Init", mock.Anything, mock.Anything).Return(errors.New("plugin initialization failed")).Once()
-		resp, err := pluginEndorser.EndorseWithPlugin(endorser.Context{
-			PluginName: "plugin",
-			Channel:    "mychannel",
-			Response:   &peer.Response{},
-		})
-		assert.Nil(t, resp)
+		endorsement, prpBytes, err := pluginEndorser.EndorseWithPlugin("plugin", "mychannel", nil, nil)
+		assert.Nil(t, endorsement)
+		assert.Nil(t, prpBytes)
 		assert.Contains(t, err.Error(), "plugin initialization failed")
 	})
 
-	// Scenario II: an empty proposal is passed in the context, and parsing fails
-	t.Run("EmptyProposal", func(t *testing.T) {
-		plugin.On("Init", mock.Anything, mock.Anything).Return(nil).Once()
-		ctx := endorser.Context{
-			Response:   &peer.Response{},
-			PluginName: "plugin",
-			ChaincodeID: &peer.ChaincodeID{
-				Name: "mycc",
-			},
-			Proposal: &peer.Proposal{},
-			Channel:  "mychannel",
-		}
-		resp, err := pluginEndorser.EndorseWithPlugin(ctx)
-		assert.Nil(t, resp)
-		assert.Contains(t, err.Error(), "could not compute proposal hash")
-	})
-
-	// Scenario III: The proposal's header is invalid
-	t.Run("InvalidHeader in the proposal", func(t *testing.T) {
-		ctx := endorser.Context{
-			Response:   &peer.Response{},
-			PluginName: "plugin",
-			ChaincodeID: &peer.ChaincodeID{
-				Name: "mycc",
-			},
-			Proposal: &peer.Proposal{
-				Header: []byte{1, 2, 3},
-			},
-			Channel: "mychannel",
-		}
-		resp, err := pluginEndorser.EndorseWithPlugin(ctx)
-		assert.Nil(t, resp)
-		assert.Contains(t, err.Error(), "failed parsing header")
-	})
-
-	// Scenario IV: The proposal's response status code indicates an error
-	t.Run("ResponseStatusContainsError", func(t *testing.T) {
-		r := &peer.Response{
-			Status:  shim.ERRORTHRESHOLD,
-			Payload: []byte{1, 2, 3},
-			Message: "bla bla",
-		}
-		resp, err := pluginEndorser.EndorseWithPlugin(endorser.Context{
-			Response: r,
-		})
-		assert.Equal(t, &peer.ProposalResponse{Response: r}, resp)
-		assert.NoError(t, err)
-	})
-
-	// Scenario V: The proposal's response is nil
-	t.Run("ResponseIsNil", func(t *testing.T) {
-		resp, err := pluginEndorser.EndorseWithPlugin(endorser.Context{})
-		assert.Nil(t, resp)
-		assert.Contains(t, err.Error(), "response is nil")
-	})
 }
 
 func transientStoreRetriever() *mocks.TransientStoreRetriever {
@@ -281,22 +199,6 @@ func TestTransientStore(t *testing.T) {
 		TransientStoreRetriever: storeRetriever,
 	})
 
-	proposal, _, err := protoutil.CreateChaincodeProposal(common.HeaderType_ENDORSER_TRANSACTION, "mychannel", &peer.ChaincodeInvocationSpec{
-		ChaincodeSpec: &peer.ChaincodeSpec{
-			ChaincodeId: &peer.ChaincodeID{Name: "mycc"},
-		},
-	}, []byte{1, 2, 3})
-	assert.NoError(t, err)
-	ctx := endorser.Context{
-		Response:   &peer.Response{},
-		PluginName: "plugin",
-		Proposal:   proposal,
-		ChaincodeID: &peer.ChaincodeID{
-			Name: "mycc",
-		},
-		Channel: "mychannel",
-	}
-
 	rws := &rwset.TxPvtReadWriteSet{
 		NsPvtRwset: []*rwset.NsPvtReadWriteSet{
 			{
@@ -316,11 +218,11 @@ func TestTransientStore(t *testing.T) {
 
 	transientStore.On("GetTxPvtRWSetByTxid", mock.Anything, mock.Anything).Return(scanner, nil)
 
-	resp, err := pluginEndorser.EndorseWithPlugin(ctx)
+	_, prpBytes, err := pluginEndorser.EndorseWithPlugin("plugin", "mychannel", nil, nil)
 	assert.NoError(t, err)
 
 	txrws := &rwset.TxPvtReadWriteSet{}
-	err = proto.Unmarshal(resp.Payload, txrws)
+	err = proto.Unmarshal(prpBytes, txrws)
 	assert.NoError(t, err)
 	assert.True(t, proto.Equal(rws, txrws))
 	scanner.AssertCalled(t, "Close")