Skip to content

test(sticktable): regression test for EntryUpdate.Marshal data encoding#30

Merged
fionera merged 1 commit into
DropMorePackets:masterfrom
fionera:test/entryupdate-marshal-regression
Jul 10, 2026
Merged

test(sticktable): regression test for EntryUpdate.Marshal data encoding#30
fionera merged 1 commit into
DropMorePackets:masterfrom
fionera:test/entryupdate-marshal-regression

Conversation

@fionera

@fionera fionera commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

What

Adds a regression test for EntryUpdate.Marshal's data encoding.

Why

EntryUpdate.Marshal used to encode the data section by calling MapData.Unmarshal instead of Marshal:

for _, data := range e.Data {
    n, err := data.Unmarshal(b[offset:]) // read, not write
    ...
}

That had two effects: the data section was written as zeros on the wire, and the source EntryUpdate's data was mutated to those zeros as a side effect. The code itself was fixed in #28 (write support) — this PR closes the test gap so it can't silently come back.

The existing TestMarshalUnmarshalMessages cannot catch this class of bug: it marshals entryUpdate, unmarshals into d, then compares *entryUpdate against d. Because the buggy Marshal mutates entryUpdate to zeros too, both sides match and the test passes even when the data is not encoded (verified by re-introducing the bug on the current tree — the existing test stays green, this new test fails with Marshal mutated source data to 0).

What this test does

  • Marshals an EntryUpdate with a non-zero gpt0 value.
  • Asserts the source data is not mutated by Marshal.
  • Unmarshals into an independent struct and asserts the value round-trips.

No production code changes.

Context: I hit the underlying bug while building a peers sender on top of this library against v0.0.7; the fix is on master but not yet in a tagged release (v0.0.8 still ships data.Unmarshal). A release including it would be very welcome.

EntryUpdate.Marshal previously encoded the data section by calling MapData.Unmarshal instead of Marshal, producing a zeroed data section on the wire and mutating the source EntryUpdate as a side effect. The existing round-trip test could not catch this because it compares against the same struct that Marshal mutates, so both sides ended up zeroed and matched.

This adds a test that uses an independent expected value and asserts the source is not mutated, so it fails if the data is not actually encoded.
@fionera
fionera merged commit f596855 into DropMorePackets:master Jul 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant