optimize slashing protection interchange import
This PR addresses a TODO in slashing_protection_common.nim related to the interchange import logic.
Changes
- Replaced inefficient
O(n log n)sort ofsigned_blockswith a linearO(n)scan to select the block with the highest slot. - Removed redundant sorting of
signed_attestations. The following loop already determines the max epochs. - Eliminated unnecessary imports (
algorithm) to reduce compile scope.
Rationale
These changes improve performance and reduce complexity with no changes to functionality.
Testing
All slashing protection tests pass:
tests/slashing_protection/test_slashing_protection_db.nim
This is a performance-only cleanup and has been verified locally with full test suite.
Unit Test Results
15 files ±0 2 660 suites ±0 1h 18m 38s :stopwatch: - 2m 49s 6 679 tests ±0 6 122 :heavy_check_mark: ±0 557 :zzz: ±0 0 :x: ±0 45 936 runs ±0 45 164 :heavy_check_mark: ±0 772 :zzz: ±0 0 :x: ±0
Results for commit 31071cdb. ± Comparison against base commit a92517b3.
:recycle: This comment has been updated with latest results.
Quick reminder about PR #7312 (“optimize slashing protection interchange import”):
- Switched signed_blocks selection from O(n log n) sort to an O(n) scan for max slot
- Removed redundant signed_attestations sort (max epochs already determined)
- Eliminated the unused
algorithmimport
All tests in tests/slashing_protection/ still pass. Let me know if you need anything else! 🚀
The core idea of replacing O(n log n) sorting with O(n) selection is sound, but this particular PR has lots of other unwarranted comment changes.
It's true that not using std/algorithms might slightly compile time, but those std/ imports are fairly fast, and not in themselves much concern, and practically, it's likely imported anyway by the time beacon_chain/validators/slashing_protection_common.nim is imported due to other modules in Nimbus, so it itself isn't that compelling on its own.
@tersec Thanks for the review! I’ve addressed the feedback:
- Removed the stray inline commentary.
- Restored the pruning explanations for blocks/attestations.
- Re-added the formal proof link and the genesis special-case in attestation checks (slashing-proofs: https://github.com/michaelsproul/slashing-proofs).
- Restored the EIP-3076 advice context next to the ZeroDigest use (EIP-3076: https://eips.ethereum.org/EIPS/eip-3076#advice-for-complete-databases).
- Kept the O(n) selection (no sorting) and left std/algorithm out since it’s no longer needed.
I also restored the chronicles.formatIt PubKey0x for log parity; Please let me know if you’d like any other comments tweaked.