From eebc23a8adbb0bad5aa1b1acb58bd561c2c1aae7 Mon Sep 17 00:00:00 2001
From: carla <kirkcohenc@gmail.com>
Date: Tue, 17 Dec 2019 17:36:28 +0200
Subject: [PATCH] chanfitness: set open time for channels with offline peers

This commit addresses a bug in the channel event store
where the opened time of a channel event log was not
set for peers that were offline on startup.

Previously, opened time was set to the time of the first
event in the event log. This worked for online peers,
because the eventlog was created with an initial online
event. However, offline peers had no inital event so had
no open time set.

This commit simplifies the creation of an event log by
removing the initial event and setting open time for all
event logs. This has the effect of potentially introducing
a gap between opened time for a log and the first peer
online event for peers with channels that exist at startup
if a peer takes time to reconnect. However, the cost of this
is less than the benefit of reducing the bug-prone custom
code path that was previously in place.
---
 chanfitness/chanevent.go           | 17 +++++++----------
 chanfitness/chaneventstore.go      |  8 +-------
 chanfitness/chaneventstore_test.go | 23 +++++++++++++++++++++++
 3 files changed, 31 insertions(+), 17 deletions(-)

diff --git a/chanfitness/chanevent.go b/chanfitness/chanevent.go
index f4caf7bf..53048ee3 100644
--- a/chanfitness/chanevent.go
+++ b/chanfitness/chanevent.go
@@ -60,14 +60,18 @@ type chanEventLog struct {
 	closedAt time.Time
 }
 
-func newEventLog(outpoint wire.OutPoint, peer route.Vertex,
+// newEventLog creates an event log for a channel with the openedAt time set.
+func newEventLog(channelPoint wire.OutPoint, peer route.Vertex,
 	now func() time.Time) *chanEventLog {
 
-	return &chanEventLog{
-		channelPoint: outpoint,
+	eventlog := &chanEventLog{
+		channelPoint: channelPoint,
 		peer:         peer,
 		now:          now,
+		openedAt:     now(),
 	}
+
+	return eventlog
 }
 
 // close sets the closing time for an event log.
@@ -91,13 +95,6 @@ func (e *chanEventLog) add(eventType eventType) {
 	}
 	e.events = append(e.events, event)
 
-	// If the eventLog does not have an opened time set, set it to the timestamp
-	// of the event. This has the effect of setting the eventLog's open time to
-	// the timestamp of the first event added.
-	if e.openedAt.IsZero() {
-		e.openedAt = event.timestamp
-	}
-
 	log.Debugf("Channel %v recording event: %v", e.channelPoint, eventType)
 }
 
diff --git a/chanfitness/chaneventstore.go b/chanfitness/chaneventstore.go
index 1853df0b..69107808 100644
--- a/chanfitness/chaneventstore.go
+++ b/chanfitness/chaneventstore.go
@@ -207,15 +207,9 @@ func (c *ChannelEventStore) addChannel(channelPoint wire.OutPoint,
 		return
 	}
 
+	// Create an event log for the channel.
 	eventLog := newEventLog(channelPoint, peer, time.Now)
 
-	// If the peer is online, add a peer online event to indicate its starting
-	// state.
-	online := c.peers[peer]
-	if online {
-		eventLog.add(peerOnlineEvent)
-	}
-
 	c.channels[channelPoint] = eventLog
 }
 
diff --git a/chanfitness/chaneventstore_test.go b/chanfitness/chaneventstore_test.go
index ce52c736..36a74990 100644
--- a/chanfitness/chaneventstore_test.go
+++ b/chanfitness/chaneventstore_test.go
@@ -466,3 +466,26 @@ func TestGetUptime(t *testing.T) {
 		})
 	}
 }
+
+// TestAddChannel tests that channels are added to the event store with
+// appropriate timestamps. This test addresses a bug where offline channels
+// did not have an opened time set.
+func TestAddChannel(t *testing.T) {
+	_, vertex, chanPoint := getTestChannel(t)
+
+	store := NewChannelEventStore(&Config{})
+
+	// Add channel to the store.
+	store.addChannel(chanPoint, vertex)
+
+	// Check that the eventlog is successfully added.
+	eventlog, ok := store.channels[chanPoint]
+	if !ok {
+		t.Fatalf("channel should be in store")
+	}
+
+	// Ensure that open time is always set.
+	if eventlog.openedAt.IsZero() {
+		t.Fatalf("channel should have opened at set")
+	}
+}