[FAB-1885] GetTransactionByID to return Tran Envelope

QSCC GetTransactionByID should return a Transaction  Envelope,
not a Transaction. The Envelope contains the signature and
Payload, which contains the transaction header and Transaction.

Clients will want the transaction header information when retrieving
the transaction.  They may also want the signature to verify that
the transaction hasn't changed since the time they submitted it.
The return type will change from Transaction proto to Envelope proto.

Change-Id: I1de238035c3b6cd00abb3ed8506c54566ee0f2b0
Signed-off-by: denyeart <[email protected]>

Author: denyeart

core/ledger/blkstorage/blockstorage.go

@@ -66,6 +66,6 @@ type BlockStore interface {
 	RetrieveBlocks(startNum uint64) (ledger.ResultsIterator, error)
 	RetrieveBlockByHash(blockHash []byte) (*common.Block, error)
 	RetrieveBlockByNumber(blockNum uint64) (*common.Block, error) // blockNum of  math.MaxUint64 will return last block
-	RetrieveTxByID(txID string) (*pb.Transaction, error)
+	RetrieveTxByID(txID string) (*common.Envelope, error)
 	Shutdown()
 }

core/ledger/blkstorage/fsblkstorage/blockfile_mgr.go

@@ -440,22 +440,22 @@ func (mgr *blockfileMgr) retrieveBlocks(startNum uint64) (*blocksItr, error) {
 	return newBlockItr(mgr, startNum), nil
 }
 
-func (mgr *blockfileMgr) retrieveTransactionByID(txID string) (*pb.Transaction, error) {
+func (mgr *blockfileMgr) retrieveTransactionByID(txID string) (*common.Envelope, error) {
 	logger.Debugf("retrieveTransactionByID() - txId = [%s]", txID)
 	loc, err := mgr.index.getTxLoc(txID)
 	if err != nil {
 		return nil, err
 	}
-	return mgr.fetchTransaction(loc)
+	return mgr.fetchTransactionEnvelope(loc)
 }
 
-func (mgr *blockfileMgr) retrieveTransactionForBlockNumTranNum(blockNum uint64, tranNum uint64) (*pb.Transaction, error) {
+func (mgr *blockfileMgr) retrieveTransactionForBlockNumTranNum(blockNum uint64, tranNum uint64) (*common.Envelope, error) {
 	logger.Debugf("retrieveTransactionForBlockNumTranNum() - blockNum = [%d], tranNum = [%d]", blockNum, tranNum)
 	loc, err := mgr.index.getTXLocForBlockNumTranNum(blockNum, tranNum)
 	if err != nil {
 		return nil, err
 	}
-	return mgr.fetchTransaction(loc)
+	return mgr.fetchTransactionEnvelope(loc)
 }
 
 func (mgr *blockfileMgr) fetchBlock(lp *fileLocPointer) (*common.Block, error) {
@@ -470,14 +470,14 @@ func (mgr *blockfileMgr) fetchBlock(lp *fileLocPointer) (*common.Block, error) {
 	return block, nil
 }
 
-func (mgr *blockfileMgr) fetchTransaction(lp *fileLocPointer) (*pb.Transaction, error) {
+func (mgr *blockfileMgr) fetchTransactionEnvelope(lp *fileLocPointer) (*common.Envelope, error) {
 	var err error
 	var txEnvelopeBytes []byte
 	if txEnvelopeBytes, err = mgr.fetchRawBytes(lp); err != nil {
 		return nil, err
 	}
 	_, n := proto.DecodeVarint(txEnvelopeBytes)
-	return extractTransaction(txEnvelopeBytes[n:])
+	return putil.GetEnvelopeFromBlock(txEnvelopeBytes[n:])
 }
 
 func (mgr *blockfileMgr) fetchBlockBytes(lp *fileLocPointer) ([]byte, error) {
@@ -562,24 +562,6 @@ func scanForLastCompleteBlock(rootDir string, fileNum int, startingOffset int64)
 	return blockStream.currentOffset, numBlocks, errRead
 }
 
-func extractTransaction(txEnvelopeBytes []byte) (*pb.Transaction, error) {
-	var err error
-	var txEnvelope *common.Envelope
-	var txPayload *common.Payload
-	var tx *pb.Transaction
-
-	if txEnvelope, err = putil.GetEnvelopeFromBlock(txEnvelopeBytes); err != nil {
-		return nil, err
-	}
-	if txPayload, err = putil.GetPayload(txEnvelope); err != nil {
-		return nil, err
-	}
-	if tx, err = putil.GetTransaction(txPayload.Data); err != nil {
-		return nil, err
-	}
-	return tx, nil
-}
-
 // checkpointInfo
 type checkpointInfo struct {
 	latestFileChunkSuffixNum int

core/ledger/blkstorage/fsblkstorage/blockfile_mgr_test.go

@@ -25,6 +25,7 @@ import (
 
 	"github.com/hyperledger/fabric/protos/common"
 	pb "github.com/hyperledger/fabric/protos/peer"
+	putil "github.com/hyperledger/fabric/protos/utils"
 )
 
 func TestBlockfileMgrBlockReadWrite(t *testing.T) {
@@ -146,11 +147,11 @@ func TestBlockfileMgrGetTxById(t *testing.T) {
 			// blockNum starts with 1
 			txID, err := extractTxID(blk.Data.Data[j])
 			testutil.AssertNoError(t, err, "")
-			txFromFileMgr, err := blkfileMgrWrapper.blockfileMgr.retrieveTransactionByID(txID)
+			txEnvelopeFromFileMgr, err := blkfileMgrWrapper.blockfileMgr.retrieveTransactionByID(txID)
 			testutil.AssertNoError(t, err, "Error while retrieving tx from blkfileMgr")
-			tx, err := extractTransaction(txEnvelopeBytes)
+			txEnvelope, err := putil.GetEnvelopeFromBlock(txEnvelopeBytes)
 			testutil.AssertNoError(t, err, "Error while unmarshalling tx")
-			testutil.AssertEquals(t, txFromFileMgr, tx)
+			testutil.AssertEquals(t, txEnvelopeFromFileMgr, txEnvelope)
 		}
 	}
 }

core/ledger/blkstorage/fsblkstorage/blockindex_test.go

@@ -22,6 +22,7 @@ import (
 
 	"github.com/hyperledger/fabric/core/ledger/blkstorage"
 	"github.com/hyperledger/fabric/core/ledger/testutil"
+	putil "github.com/hyperledger/fabric/protos/utils"
 )
 
 type noopIndex struct {
@@ -145,23 +146,25 @@ func testBlockIndexSelectiveIndexing(t *testing.T, indexItems []blkstorage.Index
 	// test 'retrieveTransactionByID'
 	txid, err := extractTxID(blocks[0].Data.Data[0])
 	testutil.AssertNoError(t, err, "")
-	tx, err := blockfileMgr.retrieveTransactionByID(txid)
+	txEnvelope, err := blockfileMgr.retrieveTransactionByID(txid)
 	if testutil.Contains(indexItems, blkstorage.IndexableAttrTxID) {
 		testutil.AssertNoError(t, err, "Error while retrieving tx by id")
-		txOrig, err := extractTransaction(blocks[0].Data.Data[0])
+		txEnvelopeBytes := blocks[0].Data.Data[0]
+		txEnvelopeOrig, err := putil.GetEnvelopeFromBlock(txEnvelopeBytes)
 		testutil.AssertNoError(t, err, "")
-		testutil.AssertEquals(t, tx, txOrig)
+		testutil.AssertEquals(t, txEnvelope, txEnvelopeOrig)
 	} else {
 		testutil.AssertSame(t, err, blkstorage.ErrAttrNotIndexed)
 	}
 
 	//test 'retrieveTrasnactionsByBlockNumTranNum
-	tx2, err := blockfileMgr.retrieveTransactionForBlockNumTranNum(1, 1)
+	txEnvelope2, err := blockfileMgr.retrieveTransactionForBlockNumTranNum(1, 1)
 	if testutil.Contains(indexItems, blkstorage.IndexableAttrBlockNumTranNum) {
 		testutil.AssertNoError(t, err, "Error while retrieving tx by blockNum and tranNum")
-		txOrig2, err2 := extractTransaction(blocks[0].Data.Data[0])
+		txEnvelopeBytes2 := blocks[0].Data.Data[0]
+		txEnvelopeOrig2, err2 := putil.GetEnvelopeFromBlock(txEnvelopeBytes2)
 		testutil.AssertNoError(t, err2, "")
-		testutil.AssertEquals(t, tx2, txOrig2)
+		testutil.AssertEquals(t, txEnvelope2, txEnvelopeOrig2)
 	} else {
 		testutil.AssertSame(t, err, blkstorage.ErrAttrNotIndexed)
 	}

core/ledger/blkstorage/fsblkstorage/fs_blockstore.go

@@ -69,7 +69,7 @@ func (store *fsBlockStore) RetrieveBlockByNumber(blockNum uint64) (*common.Block
 }
 
 // RetrieveTxByID returns a transaction for given transaction id
-func (store *fsBlockStore) RetrieveTxByID(txID string) (*pb.Transaction, error) {
+func (store *fsBlockStore) RetrieveTxByID(txID string) (*common.Envelope, error) {
 	return store.fileMgr.retrieveTransactionByID(txID)
 }
 

core/ledger/kvledger/kv_ledger.go

@@ -210,7 +210,7 @@ func recommitLostBlocks(l *kvLedger, savepoint uint64, blockHeight uint64, recov
 }
 
 // GetTransactionByID retrieves a transaction by id
-func (l *kvLedger) GetTransactionByID(txID string) (*pb.Transaction, error) {
+func (l *kvLedger) GetTransactionByID(txID string) (*common.Envelope, error) {
 	return l.blockStore.RetrieveTxByID(txID)
 }
 

core/ledger/ledger_interface.go

@@ -76,7 +76,7 @@ type OrdererLedgerProvider interface {
 type PeerLedger interface {
 	Ledger
 	// GetTransactionByID retrieves a transaction by id
-	GetTransactionByID(txID string) (*pb.Transaction, error)
+	GetTransactionByID(txID string) (*common.Envelope, error)
 	// GetBlockByHash returns a block given it's hash
 	GetBlockByHash(blockHash []byte) (*common.Block, error)
 	// NewTxSimulator gives handle to a transaction simulator.
@@ -173,9 +173,8 @@ type KV struct {
 
 // KeyModification - QueryResult for History.
 type KeyModification struct {
-	TxID        string
-	Value       []byte
-	Transaction *pb.Transaction
+	TxID  string
+	Value []byte
 }
 
 // QueryRecord - Result structure for query records. Holds a namespace, key and record.

core/scc/qscc/querier.go

@@ -192,13 +192,14 @@ func getTransactionByID(vledger ledger.PeerLedger, tid []byte) pb.Response {
 	if tid == nil {
 		return shim.Error("Transaction ID must not be nil.")
 	}
-	tx, err := vledger.GetTransactionByID(string(tid))
+	txEnvelope, err := vledger.GetTransactionByID(string(tid))
 	if err != nil {
 		return shim.Error(fmt.Sprintf("Failed to get transaction with id %s, error %s", string(tid), err))
 	}
-	// TODO: tx is *pb.Transaction, what should we return?
+	// TODO In the returned transaction, need to replace binary simulation results with a proto
+	//  structure including write set, so that clients know what this transaction wrote
 
-	bytes, err := utils.Marshal(tx)
+	bytes, err := utils.Marshal(txEnvelope)
 	if err != nil {
 		return shim.Error(err.Error())
 	}
@@ -219,6 +220,9 @@ func getBlockByNumber(vledger ledger.PeerLedger, number []byte) pb.Response {
 		return shim.Error(fmt.Sprintf("Failed to get block number %d, error %s", bnum, err))
 	}
 	// TODO: consider trim block content before returning
+	//  Specifically, trim transaction 'data' out of the transaction array Payloads
+	//  This will preserve the transaction Payload header,
+	//  and client can do GetTransactionByID() if they want the full transaction details
 
 	bytes, err := utils.Marshal(block)
 	if err != nil {
@@ -237,6 +241,9 @@ func getBlockByHash(vledger ledger.PeerLedger, hash []byte) pb.Response {
 		return shim.Error(fmt.Sprintf("Failed to get block hash %s, error %s", string(hash), err))
 	}
 	// TODO: consider trim block content before returning
+	//  Specifically, trim transaction 'data' out of the transaction array Payloads
+	//  This will preserve the transaction Payload header,
+	//  and client can do GetTransactionByID() if they want the full transaction details
 
 	bytes, err := utils.Marshal(block)
 	if err != nil {