diff --git a/core/ledger/kvledger/history/db_test.go b/core/ledger/kvledger/history/db_test.go index 9d13aaa7c39..3da78061bc7 100644 --- a/core/ledger/kvledger/history/db_test.go +++ b/core/ledger/kvledger/history/db_test.go @@ -13,8 +13,11 @@ import ( "strconv" "testing" + "github.com/golang/protobuf/proto" "github.com/hyperledger/fabric-protos-go/common" "github.com/hyperledger/fabric-protos-go/ledger/queryresult" + "github.com/hyperledger/fabric-protos-go/ledger/rwset" + "github.com/hyperledger/fabric-protos-go/ledger/rwset/kvrwset" "github.com/hyperledger/fabric-protos-go/peer" configtxtest "github.com/hyperledger/fabric/common/configtx/test" "github.com/hyperledger/fabric/common/flogging" @@ -409,6 +412,62 @@ func TestName(t *testing.T) { assert.Equal(t, "history", env.testHistoryDB.Name()) } +// TestHistoryWithKVWriteOfNilValue - See FAB-18386 for details +func TestHistoryWithKVWriteOfNilValue(t *testing.T) { + env := newTestHistoryEnv(t) + defer env.cleanup() + provider := env.testBlockStorageEnv.provider + store, err := provider.Open("ledger1") + require.NoError(t, err) + defer store.Shutdown() + + bg, gb := testutil.NewBlockGenerator(t, "ledger1", false) + + kvRWSet := &kvrwset.KVRWSet{ + Writes: []*kvrwset.KVWrite{ + // explicitly set IsDelete to false while the value to nil. As this will never be generated by simulation + {Key: "key1", IsDelete: false, Value: nil}, + }, + } + kvRWsetBytes, err := proto.Marshal(kvRWSet) + require.NoError(t, err) + + txRWSet := &rwset.TxReadWriteSet{ + NsRwset: []*rwset.NsReadWriteSet{ + { + Namespace: "ns1", + Rwset: kvRWsetBytes, + }, + }, + } + + txRWSetBytes, err := proto.Marshal(txRWSet) + require.NoError(t, err) + + block1 := bg.NextBlockWithTxid([][]byte{txRWSetBytes}, []string{"txid1"}) + + historydb, err := env.testHistoryDBProvider.GetDBHandle("ledger1") + require.NoError(t, err) + require.NoError(t, store.AddBlock(gb)) + require.NoError(t, historydb.Commit(gb)) + require.NoError(t, store.AddBlock(block1)) + require.NoError(t, historydb.Commit(block1)) + + historydbQE, err := historydb.NewQueryExecutor(store) + require.NoError(t, err) + itr, err := historydbQE.GetHistoryForKey("ns1", "key1") + require.NoError(t, err) + kmod, err := itr.Next() + require.NoError(t, err) + keyModification := kmod.(*queryresult.KeyModification) + // despite IsDelete set to "false" in the write-set, historydb results should set this to "true" + require.True(t, keyModification.IsDelete) + + kmod, err = itr.Next() + require.NoError(t, err) + require.Nil(t, kmod) +} + // verify history results func testutilVerifyResults(t *testing.T, hqe ledger.HistoryQueryExecutor, ns, key string, expectedVals []string) { itr, err := hqe.GetHistoryForKey(ns, key) diff --git a/core/ledger/kvledger/history/query_executer.go b/core/ledger/kvledger/history/query_executer.go index 903141ea766..2a348a8574a 100644 --- a/core/ledger/kvledger/history/query_executer.go +++ b/core/ledger/kvledger/history/query_executer.go @@ -135,7 +135,7 @@ func getKeyModificationFromTran(tranEnvelope *common.Envelope, namespace string, for _, kvWrite := range nsRWSet.KvRwSet.Writes { if kvWrite.Key == key { return &queryresult.KeyModification{TxId: txID, Value: kvWrite.Value, - Timestamp: timestamp, IsDelete: kvWrite.IsDelete}, nil + Timestamp: timestamp, IsDelete: rwsetutil.IsKVWriteDelete(kvWrite)}, nil } } // end keys loop logger.Debugf("key [%s] not found in namespace [%s]'s writeset", key, namespace) diff --git a/core/ledger/kvledger/tests/client.go b/core/ledger/kvledger/tests/client.go index 224039cd8da..8f0326dacfd 100644 --- a/core/ledger/kvledger/tests/client.go +++ b/core/ledger/kvledger/tests/client.go @@ -51,6 +51,10 @@ func (c *client) simulateDataTx(txid string, simulationLogic func(s *simulator)) return txAndPvtdata } +func (c *client) submitHandCraftedTx(txAndPvtdata *txAndPvtdata) { + c.simulatedTrans = append(c.simulatedTrans, txAndPvtdata) +} + func (c *client) addPostOrderTx(txid string, customTxType common.HeaderType) *txAndPvtdata { if txid == "" { txid = util.GenerateUUID() diff --git a/core/ledger/kvledger/tests/nilvalue_no_delete_marker_test.go b/core/ledger/kvledger/tests/nilvalue_no_delete_marker_test.go new file mode 100644 index 00000000000..3082a0e662a --- /dev/null +++ b/core/ledger/kvledger/tests/nilvalue_no_delete_marker_test.go @@ -0,0 +1,148 @@ +/* +Copyright IBM Corp. All Rights Reserved. + +SPDX-License-Identifier: Apache-2.0 +*/ + +package tests + +import ( + "testing" + + "github.com/golang/protobuf/proto" + "github.com/hyperledger/fabric-protos-go/ledger/rwset" + "github.com/hyperledger/fabric-protos-go/ledger/rwset/kvrwset" + "github.com/hyperledger/fabric-protos-go/peer" + "github.com/hyperledger/fabric/common/util" + "github.com/stretchr/testify/require" +) + +// TestNilValNoDeleteMarker tests for a special writeset which carries a nil value and yet the delete marker is set to false. +// This kind of write-set gets produced in previous versions. See FAB-18386 for more details. +func TestNilValNoDeleteMarker(t *testing.T) { + env := newEnv(t) + defer env.cleanup() + env.initLedgerMgmt() + + testLedger := env.newTestHelperCreateLgr("test-ledger", t) + testLedger.simulateDeployTx("cc1", []*collConf{ + { + name: "coll1", + }, + }) + testLedger.cutBlockAndCommitLegacy() + + testLedger.simulateDataTx("txid1", func(s *simulator) { + s.setState("cc1", "pubKey1", "pubValue1") + s.setPvtdata("cc1", "coll1", "pvtKey1", "pvtValue1") + s.setPvtdata("cc1", "coll1", "pvtKey2", "pvtValue2") + }) + testLedger.cutBlockAndCommitLegacy() + + testLedger.verifyPubState("cc1", "pubKey1", "pubValue1") + testLedger.verifyPvtdataHashState("cc1", "coll1", "pvtKey1", util.ComputeSHA256([]byte("pvtValue1"))) + testLedger.verifyPvtdataHashState("cc1", "coll1", "pvtKey2", util.ComputeSHA256([]byte("pvtValue2"))) + testLedger.verifyPvtState("cc1", "coll1", "pvtKey1", "pvtValue1") + testLedger.verifyPvtState("cc1", "coll1", "pvtKey2", "pvtValue2") + + // Handcraft writeset that includes a delete for each of the above keys. We handcraft here because the one generated by the simulator + // will always have IsDelete flag true and we want to test when this flag is set to false and the actual value is nil. See FAB-18386 for + // more details. + pubWrites := &kvrwset.KVRWSet{ + Writes: []*kvrwset.KVWrite{ + { + Key: "pubKey1", + IsDelete: false, + Value: nil, + }, + }, + } + + hashedWrites := &kvrwset.HashedRWSet{ + HashedWrites: []*kvrwset.KVWriteHash{ + { + KeyHash: util.ComputeSHA256([]byte("pvtKey1")), + IsDelete: false, + ValueHash: nil, + }, + { + KeyHash: util.ComputeSHA256([]byte("pvtKey2")), + IsDelete: false, + ValueHash: util.ComputeSHA256([]byte{}), + }, + }, + } + + pvtWrites := &kvrwset.KVRWSet{ + Writes: []*kvrwset.KVWrite{ + { + Key: "pvtKey1", + IsDelete: false, + }, + { + Key: "pvtKey2", + IsDelete: false, + }, + }, + } + + pubWritesBytes, err := proto.Marshal(pubWrites) + require.NoError(t, err) + + hashedWritesBytes, err := proto.Marshal(hashedWrites) + require.NoError(t, err) + + pvtWritesBytes, err := proto.Marshal(pvtWrites) + require.NoError(t, err) + + pubRwset := &rwset.TxReadWriteSet{ + DataModel: rwset.TxReadWriteSet_KV, + NsRwset: []*rwset.NsReadWriteSet{ + { + Namespace: "cc1", + Rwset: pubWritesBytes, + CollectionHashedRwset: []*rwset.CollectionHashedReadWriteSet{ + { + CollectionName: "coll1", + HashedRwset: hashedWritesBytes, + PvtRwsetHash: util.ComputeSHA256(pvtWritesBytes), + }, + }, + }, + }, + } + pubRwsetBytes, err := proto.Marshal(pubRwset) + require.NoError(t, err) + envelope, err := constructTransaction("txid2", pubRwsetBytes) + require.NoError(t, err) + + txAndPvtdata := &txAndPvtdata{ + Txid: "txid2", + Envelope: envelope, + Pvtws: &rwset.TxPvtReadWriteSet{ + DataModel: rwset.TxReadWriteSet_KV, + NsPvtRwset: []*rwset.NsPvtReadWriteSet{ + { + Namespace: "cc1", + CollectionPvtRwset: []*rwset.CollectionPvtReadWriteSet{ + { + CollectionName: "coll1", + Rwset: pvtWritesBytes, + }, + }, + }, + }, + }, + } + + testLedger.submitHandCraftedTx(txAndPvtdata) + testLedger.cutBlockAndCommitLegacy() + + testLedger.verifyTxValidationCode("txid2", peer.TxValidationCode_VALID) + testLedger.verifyPubState("cc1", "pubKey1", "") + testLedger.verifyPvtdataHashState("cc1", "coll1", "pvtKey1", nil) + testLedger.verifyPvtdataHashState("cc1", "coll1", "pvtKey2", nil) + testLedger.verifyPvtState("cc1", "coll1", "pvtKey1", "") + testLedger.verifyPvtState("cc1", "coll1", "pvtKey2", "") + testLedger.verifyHistory("cc1", "pubKey1", []string{"", "pubValue1"}) +} diff --git a/core/ledger/kvledger/tests/verifier.go b/core/ledger/kvledger/tests/verifier.go index eb7d4cf6e4a..4e734849be6 100644 --- a/core/ledger/kvledger/tests/verifier.go +++ b/core/ledger/kvledger/tests/verifier.go @@ -53,6 +53,15 @@ func (v *verifier) verifyPubState(ns, key string, expectedVal string) { v.assert.Equal(expectedValBytes, committedVal) } +func (v *verifier) verifyPvtdataHashState(ns, coll, key string, expectedValHash []byte) { + qe, err := v.lgr.NewQueryExecutor() + v.assert.NoError(err) + defer qe.Done() + committedValHash, err := qe.GetPrivateDataHash(ns, coll, key) + v.assert.NoError(err) + v.assert.Equal(expectedValHash, committedValHash) +} + func (v *verifier) verifyPvtState(ns, coll, key string, expectedVal string) { qe, err := v.lgr.NewQueryExecutor() v.assert.NoError(err) diff --git a/core/ledger/kvledger/txmgmt/rwsetutil/rwset_builder_test.go b/core/ledger/kvledger/txmgmt/rwsetutil/rwset_builder_test.go index 54f7b683ec2..8ac47c64b8c 100644 --- a/core/ledger/kvledger/txmgmt/rwsetutil/rwset_builder_test.go +++ b/core/ledger/kvledger/txmgmt/rwsetutil/rwset_builder_test.go @@ -357,3 +357,71 @@ func serializeTestProtoMsg(t *testing.T, protoMsg proto.Message) []byte { require.NoError(t, err) return msgBytes } + +func TestNilOrZeroLengthByteArrayValueConvertedToDelete(t *testing.T) { + t.Run("public_writeset", func(t *testing.T) { + rwsetBuilder := NewRWSetBuilder() + rwsetBuilder.AddToWriteSet("ns", "key1", nil) + rwsetBuilder.AddToWriteSet("ns", "key2", []byte{}) + + simulationResults, err := rwsetBuilder.GetTxSimulationResults() + require.NoError(t, err) + pubRWSet := &kvrwset.KVRWSet{} + require.NoError( + t, + proto.Unmarshal(simulationResults.PubSimulationResults.NsRwset[0].Rwset, pubRWSet), + ) + require.True(t, proto.Equal( + &kvrwset.KVRWSet{ + Writes: []*kvrwset.KVWrite{ + {Key: "key1", IsDelete: true}, + {Key: "key2", IsDelete: true}, + }, + }, + pubRWSet, + )) + }) + + t.Run("pvtdata_and_hashes_writesets", func(t *testing.T) { + rwsetBuilder := NewRWSetBuilder() + rwsetBuilder.AddToPvtAndHashedWriteSet("ns", "coll", "key1", nil) + rwsetBuilder.AddToPvtAndHashedWriteSet("ns", "coll", "key2", []byte{}) + + simulationResults, err := rwsetBuilder.GetTxSimulationResults() + require.NoError(t, err) + + t.Run("hashed_writeset", func(t *testing.T) { + hashedRWSet := &kvrwset.HashedRWSet{} + require.NoError( + t, + proto.Unmarshal(simulationResults.PubSimulationResults.NsRwset[0].CollectionHashedRwset[0].HashedRwset, hashedRWSet), + ) + require.True(t, proto.Equal( + &kvrwset.HashedRWSet{ + HashedWrites: []*kvrwset.KVWriteHash{ + {KeyHash: util.ComputeStringHash("key1"), IsDelete: true}, + {KeyHash: util.ComputeStringHash("key2"), IsDelete: true}, + }, + }, + hashedRWSet, + )) + }) + + t.Run("pvtdata_writeset", func(t *testing.T) { + pvtWSet := &kvrwset.KVRWSet{} + require.NoError( + t, + proto.Unmarshal(simulationResults.PvtSimulationResults.NsPvtRwset[0].CollectionPvtRwset[0].Rwset, pvtWSet), + ) + require.True(t, proto.Equal( + &kvrwset.KVRWSet{ + Writes: []*kvrwset.KVWrite{ + {Key: "key1", IsDelete: true}, + {Key: "key2", IsDelete: true}, + }, + }, + pvtWSet, + )) + }) + }) +} diff --git a/core/ledger/kvledger/txmgmt/rwsetutil/rwset_proto_util.go b/core/ledger/kvledger/txmgmt/rwsetutil/rwset_proto_util.go index 7021ea39979..49d561b4ea1 100644 --- a/core/ledger/kvledger/txmgmt/rwsetutil/rwset_proto_util.go +++ b/core/ledger/kvledger/txmgmt/rwsetutil/rwset_proto_util.go @@ -17,6 +17,8 @@ limitations under the License. package rwsetutil import ( + "bytes" + "github.com/golang/protobuf/proto" "github.com/hyperledger/fabric-protos-go/ledger/rwset" "github.com/hyperledger/fabric-protos-go/ledger/rwset/kvrwset" @@ -339,7 +341,7 @@ func newProtoVersion(height *version.Height) *kvrwset.Version { } func newKVWrite(key string, value []byte) *kvrwset.KVWrite { - return &kvrwset.KVWrite{Key: key, IsDelete: value == nil, Value: value} + return &kvrwset.KVWrite{Key: key, IsDelete: len(value) == 0, Value: value} } func newPvtKVReadHash(key string, version *version.Height) *kvrwset.KVReadHash { @@ -355,3 +357,17 @@ func newPvtKVWriteAndHash(key string, value []byte) (*kvrwset.KVWrite, *kvrwset. } return kvWrite, &kvrwset.KVWriteHash{KeyHash: keyHash, IsDelete: kvWrite.IsDelete, ValueHash: valueHash} } + +// IsKVWriteDelete returns true if the kvWrite indicates a delete operation. See FAB-18386 for details. +func IsKVWriteDelete(kvWrite *kvrwset.KVWrite) bool { + return kvWrite.IsDelete || len(kvWrite.Value) == 0 +} + +var ( + hashOfZeroLengthByteArray = util.ComputeHash([]byte{}) +) + +// IsKVWriteHashDelete returns true if the kvWriteHash indicates a delete operation. See FAB-18386 for details. +func IsKVWriteHashDelete(kvWriteHash *kvrwset.KVWriteHash) bool { + return kvWriteHash.IsDelete || len(kvWriteHash.ValueHash) == 0 || bytes.Equal(hashOfZeroLengthByteArray, kvWriteHash.ValueHash) +} diff --git a/core/ledger/kvledger/txmgmt/rwsetutil/rwset_proto_util_test.go b/core/ledger/kvledger/txmgmt/rwsetutil/rwset_proto_util_test.go index 7b29daec6a7..dfee74374ae 100644 --- a/core/ledger/kvledger/txmgmt/rwsetutil/rwset_proto_util_test.go +++ b/core/ledger/kvledger/txmgmt/rwsetutil/rwset_proto_util_test.go @@ -23,6 +23,7 @@ import ( "github.com/golang/protobuf/proto" "github.com/hyperledger/fabric-protos-go/ledger/rwset/kvrwset" "github.com/hyperledger/fabric/core/ledger/internal/version" + "github.com/hyperledger/fabric/core/ledger/util" "github.com/kr/pretty" "github.com/stretchr/testify/require" ) @@ -241,3 +242,33 @@ func TestVersionConversion(t *testing.T) { require.Nil(t, newProtoVersion(nil)) require.Equal(t, protoVer, newProtoVersion(internalVer)) } + +func TestIsDelete(t *testing.T) { + t.Run("kvWrite", func(t *testing.T) { + kvWritesToBeInterpretedAsDelete := []*kvrwset.KVWrite{ + {Value: nil, IsDelete: true}, + {Value: nil, IsDelete: false}, + {Value: []byte{}, IsDelete: true}, + {Value: []byte{}, IsDelete: false}, + } + + for _, k := range kvWritesToBeInterpretedAsDelete { + require.True(t, IsKVWriteDelete(k)) + } + }) + + t.Run("kvhashwrite", func(t *testing.T) { + kvHashesWritesToBeInterpretedAsDelete := []*kvrwset.KVWriteHash{ + {ValueHash: nil, IsDelete: true}, + {ValueHash: nil, IsDelete: false}, + {ValueHash: []byte{}, IsDelete: true}, + {ValueHash: []byte{}, IsDelete: false}, + {ValueHash: util.ComputeHash([]byte{}), IsDelete: true}, + {ValueHash: util.ComputeHash([]byte{}), IsDelete: false}, + } + + for _, k := range kvHashesWritesToBeInterpretedAsDelete { + require.True(t, IsKVWriteHashDelete(k)) + } + }) +} diff --git a/core/ledger/kvledger/txmgmt/txmgr/lockbased_txmgr.go b/core/ledger/kvledger/txmgmt/txmgr/lockbased_txmgr.go index ef88374a706..10e6853287b 100644 --- a/core/ledger/kvledger/txmgmt/txmgr/lockbased_txmgr.go +++ b/core/ledger/kvledger/txmgmt/txmgr/lockbased_txmgr.go @@ -347,7 +347,7 @@ func (uniquePvtData uniquePvtDataMap) updateUsingPvtWrite(pvtWrite *kvrwset.KVWr uniquePvtData[hashedCompositeKey] = &privacyenabledstate.PvtKVWrite{ Key: pvtWrite.Key, - IsDelete: pvtWrite.IsDelete, + IsDelete: rwsetutil.IsKVWriteDelete(pvtWrite), Value: pvtWrite.Value, Version: ver, } diff --git a/core/ledger/kvledger/txmgmt/txmgr/txmgr_test.go b/core/ledger/kvledger/txmgmt/txmgr/txmgr_test.go index 44884712e50..29254ed8958 100644 --- a/core/ledger/kvledger/txmgmt/txmgr/txmgr_test.go +++ b/core/ledger/kvledger/txmgmt/txmgr/txmgr_test.go @@ -14,7 +14,10 @@ import ( "strings" "testing" + "github.com/golang/protobuf/proto" "github.com/hyperledger/fabric-protos-go/ledger/queryresult" + "github.com/hyperledger/fabric-protos-go/ledger/rwset" + "github.com/hyperledger/fabric-protos-go/ledger/rwset/kvrwset" "github.com/hyperledger/fabric/common/ledger/testutil" "github.com/hyperledger/fabric/core/ledger" "github.com/hyperledger/fabric/core/ledger/internal/version" @@ -931,6 +934,31 @@ func TestConstructUniquePvtData(t *testing.T) { // ns1-coll1-key1 should be accepted pvtDataBlk3Tx1 := producePvtdata(t, 1, []string{"ns1:coll1"}, []string{"key1"}, [][]byte{v3}) + pvtDataBlk3Tx2WriteSetBytes, err := proto.Marshal( + &kvrwset.KVRWSet{ + Writes: []*kvrwset.KVWrite{ + {Key: "key5", IsDelete: false, Value: nil}, + }, + }, + ) + require.NoError(t, err) + pvtDataBlk3Tx2 := &ledger.TxPvtData{ + SeqInBlock: 2, + WriteSet: &rwset.TxPvtReadWriteSet{ + NsPvtRwset: []*rwset.NsPvtReadWriteSet{ + { + Namespace: "ns1", + CollectionPvtRwset: []*rwset.CollectionPvtReadWriteSet{ + { + CollectionName: "coll1", + Rwset: pvtDataBlk3Tx2WriteSetBytes, + }, + }, + }, + }, + }, + } + blocksPvtData := map[uint64][]*ledger.TxPvtData{ 1: { pvtDataBlk1Tx1, @@ -943,6 +971,7 @@ func TestConstructUniquePvtData(t *testing.T) { }, 3: { pvtDataBlk3Tx1, + pvtDataBlk3Tx2, }, } @@ -958,11 +987,15 @@ func TestConstructUniquePvtData(t *testing.T) { hashedCompositeKeyNs1Coll1Key1 := privacyenabledstate.HashedCompositeKey{Namespace: "ns1", CollectionName: "coll1", KeyHash: string(util.ComputeStringHash("key1"))} pvtKVWriteNs1Coll1Key1 := &privacyenabledstate.PvtKVWrite{Key: "key1", IsDelete: false, Value: v3, Version: version.NewHeight(3, 1)} + hashedCompositeKeyNs1Coll1Key5 := privacyenabledstate.HashedCompositeKey{Namespace: "ns1", CollectionName: "coll1", KeyHash: string(util.ComputeStringHash("key5"))} + pvtKVWriteNs1Coll1Key5 := &privacyenabledstate.PvtKVWrite{Key: "key5", IsDelete: true, Value: nil, Version: version.NewHeight(3, 2)} + expectedUniquePvtData := uniquePvtDataMap{ hashedCompositeKeyNs1Coll2Key3: pvtKVWriteNs1Coll2Key3, hashedCompositeKeyNs1Coll2Key4: pvtKVWriteNs1Coll2Key4, hashedCompositeKeyNs1Coll1Key2: pvtKVWriteNs1Coll1Key2, hashedCompositeKeyNs1Coll1Key1: pvtKVWriteNs1Coll1Key1, + hashedCompositeKeyNs1Coll1Key5: pvtKVWriteNs1Coll1Key5, } uniquePvtData, err := constructUniquePvtData(blocksPvtData) diff --git a/core/ledger/kvledger/txmgmt/validation/batch_preparer.go b/core/ledger/kvledger/txmgmt/validation/batch_preparer.go index 84f041fabc2..72b43919ef1 100644 --- a/core/ledger/kvledger/txmgmt/validation/batch_preparer.go +++ b/core/ledger/kvledger/txmgmt/validation/batch_preparer.go @@ -342,7 +342,7 @@ func addPvtRWSetToPvtUpdateBatch(pvtRWSet *rwsetutil.TxPvtRwSet, pvtUpdateBatch for _, ns := range pvtRWSet.NsPvtRwSet { for _, coll := range ns.CollPvtRwSets { for _, kvwrite := range coll.KvRwSet.Writes { - if !kvwrite.IsDelete { + if !rwsetutil.IsKVWriteDelete(kvwrite) { pvtUpdateBatch.Put(ns.NameSpace, coll.CollectionName, kvwrite.Key, kvwrite.Value, ver) } else { pvtUpdateBatch.Delete(ns.NameSpace, coll.CollectionName, kvwrite.Key, ver) diff --git a/core/ledger/kvledger/txmgmt/validation/tx_ops.go b/core/ledger/kvledger/txmgmt/validation/tx_ops.go index f4ccda53319..4ece6604377 100644 --- a/core/ledger/kvledger/txmgmt/validation/tx_ops.go +++ b/core/ledger/kvledger/txmgmt/validation/tx_ops.go @@ -76,7 +76,7 @@ func (txops txOps) applyTxRwset(rwset *rwsetutil.TxRwSet) error { &kvrwset.KVWrite{ Key: string(hashedWrite.KeyHash), Value: hashedWrite.ValueHash, - IsDelete: hashedWrite.IsDelete, + IsDelete: rwsetutil.IsKVWriteHashDelete(hashedWrite), }, ) } @@ -98,7 +98,7 @@ func (txops txOps) applyTxRwset(rwset *rwsetutil.TxRwSet) error { // applyKVWrite records upsertion/deletion of a kvwrite func (txops txOps) applyKVWrite(ns, coll string, kvWrite *kvrwset.KVWrite) { - if kvWrite.IsDelete { + if rwsetutil.IsKVWriteDelete(kvWrite) { txops.delete(compositeKey{ns, coll, kvWrite.Key}) } else { txops.upsert(compositeKey{ns, coll, kvWrite.Key}, kvWrite.Value) diff --git a/core/ledger/kvledger/txmgmt/validation/tx_ops_test.go b/core/ledger/kvledger/txmgmt/validation/tx_ops_test.go index 9e87284b806..55e1931f46e 100644 --- a/core/ledger/kvledger/txmgmt/validation/tx_ops_test.go +++ b/core/ledger/kvledger/txmgmt/validation/tx_ops_test.go @@ -368,6 +368,96 @@ func TestTxOpsPreparationPvtdataHashes(t *testing.T) { require.Equal(t, ck4ExpectedKeyOps, txOps[ck4Hash]) } +// TestInterpretNilValueKVWritesAsDelete - See FAB-18386 +func TestInterpretNilValueKVWritesAsDelete(t *testing.T) { + testcases := []struct { + name string + rwset *rwsetutil.TxRwSet + compositeKeysToVerify []compositeKey + }{ + { + name: "public_keys_writes", + rwset: &rwsetutil.TxRwSet{ + NsRwSets: []*rwsetutil.NsRwSet{ + { + NameSpace: "ns1", + KvRwSet: &kvrwset.KVRWSet{ + Writes: []*kvrwset.KVWrite{ + { + Key: "key1", + IsDelete: true, + }, + { + Key: "key2", + IsDelete: false, + Value: []byte{}, + }, + }, + }, + }, + }, + }, + compositeKeysToVerify: []compositeKey{ + {ns: "ns1", key: "key1"}, + {ns: "ns1", key: "key2"}, + }, + }, + { + name: "private_keys_hashes_writes", + rwset: &rwsetutil.TxRwSet{ + NsRwSets: []*rwsetutil.NsRwSet{ + { + NameSpace: "ns1", + KvRwSet: &kvrwset.KVRWSet{}, + CollHashedRwSets: []*rwsetutil.CollHashedRwSet{ + { + CollectionName: "coll1", + HashedRwSet: &kvrwset.HashedRWSet{ + HashedWrites: []*kvrwset.KVWriteHash{ + { + KeyHash: util.ComputeStringHash("key1"), + IsDelete: true, + }, + { + KeyHash: util.ComputeStringHash("key2"), + IsDelete: false, + }, + { + KeyHash: util.ComputeStringHash("key3"), + IsDelete: false, + ValueHash: util.ComputeHash([]byte{}), + }, + }, + }, + }, + }, + }, + }, + }, + compositeKeysToVerify: []compositeKey{ + {ns: "ns1", coll: "coll1", key: string(util.ComputeStringHash("key1"))}, + {ns: "ns1", coll: "coll1", key: string(util.ComputeStringHash("key2"))}, + {ns: "ns1", coll: "coll1", key: string(util.ComputeStringHash("key3"))}, + }, + }, + } + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + txOps := txOps{} + err := txOps.applyTxRwset(tc.rwset) + require.NoError(t, err) + + for _, keyToVerify := range tc.compositeKeysToVerify { + require.Equal(t, + &keyOps{flag: keyDelete}, + txOps[keyToVerify], + ) + } + }) + } +} + func testutilBuildRwset(t *testing.T, kvWrites map[compositeKey][]byte, metadataWrites map[compositeKey]map[string][]byte) *rwsetutil.TxRwSet { diff --git a/core/scc/lscc/deployedcc_infoprovider.go b/core/scc/lscc/deployedcc_infoprovider.go index 144aceaa850..8970539bfc4 100644 --- a/core/scc/lscc/deployedcc_infoprovider.go +++ b/core/scc/lscc/deployedcc_infoprovider.go @@ -14,6 +14,7 @@ import ( "github.com/hyperledger/fabric/core/common/ccprovider" "github.com/hyperledger/fabric/core/common/privdata" "github.com/hyperledger/fabric/core/ledger" + "github.com/hyperledger/fabric/core/ledger/kvledger/txmgmt/rwsetutil" "github.com/pkg/errors" ) @@ -37,7 +38,7 @@ func (p *DeployedCCInfoProvider) UpdatedChaincodes(stateUpdates map[string][]*kv updatedCCNames := map[string]bool{} for _, kvWrite := range lsccUpdates { - if kvWrite.IsDelete { + if rwsetutil.IsKVWriteDelete(kvWrite) { // lscc namespace is not expected to have deletes continue }