From 25b0c40d05599ef4bcf3de93ae553d061457a51f Mon Sep 17 00:00:00 2001 From: carla Date: Tue, 8 Sep 2020 13:47:12 +0200 Subject: [PATCH] chanfitness: fix line wrapping Original PR was written with 4 spaces instead of 8, do a once off fix here rather than fixing bit-by bit in the subsequent commits and cluttering them for review. --- chanfitness/chanevent.go | 52 ++++++----- chanfitness/chanevent_test.go | 49 +++++----- chanfitness/chaneventstore.go | 87 +++++++++-------- chanfitness/chaneventstore_test.go | 145 ++++++++++++++++++----------- 4 files changed, 197 insertions(+), 136 deletions(-) diff --git a/chanfitness/chanevent.go b/chanfitness/chanevent.go index 53048ee3..c749cf1c 100644 --- a/chanfitness/chanevent.go +++ b/chanfitness/chanevent.go @@ -51,12 +51,12 @@ type chanEventLog struct { now func() time.Time // openedAt tracks the first time this channel was seen. This is not - // necessarily the time that it confirmed on chain because channel events - // are not persisted at present. + // necessarily the time that it confirmed on chain because channel + // events are not persisted at present. openedAt time.Time - // closedAt is the time that the channel was closed. If the channel has not - // been closed yet, it is zero. + // closedAt is the time that the channel was closed. If the channel has + // not been closed yet, it is zero. closedAt time.Time } @@ -83,7 +83,8 @@ func (e *chanEventLog) close() { // The open time for the eventLog will be set to the event's timestamp if it is // not set yet. func (e *chanEventLog) add(eventType eventType) { - // If the channel is already closed, return early without adding an event. + // If the channel is already closed, return early without adding an + // event. if !e.closedAt.IsZero() { return } @@ -136,14 +137,16 @@ func (e *chanEventLog) getOnlinePeriods() []*onlinePeriod { case peerOfflineEvent: offline = true - // Do not add to uptime if there is no previous online timestamp, - // the event log has started with an offline event + // Do not add to uptime if there is no previous online + // timestamp, the event log has started with an offline + // event if lastOnline.IsZero() { continue } - // The eventLog has recorded an offline event, having previously - // been online so we add an online period to to set of online periods. + // The eventLog has recorded an offline event, having + // previously been online so we add an online period to + // set of online periods. onlinePeriods = append(onlinePeriods, &onlinePeriod{ start: lastOnline, end: event.timestamp, @@ -151,15 +154,15 @@ func (e *chanEventLog) getOnlinePeriods() []*onlinePeriod { } } - // If the last event was an peer offline event, we do not need to calculate - // a final online period and can return online periods as is. + // If the last event was an peer offline event, we do not need to + // calculate a final online period and can return online periods as is. if offline { return onlinePeriods } - // The log ended on an online event, so we need to add a final online event. - // If the channel is closed, this period is until channel closure. It it is - // still open, we calculate it until the present. + // The log ended on an online event, so we need to add a final online + // event. If the channel is closed, this period is until channel + // closure. It it is still open, we calculate it until the present. endTime := e.closedAt if endTime.IsZero() { endTime = e.now() @@ -176,7 +179,8 @@ func (e *chanEventLog) getOnlinePeriods() []*onlinePeriod { // inclusive range specified. An error is returned if the end of the range is // before the start or a zero end time is returned. func (e *chanEventLog) uptime(start, end time.Time) (time.Duration, error) { - // Error if we are provided with an invalid range to calculate uptime for. + // Error if we are provided with an invalid range to calculate uptime + // for. if end.Before(start) { return 0, fmt.Errorf("end time: %v before start time: %v", end, start) @@ -188,25 +192,27 @@ func (e *chanEventLog) uptime(start, end time.Time) (time.Duration, error) { var uptime time.Duration for _, p := range e.getOnlinePeriods() { - // The online period ends before the range we're looking at, so we can - // skip over it. + // The online period ends before the range we're looking at, so + // we can skip over it. if p.end.Before(start) { continue } - // The online period starts after the range we're looking at, so can - // stop calculating uptime. + // The online period starts after the range we're looking at, so + // can stop calculating uptime. if p.start.After(end) { break } - // If the online period starts before our range, shift the start time up - // so that we only calculate uptime from the start of our range. + // If the online period starts before our range, shift the start + // time up so that we only calculate uptime from the start of + // our range. if p.start.Before(start) { p.start = start } - // If the online period ends before our range, shift the end time - // forward so that we only calculate uptime until the end of the range. + // If the online period ends before our range, shift the end + // time forward so that we only calculate uptime until the end + // of the range. if p.end.After(end) { p.end = end } diff --git a/chanfitness/chanevent_test.go b/chanfitness/chanevent_test.go index 72733dbe..1fec1d95 100644 --- a/chanfitness/chanevent_test.go +++ b/chanfitness/chanevent_test.go @@ -40,9 +40,11 @@ func TestAdd(t *testing.T) { test.eventLog.add(test.event) for i, e := range test.expected { - if test.eventLog.events[i].eventType != e { - t.Fatalf("Expected event type: %v, got: %v", - e, test.eventLog.events[i].eventType) + eventType := test.eventLog.events[i].eventType + if eventType != e { + t.Fatalf("Expected event type: %v, "+ + "got: %v", e, eventType, + ) } } }) @@ -154,13 +156,13 @@ func TestGetOnlinePeriod(t *testing.T) { for i, o := range test.expectedOnline { if online[i].start != o.start { - t.Errorf("Expected start: %v, got %v", o.start, - online[i].start) + t.Errorf("Expected start: %v, got "+ + "%v", o.start, online[i].start) } if online[i].end != o.end { - t.Errorf("Expected end: %v, got %v", o.end, - online[i].end) + t.Errorf("Expected end: %v, got %v", + o.end, online[i].end) } } }) @@ -181,31 +183,34 @@ func TestUptime(t *testing.T) { tests := []struct { name string - // opened at is the time the channel was recorded as being open, and is - // never expected to be zero. + // opened at is the time the channel was recorded as being open, + // and is never expected to be zero. openedAt time.Time - // closed at is the tim the channel was recorded as being closed, and - // can have a zero value if the. + // closed at is the time the channel was recorded as being + // closed, and can have a zero value if the channel is not + // closed. closedAt time.Time - // events is the set of event log that we are calculating uptime for. + // events is the set of event log that we are calculating uptime + // for. events []*channelEvent - // startTime is the beginning of the period that we are calculating - // uptime for, it cannot have a zero value. + // startTime is the beginning of the period that we are + // calculating uptime for, it cannot have a zero value. startTime time.Time - // endTime is the end of the period that we are calculating uptime for, - // it cannot have a zero value. + // endTime is the end of the period that we are calculating + // uptime for, it cannot have a zero value. endTime time.Time - // expectedUptime is the amount of uptime we expect to be calculated - // over the period specified by startTime and endTime. + // expectedUptime is the amount of uptime we expect to be + // calculated over the period specified by startTime and + // endTime. expectedUptime time.Duration - // expectErr is set to true if we expect an error to be returned when - // calling the uptime function + // expectErr is set to true if we expect an error to be returned + // when calling the uptime function. expectErr bool }{ { @@ -378,7 +383,9 @@ func TestUptime(t *testing.T) { closedAt: test.closedAt, } - uptime, err := score.uptime(test.startTime, test.endTime) + uptime, err := score.uptime( + test.startTime, test.endTime, + ) if test.expectErr && err == nil { t.Fatal("Expected an error, got nil") } diff --git a/chanfitness/chaneventstore.go b/chanfitness/chaneventstore.go index b54409bf..1fa846ae 100644 --- a/chanfitness/chaneventstore.go +++ b/chanfitness/chaneventstore.go @@ -24,12 +24,12 @@ import ( ) var ( - // errShuttingDown is returned when the store cannot respond to a query because - // it has received the shutdown signal. + // errShuttingDown is returned when the store cannot respond to a query + // because it has received the shutdown signal. errShuttingDown = errors.New("channel event store shutting down") - // ErrChannelNotFound is returned when a query is made for a channel that - // the event store does not have knowledge of. + // ErrChannelNotFound is returned when a query is made for a channel + // that the event store does not have knowledge of. ErrChannelNotFound = errors.New("channel not found in event store") ) @@ -41,8 +41,8 @@ type ChannelEventStore struct { // channels maps channel points to event logs. channels map[wire.OutPoint]*chanEventLog - // peers tracks the current online status of peers based on online/offline - // events. + // peers tracks the current online status of peers based on online + // and offline events. peers map[route.Vertex]bool // lifespanRequests serves requests for the lifespan of channels. @@ -60,16 +60,17 @@ type ChannelEventStore struct { // activity. All elements of the config must be non-nil for the event store to // operate. type Config struct { - // SubscribeChannelEvents provides a subscription client which provides a - // stream of channel events. + // SubscribeChannelEvents provides a subscription client which provides + // a stream of channel events. SubscribeChannelEvents func() (*subscribe.Client, error) // SubscribePeerEvents provides a subscription client which provides a // stream of peer online/offline events. SubscribePeerEvents func() (*subscribe.Client, error) - // GetOpenChannels provides a list of existing open channels which is used - // to populate the ChannelEventStore with a set of channels on startup. + // GetOpenChannels provides a list of existing open channels which is + // used to populate the ChannelEventStore with a set of channels on + // startup. GetOpenChannels func() ([]*channeldb.OpenChannel, error) } @@ -140,7 +141,8 @@ func (c *ChannelEventStore) Start() error { return err } - // cancel should be called to cancel all subscriptions if an error occurs. + // cancel should be called to cancel all subscriptions if an error + // occurs. cancel := func() { channelClient.Cancel() peerClient.Cancel() @@ -166,8 +168,8 @@ func (c *ChannelEventStore) Start() error { return err } - // Add existing channels to the channel store with an initial peer - // online or offline event. + // Add existing channels to the channel store with an initial + // peer online or offline event. c.addChannel(ch.FundingOutpoint, peerKey) } @@ -200,10 +202,12 @@ func (c *ChannelEventStore) Stop() { func (c *ChannelEventStore) addChannel(channelPoint wire.OutPoint, peer route.Vertex) { - // Check for the unexpected case where the channel is already in the store. + // Check for the unexpected case where the channel is already in the + // store. _, ok := c.channels[channelPoint] if ok { - log.Errorf("Channel %v duplicated in channel store", channelPoint) + log.Errorf("Channel %v duplicated in channel store", + channelPoint) return } @@ -222,7 +226,8 @@ func (c *ChannelEventStore) addChannel(channelPoint wire.OutPoint, // closeChannel records a closed time for a channel, and returns early is the // channel is not known to the event store. func (c *ChannelEventStore) closeChannel(channelPoint wire.OutPoint) { - // Check for the unexpected case where the channel is unknown to the store. + // Check for the unexpected case where the channel is unknown to the + // store. eventLog, ok := c.channels[channelPoint] if !ok { log.Errorf("Close channel %v unknown to store", channelPoint) @@ -265,21 +270,24 @@ func (c *ChannelEventStore) consume(subscriptions *subscriptions) { // Process channel opened and closed events. case e := <-subscriptions.channelUpdates: switch event := e.(type) { - // A new channel has been opened, we must add the channel to the - // store and record a channel open event. + // A new channel has been opened, we must add the + // channel to the store and record a channel open event. case channelnotifier.OpenChannelEvent: + compressed := event.Channel.IdentityPub.SerializeCompressed() peerKey, err := route.NewVertexFromBytes( - event.Channel.IdentityPub.SerializeCompressed(), + compressed, ) if err != nil { - log.Errorf("Could not get vertex from: %v", - event.Channel.IdentityPub.SerializeCompressed()) + log.Errorf("Could not get vertex "+ + "from: %v", compressed) } - c.addChannel(event.Channel.FundingOutpoint, peerKey) + c.addChannel( + event.Channel.FundingOutpoint, peerKey, + ) - // A channel has been closed, we must remove the channel from the - // store and record a channel closed event. + // A channel has been closed, we must remove the channel + // from the store and record a channel closed event. case channelnotifier.ClosedChannelEvent: c.closeChannel(event.CloseSummary.ChanPoint) } @@ -287,13 +295,15 @@ func (c *ChannelEventStore) consume(subscriptions *subscriptions) { // Process peer online and offline events. case e := <-subscriptions.peerUpdates: switch event := e.(type) { - // We have reestablished a connection with our peer, and should - // record an online event for any channels with that peer. + // We have reestablished a connection with our peer, + // and should record an online event for any channels + // with that peer. case peernotifier.PeerOnlineEvent: c.peerEvent(event.PubKey, peerOnlineEvent) - // We have lost a connection with our peer, and should record an - // offline event for any channels with that peer. + // We have lost a connection with our peer, and should + // record an offline event for any channels with that + // peer. case peernotifier.PeerOfflineEvent: c.peerEvent(event.PubKey, peerOfflineEvent) } @@ -320,7 +330,10 @@ func (c *ChannelEventStore) consume(subscriptions *subscriptions) { if !ok { resp.err = ErrChannelNotFound } else { - uptime, err := channel.uptime(req.startTime, req.endTime) + uptime, err := channel.uptime( + req.startTime, req.endTime, + ) + resp.uptime = uptime resp.err = err } @@ -346,16 +359,16 @@ func (c *ChannelEventStore) GetLifespan( } // Send a request for the channel's lifespan to the main event loop, or - // return early with an error if the store has already received a shutdown - // signal. + // return early with an error if the store has already received a + // shutdown signal. select { case c.lifespanRequests <- request: case <-c.quit: return time.Time{}, time.Time{}, errShuttingDown } - // Return the response we receive on the response channel or exit early if - // the store is instructed to exit. + // Return the response we receive on the response channel or exit early + // if the store is instructed to exit. select { case resp := <-request.responseChan: return resp.start, resp.end, resp.err @@ -378,16 +391,16 @@ func (c *ChannelEventStore) GetUptime(channelPoint wire.OutPoint, startTime, } // Send a request for the channel's uptime to the main event loop, or - // return early with an error if the store has already received a shutdown - // signal. + // return early with an error if the store has already received a + // shutdown signal. select { case c.uptimeRequests <- request: case <-c.quit: return 0, errShuttingDown } - // Return the response we receive on the response channel or exit early if - // the store is instructed to exit. + // Return the response we receive on the response channel or exit early + // if the store is instructed to exit. select { case resp := <-request.responseChan: return resp.uptime, resp.err diff --git a/chanfitness/chaneventstore_test.go b/chanfitness/chaneventstore_test.go index 5a785884..793fbb66 100644 --- a/chanfitness/chaneventstore_test.go +++ b/chanfitness/chaneventstore_test.go @@ -19,14 +19,15 @@ import ( // functions fail. It does not test the mechanics of consuming events because // these are covered in a separate set of tests. func TestStartStoreError(t *testing.T) { - // Ok and erroring subscribe functions are defined here to de-clutter tests. + // Ok and erroring subscribe functions are defined here to de-clutter + // tests. okSubscribeFunc := func() (*subscribe.Client, error) { return &subscribe.Client{ Cancel: func() {}, }, nil } - errSubscribeFunc := func() (client *subscribe.Client, e error) { + errSubscribeFunc := func() (*subscribe.Client, error) { return nil, errors.New("intentional test err") } @@ -49,7 +50,7 @@ func TestStartStoreError(t *testing.T) { name: "Get open channels fails", ChannelEvents: okSubscribeFunc, PeerEvents: okSubscribeFunc, - GetChannels: func() (channels []*channeldb.OpenChannel, e error) { + GetChannels: func() ([]*channeldb.OpenChannel, error) { return nil, errors.New("intentional test err") }, }, @@ -66,8 +67,8 @@ func TestStartStoreError(t *testing.T) { }) err := store.Start() - // Check that we receive an error, because the test only checks for - // error cases. + // Check that we receive an error, because the test only + // checks for error cases. if err == nil { t.Fatalf("Expected error on startup, got: nil") } @@ -110,19 +111,22 @@ func TestMonitorChannelEvents(t *testing.T) { tests := []struct { name string - // generateEvents takes channels which represent the updates channels - // for subscription clients and passes events in the desired order. - // This function is intended to be blocking so that the test does not - // have a data race with event consumption, so the channels should not - // be buffered. - generateEvents func(channelEvents, peerEvents chan<- interface{}) + // generateEvents takes channels which represent the updates + // channels for subscription clients and passes events in the + // desired order. This function is intended to be blocking so + // that the test does not have a data race with event + // consumption, so the channels should not be buffered. + generateEvents func(channelEvents, + peerEvents chan<- interface{}) // expectedEvents is the expected set of event types in the store. expectedEvents []eventType }{ { name: "Channel opened, peer comes online", - generateEvents: func(channelEvents, peerEvents chan<- interface{}) { + generateEvents: func(channelEvents, + peerEvents chan<- interface{}) { + // Add an open channel event channelEvents <- channelnotifier.OpenChannelEvent{ Channel: &channeldb.OpenChannel{ @@ -132,13 +136,17 @@ func TestMonitorChannelEvents(t *testing.T) { } // Add a peer online event. - peerEvents <- peernotifier.PeerOnlineEvent{PubKey: vertex} + peerEvents <- peernotifier.PeerOnlineEvent{ + PubKey: vertex, + } }, expectedEvents: []eventType{peerOnlineEvent}, }, { name: "Duplicate channel open events", - generateEvents: func(channelEvents, peerEvents chan<- interface{}) { + generateEvents: func(channelEvents, + peerEvents chan<- interface{}) { + // Add an open channel event channelEvents <- channelnotifier.OpenChannelEvent{ Channel: &channeldb.OpenChannel{ @@ -148,7 +156,9 @@ func TestMonitorChannelEvents(t *testing.T) { } // Add a peer online event. - peerEvents <- peernotifier.PeerOnlineEvent{PubKey: vertex} + peerEvents <- peernotifier.PeerOnlineEvent{ + PubKey: vertex, + } // Add a duplicate channel open event. channelEvents <- channelnotifier.OpenChannelEvent{ @@ -162,9 +172,13 @@ func TestMonitorChannelEvents(t *testing.T) { }, { name: "Channel opened, peer already online", - generateEvents: func(channelEvents, peerEvents chan<- interface{}) { + generateEvents: func(channelEvents, + peerEvents chan<- interface{}) { + // Add a peer online event. - peerEvents <- peernotifier.PeerOnlineEvent{PubKey: vertex} + peerEvents <- peernotifier.PeerOnlineEvent{ + PubKey: vertex, + } // Add an open channel event channelEvents <- channelnotifier.OpenChannelEvent{ @@ -179,7 +193,9 @@ func TestMonitorChannelEvents(t *testing.T) { { name: "Channel opened, peer offline, closed", - generateEvents: func(channelEvents, peerEvents chan<- interface{}) { + generateEvents: func(channelEvents, + peerEvents chan<- interface{}) { + // Add an open channel event channelEvents <- channelnotifier.OpenChannelEvent{ Channel: &channeldb.OpenChannel{ @@ -189,7 +205,9 @@ func TestMonitorChannelEvents(t *testing.T) { } // Add a peer online event. - peerEvents <- peernotifier.PeerOfflineEvent{PubKey: vertex} + peerEvents <- peernotifier.PeerOfflineEvent{ + PubKey: vertex, + } // Add a close channel event. channelEvents <- channelnotifier.ClosedChannelEvent{ @@ -202,7 +220,9 @@ func TestMonitorChannelEvents(t *testing.T) { }, { name: "Event after channel close not recorded", - generateEvents: func(channelEvents, peerEvents chan<- interface{}) { + generateEvents: func(channelEvents, + peerEvents chan<- interface{}) { + // Add an open channel event channelEvents <- channelnotifier.OpenChannelEvent{ Channel: &channeldb.OpenChannel{ @@ -219,7 +239,9 @@ func TestMonitorChannelEvents(t *testing.T) { } // Add a peer online event. - peerEvents <- peernotifier.PeerOfflineEvent{PubKey: vertex} + peerEvents <- peernotifier.PeerOfflineEvent{ + PubKey: vertex, + } }, }, } @@ -228,12 +250,12 @@ func TestMonitorChannelEvents(t *testing.T) { test := test t.Run(test.name, func(t *testing.T) { - // Create a store with the channels and online peers specified - // by the test. + // Create a store with the channels and online peers + // specified by the test. store := NewChannelEventStore(&Config{}) - // Create channels which represent the subscriptions we have to peer - // and client events. + // Create channels which represent the subscriptions + // we have to peer and client events. channelEvents := make(chan interface{}) peerEvents := make(chan interface{}) @@ -244,21 +266,23 @@ func TestMonitorChannelEvents(t *testing.T) { cancel: func() {}, }) - // Add events to the store then kill the goroutine using store.Stop. + // Add events to the store then kill the goroutine using + // store.Stop. test.generateEvents(channelEvents, peerEvents) store.Stop() - // Retrieve the eventLog for the channel and check that its - // contents are as expected. + // Retrieve the eventLog for the channel and check that + // its contents are as expected. eventLog, ok := store.channels[chanPoint] if !ok { t.Fatalf("Expected to find event store") } for i, e := range eventLog.events { - if test.expectedEvents[i] != e.eventType { + expectedType := test.expectedEvents[i] + if expectedType != e.eventType { t.Fatalf("Expected type: %v, got: %v", - test.expectedEvents[i], e.eventType) + expectedType, e.eventType) } } }) @@ -309,26 +333,30 @@ func TestGetLifetime(t *testing.T) { // Stop the store's go routine. defer store.Stop() - // Add channel to eventStore if the test indicates that it should - // be present. + // Add channel to eventStore if the test indicates that + // it should be present. if test.channelFound { - store.channels[test.channelPoint] = &chanEventLog{ - openedAt: test.opened, - closedAt: test.closed, - } + store.channels[test.channelPoint] = + &chanEventLog{ + openedAt: test.opened, + closedAt: test.closed, + } } open, close, err := store.GetLifespan(test.channelPoint) if test.expectedError != err { - t.Fatalf("Expected: %v, got: %v", test.expectedError, err) + t.Fatalf("Expected: %v, got: %v", + test.expectedError, err) } if open != test.opened { - t.Errorf("Expected: %v, got %v", test.opened, open) + t.Errorf("Expected: %v, got %v", + test.opened, open) } if close != test.closed { - t.Errorf("Expected: %v, got %v", test.closed, close) + t.Errorf("Expected: %v, got %v", + test.closed, close) } }) } @@ -351,24 +379,28 @@ func TestGetUptime(t *testing.T) { channelPoint wire.OutPoint - // events is the set of events we expect to find in the channel store. + // events is the set of events we expect to find in the channel + // store. events []*channelEvent - // openedAt is the time the channel is recorded as open by the store. + // openedAt is the time the channel is recorded as open by the + // store. openedAt time.Time - // closedAt is the time the channel is recorded as closed by the store. - // If the channel is still open, this value is zero. + // closedAt is the time the channel is recorded as closed by the + // store. If the channel is still open, this value is zero. closedAt time.Time - // channelFound is true if we expect to find the channel in the store. + // channelFound is true if we expect to find the channel in the + // store. channelFound bool - // startTime specifies the beginning of the uptime range we want to - // calculate. + // startTime specifies the beginning of the uptime range we want + // to calculate. startTime time.Time - // endTime specified the end of the uptime range we want to calculate. + // endTime specified the end of the uptime range we want to + // calculate. endTime time.Time expectedUptime time.Duration @@ -428,8 +460,8 @@ func TestGetUptime(t *testing.T) { test := test t.Run(test.name, func(t *testing.T) { - // Set up event store with the events specified for the test and - // mocked time. + // Set up event store with the events specified for the + // test and mocked time. store := NewChannelEventStore(&Config{}) // Start goroutine which consumes GetUptime requests. @@ -443,7 +475,8 @@ func TestGetUptime(t *testing.T) { // Stop the store's goroutine. defer store.Stop() - // Add the channel to the store if it is intended to be found. + // Add the channel to the store if it is intended to be + // found. if test.channelFound { store.channels[test.channelPoint] = &chanEventLog{ events: test.events, @@ -453,16 +486,18 @@ func TestGetUptime(t *testing.T) { } } - uptime, err := store.GetUptime(test.channelPoint, test.startTime, test.endTime) + uptime, err := store.GetUptime( + test.channelPoint, test.startTime, test.endTime, + ) if test.expectedError != err { - t.Fatalf("Expected: %v, got: %v", test.expectedError, err) + t.Fatalf("Expected: %v, got: %v", + test.expectedError, err) } if uptime != test.expectedUptime { - t.Fatalf("Expected uptime percentage: %v, got %v", - test.expectedUptime, uptime) + t.Fatalf("Expected uptime percentage: %v, "+ + "got %v", test.expectedUptime, uptime) } - }) } }