Skip to content

fix: avoid wasteful reprovides outside threshold #3238

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

2color
Copy link
Contributor

@2color 2color commented Aug 6, 2025

What

This PR fixes a couple of DHT bugs related to how provider records are handled and processed:

  • fix: bug in the logic in packages/kad-dht/src/reprovider.ts that causes wasteful reproviding of records that long before reaching the threshold. This would cause valid records to be provided every REPROVIDE_INTERVAL
  • fix: don't delete expired records if they're ours to ensure we persist the user intent to provide CIDs even if the node goes down beyond expiration. These will be reprovided in the next processRecords run
  • fix: return records within PROVIDERS_VALIDITY and delete MAX_RECORD_AGE which seems to be from before the increase of record validity to 48 hours, and is in essence an outdated duplicate. This fix ensures that provider records are returned in RPC requests as long as they are within the 48 hour PROVIDERS_VALIDITY.

Other small changes:

  • Rename cleanUp (which is misleading given that it triggers the reprovides) to a more descriptive name processRecords
  • Add a few more logs and reduce the level of logs from trace to normal logs

Notes & open questions

  • For self provider records, should we check the block store to ensure they haven't been garbage collected or removed?

Change checklist

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation if necessary (this includes comments as well)
  • I have added tests that prove my fix is effective or that my feature works

2color added 6 commits August 6, 2025 18:31
The condition `(now - expires) < this.reprovideThreshold` will be true for **all non-expired records**, regardless of how far from expiration they are
The old MAX_RECORD_AGE seems to be from before the increase of record
validity to 48 hours, and is in essence an outdated duplicate.

This fix ensures that provider records are returned in RPC
requests as long as they are within the 48 hour PROVIDERS_VALIDITY
@2color 2color requested a review from a team as a code owner August 6, 2025 17:28
@2color 2color requested a review from achingbrain August 11, 2025 13:14
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.

1 participant