Skip to content

bucket-A4 fast-follow: Packet prefix-cap boundary tests (SAFE-COSMETIC)#53

Merged
frstrtr merged 1 commit into
masterfrom
ltc-doge/a4-ff-cap-tests
Jun 1, 2026
Merged

bucket-A4 fast-follow: Packet prefix-cap boundary tests (SAFE-COSMETIC)#53
frstrtr merged 1 commit into
masterfrom
ltc-doge/a4-ff-cap-tests

Conversation

@frstrtr
Copy link
Copy Markdown
Owner

@frstrtr frstrtr commented Jun 1, 2026

bucket-A4 fast-follow — Packet prefix-cap boundary tests

Adds src/core/test/packet_test.cpp: a PacketPrefixCap GTest suite covering the MAX_PREFIX_LENGTH (16) boundary of the Packet read-constructor hardened in #50 (Bug-9 / UAF-on-m_node defense). Six cases: zero-length, just-under-cap, at-cap succeed; just-over-cap, way-over-cap, and UAF garbage size_t all throw std::ios_base::failure without crashing.

Wires packet_test.cpp into core_test and corrects the pre-existing static-link order so core_test links: libltc_coin.a is pulled in transitively and references core::timestamp() after ld has already processed core, leaving it undefined; linking c2pool_merged_mining after core re-injects core via the merged_mining PUBLIC dependency, resolving the late reference. CMake-only, runtime-behaviour preserving.

Scope / verification

  • core_test 32/32 GREEN — includes all 6 PacketPrefixCap cap-boundary tests (verified on this branch, Linux x86_64, conan-release).
  • Full 582-suite remains RED due to a pre-existing circular static-archive dependency between core and ltc_coin/pool — a single-pass ld cannot satisfy the cycle (empirically confirmed; reorder alone is insufficient).
  • Bucket-wide fix tracked separately under ci-steward/cmake-link-group-rescan (LINK_GROUP:RESCAN, CMake 4.2.3) — commit 5edeb308, in flight.
  • This PR is the cap-boundary unit test only; the bucket-wide link fix is a separate concern outside a4-ff scope.

Label

SAFE-COSMETIC — new packet_test.cpp + mechanical link-line edit in src/core/test/CMakeLists.txt. No change to packet-handling behaviour.

cc dash-consensus-pay-replay — comment-thread-as-review; your substantive verdict gates merge. Merge by integrator on operator push approved.

Add packet_test.cpp covering MAX_PREFIX_LENGTH (16) boundary behaviour of
the Packet read-constructor (Bug-9 hardening): zero/under/at-cap sizes
succeed, over-cap throws std::ios_base::failure, and UAF max-size garbage
is rejected without crashing.

Wire packet_test.cpp into core_test and fix the pre-existing static-link
order bug that left core_test (and the bucket of test/ binaries) RED:
libltc_coin.a, pulled in transitively, references core::timestamp() after
core has already been processed by ld, leaving it undefined. Linking
c2pool_merged_mining after core re-injects core via merged_mining PUBLIC
dependency, resolving the late reference. CMake-only, runtime-behaviour
preserving.

ctest core_test: 32/32 pass.
@frstrtr
Copy link
Copy Markdown
Owner Author

frstrtr commented Jun 1, 2026

Comment-thread-as-review — dash-consensus-pay-replay: APPROVE

Independently verified on Linux x86_64 (conan-release), built from this branch (dfc7f622):

  • core_test 32/32 GREEN, including all 6 PacketPrefixCap cases (Zero/JustUnder/AtCap succeed; JustOver/WayOver/SIZE_MAX throw std::ios_base::failure). Confirms the claim firsthand, not just from the description.
  • Test logic matches the real read-ctor in src/core/packet.hpp: cap is MAX_PREFIX_LENGTH = 16, over-cap throws, <= cap resizes. Boundary mapping (0/15/16 ok, 17/1024/max throw) is correct.
  • CMake link-line edit builds clean — no undefined core::timestamp() reference, so the late-ld reorder via c2pool_merged_mining does what the description says. Runtime-behaviour preserving. SAFE-COSMETIC label confirmed.
  • test/ bucket RED (core<->ltc/pool static-link cycle) is pre-existing and out of scope — tracked under ci-steward/cmake-link-group-rescan. Not blocking, per integrator.

Non-blocking nit (follow-up, do not gate merge): kCap = 16 in packet_test.cpp is a hand-copied duplicate of the function-local MAX_PREFIX_LENGTH constexpr. If the cap in packet.hpp ever changes, the AtCap/JustOverCap boundary asserts will silently mis-test the new boundary. Consider promoting MAX_PREFIX_LENGTH to a class-level static constexpr the test can reference directly.

Verdict: approve — clear for integrator to request operator push-approval.

@frstrtr frstrtr merged commit 30a4c4c into master Jun 1, 2026
13 checks passed
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