refactor(p2p): attestation pool refactoring #20083
Open
+526
−864
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ointAttestation Replace separate hasCheckpointAttestation, canAddCheckpointAttestation, and hasReachedCheckpointAttestationCap methods with a single tryAddCheckpointAttestation method that handles all validation and addition in one atomic operation. This simplifies the API and ensures consistent behavior between checking and adding attestations. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
spalladino
added
ci-no-fail-fast
…everywhere - Delete memory_attestation_pool.ts and memory_attestation_pool.test.ts - Add createTestAttestationPool() factory function for test setup - Update p2p_client.test.ts to use createTestAttestationPool() - Update index.ts exports to export KvAttestationPool instead of InMemoryAttestationPool The InMemoryAttestationPool in test-helpers/testbench-utils.ts is kept as a minimal mock for benchmarking purposes. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…Pool to AttestationPool - Merge KvAttestationPool class into attestation_pool.ts and rename to AttestationPool - Move TryAddResult type definition to the class file - Delete kv_attestation_pool.ts and kv_attestation_pool.test.ts files - Add AttestationPoolApi type alias for typing mocks and test implementations - Update all imports across the codebase to use the renamed class - Update MemPools and P2PClientDeps types to use AttestationPoolApi for flexibility - Rename test file from kv_attestation_pool.test.ts to attestation_pool.test.ts Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
spalladino
force-pushed
the
palla/refactor-attestation-pool
branch
from
f996732 to
eaf3e65
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Refactors the attestation pool to simplify the API, and remove unnecessary abstractions.
Built on #20027
Changes by Commit
1.
refactor(p2p): merge checkpoint attestation methods into tryAddCheckpointAttestationMerges multiple checkpoint attestation methods into a single atomic
tryAddCheckpointAttestationmethod, following the same pattern used for proposals:TryAddProposalResult→TryAddResult(reused for both proposals and attestations)tryAddCheckpointAttestation(attestation, committeeSize)that atomically checks existence, validates cap, and addshasCheckpointAttestation,canAddCheckpointAttestation,hasReachedCheckpointAttestationCapvalidateAndStoreCheckpointAttestationto LibP2P service (mirrors the proposal pattern)processCheckpointAttestationFromPeerto use the new atomic method2.
refactor(p2p): remove InMemoryAttestationPool, use KvAttestationPool everywhereRemoves the in-memory implementation in favor of using the KV-backed pool everywhere:
createTestAttestationPool()factory function for tests (uses temp KV store)memory_attestation_pool.tsandmemory_attestation_pool.test.tstestbench-utils.tsfor benchmarking3.
refactor(p2p): remove AttestationPool interface, rename KvAttestationPool to AttestationPoolRemoves the interface abstraction and uses the class directly:
KvAttestationPool→AttestationPoolkv_attestation_pool.ts→attestation_pool.tsTryAddResulttype to the class fileAttestationPoolApitype alias for mocking in testsTest Plan
yarn buildpassesyarn formatcleanyarn lint p2pclean🤖 Generated with Claude Code