Merge pull request #1719 from tamasblummer/successful-tx-only

Do not store failed invoke transactions into the ledger

bddtests/peer_basic.feature

@@ -311,6 +311,42 @@ Feature: Network of Peers
         | foobar |
       Then I should get a JSON response with "OK" = "[{"columns":[{"Value":{"String_":"foobar"}}]}]"
 
+@doNotDecompose
+#    @wip
+	Scenario: chaincode example 01 single peer erroneous TX
+	    Given we compose "docker-compose-1.yml"
+	    When requesting "/chain" from "vp0"
+	    Then I should get a JSON response with "height" = "1"
+	    When I deploy chaincode "github.com/hyperledger/fabric/examples/chaincode/go/chaincode_example01" with ctor "init" to "vp0"
+		     | arg1 |  arg2 | arg3 | arg4 |
+		     |  a   |  100  |  b   |  200 |
+	    Then I should have received a chaincode name
+	    Then I wait up to "60" seconds for transaction to be committed to all peers
+
+	    When requesting "/chain" from "vp0"
+	    Then I should get a JSON response with "height" = "2"
+
+        When I invoke chaincode "example1" function name "invoke" on "vp0"
+			|arg1|
+			| 1  |
+	    Then I should have received a transactionID
+	    Then I wait up to "25" seconds for transaction to be committed to all peers
+
+	    When requesting "/chain" from "vp0"
+	    Then I should get a JSON response with "height" = "3"
+	    When requesting "/chain/blocks/2" from "vp0"
+	    Then I should get a JSON response containing "transactions" attribute
+
+        When I invoke chaincode "example1" function name "invoke" on "vp0"
+			|arg1|
+			| a  |
+	    Then I should have received a transactionID
+	    Then I wait "10" seconds
+        When requesting "/chain" from "vp0"
+	    Then I should get a JSON response with "height" = "4"
+        When requesting "/chain/blocks/3" from "vp0"
+	    Then I should get a JSON response containing no "transactions" attribute
+
 #    @doNotDecompose
 #    @wip
 #    Arg[0] = a, base64 = 'YQ=='

bddtests/steps/peer_basic_impl.py

@@ -127,6 +127,14 @@ def step_impl(context, path, containerName):
     context.response = resp
     print("")
 
+@then(u'I should get a JSON response containing "{attribute}" attribute')
+def step_impl(context, attribute):
+    assert attribute in context.response.json(), "Attribute not found in response (%s)" %(attribute)
+
+@then(u'I should get a JSON response containing no "{attribute}" attribute')
+def step_impl(context, attribute):
+    assert attribute not in context.response.json(), "Attribute found in response (%s)" %(attribute)
+
 @then(u'I should get a JSON response with "{attribute}" = "{expectedValue}"')
 def step_impl(context, attribute, expectedValue):
     assert attribute in context.response.json(), "Attribute not found in response (%s)" %(attribute)

consensus/helper/helper.go

@@ -180,10 +180,11 @@ func (h *Helper) ExecTxs(id interface{}, txs []*pb.Transaction) ([]byte, error)
 	// cxt := context.WithValue(context.Background(), "security", h.coordinator.GetSecHelper())
 	// TODO return directly once underlying implementation no longer returns []error
 
-	res, ccevents, txerrs, err := chaincode.ExecuteTransactions(context.Background(), chaincode.DefaultChain, txs)
-	h.curBatch = append(h.curBatch, txs...) // TODO, remove after issue 579
+	succeededTxs, res, ccevents, txerrs, err := chaincode.ExecuteTransactions(context.Background(), chaincode.DefaultChain, txs)
 
-	//copy errs to results
+	h.curBatch = append(h.curBatch, succeededTxs...) // TODO, remove after issue 579
+
+	//copy errs to result
 	txresults := make([]*pb.TransactionResult, len(txerrs))
 
 	//process errors for each transaction

consensus/obcpbft/mock_ledger_test.go

@@ -212,13 +212,7 @@ func (mock *MockLedger) commonCommitTx(id interface{}, metadata []byte, preview
 		PreviousBlockHash: previousBlockHash,
 		StateHash:         mock.curResults, // Use the current result output in the hash
 		Transactions:      mock.curBatch,
-		NonHashData: &protos.NonHashData{
-			TransactionResults: []*protos.TransactionResult{
-				{
-					Result: mock.curResults,
-				},
-			},
-		},
+		NonHashData:       &protos.NonHashData{},
 	}
 
 	if !preview {

consensus/obcpbft/obc-batch_test.go

@@ -74,9 +74,6 @@ func TestNetworkBatch(t *testing.T) {
 			t.Fatalf("Replica %d executed %d requests, expected %d",
 				ce.id, numTrans, batchSize)
 		}
-		if numTxResults := len(block.NonHashData.TransactionResults); numTxResults != 1 /*numTrans*/ {
-			t.Fatalf("Replica %d has %d txResults, expected %d", ce.id, numTxResults, numTrans)
-		}
 	}
 }
 

core/chaincode/exectransaction.go

@@ -135,24 +135,30 @@ func Execute(ctxt context.Context, chain *ChaincodeSupport, t *pb.Transaction) (
 //will return an array of errors one for each transaction. If the execution
 //succeeded, array element will be nil. returns []byte of state hash or
 //error
-func ExecuteTransactions(ctxt context.Context, cname ChainName, xacts []*pb.Transaction) (stateHash []byte, ccevents []*pb.ChaincodeEvent, txerrs []error, err error) {
+func ExecuteTransactions(ctxt context.Context, cname ChainName, xacts []*pb.Transaction) (succeededTXs []*pb.Transaction, stateHash []byte, ccevents []*pb.ChaincodeEvent, txerrs []error, err error) {
 	var chain = GetChain(cname)
 	if chain == nil {
 		// TODO: We should never get here, but otherwise a good reminder to better handle
 		panic(fmt.Sprintf("[ExecuteTransactions]Chain %s not found\n", cname))
 	}
+
 	txerrs = make([]error, len(xacts))
 	ccevents = make([]*pb.ChaincodeEvent, len(xacts))
+	var succeededTxs = make([]*pb.Transaction, 0)
 	for i, t := range xacts {
 		_, ccevents[i], txerrs[i] = Execute(ctxt, chain, t)
+		if txerrs[i] == nil {
+			succeededTxs = append(succeededTxs, t)
+		}
 	}
 
 	var lgr *ledger.Ledger
 	lgr, err = ledger.GetLedger()
 	if err == nil {
 		stateHash, err = lgr.GetTempStateHash()
 	}
-	return stateHash, ccevents, txerrs, err
+
+	return succeededTxs, stateHash, ccevents, txerrs, err
 }
 
 // GetSecureContext returns the security context from the context object or error

core/devops.go

@@ -463,16 +463,3 @@ func (d *Devops) EXP_ExecuteWithBinding(ctx context.Context, executeWithBinding
 	devopsLogger.Warning("Security NOT enabled")
 	return &pb.Response{Status: pb.Response_FAILURE, Msg: []byte("Security NOT enabled")}, nil
 }
-
-// GetTransactionResult request a TransactionResult.  The Response.Msg will contain the TransactionResult if successfully found the transaction in the chain.
-func (d *Devops) GetTransactionResult(ctx context.Context, txRequest *pb.TransactionRequest) (*pb.Response, error) {
-	txResult, err := d.coord.GetTransactionResultByUUID(txRequest.TransactionUuid)
-	if err != nil {
-		return &pb.Response{Status: pb.Response_FAILURE, Msg: []byte(fmt.Sprintf("Error getting transaction Result: %s", err.Error()))}, nil
-	}
-	txResultBytes, err := proto.Marshal(txResult)
-	if err != nil {
-		return &pb.Response{Status: pb.Response_FAILURE, Msg: []byte(fmt.Sprintf("Error getting transaction Result for tx UUID = %s, could not marshal txResult: %s", txRequest.TransactionUuid, err.Error()))}, nil
-	}
-	return &pb.Response{Status: pb.Response_SUCCESS, Msg: txResultBytes}, nil
-}

core/ledger/blockchain.go

@@ -21,8 +21,6 @@ import (
 	"encoding/binary"
 	"strconv"
 
-	"fmt"
-
 	"github.com/hyperledger/fabric/core/db"
 	"github.com/hyperledger/fabric/core/util"
 	"github.com/hyperledger/fabric/protos"
@@ -123,30 +121,6 @@ func (blockchain *blockchain) getTransactionByUUID(txUUID string) (*protos.Trans
 	return transaction, nil
 }
 
-func (blockchain *blockchain) getTransactionResultByUUID(txUUID string) (*protos.TransactionResult, error) {
-	blockNumber, txIndex, err := blockchain.indexer.fetchTransactionIndexByUUID(txUUID)
-	if err != nil {
-		return nil, err
-	}
-	block, err := blockchain.getBlock(blockNumber)
-	if err != nil {
-		return nil, err
-	}
-	nonHashData := block.GetNonHashData()
-	if nonHashData == nil {
-		return nil, fmt.Errorf("Error getting TransactionResult for txUUID = %s, nonHashData was nil", txUUID)
-	}
-	transactionResults := nonHashData.GetTransactionResults()
-	if transactionResults == nil {
-		return nil, fmt.Errorf("Error getting TransactionResult for txUUID = %s, GetTransactionResults returned nil", txUUID)
-	}
-	if txIndex > uint64(len(transactionResults))-1 {
-		return nil, fmt.Errorf("Error getting TransactionResult for txUUID = %s, txIndex (%d) out of range in TransactionResults with length = %d", txUUID, txIndex, len(transactionResults))
-	}
-	txResult := transactionResults[txIndex]
-	return txResult, nil
-}
-
 // getTransactions get all transactions in a block identified by block number
 func (blockchain *blockchain) getTransactions(blockNumber uint64) ([]*protos.Transaction, error) {
 	block, err := blockchain.getBlock(blockNumber)

core/ledger/ledger.go

@@ -159,7 +159,7 @@ func (ledger *Ledger) CommitTxBatch(id interface{}, transactions []*protos.Trans
 	writeBatch := gorocksdb.NewWriteBatch()
 	defer writeBatch.Destroy()
 	block := protos.NewBlock(transactions, metadata)
-	block.NonHashData = &protos.NonHashData{TransactionResults: transactionResults}
+	block.NonHashData = &protos.NonHashData{}
 	newBlockNumber, err := ledger.blockchain.addPersistenceChangesForNewBlock(context.TODO(), block, stateHash, writeBatch)
 	if err != nil {
 		ledger.resetForNextTxGroup(false)
@@ -180,6 +180,9 @@ func (ledger *Ledger) CommitTxBatch(id interface{}, transactions []*protos.Trans
 	ledger.blockchain.blockPersistenceStatus(true)
 
 	sendProducerBlockEvent(block)
+	if len(transactionResults) != 0 {
+		ledgerLogger.Debug("There were some erroneous transactions. We need to send a 'TX rejected' message here.")
+	}
 	return nil
 }
 
@@ -380,11 +383,6 @@ func (ledger *Ledger) GetTransactionByUUID(txUUID string) (*protos.Transaction,
 	return ledger.blockchain.getTransactionByUUID(txUUID)
 }
 
-// GetTransactionByUUID return transaction by it's uuid
-func (ledger *Ledger) GetTransactionResultByUUID(txUUID string) (*protos.TransactionResult, error) {
-	return ledger.blockchain.getTransactionResultByUUID(txUUID)
-}
-
 // PutRawBlock puts a raw block on the chain. This function should only be
 // used for synchronization between peers.
 func (ledger *Ledger) PutRawBlock(block *protos.Block, blockNumber uint64) error {
@@ -492,20 +490,4 @@ func sendProducerBlockEvent(block *protos.Block) {
 	}
 
 	producer.Send(producer.CreateBlockEvent(block))
-
-	//when we send block event, send chaincode events as well
-	sendChaincodeEvents(block)
-}
-
-//send chaincode events created by transactions in the block
-func sendChaincodeEvents(block *protos.Block) {
-	nonHashData := block.GetNonHashData()
-	if nonHashData != nil {
-		trs := nonHashData.GetTransactionResults()
-		for _, tr := range trs {
-			if tr.ChaincodeEvent != nil {
-				producer.Send(producer.CreateChaincodeEvent(tr.ChaincodeEvent))
-			}
-		}
-	}
 }

core/ledger/ledger_test.go

@@ -706,40 +706,6 @@ func TestGetTransactionByUUID(t *testing.T) {
 	testutil.AssertNil(t, ledgerTransaction)
 }
 
-func TestTransactionResult(t *testing.T) {
-	ledgerTestWrapper := createFreshDBAndTestLedgerWrapper(t)
-	ledger := ledgerTestWrapper.ledger
-
-	// Block 0
-	ledger.BeginTxBatch(0)
-	ledger.TxBegin("txUuid1")
-	ledger.SetState("chaincode1", "key1", []byte("value1A"))
-	ledger.SetState("chaincode2", "key2", []byte("value2A"))
-	ledger.SetState("chaincode3", "key3", []byte("value3A"))
-	ledger.TxFinished("txUuid1", true)
-	transaction, uuid := buildTestTx(t)
-
-	transactionResult := &protos.TransactionResult{Uuid: uuid, ErrorCode: 500, Error: "bad"}
-
-	ledger.CommitTxBatch(0, []*protos.Transaction{transaction}, []*protos.TransactionResult{transactionResult}, []byte("proof"))
-
-	block := ledgerTestWrapper.GetBlockByNumber(0)
-
-	nonHashData := block.GetNonHashData()
-	if nonHashData == nil {
-		t.Fatal("Expected block to have non hash data, but non hash data was nil.")
-	}
-
-	if nonHashData.TransactionResults == nil || len(nonHashData.TransactionResults) == 0 {
-		t.Fatal("Expected block to have non hash data transaction results.")
-	}
-
-	testutil.AssertEquals(t, nonHashData.TransactionResults[0].Uuid, uuid)
-	testutil.AssertEquals(t, nonHashData.TransactionResults[0].Error, "bad")
-	testutil.AssertEquals(t, nonHashData.TransactionResults[0].ErrorCode, uint32(500))
-
-}
-
 func TestRangeScanIterator(t *testing.T) {
 	ledgerTestWrapper := createFreshDBAndTestLedgerWrapper(t)
 	ledger := ledgerTestWrapper.ledger

core/ledger/test/ledger_test.go

@@ -170,11 +170,6 @@ var _ = Describe("Ledger", func() {
 			Expect(err).To(BeNil())
 			nonHashData := block.GetNonHashData()
 			Expect(nonHashData).ToNot(BeNil())
-			Expect(nonHashData.TransactionResults).ToNot(BeNil())
-			Expect(len(nonHashData.TransactionResults)).ToNot(Equal(0))
-			Expect(nonHashData.TransactionResults[0].Uuid).To(Equal(uuid))
-			Expect(nonHashData.TransactionResults[0].Error).To(Equal("bad"))
-			Expect(nonHashData.TransactionResults[0].ErrorCode).To(Equal(uint32(500)))
 		})
 	})
 

core/peer/peer.go

@@ -74,11 +74,6 @@ type BlockChainAccessor interface {
 	GetCurrentStateHash() (stateHash []byte, err error)
 }
 
-// TransactionAccessor interface for retrieving transaction information
-type TransactionAccessor interface {
-	GetTransactionResultByUUID(txUuid string) (*pb.TransactionResult, error)
-}
-
 // BlockChainModifier interface for applying changes to the block chain
 type BlockChainModifier interface {
 	ApplyStateDelta(id interface{}, delta *statemgmt.StateDelta) error
@@ -117,7 +112,6 @@ type MessageHandlerCoordinator interface {
 	BlockChainModifier
 	BlockChainUtil
 	StateAccessor
-	TransactionAccessor
 	RegisterHandler(messageHandler MessageHandler) error
 	DeregisterHandler(messageHandler MessageHandler) error
 	Broadcast(*pb.Message, pb.PeerEndpoint_Type) []error
@@ -788,13 +782,6 @@ func (p *PeerImpl) signMessageMutating(msg *pb.Message) error {
 	return nil
 }
 
-// GetTransactionResultByUUID Return the TransactionResult for the specified transaction ID.
-func (p *PeerImpl) GetTransactionResultByUUID(txUuid string) (*pb.TransactionResult, error) {
-	p.ledgerWrapper.RLock()
-	defer p.ledgerWrapper.RUnlock()
-	return p.ledgerWrapper.ledger.GetTransactionResultByUUID(txUuid)
-}
-
 // initDiscovery load the addresses from the discovery list previously saved to disk and adds them to the current discovery list
 func (p *PeerImpl) initDiscovery() []string {
 	p.discHelper = discovery.NewDiscoveryImpl()

core/peer/statetransfer/statetransfer_mock_test.go

@@ -624,17 +624,9 @@ func SimpleEncodeUint64(num uint64) []byte {
 
 func SimpleHashBlock(block *protos.Block) []byte {
 	buffer := make([]byte, binary.MaxVarintLen64)
-	if nil != block.NonHashData && nil != block.NonHashData.TransactionResults {
-		for _, txResult := range block.NonHashData.TransactionResults {
-			for i, b := range txResult.Result {
-				buffer[i%binary.MaxVarintLen64] += b
-			}
-		}
-	} else {
-		for _, transaction := range block.Transactions {
-			for i, b := range transaction.Payload {
-				buffer[i%binary.MaxVarintLen64] += b
-			}
+	for _, transaction := range block.Transactions {
+		for i, b := range transaction.Payload {
+			buffer[i%binary.MaxVarintLen64] += b
 		}
 	}
 	return []byte(fmt.Sprintf("BlockHash:%s-%s-%s", buffer, block.StateHash, block.ConsensusMetadata))

core/rest/rest_api_test.go

@@ -131,10 +131,6 @@ func (d *mockDevops) EXP_ExecuteWithBinding(ctx context.Context, executeWithBind
 	return nil, nil
 }
 
-func (d *mockDevops) GetTransactionResult(ctx context.Context, txRequest *protos.TransactionRequest) (*protos.Response, error) {
-	return nil, nil
-}
-
 func initGlobalServerOpenchain(t *testing.T) {
 	var err error
 	serverOpenchain, err = NewOpenchainServerWithPeerInfo(new(peerInfo))