Skip to content

fix(proto): Ignore PATH_CIDS_BLOCKED when it refers to abandoned paths#710

Open
matheus23 wants to merge 2 commits into
mainfrom
matheus23/regression-path-cids-blocked
Open

fix(proto): Ignore PATH_CIDS_BLOCKED when it refers to abandoned paths#710
matheus23 wants to merge 2 commits into
mainfrom
matheus23/regression-path-cids-blocked

Conversation

@matheus23

@matheus23 matheus23 commented Jun 17, 2026

Copy link
Copy Markdown
Member

Description

When receiving a PATH_CIDS_BLOCKED frame we used to check the next_seq value for protocol compliance like so:

if next_seq.0
    > self
        .local_cid_state
        .get(&path_id)
        .map(|cid_state| cid_state.active_seq().1 + 1)
        .unwrap_or_default()
{
    return Err(TransportError::PROTOCOL_VIOLATION(
        "PATH_CIDS_BLOCKED next sequence number larger than in local state",
    ));
}

But the .unwrap_or_default() would fail if we don't actually store path state for the given path. This can occur when the path has already been abandoned and discarded.
Usually, we wouldn't hit this case because the most likely value for next_seq for PATH_CIDS_BLOCKED is 0, unless it is lost and retransmitted.

The added regression test simulates all of these rare circumstances at once:

  • All PATH_NEW_CONNECTION_ID frames from server to client are delayed such that opening a path on the client side generates a PATH_CIDS_BLOCKED frame
  • The PATH_CIDS_BLOCKED frame is lost as well to ensure it is resent with a newer next_seq number after the delayed PATH_NEW_CONNECTION_ID frames have come in
  • The path is abadoned and discarded on both ends before we send the delayed PATH_CIDS_BLOCKED frame triggering the bug

The fix is to correctly check for !self.abandoned_paths.contains(&path_id) in the above condition.

Breaking Changes

None

Notes & open questions

This bug report is what originally prompted this work: n0-computer/iroh#4347

It should now be fixed.

Change checklist

  • Self-review.
  • Tests if relevant.
  • All breaking changes documented.

@matheus23 matheus23 self-assigned this Jun 17, 2026
@matheus23 matheus23 force-pushed the matheus23/regression-path-cids-blocked branch from 3a75cc5 to d2d97cf Compare June 17, 2026 11:24
@github-actions

github-actions Bot commented Jun 17, 2026

Copy link
Copy Markdown

Performance Comparison Report

3a75cc5c28828bd3c90ed6443bfd1ee4a6753fd5 - artifacts

No results available

---
d2d97cf5c1a1db7edea7e8e64863e25d5798cf6e - artifacts

Raw Benchmarks (localhost)

Scenario noq upstream Delta CPU (avg/max)
large-single 5280.0 Mbps 7934.9 Mbps -33.5% 92.2% / 97.3%
medium-concurrent 5334.9 Mbps 7741.9 Mbps -31.1% 93.8% / 101.0%
medium-single 4026.0 Mbps 4551.9 Mbps -11.6% 93.0% / 102.0%
small-concurrent 3786.3 Mbps 5292.3 Mbps -28.5% 97.2% / 155.0%
small-single 3483.4 Mbps 4733.1 Mbps -26.4% 99.0% / 154.0%

Netsim Benchmarks (network simulation)

Condition noq upstream Delta
ideal 3058.9 Mbps 4023.0 Mbps -24.0%
lan 782.4 Mbps 810.3 Mbps -3.4%
lossy 69.8 Mbps 68.6 Mbps +1.9%
wan 83.8 Mbps 83.8 Mbps ~0%

Summary

noq is 26.5% slower on average

---
c49030feba544e762115a262ef73c9d310e07cf2 - artifacts

Raw Benchmarks (localhost)

Scenario noq upstream Delta CPU (avg/max)
large-single 5574.8 Mbps 7935.9 Mbps -29.8% 96.6% / 150.0%
medium-concurrent 5380.2 Mbps 7717.7 Mbps -30.3% 97.5% / 149.0%
medium-single 3944.9 Mbps 4546.1 Mbps -13.2% 99.3% / 151.0%
small-concurrent 3932.7 Mbps 5330.1 Mbps -26.2% 95.4% / 102.0%
small-single 3475.0 Mbps 4710.8 Mbps -26.2% 94.1% / 102.0%

Netsim Benchmarks (network simulation)

Condition noq upstream Delta
ideal 3072.7 Mbps 4070.6 Mbps -24.5%
lan 782.4 Mbps 810.3 Mbps -3.4%
lossy 69.8 Mbps 55.9 Mbps +25.0%
wan 83.8 Mbps 83.8 Mbps ~0%

Summary

noq is 25.4% slower on average

@github-actions

github-actions Bot commented Jun 17, 2026

Copy link
Copy Markdown

Documentation for this PR has been generated and is available at: https://n0-computer.github.io/noq/pr/710/docs/noq/

Last updated: 2026-06-17T11:48:57Z

@matheus23 matheus23 changed the title test(proto): Write regression test for receiving delayed PATH_CIDS_BLOCKED fix(proto): Ignore PATH_CIDS_BLOCKED when it refers to abandoned paths Jun 17, 2026
@matheus23 matheus23 marked this pull request as ready for review June 17, 2026 11:47
@n0bot n0bot Bot added this to iroh Jun 17, 2026
@github-project-automation github-project-automation Bot moved this to 🚑 Needs Triage in iroh Jun 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: 🚑 Needs Triage

Development

Successfully merging this pull request may close these issues.

1 participant