[Adversarial review] Core Endpoint membership, probing, scheduling, and event correctness

[al8n]

Defensive review: six new core memberlist-proto::Endpoint correctness defects

Scope and methodology

Reviewed exactly aeef956bc0bdf0ed21ba386b329dab6aae64b47e, limited to the standard memberlist-proto::Endpoint membership machine and directly owned state.

I reconstructed membership transitions, probe and suspicion FSMs, scheduler ordering, merge behavior, event publication, broadcast ownership, lifecycle gates, admission limits, and local arithmetic/resource bounds. Stream/QUIC endpoints, drivers, codecs, and transport behavior were excluded except where needed to evaluate an explicit Endpoint contract.

The invocation ran exactly gpt-5.6-sol with model_reasoning_effort=max. No recursive Codex invocation or other model was used.

I inspected and excluded duplicates from #157, #158, #162, #163, and #164.

Severity summary

Severity	Count
Critical	0
High	0
Medium	4
Low	2

Confirmed findings

Medium — EP-BCAST-GC-001: dead-member GC orphans membership broadcasts and defeats max_members

Sources: max_members promises to bound the broadcast queue; reset_nodes removes only membership entries; BroadcastQueue::prune is unwired and assumes the queue is member-bounded; broadcasts are keyed by member ID; queue length is publicly observable.

Minimal trigger:

1. Configure max_members = Some(2) with the local member occupying one slot and no usable gossip target, for example with gossip disabled.
2. Admit X through Alive.
3. Apply an ordinary Dead(X, from != X) and advance beyond gossip_to_the_dead_time.
4. reset_nodes removes X, freeing the membership slot, but its keyed Dead broadcast remains.
5. Repeat with X2, X3, and so on.

num_members() remains at or below two while broadcast_queue_len() grows by one per reclaimed identity.

Violated invariant: The documented admission ceiling must bound every per-member structure, including the broadcast queue. Queue ownership must end when the owning member is reclaimed.

Impact/reachability: An unauthenticated peer can cause unbounded retained memory over endpoint lifetime despite max_members. The trigger is rate-limited by tombstone GC, but there is no ultimate bound. With unsendable entries and active gossip, stale entries can also increase repeated queue-scanning work. This is distinct from the aggregate push/pull and output-resource findings in #158/#162.

Remediation direction: Purge membership broadcasts by ID during reset_nodes, or move reclaimed tombstones into separately bounded accounting. Do not rely on tracked membership count as an implicit queue bound.

Missing regression: Repeatedly admit, deaden, and reclaim distinct IDs under max_members = 2; assert that old IDs leave the queue and its length remains bounded. Include disabled-gossip and unsendable-entry variants.

---

Medium — EP-PROBE-MTU-002: an admitted long peer ID makes mandatory probe packets exceed gossip_mtu

Sources: gossip_mtu is the maximum plaintext outbound datagram size; construction validates only messages formed from the local identity; the code recognizes that peer IDs are unbounded; a lone Ping is emitted without measuring it, while only the compound case is size-gated.

Minimal trigger:

1. Construct an Endpoint with a small local ID and gossip_mtu = 1400; initialization succeeds.
2. Merge one valid Alive member whose ID is roughly 1.5 KiB through the reliable push/pull path.
3. Start a periodic probe of that member.
4. Endpoint emits a single Transmit::Packet(Ping) whose encoded plaintext is larger than 1400 bytes.

If the caller enforces the advertised cap, or the network drops the oversized datagram, the local sizing failure progresses through the normal probe timeout and can suspect a healthy member.

Violated invariant: Every Endpoint datagram must respect gossip_mtu; failure evidence must represent peer nonresponse, not a locally unsendable control message.

Impact/reachability: A remotely chosen, otherwise valid member ID can deterministically violate the public output contract and cause false failure detection when reliable fallback is disabled or unavailable. This is a single mandatory core probe after admission, not the oversized aggregate push/pull output already reported in #158.

Remediation direction: Before admitting a member, ensure all mandatory Ping/IndirectPing/Suspect forms involving its identity fit, or represent it explicitly as UDP-unprobeable and use a defined reliable-only path. Independently enforce the MTU on every Packet output.

Missing regression: Merge a member whose individual Ping exceeds the MTU, then assert that Endpoint emits no over-cap Packet and does not convert local oversizing into Suspect.

---

Medium — EP-LHA-SCHED-003: awareness-scaled probes overlap because scheduling remains at the unscaled interval

Sources: Awareness is documented as slowing the node’s probing rate; each detection probe receives an awareness-scaled failure deadline; the scheduler nevertheless starts another probe every base interval without a single-flight guard. The canonical implementation performs probe work synchronously, so an overrun does not create another concurrent round (HashiCorp v0.5.2).

Minimal trigger: With health score 1, probe interval P, and probe timeout below P, a probe started at t0 has failure deadline t0 + 2P. At t0 + P, the scheduler starts another Detection probe while the first remains active and may already own indirect/reliable fallback work.

At score s, as many as s + 1 scheduled rounds coexist. The default awareness ceiling permits eight; the ceiling is configurable.

Violated invariant: Local health awareness should reduce and serialize failure-detection work while the local node is degraded.

Impact/reachability: During CPU or network degradation, Endpoint retains multiple probe/ACK/fallback states and performs up to s + 1 times the intended degraded probe work. This creates positive feedback precisely when Lifeguard is meant to reduce false suspicions and local load.

Remediation direction: Keep application pings independent, but make periodic Detection rounds single-flight and prevent the next round from becoming eligible before the awareness-scaled period/previous terminalization. Preserve missed-tick rather than catch-up semantics.

Missing regression: Raise the health score, fire the base scheduler tick before the scaled deadline, and assert that no second Detection probe or second fallback wave starts before the first terminates.

---

Medium — EP-PING-BIND-004: ping(Node X@B) transmits to B but validates ACKs against stored X@A

Sources: ping reuses membership state by ID while transmitting to the caller-supplied address; ACK source validation consults the stored probe target; timeout reports that same stored target.

Minimal trigger:

1. Membership contains X@A.
2. Call the public ping(Node::new(X, B), now).
3. Endpoint sends the Ping to B, but stores X@A in the probe.
4. A timely ACK from B is rejected because the expected source is A.
5. Timeout emits PingFailed identifying X@A, not the endpoint actually contacted.

Violated invariant: The expected responder, emitted terminal event, and endpoint actually contacted must describe the same address generation.

Impact/reachability: Directed pings to an alternate claimant or changed address deterministically fail and produce misleading diagnostics. This exists at probe creation and requires neither expiration nor member replacement, so it is distinct from #158’s expired-probe generation finding.

Remediation direction: Snapshot the caller-supplied ID and address as the probe target and correlation address. Reuse stored member metadata only when both ID and address match.

Missing regression: With X@A known, ping X@B; assert that B is accepted, A is rejected, and exactly one terminal event identifies X@B.

---

Low — EP-EVENT-VSN-005: version-only Alive updates emit no NodeUpdated

Sources: NodeUpdated covers metadata/version changes; Alive processing snapshots only old metadata, installs new protocol/delegate versions, but emits NodeUpdated only when Meta differs.

Minimal trigger: Admit X at incarnation 1 with versions V1/V1. Process incarnation 2 with the same address and metadata but a changed protocol or delegate version. member(X) and the published snapshot change, but poll_event yields no NodeUpdated.

Violated invariant: Event-driven membership mirrors must receive every change documented as a NodeUpdated.

Impact/reachability: Event-only consumers retain stale protocol/delegate capability information until another event or full snapshot reconciliation.

Remediation direction: Compare old protocol and delegate versions alongside metadata, or compare the complete event-relevant server tuple.

Missing regression: Higher-incarnation protocol-only and delegate-only updates should each emit exactly one NodeUpdated.

---

Low — EP-SUSP-PRECISION-006: sub-millisecond probe intervals collapse suspicion timeouts to zero

Sources: suspicion_timeouts truncates probe_interval through as_millis() before scaling; the builder imposes no minimum precision; confirmation rescaling also quantizes to milliseconds. HashiCorp’s corresponding calculation multiplies the full Duration rather than first truncating the interval (v0.5.2).

Minimal trigger: Configure probe_interval = 500µs and suspicion_mult = 4. as_millis() returns zero, making both suspicion bounds zero. A valid Suspect transition therefore receives deadline now; the next same-timestamp timeout pass can mark the member Dead. The intended minimum is 2 ms before cluster-size scaling.

Violated invariant: A valid nonzero Duration must not become an immediate timeout solely because of unit conversion.

Impact/reachability: Valid low-latency configurations can turn suspicion into immediate false death. Nonintegral-millisecond intervals are also systematically shortened.

Remediation direction: Scale checked, full-precision Durations throughout, including confirmation rescaling, or reject sub-millisecond configuration explicitly.

Missing regression: Cover 500µs and 1.5ms probe intervals and assert nonzero, precision-preserving suspicion bounds and deadlines.

Architecture observations

• NodeState.state is intentionally fixed at insertion; current liveness belongs to LocalNodeState and member_liveness() (documented distinction). Consequently, a NodeLeft payload whose embedded wire state remains Alive is not itself a defect; the event variant communicates the transition.
• Dead/Left GC is the ultimate identity-tombstone lifetime. dead_node_reclaim_time = 0 disables early different-address reclaim while the tombstone exists; it does not retain that identity forever.
• Push/pull entries are folded sequentially rather than normalized by ID. Honest Endpoint snapshots contain unique IDs, and malicious duplicate ordering did not provide a new capability beyond sending the same transitions sequentially.
• Membership identity is ID-based while ACK/NACK eligibility is address-based. This is an explicit non-Byzantine boundary; EP-PING-BIND-004 is different because Endpoint internally chooses two contradictory addresses for one operation.

Rejected or narrowed candidates

• All findings already present in [Adversarial review] Correctness, lifecycle, and architecture findings across proto/compio/reactor #157/Adversarial review: standard Endpoint, StreamEndpoint, and QuicEndpoint #158/[Adversarial review] Cross-driver contracts and dependency-aware remediation #162 were excluded, including maximum-incarnation refutation, non-monotonic failure evidence, retransmit multiplication overflow, self-leave takeover, replacement-generation probes, oversized push/pull output, and driver/output lifecycle behavior.
• Lower/equal/higher transition permutations produced no additional contradictory state, duplicate membership event, or resurrection path beyond those known findings.
• ACK/NACK lateness and exact-deadline ordering are consistent: terminal paths remove both probe and ACK registry state, preventing success/failure double terminalization.
• Sequence allocation skips live u32 slots. Collision requires retaining essentially the complete sequence space and did not survive as a practical independent finding.
• Suspicion confirmations are deduplicated by confirmer ID, exclude the original suspector, and stop at k. Arbitrary claimed identities require a stronger authentication model and did not yield a distinct core defect under the present trust model.
• Remote push/pull Left is deliberately converted to ordinary Dead, while Dead/Suspect becomes suspicion; no new immediate address-reclaim path survived.
• Leaving/Left gates stop core timers, admission, and new protocol work. Cross-layer delivery completion remains covered by [Adversarial review] Cross-driver contracts and dependency-aware remediation #162.
• Membership broadcasts correctly supersede older broadcasts for the same still-tracked ID. The defect is specifically ownership after member GC.
• suspicion_max_timeout_mult = 0, astronomical Duration overflow, and saturated Instant behavior were narrowed to configuration-validation debt without a credible independent remote trigger.
• Push/pull scheduler overlap under intervals shorter than stream timeout was not reported separately because its meaningful consequences overlap the known output/resource findings.

Commands and test results

git rev-parse HEAD
aeef956bc0bdf0ed21ba386b329dab6aae64b47e

git diff --quiet aeef956bc0bdf0ed21ba386b329dab6aae64b47e -- memberlist-proto
exit 0

gh issue view 157 158 162 163 164 --repo al8n/memberlist ...
all issue bodies inspected

cargo test -p memberlist-proto --lib -- --list
failed before compilation:
failed to open /Users/al/.cargo/target/debug/.cargo-build-lock:
Operation not permitted

/Users/al/.cargo/target/debug/deps/memberlist_proto-bce9dbea35d88132 --test-threads=1
723 passed; 0 failed

Focused existing controls also passed, including membership metadata change, tombstone reaping, max-members admission, incarnation-only snapshot publication, MTU broadcast rescue, buddy-probe splitting, application ping success, scheduled probing, and scaled detection deadlines.

No repository files were changed. A pre-existing untracked .codex-endpoint-review.md was left untouched.

Confidence and limitations

Confidence is high in the six source-level mechanisms and public-contract violations. The false-suspicion consequence of EP-PROBE-MTU-002 depends on the caller/network rejecting or losing the oversized packet, although the Endpoint MTU violation itself is unconditional.

A fresh exact-commit build was blocked by the read-only Cargo target lock. The full passing run used the pre-existing current-worktree test binary, which postdated the reviewed sources, so it is supporting evidence rather than a freshly built artifact. No new regression tests were added because the review contract prohibited repository edits.