feat: replace SharedDomainType with SharedForkLifecycle for fork-aware peer selection#2
Closed
diegomrsantos wants to merge 4 commits intofix/fork-aware-observed-peer-subnetsfrom
Closed
Conversation
Add a new lifecycle module to the fork crate that models fork transition states as a single enum with three variants: - Normal: operating on a single fork (pre-fork or post-grace-period) - WarmUp: preparing for an upcoming fork (dual-subscribing to new topics) - GracePeriod: fork activated but keeping old subscriptions for late messages SharedForkLifecycle wraps this in Arc<RwLock<T>> for cross-component sharing, following the same pattern as the SharedDomainType it will replace. Each variant carries domain_type, making invalid states unrepresentable. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
ForkMonitor now accepts a SharedForkLifecycle and updates it right
before broadcasting each ForkPhase event:
- Preparing → WarmUp { current, upcoming, domain_type }
- Activated → GracePeriod { current, previous, domain_type }
- GracePeriodEnded → Normal { current, domain_type }
This makes ForkMonitor the single writer of fork lifecycle state.
Components that previously needed to react to events just to cache
derived values can now read the shared state directly.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace the SharedDomainType abstraction with SharedForkLifecycle across all network components, making peer selection fork-aware. Key changes: - Remove SharedDomainType from network crate entirely - Discovery and Handshake read domain_type via fork_lifecycle - Network::on_fork_phase simplified to only update ENR (domain type updates are now handled by ForkMonitor via SharedForkLifecycle) - ConnectionManager uses fork lifecycle to filter peer subnets: in Normal state only the current fork's bitmap is returned; during WarmUp/GracePeriod both forks are aggregated - Client creates SharedForkLifecycle and passes to both ForkMonitor and Network This prevents peers subscribed only to a defunct fork from appearing useful for subnet coverage after the grace period ends. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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.
Issue Addressed
Follows up on PR sigp#820 (fork-aware
observed_peer_subnets). While sigp#820 correctly stores per-fork bitmaps, all query methods still aggregate across forks with a blindunion(). After the Boole grace period ends, peers only subscribed to Alan topics appear useful for Boole subnets — leading to incorrect peer selection.Proposed Changes
ForkLifecycleenum (Normal,WarmUp,GracePeriod) andSharedForkLifecycleshared state in the fork crate, replacingSharedDomainTypeForkMonitorupdates the shared lifecycle state before broadcasting eachForkPhaseeventConnectionManagernow consult the lifecycle: inNormalstate only the current fork's bitmap is considered; duringWarmUp/GracePeriodboth forks are aggregatedSharedDomainType(Discovery, Handshake) now readSharedForkLifecycle::domain_type()insteadNetwork::on_fork_phasesimplified — no longer sets domain type (monitor handles it), only updates ENRAdditional Info
SharedForkLifecyclefollows the sameArc<RwLock<T>>single-writer pattern as theSharedDomainTypeit replacesNetwork::on_fork_phasebecause they require explicit discv5 mutationcount_observed_peers_for_subnetshoists the lifecycle read outside the per-peer loop to avoid repeated lock acquisition