split test and improved lock mechanism.

multiversx · Oct 1, 2024 · 899b04d · 899b04d
1 parent d214027
commit 899b04d
Show file tree

Hide file tree

Showing 2 changed files with 102 additions and 32 deletions.
diff --git a/process/interceptors/interceptedDataVerifier.go b/process/interceptors/interceptedDataVerifier.go
@@ -47,11 +47,10 @@ func (idv *interceptedDataVerifier) Verify(interceptedData process.InterceptedDa
 		return interceptedData.CheckValidity()
 	}
 
-	idv.km.RLock(hash)
-	val, ok := idv.cache.Get(interceptedData.Hash())
-	idv.km.RUnlock(hash)
+	idv.km.Lock(hash)
+	defer idv.km.Unlock(hash)
 
-	if ok {
+	if val, ok := idv.cache.Get(interceptedData.Hash()); ok {
 		if val == ValidInterceptedData {
 			return nil
 		}
@@ -61,10 +60,7 @@ func (idv *interceptedDataVerifier) Verify(interceptedData process.InterceptedDa
 
 	err := interceptedData.CheckValidity()
 	if err != nil {
-		idv.km.Lock(hash)
 		idv.cache.Put(interceptedData.Hash(), InvalidInterceptedData, interceptedDataStatusBytesSize)
-		idv.km.Unlock(hash)
-
 		return ErrInvalidInterceptedData
 	}
 

diff --git a/process/interceptors/interceptedDataVerifier_test.go b/process/interceptors/interceptedDataVerifier_test.go
@@ -1,7 +1,6 @@
 package interceptors
 
 import (
-	"errors"
 	"sync"
 	"testing"
 	"time"
@@ -46,7 +45,7 @@ func TestInterceptedDataVerifier_CheckValidityShouldWork(t *testing.T) {
 	verifier := defaultInterceptedDataVerifier(defaultSpan)
 
 	err := verifier.Verify(interceptedData)
-	require.Nil(t, err)
+	require.NoError(t, err)
 
 	errCount := atomic.Counter{}
 	wg := sync.WaitGroup{}
@@ -74,19 +73,7 @@ func TestInterceptedDataVerifier_CheckValidityShouldNotWork(t *testing.T) {
 	interceptedData := &testscommon.InterceptedDataStub{
 		CheckValidityCalled: func() error {
 			checkValidityCounter.Add(1)
-			return nil
-		},
-		IsForCurrentShardCalled: func() bool {
-			return false
-		},
-		HashCalled: func() []byte {
-			return []byte("hash")
-		},
-	}
-
-	interceptedDataWithErr := &testscommon.InterceptedDataStub{
-		CheckValidityCalled: func() error {
-			return errors.New("error")
+			return ErrInvalidInterceptedData
 		},
 		IsForCurrentShardCalled: func() bool {
 			return false
@@ -98,18 +85,105 @@ func TestInterceptedDataVerifier_CheckValidityShouldNotWork(t *testing.T) {
 
 	verifier := defaultInterceptedDataVerifier(defaultSpan)
 
-	err := verifier.Verify(interceptedDataWithErr)
-	require.Equal(t, ErrInvalidInterceptedData, err)
-	require.Equal(t, int64(0), checkValidityCounter.Get())
-
-	err = verifier.Verify(interceptedData)
-	// It is still invalid because it has the same hash.
+	err := verifier.Verify(interceptedData)
 	require.Equal(t, ErrInvalidInterceptedData, err)
-	require.Equal(t, int64(0), checkValidityCounter.Get())
 
-	<-time.After(defaultSpan + 100*time.Millisecond)
+	errCount := atomic.Counter{}
+	wg := sync.WaitGroup{}
+	for i := 0; i < 3; i++ {
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+			err := verifier.Verify(interceptedData)
+			if err != nil {
+				errCount.Add(1)
+			}
+		}()
+	}
+	wg.Wait()
 
-	err = verifier.Verify(interceptedData)
-	require.NoError(t, err)
+	require.Equal(t, int64(3), errCount.Get())
 	require.Equal(t, int64(1), checkValidityCounter.Get())
 }
+
+func TestInterceptedDataVerifier_CheckExpiryTime(t *testing.T) {
+	t.Parallel()
+
+	t.Run("expiry on valid data", func(t *testing.T) {
+		expiryTestDuration := defaultSpan * 2
+
+		checkValidityCounter := atomic.Counter{}
+
+		interceptedData := &testscommon.InterceptedDataStub{
+			CheckValidityCalled: func() error {
+				checkValidityCounter.Add(1)
+				return nil
+			},
+			IsForCurrentShardCalled: func() bool {
+				return false
+			},
+			HashCalled: func() []byte {
+				return []byte("hash")
+			},
+		}
+
+		verifier := defaultInterceptedDataVerifier(expiryTestDuration)
+
+		// First retrieval, check validity is reached.
+		err := verifier.Verify(interceptedData)
+		require.NoError(t, err)
+		require.Equal(t, int64(1), checkValidityCounter.Get())
+
+		// Second retrieval should be from the cache.
+		err = verifier.Verify(interceptedData)
+		require.NoError(t, err)
+		require.Equal(t, int64(1), checkValidityCounter.Get())
+
+		// Wait for the cache expiry
+		<-time.After(expiryTestDuration + 100*time.Millisecond)
+
+		// Third retrieval should reach validity check again.
+		err = verifier.Verify(interceptedData)
+		require.NoError(t, err)
+		require.Equal(t, int64(2), checkValidityCounter.Get())
+	})
+
+	t.Run("expiry on invalid data", func(t *testing.T) {
+		expiryTestDuration := defaultSpan * 2
+
+		checkValidityCounter := atomic.Counter{}
+
+		interceptedData := &testscommon.InterceptedDataStub{
+			CheckValidityCalled: func() error {
+				checkValidityCounter.Add(1)
+				return ErrInvalidInterceptedData
+			},
+			IsForCurrentShardCalled: func() bool {
+				return false
+			},
+			HashCalled: func() []byte {
+				return []byte("hash")
+			},
+		}
+
+		verifier := defaultInterceptedDataVerifier(expiryTestDuration)
+
+		// First retrieval, check validity is reached.
+		err := verifier.Verify(interceptedData)
+		require.Equal(t, ErrInvalidInterceptedData, err)
+		require.Equal(t, int64(1), checkValidityCounter.Get())
+
+		// Second retrieval should be from the cache.
+		err = verifier.Verify(interceptedData)
+		require.Equal(t, ErrInvalidInterceptedData, err)
+		require.Equal(t, int64(1), checkValidityCounter.Get())
+
+		// Wait for the cache expiry
+		<-time.After(expiryTestDuration + 100*time.Millisecond)
+
+		// Third retrieval should reach validity check again.
+		err = verifier.Verify(interceptedData)
+		require.Equal(t, ErrInvalidInterceptedData, err)
+		require.Equal(t, int64(2), checkValidityCounter.Get())
+	})
+}