Skip to content

fix(p2p): update noq to 1.1.0 - #1171

Merged
jackzampolin merged 3 commits into
mainfrom
iverc/noq-1-1-panic-fixes
Jul 22, 2026
Merged

fix(p2p): update noq to 1.1.0#1171
jackzampolin merged 3 commits into
mainfrom
iverc/noq-1-1-panic-fixes

Conversation

@iverc

@iverc iverc commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Problem

DefraDB is locked to noq 1.0.1, which contains two production crash paths:

  • A late datagram for a removed multipath path reaches .expect("known path") and panics.
  • A duplicate Draining event decrements active_connections twice, causing an underflow panic with overflow checks or a shutdown hang without them.

Both defects were fixed upstream after 1.0.1, but no released upgrade was available when they were reported.

What changed

  • Update noq, noq-proto, and noq-udp to 1.1.0.
  • Pick up the upstream stale abandoned-path packet fix from n0-computer/noq#732.
  • Pick up the upstream duplicate draining-event fix from n0-computer/noq#748.
  • Refresh the transitive lockfile entries required by the new release.

Validation

  • cargo test -p p2p --features iroh-transport
  • cargo test -p defra-p2p-adapter --features iroh
  • cargo clippy -p p2p -p defra-p2p-adapter --all-targets --features p2p/iroh-transport,defra-p2p-adapter/iroh -- -D warnings
  • cargo clippy --all -- -D warnings
  • cargo fmt --all -- --check
  • Confirmed the published noq-proto 1.1.0 source includes regression coverage for both failure modes.

Closes #1090.
Closes #1091.

@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown

Important

Review skipped

Review was skipped due to path filters

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock

CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including **/dist/** will override the default block on the dist directory, by removing the pattern from both the lists.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f44cbca9-9f80-4a0b-b1a5-20f9294fa371

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@iverc iverc self-assigned this Jul 21, 2026
@iverc
iverc requested a review from jackzampolin July 21, 2026 07:40
@jackzampolin

Copy link
Copy Markdown
Collaborator

The noq/noq-proto/noq-udp 1.0.1 → 1.1.0 bump itself looks right (picks up n0-computer/noq#732 and #748, matching what we were waiting on for #1090/#1091, and keeps overflow checks untouched).

One concern before merging: the Cargo.lock diff contains a number of downgrades on crates unrelated to noq — e.g. windows-sys 0.61.2 → 0.48.0/0.59.0/0.60.2 across several untouched packages, socket2 0.6.3 → 0.5.10 in quinn/hyper-util, and one dep entry moving from syn 2.0.117 to syn 1.0.109. A targeted cargo update -p noq -p noq-proto -p noq-udp shouldn't touch those entries, so it looks like the lockfile was regenerated wholesale (or from a stale base). The downgrades are latent risk on paths CI doesn't exercise (Windows builds in particular).

Could you redo the lock change as a targeted update so the diff only carries the noq crates and their genuinely-required transitive bumps? Happy to merge right after.

jackzampolin and others added 2 commits July 22, 2026 10:36
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Restore main's Cargo.lock and apply only the entries the noq 1.1.0
release requires: noq, noq-proto, noq-udp 1.0.1 -> 1.1.0 and
cfg_aliases 0.2.1 -> 0.2.2 (required by noq 1.1.0). Drops the
unrelated windows-sys/socket2/syn downgrades from the original
wholesale lock regeneration.

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

Copy link
Copy Markdown
Collaborator

Lockfile redone in 213ec7e — the diff vs main is now exactly the three noq crates at 1.1.0 plus cfg_aliases 0.2.1→0.2.2 (a hard requirement of noq 1.1.0). No windows-sys/socket2/syn changes remain, and overflow-checks = true is untouched.

Root cause of the original churn, for the record: with the workspace's rust-version = 1.91, cargo ≥1.84's MSRV-aware resolver (incompatible-rust-versions = fallback) de-prioritizes MSRV-incompatible versions whenever it rewrites the lock, and full re-resolution flips ambiguous dep edges (e.g. windows-sys 0.61.2→0.60.2 where both exist in the lock). So even a targeted cargo update -p noq reproduces most of the downgrades locally — the minimal diff required restoring main's lock and applying only the required stanzas, verified with cargo metadata --locked.

Validated on the final lockfile: cargo test -p p2p --features iroh-transport, cargo clippy --all -- -D warnings, cargo fmt --all -- --check — all clean. Merging.

@jackzampolin
jackzampolin merged commit afbd01d into main Jul 22, 2026
5 of 6 checks passed
@iverc
iverc deleted the iverc/noq-1-1-panic-fixes branch July 30, 2026 03:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants