address comments from code review

internal/model/packet.go

@@ -352,11 +352,6 @@ func (p *Packet) Log(logger Logger, direction int) {
 		return
 	}
 
-	payloadLen := 0
-	if p.Payload != nil {
-		payloadLen = len(p.Payload)
-	}
-
 	logger.Debugf(
 		"%s %s {id=%d, acks=%v} localID=%x remoteID=%x [%d bytes]",
 		dir,
@@ -365,6 +360,6 @@ func (p *Packet) Log(logger Logger, direction int) {
 		p.ACKs,
 		p.LocalSessionID,
 		p.RemoteSessionID,
-		payloadLen,
+		len(p.Payload),
 	)
 }

internal/reliabletransport/common_test.go

@@ -4,6 +4,7 @@ import (
 	"github.com/apex/log"
 	"github.com/ooni/minivpn/internal/bytesx"
 	"github.com/ooni/minivpn/internal/model"
+	"github.com/ooni/minivpn/internal/runtimex"
 	"github.com/ooni/minivpn/internal/session"
 	"github.com/ooni/minivpn/internal/workers"
 )
@@ -16,9 +17,7 @@ import (
 func initManagers() (*workers.Manager, *session.Manager) {
 	w := workers.NewManager(log.Log)
 	s, err := session.NewManager(log.Log)
-	if err != nil {
-		panic(err)
-	}
+	runtimex.PanicOnError(err, "cannot create session manager")
 	return w, s
 }
 

internal/reliabletransport/receiver.go

@@ -41,7 +41,7 @@ func (ws *workersState) moveUpWorker() {
 			// We should be able to deterministically test how this affects the state machine.
 
 			// drop a packet that is not for our session
-			if !bytes.Equal([]byte(packet.RemoteSessionID[:]), []byte(ws.sessionManager.LocalSessionID())) {
+			if !bytes.Equal(packet.RemoteSessionID[:], ws.sessionManager.LocalSessionID()) {
 				ws.logger.Warnf(
 					"%s: packet with invalid RemoteSessionID: expected %x; got %x",
 					workerName,

internal/runtimex/runtimex.go

@@ -1,6 +1,8 @@
 // Package runtimex contains [runtime] extensions.
 package runtimex
 
+import "fmt"
+
 // PanicIfFalse calls panic with the given message if the given statement is false.
 func PanicIfFalse(stmt bool, message interface{}) {
 	if !stmt {
@@ -17,3 +19,11 @@ func PanicIfTrue(stmt bool, message interface{}) {
 
 // Assert calls panic with the given message if the given statement is false.
 var Assert = PanicIfFalse
+
+// PanicOnError calls panic() if err is not nil. The type passed to panic
+// is an error type wrapping the original error.
+func PanicOnError(err error, message string) {
+	if err != nil {
+		panic(fmt.Errorf("%s: %w", message, err))
+	}
+}

internal/vpntest/packetio.go

@@ -143,15 +143,17 @@ type Witness struct {
 	reader *PacketReader
 }
 
+// NewWitness constructs a Witness from a [PacketReader].
 func NewWitness(r *PacketReader) *Witness {
 	return &Witness{r}
 }
 
+// Log returns the packet log from the internal reader this witness uses.
 func (w *Witness) Log() PacketLog {
 	return w.reader.Log()
 }
 
-// VerifyACKs tells the underlying reader to wait for a given number of acks,
+// VerifyNumberOfACKs tells the underlying reader to wait for a given number of acks,
 // returns true if we have the same number of acks.
 func (w *Witness) VerifyNumberOfACKs(start, total int, t time.Time) bool {
 	w.reader.WaitForNumberOfACKs(total, t)