go-libp2p
go-libp2p copied to clipboard
fix(pstoreds): Fix cyclic batch threshold handling and add error checking for peer ID decoding
Summary
This pull request improves robustness and correctness in the pstoreds package by:
- Fixing incorrect
cyclicBatchthreshold initialization that could cause unintended immediate commits. - Adding proper error handling during peer ID decoding in
uniquePeerIdsto 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
thresholdfield innewCyclicBatch. - Handle errors from
base32.RawStdEncoding.DecodeStringandpeer.IDFromByteswhen extracting peer IDs. - Add
cyclic_batch_test.gowith a test verifying that batching only happens after the configured threshold is reached. - Add
peerstore_test.gowith a test verifying that invalid datastore entries are gracefully skipped inuniquePeerIds.
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.
Can you please add new tests for this as well? Thanks!
@MarcoPolo I have added the requested unit tests for both cyclic batch threshold handling and peer ID decoding error handling
Hi @MarcoPolo! Quick reminder about PR #3274 (“fix(pstoreds): Fix cyclic batch threshold handling and add error checking for peer ID decoding”):
- Correctly initialize
cyclicBatchthreshold to avoid premature commits - Add error handling in
uniquePeerIdsto 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! 🔧