simplify how the largest acked is passed to the ackhandler

internal/ackhandler/packet.go

@@ -11,14 +11,12 @@ import (
 type Packet struct {
 	PacketNumber    protocol.PacketNumber
 	PacketType      protocol.PacketType
-	Ack             *wire.AckFrame
 	Frames          []wire.Frame
+	LargestAcked    protocol.PacketNumber // InvalidPacketNumber if the packet doesn't contain an ACK
 	Length          protocol.ByteCount
 	EncryptionLevel protocol.EncryptionLevel
 	SendTime        time.Time
 
-	largestAcked protocol.PacketNumber // if the packet contains an ACK, the LargestAcked value of that ACK
-
 	// There are two reasons why a packet cannot be retransmitted:
 	// * it was already retransmitted
 	// * this packet is a retransmission, and we already received an ACK for the original packet

internal/ackhandler/sent_packet_handler.go

@@ -172,13 +172,6 @@ func (h *sentPacketHandler) sentPacketImpl(packet *Packet) bool /* is ack-elicit
 	}
 
 	pnSpace.largestSent = packet.PacketNumber
-
-	packet.largestAcked = protocol.InvalidPacketNumber
-	if packet.Ack != nil {
-		packet.largestAcked = packet.Ack.LargestAcked()
-	}
-	packet.Ack = nil // no need to save the ACK
-
 	isAckEliciting := len(packet.Frames) > 0
 
 	if isAckEliciting {
@@ -237,8 +230,8 @@ func (h *sentPacketHandler) ReceivedAck(ackFrame *wire.AckFrame, withPacketNumbe
 
 	priorInFlight := h.bytesInFlight
 	for _, p := range ackedPackets {
-		if p.largestAcked != protocol.InvalidPacketNumber && encLevel == protocol.Encryption1RTT {
-			h.lowestNotConfirmedAcked = utils.MaxPacketNumber(h.lowestNotConfirmedAcked, p.largestAcked+1)
+		if p.LargestAcked != protocol.InvalidPacketNumber && encLevel == protocol.Encryption1RTT {
+			h.lowestNotConfirmedAcked = utils.MaxPacketNumber(h.lowestNotConfirmedAcked, p.LargestAcked+1)
 		}
 		if err := h.onPacketAcked(p, rcvTime); err != nil {
 			return err

internal/ackhandler/sent_packet_handler_test.go

@@ -31,7 +31,7 @@ func ackElicitingPacket(p *Packet) *Packet {
 func nonAckElicitingPacket(p *Packet) *Packet {
 	p = ackElicitingPacket(p)
 	p.Frames = nil
-	p.Ack = &wire.AckFrame{AckRanges: []wire.AckRange{{Smallest: 1, Largest: 1}}}
+	p.LargestAcked = 1
 	return p
 }
 
@@ -332,11 +332,9 @@ var _ = Describe("SentPacketHandler", func() {
 
 		Context("determining which ACKs we have received an ACK for", func() {
 			BeforeEach(func() {
-				ack1 := &wire.AckFrame{AckRanges: []wire.AckRange{{Smallest: 80, Largest: 100}}}
-				ack2 := &wire.AckFrame{AckRanges: []wire.AckRange{{Smallest: 50, Largest: 200}}}
 				morePackets := []*Packet{
-					{PacketNumber: 13, Ack: ack1, Frames: []wire.Frame{&streamFrame}, Length: 1, EncryptionLevel: protocol.Encryption1RTT},
-					{PacketNumber: 14, Ack: ack2, Frames: []wire.Frame{&streamFrame}, Length: 1, EncryptionLevel: protocol.Encryption1RTT},
+					{PacketNumber: 13, LargestAcked: 100, Frames: []wire.Frame{&streamFrame}, Length: 1, EncryptionLevel: protocol.Encryption1RTT},
+					{PacketNumber: 14, LargestAcked: 200, Frames: []wire.Frame{&streamFrame}, Length: 1, EncryptionLevel: protocol.Encryption1RTT},
 					{PacketNumber: 15, Frames: []wire.Frame{&streamFrame}, Length: 1, EncryptionLevel: protocol.Encryption1RTT},
 				}
 				for _, packet := range morePackets {

internal/protocol/packet_number.go

@@ -5,7 +5,7 @@ type PacketNumber int64
 
 // InvalidPacketNumber is a packet number that is never sent.
 // In QUIC, 0 is a valid packet number.
-const InvalidPacketNumber = -1
+const InvalidPacketNumber PacketNumber = -1
 
 // PacketNumberLen is the length of the packet number in bytes
 type PacketNumberLen uint8

packet_packer.go

@@ -63,10 +63,14 @@ func (p *packedPacket) IsAckEliciting() bool {
 }
 
 func (p *packedPacket) ToAckHandlerPacket() *ackhandler.Packet {
+	largestAcked := protocol.InvalidPacketNumber
+	if p.ack != nil {
+		largestAcked = p.ack.LargestAcked()
+	}
 	return &ackhandler.Packet{
 		PacketNumber:    p.header.PacketNumber,
 		PacketType:      p.header.Type,
-		Ack:             p.ack,
+		LargestAcked:    largestAcked,
 		Frames:          p.frames,
 		Length:          protocol.ByteCount(len(p.raw)),
 		EncryptionLevel: p.EncryptionLevel(),

packet_packer_test.go

@@ -5,6 +5,7 @@ import (
 	"errors"
 	"math/rand"
 	"net"
+	"time"
 
 	"github.com/golang/mock/gomock"
 	"github.com/lucas-clemente/quic-go/internal/ackhandler"
@@ -969,3 +970,29 @@ var _ = Describe("Packet packer", func() {
 		})
 	})
 })
+
+var _ = Describe("Converting to AckHandler packets", func() {
+	It("convert a packet", func() {
+		packet := &packedPacket{
+			header: &wire.ExtendedHeader{Header: wire.Header{}},
+			frames: []wire.Frame{&wire.MaxDataFrame{}, &wire.PingFrame{}},
+			ack:    &wire.AckFrame{AckRanges: []wire.AckRange{{Largest: 100, Smallest: 80}}},
+			raw:    []byte("foobar"),
+		}
+		p := packet.ToAckHandlerPacket()
+		Expect(p.Length).To(Equal(protocol.ByteCount(6)))
+		Expect(p.Frames).To(Equal(packet.frames))
+		Expect(p.LargestAcked).To(Equal(protocol.PacketNumber(100)))
+		Expect(p.SendTime).To(BeTemporally("~", time.Now(), 50*time.Millisecond))
+	})
+
+	It("sets the LargestAcked to invalid, if the packet doesn't have an ACK frame", func() {
+		packet := &packedPacket{
+			header: &wire.ExtendedHeader{Header: wire.Header{}},
+			frames: []wire.Frame{&wire.MaxDataFrame{}, &wire.PingFrame{}},
+			raw:    []byte("foobar"),
+		}
+		p := packet.ToAckHandlerPacket()
+		Expect(p.LargestAcked).To(Equal(protocol.InvalidPacketNumber))
+	})
+})