diff --git a/Cargo.lock b/Cargo.lock index d1b6c51..a810e8e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1437,7 +1437,7 @@ dependencies = [ [[package]] name = "dig-gossip" -version = "0.22.2" +version = "0.22.3" dependencies = [ "arti-client", "bincode 1.3.3", diff --git a/Cargo.toml b/Cargo.toml index 1f0cd6c..0386663 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -20,7 +20,7 @@ [package] name = "dig-gossip" -version = "0.22.2" +version = "0.22.3" edition = "2021" license = "Apache-2.0" description = "Peer-to-peer networking and gossip for the DIG Network L2 blockchain" diff --git a/DEVELOPMENT_LOG.md b/DEVELOPMENT_LOG.md index 218ff2c..9a7ea47 100644 --- a/DEVELOPMENT_LOG.md +++ b/DEVELOPMENT_LOG.md @@ -584,3 +584,59 @@ a hand-written README.** Both vendored READMEs understated their fork, and a han wrong twice in one investigation. `vendor/fork-delta.sh ` regenerates it; the vendored trees are unpacked tarballs of a known version, so the same-version registry source is an exact baseline and everything the diff reports is DIG's by construction. + +## Rebasing the vendored chia forks is a cross-repo cascade, not a local bump + +Attempting to move `vendor/chia-protocol` from 0.26.0 to 0.36.1 produced the most dangerous shape a +build failure can take: **none at all**. + +``` +$ cargo metadata --offline # after setting vendor/chia-protocol version = "0.36.1" +warning: patch `chia-protocol v0.36.1 (vendor/chia-protocol)` was not used in the crate graph + Locking 1 package to latest compatible version + Adding chia-protocol v0.26.0 (available: v0.47.0) +EXIT=0 +``` + +`[patch.crates-io]` only substitutes a package where the patched version SATISFIES the existing +requirement. dig-gossip does not depend on `chia-protocol` alone — it depends on +**`dig-peer-protocol`, which pins `chia-protocol = "0.26"` and `chia-sdk-client = "0.28"`** (still +true at the latest published `dig-peer-protocol` 0.3.0). A 0.36.1 patch satisfies neither, so Cargo +**silently drops the patch and resolves the pristine upstream crate** — with a warning, and exit 0. + +Two things follow, and both are load-bearing: + +- **The chia version is not independently choosable here.** Rebasing the forks requires + `dig-peer-protocol` to move first and be republished (release-first, §4.1), then dig-gossip's own + direct `chia-protocol` / `chia_streamable_macro` pins, then the vendored trees. A lane scoped to + dig-gossip alone cannot do it. +- **An unused patch is a warning, not an error.** The only reason this fails loudly at all is that + `ProtocolMessageTypes::RegisterPeer` ceases to exist without the fork, so the code and the golden + vectors stop compiling. That compile break is the real guard — treat it as one, and never "fix" a + patch-not-used warning by deleting the reference that surfaces it. + +### Can each fork die instead of being rebased? Measured, three noes + +- **`chia-protocol` — no, not without moving the wire off the typed `Message`.** Upstream's + `ProtocolMessageTypes` still stops at `RespondCostInfo = 107` at 0.26.0, 0.36.1 **and 0.47.0**, so + there is no collision with DIG's 200-222 and no forced renumber. But the enum is a closed + `#[repr(u8)]` (not `#[non_exhaustive]`) and `Message.msg_type` is typed as it, so `Message::from_bytes` + rejects a DIG opcode without the fork. `dig_peer_protocol::DigMessage` is a raw-`u8` envelope with the + identical layout and is the escape hatch, but adopting it means the inbound decode, the broadcast + classifier and the rate-limit keying all stop being enum-typed — a redesign, not a deletion, and one + that only moves the delta into the `chia-sdk-client` fork we keep anyway. +- **`chia-sdk-client` — no.** Upstream **0.34.0** has none of the three fork items: no + `send_protocol_message`, no `from_server_websocket` (its `Sink`/`Stream` are still the concrete + `SplitSink`/`SplitStream`), and no inbound-`RequestPeers` routing. The "ten files for one method" + framing is wrong twice over — the delta is ONE file, and it is three items, of which the + type-erasure for the rustls inbound acceptor (#1371) is the largest. +- **`native-tls` — no, and it is security-load-bearing.** `native-tls` is dig-gossip's DEFAULT feature + and `native_tls_acceptor` is compiled under `all(feature = "native-tls", not(feature = "rustls"))`, + so a stock `cargo build` uses the patched acceptor. Upstream `TlsAcceptorBuilder` exposes only + `min_protocol_version` / `max_protocol_version` / `accept_alpn` / `build` — there is no way to + request a client certificate. Dropping the patch would silently accept inbound peers with no client + cert at all. Only dig-node escapes this path, via `default-features = false, features = ["rustls", + "relay"]`. + +The `RateLimits::dig_wire` item sometimes listed as a fourth delta **no longer exists** — it was +removed in dig_ecosystem#2228 and now lives in `connection::dig_rate_limiter`. diff --git a/tests/wire_golden_vectors.rs b/tests/wire_golden_vectors.rs new file mode 100644 index 0000000..72197d8 --- /dev/null +++ b/tests/wire_golden_vectors.rs @@ -0,0 +1,158 @@ +//! Golden wire vectors for the DIG extension opcodes — the instrument that proves a vendored-fork +//! rebase did not move the wire. +//! +//! ## Why these exist +//! +//! `dig-gossip` speaks to a LIVE peer network, and its wire is vendored byte-identical into +//! `dig-relay` (GPL-2.0). A change to how a DIG frame encodes is therefore not a refactor — it is a +//! network-wide coordination event. When the vendored `chia-protocol` / `chia-sdk-client` trees are +//! rebased onto a newer upstream, the ONLY question that matters is whether the bytes on the wire +//! stayed the same. +//! +//! These vectors are recorded against the PRE-rebase tree and asserted unchanged afterwards. A +//! vector added *after* a change measures nothing — it merely re-records whatever the new code +//! happens to emit. That is the entire reason this file is committed separately from, and before, +//! any vendor bump. +//! +//! ## What is pinned +//! +//! The `Message` envelope — `[u8 msg_type][bool has_id][u16 id?][u32 data_len][u8… data]` — carried +//! for each of the three DIG opcodes dig-gossip puts on the wire, plus the raw discriminant of every +//! DIG opcode in the vendored `ProtocolMessageTypes`. Two properties are load-bearing and are +//! asserted independently: +//! +//! 1. **The discriminant.** Opcode 220 must stay 220. An upstream that later claims 220 for its own +//! message would force a renumber, which is a wire break. +//! 2. **The envelope encoding.** Field order, the `Option` presence byte, and the big-endian +//! `u32` length prefix must all survive the rebase. + +use dig_peer_protocol::{Bytes, Message, ProtocolMessageTypes, Streamable}; + +/// Encode a `Message` to its wire bytes as a lowercase hex string. +/// +/// Hex rather than a byte array so a failing assertion prints a diff a human can read against the +/// layout comment above, instead of a wall of decimal. +fn frame_hex(msg_type: ProtocolMessageTypes, id: Option, data: &[u8]) -> String { + let msg = Message { + msg_type, + id, + data: Bytes::new(data.to_vec()), + }; + msg.to_bytes() + .expect("a Message over an in-memory payload always encodes") + .iter() + .map(|b| format!("{b:02x}")) + .collect() +} + +/// The DIG opcodes MUST keep the exact discriminants the live network already speaks. +/// +/// Pinned as literals rather than derived from the enum: deriving them from the thing under test +/// would make this assertion circular, and a renumber is precisely the failure it exists to catch. +#[test] +fn dig_opcode_discriminants_are_pinned() { + // DIG L2 consensus band (200–217) — extends Chia's namespace so a stock `Message` can carry a + // DIG consensus opcode (#1404). + assert_eq!(ProtocolMessageTypes::NewAttestation as u8, 200); + assert_eq!(ProtocolMessageTypes::PlumtreeRequestByHash as u8, 217); + + // DIG introducer registration (218–219, DSC-005). + assert_eq!(ProtocolMessageTypes::RegisterPeer as u8, 218); + assert_eq!(ProtocolMessageTypes::RegisterAck as u8, 219); + + // DIG directed envelope + broadcast (220–222). + assert_eq!(ProtocolMessageTypes::DigMessage as u8, 220); // WU6 / epic #796 + assert_eq!(ProtocolMessageTypes::StoreMelted as u8, 221); // epic #1316 + assert_eq!(ProtocolMessageTypes::HoldingsAnnounce as u8, 222); // #1428 +} + +/// `RegisterPeer` (218) — the introducer registration request, sent with a correlation id so the +/// `RegisterAck` can be matched to it. +#[test] +fn register_peer_frame_is_byte_stable() { + // da = 218, 01 = id present, 002a = id 42, 00000004 = 4-byte payload, deadbeef = payload. + assert_eq!( + frame_hex( + ProtocolMessageTypes::RegisterPeer, + Some(42), + &[0xde, 0xad, 0xbe, 0xef] + ), + "da01002a00000004deadbeef" + ); +} + +/// `RegisterAck` (219) — the introducer's reply, which MUST come back on the requester's id. +#[test] +fn register_ack_frame_is_byte_stable() { + // db = 219, id 42 echoed back, 1-byte payload. + assert_eq!( + frame_hex(ProtocolMessageTypes::RegisterAck, Some(42), &[0x01]), + "db01002a0000000101" + ); +} + +/// `DigMessage` (220) — the directed envelope, whose payload is opaque bytes to this layer. +/// +/// Also pinned WITHOUT an id, because the `Option` presence byte is the one part of the +/// envelope whose encoding a streamable-macro change could plausibly alter without any DIG code +/// changing. +#[test] +fn dig_message_frame_is_byte_stable() { + // dc = 220, 01 = id present, 0007 = id 7, 3-byte opaque payload. + assert_eq!( + frame_hex( + ProtocolMessageTypes::DigMessage, + Some(7), + &[0xaa, 0xbb, 0xcc] + ), + "dc01000700000003aabbcc" + ); + + // dc = 220, 00 = NO id (no u16 follows), 3-byte opaque payload. + assert_eq!( + frame_hex(ProtocolMessageTypes::DigMessage, None, &[0xaa, 0xbb, 0xcc]), + "dc0000000003aabbcc" + ); +} + +/// An empty payload must still emit its four-byte length prefix. +/// +/// The zero-length case is where a length-prefix change hides: a codec that switched to a varint +/// would encode `0` as one byte and every non-empty vector above would still look plausible. +#[test] +fn empty_payload_keeps_its_four_byte_length_prefix() { + assert_eq!( + frame_hex(ProtocolMessageTypes::DigMessage, None, &[]), + "dc0000000000" + ); +} + +/// A DIG frame must survive a full round trip through the vendored decoder. +/// +/// The encode-only vectors above prove the bytes we emit are unchanged; this proves the vendored +/// `Message::from_bytes` still ACCEPTS a DIG opcode. That acceptance is the sole reason the +/// `chia-protocol` fork exists, so a rebase that silently dropped the enum extension would pass +/// every assertion above and fail here. +#[test] +fn dig_frames_round_trip_through_the_vendored_decoder() { + for opcode in [ + ProtocolMessageTypes::RegisterPeer, + ProtocolMessageTypes::RegisterAck, + ProtocolMessageTypes::DigMessage, + ProtocolMessageTypes::StoreMelted, + ProtocolMessageTypes::HoldingsAnnounce, + ] { + let original = Message { + msg_type: opcode, + id: Some(9), + data: Bytes::new(vec![0x01, 0x02, 0x03]), + }; + let encoded = original.to_bytes().expect("encodes"); + let decoded = Message::from_bytes(&encoded) + .expect("the vendored enum must accept a DIG opcode off the wire"); + assert_eq!( + decoded, original, + "round trip changed the frame for {opcode:?}" + ); + } +} diff --git a/vendor/chia-protocol/README.dig-gossip.md b/vendor/chia-protocol/README.dig-gossip.md index 1aad761..ee7e297 100644 --- a/vendor/chia-protocol/README.dig-gossip.md +++ b/vendor/chia-protocol/README.dig-gossip.md @@ -45,3 +45,25 @@ changes.** All 23 are purely additive — no renumbering or semantic changes to existing opcodes. The fork exists only because `ProtocolMessageTypes` is upstream-owned; if upstream accepts the opcodes, this fork retires. + +Upstream has **not** claimed any of 200–222: `ProtocolMessageTypes` stops at `RespondCostInfo = 107` +at 0.26.0, 0.36.1 and 0.47.0 alike. There is no collision and no renumber pressure, so the vendored +version can stay where it is indefinitely without risking the wire. + +## Rebasing onto a newer upstream — read this first + +**The chia version is not choosable in this repo alone.** dig-gossip reaches `chia-protocol` through +`dig-peer-protocol`, which pins `chia-protocol = "0.26"` (and `chia-sdk-client = "0.28"`). A +`[patch.crates-io]` entry substitutes a package only where the patched version SATISFIES the existing +requirement, so bumping this tree to 0.36.1 makes Cargo **drop the patch and resolve pristine upstream +instead** — reported as a *warning*, with `cargo metadata` still exiting 0: + +``` +warning: patch `chia-protocol v0.36.1 (vendor/chia-protocol)` was not used in the crate graph +``` + +A rebase is therefore a release-first cascade: `dig-peer-protocol` moves and republishes, then +dig-gossip's own direct `chia-protocol` / `chia_streamable_macro` pins, then this tree. The only +reason the dropped patch is not silent is that `ProtocolMessageTypes::RegisterPeer` stops existing and +the tree stops compiling — including `tests/wire_golden_vectors.rs`. That compile break is a guard. +Never resolve a patch-not-used warning by removing what surfaces it. diff --git a/vendor/chia-sdk-client/README.dig-gossip.md b/vendor/chia-sdk-client/README.dig-gossip.md index 6f2f1a2..2b5fe2e 100644 --- a/vendor/chia-sdk-client/README.dig-gossip.md +++ b/vendor/chia-sdk-client/README.dig-gossip.md @@ -42,6 +42,18 @@ because `Peer`'s fields are private — so all three are genuine upstream candid fork retires. Note that item 3 is a behavioural fix rather than pure API addition, so it needs to be argued as such in that PR (dig_ecosystem#2228 S3). +As of upstream **0.34.0** none of the three has landed: there is no `send_protocol_message`, no +`from_server_websocket`, the split halves are still the concrete `SplitSink` / `SplitStream`, and +inbound `RequestPeers` is still matched against the outbound `RequestMap`. So the fork cannot yet +retire on any of the three counts. + +## Rebasing onto a newer upstream — read this first + +The same cascade constraint as the sibling `chia-protocol` fork applies, for the same reason: +`dig-peer-protocol` pins `chia-sdk-client = "0.28"`, and a `[patch.crates-io]` entry whose version +does not satisfy that requirement is **silently dropped with a warning**, not an error. See +`vendor/chia-protocol/README.dig-gossip.md` for the measured evidence and the ordering. + ## What used to be here and no longer is `RateLimits::dig_wire` and `RateLimiter::check_dig_extension` were removed in dig_ecosystem#2228. diff --git a/vendor/native-tls/README.dig-gossip.md b/vendor/native-tls/README.dig-gossip.md index 8de3bc7..92ee3c5 100644 --- a/vendor/native-tls/README.dig-gossip.md +++ b/vendor/native-tls/README.dig-gossip.md @@ -25,6 +25,19 @@ The patch block is marked **dig-gossip vendor patch**. See the comment at that s `chia_ca.crt` is copied from the matching `chia-ssl` release (the Chia Network's vendored CA bundle). +## Why upstream cannot replace this + +`native-tls` is dig-gossip's **default** feature, and `connection::listener::native_tls_acceptor` is +compiled under `all(feature = "native-tls", not(feature = "rustls"))` — so a stock `cargo build` runs +through this patched acceptor. Upstream's `TlsAcceptorBuilder` exposes only `min_protocol_version`, +`max_protocol_version`, `accept_alpn` and `build`; it offers no way to request or require a client +certificate. Dropping the patch would therefore not fail to compile — it would silently accept inbound +peers presenting **no client certificate at all**, defeating CON-009 mTLS. + +Only a consumer that opts out reaches a different path: dig-node takes dig-gossip with +`default-features = false, features = ["rustls", "relay"]` and uses the rustls inbound acceptor +instead. + ## Platform scope OpenSSL backend is used on Linux/Android; macOS (SecureTransport) and Windows (SChannel) paths are