From eb077441d043215ff56485920bb37813bc0a450d Mon Sep 17 00:00:00 2001 From: Will Lahti Date: Fri, 2 Jun 2017 12:33:05 -0400 Subject: [PATCH] [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 --- core/chaincode/shim/response.go | 7 ++- core/endorser/endorser.go | 12 ++--- core/scc/escc/endorser_onevalidsignature.go | 9 ++-- .../escc/endorser_onevalidsignature_test.go | 20 ++++--- peer/chaincode/invoke_test.go | 52 ++++++++++--------- protos/utils/txutils.go | 2 +- 6 files changed, 58 insertions(+), 44 deletions(-) diff --git a/core/chaincode/shim/response.go b/core/chaincode/shim/response.go index 6de4954d204..2250ad16455 100644 --- a/core/chaincode/shim/response.go +++ b/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 ) diff --git a/core/endorser/endorser.go b/core/endorser/endorser.go index 57a2d461b02..8c376076084 100644 --- a/core/endorser/endorser.go +++ b/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} } diff --git a/core/scc/escc/endorser_onevalidsignature.go b/core/scc/escc/endorser_onevalidsignature.go index 2a6e06d418c..623caca47d2 100644 --- a/core/scc/escc/endorser_onevalidsignature.go +++ b/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) diff --git a/core/scc/escc/endorser_onevalidsignature_test.go b/core/scc/escc/endorser_onevalidsignature_test.go index 1c66f3f2e68..083ccdfcdaa 100644 --- a/core/scc/escc/endorser_onevalidsignature_test.go +++ b/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) diff --git a/peer/chaincode/invoke_test.go b/peer/chaincode/invoke_test.go index a851e0ed89c..15d132c38fc 100644 --- a/peer/chaincode/invoke_test.go +++ b/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 diff --git a/protos/utils/txutils.go b/protos/utils/txutils.go index af19a27bae2..bae903ecd20 100644 --- a/protos/utils/txutils.go +++ b/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 {