Skip to content

kad: Fix local provider eviction and removal panics - #666

Merged
DenzelPenzel merged 2 commits into
masterfrom
denzelpenzel/kad-local-provider-tracking
Sep 30, 2026
Merged

DenzelPenzel merged 2 commits into
masterfrom
denzelpenzel/kad-local-provider-tracking

Conversation

@DenzelPenzel

@DenzelPenzel DenzelPenzel commented Sep 18, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #665.

  • Keep local providers separate from remote records so eviction or expiry cannot remove an active local registration or cause a panic on removal.
  • Enforce separate local/remote provider-key limits and report QueryFailed without publishing rejected registrations.
  • Add regression coverage for eviction, expiry, refresh at capacity, and registration rejection.

@DenzelPenzel
DenzelPenzel force-pushed the denzelpenzel/kad-local-provider-tracking branch from d7ec8cc to ba5deac Compare September 18, 2026 16:04
@DenzelPenzel
DenzelPenzel requested review from dmitry-markin, gab8i and lexnv and removed request for lexnv September 18, 2026 16:12

@gab8i gab8i left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

With this change max_provider_keys bounds remote keys and local registrations independently, so a node can hold up to 2 × max_provider_keys in total. This should be documented in the docs of MemoryStoreConfig::max_provider_keys, with_max_provider_keys, and in the changelog. Alternatively, would a dedicated limit for local keys be clearer than reusing the same value?

Comment thread src/protocol/libp2p/kademlia/store.rs Outdated
Comment thread src/protocol/libp2p/kademlia/store.rs Outdated
@DenzelPenzel

Copy link
Copy Markdown
Contributor Author

With this change max_provider_keys bounds remote keys and local registrations independently, so a node can hold up to 2 × max_provider_keys in total. This should be documented in the docs of MemoryStoreConfig::max_provider_keys, with_max_provider_keys, and in the changelog. Alternatively, would a dedicated limit for local keys be clearer than reusing the same value?

Thanks for the review! Went with documenting the current behaviour

@AndreiEres AndreiEres left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Looks good

@gab8i gab8i left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM!

@DenzelPenzel
DenzelPenzel force-pushed the denzelpenzel/kad-local-provider-tracking branch from 71c66d8 to 52bbc44 Compare September 30, 2026 14:13
@DenzelPenzel
DenzelPenzel merged commit 1938dee into master Sep 30, 2026
11 checks passed
@DenzelPenzel
DenzelPenzel deleted the denzelpenzel/kad-local-provider-tracking branch September 30, 2026 14:42
DenzelPenzel added a commit that referenced this pull request Sep 30, 2026
Sort the generated entries into Added / Fixed and add a summary paragraph.
Drop the duplicate entry for #666, which the #665 entry already describes.
Restore the 0.15.3 section: that release was cut from a separate branch, so
its changelog never landed on master.
skunert added a commit that referenced this pull request Oct 2, 2026
## [0.16.0] - 2026-09-30

- kad: Fix local provider eviction and removal panics
([#666](#666))
- Report `SubstreamOpenFailure` when switching to secondary connection
([#664](#664))
- Ensure we report connection closed to the upper layers
([#663](#663))
- kad: Implement client-server mode for Kademlia
([#611](#611))


---

> **Reviewer action required:** the changelog entries above are
auto-generated from merged PRs and are **unsorted**. Please edit
`CHANGELOG.md` on this branch to:
> - Group the entries under `### Added` / `### Changed` / `### Fixed`.
> - Add a short summary paragraph at the top of the `## [0.16.0]`
section.
>
> Then **commit the adjusted `CHANGELOG.md` to this PR**.

**After merging:** run the **Release Publish** workflow from the Actions
tab with version `0.16.0` to tag `master` and publish to crates.io. See
`RELEASING.md`.

---

> _CI note: GitHub does not run `pull_request` workflows on PRs opened
by `GITHUB_TOKEN`. If CI hasn't started on this PR, push an empty commit
to trigger it:_
> ```bash
> git fetch origin && git checkout release-v0.16.0
> git commit --allow-empty -m "trigger ci" && git push
> ```

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: DenzelPenzel <denis.samsonov@parity.io>
Co-authored-by: Sebastian Kunert <mail@skunert.dev>
ptechrelease-forkwrite-polkadotsdk Bot pushed a commit to paritytech-release/polkadot-sdk that referenced this pull request Oct 6, 2026
## Summary

Bumps `litep2p` from 0.15.3 to 0.16.0 ([release
notes](https://github.com/paritytech/litep2p/releases/tag/v0.16.0)).

0.16.0 adds the client-server mode for Kademlia (paritytech/litep2p#611)
and fixes the panics around local provider records in the Kademlia
memory store (paritytech/litep2p#666)
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.

kad: MemoryStore can evict its own provider record, then panics when removing it

3 participants