candles: Respect the cursor location when copying.

client/core/bookie.go

@@ -71,19 +71,6 @@ type candleCache struct {
 	on        uint32
 }
 
-// copy creates a copy of the candles.
-func (c *candleCache) copy() []candles.Candle {
-	var cands []candles.Candle
-	// The last candle can be modified after creation. Make a copy.
-	c.candleMtx.RLock()
-	defer c.candleMtx.RUnlock()
-	if len(c.Candles) > 0 {
-		cands = make([]candles.Candle, len(c.Candles))
-		copy(cands, c.Candles)
-	}
-	return cands
-}
-
 // init resets the candles with the supplied set.
 func (c *candleCache) init(in []*msgjson.Candle) {
 	c.candleMtx.Lock()
@@ -285,14 +272,17 @@ func (b *bookie) candles(durStr string, feedID uint32) error {
 			return
 		}
 		dur, _ := time.ParseDuration(durStr)
+		cache.candleMtx.RLock()
+		cdls := cache.CandlesCopy()
+		cache.candleMtx.RUnlock()
 		f.c <- &BookUpdate{
 			Action:   FreshCandlesAction,
 			Host:     b.dc.acct.host,
 			MarketID: marketName(b.base, b.quote),
 			Payload: &CandlesPayload{
 				Dur:          durStr,
 				DurMilliSecs: uint64(dur.Milliseconds()),
-				Candles:      cache.copy(),
+				Candles:      cdls,
 			},
 		}
 	}()

dex/candles/candles.go

@@ -101,10 +101,27 @@ func (c *Cache) WireCandles(count int) *msgjson.WireCandles {
 	return wc
 }
 
-// Delta calculates the the change in rate, as a percentage, and total volume
-// over the specified period going backwards from now. Because the first candle
-// does not necessarily align with the cutoff, the rate and volume contribution
-// from that candle is linearly interpreted between the endpoints. The caller is
+// CandlesCopy returns a deep copy of Candles with the oldest candle at the
+// first index.
+func (c *Cache) CandlesCopy() []msgjson.Candle {
+	sz := len(c.Candles)
+	candles := make([]msgjson.Candle, sz)
+	switch {
+	case sz == 0:
+	case sz == c.cap: // circular mode
+		oldIdx := (c.cursor + 1) % c.cap
+		copy(candles, c.Candles[oldIdx:])
+		copy(candles[sz-oldIdx:], c.Candles)
+	default:
+		copy(candles, c.Candles)
+	}
+	return candles
+}
+
+// Delta calculates the change in rate, as a percentage, and total volume over
+// the specified period going backwards from now. Because the first candle does
+// not necessarily align with the cutoff, the rate and volume contribution from
+// that candle is linearly interpreted between the endpoints. The caller is
 // responsible for making sure that dur >> binSize, otherwise the results will
 // be of little value.
 func (c *Cache) Delta(since time.Time) (changePct float64, vol, high, low uint64) {

dex/candles/candles_test.go

@@ -243,3 +243,74 @@ func TestDeltaPartialDays(t *testing.T) {
 		t.Fatalf("wrong 12-hour volume. wanted 110, got %d", vol6)
 	}
 }
+
+func TestCandlesCopy(t *testing.T) {
+	smallCap := 10
+	binSize := uint64(60 * 5 * 1000)
+	cacheWithCandles := func(adds uint64) *Cache {
+		cache := NewCache(smallCap, binSize)
+		for i := uint64(0); i < adds; i++ {
+			candle := &Candle{
+				StartStamp: i * binSize,
+				EndStamp:   (i + 1) * binSize,
+			}
+			cache.Add(candle)
+		}
+		return cache
+	}
+	tests := []struct {
+		name                string
+		adds                uint64
+		wantLen             int
+		wantFirst, wantLast uint64 // EndStamp times
+	}{{
+		name: "no candles",
+	}, {
+		name:      "one candle",
+		adds:      1,
+		wantLen:   1,
+		wantFirst: binSize,
+		wantLast:  binSize,
+	}, {
+		name:      "five candles",
+		adds:      5,
+		wantLen:   5,
+		wantFirst: binSize,
+		wantLast:  binSize * 5,
+	}, {
+		name:      "at cap before reusing zero",
+		adds:      uint64(smallCap),
+		wantLen:   smallCap,
+		wantFirst: binSize,
+		wantLast:  binSize * 10,
+	}, {
+		name:      "first candle over cap",
+		adds:      11,
+		wantLen:   smallCap,
+		wantFirst: binSize * (11 + 1 - uint64(smallCap)),
+		wantLast:  binSize * 11,
+	}, {
+		name:      "many times over cap",
+		adds:      1345,
+		wantLen:   smallCap,
+		wantFirst: binSize * (1345 + 1 - uint64(smallCap)),
+		wantLast:  binSize * 1345,
+	}}
+	for _, test := range tests {
+		c := cacheWithCandles(test.adds)
+		cc := c.CandlesCopy()
+		if len(cc) != test.wantLen {
+			t.Fatalf("%q: wanted len %d but got %d", test.name, test.wantLen, len(cc))
+		}
+		if test.wantLen != 0 {
+			first := cc[0].EndStamp
+			if first != test.wantFirst {
+				t.Fatalf("%q: wanted first end stamp %d but got %d", test.name, test.wantFirst, first)
+			}
+			last := cc[len(cc)-1].EndStamp
+			if last != test.wantLast {
+				t.Fatalf("%q: wanted last end stamp %d but got %d", test.name, test.wantLast, last)
+			}
+		}
+	}
+}

server/apidata/apidata.go

@@ -20,7 +20,6 @@ import (
 var (
 	// Our internal millisecond representation of the bin sizes.
 	binSizes []uint64
-	bin5min  uint64 = 60 * 5 * 1000
 	started  uint32
 )