[FAB-3618] Lower endorsement error threshold to 400

This CR lowers the threshold for endorsing/rejecting a proposal
from 500 to 400. This keeps endorsement consistent with HTTP status
codes, which define errors as having a status code >=400.

Change-Id: Ib1e174905527858e232132d0ca1779d301e5e475
Signed-off-by: Will Lahti <[email protected]>

Author: wlahti

core/chaincode/shim/response.go

@@ -20,11 +20,14 @@ import (
 )
 
 const (
-	// If status code less than 500, endorser will endorse it.
+	// OK constant - status code less than 400, endorser will endorse it.
 	// OK means init or invoke successfully.
 	OK = 200
 
-	// Code that greater than or equal to 500 will be considered an error and rejected by endorser.
+	// ERRORTHRESHOLD constant - status code greater than or equal to 400 will be considered an error and rejected by endorser.
+	ERRORTHRESHOLD = 400
+
+	// ERROR constant - default error value
 	ERROR = 500
 )
 

core/endorser/endorser.go

@@ -129,10 +129,10 @@ func (e *Endorser) callChaincode(ctxt context.Context, chainID string, version s
 		return nil, nil, err
 	}
 
-	//per doc anything < 500 can be sent as TX.
-	//fabric errors will always be >= 500 (ie, unambiguous errors )
-	//"lscc" will respond with status 200 or >=500 (ie, unambiguous OK or ERROR)
-	if res.Status >= shim.ERROR {
+	//per doc anything < 400 can be sent as TX.
+	//fabric errors will always be >= 400 (ie, unambiguous errors )
+	//"lscc" will respond with status 200 or 500 (ie, unambiguous OK or ERROR)
+	if res.Status >= shim.ERRORTHRESHOLD {
 		return res, nil, nil
 	}
 
@@ -370,7 +370,7 @@ func (e *Endorser) endorseProposal(ctx context.Context, chainID string, txid str
 		return nil, err
 	}
 
-	if res.Status >= shim.ERROR {
+	if res.Status >= shim.ERRORTHRESHOLD {
 		return &pb.ProposalResponse{Response: res}, nil
 	}
 
@@ -520,7 +520,7 @@ func (e *Endorser) ProcessProposal(ctx context.Context, signedProp *pb.SignedPro
 			return &pb.ProposalResponse{Response: &pb.Response{Status: 500, Message: err.Error()}}, err
 		}
 		if pResp != nil {
-			if res.Status >= shim.ERROR {
+			if res.Status >= shim.ERRORTHRESHOLD {
 				endorserLogger.Debugf("endorseProposal() resulted in chaincode error for txid: %s", txid)
 				return pResp, &chaincodeError{res.Status, res.Message}
 			}

core/scc/escc/endorser_onevalidsignature.go

@@ -57,7 +57,6 @@ func (e *EndorserOneValidSignature) Init(stub shim.ChaincodeStubInterface) pb.Re
 // args[5] - binary blob of simulation results
 // args[6] - serialized events
 // args[7] - payloadVisibility
-
 //
 // NOTE: this chaincode is meant to sign another chaincode's simulation
 // results. It should not manipulate state as any state change will be
@@ -101,7 +100,7 @@ func (e *EndorserOneValidSignature) Invoke(stub shim.ChaincodeStubInterface) pb.
 	}
 
 	// handle executing chaincode result
-	// Status code < 500 can be endorsed
+	// Status code < shim.ERRORTHRESHOLD can be endorsed
 	if args[4] == nil {
 		return shim.Error("Response of chaincode executing is null")
 	}
@@ -111,8 +110,8 @@ func (e *EndorserOneValidSignature) Invoke(stub shim.ChaincodeStubInterface) pb.
 		return shim.Error(fmt.Sprintf("Failed to get Response of executing chaincode: %s", err.Error()))
 	}
 
-	if response.Status >= shim.ERROR {
-		return shim.Error(fmt.Sprintf("Status code less than 500 will be endorsed, get status code: %d", response.Status))
+	if response.Status >= shim.ERRORTHRESHOLD {
+		return shim.Error(fmt.Sprintf("Status code less than %d will be endorsed, received status code: %d", shim.ERRORTHRESHOLD, response.Status))
 	}
 
 	// handle simulation results
@@ -164,7 +163,7 @@ func (e *EndorserOneValidSignature) Invoke(stub shim.ChaincodeStubInterface) pb.
 	// marshall the proposal response so that we return its bytes
 	prBytes, err := utils.GetBytesProposalResponse(presp)
 	if err != nil {
-		return shim.Error(fmt.Sprintf("Could not marshall ProposalResponse: err %s", err))
+		return shim.Error(fmt.Sprintf("Could not marshal ProposalResponse: err %s", err))
 	}
 
 	pResp, err := utils.GetProposalResponse(prBytes)

core/scc/escc/endorser_onevalidsignature_test.go

@@ -31,6 +31,7 @@ import (
 	"github.com/hyperledger/fabric/protos/common"
 	pb "github.com/hyperledger/fabric/protos/peer"
 	putils "github.com/hyperledger/fabric/protos/utils"
+	"github.com/stretchr/testify/assert"
 )
 
 func TestInit(t *testing.T) {
@@ -49,8 +50,10 @@ func TestInvoke(t *testing.T) {
 	stub := shim.NewMockStub("endorseronevalidsignature", e)
 	successResponse := &pb.Response{Status: 200, Payload: []byte("payload")}
 	failResponse := &pb.Response{Status: 500, Message: "error"}
+	ccFailResponse := &pb.Response{Status: 400, Message: "chaincode error"}
 	successRes, _ := putils.GetBytesResponse(successResponse)
 	failRes, _ := putils.GetBytesResponse(failResponse)
+	ccFailRes, _ := putils.GetBytesResponse(ccFailResponse)
 	ccid := &pb.ChaincodeID{Name: "foo", Version: "v1"}
 	ccidBytes, err := proto.Marshal(ccid)
 	if err != nil {
@@ -152,12 +155,17 @@ func TestInvoke(t *testing.T) {
 		t.Fatalf("escc invoke should have failed with a null action struct.  args: %v", args)
 	}
 
-	// Failed path: status code >=500
+	// Failed path: status code = 500
 	args = [][]byte{[]byte("test"), []byte("test"), []byte("test"), ccidBytes, failRes, []byte("test")}
-	if res := stub.MockInvoke("1", args); res.Status == shim.OK {
-		fmt.Println("Invoke", args, "failed", string(res.Message))
-		t.Fatalf("escc invoke should have failed with a null response.  args: %v", args)
-	}
+	res := stub.MockInvoke("1", args)
+	assert.NotEqual(t, res.Status, shim.OK, "Invoke should have failed with status code: %d", ccFailResponse.Status)
+	assert.Contains(t, res.Message, fmt.Sprintf("Status code less than %d will be endorsed", shim.ERRORTHRESHOLD))
+
+	// Failed path: status code = 400
+	args = [][]byte{[]byte("test"), []byte("test"), []byte("test"), ccidBytes, ccFailRes, []byte("test")}
+	res = stub.MockInvoke("1", args)
+	assert.NotEqual(t, res.Status, shim.OK, "Invoke should have failed with status code: %d", ccFailResponse.Status)
+	assert.Contains(t, res.Message, fmt.Sprintf("Status code less than %d will be endorsed", shim.ERRORTHRESHOLD))
 
 	// Successful path - create a proposal
 	cs := &pb.ChaincodeSpec{
@@ -199,7 +207,7 @@ func TestInvoke(t *testing.T) {
 
 	// success test 1: invocation with mandatory args only
 	args = [][]byte{[]byte(""), proposal.Header, proposal.Payload, ccidBytes, successRes, simRes}
-	res := stub.MockInvoke("1", args)
+	res = stub.MockInvoke("1", args)
 	if res.Status != shim.OK {
 		t.Fail()
 		t.Fatalf("escc invoke failed with: %s", res.Message)

peer/chaincode/invoke_test.go

@@ -139,31 +139,35 @@ func TestInvokeCmdEndorsementError(t *testing.T) {
 
 func TestInvokeCmdEndorsementFailure(t *testing.T) {
 	InitMSP()
-	ccRespStatus := int32(502)
-	ccRespPayload := []byte("Invalid function name")
-	mockCF, err := getMockChaincodeCmdFactoryEndorsementFailure(ccRespStatus, ccRespPayload)
-	assert.NoError(t, err, "Error getting mock chaincode command factory")
-
-	cmd := invokeCmd(mockCF)
-	AddFlags(cmd)
-	args := []string{"-n", "example02", "-p", "github.com/hyperledger/fabric/examples/chaincode/go/chaincode_example02",
-		"-c", "{\"Args\": [\"invokeinvalid\",\"a\",\"b\",\"10\"]}"}
-	cmd.SetArgs(args)
-
-	// set logger to logger with a backend that writes to a byte buffer
-	var buffer bytes.Buffer
-	logger.SetBackend(logging.AddModuleLevel(logging.NewLogBackend(&buffer, "", 0)))
-	// reset the logger after test
-	defer func() {
-		flogging.Reset()
-	}()
-	// make sure buffer is "clean" before running the invoke
-	buffer.Reset()
+	ccRespStatus := [2]int32{502, 400}
+	ccRespPayload := [][]byte{[]byte("Invalid function name"), []byte("Incorrect parameters")}
+
+	for i := 0; i < 2; i++ {
+		mockCF, err := getMockChaincodeCmdFactoryEndorsementFailure(ccRespStatus[i], ccRespPayload[i])
+		assert.NoError(t, err, "Error getting mock chaincode command factory")
+
+		cmd := invokeCmd(mockCF)
+		AddFlags(cmd)
+		args := []string{"-n", "example02", "-p", "github.com/hyperledger/fabric/examples/chaincode/go/chaincode_example02",
+			"-c", "{\"Args\": [\"invokeinvalid\",\"a\",\"b\",\"10\"]}"}
+		cmd.SetArgs(args)
+
+		// set logger to logger with a backend that writes to a byte buffer
+		var buffer bytes.Buffer
+		logger.SetBackend(logging.AddModuleLevel(logging.NewLogBackend(&buffer, "", 0)))
+		// reset the logger after test
+		defer func() {
+			flogging.Reset()
+		}()
+		// make sure buffer is "clean" before running the invoke
+		buffer.Reset()
+
+		err = cmd.Execute()
+		assert.NoError(t, err)
+		assert.Regexp(t, "Endorsement failure during invoke", buffer.String())
+		assert.Regexp(t, fmt.Sprintf("chaincode result: status:%d payload:\"%s\"", ccRespStatus[i], ccRespPayload[i]), buffer.String())
+	}
 
-	err = cmd.Execute()
-	assert.NoError(t, err)
-	assert.Regexp(t, "Endorsement failure during invoke", buffer.String())
-	assert.Regexp(t, fmt.Sprintf("chaincode result: status:%d payload:\"%s\"", ccRespStatus, ccRespPayload), buffer.String())
 }
 
 // Returns mock chaincode command factory

protos/utils/txutils.go

@@ -252,7 +252,7 @@ func CreateProposalResponse(hdrbytes []byte, payl []byte, response *peer.Respons
 
 // CreateProposalResponseFailure creates a proposal response for cases where
 // endorsement proposal fails either due to a endorsement failure or a chaincode
-// failure (chaincode response status >=500)
+// failure (chaincode response status >= shim.ERRORTHRESHOLD)
 func CreateProposalResponseFailure(hdrbytes []byte, payl []byte, response *peer.Response, results []byte, events []byte, ccid *peer.ChaincodeID, visibility []byte) (*peer.ProposalResponse, error) {
 	hdr, err := GetHeader(hdrbytes)
 	if err != nil {