refactor(p2p): split mempool client, replace register callbacks, extract gossip message processor#24589
Draft
AztecBot wants to merge 3 commits into
Draft
refactor(p2p): split mempool client, replace register callbacks, extract gossip message processor#24589AztecBot wants to merge 3 commits into
AztecBot wants to merge 3 commits into
Conversation
AztecBot
added
ci-draft
P2PClient's public surface was dominated by thin pass-throughs to the tx pool and attestation pool (getPendingTxs, tryAddBlockProposal, block-stream reactions to mined/finalized/pruned blocks, etc), mixing mempool bookkeeping with the client's actual job of peer/gossip/reqresp orchestration. Move all pool access and the L2BlockStream reactions that mutate the pools into a new internal MempoolClient collaborator that P2PClient composes. The public P2P interface, method names, and behavior (including the 'protected'->'pending' tx-status remap and pool-write-before-propagate ordering in broadcastProposal/broadcastCheckpointProposal) are unchanged.
…face and typed events The P2P client and service exposed six imperative register methods, each threading a callback down through P2PClient into LibP2PService, a pattern the code itself flagged as a leaky abstraction (#7963). Collapse them: - The three request/response callbacks (block proposal, validator checkpoint proposal, all-nodes checkpoint proposal) become a single P2PProposalHandler interface set via setProposalHandler(). Handlers merge over the current ones, preserving the old per-slot registration semantics where the validator client fulfills its handlers without disturbing the all-nodes handler set by the standalone ProposalHandler. - The three fire-and-forget notifications (duplicate proposal, duplicate attestation, accepted checkpoint attestation) become typed events (P2PServiceEvents) emitted by the service and forwarded by the client. Default handler behaviors in LibP2PService (warn-and-accept block proposals, throw on unregistered all-nodes checkpoint handler) and the first-duplicate-only emission points are unchanged. Closes #7963
LibP2PService mixed gossip transport with domain processing: mempool writes, equivocation detection, and proposal-handler invocation lived inside the service (an existing REFACTOR(palla) comment already flagged processValidBlockProposal as misplaced). Move the five domain methods (validateAndStoreBlockProposal, validateAndStoreCheckpointProposal, validateAndStoreCheckpointAttestation, processValidBlockProposal, processValidCheckpointProposal) and the proposal-handler state into a new GossipMessageProcessor that the service delegates to inline. Delegation, not relocation, on purpose: gossipsub topic validation must return Accept/Reject/Ignore within the message-cache window, so the processor is called synchronously from the same paths as before. Method bodies, ordering, first-duplicate-only (count === 2) equivocation emission, and trace span names are unchanged; the service keeps thin protected wrappers with the same signatures so subclasses and tests are unaffected.
AztecBot
force-pushed
the
cb/p2p-mempool-registrar-refactor
branch
from
bf6239e to
5a8f806
Compare
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Implements the three p2p refactors proposed by @spalladino (santiago) in the yarn-project refactor-audit thread, one commit each:
refactor(p2p): extract MempoolClient collaborator from P2PClient— P2PClient's public surface was dominated by thin pass-throughs to the tx/attestation pools, plus the L2 block-stream reactions (mined/finalized/pruned) that mutate them. All pool access moves into an internalMempoolClientcollaborator; the publicP2Pinterface and behavior (including the'protected'→'pending'tx-status remap and pool-write-before-propagate ordering in the broadcast methods) are unchanged.refactor(p2p): replace register callbacks with proposal handler interface and typed events— the sixregisterXCallback/registerXHandlermethods threading callbacks from validator-client through P2PClient into LibP2PService (the leaky abstraction tracked in chore(vc): remove the p2p client validator callback #7963, flagged in three separate code comments) collapse into:P2PProposalHandlerinterface set viasetProposalHandler()for the three request/response callbacks (block proposal, validator checkpoint proposal, all-nodes checkpoint proposal), with merge semantics preserving the old per-slot registration behavior betweenProposalHandler.register()andValidatorClient.registerHandlers();P2PServiceEvents:duplicateProposal,duplicateAttestation,checkpointAttestation) for the three fire-and-forget notifications, emitted by the service and forwarded by the client.Default handler behaviors and the first-duplicate-only (
count === 2) emission points are unchanged.refactor(p2p): extract GossipMessageProcessor from LibP2PService— the domain-side processing of gossiped consensus messages (validation, mempool writes, equivocation detection, proposal-handler invocation — including the method an existingREFACTOR(palla)comment flagged as misplaced) moves into a newGossipMessageProcessorthat the service delegates to inline. Delegation rather than relocation on purpose: gossipsub topic validation must return Accept/Reject/Ignore within the message-cache window, so the processor is called synchronously from the same paths. Method bodies, ordering, and trace span names are unchanged, and the service keeps thin protected wrappers with the same signatures so subclasses and tests are unaffected.All three are intended to be behavior-preserving; no wire format, timing, or consensus behavior changes. Marked
ci-no-squashto keep the three commits separate for review.Closes #7963
Created by claudebox · group:
slackbot