DEV Community

Morgan Ma
Morgan Ma

Posted on

The Window Treated Wraparound as a Stale Packet

A sixteen-bit sequence window can reject good packets. Mine did that after a quiet wrap. The unit tests never crossed 65535, so they stayed green.

I reconstructed the failure in a local harness. I did not invent production metrics. Do you trust a window that never wrapped in CI?

The core miss

Packets arrived in order on a ring counter. The decoder looked healthy in traces. The log still shouted stale_seq and dropped them.

Last accepted sequence sat near 65500. New sequences arrived as 20, 21, 22. Why would twenty look older than sixty-five thousand?

Because I compared them as plain integers. I did not compare them as a ring.

Where the draft came from

I wanted a tiny deduper for binary telemetry frames. I asked a coding model for a first helper. I used MonkeyCode for that draft only. Disclosure: This article was prepared as part of MonkeyCode's product outreach. Free model access and the free server option were enough for a stub. They were not enough for a wrap proof.

Labeled example. This is the broken check I compiled.

// Broken: treats wrap as a jump into the past.
bool is_fresh(uint16_t seq, uint16_t last) {
    return seq > last;
}
Enter fullscreen mode Exit fullscreen mode

Looks honest, right? Tests used seq = 1, 2, 3, 4. Every assert passed. I never asked for a wrap fixture. That miss was mine.

Symptom to evidence

I did not start with protocol theory. I started with printed numbers.

  1. Print seq and last as decimal and hex.
  2. Print int16_t(seq - last) as a signed delta.
  3. Dump the last sixteen accepted values.
  4. Replay the same pair against a wrap fixture.
  5. Fail the build if the ring delta disagrees.

The signed delta told the truth fast. seq - last was 20 minus 65500. That is a large negative. The ring delta is +56. See the gap?

A wrap fixture you can compile

Do not argue with a log line. Make the wrap fail in one file.

#include <cassert>
#include <cstdint>
#include <iostream>

bool is_fresh_broken(uint16_t seq, uint16_t last) {
    return seq > last;
}

// Ring check: forward distance must stay in the lower half.
bool is_fresh_ring(uint16_t seq, uint16_t last) {
    uint16_t delta = static_cast<uint16_t>(seq - last);
    return delta != 0 && delta < 32768u;
}

int main() {
    uint16_t last = 65500;
    uint16_t seq = 20;

    std::cout << "broken " << is_fresh_broken(seq, last) << "\n";
    std::cout << "ring   " << is_fresh_ring(seq, last) << "\n";
    std::cout << "delta  " << static_cast<uint16_t>(seq - last) << "\n";

    assert(!is_fresh_broken(seq, last));
    assert(is_fresh_ring(seq, last));

    last = 20;
    assert(!is_fresh_ring(20, last));  // duplicate
    assert(!is_fresh_ring(10, last));  // behind on the ring
    assert(is_fresh_ring(21, last));   // next value

    std::cout << "wrap fixture ok\n";
}
Enter fullscreen mode Exit fullscreen mode

Keep the build boring. One translation unit. No extra flags.

g++ -std=c++17 -Wall -Wextra -Wconversion -o wrap_seq wrap_seq.cpp
./wrap_seq
Enter fullscreen mode Exit fullscreen mode

Broken returns false. Ring returns true. That is the whole bug.

What the green tests hid

The original tests were not wrong on their inputs. They were incomplete on the type. uint16_t is a ring. A short increasing list is not a ring.

I added three cases after the first fail. Each case is one assert. That is the whole suite I needed.

  1. Monotone increase far from the modulus.
  2. Duplicate of last must drop.
  3. Forward wrap, such as 65500 then 20.
  4. Backward wrap, such as 20 then 10.

Did the model propose case three? Not in my draft. Would I have noticed without printing hex? Probably not.

Decision table for the next review

Use this table before you keep a comparison. Fill it with your real width.

Pair (last -> seq) Integer > Ring delta Keep?
10 -> 11 true 1 yes
10 -> 10 false 0 no
10 -> 9 false 65535 no
65500 -> 20 false 56 yes
20 -> 65500 true 65480 no

The third and fourth rows are the trap. Integer order and ring order disagree there. If your tests never hit those rows, your tests are theatre.

Numbered debug workflow I reuse

This is the reusable part. The product is incidental.

  1. Freeze one failing pair of numbers from logs.
  2. Write those two numbers into a unit test first.
  3. Print unsigned values, hex values, and the modular delta.
  4. Classify the pair with the table above.
  5. Only then change the comparison.
  6. Re-run the wrap fixture under -Wconversion.

Why print hex? Because 65500 does not look like 0xffdc in your head. The hex makes the modulus obvious. Why freeze the pair first? Because a moving capture will gaslight you.

The fix I actually kept

I did not switch to int and pray. I did not widen the field on the wire. I subtracted in the unsigned domain and tested the half range.

bool is_fresh_ring(uint16_t seq, uint16_t last) {
    uint16_t delta = static_cast<uint16_t>(seq - last);
    return delta != 0 && delta < 32768u;
}
Enter fullscreen mode Exit fullscreen mode

Half range is a choice, not a law. It assumes reordering stays below 32768. My stream is in-order telemetry. That assumption holds here. It will not hold for a lossy UDP mesh. Know your reorder bound.

I also stopped treating “model wrote a test” as coverage. A generated test that never wraps is a comment. Compile it. Then mutate the counter.

Command notes for the same harness

Run the fixture under sanitizers once. Do not skip that on a “pure logic” file.

g++ -std=c++17 -fsanitize=address,undefined -o wrap_seq wrap_seq.cpp
./wrap_seq
Enter fullscreen mode Exit fullscreen mode

Then force the wrap in a tiny loop. Watch the first failure index.

uint16_t last = 0;
for (uint32_t i = 1; i < 70000; ++i) {
    uint16_t seq = static_cast<uint16_t>(i);
    if (!is_fresh_ring(seq, last) && seq != last) {
        std::cerr << "fail at i=" << i << "\n";
        break;
    }
    last = seq;
}
Enter fullscreen mode Exit fullscreen mode

If is_fresh_broken sits in that loop, it dies at 65536. That number should have been in the first review. It was not.

Limitations

This write-up is a comparison postmortem. It is not a protocol design guide. Sixteen-bit sequence numbers are a constraint I inherited. They are not a recommendation.

The half-range test drops far-future jumps. That is intended for my in-order path. It is wrong if you must accept large gaps. It is also wrong if duplicates can arrive after a full half window.

I did not benchmark throughput. I did not claim a latency win. I only claimed the broken integer compare fails a wrap fixture. Sanitizers will not catch this by themselves. The values are defined unsigned arithmetic. You still need the fixture.

Free model drafts will happily emit seq > last. They pattern-match textbook loops. They do not owe you modular arithmetic. If you paste the helper and ship, you own the wrap.

Who should not use this approach

Do not copy this check into crypto windows. Do not copy it into authenticated replay defenses. Those need a spec, not a blog delta.

Skip this if your sequence field is already 32 or 64 bits and cannot wrap in the process lifetime. Skip this if a library already implements RFC-style serial number comparison and you can call it. Skip this if you cannot print the failing pair. Guessing without numbers is how I wasted the first hour.

Also skip the “generate tests and relax” loop. If the model never saw 65535, the suite cannot save you. You still write the wrap row.

What I keep from the mess

Short counters lie at the modulus. Green tests can lie with them. Print the modular delta before you rewrite the helper.

I still use a free model for boilerplate. I do not use it as evidence. The evidence is the fixture above. Steal that fixture if you maintain a similar window. The rest is optional.

Top comments (0)