diff --git a/common/ledger/blkstorage/blockfile_mgr.go b/common/ledger/blkstorage/blockfile_mgr.go index 67d99b0b1cf..6cd5f11009d 100644 --- a/common/ledger/blkstorage/blockfile_mgr.go +++ b/common/ledger/blkstorage/blockfile_mgr.go @@ -586,6 +586,10 @@ func (mgr *blockfileMgr) retrieveBlocks(startNum uint64) (*blocksItr, error) { return newBlockItr(mgr, startNum), nil } +func (mgr *blockfileMgr) txIDExists(txID string) (bool, error) { + return mgr.index.txIDExists(txID) +} + func (mgr *blockfileMgr) retrieveTransactionByID(txID string) (*common.Envelope, error) { logger.Debugf("retrieveTransactionByID() - txId = [%s]", txID) loc, err := mgr.index.getTxLoc(txID) diff --git a/common/ledger/blkstorage/blockfile_mgr_test.go b/common/ledger/blkstorage/blockfile_mgr_test.go index 5c17cd7f28f..9127e618c32 100644 --- a/common/ledger/blkstorage/blockfile_mgr_test.go +++ b/common/ledger/blkstorage/blockfile_mgr_test.go @@ -170,6 +170,49 @@ func TestBlockfileMgrBlockchainInfo(t *testing.T) { require.Equal(t, uint64(10), bcInfo.Height) } +func TestTxIDExists(t *testing.T) { + t.Run("green-path", func(t *testing.T) { + env := newTestEnv(t, NewConf(testPath(), 0)) + defer env.Cleanup() + + blkStore, err := env.provider.Open("testLedger") + require.NoError(t, err) + defer blkStore.Shutdown() + + blocks := testutil.ConstructTestBlocks(t, 2) + for _, blk := range blocks { + require.NoError(t, blkStore.AddBlock(blk)) + } + + for _, blk := range blocks { + for i := range blk.Data.Data { + txID, err := protoutil.GetOrComputeTxIDFromEnvelope(blk.Data.Data[i]) + require.NoError(t, err) + exists, err := blkStore.TxIDExists(txID) + require.NoError(t, err) + require.True(t, exists) + } + } + exists, err := blkStore.TxIDExists("non-existant-txid") + require.NoError(t, err) + require.False(t, exists) + }) + + t.Run("error-path", func(t *testing.T) { + env := newTestEnv(t, NewConf(testPath(), 0)) + defer env.Cleanup() + + blkStore, err := env.provider.Open("testLedger") + require.NoError(t, err) + defer blkStore.Shutdown() + + env.provider.Close() + exists, err := blkStore.TxIDExists("random") + require.EqualError(t, err, "error while trying to check the presence of TXID [random]: internal leveldb error while obtaining db iterator: leveldb: closed") + require.False(t, exists) + }) +} + func TestBlockfileMgrGetTxById(t *testing.T) { env := newTestEnv(t, NewConf(testPath(), 0)) defer env.Cleanup() diff --git a/common/ledger/blkstorage/blockindex.go b/common/ledger/blkstorage/blockindex.go index 6f18d7c9039..64fcad031ed 100644 --- a/common/ledger/blkstorage/blockindex.go +++ b/common/ledger/blkstorage/blockindex.go @@ -223,6 +223,24 @@ func (index *blockIndex) getTxValidationCodeByTxID(txID string) (peer.TxValidati return peer.TxValidationCode(v.TxValidationCode), nil } +func (index *blockIndex) txIDExists(txID string) (bool, error) { + if !index.isAttributeIndexed(IndexableAttrTxID) { + return false, ErrAttrNotIndexed + } + rangeScan := constructTxIDRangeScan(txID) + itr, err := index.db.GetIterator(rangeScan.startKey, rangeScan.stopKey) + if err != nil { + return false, errors.WithMessagef(err, "error while trying to check the presence of TXID [%s]", txID) + } + defer itr.Release() + + present := itr.Next() + if err := itr.Error(); err != nil { + return false, errors.Wrapf(err, "error while trying to check the presence of TXID [%s]", txID) + } + return present, nil +} + func (index *blockIndex) getTxIDVal(txID string) (*TxIDIndexValue, error) { if !index.isAttributeIndexed(IndexableAttrTxID) { return nil, ErrAttrNotIndexed diff --git a/common/ledger/blkstorage/blockindex_test.go b/common/ledger/blkstorage/blockindex_test.go index 1d9928beab0..897748329c6 100644 --- a/common/ledger/blkstorage/blockindex_test.go +++ b/common/ledger/blkstorage/blockindex_test.go @@ -151,6 +151,17 @@ func testBlockIndexSelectiveIndexing(t *testing.T, indexItems []IndexableAttr) { require.Exactly(t, ErrAttrNotIndexed, err) } + // test txIDExists + txid, err = protoutil.GetOrComputeTxIDFromEnvelope(blocks[0].Data.Data[0]) + require.NoError(t, err) + exists, err := blockfileMgr.txIDExists(txid) + if containsAttr(indexItems, IndexableAttrTxID) { + require.NoError(t, err) + require.True(t, exists) + } else { + require.Exactly(t, ErrAttrNotIndexed, err) + } + //test 'retrieveTrasnactionsByBlockNumTranNum txEnvelope2, err := blockfileMgr.retrieveTransactionByBlockNumTranNum(0, 0) if containsAttr(indexItems, IndexableAttrBlockNumTranNum) { diff --git a/common/ledger/blkstorage/blockstore.go b/common/ledger/blkstorage/blockstore.go index 0822de5ea36..553cd30f8ff 100644 --- a/common/ledger/blkstorage/blockstore.go +++ b/common/ledger/blkstorage/blockstore.go @@ -72,6 +72,11 @@ func (store *BlockStore) RetrieveBlockByNumber(blockNum uint64) (*common.Block, return store.fileMgr.retrieveBlockByNumber(blockNum) } +// TxIDExists returns true if a transaction with the txID is ever committed +func (store *BlockStore) TxIDExists(txID string) (bool, error) { + return store.fileMgr.txIDExists(txID) +} + // RetrieveTxByID returns a transaction for given transaction id func (store *BlockStore) RetrieveTxByID(txID string) (*common.Envelope, error) { return store.fileMgr.retrieveTransactionByID(txID) diff --git a/common/ledger/blkstorage/snapshot_test.go b/common/ledger/blkstorage/snapshot_test.go index 4888d003585..47ebda4192c 100644 --- a/common/ledger/blkstorage/snapshot_test.go +++ b/common/ledger/blkstorage/snapshot_test.go @@ -491,6 +491,10 @@ func verifyQueriesOnBlocksPriorToSnapshot( _, err = bootstrappedBlockStore.RetrieveTxValidationCodeByTxID(txID) require.EqualError(t, err, expectedErrorStr) + + exists, err := bootstrappedBlockStore.TxIDExists(txID) + require.NoError(t, err) + require.True(t, exists) } } } @@ -540,6 +544,10 @@ func verifyQueriesOnBlocksAddedAfterBootstrapping(t *testing.T, expectedTxEnv, err := protoutil.GetEnvelopeFromBlock(block.Data.Data[j]) require.NoError(t, err) require.Equal(t, expectedTxEnv, retrievedTxEnv) + + exists, err := bootstrappedBlockStore.TxIDExists(txID) + require.NoError(t, err) + require.True(t, exists) } for j, validationCode := range d.validationCodes { diff --git a/core/chaincode/mock/peer_ledger.go b/core/chaincode/mock/peer_ledger.go index f59ebc049ed..8f203c612eb 100644 --- a/core/chaincode/mock/peer_ledger.go +++ b/core/chaincode/mock/peer_ledger.go @@ -290,6 +290,19 @@ type PeerLedger struct { submitSnapshotRequestReturnsOnCall map[int]struct { result1 error } + TxIDExistsStub func(string) (bool, error) + txIDExistsMutex sync.RWMutex + txIDExistsArgsForCall []struct { + arg1 string + } + txIDExistsReturns struct { + result1 bool + result2 error + } + txIDExistsReturnsOnCall map[int]struct { + result1 bool + result2 error + } invocations map[string][][]interface{} invocationsMutex sync.RWMutex } @@ -1649,6 +1662,69 @@ func (fake *PeerLedger) SubmitSnapshotRequestReturnsOnCall(i int, result1 error) }{result1} } +func (fake *PeerLedger) TxIDExists(arg1 string) (bool, error) { + fake.txIDExistsMutex.Lock() + ret, specificReturn := fake.txIDExistsReturnsOnCall[len(fake.txIDExistsArgsForCall)] + fake.txIDExistsArgsForCall = append(fake.txIDExistsArgsForCall, struct { + arg1 string + }{arg1}) + fake.recordInvocation("TxIDExists", []interface{}{arg1}) + fake.txIDExistsMutex.Unlock() + if fake.TxIDExistsStub != nil { + return fake.TxIDExistsStub(arg1) + } + if specificReturn { + return ret.result1, ret.result2 + } + fakeReturns := fake.txIDExistsReturns + return fakeReturns.result1, fakeReturns.result2 +} + +func (fake *PeerLedger) TxIDExistsCallCount() int { + fake.txIDExistsMutex.RLock() + defer fake.txIDExistsMutex.RUnlock() + return len(fake.txIDExistsArgsForCall) +} + +func (fake *PeerLedger) TxIDExistsCalls(stub func(string) (bool, error)) { + fake.txIDExistsMutex.Lock() + defer fake.txIDExistsMutex.Unlock() + fake.TxIDExistsStub = stub +} + +func (fake *PeerLedger) TxIDExistsArgsForCall(i int) string { + fake.txIDExistsMutex.RLock() + defer fake.txIDExistsMutex.RUnlock() + argsForCall := fake.txIDExistsArgsForCall[i] + return argsForCall.arg1 +} + +func (fake *PeerLedger) TxIDExistsReturns(result1 bool, result2 error) { + fake.txIDExistsMutex.Lock() + defer fake.txIDExistsMutex.Unlock() + fake.TxIDExistsStub = nil + fake.txIDExistsReturns = struct { + result1 bool + result2 error + }{result1, result2} +} + +func (fake *PeerLedger) TxIDExistsReturnsOnCall(i int, result1 bool, result2 error) { + fake.txIDExistsMutex.Lock() + defer fake.txIDExistsMutex.Unlock() + fake.TxIDExistsStub = nil + if fake.txIDExistsReturnsOnCall == nil { + fake.txIDExistsReturnsOnCall = make(map[int]struct { + result1 bool + result2 error + }) + } + fake.txIDExistsReturnsOnCall[i] = struct { + result1 bool + result2 error + }{result1, result2} +} + func (fake *PeerLedger) Invocations() map[string][][]interface{} { fake.invocationsMutex.RLock() defer fake.invocationsMutex.RUnlock() @@ -1698,6 +1774,8 @@ func (fake *PeerLedger) Invocations() map[string][][]interface{} { defer fake.pendingSnapshotRequestsMutex.RUnlock() fake.submitSnapshotRequestMutex.RLock() defer fake.submitSnapshotRequestMutex.RUnlock() + fake.txIDExistsMutex.RLock() + defer fake.txIDExistsMutex.RUnlock() copiedInvocations := map[string][][]interface{}{} for key, value := range fake.invocations { copiedInvocations[key] = value diff --git a/core/committer/committer_test.go b/core/committer/committer_test.go index 8fd1b6b61c2..8b5ffd73318 100644 --- a/core/committer/committer_test.go +++ b/core/committer/committer_test.go @@ -59,6 +59,12 @@ func (m *mockLedger) Close() { } +// TxIDExists returns true if the specified txID is already present in one of the already committed blocks +func (m *mockLedger) TxIDExists(txID string) (bool, error) { + args := m.Called(txID) + return args.Get(0).(bool), args.Error(1) +} + func (m *mockLedger) GetTransactionByID(txID string) (*peer.ProcessedTransaction, error) { args := m.Called(txID) return args.Get(0).(*peer.ProcessedTransaction), args.Error(1) diff --git a/core/committer/txvalidator/v14/validator.go b/core/committer/txvalidator/v14/validator.go index a79573311aa..bbcc2704fdb 100644 --- a/core/committer/txvalidator/v14/validator.go +++ b/core/committer/txvalidator/v14/validator.go @@ -453,29 +453,25 @@ func (v *TxValidator) checkTxIdDupsLedger(tIdx int, chdr *common.ChannelHeader, txID := chdr.TxId // Look for a transaction with the same identifier inside the ledger - _, err := ldgr.GetTransactionByID(txID) + exists, err := ldgr.TxIDExists(txID) - switch err.(type) { - case nil: - // invalid case, returned error is nil. It means that there is already a tx in the ledger with the same id - logger.Error("Duplicate transaction found, ", txID, ", skipping") - return &blockValidationResult{ - tIdx: tIdx, - validationCode: peer.TxValidationCode_DUPLICATE_TXID, - } - case ledger.NotFoundInIndexErr: - // valid case, returned error is of type NotFoundInIndexErr. - // It means that no tx with the same id is found in the ledger - return nil - default: - // invalid case, returned error is not of type NotFoundInIndexErr. - // It means that we could not verify whether a tx with the supplied id is in the ledger + if err != nil { logger.Errorf("Ledger failure while attempting to detect duplicate status for txid %s: %s", txID, err) return &blockValidationResult{ tIdx: tIdx, err: err, } } + + if exists { + logger.Error("Duplicate transaction found, ", txID, ", skipping") + return &blockValidationResult{ + tIdx: tIdx, + validationCode: peer.TxValidationCode_DUPLICATE_TXID, + } + } + + return nil } // generateCCKey generates a unique identifier for chaincode in specific channel diff --git a/core/committer/txvalidator/v14/validator_test.go b/core/committer/txvalidator/v14/validator_test.go index 77827380fed..272f88c5137 100644 --- a/core/committer/txvalidator/v14/validator_test.go +++ b/core/committer/txvalidator/v14/validator_test.go @@ -1426,6 +1426,12 @@ type mockLedger struct { mock.Mock } +// TxIDExists returns true if the specified txID is already present in one of the already committed blocks +func (m *mockLedger) TxIDExists(txID string) (bool, error) { + args := m.Called(txID) + return args.Get(0).(bool), args.Error(1) +} + // GetTransactionByID returns transaction by ud func (m *mockLedger) GetTransactionByID(txID string) (*peer.ProcessedTransaction, error) { args := m.Called(txID) @@ -1715,7 +1721,7 @@ func TestLedgerIsNoAvailable(t *testing.T) { ccID := "mycc" tx := getEnv(ccID, nil, createRWset(t, ccID), t) - theLedger.On("GetTransactionByID", mock.Anything).Return(&peer.ProcessedTransaction{}, ledger.NotFoundInIndexErr("")) + theLedger.On("TxIDExists", mock.Anything).Return(false, nil) queryExecutor := new(mockQueryExecutor) queryExecutor.On("GetState", mock.Anything, mock.Anything).Return([]byte{}, errors.New("Unable to connect to DB")) @@ -1751,7 +1757,7 @@ func TestLedgerIsNotAvailableForCheckingTxidDuplicate(t *testing.T) { ccID := "mycc" tx := getEnv(ccID, nil, createRWset(t, ccID), t) - theLedger.On("GetTransactionByID", mock.Anything).Return(&peer.ProcessedTransaction{}, errors.New("Unable to connect to DB")) + theLedger.On("TxIDExists", mock.Anything).Return(false, errors.New("Unable to connect to DB")) b := &common.Block{ Data: &common.BlockData{Data: [][]byte{protoutil.MarshalOrPanic(tx)}}, @@ -1781,7 +1787,7 @@ func TestDuplicateTxId(t *testing.T) { ccID := "mycc" tx := getEnv(ccID, nil, createRWset(t, ccID), t) - theLedger.On("GetTransactionByID", mock.Anything).Return(&peer.ProcessedTransaction{}, nil) + theLedger.On("TxIDExists", mock.Anything).Return(true, nil) b := &common.Block{ Data: &common.BlockData{Data: [][]byte{protoutil.MarshalOrPanic(tx)}}, @@ -1822,7 +1828,7 @@ func TestValidationInvalidEndorsing(t *testing.T) { ccID := "mycc" tx := getEnv(ccID, nil, createRWset(t, ccID), t) - theLedger.On("GetTransactionByID", mock.Anything).Return(&peer.ProcessedTransaction{}, ledger.NotFoundInIndexErr("")) + theLedger.On("TxIDExists", mock.Anything).Return(false, nil) cd := &ccp.ChaincodeData{ Name: ccID, @@ -1851,7 +1857,7 @@ func TestValidationInvalidEndorsing(t *testing.T) { func createMockLedger(t *testing.T, ccID string) *mockLedger { l := new(mockLedger) - l.On("GetTransactionByID", mock.Anything).Return(&peer.ProcessedTransaction{}, ledger.NotFoundInIndexErr("")) + l.On("TxIDExists", mock.Anything).Return(false, nil) cd := &ccp.ChaincodeData{ Name: ccID, Version: ccVersion, diff --git a/core/committer/txvalidator/v20/mocks/ledger_resources.go b/core/committer/txvalidator/v20/mocks/ledger_resources.go index 3eefdae545a..6242c7398ca 100644 --- a/core/committer/txvalidator/v20/mocks/ledger_resources.go +++ b/core/committer/txvalidator/v20/mocks/ledger_resources.go @@ -5,8 +5,6 @@ package mocks import ( ledger "github.com/hyperledger/fabric/core/ledger" mock "github.com/stretchr/testify/mock" - - peer "github.com/hyperledger/fabric-protos-go/peer" ) // LedgerResources is an autogenerated mock type for the LedgerResources type @@ -14,22 +12,22 @@ type LedgerResources struct { mock.Mock } -// GetTransactionByID provides a mock function with given fields: txID -func (_m *LedgerResources) GetTransactionByID(txID string) (*peer.ProcessedTransaction, error) { - ret := _m.Called(txID) +// NewQueryExecutor provides a mock function with given fields: +func (_m *LedgerResources) NewQueryExecutor() (ledger.QueryExecutor, error) { + ret := _m.Called() - var r0 *peer.ProcessedTransaction - if rf, ok := ret.Get(0).(func(string) *peer.ProcessedTransaction); ok { - r0 = rf(txID) + var r0 ledger.QueryExecutor + if rf, ok := ret.Get(0).(func() ledger.QueryExecutor); ok { + r0 = rf() } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(*peer.ProcessedTransaction) + r0 = ret.Get(0).(ledger.QueryExecutor) } } var r1 error - if rf, ok := ret.Get(1).(func(string) error); ok { - r1 = rf(txID) + if rf, ok := ret.Get(1).(func() error); ok { + r1 = rf() } else { r1 = ret.Error(1) } @@ -37,22 +35,20 @@ func (_m *LedgerResources) GetTransactionByID(txID string) (*peer.ProcessedTrans return r0, r1 } -// NewQueryExecutor provides a mock function with given fields: -func (_m *LedgerResources) NewQueryExecutor() (ledger.QueryExecutor, error) { - ret := _m.Called() +// TxIDExists provides a mock function with given fields: txID +func (_m *LedgerResources) TxIDExists(txID string) (bool, error) { + ret := _m.Called(txID) - var r0 ledger.QueryExecutor - if rf, ok := ret.Get(0).(func() ledger.QueryExecutor); ok { - r0 = rf() + var r0 bool + if rf, ok := ret.Get(0).(func(string) bool); ok { + r0 = rf(txID) } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(ledger.QueryExecutor) - } + r0 = ret.Get(0).(bool) } var r1 error - if rf, ok := ret.Get(1).(func() error); ok { - r1 = rf() + if rf, ok := ret.Get(1).(func(string) error); ok { + r1 = rf(txID) } else { r1 = ret.Error(1) } diff --git a/core/committer/txvalidator/v20/mocks/query_executor.go b/core/committer/txvalidator/v20/mocks/query_executor.go index b0027c62405..5a4b84b0f20 100644 --- a/core/committer/txvalidator/v20/mocks/query_executor.go +++ b/core/committer/txvalidator/v20/mocks/query_executor.go @@ -2,9 +2,12 @@ package mocks -import coreledger "github.com/hyperledger/fabric/core/ledger" -import ledger "github.com/hyperledger/fabric/common/ledger" -import mock "github.com/stretchr/testify/mock" +import ( + ledger "github.com/hyperledger/fabric/common/ledger" + coreledger "github.com/hyperledger/fabric/core/ledger" + + mock "github.com/stretchr/testify/mock" +) // QueryExecutor is an autogenerated mock type for the QueryExecutor type type QueryExecutor struct { diff --git a/core/committer/txvalidator/v20/txvalidator_test.go b/core/committer/txvalidator/v20/txvalidator_test.go index 2d31dfaa4af..df6a55dc58d 100644 --- a/core/committer/txvalidator/v20/txvalidator_test.go +++ b/core/committer/txvalidator/v20/txvalidator_test.go @@ -17,7 +17,6 @@ import ( "github.com/hyperledger/fabric/common/semaphore" tmocks "github.com/hyperledger/fabric/core/committer/txvalidator/mocks" "github.com/hyperledger/fabric/core/committer/txvalidator/v20/mocks" - ledger2 "github.com/hyperledger/fabric/core/ledger" "github.com/hyperledger/fabric/core/ledger/kvledger/txmgmt/rwsetutil" mocktxvalidator "github.com/hyperledger/fabric/core/mocks/txvalidator" "github.com/hyperledger/fabric/internal/pkg/txflags" @@ -58,7 +57,7 @@ func testValidationWithNTXes(t *testing.T, nBlocks int) { mockDispatcher := &mockDispatcher{} mockLedger := &mocks.LedgerResources{} mockCapabilities := &tmocks.ApplicationCapabilities{} - mockLedger.On("GetTransactionByID", mock.Anything).Return(nil, ledger2.NotFoundInIndexErr("Day after day, day after day")) + mockLedger.On("TxIDExists", mock.Anything).Return(false, nil) tValidator := &TxValidator{ ChannelID: "", Semaphore: semaphore.New(10), @@ -127,7 +126,7 @@ func TestBlockValidationDuplicateTXId(t *testing.T) { mockCapabilities := &tmocks.ApplicationCapabilities{} mockCapabilities.On("ForbidDuplicateTXIdInBlock").Return(true) mockLedger := &mocks.LedgerResources{} - mockLedger.On("GetTransactionByID", mock.Anything).Return(nil, ledger2.NotFoundInIndexErr("As idle as a painted ship upon a painted ocean")) + mockLedger.On("TxIDExists", mock.Anything).Return(false, nil) tValidator := &TxValidator{ ChannelID: "", Semaphore: semaphore.New(10), @@ -174,7 +173,7 @@ func TestTxValidationFailure_InvalidTxid(t *testing.T) { require.NoError(t, err) mockLedger := &mocks.LedgerResources{} - mockLedger.On("GetTransactionByID", mock.Anything).Return(nil, ledger2.NotFoundInIndexErr("Water, water, everywhere, nor any drop to drink")) + mockLedger.On("TxIDExists", mock.Anything).Return(false, nil) mockCapabilities := &tmocks.ApplicationCapabilities{} tValidator := &TxValidator{ ChannelID: "", diff --git a/core/committer/txvalidator/v20/validator.go b/core/committer/txvalidator/v20/validator.go index 34d3dfc796a..83a65644c6f 100644 --- a/core/committer/txvalidator/v20/validator.go +++ b/core/committer/txvalidator/v20/validator.go @@ -59,8 +59,8 @@ type ChannelResources interface { // LedgerResources provides access to ledger artefacts or // functions to interact with them type LedgerResources interface { - // GetTransactionByID retrieves a transaction by id - GetTransactionByID(txID string) (*peer.ProcessedTransaction, error) + // TxIDExists returns true if the specified txID is already present in one of the already committed blocks + TxIDExists(txID string) (bool, error) // NewQueryExecutor gives handle to a query executor. // A client can obtain more than one 'QueryExecutor's for parallel execution. @@ -462,29 +462,22 @@ func (v *TxValidator) checkTxIdDupsLedger(tIdx int, chdr *common.ChannelHeader, txID := chdr.TxId // Look for a transaction with the same identifier inside the ledger - _, err := ldgr.GetTransactionByID(txID) - - switch err.(type) { - case nil: - // invalid case, returned error is nil. It means that there is already a tx in the ledger with the same id - logger.Error("Duplicate transaction found, ", txID, ", skipping") - return &blockValidationResult{ - tIdx: tIdx, - validationCode: peer.TxValidationCode_DUPLICATE_TXID, - } - case ledger.NotFoundInIndexErr: - // valid case, returned error is of type NotFoundInIndexErr. - // It means that no tx with the same id is found in the ledger - return nil - default: - // invalid case, returned error is not of type NotFoundInIndexErr. - // It means that we could not verify whether a tx with the supplied id is in the ledger + exists, err := ldgr.TxIDExists(txID) + if err != nil { logger.Errorf("Ledger failure while attempting to detect duplicate status for txid %s: %s", txID, err) return &blockValidationResult{ tIdx: tIdx, err: err, } } + if exists { + logger.Error("Duplicate transaction found, ", txID, ", skipping") + return &blockValidationResult{ + tIdx: tIdx, + validationCode: peer.TxValidationCode_DUPLICATE_TXID, + } + } + return nil } type dynamicDeserializer struct { diff --git a/core/committer/txvalidator/v20/validator_test.go b/core/committer/txvalidator/v20/validator_test.go index e0997bb836a..7dc6d6eff89 100644 --- a/core/committer/txvalidator/v20/validator_test.go +++ b/core/committer/txvalidator/v20/validator_test.go @@ -33,7 +33,6 @@ import ( ccp "github.com/hyperledger/fabric/core/common/ccprovider" validation "github.com/hyperledger/fabric/core/handlers/validation/api" "github.com/hyperledger/fabric/core/handlers/validation/builtin" - "github.com/hyperledger/fabric/core/ledger" "github.com/hyperledger/fabric/core/ledger/kvledger/txmgmt/rwsetutil" mocktxvalidator "github.com/hyperledger/fabric/core/mocks/txvalidator" "github.com/hyperledger/fabric/core/scc/lscc" @@ -171,7 +170,7 @@ func setupValidatorWithMspMgr(mspmgr msp.MSPManager, mockID *supportmocks.Identi mockQE.On("GetState", "lscc", "escc").Return(nil, nil) mockLedger := &txvalidatormocks.LedgerResources{} - mockLedger.On("GetTransactionByID", mock.Anything).Return(nil, ledger.NotFoundInIndexErr("As idle as a painted ship upon a painted ocean")) + mockLedger.On("TxIDExists", mock.Anything).Return(false, nil) mockLedger.On("NewQueryExecutor").Return(mockQE, nil) mockCpmg := &plugindispatchermocks.ChannelPolicyManagerGetter{} @@ -1017,7 +1016,7 @@ func TestLedgerIsNotAvailableForCheckingTxidDuplicate(t *testing.T) { mockLedger := &txvalidatormocks.LedgerResources{} v.LedgerResources = mockLedger - mockLedger.On("GetTransactionByID", mock.Anything).Return(nil, errors.New("uh, oh")) + mockLedger.On("TxIDExists", mock.Anything).Return(false, errors.New("uh, oh")) b := &common.Block{ Data: &common.BlockData{Data: [][]byte{protoutil.MarshalOrPanic(tx)}}, @@ -1038,7 +1037,7 @@ func TestDuplicateTxId(t *testing.T) { mockLedger := &txvalidatormocks.LedgerResources{} v.LedgerResources = mockLedger - mockLedger.On("GetTransactionByID", mock.Anything).Return(&peer.ProcessedTransaction{}, nil) + mockLedger.On("TxIDExists", mock.Anything).Return(true, nil) tx := getEnv(ccID, nil, createRWset(t, ccID), t) @@ -1080,7 +1079,7 @@ func TestValidationInvalidEndorsing(t *testing.T) { mockQE.On("Done").Return(nil) mockLedger := &txvalidatormocks.LedgerResources{} - mockLedger.On("GetTransactionByID", mock.Anything).Return(nil, ledger.NotFoundInIndexErr("As idle as a painted ship upon a painted ocean")) + mockLedger.On("TxIDExists", mock.Anything).Return(false, nil) mockLedger.On("NewQueryExecutor").Return(mockQE, nil) mockCpmg := &plugindispatchermocks.ChannelPolicyManagerGetter{} @@ -1155,7 +1154,7 @@ func TestValidationPluginExecutionError(t *testing.T) { }), nil) mockLedger := &txvalidatormocks.LedgerResources{} - mockLedger.On("GetTransactionByID", mock.Anything).Return(nil, ledger.NotFoundInIndexErr("As idle as a painted ship upon a painted ocean")) + mockLedger.On("TxIDExists", mock.Anything).Return(false, nil) mockLedger.On("NewQueryExecutor").Return(mockQE, nil) mockCpmg := &plugindispatchermocks.ChannelPolicyManagerGetter{} @@ -1208,7 +1207,7 @@ func TestValidationPluginNotFound(t *testing.T) { }), nil) mockLedger := &txvalidatormocks.LedgerResources{} - mockLedger.On("GetTransactionByID", mock.Anything).Return(nil, ledger.NotFoundInIndexErr("As idle as a painted ship upon a painted ocean")) + mockLedger.On("TxIDExists", mock.Anything).Return(false, nil) mockLedger.On("NewQueryExecutor").Return(mockQE, nil) mockCpmg := &plugindispatchermocks.ChannelPolicyManagerGetter{} diff --git a/core/ledger/kvledger/kv_ledger.go b/core/ledger/kvledger/kv_ledger.go index f57befd00f4..f354a449bd7 100644 --- a/core/ledger/kvledger/kv_ledger.go +++ b/core/ledger/kvledger/kv_ledger.go @@ -484,6 +484,13 @@ func (l *kvLedger) recommitLostBlocks(firstBlockNum uint64, lastBlockNum uint64, return nil } +// TxIDExists returns true if the specified txID is already present in one of the already committed blocks +func (l *kvLedger) TxIDExists(txID string) (bool, error) { + l.blockAPIsRWLock.RLock() + defer l.blockAPIsRWLock.RUnlock() + return l.blockStore.TxIDExists(txID) +} + // GetTransactionByID retrieves a transaction by id func (l *kvLedger) GetTransactionByID(txID string) (*peer.ProcessedTransaction, error) { l.blockAPIsRWLock.RLock() diff --git a/core/ledger/kvledger/kv_ledger_test.go b/core/ledger/kvledger/kv_ledger_test.go index af0328ff715..634dbb10681 100644 --- a/core/ledger/kvledger/kv_ledger_test.go +++ b/core/ledger/kvledger/kv_ledger_test.go @@ -59,89 +59,117 @@ func TestKVLedgerNilHistoryDBProvider(t *testing.T) { } func TestKVLedgerBlockStorage(t *testing.T) { - conf, cleanup := testConfig(t) - defer cleanup() - provider := testutilNewProvider(conf, t, &mock.DeployedChaincodeInfoProvider{}) - defer provider.Close() - - bg, gb := testutil.NewBlockGenerator(t, "testLedger", false) - gbHash := protoutil.BlockHeaderHash(gb.Header) - ledger, err := provider.CreateFromGenesisBlock(gb) - require.NoError(t, err) - defer ledger.Close() - - bcInfo, _ := ledger.GetBlockchainInfo() - require.Equal(t, &common.BlockchainInfo{ - Height: 1, CurrentBlockHash: gbHash, PreviousBlockHash: nil, - }, bcInfo) - - txid := util.GenerateUUID() - simulator, _ := ledger.NewTxSimulator(txid) - require.NoError(t, simulator.SetState("ns1", "key1", []byte("value1"))) - require.NoError(t, simulator.SetState("ns1", "key2", []byte("value2"))) - require.NoError(t, simulator.SetState("ns1", "key3", []byte("value3"))) - simulator.Done() - simRes, _ := simulator.GetTxSimulationResults() - pubSimBytes, _ := simRes.GetPubSimulationBytes() - block1 := bg.NextBlock([][]byte{pubSimBytes}) - require.NoError(t, ledger.CommitLegacy(&lgr.BlockAndPvtData{Block: block1}, &lgr.CommitOptions{})) + t.Run("green-path", func(t *testing.T) { + conf, cleanup := testConfig(t) + defer cleanup() + provider := testutilNewProvider(conf, t, &mock.DeployedChaincodeInfoProvider{}) + defer provider.Close() - bcInfo, _ = ledger.GetBlockchainInfo() - block1Hash := protoutil.BlockHeaderHash(block1.Header) - require.Equal(t, &common.BlockchainInfo{ - Height: 2, CurrentBlockHash: block1Hash, PreviousBlockHash: gbHash, - }, bcInfo) + bg, gb := testutil.NewBlockGenerator(t, "testLedger", false) + gbHash := protoutil.BlockHeaderHash(gb.Header) + ledger, err := provider.CreateFromGenesisBlock(gb) + require.NoError(t, err) + defer ledger.Close() + + bcInfo, _ := ledger.GetBlockchainInfo() + require.Equal(t, &common.BlockchainInfo{ + Height: 1, CurrentBlockHash: gbHash, PreviousBlockHash: nil, + }, bcInfo) + + txid := util.GenerateUUID() + simulator, _ := ledger.NewTxSimulator(txid) + require.NoError(t, simulator.SetState("ns1", "key1", []byte("value1"))) + require.NoError(t, simulator.SetState("ns1", "key2", []byte("value2"))) + require.NoError(t, simulator.SetState("ns1", "key3", []byte("value3"))) + simulator.Done() + simRes, _ := simulator.GetTxSimulationResults() + pubSimBytes, _ := simRes.GetPubSimulationBytes() + block1 := bg.NextBlock([][]byte{pubSimBytes}) + require.NoError(t, ledger.CommitLegacy(&lgr.BlockAndPvtData{Block: block1}, &lgr.CommitOptions{})) + + bcInfo, _ = ledger.GetBlockchainInfo() + block1Hash := protoutil.BlockHeaderHash(block1.Header) + require.Equal(t, &common.BlockchainInfo{ + Height: 2, CurrentBlockHash: block1Hash, PreviousBlockHash: gbHash, + }, bcInfo) + + txid = util.GenerateUUID() + simulator, _ = ledger.NewTxSimulator(txid) + require.NoError(t, simulator.SetState("ns1", "key1", []byte("value4"))) + require.NoError(t, simulator.SetState("ns1", "key2", []byte("value5"))) + require.NoError(t, simulator.SetState("ns1", "key3", []byte("value6"))) + simulator.Done() + simRes, _ = simulator.GetTxSimulationResults() + pubSimBytes, _ = simRes.GetPubSimulationBytes() + block2 := bg.NextBlock([][]byte{pubSimBytes}) + require.NoError(t, ledger.CommitLegacy(&lgr.BlockAndPvtData{Block: block2}, &lgr.CommitOptions{})) + + bcInfo, _ = ledger.GetBlockchainInfo() + block2Hash := protoutil.BlockHeaderHash(block2.Header) + require.Equal(t, &common.BlockchainInfo{ + Height: 3, CurrentBlockHash: block2Hash, PreviousBlockHash: block1Hash}, bcInfo) + + b0, _ := ledger.GetBlockByHash(gbHash) + require.True(t, proto.Equal(b0, gb), "proto messages are not equal") + + b1, _ := ledger.GetBlockByHash(block1Hash) + require.True(t, proto.Equal(b1, block1), "proto messages are not equal") + + b0, _ = ledger.GetBlockByNumber(0) + require.True(t, proto.Equal(b0, gb), "proto messages are not equal") + + b1, _ = ledger.GetBlockByNumber(1) + require.Equal(t, block1, b1) + + // get the tran id from the 2nd block, then use it to test GetTransactionByID() + txEnvBytes2 := block1.Data.Data[0] + txEnv2, err := protoutil.GetEnvelopeFromBlock(txEnvBytes2) + require.NoError(t, err, "Error upon GetEnvelopeFromBlock") + payload2, err := protoutil.UnmarshalPayload(txEnv2.Payload) + require.NoError(t, err, "Error upon GetPayload") + chdr, err := protoutil.UnmarshalChannelHeader(payload2.Header.ChannelHeader) + require.NoError(t, err, "Error upon GetChannelHeaderFromBytes") + txID2 := chdr.TxId + + exists, err := ledger.TxIDExists(txID2) + require.NoError(t, err) + require.True(t, exists) - txid = util.GenerateUUID() - simulator, _ = ledger.NewTxSimulator(txid) - require.NoError(t, simulator.SetState("ns1", "key1", []byte("value4"))) - require.NoError(t, simulator.SetState("ns1", "key2", []byte("value5"))) - require.NoError(t, simulator.SetState("ns1", "key3", []byte("value6"))) - simulator.Done() - simRes, _ = simulator.GetTxSimulationResults() - pubSimBytes, _ = simRes.GetPubSimulationBytes() - block2 := bg.NextBlock([][]byte{pubSimBytes}) - require.NoError(t, ledger.CommitLegacy(&lgr.BlockAndPvtData{Block: block2}, &lgr.CommitOptions{})) + processedTran2, err := ledger.GetTransactionByID(txID2) + require.NoError(t, err, "Error upon GetTransactionByID") + // get the tran envelope from the retrieved ProcessedTransaction + retrievedTxEnv2 := processedTran2.TransactionEnvelope + require.Equal(t, txEnv2, retrievedTxEnv2) - bcInfo, _ = ledger.GetBlockchainInfo() - block2Hash := protoutil.BlockHeaderHash(block2.Header) - require.Equal(t, &common.BlockchainInfo{ - Height: 3, CurrentBlockHash: block2Hash, PreviousBlockHash: block1Hash}, bcInfo) + // get the tran id from the 2nd block, then use it to test GetBlockByTxID + b1, _ = ledger.GetBlockByTxID(txID2) + require.True(t, proto.Equal(b1, block1), "proto messages are not equal") - b0, _ := ledger.GetBlockByHash(gbHash) - require.True(t, proto.Equal(b0, gb), "proto messages are not equal") + // get the transaction validation code for this transaction id + validCode, _ := ledger.GetTxValidationCodeByTxID(txID2) + require.Equal(t, peer.TxValidationCode_VALID, validCode) - b1, _ := ledger.GetBlockByHash(block1Hash) - require.True(t, proto.Equal(b1, block1), "proto messages are not equal") + exists, err = ledger.TxIDExists("random-txid") + require.NoError(t, err) + require.False(t, exists) + }) - b0, _ = ledger.GetBlockByNumber(0) - require.True(t, proto.Equal(b0, gb), "proto messages are not equal") + t.Run("error-path", func(t *testing.T) { + conf, cleanup := testConfig(t) + defer cleanup() + provider := testutilNewProvider(conf, t, &mock.DeployedChaincodeInfoProvider{}) + defer provider.Close() - b1, _ = ledger.GetBlockByNumber(1) - require.Equal(t, block1, b1) - - // get the tran id from the 2nd block, then use it to test GetTransactionByID() - txEnvBytes2 := block1.Data.Data[0] - txEnv2, err := protoutil.GetEnvelopeFromBlock(txEnvBytes2) - require.NoError(t, err, "Error upon GetEnvelopeFromBlock") - payload2, err := protoutil.UnmarshalPayload(txEnv2.Payload) - require.NoError(t, err, "Error upon GetPayload") - chdr, err := protoutil.UnmarshalChannelHeader(payload2.Header.ChannelHeader) - require.NoError(t, err, "Error upon GetChannelHeaderFromBytes") - txID2 := chdr.TxId - processedTran2, err := ledger.GetTransactionByID(txID2) - require.NoError(t, err, "Error upon GetTransactionByID") - // get the tran envelope from the retrieved ProcessedTransaction - retrievedTxEnv2 := processedTran2.TransactionEnvelope - require.Equal(t, txEnv2, retrievedTxEnv2) - - // get the tran id from the 2nd block, then use it to test GetBlockByTxID - b1, _ = ledger.GetBlockByTxID(txID2) - require.True(t, proto.Equal(b1, block1), "proto messages are not equal") + _, gb := testutil.NewBlockGenerator(t, "testLedger", false) + ledger, err := provider.CreateFromGenesisBlock(gb) + require.NoError(t, err) + defer ledger.Close() - // get the transaction validation code for this transaction id - validCode, _ := ledger.GetTxValidationCodeByTxID(txID2) - require.Equal(t, peer.TxValidationCode_VALID, validCode) + provider.blkStoreProvider.Close() + exists, err := ledger.TxIDExists("random-txid") + require.EqualError(t, err, "error while trying to check the presence of TXID [random-txid]: internal leveldb error while obtaining db iterator: leveldb: closed") + require.False(t, exists) + }) } func TestAddCommitHash(t *testing.T) { diff --git a/core/ledger/ledger_interface.go b/core/ledger/ledger_interface.go index 3a4844a1e97..65eebc96060 100644 --- a/core/ledger/ledger_interface.go +++ b/core/ledger/ledger_interface.go @@ -157,6 +157,9 @@ type PeerLedgerProvider interface { // that tells apart valid transactions from invalid ones type PeerLedger interface { commonledger.Ledger + // TxIDExists returns true if the specified txID is already present in one of the already committed blocks. + // This function returns error only if there is an underlying condition that prevents checking for the txID, such as an I/O error. + TxIDExists(txID string) (bool, error) // GetTransactionByID retrieves a transaction by id GetTransactionByID(txID string) (*peer.ProcessedTransaction, error) // GetBlockByHash returns a block given it's hash diff --git a/core/peer/mock/peer_ledger.go b/core/peer/mock/peer_ledger.go index 2abe50ec869..21d8a7e6648 100644 --- a/core/peer/mock/peer_ledger.go +++ b/core/peer/mock/peer_ledger.go @@ -290,6 +290,19 @@ type PeerLedger struct { submitSnapshotRequestReturnsOnCall map[int]struct { result1 error } + TxIDExistsStub func(string) (bool, error) + txIDExistsMutex sync.RWMutex + txIDExistsArgsForCall []struct { + arg1 string + } + txIDExistsReturns struct { + result1 bool + result2 error + } + txIDExistsReturnsOnCall map[int]struct { + result1 bool + result2 error + } invocations map[string][][]interface{} invocationsMutex sync.RWMutex } @@ -1649,6 +1662,69 @@ func (fake *PeerLedger) SubmitSnapshotRequestReturnsOnCall(i int, result1 error) }{result1} } +func (fake *PeerLedger) TxIDExists(arg1 string) (bool, error) { + fake.txIDExistsMutex.Lock() + ret, specificReturn := fake.txIDExistsReturnsOnCall[len(fake.txIDExistsArgsForCall)] + fake.txIDExistsArgsForCall = append(fake.txIDExistsArgsForCall, struct { + arg1 string + }{arg1}) + fake.recordInvocation("TxIDExists", []interface{}{arg1}) + fake.txIDExistsMutex.Unlock() + if fake.TxIDExistsStub != nil { + return fake.TxIDExistsStub(arg1) + } + if specificReturn { + return ret.result1, ret.result2 + } + fakeReturns := fake.txIDExistsReturns + return fakeReturns.result1, fakeReturns.result2 +} + +func (fake *PeerLedger) TxIDExistsCallCount() int { + fake.txIDExistsMutex.RLock() + defer fake.txIDExistsMutex.RUnlock() + return len(fake.txIDExistsArgsForCall) +} + +func (fake *PeerLedger) TxIDExistsCalls(stub func(string) (bool, error)) { + fake.txIDExistsMutex.Lock() + defer fake.txIDExistsMutex.Unlock() + fake.TxIDExistsStub = stub +} + +func (fake *PeerLedger) TxIDExistsArgsForCall(i int) string { + fake.txIDExistsMutex.RLock() + defer fake.txIDExistsMutex.RUnlock() + argsForCall := fake.txIDExistsArgsForCall[i] + return argsForCall.arg1 +} + +func (fake *PeerLedger) TxIDExistsReturns(result1 bool, result2 error) { + fake.txIDExistsMutex.Lock() + defer fake.txIDExistsMutex.Unlock() + fake.TxIDExistsStub = nil + fake.txIDExistsReturns = struct { + result1 bool + result2 error + }{result1, result2} +} + +func (fake *PeerLedger) TxIDExistsReturnsOnCall(i int, result1 bool, result2 error) { + fake.txIDExistsMutex.Lock() + defer fake.txIDExistsMutex.Unlock() + fake.TxIDExistsStub = nil + if fake.txIDExistsReturnsOnCall == nil { + fake.txIDExistsReturnsOnCall = make(map[int]struct { + result1 bool + result2 error + }) + } + fake.txIDExistsReturnsOnCall[i] = struct { + result1 bool + result2 error + }{result1, result2} +} + func (fake *PeerLedger) Invocations() map[string][][]interface{} { fake.invocationsMutex.RLock() defer fake.invocationsMutex.RUnlock() @@ -1698,6 +1774,8 @@ func (fake *PeerLedger) Invocations() map[string][][]interface{} { defer fake.pendingSnapshotRequestsMutex.RUnlock() fake.submitSnapshotRequestMutex.RLock() defer fake.submitSnapshotRequestMutex.RUnlock() + fake.txIDExistsMutex.RLock() + defer fake.txIDExistsMutex.RUnlock() copiedInvocations := map[string][][]interface{}{} for key, value := range fake.invocations { copiedInvocations[key] = value diff --git a/internal/peer/node/mock/peer_ledger.go b/internal/peer/node/mock/peer_ledger.go index f59ebc049ed..8f203c612eb 100644 --- a/internal/peer/node/mock/peer_ledger.go +++ b/internal/peer/node/mock/peer_ledger.go @@ -290,6 +290,19 @@ type PeerLedger struct { submitSnapshotRequestReturnsOnCall map[int]struct { result1 error } + TxIDExistsStub func(string) (bool, error) + txIDExistsMutex sync.RWMutex + txIDExistsArgsForCall []struct { + arg1 string + } + txIDExistsReturns struct { + result1 bool + result2 error + } + txIDExistsReturnsOnCall map[int]struct { + result1 bool + result2 error + } invocations map[string][][]interface{} invocationsMutex sync.RWMutex } @@ -1649,6 +1662,69 @@ func (fake *PeerLedger) SubmitSnapshotRequestReturnsOnCall(i int, result1 error) }{result1} } +func (fake *PeerLedger) TxIDExists(arg1 string) (bool, error) { + fake.txIDExistsMutex.Lock() + ret, specificReturn := fake.txIDExistsReturnsOnCall[len(fake.txIDExistsArgsForCall)] + fake.txIDExistsArgsForCall = append(fake.txIDExistsArgsForCall, struct { + arg1 string + }{arg1}) + fake.recordInvocation("TxIDExists", []interface{}{arg1}) + fake.txIDExistsMutex.Unlock() + if fake.TxIDExistsStub != nil { + return fake.TxIDExistsStub(arg1) + } + if specificReturn { + return ret.result1, ret.result2 + } + fakeReturns := fake.txIDExistsReturns + return fakeReturns.result1, fakeReturns.result2 +} + +func (fake *PeerLedger) TxIDExistsCallCount() int { + fake.txIDExistsMutex.RLock() + defer fake.txIDExistsMutex.RUnlock() + return len(fake.txIDExistsArgsForCall) +} + +func (fake *PeerLedger) TxIDExistsCalls(stub func(string) (bool, error)) { + fake.txIDExistsMutex.Lock() + defer fake.txIDExistsMutex.Unlock() + fake.TxIDExistsStub = stub +} + +func (fake *PeerLedger) TxIDExistsArgsForCall(i int) string { + fake.txIDExistsMutex.RLock() + defer fake.txIDExistsMutex.RUnlock() + argsForCall := fake.txIDExistsArgsForCall[i] + return argsForCall.arg1 +} + +func (fake *PeerLedger) TxIDExistsReturns(result1 bool, result2 error) { + fake.txIDExistsMutex.Lock() + defer fake.txIDExistsMutex.Unlock() + fake.TxIDExistsStub = nil + fake.txIDExistsReturns = struct { + result1 bool + result2 error + }{result1, result2} +} + +func (fake *PeerLedger) TxIDExistsReturnsOnCall(i int, result1 bool, result2 error) { + fake.txIDExistsMutex.Lock() + defer fake.txIDExistsMutex.Unlock() + fake.TxIDExistsStub = nil + if fake.txIDExistsReturnsOnCall == nil { + fake.txIDExistsReturnsOnCall = make(map[int]struct { + result1 bool + result2 error + }) + } + fake.txIDExistsReturnsOnCall[i] = struct { + result1 bool + result2 error + }{result1, result2} +} + func (fake *PeerLedger) Invocations() map[string][][]interface{} { fake.invocationsMutex.RLock() defer fake.invocationsMutex.RUnlock() @@ -1698,6 +1774,8 @@ func (fake *PeerLedger) Invocations() map[string][][]interface{} { defer fake.pendingSnapshotRequestsMutex.RUnlock() fake.submitSnapshotRequestMutex.RLock() defer fake.submitSnapshotRequestMutex.RUnlock() + fake.txIDExistsMutex.RLock() + defer fake.txIDExistsMutex.RUnlock() copiedInvocations := map[string][][]interface{}{} for key, value := range fake.invocations { copiedInvocations[key] = value