peer: reject invalid X-DART-Hop values at both servers

[Coldwings]

Fixes #50

What

Both peer HTTP servers (Server and StreamServer) parsed X-DART-Hop with hop, _ := strconv.Atoi(...), discarding the error. A malformed value silently became 0 and a negative value stayed negative. The engine bounds relay recursion with hop >= maxHop (64) and every relay increments the value, so a negative start delayed the loop-safety cutoff — a huge negative one (e.g. min int64) effectively forever — exactly when membership skew creates a relay cycle.

This PR adds a shared parseHop decoder used identically by both servers: the header is optional (absent = depth 0); any malformed, negative, or out-of-range value is rejected with 400 Bad Request before the Source is invoked. The maxHop bound itself stays with the engine, so any non-negative hop remains a valid wire value at the transport layer.

Triage evidence

Verified against 84aac4e (main HEAD, the commit the issue names):

• internal/peer/peer.go:103 and internal/peer/stream.go:95 both read hop, _ := strconv.Atoi(r.Header.Get(HeaderHop)).
• internal/engine/engine.go:360,437 and internal/engine/stream.go:70 reject only hop >= maxHop; nothing rejects a negative hop.
• Behavioral repro: with the old code, a GET /peer/v1/block/abcdef/7 carrying X-DART-Hop: -1 was served 200 with the block bytes by both servers, and the source observed Hop: -1. With this PR both servers answer 400 and never call the source.

Classification: implementation defect (protocol validation / loop safety), as the issue states. Not a deliberate simplification — the header is documented as the loop-safety bound, and the validation is cheap. The fix matches the issue's "Expected behavior": both server implementations now share the same validation semantics.

Fixes

• internal/peer/peer.go: new parseHop (absent → 0; malformed / negative / int-overflow → invalid); Server.ServeHTTP rejects invalid hops with 400; BlockRequest.Hop doc updated.
• internal/peer/stream.go: StreamServer.ServeHTTP uses the same parseHop validation — identical semantics on both servers.
• docs/peer.md: wire-form section documents the X-DART-Hop domain (optional, non-negative decimal; 400 otherwise) and why; test-table rows added.
• docs/engine.md: test-table row for the new boundary test.

Regression tests

• TestServerHopValidation (internal/peer): table-driven over both server implementations — absent/0/63/64/4096 accepted with the exact hop observed by the source; -1, -4096, min int64, int64 overflow, abc, 1.5, leading-space, 1_000 → 400 with the source never invoked. Proven to fail on the old code (old code returned 200 and served bytes for hop=-1).
• TestParseHop (internal/peer): decoder edges ("", +5, -0, overflow).
• TestRelayHopBoundary (internal/engine): pins the maxHop gate on both PeerSource and PeerStreamSource — maxHop-1 still relays (origin contacted), maxHop declines without touching the origin.

Verification

Run in the fix worktree (go1.22.12, module targets go 1.22):

gofmt -l internal/ cmd/            # clean
go vet ./...                       # clean
go test ./... -race -count=1       # all 13 packages ok
go test ./... -cover -count=1      # all ok (peer 87.7%, engine 86.1%)

Old-code check: with only the fix stashed, go test ./internal/peer/ -run TestServerHopValidation fails on every invalid-hop case (status = 200, want 400); with the fix restored, the full suite passes.

[Coldwings]

@copilot please review this PR. Focus points: (1) the new parseHop domain check in internal/peer — is rejecting malformed/negative/overflow hops with 400 before the Source runs the right contract, and is it identical on both Server and StreamServer; (2) the decision to keep the maxHop bound in the engine (transport accepts any non-negative hop); (3) the regression tests — do TestServerHopValidation and TestRelayHopBoundary actually pin the fixed behavior. Please do NOT push commits; review comments only.

[Copilot]

@copilot please review this PR. Focus points: (1) the new parseHop domain check in internal/peer — is rejecting malformed/negative/overflow hops with 400 before the Source runs the right contrac...

Review complete; no code changes requested. (1) parseHop correctly rejects Atoi errors and negatives, including overflow, before either Source call; both Server and StreamServer use the same helper. (2) Keeping maxHop in the engine is consistent: transport accepts non-negative depths such as 64, while both engine sources decline misses at hop >= maxHop; local hits remain serveable. (3) TestServerHopValidation exercises both implementations, verifies accepted values are forwarded unchanged, and verifies rejected values never invoke Source. TestRelayHopBoundary covers both source paths and checks origin contact at 63 vs. 64. One documentation/coverage discrepancy: the PR description and docs list TestParseHop, but no such test exists in the commit (rg finds only the docs row). The server table still covers the important behavior, but the claimed direct decoder-edge test (especially +5 and -0) is absent. CI’s completed jobs are green; the remaining race/cover jobs were still in progress when checked.

[Coldwings]

@copilot thanks — you were right: TestParseHop had been accidentally trimmed from the committed file during my old-code repro (I restored the trimmed copy). Re-added the decoder-edge table (+5, -0, overflow, etc.) in 1f340fc; go vet, gofmt, go test ./... -race -count=1 and -cover -count=1 all green locally. Please re-review. Do NOT push commits.

[Copilot]

@copilot thanks — you were right: TestParseHop had been accidentally trimmed from the committed file during my old-code repro (I restored the trimmed copy). Re-added the decoder-edge table (+5, ...

Re-review complete. TestParseHop is now present in 1f340fc and covers empty/zero/ordinary values, +5, -0, negative input, malformed input, and int overflow. The decoder and both server call sites remain correct and consistent; the engine boundary tests still pin the separate maxHop behavior. Targeted peer/engine tests and go vet ./... pass locally, and the CI run has no reported failures (remaining jobs were still in progress when checked). No further changes are needed.