clients/horizonclient: fix theoretical bug in currentServerTime (#4596)

address the case where the host name provided to currentServerTime isn't contained in the global ServerTimeMap.

clients/horizonclient/internal.go

@@ -140,9 +140,9 @@ func setCurrentServerTime(host string, serverDate []string, clock *clock.Clock)
 // currentServerTime returns the current server time for a given horizon server
 func currentServerTime(host string, currentTimeUTC int64) int64 {
 	serverTimeMapMutex.Lock()
-	st := ServerTimeMap[host]
+	st, has := ServerTimeMap[host]
 	serverTimeMapMutex.Unlock()
-	if &st == nil {
+	if !has {
 		return 0
 	}
 

clients/horizonclient/internal_test.go

@@ -0,0 +1,44 @@
+package horizonclient
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestCurrentServerTime(t *testing.T) {
+	t.Run("non-existing-host-name", func(t *testing.T) {
+		currentTime := currentServerTime("non-existing-host-name", 60)
+		require.Zerof(t, currentTime, "server time for non-existing time is expected to be zero, but was %d instead", currentTime)
+	})
+
+	t.Run("server-behind", func(t *testing.T) {
+		serverTimeMapMutex.Lock()
+		ServerTimeMap["TestCurrentServerTime-server-behind"] = ServerTimeRecord{ServerTime: 27, LocalTimeRecorded: 23}
+		serverTimeMapMutex.Unlock()
+
+		currentTime := currentServerTime("TestCurrentServerTime-server-behind", 500)
+		defer func() {
+			serverTimeMapMutex.Lock()
+			delete(ServerTimeMap, "TestCurrentServerTime-server-behind")
+			serverTimeMapMutex.Unlock()
+		}()
+
+		require.Zerof(t, currentTime, "server time is too old and the method should have returned 0; instead, %d was returned", currentTime)
+	})
+
+	t.Run("normal", func(t *testing.T) {
+		serverTimeMapMutex.Lock()
+		ServerTimeMap["TestCurrentServerTime-server"] = ServerTimeRecord{ServerTime: 27, LocalTimeRecorded: 23}
+		serverTimeMapMutex.Unlock()
+
+		defer func() {
+			serverTimeMapMutex.Lock()
+			delete(ServerTimeMap, "TestCurrentServerTime-server")
+			serverTimeMapMutex.Unlock()
+		}()
+
+		currentTime := currentServerTime("TestCurrentServerTime-server", 37)
+		require.Equalf(t, currentTime, int64(41), "currentServerTime should have returned %d, but returned %d instead", 41, currentTime)
+	})
+}

clients/horizonclient/main_test.go

@@ -58,7 +58,7 @@ func TestCheckMemoRequired(t *testing.T) {
 		Asset:       txnbuild.NativeAsset{},
 	}
 
-	asset := txnbuild.CreditAsset{"ABCD", kp.Address()}
+	asset := txnbuild.CreditAsset{Code: "ABCD", Issuer: kp.Address()}
 	pathPaymentStrictSend := txnbuild.PathPaymentStrictSend{
 		SendAsset:   asset,
 		SendAmount:  "10",
@@ -1455,7 +1455,9 @@ func TestFetchTimebounds(t *testing.T) {
 	// When no saved server time, return local time
 	st, err := client.FetchTimebounds(100)
 	if assert.NoError(t, err) {
+		serverTimeMapMutex.Lock()
 		assert.IsType(t, ServerTimeMap["localhost"], ServerTimeRecord{})
+		serverTimeMapMutex.Unlock()
 		assert.Equal(t, st.MinTime, int64(0))
 	}
 
@@ -1472,15 +1474,19 @@ func TestFetchTimebounds(t *testing.T) {
 	// get saved server time
 	st, err = client.FetchTimebounds(100)
 	if assert.NoError(t, err) {
+		serverTimeMapMutex.Lock()
 		assert.IsType(t, ServerTimeMap["localhost"], ServerTimeRecord{})
+		serverTimeMapMutex.Unlock()
 		assert.Equal(t, st.MinTime, int64(0))
 		// serverTime + 100seconds
 		assert.Equal(t, st.MaxTime, int64(1560947196))
 	}
 
 	// mock server time
 	newRecord := ServerTimeRecord{ServerTime: 100, LocalTimeRecorded: 1560947096}
+	serverTimeMapMutex.Lock()
 	ServerTimeMap["localhost"] = newRecord
+	serverTimeMapMutex.Unlock()
 	st, err = client.FetchTimebounds(100)
 	assert.NoError(t, err)
 	assert.IsType(t, st, txnbuild.TimeBounds{})