From 6bdfa9313459d7e6264e6065a3396fd5677c97d8 Mon Sep 17 00:00:00 2001
From: Sean DuBois <sean@pion.ly>
Date: Sat, 2 Sep 2023 13:52:20 -0400
Subject: [PATCH] Revert ReplayDetector API Changes

This reverts the following commits
* f2dffd80896b0c20da441a6d4984a349a9b8a5f1
* 3cefa92c188a218b6a303972882e3e824939bc22
* d490a2cb6efc291411b66766587fb377bd15a5f7
---
 replaydetector/replaydetector.go      |  35 +--
 replaydetector/replaydetector_test.go | 393 +++++++++++---------------
 2 files changed, 174 insertions(+), 254 deletions(-)

diff --git a/replaydetector/replaydetector.go b/replaydetector/replaydetector.go
index d407995..4358d8f 100644
--- a/replaydetector/replaydetector.go
+++ b/replaydetector/replaydetector.go
@@ -8,14 +8,7 @@ package replaydetector
 type ReplayDetector interface {
 	// Check returns true if given sequence number is not replayed.
 	// Call accept() to mark the packet is received properly.
-	// The return value of accept() indicates whether the accepted packet is
-	// has the latest observed sequence number.
-	Check(seq uint64) (accept func() bool, ok bool)
-}
-
-// nop is a no-op func that is returned in the case that Check() fails.
-func nop() bool {
-	return false
+	Check(seq uint64) (accept func(), ok bool)
 }
 
 type slidingWindowDetector struct {
@@ -37,33 +30,30 @@ func New(windowSize uint, maxSeq uint64) ReplayDetector {
 	}
 }
 
-func (d *slidingWindowDetector) Check(seq uint64) (func() bool, bool) {
+func (d *slidingWindowDetector) Check(seq uint64) (accept func(), ok bool) {
 	if seq > d.maxSeq {
 		// Exceeded upper limit.
-		return nop, false
+		return func() {}, false
 	}
 
 	if seq <= d.latestSeq {
 		if d.latestSeq >= uint64(d.windowSize)+seq {
-			return nop, false
+			return func() {}, false
 		}
 		if d.mask.Bit(uint(d.latestSeq-seq)) != 0 {
 			// The sequence number is duplicated.
-			return nop, false
+			return func() {}, false
 		}
 	}
 
-	return func() bool {
-		latest := seq == 0
+	return func() {
 		if seq > d.latestSeq {
 			// Update the head of the window.
 			d.mask.Lsh(uint(seq - d.latestSeq))
 			d.latestSeq = seq
-			latest = true
 		}
 		diff := (d.latestSeq - seq) % d.maxSeq
 		d.mask.SetBit(uint(diff))
-		return latest
 	}, true
 }
 
@@ -85,10 +75,10 @@ type wrappedSlidingWindowDetector struct {
 	init       bool
 }
 
-func (d *wrappedSlidingWindowDetector) Check(seq uint64) (func() bool, bool) {
+func (d *wrappedSlidingWindowDetector) Check(seq uint64) (accept func(), ok bool) {
 	if seq > d.maxSeq {
 		// Exceeded upper limit.
-		return nop, false
+		return func() {}, false
 	}
 	if !d.init {
 		if seq != 0 {
@@ -109,24 +99,21 @@ func (d *wrappedSlidingWindowDetector) Check(seq uint64) (func() bool, bool) {
 
 	if diff >= int64(d.windowSize) {
 		// Too old.
-		return nop, false
+		return func() {}, false
 	}
 	if diff >= 0 {
 		if d.mask.Bit(uint(diff)) != 0 {
 			// The sequence number is duplicated.
-			return nop, false
+			return func() {}, false
 		}
 	}
 
-	return func() bool {
-		latest := false
+	return func() {
 		if diff < 0 {
 			// Update the head of the window.
 			d.mask.Lsh(uint(-diff))
 			d.latestSeq = seq
-			latest = true
 		}
 		d.mask.SetBit(uint(d.latestSeq - seq))
-		return latest
 	}, true
 }
diff --git a/replaydetector/replaydetector_test.go b/replaydetector/replaydetector_test.go
index f19836e..eaf01b2 100644
--- a/replaydetector/replaydetector_test.go
+++ b/replaydetector/replaydetector_test.go
@@ -8,224 +8,150 @@ import (
 	"testing"
 )
 
-type testCase struct {
-	windowSize uint
-	maxSeq     uint64
-	input      []uint64
-	valid      []bool
-	latest     []bool
-	expected   []uint64
-}
-
-const (
-	largeSeq = 0x100000000000
-	hugeSeq  = 0x1000000000000
-)
-
-var commonCases = map[string]testCase{ //nolint:gochecknoglobals
-	"Continuous": {
-		16, 0x0000FFFFFFFFFFFF,
-		[]uint64{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20},
-		[]bool{
-			true, true, true, true, true, true, true, true, true, true,
-			true, true, true, true, true, true, true, true, true, true,
-			true,
-		},
-		[]bool{
-			true, true, true, true, true, true, true, true, true, true,
-			true, true, true, true, true, true, true, true, true, true,
-			true,
-		},
-		[]uint64{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20},
-	},
-	"ValidLargeJump": {
-		16, 0x0000FFFFFFFFFFFF,
-		[]uint64{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, largeSeq, 11, largeSeq + 1, largeSeq + 2, largeSeq + 3},
-		[]bool{
-			true, true, true, true, true, true, true, true, true, true,
-			true, false, true, true, true,
-		},
-		[]bool{
-			true, true, true, true, true, true, true, true, true, true,
-			true, false, true, true, true,
-		},
-		[]uint64{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, largeSeq, largeSeq + 1, largeSeq + 2, largeSeq + 3},
-	},
-	"InvalidLargeJump": {
-		16, 0x0000FFFFFFFFFFFF,
-		[]uint64{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, hugeSeq, 11, 12, 13, 14, 15},
-		[]bool{
-			true, true, true, true, true, true, true, true, true, true,
-			false, true, true, true, true, true,
-		},
-		[]bool{
-			true, true, true, true, true, true, true, true, true, true,
-			false, true, true, true, true, true,
-		},
-		[]uint64{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 11, 12, 13, 14, 15},
-	},
-	"DuplicateAfterValidJump": {
-		196, 0x0000FFFFFFFFFFFF,
-		[]uint64{0, 1, 2, 129, 0, 1, 2},
-		[]bool{
-			true, true, true, true, false, false, false,
-		},
-		[]bool{
-			true, true, true, true, false, false, false,
-		},
-		[]uint64{0, 1, 2, 129},
-	},
-	"DuplicateAfterInvalidJump": {
-		196, 0x0000FFFFFFFFFFFF,
-		[]uint64{0, 1, 2, hugeSeq, 0, 1, 2},
-		[]bool{
-			true, true, true, false, false, false, false,
-		},
-		[]bool{
-			true, true, true, false, false, false, false,
-		},
-		[]uint64{0, 1, 2},
-	},
-	"ContinuousOffset": {
-		16, 0x0000FFFFFFFFFFFF,
-		[]uint64{100, 101, 102, 103, 104, 105, 106, 107, 108, 109, 110, 111, 112, 113, 114},
-		[]bool{
-			true, true, true, true, true, true, true, true, true, true,
-			true, true, true, true, true,
-		},
-		[]bool{
-			true, true, true, true, true, true, true, true, true, true,
-			true, true, true, true, true,
-		},
-		[]uint64{100, 101, 102, 103, 104, 105, 106, 107, 108, 109, 110, 111, 112, 113, 114},
-	},
-	"Reordered": {
-		128, 0x0000FFFFFFFFFFFF,
-		[]uint64{96, 64, 16, 80, 32, 48, 8, 24, 88, 40, 128, 56, 72, 112, 104, 120},
-		[]bool{
-			true, true, true, true, true, true, true, true, true, true,
-			true, true, true, true, true, true,
-		},
-		[]bool{
-			true, false, false, false, false, false, false, false, false, false,
-			true, false, false, false, false, false,
+func TestReplayDetector(t *testing.T) {
+	const largeSeq = 0x100000000000
+	cases := map[string]struct {
+		windowSize   uint
+		maxSeq       uint64
+		input        []uint64
+		valid        []bool
+		expected     []uint64
+		expectedWrap []uint64 // nil means it's same as expected
+	}{
+		"Continuous": {
+			16, 0x0000FFFFFFFFFFFF,
+			[]uint64{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20},
+			[]bool{
+				true, true, true, true, true, true, true, true, true, true,
+				true, true, true, true, true, true, true, true, true, true,
+				true,
+			},
+			[]uint64{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20},
+			nil,
 		},
-		[]uint64{96, 64, 16, 80, 32, 48, 8, 24, 88, 40, 128, 56, 72, 112, 104, 120},
-	},
-	"Old": {
-		100, 0x0000FFFFFFFFFFFF,
-		[]uint64{24, 32, 40, 48, 56, 64, 72, 80, 88, 96, 104, 112, 120, 128, 8, 16},
-		[]bool{
-			true, true, true, true, true, true, true, true, true, true,
-			true, true, true, true, false, false,
+		"ValidLargeJump": {
+			16, 0x0000FFFFFFFFFFFF,
+			[]uint64{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, largeSeq, 11, largeSeq + 1, largeSeq + 2, largeSeq + 3},
+			[]bool{
+				true, true, true, true, true, true, true, true, true, true,
+				true, true, true, true, true,
+			},
+			[]uint64{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, largeSeq, largeSeq + 1, largeSeq + 2, largeSeq + 3},
+			nil,
 		},
-		[]bool{
-			true, true, true, true, true, true, true, true, true, true,
-			true, true, true, true, false, false,
+		"InvalidLargeJump": {
+			16, 0x0000FFFFFFFFFFFF,
+			[]uint64{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, largeSeq, 11, 12, 13, 14, 15},
+			[]bool{
+				true, true, true, true, true, true, true, true, true, true,
+				false, true, true, true, true, true,
+			},
+			[]uint64{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 11, 12, 13, 14, 15},
+			nil,
 		},
-		[]uint64{24, 32, 40, 48, 56, 64, 72, 80, 88, 96, 104, 112, 120, 128},
-	},
-	"ContinuousReplayed": {
-		8, 0x0000FFFFFFFFFFFF,
-		[]uint64{16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25},
-		[]bool{
-			true, true, true, true, true, true, true, true, true, true,
-			false, false, false, false, false, false, false, false, false, false,
+		"DuplicateAfterValidJump": {
+			196, 0x0000FFFFFFFFFFFF,
+			[]uint64{0, 1, 2, 129, 0, 1, 2},
+			[]bool{
+				true, true, true, true, true, true, true,
+			},
+			[]uint64{0, 1, 2, 129},
+			nil,
 		},
-		[]bool{
-			true, true, true, true, true, true, true, true, true, true,
-			false, false, false, false, false, false, false, false, false, false,
+		"DuplicateAfterInvalidJump": {
+			196, 0x0000FFFFFFFFFFFF,
+			[]uint64{0, 1, 2, 128, 0, 1, 2},
+			[]bool{
+				true, true, true, false, true, true, true,
+			},
+			[]uint64{0, 1, 2},
+			nil,
 		},
-		[]uint64{16, 17, 18, 19, 20, 21, 22, 23, 24, 25},
-	},
-	"ReplayedLater": {
-		128, 0x0000FFFFFFFFFFFF,
-		[]uint64{16, 32, 48, 64, 80, 96, 112, 128, 16, 32, 48, 64, 80, 96, 112, 128},
-		[]bool{
-			true, true, true, true, true, true, true, true, false, false,
-			false, false, false, false, false, false,
+		"ContinuousOffset": {
+			16, 0x0000FFFFFFFFFFFF,
+			[]uint64{100, 101, 102, 103, 104, 105, 106, 107, 108, 109, 110, 111, 112, 113, 114},
+			[]bool{
+				true, true, true, true, true, true, true, true, true, true,
+				true, true, true, true, true,
+			},
+			[]uint64{100, 101, 102, 103, 104, 105, 106, 107, 108, 109, 110, 111, 112, 113, 114},
+			nil,
 		},
-		[]bool{
-			true, true, true, true, true, true, true, true, false, false,
-			false, false, false, false, false, false,
+		"Reordered": {
+			128, 0x0000FFFFFFFFFFFF,
+			[]uint64{96, 64, 16, 80, 32, 48, 8, 24, 88, 40, 128, 56, 72, 112, 104, 120},
+			[]bool{
+				true, true, true, true, true, true, true, true, true, true,
+				true, true, true, true, true, true,
+			},
+			[]uint64{96, 64, 16, 80, 32, 48, 8, 24, 88, 40, 128, 56, 72, 112, 104, 120},
+			nil,
 		},
-		[]uint64{16, 32, 48, 64, 80, 96, 112, 128},
-	},
-	"ReplayedQuick": {
-		128, 0x0000FFFFFFFFFFFF,
-		[]uint64{16, 16, 32, 32, 48, 48, 64, 64, 80, 80, 96, 96, 112, 112, 128, 128},
-		[]bool{
-			true, false, true, false, true, false, true, false, true, false,
-			true, false, true, false, true, false,
+		"Old": {
+			100, 0x0000FFFFFFFFFFFF,
+			[]uint64{24, 32, 40, 48, 56, 64, 72, 80, 88, 96, 104, 112, 120, 128, 8, 16},
+			[]bool{
+				true, true, true, true, true, true, true, true, true, true,
+				true, true, true, true, true, true,
+			},
+			[]uint64{24, 32, 40, 48, 56, 64, 72, 80, 88, 96, 104, 112, 120, 128},
+			nil,
 		},
-		[]bool{
-			true, false, true, false, true, false, true, false, true, false,
-			true, false, true, false, true, false,
+		"ContinuousReplayed": {
+			8, 0x0000FFFFFFFFFFFF,
+			[]uint64{16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25},
+			[]bool{
+				true, true, true, true, true, true, true, true, true, true,
+				true, true, true, true, true, true, true, true, true, true,
+			},
+			[]uint64{16, 17, 18, 19, 20, 21, 22, 23, 24, 25},
+			nil,
 		},
-		[]uint64{16, 32, 48, 64, 80, 96, 112, 128},
-	},
-	"Strict": {
-		0, 0x0000FFFFFFFFFFFF,
-		[]uint64{1, 3, 2, 4, 5, 6, 7, 8, 9, 10},
-		[]bool{
-			true, true, false, true, true, true, true, true, true, true,
+		"ReplayedLater": {
+			128, 0x0000FFFFFFFFFFFF,
+			[]uint64{16, 32, 48, 64, 80, 96, 112, 128, 16, 32, 48, 64, 80, 96, 112, 128},
+			[]bool{
+				true, true, true, true, true, true, true, true, true, true,
+				true, true, true, true, true, true,
+			},
+			[]uint64{16, 32, 48, 64, 80, 96, 112, 128},
+			nil,
 		},
-		[]bool{
-			true, true, false, true, true, true, true, true, true, true,
+		"ReplayedQuick": {
+			128, 0x0000FFFFFFFFFFFF,
+			[]uint64{16, 16, 32, 32, 48, 48, 64, 64, 80, 80, 96, 96, 112, 112, 128, 128},
+			[]bool{
+				true, true, true, true, true, true, true, true, true, true,
+				true, true, true, true, true, true,
+			},
+			[]uint64{16, 32, 48, 64, 80, 96, 112, 128},
+			nil,
 		},
-		[]uint64{1, 3, 4, 5, 6, 7, 8, 9, 10},
-	},
-	"Overflow": {
-		128, 0x0000FFFFFFFFFFFF,
-		[]uint64{0x0000FFFFFFFFFFFE, 0x0000FFFFFFFFFFFF, 0x0001000000000000, 0x0001000000000001},
-		[]bool{
-			true, true, false, false,
+		"Strict": {
+			0, 0x0000FFFFFFFFFFFF,
+			[]uint64{1, 3, 2, 4, 5, 6, 7, 8, 9, 10},
+			[]bool{
+				true, true, true, true, true, true, true, true, true, true,
+			},
+			[]uint64{1, 3, 4, 5, 6, 7, 8, 9, 10},
+			nil,
 		},
-		[]bool{
-			true, true, false, false,
+		"Overflow": {
+			128, 0x0000FFFFFFFFFFFF,
+			[]uint64{0x0000FFFFFFFFFFFE, 0x0000FFFFFFFFFFFF, 0x0001000000000000, 0x0001000000000001},
+			[]bool{
+				true, true, true, true,
+			},
+			[]uint64{0x0000FFFFFFFFFFFE, 0x0000FFFFFFFFFFFF},
+			nil,
 		},
-		[]uint64{0x0000FFFFFFFFFFFE, 0x0000FFFFFFFFFFFF},
-	},
-}
-
-func TestReplayDetector(t *testing.T) {
-	for name, c := range commonCases {
-		c := c
-		t.Run(name, func(t *testing.T) {
-			det := New(c.windowSize, c.maxSeq)
-			var out []uint64
-			for i, seq := range c.input {
-				accept, ok := det.Check(seq)
-				if ok != c.valid[i] {
-					t.Errorf("Unexpected validity (%d):\nexpected: %v\ngot: %v", seq, c.valid[i], ok)
-				}
-				if ok {
-					out = append(out, seq)
-				}
-				if latest := accept(); latest != c.latest[i] {
-					t.Errorf("Unexpected sequence latest status (%d):\nexpected: %v\ngot: %v", seq, c.latest[i], latest)
-				}
-			}
-			if !reflect.DeepEqual(c.expected, out) {
-				t.Errorf("Wrong replay detection result:\nexpected: %v\ngot:      %v",
-					c.expected, out,
-				)
-			}
-		})
-	}
-}
-
-func TestReplayDetectorWrapped(t *testing.T) {
-	cases := map[string]testCase{
 		"WrapContinuous": {
 			64, 0xFFFF,
 			[]uint64{0xFFFC, 0xFFFD, 0xFFFE, 0xFFFF, 0x0000, 0x0001, 0x0002, 0x0003},
 			[]bool{
 				true, true, true, true, true, true, true, true,
 			},
-			[]bool{
-				true, true, true, true, true, true, true, true,
-			},
+			[]uint64{0xFFFC, 0xFFFD, 0xFFFE, 0xFFFF},
 			[]uint64{0xFFFC, 0xFFFD, 0xFFFE, 0xFFFF, 0x0000, 0x0001, 0x0002, 0x0003},
 		},
 		"WrapReordered": {
@@ -234,50 +160,57 @@ func TestReplayDetectorWrapped(t *testing.T) {
 			[]bool{
 				true, true, true, true, true, true, true, true,
 			},
-			[]bool{
-				true, false, true, false, false, false, false, true,
-			},
+			[]uint64{0xFFFD, 0xFFFC, 0xFFFE, 0xFFFF},
 			[]uint64{0xFFFD, 0xFFFC, 0x0002, 0xFFFE, 0x0000, 0x0001, 0xFFFF, 0x0003},
 		},
 		"WrapReorderedReplayed": {
 			64, 0xFFFF,
 			[]uint64{0xFFFD, 0xFFFC, 0xFFFC, 0x0002, 0xFFFE, 0xFFFC, 0x0000, 0x0001, 0x0001, 0xFFFF, 0x0001, 0x0003},
 			[]bool{
-				true, true, false, true, true, false, true, true, false, true, false, true,
-			},
-			[]bool{
-				true, false, false, true, false, false, false, false, false, false, false, true,
+				true, true, true, true, true, true, true, true, true, true, true, true,
 			},
+			[]uint64{0xFFFD, 0xFFFC, 0xFFFE, 0xFFFF},
 			[]uint64{0xFFFD, 0xFFFC, 0x0002, 0xFFFE, 0x0000, 0x0001, 0xFFFF, 0x0003},
 		},
 	}
-	for name, c := range commonCases {
-		if _, ok := cases[name]; ok {
-			t.Fatalf("Duplicate test case name: %q", name)
-		}
-		cases[name] = c
-	}
 	for name, c := range cases {
 		c := c
+		if c.expectedWrap == nil {
+			c.expectedWrap = c.expected
+		}
 		t.Run(name, func(t *testing.T) {
-			det := WithWrap(c.windowSize, c.maxSeq)
-			var out []uint64
-			for i, seq := range c.input {
-				accept, ok := det.Check(seq)
-				if ok != c.valid[i] {
-					t.Errorf("Unexpected validity (%d):\nexpected: %v\ngot: %v", seq, c.valid[i], ok)
-				}
-				if ok {
-					out = append(out, seq)
-				}
-				if latest := accept(); latest != c.latest[i] {
-					t.Errorf("Unexpected sequence latest status (%d):\nexpected: %v\ngot: %v", seq, c.latest[i], latest)
-				}
-			}
-			if !reflect.DeepEqual(c.expected, out) {
-				t.Errorf("Wrong replay detection result:\nexpected: %v\ngot:      %v",
-					c.expected, out,
-				)
+			for typeName, typ := range map[string]struct {
+				newFunc  func(uint, uint64) ReplayDetector
+				expected []uint64
+			}{
+				"NoWrap": {
+					newFunc:  New,
+					expected: c.expected,
+				},
+				"Wrap": {
+					newFunc:  WithWrap,
+					expected: c.expectedWrap,
+				},
+			} {
+				typ := typ
+				t.Run(typeName, func(t *testing.T) {
+					det := typ.newFunc(c.windowSize, c.maxSeq)
+					var out []uint64
+					for i, seq := range c.input {
+						accept, ok := det.Check(seq)
+						if ok {
+							if c.valid[i] {
+								out = append(out, seq)
+								accept()
+							}
+						}
+					}
+					if !reflect.DeepEqual(typ.expected, out) {
+						t.Errorf("Wrong replay detection result:\nexpected: %v\ngot:      %v",
+							typ.expected, out,
+						)
+					}
+				})
 			}
 		})
 	}