Skip to content

fix(bitswap): ignore identity CIDs instead of killing connection#1117

Merged
lidel merged 4 commits intomainfrom
fix/bitswap-ignore-identity-cids
Mar 13, 2026
Merged

fix(bitswap): ignore identity CIDs instead of killing connection#1117
lidel merged 4 commits intomainfrom
fix/bitswap-ignore-identity-cids

Conversation

@lidel
Copy link
Member

@lidel lidel commented Mar 9, 2026

third-party implementations may not be aware that identity CIDs should not appear in Bitswap wantlists, and their users should not be punished with dropped connections for naive CID handling.

unify behavior with oversized CIDs to reduce connection churn: silently ignore and continue processing the rest of the message.

  • engine.go: replace error return with debug log + continue
  • engine_test.go: flip assertions to verify no disconnect, check identity CID is silently dropped from wantlist
  • (we also improved test coverage for oversized cids to follow the similar pattern)

third-party implementations may not be aware that identity CIDs
should not appear in Bitswap wantlists, and their users should not
be punished with dropped connections for naive CID handling.

unify behavior with oversized CIDs to reduce connection churn:
silently ignore and continue processing the rest of the message.

- engine.go: replace error return with debug log + continue
- engine_test.go: flip assertions to verify no disconnect, check
  identity CID is silently dropped from wantlist
@lidel lidel requested a review from a team as a code owner March 9, 2026 15:01
@codecov
Copy link

codecov bot commented Mar 9, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 62.45%. Comparing base (95bf79f) to head (a0190ae).
⚠️ Report is 3 commits behind head on main.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1117      +/-   ##
==========================================
- Coverage   62.50%   62.45%   -0.06%     
==========================================
  Files         260      260              
  Lines       26173    26174       +1     
==========================================
- Hits        16359    16346      -13     
- Misses       8130     8139       +9     
- Partials     1684     1689       +5     
Files with missing lines Coverage Δ
bitswap/server/internal/decision/engine.go 91.42% <100.00%> (-0.65%) ⬇️

... and 8 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

lidel added 3 commits March 9, 2026 16:07
- restructure into subtests covering want-block, want-have, cancel, and all-identity-CIDs edge case
- verify the specific CID that remains, not just the count
- verify identity cancel does not corrupt existing wantlist entries
- use t.Context() instead of context.Background()
- restructure into subtests covering want-block, want-have, cancel, and all-oversized edge case
- verify the specific CID that remains, not just the count
- verify oversized cancel does not corrupt existing wantlist entries
@lidel lidel mentioned this pull request Mar 9, 2026
22 tasks
Copy link
Contributor

@guillaumemichel guillaumemichel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks sensible

@lidel lidel merged commit 077caa6 into main Mar 13, 2026
17 checks passed
@lidel lidel deleted the fix/bitswap-ignore-identity-cids branch March 13, 2026 12:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants