Don't use EndorseResponse.Result field

This duplicates information already held within the transaction envelope, and can cause failures on gRPC message size limit with large transaction results. Also: - nil out the ProposalResponse.Response.Payload field in EndorseResponse messages since, in the successful endorsement case, this is typically populated with a duplicate of the transaction result. This field is not required by the Fabric Gateway client and can cause gRPC message size limit failures. - prefer the transaction result within the proposal response payload for EvaluateResponse messages since the Response.Payload in the proposal response is not required to be the transaction result and may instead contain metadata. Signed-off-by: Mark S. Lewis <[email protected]>
hyperledger · Nov 18, 2021 · f557fbe · f557fbe
1 parent c463b32
commit f557fbe
Show file tree

Hide file tree

Showing 6 changed files with 61 additions and 14 deletions.
diff --git a/integration/gateway/endorsing_orgs_test.go b/integration/gateway/endorsing_orgs_test.go
@@ -18,6 +18,7 @@ import (
 	"github.com/hyperledger/fabric-protos-go/gateway"
 	"github.com/hyperledger/fabric-protos-go/peer"
 	"github.com/hyperledger/fabric/integration/nwo"
+	"github.com/hyperledger/fabric/protoutil"
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 	"github.com/tedsuo/ifrit"
@@ -139,7 +140,11 @@ func submitCheckEndorsingOrgsTransaction(ctx context.Context, client gateway.Gat
 	endorseResponse, err := client.Endorse(ctx, endorseRequest)
 	Expect(err).NotTo(HaveOccurred())
 
-	result := endorseResponse.GetResult()
+	chaincodeAction, err := protoutil.GetActionFromEnvelopeMsg(endorseResponse.GetPreparedTransaction())
+	Expect(err).NotTo(HaveOccurred())
+
+	result := chaincodeAction.GetResponse()
+
 	expectedPayload := "Peer mspid OK"
 	Expect(string(result.Payload)).To(Equal(expectedPayload))
 	expectedResult := &peer.Response{

diff --git a/integration/gateway/gateway_discovery_test.go b/integration/gateway/gateway_discovery_test.go
@@ -236,7 +236,10 @@ var _ = Describe("GatewayService with endorser discovery", func() {
 		Expect(err).NotTo(HaveOccurred())
 		Expect(statusResponse.Result).To(Equal(peer.TxValidationCode_VALID))
 
-		return endorseResponse.Result
+		chaincodeAction, err := protoutil.GetActionFromEnvelopeMsg(endorseResponse.GetPreparedTransaction())
+		Expect(err).NotTo(HaveOccurred())
+
+		return chaincodeAction.GetResponse()
 	}
 
 	evaluateTransaction := func(
@@ -587,6 +590,6 @@ var _ = Describe("GatewayService with endorser discovery", func() {
 			[]string{org1Peer0.ID()},
 		)
 
-		Expect(result.Payload).To(Equal([]byte("abcd")))
+		Expect(result.GetPayload()).To(Equal([]byte("abcd")))
 	})
 })
diff --git a/integration/gateway/gateway_test.go b/integration/gateway/gateway_test.go
@@ -179,7 +179,10 @@ var _ = Describe("GatewayService", func() {
 		_, err = gatewayClient.Submit(ctx, submitRequest)
 		Expect(err).NotTo(HaveOccurred())
 
-		return endorseResponse.Result, transactionID
+		chaincodeAction, err := protoutil.GetActionFromEnvelopeMsg(endorseResponse.GetPreparedTransaction())
+		Expect(err).NotTo(HaveOccurred())
+
+		return chaincodeAction.GetResponse(), transactionID
 	}
 
 	commitStatus := func(transactionID string, identity func() ([]byte, error), sign func(msg []byte) ([]byte, error)) (*gateway.CommitStatusResponse, error) {

diff --git a/internal/pkg/gateway/api.go b/internal/pkg/gateway/api.go
@@ -82,13 +82,22 @@ func (gs *Server) Evaluate(ctx context.Context, request *gp.EvaluateRequest) (*g
 		}
 
 		response = pr.GetResponse()
-		if response != nil && (response.Status < 200 || response.Status >= 400) {
-			logger.Debugw("Evaluate call to endorser returned a malformed or error response", "chaincode", chaincodeID, "channel", channel, "txid", request.TransactionId, "endorserAddress", endorser.endpointConfig.address, "endorserMspid", endorser.endpointConfig.mspid, "status", response.Status, "message", response.Message)
-			err = fmt.Errorf("error %d returned from chaincode %s on channel %s: %s", response.Status, chaincodeID, channel, response.Message)
-			endpointErr := errorDetail(endorser.endpointConfig, err)
-			errDetails = append(errDetails, endpointErr)
-			// this is a chaincode error response - don't retry
-			return nil, newRpcError(codes.Aborted, "evaluate call to endorser returned error: "+response.Message, errDetails...)
+		if response != nil {
+			if response.Status < 200 || response.Status >= 400 {
+				logger.Debugw("Evaluate call to endorser returned a malformed or error response", "chaincode", chaincodeID, "channel", channel, "txid", request.TransactionId, "endorserAddress", endorser.endpointConfig.address, "endorserMspid", endorser.endpointConfig.mspid, "status", response.Status, "message", response.Message)
+				err = fmt.Errorf("error %d returned from chaincode %s on channel %s: %s", response.Status, chaincodeID, channel, response.Message)
+				endpointErr := errorDetail(endorser.endpointConfig, err)
+				errDetails = append(errDetails, endpointErr)
+				// this is a chaincode error response - don't retry
+				return nil, newRpcError(codes.Aborted, "evaluate call to endorser returned error: "+response.Message, errDetails...)
+			}
+
+			// Prefer result from proposal response as Response.Payload is not required to be transaction result
+			if result, err := getResultFromProposalResponse(pr); err == nil {
+				response.Payload = result
+			} else {
+				logger.Warnw("Successful proposal response contained no transaction result", "error", err.Error(), "chaincode", chaincodeID, "channel", channel, "txid", request.TransactionId, "endorserAddress", endorser.endpointConfig.address, "endorserMspid", endorser.endpointConfig.mspid, "status", response.Status, "message", response.Message)
+			}
 		}
 	}
 
@@ -236,6 +245,12 @@ func (gs *Server) Endorse(ctx context.Context, request *gp.EndorseRequest) (*gp.
 						e = plan.retry(e)
 					default:
 						logger.Debugw("Endorse call to endorser returned success", "channel", request.ChannelId, "txid", request.TransactionId, "numEndorsers", len(endorsers), "endorserAddress", e.endpointConfig.address, "endorserMspid", e.endpointConfig.mspid, "status", response.Response.Status, "message", response.Response.Message)
+
+						responseMessage := response.GetResponse()
+						if responseMessage != nil {
+							responseMessage.Payload = nil // Remove any duplicate response payload
+						}
+
 						endorsements := plan.update(e, response)
 						responseCh <- &endorserResponse{endorsementSet: endorsements}
 						e = nil
@@ -267,7 +282,6 @@ func (gs *Server) Endorse(ctx context.Context, request *gp.EndorseRequest) (*gp.
 	}
 
 	endorseResponse := &gp.EndorseResponse{
-		Result:              endorsements[0].GetResponse(),
 		PreparedTransaction: env,
 	}
 	return endorseResponse, nil

diff --git a/internal/pkg/gateway/api_test.go b/internal/pkg/gateway/api_test.go
@@ -838,11 +838,14 @@ func TestEndorse(t *testing.T) {
 
 			// test the assertions
 			require.NoError(t, err)
+
 			// assert the preparedTxn is the payload from the proposal response
-			require.Equal(t, []byte("mock_response"), response.Result.Payload, "Incorrect response")
+			chaincodeAction, err := protoutil.GetActionFromEnvelopeMsg(response.GetPreparedTransaction())
+			require.NoError(t, err)
+			require.Equal(t, []byte("mock_response"), chaincodeAction.GetResponse().GetPayload(), "Incorrect response")
 
 			// check the generated transaction envelope contains the correct endorsements
-			checkTransaction(t, tt.expectedEndorsers, response.PreparedTransaction)
+			checkTransaction(t, tt.expectedEndorsers, response.GetPreparedTransaction())
 
 			// check the correct endorsers (mocks) were called with the right parameters
 			checkEndorsers(t, tt.expectedEndorsers, test)

diff --git a/internal/pkg/gateway/apiutils.go b/internal/pkg/gateway/apiutils.go
@@ -13,6 +13,7 @@ import (
 	gp "github.com/hyperledger/fabric-protos-go/gateway"
 	"github.com/hyperledger/fabric-protos-go/peer"
 	"github.com/hyperledger/fabric/protoutil"
+	"github.com/pkg/errors"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
 )
@@ -78,3 +79,21 @@ func toRpcError(err error, unknownCode codes.Code) error {
 func errorDetail(e *endpointConfig, err error) *gp.ErrorDetail {
 	return &gp.ErrorDetail{Address: e.address, MspId: e.mspid, Message: err.Error()}
 }
+
+func getResultFromProposalResponse(proposalResponse *peer.ProposalResponse) ([]byte, error) {
+	responsePayload := &peer.ProposalResponsePayload{}
+	if err := proto.Unmarshal(proposalResponse.GetPayload(), responsePayload); err != nil {
+		return nil, errors.Wrap(err, "failed to deserialize proposal response payload")
+	}
+
+	return getResultFromProposalResponsePayload(responsePayload)
+}
+
+func getResultFromProposalResponsePayload(responsePayload *peer.ProposalResponsePayload) ([]byte, error) {
+	chaincodeAction := &peer.ChaincodeAction{}
+	if err := proto.Unmarshal(responsePayload.GetExtension(), chaincodeAction); err != nil {
+		return nil, errors.Wrap(err, "failed to deserialize chaincode action")
+	}
+
+	return chaincodeAction.GetResponse().GetPayload(), nil
+}