go-libp2p icon indicating copy to clipboard operation
go-libp2p copied to clipboard

fix(pstoreds): Fix cyclic batch threshold handling and add error checking for peer ID decoding

Open tomasandroil opened this issue 7 months ago • 2 comments

Summary

This pull request improves robustness and correctness in the pstoreds package by:

  • Fixing incorrect cyclicBatch threshold initialization that could cause unintended immediate commits.
  • Adding proper error handling during peer ID decoding in uniquePeerIds to skip invalid datastore entries instead of causing potential panics.
  • Adding unit tests for:
    • Cyclic batch threshold behavior.
    • Handling of invalid peer IDs in uniquePeerIds.

Changes

  • Correctly assign the threshold field in newCyclicBatch.
  • Handle errors from base32.RawStdEncoding.DecodeString and peer.IDFromBytes when extracting peer IDs.
  • Add cyclic_batch_test.go with a test verifying that batching only happens after the configured threshold is reached.
  • Add peerstore_test.go with a test verifying that invalid datastore entries are gracefully skipped in uniquePeerIds.

Motivation

Robust error handling and correct thresholding are critical in persistent peerstore operations to ensure system stability, especially under datastore corruption scenarios or large-scale peer management.

Adding unit tests ensures that regressions in these critical code paths are less likely to occur in the future.

Testing

All new unit tests pass:

go test ./p2p/host/peerstore/pstoreds/...

No regressions in existing functionality.

tomasandroil avatar Apr 24 '25 18:04 tomasandroil

Can you please add new tests for this as well? Thanks!

MarcoPolo avatar Apr 24 '25 22:04 MarcoPolo

@MarcoPolo I have added the requested unit tests for both cyclic batch threshold handling and peer ID decoding error handling

tomasandroil avatar Apr 28 '25 11:04 tomasandroil

Hi @MarcoPolo! Quick reminder about PR #3274 (“fix(pstoreds): Fix cyclic batch threshold handling and add error checking for peer ID decoding”):

  • Correctly initialize cyclicBatch threshold to avoid premature commits
  • Add error handling in uniquePeerIds to skip invalid peer IDs instead of panicking
  • New unit tests for batch threshold behavior and invalid-peer-ID cases

Let me know if you need anything else! 🔧

tomasandroil avatar Aug 05 '25 19:08 tomasandroil