Fix for nil pointer error during phantom validation

https://jira.hyperledger.org/browse/FAB-2618

- Added a nil check
- Added a test case for this situation

Change-Id: I245c68b7c1e732bd1dd694e57438232a0f4e750c
Signed-off-by: manish <[email protected]>

Author: manish-sethi

core/ledger/kvledger/txmgmt/validator/statebasedval/combined_iterator.go

@@ -16,8 +16,11 @@ limitations under the License.
 
 package statebasedval
 
-import "github.com/hyperledger/fabric/core/ledger/kvledger/txmgmt/statedb"
-import "strings"
+import (
+	"strings"
+
+	"github.com/hyperledger/fabric/core/ledger/kvledger/txmgmt/statedb"
+)
 
 // combinedIterator implements the interface statedb.ResultsIterator.
 // Internally, it maintains two iterators
@@ -131,15 +134,21 @@ func (itr *combinedIterator) serveEndKeyIfNeeded() (statedb.QueryResult, error)
 	var vv *statedb.VersionedValue
 	var err error
 	vv = itr.updates.Get(itr.ns, itr.endKey)
+	logger.Debugf("endKey value from updates:%s", vv)
 	if vv == nil {
 		if vv, err = itr.db.GetState(itr.ns, itr.endKey); err != nil {
 			return nil, err
 		}
+		logger.Debugf("endKey value from stateDB:%s", vv)
+	}
+	itr.endKeyServed = true
+	if vv == nil {
+		return nil, nil
 	}
 	vkv := &statedb.VersionedKV{
 		CompositeKey:   statedb.CompositeKey{Namespace: itr.ns, Key: itr.endKey},
 		VersionedValue: statedb.VersionedValue{Value: vv.Value, Version: vv.Version}}
-	itr.endKeyServed = true
+
 	if isDelete(vkv) {
 		return nil, nil
 	}

core/ledger/kvledger/txmgmt/validator/statebasedval/combined_iterator_test.go

@@ -50,55 +50,70 @@ func TestCombinedIterator(t *testing.T) {
 	// prepare batch2 (empty)
 	batch2 := statedb.NewUpdateBatch()
 
-	// Test db + batch1 updates
+	// Test db + batch1 updates (exclude endKey)
 	itr1, _ := newCombinedIterator(db, batch1, "ns", "key2", "key8", false)
 	defer itr1.Close()
-	checkItrResults(t, itr1, []*statedb.VersionedKV{
+	checkItrResults(t, "ExcludeEndKey", itr1, []*statedb.VersionedKV{
 		constructVersionedKV("ns", "key3", []byte("value3"), version.NewHeight(1, 1)),
 		constructVersionedKV("ns", "key4", []byte("value4"), version.NewHeight(1, 1)),
 		constructVersionedKV("ns", "key6", []byte("value6_new"), version.NewHeight(1, 1)),
 		constructVersionedKV("ns", "key7", []byte("value7"), version.NewHeight(1, 1)),
 	})
 
+	// Test db + batch1 updates (include endKey)
 	itr1WithEndKey, _ := newCombinedIterator(db, batch1, "ns", "key2", "key8", true)
 	defer itr1WithEndKey.Close()
-	checkItrResults(t, itr1WithEndKey, []*statedb.VersionedKV{
+	checkItrResults(t, "IncludeEndKey", itr1WithEndKey, []*statedb.VersionedKV{
 		constructVersionedKV("ns", "key3", []byte("value3"), version.NewHeight(1, 1)),
 		constructVersionedKV("ns", "key4", []byte("value4"), version.NewHeight(1, 1)),
 		constructVersionedKV("ns", "key6", []byte("value6_new"), version.NewHeight(1, 1)),
 		constructVersionedKV("ns", "key7", []byte("value7"), version.NewHeight(1, 1)),
 		constructVersionedKV("ns", "key8", []byte("value8"), version.NewHeight(1, 1)),
 	})
 
-	// Test db + batch2 updates
-	itr2, _ := newCombinedIterator(db, batch2, "ns", "key2", "key8", false)
-	defer itr2.Close()
-	checkItrResults(t, itr2, []*statedb.VersionedKV{
+	// Test db + batch1 updates (include endKey) for extra range
+	itr1WithEndKeyExtraRange, _ := newCombinedIterator(db, batch1, "ns", "key0", "key9", true)
+	defer itr1WithEndKeyExtraRange.Close()
+	checkItrResults(t, "IncludeEndKey_ExtraRange", itr1WithEndKeyExtraRange, []*statedb.VersionedKV{
+		constructVersionedKV("ns", "key1", []byte("value1"), version.NewHeight(1, 1)),
+		constructVersionedKV("ns", "key3", []byte("value3"), version.NewHeight(1, 1)),
 		constructVersionedKV("ns", "key4", []byte("value4"), version.NewHeight(1, 1)),
-		constructVersionedKV("ns", "key6", []byte("value6"), version.NewHeight(1, 1)),
+		constructVersionedKV("ns", "key6", []byte("value6_new"), version.NewHeight(1, 1)),
+		constructVersionedKV("ns", "key7", []byte("value7"), version.NewHeight(1, 1)),
+		constructVersionedKV("ns", "key8", []byte("value8"), version.NewHeight(1, 1)),
 	})
 
 	// Test db + batch1 updates with full range query
 	itr3, _ := newCombinedIterator(db, batch1, "ns", "", "", false)
 	defer itr3.Close()
-	checkItrResults(t, itr3, []*statedb.VersionedKV{
+	checkItrResults(t, "ExcludeEndKey_FullRange", itr3, []*statedb.VersionedKV{
 		constructVersionedKV("ns", "key1", []byte("value1"), version.NewHeight(1, 1)),
 		constructVersionedKV("ns", "key3", []byte("value3"), version.NewHeight(1, 1)),
 		constructVersionedKV("ns", "key4", []byte("value4"), version.NewHeight(1, 1)),
 		constructVersionedKV("ns", "key6", []byte("value6_new"), version.NewHeight(1, 1)),
 		constructVersionedKV("ns", "key7", []byte("value7"), version.NewHeight(1, 1)),
 		constructVersionedKV("ns", "key8", []byte("value8"), version.NewHeight(1, 1)),
 	})
+
+	// Test db + batch2 updates
+	itr2, _ := newCombinedIterator(db, batch2, "ns", "key2", "key8", false)
+	defer itr2.Close()
+	checkItrResults(t, "ExcludeEndKey_EmptyUpdates", itr2, []*statedb.VersionedKV{
+		constructVersionedKV("ns", "key4", []byte("value4"), version.NewHeight(1, 1)),
+		constructVersionedKV("ns", "key6", []byte("value6"), version.NewHeight(1, 1)),
+	})
 }
 
-func checkItrResults(t *testing.T, itr statedb.ResultsIterator, expectedResults []*statedb.VersionedKV) {
-	for i := 0; i < len(expectedResults); i++ {
-		res, _ := itr.Next()
-		testutil.AssertEquals(t, res, expectedResults[i])
-	}
-	lastRes, err := itr.Next()
-	testutil.AssertNoError(t, err, "")
-	testutil.AssertNil(t, lastRes)
+func checkItrResults(t *testing.T, testName string, itr statedb.ResultsIterator, expectedResults []*statedb.VersionedKV) {
+	t.Run(testName, func(t *testing.T) {
+		for i := 0; i < len(expectedResults); i++ {
+			res, _ := itr.Next()
+			testutil.AssertEquals(t, res, expectedResults[i])
+		}
+		lastRes, err := itr.Next()
+		testutil.AssertNoError(t, err, "")
+		testutil.AssertNil(t, lastRes)
+	})
 }
 
 func constructVersionedKV(ns string, key string, value []byte, version *version.Height) *statedb.VersionedKV {