first revision plan + docs
This commit is contained in:
17 files changed
+6052
No files matched your search
@@ -0,0 +1,189 @@
|
||||
# Clean-Room Implementation Notes
|
||||
|
||||
## Purpose
|
||||
|
||||
This document records the clean-room boundary for the IRTT client
|
||||
reimplementation project. The goal is to ensure that the implementation
|
||||
is developed from a behavioral/protocol specification only, without exposure
|
||||
to the GPL-licensed upstream source code.
|
||||
|
||||
## Clean-Room Boundary
|
||||
|
||||
- **Contaminated side:** The specification agent (this side) inspected the
|
||||
upstream source code, documentation, tests, CLI behavior, and release
|
||||
notes to understand the IRTT protocol.
|
||||
|
||||
- **Clean side:** The implementation agent will receive only the
|
||||
`IRTT_CLIENT_PROTOCOL_SPEC.md` document. The implementation agent MUST NOT
|
||||
see the upstream source code.
|
||||
|
||||
## Source Materials Inspected
|
||||
|
||||
The following materials from the upstream IRTT repository (heistp/irtt,
|
||||
GPLv2) were inspected to produce the protocol specification:
|
||||
|
||||
1. **Documentation files:**
|
||||
- `README.md` — project overview, FAQ, features, limitations
|
||||
- `doc/irtt.md` — main man page
|
||||
- `doc/irtt-client.md` — client man page with CLI options and JSON output format
|
||||
- `doc/irtt-server.md` — server man page (referenced, not fully read)
|
||||
- `CHANGES.md` — changelog
|
||||
|
||||
2. **Source files (inspected for protocol behavior only):**
|
||||
- Protocol version and constants
|
||||
- Packet format, field layout, magic bytes, flags
|
||||
- Parameter serialization format (tag-value pairs, varint encoding)
|
||||
- Connection lifecycle (open, echo, close handshakes)
|
||||
- Client configuration and defaults
|
||||
- Server connection handling and parameter restriction logic
|
||||
- HMAC computation
|
||||
- Timestamp handling and clock types
|
||||
- Received stats (count and window) format
|
||||
- Round-trip data recording and measurement calculations
|
||||
- Result computation (RTT, OWD, IPDV, loss)
|
||||
- Error codes and error handling
|
||||
- Packet test vectors (serialized bytes for known inputs)
|
||||
- Network layer (IP version, DSCP, socket options)
|
||||
- Wait/drain behavior
|
||||
|
||||
3. **Test files:**
|
||||
- Packet serialization tests (used to derive test vectors only; test
|
||||
code itself was not copied)
|
||||
|
||||
## Statement of Non-Copying
|
||||
|
||||
No upstream source code, comments, or pseudocode were copied into the
|
||||
specification document (`IRTT_CLIENT_PROTOCOL_SPEC.md`). The specification
|
||||
describes only externally observable protocol behavior, wire format, and
|
||||
interoperability requirements.
|
||||
|
||||
Test vectors in the specification were derived from the known inputs and
|
||||
expected byte sequences found in the upstream test suite. Only the input
|
||||
parameters and resulting byte arrays were used; no test logic or code was
|
||||
copied.
|
||||
|
||||
## Post-Drafting Audit
|
||||
|
||||
After the initial specification was drafted, a clean-room audit was
|
||||
performed. The following issues were identified and corrected:
|
||||
|
||||
1. **Upstream private type names leaked:** "ctoken" and "seqno" appeared
|
||||
as parenthetical abbreviations in the terminology table. Removed.
|
||||
|
||||
2. **Upstream implementation preference disclosed:** A note about IPv4
|
||||
preference in dual-stack resolution revealed an upstream implementation
|
||||
choice. Removed.
|
||||
|
||||
3. **Go-specific function references:** References to `binary.PutUvarint`,
|
||||
`binary.PutVarint`, and `encoding/binary` package named the upstream
|
||||
language's standard library. Replaced with language-neutral descriptions
|
||||
(LEB128, protobuf-style zigzag).
|
||||
|
||||
4. **Internal scheduling algorithm described:** The send scheduling section
|
||||
described a specific "snap-to-interval" drift-correction algorithm that
|
||||
mirrored upstream control flow. Replaced with observable behavior
|
||||
description (packets sent at approximately ideal times, drift
|
||||
compensation is implementation-defined).
|
||||
|
||||
5. **Upstream timer algorithm named:** "The upstream uses exponential
|
||||
averaging of timer errors" revealed an internal implementation choice.
|
||||
Removed.
|
||||
|
||||
6. **RTT edge case speculated:** The spec asserted that server processing
|
||||
time exceeding raw RTT should cause subtraction to be skipped — a guard
|
||||
the upstream does not actually implement. Moved to Open Questions.
|
||||
|
||||
7. **Received window validity asserted normatively:** The behavior of
|
||||
window=0 as an invalidity sentinel was stated as fact when it should be
|
||||
verification-required. Softened with reference to Open Questions.
|
||||
|
||||
8. **Go concurrency term leaked:** "goroutine" appeared in the result model
|
||||
section. Replaced with "operation."
|
||||
|
||||
9. **Upstream test design referenced:** Section 18.2 described what "the
|
||||
upstream test" was testing internally. Rewritten to describe the test
|
||||
vector neutrally.
|
||||
|
||||
10. **Upstream constants asserted normatively:** The minimum restricted
|
||||
interval (1 second) and maximum parameter buffer size (128 bytes) are
|
||||
internal constants that were stated as protocol requirements. Moved to
|
||||
Open Questions with verification suggestions.
|
||||
|
||||
11. **Timestamp capture order stated as MUST:** Implementation-level timing
|
||||
advice was stated as a protocol requirement. Changed to SHOULD with
|
||||
measurement-accuracy rationale.
|
||||
|
||||
12. **Internal recording structure described:** Section 19.6 described a
|
||||
"pre-send timestamp" used for send-call-time measurement, which is an
|
||||
internal recording detail. Rewritten to focus on observable behavior.
|
||||
|
||||
13. **Self-contradictory correction note:** Section 8.2 contained an inline
|
||||
"Correction:" note that was confusing and revealed the drafting process.
|
||||
Cleaned up.
|
||||
|
||||
14. **Field ordering lacked verification note:** The critical field order
|
||||
in Section 8.1.3 was derived from source and stated without any
|
||||
verification caveat. Added verification note and new Open Question 19.11.
|
||||
|
||||
## Clean-Room Compliance Checklist
|
||||
|
||||
- [x] No upstream source code included.
|
||||
- [x] No upstream comments quoted.
|
||||
- [x] No upstream private function names included.
|
||||
- [x] No upstream private type names included.
|
||||
- [x] No upstream file/module layout described.
|
||||
- [x] No pseudocode derived from upstream implementation.
|
||||
- [x] No implementation-language API design included.
|
||||
- [x] No Go-specific language references remain.
|
||||
- [x] No upstream implementation preferences or algorithm choices disclosed.
|
||||
- [x] Protocol behavior described independently.
|
||||
- [x] Source-derived constants marked for verification.
|
||||
- [x] Ambiguous behavior marked for verification (Section 19).
|
||||
- [x] Black-box tests proposed where needed (Section 17).
|
||||
- [x] Post-drafting audit completed and documented (this section).
|
||||
- [x] Second-pass scrub completed (see below).
|
||||
|
||||
## Second-Pass Clean-Room Scrub
|
||||
|
||||
A second independent review was performed on 2026-04-28 by an agent that
|
||||
had not seen the upstream source. The following issues were identified and
|
||||
corrected:
|
||||
|
||||
1. **Implementation architecture directive in scope:** Section 2 stated
|
||||
"Library-first implementation target," which prescribes internal
|
||||
architecture. Removed; the spec now says only that CLI design is not
|
||||
prescribed.
|
||||
|
||||
2. **Concurrency architecture leaked:** Section 6.3 described "a separate
|
||||
receive path processes incoming reply packets concurrently," which
|
||||
prescribes an internal threading/task model. Rewritten to state the
|
||||
observable requirement (send and receive occur concurrently) without
|
||||
prescribing how.
|
||||
|
||||
3. **Source-derived design rationale in echo request:** Section 8.4
|
||||
explained that zeroed fields exist to "use the same field layout that
|
||||
appears in echo replies," which reveals internal design reasoning.
|
||||
Simplified to state only the observable purpose (reaching the
|
||||
negotiated packet length).
|
||||
|
||||
4. **Source-exhaustive knowledge parenthetical:** Section 8.6 said
|
||||
"(currently only ServerFill)" for string-typed values, revealing
|
||||
knowledge of the complete parameter set from source inspection.
|
||||
Removed the parenthetical.
|
||||
|
||||
5. **Upstream CLI syntax leaked:** Section 10.10 used `NxD`/`NrD`/`D`
|
||||
notation for wait variants, which mirrors upstream CLI option syntax.
|
||||
Replaced with plain-language descriptions.
|
||||
|
||||
6. **Algorithm name disclosed:** Section 12.9 named "Welford's online
|
||||
algorithm" as a MAY recommendation. This is implementation guidance.
|
||||
Replaced with "the method of computing running statistics is
|
||||
implementation-defined."
|
||||
|
||||
7. **Implementation-specific error category:** Section 14 listed
|
||||
"Allocate results buffer failure," implying a pre-allocated buffer
|
||||
strategy. Generalized to "Insufficient resources for test parameters."
|
||||
|
||||
8. **Language-specific references in CLEANROOM_NOTES.md:** The Purpose
|
||||
section named "the Rust implementation" and the checklist mentioned
|
||||
"No Rust API design." Both replaced with language-neutral phrasing.
|
||||
Reference in new issue
Block a user