Skip to content

feat(deps)!: delete the vendored chia-protocol and chia-sdk-client forks - #63

Merged
MichaelTaylor3d merged 38 commits into
mainfrom
feat/adopt-dpp-040-delete-chia-fork
Aug 12, 2026
Merged

feat(deps)!: delete the vendored chia-protocol and chia-sdk-client forks#63
MichaelTaylor3d merged 38 commits into
mainfrom
feat/adopt-dpp-040-delete-chia-fork

Conversation

@MichaelTaylor3d

@MichaelTaylor3d MichaelTaylor3d commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

DO NOT MERGE — BLOCKED on a gating regression. The triple gate returned CHANGES-REQUIRED (correctness) and REFUTED (adversarial): deleting the fork removes an opcode carve-out whose absence makes two peers tear each other's link down on the first keepalive. Fix shape is being settled; this PR stays a draft until it lands and the gates re-run.

Closes DIG-Network/dig_ecosystem#2228.
Closes DIG-Network/dig_ecosystem#2767.

Does not close DIG-Network/dig_ecosystem#2225: that ticket is that a TLS-disabled feature configuration does not compile, which remains true (it already failed on main). This PR only deletes the false claim that it stayed coherent, plus the unreachable stubs serving that claim.

What this does

Deletes the vendored chia-protocol and chia-sdk-client forks and moves dig-gossip's transport onto dig_peer_protocol::DigLink. 89 files, +1031/−7757.

An earlier revision of this PR reported the deletion as blocked. That finding has been superseded: the blocker it identified was real, and the transport migration it prescribed is what this PR now contains.

Why the forks existed

Not a compile error — a runtime decode. Every inbound frame went through vendor/chia-sdk-client/src/peer.rs:446, Message::from_bytes(&binary)?. Upstream chia_protocol::ProtocolMessageTypes is a closed #[repr(u8)] enum (not #[non_exhaustive]) that stops at RespondCostInfo = 107, so without the fork that call rejects every DIG opcode 200–222 before dig-gossip's own code is reached. Message's fields are private under #[streamable(no_serde)], so no local wrapper could paper over it.

DigLink is the upstream-intended replacement and retires both vendored crates rather than one: it frames DigMessage, whose msg_type is a raw u8, and supplies from_server_websocket and send_message — two of the three reasons the chia-sdk-client fork existed. The third (inbound RequestPeers routed to the application channel rather than the outbound RequestMap) was originally described here as "structurally moot". That was wrong, and two independent gates have now falsified it. DigLink::read_inbound matches on id FIRST (dig-peer-protocol-0.5.0/src/link.rs:470-478), so a colliding id is delivered to our waiter and never reaches the receiver. Both RequestMaps start next_id at 0 (request_map.rs:45) and both peers fire the first keepalive probe simultaneously, so the first probes collide on id = 0 and the two peers disconnect each other. See the blocking issue linked below.

Evidence the fork is actually gone

Not inferred from the build succeeding — [patch.crates-io] substitutes only where the patched version satisfies the requirement, and a mismatch drops the patch silently with exit 0. So this is read from the lock file and the vendor tree directly:

Check Result
[patch.crates-io] native-tls only — no chia entry
Every chia-* in Cargo.lock source = "registry+https://github.com/rust-lang/crates.io-index"; zero path/vendor sources
vendor/ tree vendor/native-tls only
git = deps none

CI corroborates independently: it downloads chia-protocol v0.26.0 and chia-sdk-client v0.28.0 from crates.io and compiles only vendor/native-tls locally.

vendor/native-tls stays and is untouched. It is security-load-bearing: it sets CERT_REQUIRED + Chia CA trust for inbound mTLS (CON-009), and upstream TlsAcceptorBuilder exposes no way to require a client certificate.

The wire did NOT move

dig-relay vendors this wire byte-identically (GPL-2.0), so this is a cross-repo contract and needed proof rather than reasoning.

The golden vectors were pinned in 5119249 before any refactor, then edited in ac9fdb8 — which is exactly the shape in which a wire break ships green. Every expected literal was therefore extracted from both revisions and compared:

BEFORE: 9 literals   AFTER: 9 literals   diff: (empty)

All nine unchanged. Only the construction changed: Message { .. }DigMessage::new, and to_bytes() became infallible so the .expect() calls were dropped.

No coordinated network change and no dig-relay update is implied by this PR.

OpcodeRateLimits — the decision this PR was stalled on

OpcodeRateLimits's fields and its from_chia are private, and Default (derived from V2_RATE_LIMITS) is its only public constructor — so tests/con_005_tests.rs cannot build a small table to exhaust cheaply.

Settled by adding the public conversion to dig-peer-protocol (release-first: DIG-Network/dig-peer-protocol#7 → 0.5.0 → publish → re-pin here), rather than rewriting the tests against Default or deleting them.

  • Rewriting against Default + limit_factor would pin each test to upstream Chia's exact numbers, so an upstream table change would silently alter what they measure — and the frequency = 1/2 cases become unreachable.
  • Deleting them costs five live properties of the Chia bound that this crate's InboundRateLimiter composes and depends on: per-connection counter independence, the frequency cap, the per-message max_size cap, limit_factor scaling, and window reset. None is fork-specific, so none is dead surface.
  • from_chia(&RateLimits) already exists and does the work; exposing it as a From is additive and preserves the "derive, never copy" property the module's own lockstep note depends on.

Known follow-ups

  • crates.io publish stays blocked, for a different reason than the workflow states. publish.yml is guarded off citing three vendored forks; two are now gone, but cargo publish strips [patch.crates-io], so a published dig-gossip would build against upstream native-tls and silently accept inbound peers presenting no client certificate. Tracked as dig_ecosystem#2647; the guard must stay.
  • #2391 frame tolerance (PR fix(connection): adopt DigLink so one bad frame cannot tear down a peer link #60) rebases onto this and becomes the acceptance test for the new transport. The vulnerable receive loop is deleted here, but "deleted" and "the replacement is tolerant" are different claims.

Bump

0.22.20.23.0. Breaking (the transport type changes on the public surface); on a 0.x crate the breaking bump is the MINOR, so consumers must re-pin.


Round 3 — #2767 keepalive collision, #2228 arm classification, #2225 false claim

1. The keepalive probe no longer parks a correlation waiter (dig_ecosystem#2767) — GATING

Withdrawal: the earlier claim that the fork's RequestPeers carve-out was "structurally moot" was false. DigLink::read_inbound (dig-peer-protocol-0.5.0/src/link.rs:470-478) matches on correlation id before forwarding. Both peers start next_id at 0 (request_map.rs:45) and both keepalive loops start at handshake on a shared interval, so both first probes carried id 0, each waiter received the peer's RequestPeers, the peer's request never reached the auto-reply at listener.rs:875, and both sides tore the link down at the staleness check while logging "no successful probe within PEER_TIMEOUT_SECS" — which names the wrong cause.

Fix: the probe goes out uncorrelated (DigLink::send, id: None) and liveness is observed on the application inbound broadcast. An id: None frame skips the id-match arm entirely. Fails loose: an unobservable round is skipped and the staleness window is reset, so the peer is kept — the correct direction for a probe whose only action is to disconnect.

2. CON-006 metering — the keepalive-side call was DROPPED (asked, measured, not assumed)

Today the correlated RespondPeers is consumed by the request map and never reaches inbound_rx, so keepalive.rs:232 was the only meter for it. Once the reply arrives uncorrelated it flows through the forwarder (listener.rs:872 / gossip_handle.rs:1018), which already calls record_live_peer_inbound_bytes. Keeping the keepalive-side call would double-count every probe, so it is gone.

tests/con_006_tests.rs was read first: every case drives record_live_peer_inbound_bytes / PeerConnectionWireMetrics directly with synthetic frames and none assumes keepalive-side metering. con_006_tests: 9 passed.

3. A non-Handshake first frame is POLICY, not transport (dig_ecosystem#2228)

dial_error::non_handshake_first_frame classifies all three sites. ClientError::{InvalidResponse,UnexpectedMessage} take ProtocolMessageTypes, so where the opcode maps it is the typed ClientError; where it does not, a dig-gossip-owned DialError::UnknownOpcode(u8) -> GossipError::UnknownHandshakeOpcode(u8) carries the raw byte. No formatted string stands in for a type. ProtocolMessageTypes is #[repr(u8)] Streamable with no TryFrom<u8>, so the single-byte decode is the total mapping.

4. A false claim about a build that does not exist (dig_ecosystem#2225)

introducer_client.rs claimed --no-default-features builds "remain coherent". They do not compile (dig_peer_protocol::{Client, ClientState} imported unconditionally at service/state.rs, service/gossip_service.rs, lib.rs; gossip_handle.rs:795,801), and no CI job builds without a TLS backend. The claim and the #[cfg(not(any(...)))] stubs that existed only to serve it are deleted, with the real state recorded in the module docs. Pre-existing, not a regression from this PR.

5. The crates.io publish guard is now UNCONDITIONAL (dig_ecosystem#2647)

.github/workflows/publish.yml blocked publish only on tag push; workflow_dispatch still published. Its own header admitted dispatch "does NOT make it safe". Anyone with dispatch permission could have shipped a dig-gossip built against upstream native-tls that silently accepts inbound peers presenting no client certificate. The guard now fails on every event, and the surrounding comments no longer imply dispatch is a sanctioned escape hatch.

Blast radius checked

gitnexus was NOT used: this worktree carries no .gitnexus index, and the per-worktree analyze was skipped under the 10-minute/disk bound. Blast radius was established by ripgrep + direct reads of every call site, per the sanctioned fallback.

Symbol Callers found Scope
keepalive_loop spawn_keepalive_task only; spawned at listener.rs:798 (inbound) and gossip_handle.rs (outbound) both legs covered by the new integration test
record_live_peer_inbound_bytes keepalive.rs:232 (removed), listener.rs:872, gossip_handle.rs:1018 CON-006 re-verified, 9 passed
DialError outbound.rs, introducer_client.rs, From<DialError> for GossipError new arm handled exhaustively; compiler-checked
GossipError new variant only; no existing variant altered additive
tests asserting LinkError con_001, dsc_004, dsc_005 — all on the refused-connect path, not the wrong-opcode path unaffected; all green

src/relay/ has a ZERO-byte diff vs origin/main (git diff origin/main -- src/relay/ is empty) — dig-relay's byte-identical vendoring is intact. No wire change.

Follow-ups — recorded, NOT fixed here

Two further instances of the same correlation-id steal exist and belong to the dig-peer-protocol cascade, not to a dig-gossip carve-out:

  1. src/service/gossip_handle.rs:539-552 send_stream_frame sends DIG-band opcode-220 frames with id: Some(stream_id & 0xFFFF) from an independent id space, so a collision delivers a stream frame to a keepalive waiter and it silently never reaches the stream reassembler.
  2. The simultaneous-dial collision on request_infallible(RequestPeers) (listener.rs:869, gossip_handle.rs:842).

Verification

Command Result
cargo test --lib 111 passed; 0 failed
cargo test --test con_2767_keepalive_correlation_tests 2 passed; 0 failed (13.59s)
cargo test --test con_004_tests 4 passed
cargo test --test con_006_tests 9 passed
cargo test --test con_001_tests 8 passed
cargo test --test dsc_004_tests 6 passed
cargo test --test dsc_005_tests 7 passed
cargo clippy --all-targets --all-features clean
cargo fmt --all -- --check clean

Load-bearing proof (each test observed RED for the right reason):

  • colliding_correlation_ids_do_not_tear_the_link_down — with keepalive.rs restored to ce4c08e: FAILED — A must still hold B after 6s of probing. With the fix: passes.
  • probe_reaches_the_peer_application_despite_an_outstanding_correlated_waiter — with send_probe reverted to request_raw: FAILED — probe sends: RequestTimeout(43), i.e. the mutual deadlock itself. With the fix: passes.
  • an_unobservable_probe_does_not_disconnect_the_peer — failed until the skip path also reset the staleness window; without that reset the peer was still torn down a few rounds later.

Version stays 0.23.0 (already bumped, still breaking).

MichaelTaylor3d and others added 8 commits August 10, 2026 17:22
dig-gossip's wire is vendored byte-identical into dig-relay (GPL-2.0) and
runs on a live peer network, so the migration off the vendored chia-protocol
fork (dig_ecosystem#2228) must not move a single byte. Pin the encoded frames
BEFORE any refactor so the claim is measurable rather than asserted: a vector
added afterwards would only record whatever the new code produces.

Each vector distinguishes the real encoding from the nearest wrong one --
both Option<u16> correlation-id states, a payload with distinguishable ends
so a reversed body or wrong length-prefix width is visible, a non-default
NodeType discriminant, and both RegisterAck outcomes.

Refs: dig_ecosystem#2228

Co-Authored-By: Claude <noreply@anthropic.com>
…directly

dig-peer-protocol 0.4.0 replaced its blanket `pub use chia_protocol::*` with
three named re-exports, so the ~20 genuine chia full-node types dig-gossip used
through that glob (Bytes32, Handshake, RequestPeers, SpendBundle, ...) now come
from a direct chia-protocol import. That is the intended shape -- chia types for
chia peers -- not a workaround.

Handshake.node_type is chia_protocol::NodeType while every DIG-side surface
speaks dig_peer_protocol::NodeType. Bridge them explicitly rather than casting:
both are closed enums over the same seven roles, so the conversion is total in
each direction and an exhaustive match makes a future divergence in either crate
a build failure instead of a silently wrong role.

The transport type (Message -> DigMessage) is deliberately left for the next
commit; it is the transport migration proper, not an import fix.

Refs: dig_ecosystem#2228

Co-Authored-By: Claude <noreply@anthropic.com>
… enum

DigMessage.msg_type is a raw wire byte, so a DIG opcode no longer needs a
ProtocolMessageTypes variant to name it -- which is the entire reason the
vendored chia-protocol fork existed on this path. frame_envelope,
frame_dig_message, frame_holdings_announce and frame_store_melted now build
DigMessage directly, and frame_dig_message loses its from_bytes round-trip
through the enum.

message_wire_len stops returning Result: DigMessage::to_bytes is infallible
because msg_type is already a byte and data is already serialized.

The golden vectors pinned in 5119249 must still pass byte-identically; that is
what makes this a refactor rather than a wire change.

Refs: dig_ecosystem#2228

Co-Authored-By: Claude <noreply@anthropic.com>
DOES NOT COMPILE (2 errors) -- unfinished plumbing, not a flawed approach.
Salvaged from a lane killed by a session limit so the work is not lost.

Swaps chia_sdk_client::Peer for dig_peer_protocol::DigLink, which is what
retires BOTH vendored crates: DigLink carries chia typed traffic via
opcode_of::<T>() AND the DIG band via send_dig/send_message, and supplies the
from_server_websocket the vendored fork exists to provide.

Refs #2619.
… coupling

chia_sdk_client::Peer decodes every inbound frame with chia_protocol::Message::
from_bytes, whose ProtocolMessageTypes stops at RespondCostInfo = 107 -- so it
rejects DIG opcodes 200-222 before dig-gossip's own code runs. That runtime
decode, not any compile-time type, is what kept the vendored chia-protocol fork
alive. DigLink frames a raw opcode byte and so carries the DIG band natively,
while still sending Chia-typed bodies via opcode_of::<T>(); one link per
connection serves both bands.

Inbound rate limiting moves from chia_sdk_client::RateLimiter, which is keyed by
ProtocolMessageTypes and therefore cannot express a DIG opcode at all, to
dig_peer_protocol::OpcodeRateLimiter, which re-keys Chia's own V2_RATE_LIMITS
onto the wire byte and so preserves every Chia bound unchanged.

Chia-band opcodes are derived from ProtocolMessageTypes in one small module
rather than written as literals, keeping chia-protocol the single authority for
Chia opcode numbering.

Refs: dig_ecosystem#2228

Co-Authored-By: Claude <noreply@anthropic.com>
…tocol

RegisterPeer and RegisterAck were declared here with #[streamable(message)],
which makes the proc-macro emit a ProtocolMessageTypes::RegisterPeer path -- a
variant that exists only in the forked chia-protocol. That one attribute was the
second thing anchoring the fork.

dig-peer-protocol declares the same bodies with plain #[streamable] plus
to_dig_message/from_dig_message, carrying the opcode as a raw byte. No enum
variant is needed, and the bodies now have one definition shared across repos
instead of one per repo. Registration correspondingly moves from the Chia-typed
request_infallible to request_dig.

Adds the CON-005 guard battery for the rate-limit re-key: an entry that fails to
carry from ProtocolMessageTypes onto the wire byte does not error, it silently
falls through to the far looser default_settings, which is a DoS surface no
compiler can see. Bounds are pinned as absolute literals from both sides, with a
control proving an untabled opcode really is looser -- without it the battery
would pass even if every opcode were capped identically.

All 9 golden wire vectors pass byte-identically. The wire has not moved.

Refs: dig_ecosystem#2228

Co-Authored-By: Claude <noreply@anthropic.com>
Both forks existed for one reason: Chia's ProtocolMessageTypes stops at
RespondCostInfo = 107 and cannot name a DIG opcode (200-222). chia-protocol was
forked to add the 23 variants; chia-sdk-client was forked because its Peer
reader decodes with chia_protocol::Message::from_bytes, which rejects any frame
whose opcode has no variant -- and because upstream has no server-side
constructor for an accepted websocket.

dig_peer_protocol::DigLink frames a raw opcode byte and ships
from_server_websocket, so neither reason survives. Both vendored trees are
deleted and both [patch.crates-io] entries removed.

native-tls STAYS patched. It is unrelated to this fork and is security
load-bearing: upstream TlsAcceptorBuilder exposes no way to require a client
certificate, so dropping it would still compile while silently accepting inbound
peers with no client cert at all (CON-009).

Verified from the resolved graph rather than from a green build, because a
mismatched [patch] disappears with a warning and exit 0: cargo metadata reports
no unused-patch warnings, no vendored chia-* package, and exactly one version
each of chia-protocol (0.26.0) and chia-sdk-client (0.28.0), both from the
registry. native-tls remains the sole vendored entry.

BREAKING CHANGE: the peer transport is dig_peer_protocol::DigLink rather than
chia_sdk_client::Peer; inbound channels carry DigMessage rather than
chia_protocol::Message; classify_broadcast takes a wire byte; message_wire_len
is infallible; GossipError gains a LinkError variant.

Refs: dig_ecosystem#2228

Co-Authored-By: Claude <noreply@anthropic.com>
The chia-protocol/chia-sdk-client fork deletion changed the inbound and
outbound transport types the integration harness builds against. Port the
test suite to DigLink's surface and bump the version to 0.23.0 (0.x
breaking bump for the transport change).

Co-Authored-By: Claude <noreply@anthropic.com>
@MichaelTaylor3d

Copy link
Copy Markdown
Contributor Author

Lane resumed — orchestrator status @ 5ac7be3

The previous lane stalled mid-port. Its uncommitted work (27 files: the integration-test port + the 0.23.0 bump) is now committed and pushed as 5ac7be3 — nothing was lost.

The fork is GONE — measured, not assumed

Check Result
[patch.crates-io] stanza native-tls only. No chia entry.
Every chia-* in Cargo.lock source = "registry+https://github.com/rust-lang/crates.io-index"zero path/vendor sources
vendor/ tree vendor/native-tls only
git = deps in Cargo.toml none

CI independently corroborates: the last run downloaded chia-protocol v0.26.0 / chia-sdk-client v0.28.0 from crates.io and compiled only vendor/native-tls locally.

The wire did NOT move — this is the dig-relay contract

dig-relay vendors this wire byte-identically, so this needed proof rather than reasoning. The golden vectors were pinned in 5119249 before any refactor, then edited in ac9fdb8 — which is exactly how a wire break ships green. So every expected literal was extracted from both revisions and compared:

BEFORE: 9 literals   AFTER: 9 literals   diff: (empty)

All nine unchanged (dc01123400000008deadbeef0102037f, c80000000008deadbeef0102037f, da010007000000110000000a3139322e302e322e3838255301, …). Only the construction changed — Message { .. }DigMessage::new, and to_bytes() became infallible so the .expect() calls were dropped. No coordinated network change and no dig-relay update is implied.

OpcodeRateLimits — the stall point, decided

OpcodeRateLimits's fields and its from_chia are private; Default (derived from V2_RATE_LIMITS) is the only public constructor, so tests/con_005_tests.rs cannot build a cheap table to exhaust.

Decision: add the public conversion to dig-peer-protocol (release-first) rather than rewrite or drop the tests.

  • Rewriting against Default + limit_factor would pin each test to upstream Chia's exact numbers, so an upstream table change would silently alter what they measure — and the frequency = 1/2 cases become unreachable entirely.
  • Dropping them loses five live properties of the Chia bound that this crate's InboundRateLimiter composes and depends on: per-connection counter independence, the frequency cap, the per-message max_size cap, limit_factor scaling, and window reset. None of them is fork-specific.
  • from_chia(&RateLimits) already exists and does the work; making it a public From is additive and preserves the "derive, never copy" property the module's own lockstep note depends on. dig-peer-protocol has zero open PRs, so single-writer is clean.

Also found

  • A vacuous-green trap at inbound_limits.rs:385, public_flood_opcode_set_is_exactly_221_and_222. Upstream ProtocolMessageTypes::from_bytes now fails for 221/222, so the loop's continue skips precisely the two opcodes the test is named after — it would assert an empty set and pass while proving nothing, and it is the only guard that the public-flood classification does not silently widen. Being ported to raw u8 with an explicit falsifiability check.
  • The crates.io publish stays blocked, and not for the reason the workflow states. publish.yml is guarded off citing three vendored forks; two are now gone, but cargo publish strips [patch.crates-io], so a published dig-gossip would build against upstream native-tls — which has no way to require a client certificate — and would silently accept inbound peers presenting none (CON-009). Ticketed separately; the guard must stay.

NEXT ACTION

  1. dig-peer-protocol: public From<&RateLimits> for OpcodeRateLimits → 0.5.0 → publish.
  2. dig-gossip: port the src/ cfg(test) units to u8 opcodes and clear the five unused-import clippy failures.
  3. Re-pin dpp 0.5, port con_005_tests.rs, run the docs/SPEC sweep, reword the wip(...) commit for commitlint, then the triple gate.

This PR stays DRAFT until the gate round returns.

@MichaelTaylor3d MichaelTaylor3d changed the title test(wire): pin golden frame vectors; chia-protocol fork deletion blocked on the Peer->DigLink transport migration feat(deps)!: delete the vendored chia-protocol and chia-sdk-client forks Aug 11, 2026
@MichaelTaylor3d

Copy link
Copy Markdown
Contributor Author

Gate finding (orchestrator, pre-review): the golden vectors are ENCODE-only

All nine vectors assert hex(frame) == "...". There is not one from_bytes, decode, or round-trip assertion in tests/wire_golden_vectors_tests.rs:

$ grep -nE 'from_bytes|decode|round.?trip' tests/wire_golden_vectors_tests.rs
(no matches)

That is the wrong half of the wire to pin for this change, for three reasons that compound:

  1. The fork existed because of a DECODE failure, not an encode one. The whole diagnosis on this PR is vendor/chia-sdk-client/src/peer.rs:446, Message::from_bytes(&binary)?, rejecting every DIG opcode 200–222 because ProtocolMessageTypes is a closed enum. Encode was never the blocked direction.
  2. This PR replaces the decode path. Inbound frames no longer arrive through chia-sdk-client's reader loop; they arrive on DigLink's own receiver. So the component whose behaviour actually changed is the one with no golden coverage.
  3. dig-relay is on the other end. It vendors this wire byte-identically (GPL-2.0) and encodes frames dig-gossip must decode. Proving we still emit the same bytes says nothing about whether we still accept theirs. A stricter length-prefix check, a rejected id-absent frame, or a tightened opcode range would all pass every vector here and still break interop in production.

Worth noting: PR #62's earlier vector set explicitly included "the round trip through the vendored decoder". That coverage existed and was not carried forward into this PR's nine — so this is a regression on the one axis the fork was ever about, not merely a gap that was always there.

Required before merge

Add decode-side vectors that feed the exact same nine byte strings back in and assert the recovered msg_type / id / data — i.e. pin the wire in both directions from one set of literals, so the two halves cannot drift apart. At minimum:

  • DigMessage::from_bytes on each 220/200/217 literal, checking msg_type, the Some/None correlation id, and the payload bytes.
  • The 218/219 literals decoded through to RegisterPeer / RegisterAck bodies, including the Introducer discriminant and the success = false case.
  • A negative vector: a frame carrying an opcode with no ProtocolMessageTypes variant must decode successfully now (that is the entire point of DigMessage's raw u8), where the pre-fork-deletion path returned Err. Without this one, nothing distinguishes "decodes DIG opcodes" from "happens not to reject them yet".

Anchor the expectations to the literals already in this file rather than to whatever the decoder currently returns — an expectation captured from a run cannot fail when the run is wrong.

MichaelTaylor3d and others added 18 commits August 10, 2026 21:42
Upstream `chia_protocol::ProtocolMessageTypes` is a closed enum ending at
`RespondCostInfo = 107`, so the fork-only `HoldingsAnnounce` (222) and
`StoreMelted` (221) variants no longer exist. Port every `#[cfg(test)]`
module in `src/` onto the raw `u8` opcode constants and the
`dig-peer-protocol` 0.4.0 `OpcodeRateLimiter` API.

Two guards were silently defused by the enum's disappearance and are
repaired, not merely recompiled:

- `public_flood_opcode_set_is_exactly_221_and_222` filtered each opcode
  through `ProtocolMessageTypes::from_bytes`, which now FAILS for 221 and
  222 — so the loop skipped exactly the two opcodes the test is named
  after and asserted the empty set. It now walks all 256 raw opcodes with
  no decode filter, and additionally names the resulting set in full.
- `every_220_band_opcode_is_classified` had the same filter over the
  220-band, which now matches nothing at all. It enumerates
  `dig_peer_protocol::ALL_DIG_OPCODES` — the canonical namespace list —
  and asserts the band it checks is non-empty.

The staggered-construction fixture in `dig_rate_limiter` is preserved: the
Chia half is now an `OpcodeRateLimiter` built at the window start against
the published `Handshake` row (5/window), the DIG half ~1 s later, so an
elapsed-since-construction window still fails observably.

Co-Authored-By: Claude <noreply@anthropic.com>
`DigMessage::to_bytes` is an inherent, infallible method, so the trait
import is dead and fails the `-D warnings` gate.

Co-Authored-By: Claude <noreply@anthropic.com>
`DigMessage::msg_type` is already a raw `u8` since the port off
`ProtocolMessageTypes`, so these `as u8` casts are no-ops that trip
`clippy::unnecessary_cast` under `-D warnings`.

Co-Authored-By: Claude <noreply@anthropic.com>
0.5.0 adds `impl From<&RateLimits> for OpcodeRateLimits`, which the
CON-005 tests need. A 0.x minor is semver-incompatible, so the old
"0.4" requirement could never resolve it.

Co-Authored-By: Claude <noreply@anthropic.com>
`dig-peer-protocol` 0.5.0 replaces chia's `ProtocolMessageTypes`-keyed
`RateLimiter` with `OpcodeRateLimiter`, keyed by the raw wire opcode, so
the CON-005 tests move onto it: `OpcodeRateLimits::from(&limits)` re-keys
the same chia table the old fixtures used, and `allow` replaces
`handle_message`.

The one semantic difference — `admit` does not charge a refused message,
where chia's inbound limiter did — changes no assertion here: every test
exhausts its budget with ADMITTED frames and then asserts the next one is
refused, so nothing depends on a rejection consuming budget. The
charge-on-reject property still lives in dig-gossip's own
`DigRateLimiter`, which keeps the `incoming` flag.

`test_rate_limit_blocks_oversized_message` gets stronger rather than
merely ported: the bound is pinned from both sides (at `max_size` admits,
one byte over refuses) and the refusal is asserted as `Unsendable`, not
just "not admitted" — a size refusal survives every window roll, so
reporting it as `Deferred` would send a retrying caller to wait on a
budget that can never clear. `Admission` is re-exported to make that
assertable.

Also clears the warnings the previously-failing target masked: unused
imports across seven test files and two more no-op `as u8` casts.

Co-Authored-By: Claude <noreply@anthropic.com>
The nine vectors were encode-only, which is the wrong half of the
contract. The vendored `chia-protocol` fork existed *because*
`Message::from_bytes` rejected DIG opcodes; this PR replaces that decode
path, and dig-relay vendors the wire byte-identically and sends frames
dig-gossip must accept. Proving we still emit the same bytes says nothing
about whether we still read theirs.

Each hex literal is now a `const` consumed by BOTH an encode and a decode
test, so the two directions cannot drift apart — an expectation captured
from a decoder run could not fail when the run is wrong.

Adds a negative vector: a frame whose opcode has no
`ProtocolMessageTypes` variant must decode successfully, which is the
entire reason `DigMessage` keys on a raw `u8`. It is paired with an
assertion that `ProtocolMessageTypes::from_bytes` genuinely rejects that
opcode — without it the vector would pass just as happily on an opcode
the forked enum accepts, and so could not distinguish "decodes DIG
opcodes" from "has not happened to reject one yet".

Also applies rustfmt, which had drifted on this branch and in several
test files the fmt gate would have failed on.

Co-Authored-By: Claude <noreply@anthropic.com>
`test_outbound_connect_failure` was the only failing test left in the
suite: since the outbound dial moved off `chia-sdk-client` onto
`DigLink`, a refused TCP connection arrives as
`GossipError::LinkError(WebSocket(Io(ConnectionRefused)))`, not
`ClientError`.

The truthful contract is the new one, so the CON-001 spec row moves with
it rather than the mapping being papered over: `ClientError` still
carries handshake-POLICY failures (network-id mismatch, incompatible
protocol version), and keeping the two variants distinct is what lets a
caller tell "never reached the peer" from "the peer refused us". The test
therefore asserts the specific variant — `is_err()` would pass on a
handshake rejection that never touched the wire.

Co-Authored-By: Claude <noreply@anthropic.com>
…k deletion

The publish guard's comments and echo text listed three vendored forks in
[patch.crates-io] (chia-protocol, chia-sdk-client, native-tls) and attributed
the block to 19 compile errors from cargo publish stripping them. Two of those
forks were deleted in dig_ecosystem#2228; only native-tls remains.

The publish is still correctly blocked, but for a different and more dangerous
reason: cargo publish strips [patch.crates-io], so a published dig-gossip would
build against upstream native-tls, compile cleanly, and silently accept inbound
peers presenting no client certificate (CON-009). The failure mode is a silent
security regression, not a build break.

Comments and echo text only — the guard step, its condition and its exit 1 are
unchanged.

Refs dig_ecosystem#2228, dig_ecosystem#2647

Co-Authored-By: Claude <noreply@anthropic.com>
…ransport

Sections 1 through 1.7 described the peer connection as chia-sdk-client's
`Peer`, reached by `connect_peer()`, with rate limiting by `RateLimiter` +
`V2_RATE_LIMITS` and payloads delivered as `chia-protocol::Message`. None of
that is the contract any more: dig-gossip depends on `dig-peer-protocol`, whose
`DigLink` frames a raw u8 opcode, and delivers `DigMessage`. The Chia types are
still used, but as re-exports rather than direct dependencies.

SPEC.md is normative, so an independent reimplementation built against these
sentences would have named types that dig-gossip does not use.

Refs dig_ecosystem#2228

Co-Authored-By: Claude <noreply@anthropic.com>
… API surface

Sections 2, 5, 8 and 10 still described a vendored `chia-protocol`, a
`vendor/` thin-wrapper policy, and an API built on `chia-sdk-client`'s `Peer`,
`PeerOptions`, `RateLimiter` and `connect_peer()`. The vendored Chia forks were
deleted in dig_ecosystem#2228; the peer link is `dig_peer_protocol::DigLink`,
inbound uses `DigLink::from_server_websocket()`, and the framing encoder writes
a raw u8 opcode instead of mirroring a discriminant onto `ProtocolMessageTypes`.

Corrected in this commit:
- 2.1 / 10.2 re-export blocks, which named types the crate does not re-export
- 2.4 PeerConnection, whose `peer` field is a `DigLink`
- 2.3 / 8.x frame and payload signatures, which take and return `DigMessage`
- 5.1 / 5.2 connection flows, incl. why outbound mirrors rather than calls
  upstream's `connect.rs` (it must capture the remote SPKI DER)
- 5.3's `Peer::from_server_websocket` and the native-tls patch rationale
- 8.1's "vendored `chia-protocol`" clone note and `vendor/` policy reference
- the dispatch-authority paragraph's "mirrors onto the vendored
  `ProtocolMessageTypes`" claim
- 10.3 / 10.4 feature-flag and dependency snippets

The wire is unchanged: all nine golden hex vectors are byte-identical across
the refactor, so no on-wire statement in this document moved.

Refs dig_ecosystem#2228

Co-Authored-By: Claude <noreply@anthropic.com>
…dig-peer-protocol

STR-001, STR-003, STR-004, IMPLEMENTATION_ORDER and prompt/start.md still
required a direct chia-sdk-client dependency, feature forwards to
chia-sdk-client/*, and re-exports of Peer/PeerOptions/RateLimiter. The manifest
and the str_001 tests already assert the opposite: dig-peer-protocol at 0.4 with
default-features = false, and both TLS features forwarding through it.

STR-001 gains an acceptance criterion that [patch.crates-io] contains native-tls
and nothing else, so a re-vendored Chia crate is a spec violation rather than a
silent regression.

Refs dig_ecosystem#2228

Co-Authored-By: Claude <noreply@anthropic.com>
…opcode band

The connection, crate_api, discovery, relay, privacy and erlay domain specs
still specified chia-sdk-client's Peer, PeerOptions, connect_peer(),
Peer::from_websocket() and RateLimiter, and two TRACKING.yaml files cited
vendor/chia-protocol and vendor/chia-sdk-client paths that no longer exist.

Corrections of substance, beyond the renames:
- CON-001 no longer specifies calling connect_peer(). The crate deliberately
  mirrors upstream's connect.rs instead, because upstream discards the parsed
  Handshake and never exposes the remote TLS SPKI DER, and API-005 needs both.
- CON-002 states why inbound needs from_server_websocket: from_websocket types
  the stream as the client-oriented MaybeTlsStream.
- CON-005 keys both rate-limit bounds by the raw wire opcode; the previous text
  had the Chia bound keyed by ProtocolMessageTypes.
- DSC-005 tracking no longer claims a vendored chia-protocol supplies the
  218/219 enum variants. They travel as raw DigMessage msg_type bytes.
- CON-009 tracking names vendor/native-tls as the only remaining vendored crate.

All TRACKING.yaml and registry files re-validated as YAML.

Refs dig_ecosystem#2228

Co-Authored-By: Claude <noreply@anthropic.com>
… trap

Carries forward the one durable lesson from the abandoned rebase investigation
(PR #62): [patch.crates-io] substitutes only where the patched version SATISFIES
the requirement, so a mismatched patch is dropped with a warning and EXIT=0 —
the failure has no shape. Rewritten for the post-deletion world, where it now
explains why the remaining native-tls patch is guarded in publish.yml rather
than by the compiler: nothing stops compiling if it is stripped, the acceptor
just stops requiring a client certificate.

Also kept from #62: upstream ProtocolMessageTypes stops at RespondCostInfo = 107
at 0.26.0, 0.36.1 and 0.47.0, so the DIG 200-222 band collides with nothing.

Not carried forward: #62's verdicts that chia-protocol and chia-sdk-client could
not be deleted. The redesign it named as a prerequisite is exactly what was done.
The new entry records the mechanism — DigMessage's raw u8 msg_type, which makes
the opcode band expressible with no enum to extend — and that the wire was pinned
by golden vectors before the refactor and is byte-identical after.

Also corrects the #1371 entry, which credited from_server_websocket to the
vendored chia-sdk-client; it now lives on DigLink, and is one of the reasons the
fork could go. README's PeerConnection field and Cargo.toml's dependency-shape
rationale follow the same rename (Cargo.toml comments only — no dependency,
version or patch line is touched).

Refs dig_ecosystem#2228, dig_ecosystem#2647

Co-Authored-By: Claude <noreply@anthropic.com>
…nt blocks

Three dependency comments still pointed at `chia_sdk_client::Peer` and one
called the rustls backend 'the vendored chia-sdk-client outbound connector'.
The type is `dig_peer_protocol::DigLink` and chia-sdk-client is now an ordinary
transitive crates.io dependency.

Comments only: the diff contains no dependency, version, feature or
[patch.crates-io] line.

Refs dig_ecosystem#2228

Co-Authored-By: Claude <noreply@anthropic.com>
listener.rs and outbound.rs still documented `chia-sdk-client` and
`vendor/chia-sdk-client` as the transport, and outbound.rs linked a
docs.rs page for a function neither crate exposes here. Port the
reasoning to what the code now does.

Co-Authored-By: Claude <noreply@anthropic.com>
Four doc comments referenced vendored chia-protocol and chia-sdk-client
directories that were deleted during the DPP 0.4.0 adoption. Update them
to describe what the code actually does now:

- introducer_client.rs:19 — point to the real dial reference
  (src/connection/outbound.rs) instead of vendor path; note that
  chia-sdk-client remains a legitimate crates.io transitive dependency
- introducer_register_wire.rs:13 — replace "fork keeps it alive" with
  "DIG opcodes now travel as raw msg_type bytes, making the fork removable"
- discovery/mod.rs:13 — drop vendor-path reference; explain that opcodes
  218/219 re-export from dig-peer-protocol
- gossip_handle.rs:988 — replace "(see vendor/chia-sdk-client patch)" with
  "a property of the DigLink receiver"

Code behavior unchanged; comment text only.

Co-Authored-By: Claude <noreply@anthropic.com>
The introducer client now uses DigLink for the dial, so a refused TCP
connect surfaces as GossipError::LinkError, not ClientError. This matches
the CON-001 pattern where the dial's typed connection-level error is
LinkError; ClientError remains reserved for handshake-policy failures
(dig_ecosystem#2228).

Add LinkError to the match in test_query_introducer_connect_fail and
update DSC-004 acceptance criteria and test plan to clarify the expected
error variant.

Co-Authored-By: Claude <noreply@anthropic.com>
220 is `DIG_MESSAGE`, 221 `STORE_MELTED`, 222 `HOLDINGS_ANNOUNCE`, and all
three are live. dig-relay vendors this wire byte-identically, so an
understated band in a dependency comment is exactly the kind of number
that gets copied into a second implementation.

Comment only — the dependency line is unchanged.

Co-Authored-By: Claude <noreply@anthropic.com>
MichaelTaylor3d and others added 2 commits August 11, 2026 07:20
…port APIs

Replace stale doc/comment text referencing deleted `vendor/chia-protocol` and
`vendor/chia-sdk-client` paths; update references to the old `connect_peer()`
function (which does not exist) with descriptions of the actual transport
establishment via TLS + handshake in `dig_peer_protocol::DigLink`. Update
`send_protocol_message` reference to the actual `send_message` method.

Facts:
- chia_sdk_client::Peer → dig_peer_protocol::DigLink (the connection type)
- connect_peer() does not exist (outbound dial is in src/connection/outbound.rs)
- send_protocol_message → send_message
- vendor/chia-protocol/, vendor/chia-sdk-client/ paths no longer exist

Files changed: 8 test files with doc/comment-only updates.
No test logic, assertions, fixtures, or use statements changed.

Co-Authored-By: Claude <noreply@anthropic.com>
…_peer()

The SPEC traceability block quoted `connect_peer()`, a function no crate
in this tree exposes. SPEC.md no longer uses that name either, so the
citations pointed at language that had been rewritten.

Co-Authored-By: Claude <noreply@anthropic.com>
@MichaelTaylor3d
MichaelTaylor3d force-pushed the feat/adopt-dpp-040-delete-chia-fork branch from 22f27bd to 661133e Compare August 11, 2026 14:28
@MichaelTaylor3d

Copy link
Copy Markdown
Contributor Author

Adversarial gate: REFUTED — this PR silently drops the inbound rate limiter's anti-flood ratchet

Merge is blocked. The fix is in flight as dig-peer-protocol 0.6.0.

The break

src/connection/inbound_limits.rs:83 now builds the Chia half of the inbound gate as:

chia: OpcodeRateLimiter::new(RESET_SECONDS, rate_limit_factor, OpcodeRateLimits::default()),

On origin/main that same field was:

RateLimiter::new(true, RESET_SECONDS, rate_limit_factor, (*V2_RATE_LIMITS).clone())

The leading true is incoming, and it is load-bearing. In the limiter it replaced — and identically in the UNFORKED upstream chia-sdk-client-0.28.0/src/rate_limiter.rs:104:

if self.incoming || passed {
    // commit message_counts, message_cumulative_sizes, non_tx_count, non_tx_size
}

With incoming = true, a refused inbound frame is still charged. That is the anti-flood ratchet: a peer whose frames are being rejected keeps burning the per-opcode cumulative-size budget and the shared non_tx_count / non_tx_max_total_size aggregates, so the window stays exhausted for the full 60 s.

OpcodeRateLimiter has no incoming field at all. admit() returns Unsendable/Deferred before touching any counter, and its own doc states the inverse rule outright: "A refused message is NOT charged, so a caller that backs off and retries is not permanently penalised for having asked early." That is the outbound semantic, now applied to the inbound gate.

Worked example — no DIG opcode required

Handshake sits in other with max_size = 10 KiB. A stream of 11 KiB Handshake frames used to fail size > max_size while still incrementing non_tx_count / non_tx_size on every frame — roughly 1000 frames and every other opcode on that connection is locked out for the window. Under this PR, fits_an_empty_window is false, Unsendable returns before the commit block, and non_tx_count stays at 0 forever. Refused traffic is free.

Why this is a gate failure and not a defensible design change

  1. The fork deletion did not require it. Unforked chia-sdk-client 0.28.0 still carries incoming at line 104. Nothing about removing the vendored crates forced this.
  2. The file's own documentation still asserts the old semantic, in four places: inbound_limits.rs:78 ("Builds the gate for one inbound connection — incoming = true"), inbound_limits.rs:112 ("Always apply the Chia base bound first (and unconditionally, so its counters advance)"), service/state.rs:137, and tests/con_005_tests.rs:32. Line 112 is now factually wrong about the line directly beneath it.
  3. Its sibling in the same struct kept the flag. DigRateLimiter::check still ends if self.incoming || passed { … } and is still constructed with true at inbound_limits.rs:89. One composed gate now runs two opposite accounting rules.

Why no existing gate could catch it

The 167 new lines in tests/con_005_rate_limit_rekey_tests.rs pin the keys and the numbers. Not one test drives a frame past refusal and then re-inspects the window. A coverage floor sees nothing here — this is a softening, not a deletion.

Stated without overclaiming: allows() still returns false, so the frame is still dropped, and rejected_frame_incurs_penalty still charges misbehaviour for non-flood opcodes. But for the 220-band, that penalty is deliberately exempt on RATE rejections — so a 221/222 flood past the base bound now advances neither limiter's counters nor the misbehaviour score.

Fix

Root-caused into dig-peer-protocol rather than patched here, because the type genuinely cannot express the inbound rule today. 0.6.0 adds:

pub enum Direction { Inbound, Outbound }
OpcodeRateLimiter::new(direction, reset_seconds, limit_factor, limits)

Deliberately not upstream's bare bool: the entire reason this was invisible is that a positional true vanished during a signature change and nothing read as missing. Direction::Inbound at a call site is checkable at a glance and cannot be silently dropped by a future signature change.

Then here: re-pin 0.6, pass Direction::Inbound at inbound_limits.rs:83, correct the four stale doc sites, and add the missing test — drive frames past refusal, then assert a subsequently legal frame is still refused, including on the non_tx aggregates.

Attacks that FAILED (recorded so they are not re-run)

  • Two ProtocolMessageTypesCargo.lock has exactly one chia-protocol (0.26.0). The 0.22/0.26 duplication is chia-bls/chia-sha2/chia-traits/chia_streamable_macro via clvm_tools_rs; the enum cannot resolve twice, so the re-key cannot key off a shifted discriminant.
  • The wire moved — all 9 hex literals byte-identical to 5119249. The decode side is not captured from a run: it reads the same const the encode assertion checks, so a wrong decoder cannot self-agree. DigMessage::MAX_MESSAGE_SIZE (16 MiB) exceeds WS_MAX_MESSAGE_BYTES (8 MiB, connection/mod.rs:47), so the transport clips first and no legitimate frame falls in the gap.
  • native-tls downgradedgit diff origin/main...HEAD -- vendor/native-tls is empty; inbound still uses native_tls::TlsAcceptor on both feature paths (rustls affects outbound dials only). No CON-009 downgrade.
  • Raw u8 opens an unknown-opcode path — no default arm maps an unknown opcode onto a known one; unknowns route to deferred_dispatch_error.
  • A test was deleted — zero test functions removed from tests/ or src/; the count went 976 → 1005.

@MichaelTaylor3d

Copy link
Copy Markdown
Contributor Author

Second blocker: handshake-POLICY failures are being wrapped in LinkError, which inverts the decision made on this PR

CI on 661133e: test_query_introducer_handshake_wrong_network fails at tests/dsc_004_tests.rs:228.

expected ClientError from WrongNetwork, got
LinkError(Io(Custom { kind: Other, error: "Expected network a592cc…04ae, but found not-the-test-network-id" }))

The behaviour is correct — the wrong-network handshake IS rejected, fail-closed, with an accurate message. Only the error variant changed. That makes this look like the same trivial fix as test_query_introducer_connect_fail, which was resolved by accepting LinkError. It is not, and resolving it the same way would be a mistake.

Why relaxing this test would undo the decision made earlier on this PR

The settled rule on this PR is that LinkError and ClientError stay distinct because they carry opposite retry semantics:

  • LinkError — never reached the peer (ConnectionRefused, timeout). Worth retrying against the same address later.
  • ClientError — reached the peer, and the peer or the handshake failed policy. Retrying cannot help.

A wrong network_id is unambiguously the second kind. We connected, we completed TLS, we received a Handshake, we validated it, and we rejected it. Filing that under LinkError means a caller sees LinkError(Io(ConnectionRefused)) and LinkError(Io(Custom("Expected network …"))) as the same class — and there is no way to tell them apart except by matching on the error string, which is not a contract.

So test_query_introducer_connect_fail accepting LinkError was right (it genuinely never reached the peer), and this test demanding ClientError is also right. The two are consistent; the code is what is inconsistent.

This is self-inflicted, not inherited from DigLink

dig-peer-protocol 0.5.0 does not contain the string Expected network anywhere:

$ grep -rn 'Expected network' ~/.cargo/registry/src/*/dig-peer-protocol-0.5.0/src/
(no matches)

The message is dig-gossip's own, and dig-gossip's own code chooses the wrapper. src/discovery/introducer_client.rs:121 does the same thing for the sibling case:

return Err(LinkError::Io(std::io::Error::other("missing handshake")));

Both of those are policy verdicts this crate reaches itself, downgraded into a transport error on the way out. The module's own doc-comment at introducer_client.rs:88 still describes the intended contract — Err(GossipError::ClientError(_)) for wire-level failures — so the code has drifted from its own documented surface as well.

Fix (batched, not applied yet)

Route dig-gossip's OWN handshake-policy verdicts to ClientError, leaving LinkError for failures that genuinely originate in the transport:

  • the network_id mismatch,
  • missing handshake (introducer_client.rs:121),
  • any sibling policy check on that path — worth grepping for other LinkError::Io(std::io::Error::other(...)) constructions, since that shape is how the downgrade happens.

Do NOT distinguish these by matching on the error string.

Holding this rather than pushing it now: the adversarial gate has already refuted this PR over the dropped incoming flag, and the fix for that lands via dig-peer-protocol 0.6.0, which forces a re-pin here anyway. Both fixes will land together and the PR will be re-gated as one combined delta — a gate verdict against a head that has since moved is a post-mortem, not a gate.

Note on how this surfaced

This failure was invisible until test_query_introducer_connect_fail was fixed, because nextest stops at the first failing target. The run count went 529 → 531. A suite count from a red run is a floor, not a total — there may be further failures behind this one, so the next green run is the first real measurement of this branch.

@MichaelTaylor3d MichaelTaylor3d left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CHANGES-REQUIRED - independent correctness gate, head 661133e

The fork deletion itself is clean and well-evidenced. Verified independently:

  • (a) The fork is provably gone from the lock. Every chia-* entry in Cargo.lock carries source = "registry+https://github.com/rust-lang/crates.io-index"; the only two packages in the lock with no source are dig-gossip itself and the patched native-tls. [patch.crates-io] contains native-tls only, and vendor/ holds only native-tls + fork-delta.sh. git diff base..head -- vendor/native-tls/ is empty - the security-load-bearing CERT_REQUIRED fork is untouched and its patch entry survives.
  • (b) The wire did not move. All nine hex literals pinned in 5119249 are byte-identical at head; exactly one literal is added (fe0000000008deadbeef0102037f, the negative vector). Encode and decode share one const per vector, so the two directions cannot drift into agreeing with each other.
  • (c) The item-5 vacuity check bites - measured, not read. I widened is_public_flood_opcode's matches! arm with | 220u8 in my own worktree: public_flood_opcode_set_is_exactly_221_and_222 went RED (opcode 220 public-flood classification, left: true, right: false, inbound_limits.rs:408). Restored; the shared checkout was never touched.
  • (d) No con_005 property was lost. All five survive (per-connection independence, frequency cap, max_size, limit_factor, window reset), and the max_size case is strengthened - it now pins both sides of the bound and asserts Admission::Unsendable rather than a bare refusal. None of the five depended on the old chia limiter charging rejected frames.
  • No test was deleted. Test-function names: 943 at base, 972 at head, zero removed.
  • The 0xfe negative vector is paired with an explicit ProtocolMessageTypes::from_bytes(&[0xfe]).is_err() assertion, so it genuinely distinguishes "decodes DIG opcodes" from "has not happened to reject one yet".

What blocks it: the settled LinkError-vs-ClientError design was applied to only one of the two handshake legs, which leaves the crate red. Test Suite + Coverage (>=80%) is FAILING at this head (run 31501745084) and aborted at 531/977 tests, so the 80% coverage floor is not merely unmet - it is unmeasured. Three findings below; details inline.

Doc-only findings 2 and 3 are handed to @copilot. Finding 1 (and the CON-002/CON-003 sweep that depends on its resolution) is an error-contract decision plus a test, and is NOT for Copilot - route it to an implementer.

Comment thread src/connection/outbound.rs Outdated
Comment thread docs/resources/SPEC.md Outdated
Comment thread docs/resources/SPEC.md Outdated
@MichaelTaylor3d

Copy link
Copy Markdown
Contributor Author

RESUME CHECKPOINT — machine restart. Written for an agent with zero context.

Everything is pushed. No uncommitted work anywhere.

Repo Branch HEAD State
dig-gossip feat/adopt-dpp-040-delete-chia-fork (PR #63, draft) 661133e clean, pushed
dig-peer-protocol fix/rate-limit-direction (PR #8, draft) c53544b WIP, mid-implementation, does not necessarily compile
dig-gossip docs/2228-spec-sweep-post-fork e0c8fdf fully cherry-picked into #63; branch is now redundant

THE FORK IN THE ROAD — OpcodeRateLimits. SETTLED. Do not re-derive.

The previous lane stalled on: "OpcodeRateLimits exposes no custom-table constructor, so these tests can't be mechanically ported."

Decision taken: option (a) — added a public constructor to the owning crate. No tests were rewritten against a weaker surface and NO coverage was dropped.

from_chia(&RateLimits) already existed as a private fn in dig-peer-protocol/src/rate_limit.rs:48. It was exposed as impl From<&RateLimits> for OpcodeRateLimits, shipped as dig-peer-protocol 0.5.0, which is MERGED and LIVE on crates.io (verified {"num":"0.5.0"}, tag v0.5.0). dig-gossip re-pinned "0.4""0.5" and tests/con_005_tests.rs was ported — 15/15 pass.

Why not the alternatives:

  • Rewriting against Default + limit_factor would pin every test to upstream Chia's exact numbers, so an upstream table change silently alters what they measure — and the frequency = 1/2 cases become unreachable.
  • Dropping them would cost five live properties of the Chia bound that InboundRateLimiter composes: per-connection counter independence, the frequency cap, the per-message max_size cap, limit_factor scaling, window reset. None is fork-specific.
  • Fields stayed private and no field-wise constructor was added, preserving the "table is DERIVED from Chia's numbers, never copied" property the module's own lockstep note depends on.

One test was strengthened rather than merely ported: test_rate_limit_blocks_oversized_message now pins the bound from both sides and asserts Admission::Unsendable specifically (a Deferred there would send a retrying caller to wait on a budget that can never clear).


THE FORK IS GONE — measured, not inferred

  • [patch.crates-io] = native-tls only. No chia entry.
  • Every chia-* in Cargo.lock resolves source = "registry+https://github.com/rust-lang/crates.io-index"zero path/vendor sources.
  • vendor/ tree = vendor/native-tls only. No git deps.
  • Beware: a [patch] that does not match is dropped silently with exit 0, so a green build proves nothing here. Read the lock.

The wire did NOT move. All 9 golden hex literals byte-identical across the refactor (pinned in 5119249 before it). Decode vectors read the same const the encode side asserts, so a wrong decoder cannot self-agree. dig-relay needs no change.


PR #63 — BLOCKED by 3 findings from the triple gate (adversarial REFUTED, security PASS+medium, correctness CHANGES-REQUIRED)

B1 — inbound rate limiter lost incoming = true. src/connection/inbound_limits.rs:83 builds OpcodeRateLimiter::new(RESET_SECONDS, factor, …); main had RateLimiter::new(true, …). That flag charged refused frames (if self.incoming || passed), the anti-flood ratchet. Unforked upstream chia-sdk-client-0.28.0/src/rate_limiter.rs:104 still has it, so the fork deletion never required dropping it. Honest severity (security gate refined the adversarial's claim): the admission predicates are equivalent, so the ceiling is unchanged — what is lost is that junk no longer burns the sender's own budget or accrues penalties. Four doc sites still assert the old semantic (inbound_limits.rs:78,112, service/state.rs:137, tests/con_005_tests.rs:32); :112 is wrong about the line beneath it. Sibling DigRateLimiter kept its flag — one gate, two opposite rules.

B2 — handshake-POLICY failures wrapped as LinkError. CI RED: tests/dsc_004_tests.rs:228. Root cause found by the correctness gate: src/connection/outbound.rs:269 maps validate_remote_handshake with LinkError::from while inbound src/connection/listener.rs:688 uses ClientError::from — same rejection, different variant by who dialled. Also src/connection/handshake.rs:176-191 degrades a typed ClientError::WrongNetwork(expected, actual) into LinkError::Io(Error::other(String)), and src/discovery/introducer_client.rs:121 does the same for missing handshake.

B3 — RequestPeers correlation-id carve-out is gone. The PR claimed it "structurally moot"; it is not. DigLink::read_inbound does requests.remove(id) for every opcode. Deleted vendor/chia-sdk-client/src/peer.rs:453-462 had the carve-out with a written rationale. Both ends run CON-004 keepalive request_raw(RequestPeers::new()) and RequestMap starts at next_id = 0 per link → on collision both sides fail and disconnect. Comment at src/service/gossip_handle.rs:986-988 now asserts the false claim.

SETTLED DESIGN CALL — do not relitigate. LinkError = never reached the peer (retryable). ClientError = reached the peer, policy rejected (never retryable). Recorded in a PR comment. Do NOT "fix" B2 by relaxing the test to accept either variant, and never match on the error string.


MEASUREMENT WARNING

cargo nextest here has no --no-fail-fast, so the job stops at the first failing target. The last run reached 531 of 977446 tests never ran and the 80% coverage floor is UNMEASURED, not merely unmet. Every suite count so far is a floor. The first fully green run is the first honest measurement of this branch.


NEXT ACTIONS, in order

  1. Finish dpp PR fix(stream): bound StreamReassembler (chunk/byte/stream caps, DoS) #8 (c53544b, WIP). Two fixes in one release, 0.5.00.6.0:
    • Fix 1 (believed done, d098208): pub enum Direction { Inbound, Outbound } as first param of OpcodeRateLimiter::new. Inbound charges refused frames incl. both non_tx aggregates; Outbound unchanged. DigLink (src/link.rs:245) passes Outbound. Deliberately an enum, not upstream's bare bool — a positional true is exactly what vanished unnoticed.
    • Fix 2 (HALF-DONE — this is the mid-edit): files left dirty and now committed WIP are src/link.rs, src/request_map.rs, tests/inbound_dig_opcode.rs, tests/link_liveness.rs. Record the expected response opcode in RequestMap at insert; in read_inbound complete a waiter only if msg.msg_type matches it, else route to the inbound receiver. Do NOT hardcode an opcode list — that recreates the closed-allow-list problem. Test: two links at the same outstanding id, one sends a REQUEST with it — assert it reaches the inbound receiver and the waiter stays pending.
  2. Merge dpp fix(stream): bound StreamReassembler (chunk/byte/stream caps, DoS) #8 → auto-tag v0.6.0 → verify LIVE on crates.io (curl -H 'User-Agent: dig-loop' https://crates.io/api/v1/crates/dig-peer-protocol/0.6.0 — crates.io 403s without a User-Agent).
  3. dig-gossip: re-pin "0.5""0.6", pass Direction::Inbound at inbound_limits.rs:83, fix the 4 stale doc sites, add the missing regression test (drive frames past refusal, then assert a subsequently legal frame is still refused, incl. the non_tx aggregates).
  4. dig-gossip B2 fix — an implementer was dispatched for this and had written NOTHING before the restart, so it is entirely unstarted.
  5. Also pending on feat(deps)!: delete the vendored chia-protocol and chia-sdk-client forks #63: cosmetic over-eager PeerDigLink rename damage in prose (src/types/peer.rs "DigLink protocol version string", src/service/state.rs "DigMessage cache", src/connection/listener.rs "DigLink Info Relay").
  6. @copilot owns two SPEC.md findings — incl. a false normative claim that dig-peer-protocol is "the single dependency" while Cargo.toml declares chia-protocol/chia-traits directly. Do NOT work SPEC.md in parallel; wait for its push, then re-gate the combined delta.
  7. Re-gate feat(deps)!: delete the vendored chia-protocol and chia-sdk-client forks #63 (full triple — breaking peer-facing transport), then merge.
  8. docs(vendor): the chia fork rebase is a cross-repo cascade; no fork can be deleted #62 — CLOSE unmerged. Its verdict ("no fork can be deleted") is falsified and it edits READMEs in directories feat(deps)!: delete the vendored chia-protocol and chia-sdk-client forks #63 deletes. Its one durable lesson (the [patch]-silently-dropped trap) has ALREADY been salvaged into feat(deps)!: delete the vendored chia-protocol and chia-sdk-client forks #63's DEVELOPMENT_LOG.md. Safe to close.
  9. fix(connection): adopt DigLink so one bad frame cannot tear down a peer link #60 — rebase onto merged feat(deps)!: delete the vendored chia-protocol and chia-sdk-client forks #63. Its link_adapter.rs is likely subsumed, but tests/sec_2391_frame_tolerance_tests.rs + the hostile-frame harness survive and become the acceptance test for the new transport. If tolerance does NOT hold on DigLink, that is a finding, not a rebase conflict.

SHIPPED THIS LANE

dig-peer-protocol 0.5.0 live on crates.io (both gates PASS). Nine tickets filed: #2647 (native-tls publish blocker — a published dig-gossip would silently accept certless inbound peers), #2665, #2706/#2707/#2708, #2709 (Windows/macOS ban evasion — peer_id_for_addr folds the ephemeral source port into peer identity, so a banned peer reconnects to a fresh identity; Linux fails closed), #2710, #2718, #2720, #2721. #1234 closed as a stale duplicate.

Superproject pointer bump owed (main orchestrator only — I am a sub-orchestrator and did not touch modules/): dig-peer-protocol → v0.5.0 (cd4586c1128764c3499d74e7d0b7e33dc8e860e0).

…n both dial legs

Recovered WIP from a lane that stopped before pushing. Introduces `DialError`, the
two-armed union a dial returns so a policy verdict is not downgraded into a transport
error on the way out.

Before this, the outbound leg and the introducer client had only `LinkError` available
in their return type, so `ClientError::WrongNetwork` / `WrongNodeType` were rendered as
`LinkError::Io(Custom(<formatted string>))`. That put "never reached the peer" and "the
peer failed our policy" in one variant distinguishable only by matching the error
string, which is not a contract, and it contradicted the settled decision on this PR
that the two stay distinct because they carry opposite retry semantics.

The inbound listener already returned `ClientError` for a policy rejection, so this
brings the outbound leg into line rather than inventing a new shape.

WIP: not yet compiled or tested on this branch; pushed first so the work survives.

Co-Authored-By: Claude <noreply@anthropic.com>
`cargo nextest` stops at the first failing target by default, and the run then
reports a pass count that is indistinguishable from a complete one. On this
branch a red head stopped at 531 of 977: 446 tests never executed, and because
the run aborted before finishing, `--fail-under-lines 80` never measured the
coverage floor either.

That is worse than a plain failure. "Unmeasured" and "measured and under" are
different verdicts, and the job was rendering the first as though it were a
count you could reason about. Every suite total quoted from such a run is a
lower bound with no upper bound attached.

`--no-fail-fast` makes the job finish counting before it fails. It does not
weaken the gate: nextest still exits non-zero on any real failure, `--retries 2`
still classifies transient loopback flakes, and the coverage floor now gets a
complete measurement to apply itself to. Added to the rustls `--lib` job for the
same reason.

Refs dig_ecosystem#2228

Co-Authored-By: Claude <noreply@anthropic.com>
@MichaelTaylor3d

Copy link
Copy Markdown
Contributor Author

Lane status — 7b2a171. Written so a fresh agent resumes with no re-derivation.

Still DRAFT. It stays a draft until the gate round returns; undrafted only immediately before merge.

Recovered work that was never pushed

The dg-fork worktree held uncommitted changes from a lane that stopped before pushing: a new
src/connection/dial_error.rs plus edits to connection/mod.rs, connection/outbound.rs and
discovery/introducer_client.rs. That is the B2 fix, and it was one power cut away from being lost.
Committed and pushed as 2ce46c3; cargo check --all-targets --all-features exits 0 on it.

DialError is a two-armed union a dial returns so a policy verdict is not downgraded on the way out —
Client for "reached the peer, policy rejected", Link for "never reached the peer". It is a better
shape than the reviewer's suggested .map_err(ClientError::from) because it makes the distinction part
of the dial's return TYPE rather than a mapping each call site has to remember.

The measurement instrument was the first thing fixed (7b2a171)

cargo nextest stops at the first failing target by default, so the previous run's 531 of 977 was a
floor with no upper bound: 446 tests never executed, and --fail-under-lines 80 never measured coverage
at all. A job that aborts mid-suite prints a pass count indistinguishable from a complete one.

--no-fail-fast is now on both the coverage job and the rustls --lib job. It does not weaken the gate —
nextest still exits non-zero on any real failure — it just finishes counting first, so "unmeasured" and
"measured and under" stop looking alike. Every suite total quoted on this PR before 7b2a171 is a lower
bound and should not be cited.

Blocker status

# Blocker Where the fix lives State
B1 Inbound rate limiter lost incoming = true, so refused frames are no longer charged and a flood is free dig-peer-protocol NOT in this repo
B2 Handshake-POLICY failures wrapped as LinkError dig-gossip fixed at 2ce46c3, compiles; doc/spec sweep in flight
B3 RequestPeers correlation-id carve-out — read_inbound does requests.remove(id) for every opcode dig-peer-protocol NOT in this repo

B1 and B3 have no dig-gossip-local fix, and this was measured rather than assumed.
OpcodeRateLimits's five fields (default_settings, non_tx_frequency, non_tx_max_total_size, tx,
other) are all private with no accessors, so dig-gossip cannot re-key the table itself without
duplicating the crate's From<&RateLimits> — and that crate's own module doc explains why a
locally-assembled table is dangerous: keys re-keyed from a different chia_protocol version shift bytes,
every Chia opcode misses its entry and falls to default_settings, a silent loosening with no compile
error
. RequestMap and read_inbound are likewise pub(crate) inside that crate. Re-implementing
either here would be exactly the second-implementation drift this ecosystem's crate rules forbid.

The one thing standing between this PR and merge

dig-peer-protocol PR #8 (fix/rate-limit-direction, head 786aa55c) fixes both B1 and B3, and
its state is better than this PR's notes claimed — the earlier checkpoint recorded it as "WIP,
mid-implementation, does not necessarily compile"
at c53544b. It has since advanced and is now
mergeable with all 8 checks green: Format/Clippy/Test/Docs, Check version increment, Lint commit
messages, Coverage (>=80% lines), CodeQL ×3.

  • B1 → pub enum Direction { Inbound, Outbound } as the first parameter of OpcodeRateLimiter::new.
    Deliberately an enum rather than upstream's positional bool, because a bare true reading as nothing
    at a call site is precisely how this defect vanished unnoticed in the first place.
  • B3 → Request records the opcodes that may complete it, and remove(id) becomes
    take_matching(id, msg_type). No hardcoded opcode list, so it does not recreate the closed-allow-list
    problem the fork existed to escape.

Its body says DO NOT MERGE — gate round pending, and that must be respected. It is a !-breaking
change to a published crate on the peer wire; merging it publishes an immutable 0.6.0 to crates.io. It
needs its gate round, not a fast merge — a gate verdict that arrives after publication is a post-mortem.

Once 0.6.0 is live, the delta here is small and mechanical:

  1. Cargo.toml: dig-peer-protocol = { version = "0.6", default-features = false }, then re-lock
  2. src/connection/inbound_limits.rs:83: pass Direction::Inbound
  3. restore the four doc sites to assert the ratchet again (inbound_limits.rs:78,112,
    service/state.rs:137, tests/con_005_tests.rs:32)
  4. add the B1 regression test: drive frames past refusal, then assert a subsequently legal frame is
    still refused, including both non_tx aggregates
  5. version bump last

Settled, do not relitigate

  • LinkError vs ClientError stay distinct. Do not "fix" a red test by accepting either variant, and
    never match on the error string.
  • OpcodeRateLimits was solved by adding the public From<&RateLimits> to the owning crate (shipped as
    0.5.0). con_005_tests.rs ports 15/15 with one test strengthened. No coverage was dropped.
  • The wire did not move: 9/9 golden literals byte-identical, and the decode tests read the same
    const the encode side asserts, so a wrong decoder cannot self-agree with a wrong encoder.
    dig-relay needs no change.

Sibling PRs

  • docs(vendor): the chia fork rebase is a cross-repo cascade; no fork can be deleted #62 — CLOSED unmerged. Its central finding ("no fork can be deleted") is falsified by this PR, four
    of its seven files live in directories this PR deletes, and its test file is typed on the closed
    ProtocolMessageTypes enum so it could not compile here anyway. Everything durable in it was already
    salvaged into this PR's DEVELOPMENT_LOG.md; the close comment itemises what went where. One unique
    fragment is still owed: the "Why upstream cannot replace this" section of
    vendor/native-tls/README.dig-gossip.md, whose consumer-visible half (dig-node takes dig-gossip with
    default-features = false, features = ["rustls", "relay"] and so reaches the rustls acceptor, the one
    path where the native-tls patch is not load-bearing) is worth keeping.
  • fix(connection): adopt DigLink so one bad frame cannot tear down a peer link #60 — waits for this PR. Its src/connection/link_adapter.rs translated DigMessage
    chia_protocol::Message at one seam so the rest of the crate could stay on Message. This PR migrates
    the whole crate to DigMessage, so that adapter is entirely subsumed — there is no Message left
    to translate. What survives and matters is tests/sec_2391_frame_tolerance_tests.rs plus the
    hostile-frame harness, which become the acceptance test for the new transport and for B3's fix. If
    tolerance does not hold on DigLink, that is a finding, not a rebase conflict.

…laims

SPEC.md is the contract an independent reimplementation is built against, so
each of these sentences was a real defect rather than a cosmetic one.

* Dependency routing. SPEC.md claimed dig-peer-protocol was "the single
  dependency" reaching the Chia ecosystem, and that the Chia crates were
  "reached through the re-exports above", while its own table eight lines
  later marked chia-protocol and chia-traits as direct reuse. Cargo.toml
  declares both directly and lib.rs re-exports the wire types straight from
  chia_protocol, so the prose was the wrong half. Also corrects chia-ssl,
  which the table called direct reuse but which is not in Cargo.toml at all,
  and the Cargo.toml comment making the same over-claim.

* Superseded transport type. The crate transports dig_peer_protocol::DigMessage;
  chia_protocol::Message appears zero times under src/. A reimplementer reading
  the old sentence would build on Chia's closed #[repr(u8)] discriminant, which
  is precisely the type this branch exists to stop using, and would hit the
  decode rejection the vendored fork was created to work around.

* Rate limiter. Four code sites and three CON-005 rows asserted the inbound
  gate charges refused frames on both halves. Only DigRateLimiter is
  inbound-shaped today; OpcodeRateLimiter exposes no such flag in
  dig-peer-protocol 0.5, and OpcodeRateLimits' fields are private so there is
  no local substitute. Each site now states what is true and marks the gap
  with TODO(dig_ecosystem#2228) rather than asserting a security property the
  code does not have.

* Rename collateral. A Peer -> DigLink rename ran through prose it should not
  have, mangling quoted SPEC headings ("Peer identity from mTLS", "Peer
  sharing via gossip", "Message cache") into names those sections do not have.

Co-Authored-By: Claude <noreply@anthropic.com>
@MichaelTaylor3d

Copy link
Copy Markdown
Contributor Author

First complete measurement of this branch — 7b2a171

The --no-fail-fast change took effect, so these are totals, not floors. Run 31549135687, job
Test Suite + Coverage (>=80%):

Summary [ 780.956s] 977 tests run: 976 passed, 1 failed, 3 skipped

Line coverage: 88.26% — the ≥80% floor is now measured and passing. It had never been measured on
this branch before: the previous run aborted at 531/977, so --fail-under-lines 80 never ran at all.
Every count quoted on this PR before 7b2a171 was a lower bound and should not be cited.

The one failure, and why it was invisible until now

dig-gossip::dsc_005_tests::test_register_introducer_connect_fail, failing all three attempts
(--retries 2), so not flaky:

tests/dsc_005_tests.rs:231: unexpected error:
LinkError(WebSocket(Io(Os { code: 111, kind: ConnectionRefused, message: "Connection refused" })))

The test dials wss://127.0.0.1:7/ws with nothing listening and accepts only
GossipError::ClientError(_) | GossipError::IntroducerError(_). A refused TCP connect is unambiguously
"never reached the peer", which is exactly what the settled design assigns to LinkError. The code
is right and the test expectation is stale.

This is the same defect as dsc_004_tests::test_query_introducer_connect_fail, which was already fixed on
this branch in 1e29f2e by adding LinkError to the accepted set. The dsc_005 twin was missed because
the fail-fast run stopped inside dsc_004 and never reached dsc_005
— it sat in the 446 tests that never
executed. That is precisely the class of defect the instrument change exists to surface, found on its
first run.

Fix is the sibling's shape exactly, plus the DSC-005 acceptance row in the same unit of work, as 1e29f2e
did for DSC-004.

To be explicit about what this is NOT: this is not "relaxing a test to make the variant error go
away". The prohibition on this PR is against accepting either variant for a handshake-policy
rejection, where ClientError is the only correct answer — that is
test_query_introducer_handshake_wrong_network, and it still demands ClientError specifically. A
connect-refused test is the other side of the same distinction, and LinkError is the correct answer
there.

Where that leaves the branch

With this one test corrected, the suite is 977/977 with coverage at 88.26% — and the only remaining
work is the dig-peer-protocol 0.6.0 re-pin for B1 and B3, neither of which has a dig-gossip-local fix.

`test_register_introducer_connect_fail` dials `wss://127.0.0.1:7/ws` with nothing
listening and accepted only `ClientError | IntroducerError`. A refused TCP connect
is unambiguously "never reached the peer", which is what the settled design assigns
to `LinkError` — so the code was right and the expectation was stale.

This is the same defect `1e29f2e` fixed in `dsc_004_tests::test_query_introducer_connect_fail`,
and it takes that commit's shape exactly, including the DSC-005 acceptance row.

Two things are worth recording about why it survived that commit:

- The fail-fast nextest run stopped inside `dsc_004` and never reached `dsc_005`, so
  the twin sat in the 446 tests that never executed. It surfaced on the first run
  after `--no-fail-fast` landed.
- A local `--all-features` run reports it PASSING. That configuration enables
  `native-tls` and `rustls` together, which takes a different connector path than CI's
  `--no-default-features --features native-tls,...`. An `--all-features` green is not
  evidence for this crate.

This is not a relaxation. The prohibition on this branch is against accepting either
variant for a handshake-POLICY rejection, where `ClientError` is the only correct
answer; `test_query_introducer_handshake_wrong_network` still demands it specifically
and still passes.

Verified in CI's exact feature set: `13 tests run: 13 passed, 0 skipped`.

Also salvages the one non-duplicated section of PR #62 before it is discarded: why
upstream `native-tls` cannot replace the vendored patch, and the single consumer
configuration (dig-node, `default-features = false, features = ["rustls", "relay"]`)
in which the patch is not load-bearing — verified against dig-node's manifest.

Refs dig_ecosystem#2228

Co-Authored-By: Claude <noreply@anthropic.com>

@MichaelTaylor3d MichaelTaylor3d left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CHANGES-REQUIRED — correctness gate (head da4c23e2c1c27608ad896af249f9e7a44c839063)

Most of this PR verifies clean. What blocks it is one thing: the RequestPeers correlation-id carve-out is NOT "structurally moot", and the code being deleted in this very PR contains the comment explaining why.

What I verified GREEN (independently)

  • Wire did not move. Diffed every hex literal in tests/wire_golden_vectors_tests.rs at d8f8f33 vs HEAD: identical, sole addition "fe0000000008deadbeef0102037f". The test also got stronger — every vector is now asserted in both directions off one literal. The negative-vector guard the_unassigned_opcode_really_is_unrepresentable_as_a_protocol_message_type is real (ProtocolMessageTypes::from_bytes(&[0xfe]).is_err()), so the 9th vector is falsifiable. dig-relay's byte-identical vendoring is safe.
  • Defect (a), the lost incoming flag: genuinely fixed. dig_rate_limiter.rs:113 charges on self.incoming || passed, and it is pinned from BOTH sides — inbound_charges_rejected_frames_so_junk_buys_no_retries fails if the flag is dropped (charge-only-admitted), outbound_charges_only_admitted_frames fails if it is hardcoded true. check() still returns the verdict. The window test staggers construction by ~1 s of a 2 s window specifically to kill an elapsed-since-construction implementation, with a control assertion that the boundary was not crossed early. This is not a vacuous suite.
  • Defect (b), the WIP salvage: completed, not merely compiling. tests/dsc_004_tests.rs:229 stands up a real WSS introducer serving a wrong network_id and asserts matches!(err, GossipError::ClientError(_)) — the settled variant, not a string, not a tolerant either-arm match. validate_remote_handshake(...)? at outbound.rs:265 routes through From<HandshakeValidationError> for DialErrorClient, preserving WrongNetwork.
  • con_005_tests.rs: 15/15 port is real, not degraded to Default. All five claimed properties still measured against the real V2_RATE_LIMITS-derived table. Zero #[test] functions deleted anywhere in tests/ or src/ (+808/-266 in tests/, plus a new con_005_rate_limit_rekey_tests.rs). No coverage was lost by deletion.

Findings below, ranked.

Comment thread src/connection/listener.rs
Comment thread src/connection/outbound.rs Outdated
Comment thread src/connection/outbound.rs Outdated
Co-authored-by: MichaelTaylor3d <5665004+MichaelTaylor3d@users.noreply.github.com>
@MichaelTaylor3d

Copy link
Copy Markdown
Contributor Author

Gate round returned — BLOCKED on a gating regression

Verdicts at da4c23e2: correctness gate CHANGES-REQUIRED; adversarial gate REFUTED. Security audit still running. PR stays draft.

The blocker (R1) — near-deterministic mutual teardown

Deleting the fork removed an opcode-based carve-out that was load-bearing. Two independent gates reached the same mechanism:

  • dig-peer-protocol-0.5.0/src/link.rs:470-478read_inbound matches on correlation id first, unconditionally. A colliding id is delivered to our waiter and never reaches the application channel.
  • dig-peer-protocol-0.5.0/src/request_map.rs:45next_id starts at 0 on both peers.
  • src/connection/keepalive.rs:224 probes every live peer via request_raw(RequestPeers::new()); both peers start the timer at handshake with the same interval, so the first probes are simultaneous and both carry id = 0.
  • Each side's waiter then receives the peer's RequestPeers; keepalive.rs:228 runs RespondPeers::from_bytes on an empty body, errors, and keepalive.rs:257 disconnects. The peer's request never reaches listener.rs:875, so its auto-reply never fires and it disconnects us too.

The deleted fork stated this in its own comment (vendor/chia-sdk-client/src/peer.rs:453-459 at d8f8f33). No existing test can see it — all 977 exercise one link end or a mock. Fix shape is being settled; it is not a wire-format move, so no dig-relay update is implied.

What is confirmed GOOD (do not re-derive)

  • Fork is gone, read from Cargo.lock not inferred: all 18 chia-* resolve from crates.io; the only sourceless packages are the root crate and the sanctioned native-tls patch; vendor/ holds native-tls only.
  • Wire did not move. All 8 pre-existing golden literals identical vs the pre-refactor pin d8f8f33; the 9th (0xfe) is a new negative vector with a real falsifiability guard. src/relay/ has a zero-byte diff, and that — not the binary vectors — is the surface dig-relay vendors byte-identically. dig-relay needs no change.
  • Complete suite: 977 run / 977 passed / 3 skipped, coverage 88.34% lines. A total, not a fail-fast floor (--no-fail-fast added in 7b2a171).
  • The incoming rate-limiter flag is fixed and pinned from both sides by discriminating tests.
  • Closes #2228 is legitimate — the diff fixes both halves of that ticket's subject.

Remaining items

  1. R1 above — blocking.
  2. Wrong-opcode-after-connect classified as Link (outbound.rs:248, introducer_client.rs:131,222). Note ClientError::{InvalidResponse,UnexpectedMessage} both take ProtocolMessageTypes, not a raw u8, so this may have been forced by the migration; needs a decided contract, not a guess.
  3. --no-default-features does not compile and no CI job would notice (pre-existing), yet introducer_client.rs:26 claims it stays coherent.
  4. Missing-handshake typed variant — fixed by Copilot in ce4c08e7; ClientError::MissingHandshake confirmed present at chia-sdk-client-0.28.0/src/error.rs:48 and re-exported by dpp 0.5.

Note on ce4c08e7

That bot commit left three required contexts (Check version increment, Format, Clippy & Docs, Test Suite + Coverage) at action_requiredabsent from the rollup entirely, not red or pending. They re-run on the next commit from the loop identity.

MichaelTaylor3d and others added 5 commits August 11, 2026 20:46
Two dig-gossip peers tore each other's link down. Both allocate correlation
ids from a counter starting at zero and both keepalive loops start at
handshake on a shared interval, so the first probes carried the same id.
DigLink matches inbound frames on correlation id before forwarding, so each
side's waiter received the peer's RequestPeers, the peer's request never
reached the forwarder, its auto-reply never fired, and both sides tore down
at the staleness check while logging a timeout that named the wrong cause.

The probe now goes out with id: None and liveness is observed on the
application inbound broadcast, which skips the id-match arm entirely. An
unavailable broadcast is liveness-neutral: the round is skipped and the peer
is kept, the correct direction for a probe whose only action is to disconnect.

Co-Authored-By: Claude <noreply@anthropic.com>
…ion waiter (#2767)

Co-Authored-By: Claude <noreply@anthropic.com>
…ion (#2767)

Co-Authored-By: Claude <noreply@anthropic.com>
…ort (#2228)

An opcode outside ProtocolMessageTypes cannot be expressed as a typed
ClientError, so it takes a dig-gossip-owned DialError::UnknownOpcode
carrying the raw byte rather than a formatted string.

Also removes the false --no-default-features coherence claim in
introducer_client and the stubs that existed only to serve it: no TLS
backend does not build today (dig_ecosystem#2225), and makes the
crates.io publish guard unconditional so dispatch is not an escape
hatch around the stripped native-tls patch (dig_ecosystem#2647).

Co-Authored-By: Claude <noreply@anthropic.com>
…#2767)

Co-Authored-By: Claude <noreply@anthropic.com>

@MichaelTaylor3d MichaelTaylor3d left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correctness gate (fresh context) — PASS

Head reviewed: 7db09b9333fbdfd6cab868a660e44b0dbe901aec (resolved from the remote, not from the dispatch).
Scope: the combined delta da4c23e2..7db09b9 (six commits, two authors) plus re-verification of the three round-1 findings.

The three round-1 findings are genuinely closed

  1. RequestPeers correlation-id carve-out — closed at 91bca94. send_probe uses DigLink::send (id: None), the reply is observed on the application inbound broadcast, and the id-match arm is structurally unreachable for the probe.
  2. Typed variant downgraded to a formatted string — closed at ce4c08e. Both outbound.rs:242 and introducer_client.rs:131,219 now return ClientError::MissingHandshake; no formatted-string handshake error remains.
  3. Wrong-opcode-after-connect classified as transport — closed at 5a37486. non_handshake_first_frame maps a Chia-band opcode to the typed ClientError::InvalidResponse and an unmappable one to DialError::UnknownOpcode(u8) -> GossipError::UnknownHandshakeOpcode(u8). No error-string matching anywhere; both arms pinned from both sides.

Anti-vacuity: three mutations RE-RUN, not taken on report

All run in my own throwaway worktree (C:/tmp/worktrees/dg-gate63, since removed). No shared checkout was mutated.

Mutation Result
A. revert src/connection/keepalive.rs to ce4c08e (pre-fix) REDcolliding_correlation_ids_do_not_tear_the_link_down panics at tests/con_2767_keepalive_correlation_tests.rs:107, "A must still hold B after 6s of probing". Exactly the claimed message.
B. delete the fail-open last_success = Instant::now() reset in the subscribe_inbound else-branch REDan_unobservable_probe_does_not_disconnect_the_peer panics at :161. The reset is genuinely load-bearing; a bare continue would have been torn down by the staleness check, as the round-1 note predicted.
C. (the important one) pre-fix keepalive AND remove connect_and_aligns b.request_peers_from(...) GREEN — the buggy build passes. This proves the id-alignment step is what makes the regression test discriminate at all, and independently explains why the pre-existing test_keepalive_bidirectional was green with the defect present. The collision is forced, not hoped for.

Baseline green re-run for all of: con_2767_keepalive_correlation_tests (2), con_004_tests (4), dsc_004_tests (6), dsc_005_tests (7), con_006_tests (9), plus the new connection::dial_error (2) and connection::keepalive (4) unit tests. All pass.

CON-006 metering — verified metered exactly once

Dropping record_live_peer_inbound_bytes from keepalive.rs is correct, and I checked the mechanism rather than the summary. An uncorrelated RespondPeers is no longer claimed by a correlation waiter, so it reaches the forwarder, and both forwarders (listener.rs:872 and gossip_handle.rs:1018) charge record_live_peer_inbound_bytes on every allowed inbound frame before the auto-reply and the broadcast. Keeping the manual call would have double-counted; removing it leaves exactly one charge. Not twice, not zero.

Task-3 deletions — nothing live removed

I verified the deletion rationale empirically rather than accepting it. cargo check --no-default-features already fails on origin/main (5 errors) — src/service/state.rs:67 and lib.rs:177 import/re-export ClientState unconditionally. So the cfg(not(any(...))) stubs at the old :283-310 were unreachable dead code serving a doc claim that was false, and replacing that claim with an honest note plus the #2225 tracking reference is the right resolution.

The wire did not move

git diff origin/main -- src/relay/ is empty at this head. Re-confirmed. dig-relay's vendored byte-identical copy is unaffected.

Craft

The new surfaces read well: send_probe / await_respond_peers / subscribe_inbound / non_handshake_first_frame are intent-revealing, every non-obvious branch carries a WHY comment (the Lagged-is-liveness note and the fail-open rationale are both exemplary), and every new public item is doc-commented. The #2767 module doc states the property, not the outcome.

Blast radius

gitnexus was not used (no index in the worktree) — the sanctioned ripgrep + direct-read fallback, and the PR body says so. I re-did it independently: all call sites of the changed symbols enumerated, and no downstream code matches on GossipError at all (checked dig-node and dig-relay), so the two new public enum variants break nothing today. The 0.22.2 -> 0.23.0 bump is the correct breaking level for a 0.x crate.

Four non-gating notes

Posted inline below. None blocks merge; I am resolving all four myself so they cannot trip required_conversation_resolution.

Nothing handed to Copilot — there is no gating finding to hand over.

Note for the orchestrator: Test Suite + Coverage (>=80%) was still IN_PROGRESS when I read the rollup. Assert required checks by name with check-merge-preconditions.sh before merging; the PR is also still a draft.

Comment thread docs/requirements/domains/connection/specs/CON-004.md
Comment thread src/error.rs
Comment thread src/connection/outbound.rs
Comment thread src/discovery/introducer_client.rs
@MichaelTaylor3d
MichaelTaylor3d marked this pull request as ready for review August 12, 2026 04:34
@MichaelTaylor3d
MichaelTaylor3d merged commit 6a231f1 into main Aug 12, 2026
12 checks passed
@MichaelTaylor3d
MichaelTaylor3d deleted the feat/adopt-dpp-040-delete-chia-fork branch August 12, 2026 04:34
MichaelTaylor3d added a commit that referenced this pull request Aug 12, 2026
#64)

* test(sec-2391): stub — re-derive frame-tolerance acceptance test on post-#63 main

Co-Authored-By: Claude <noreply@anthropic.com>

* test(sec-2391): prove one anomalous frame cannot tear down a peer link

DigLink's inbound loop removes three former one-frame kill switches
(unmatched correlation id, unknown opcode, undecodable frame), but
nothing in this crate proved it, so a future change could silently
restore one.

Drives each anomaly over a real TLS websocket and asserts a subsequent
correlated request is still served, plus a negative control proving that
assertion fails against a genuinely torn-down link.

Refs DIG-Network/dig_ecosystem#2391

Co-Authored-By: Claude <noreply@anthropic.com>

* test(sec-2391): guard the hostile fixtures and bump to 0.23.1

Pins each hostile frame to the decoder so a fixture that quietly stopped
being hostile cannot leave the tolerance tests green while exercising
nothing. Test-only change, so patch.

Co-Authored-By: Claude <noreply@anthropic.com>

* test(sec-2391): tighten the hostile opcode guard

Co-authored-by: MichaelTaylor3d <5665004+MichaelTaylor3d@users.noreply.github.com>

* chore: report validated review fix

Co-authored-by: MichaelTaylor3d <5665004+MichaelTaylor3d@users.noreply.github.com>

* chore: drop incidental index count updates

Co-authored-by: MichaelTaylor3d <5665004+MichaelTaylor3d@users.noreply.github.com>

* test(sec-2391): key the hostile opcode guard to ALL_DIG_OPCODES

The guard listed three DIG opcodes by hand, so it covered 3 of the 23
assigned and could not catch its own drift. ALL_DIG_OPCODES is the
canonical set already used for the same purpose in
src/connection/inbound_limits.rs.

Co-Authored-By: Claude <noreply@anthropic.com>

---------

Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
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.

2 participants