feat: centralized peer registry, transport abstraction, and LEGACYSYNCING removal - #565
Closed
oskarszoon wants to merge 69 commits into
Closed
oskarszoon wants to merge 69 commits into
oskarszoon wants to merge 69 commits into
Conversation
oskarszoon
force-pushed
the
feature/legacy-peer-registry
branch
from
March 18, 2026 12:22
cd79d69 to
37eeff4
Compare
Contributor
Benchmark Comparison ReportBaseline: Current: Summary
All benchmark results (sec/op)
Threshold: >10% with p < 0.05 | Generated: 2026-05-08 10:09 UTC |
oskarszoon
force-pushed
the
feature/legacy-peer-registry
branch
from
March 27, 2026 13:53
571adfd to
6a0b37c
Compare
oskarszoon
changed the title
oskarszoon
force-pushed
the
feature/legacy-peer-registry
branch
6 times, most recently
from
March 27, 2026 15:45
53e28e8 to
3ed504d
Compare
…CING removal Introduce a centralized peer registry in the blockchain service that tracks peers across both HTTP (P2P/DataHub) and wire protocol (legacy Bitcoin) transports. This replaces the fragmented per-service peer tracking with a single source of truth for peer information, reputation scoring, and transport-aware catchup orchestration. Key changes: Centralized Peer Registry (blockchain service): - Thread-safe in-memory registry with reputation scoring algorithm - gRPC API: RegisterPeer, UpdatePeerMetrics, RemovePeer, ListPeers, GetPeer - JSON file persistence with atomic writes and TTL-based cleanup - PeerRegistryClientI interface with connection ownership tracking Transport Abstraction (blockvalidation): - CatchupTransport interface abstracting HTTP and wire protocol fetching - HTTPTransport: extracts existing DataHub HTTP fetch logic - WireTransport: delegates to Legacy service via gRPC for wire protocol - Central registry polling for autonomous catchup orchestration Legacy Service Integration: - FetchHeadersFromPeer/FetchBlockFromPeer gRPC endpoints for wire protocol - Dual-write to central registry on peer connect/disconnect/metrics - One-shot request pattern with LoadOrStore for concurrency safety FSM Simplification: - Remove LEGACYSYNCING state — consolidate into CATCHINGBLOCKS - Simplify P2P SyncCoordinator (remove Kafka catchup publishing) - Legacy SyncManager now delegates catchup to BlockValidation - All FSM transitions and references updated across codebase Proto Changes: - Add PeerRegistryService to blockchain_api.proto - Add FetchHeadersFromPeer/FetchBlockFromPeer to legacy peer_api.proto - Reserve removed LEGACYSYNCING/LEGACYSYNC enum values - Regenerate all protobuf Go code
- Remove all LEGACYSYNCING/LEGACYSYNC references from miner guides, CLI docs, sync tutorials, dashboard docs, and protobuf docs - Update mermaid sync state flow diagram for 3-state FSM - Remove dead LEGACYSYNC event handling from dashboard UI - Remove legacy sync button, state colors, and API functions from dashboard - Fix pre-existing markdown lint issues (MD025, MD036, MD051) - Delete obsolete fsm_legacy_sync PlantUML diagram and SVG
oskarszoon
force-pushed
the
feature/legacy-peer-registry
branch
from
March 27, 2026 15:49
3ed504d to
b7cd309
Compare
…s, sync coordinator - Transition FSM to CATCHINGBLOCKS immediately when catchup starts (Step 1.5) so subtree validation stops processing network messages during sync - Central registry poller: fast 3s initial interval for 10 attempts, then 30s - Central registry poller: skip poll when catchup already in progress - Central registry poller: prefer full nodes over pruned for catchup - Propagate storage mode (full/pruned) to central registry via updateStorage - SyncCoordinator: stop rotating peers when behind (defer to central poller) - SyncCoordinator: don't clear sync peer in handleRunningState if already set - Raise max accumulated headers to reach next checkpoint for quick validation - Pass maxHeadersOverride through catchupGetBlockHeaders without mutating settings
- Add ban scoring with decay, threshold, auto-unban to CentralizedPeerRegistry - Add BanConfig with defaults matching existing P2P BanManager - Add gRPC RPCs: AddBanScore, IsPeerBanned, ListBannedPeers, ClearBannedPeers - Add ReconsiderBadPeers for reputation recovery after cooldown - Add StartBanDecay background goroutine for score decay - Update PeerRegistryClientI with ban methods - Update all mock implementations
- Track per-peer cooldowns after failed catchup attempts - Exponential backoff: 30s, 60s, 120s, up to 5min max per peer - Skip peers on cooldown, try next best peer instead - Clear all cooldowns on successful catchup
- Remove peerRegistry, syncCoordinator, banManager, peerSelector from Server - centralRegistry is now REQUIRED (checked in Init) - All peer/ban/metrics ops go through centralRegistry gRPC - Rewire all test files to use central registry mocks - Add //go:build ignore to SyncCoordinator test files (code being removed) - Adapt Server_test.go, server_handler_test.go, report_invalid_block_test.go
BlockValidation now reports catchup metrics (success, failure, malicious, attempt) directly to the central registry via UpdatePeerMetrics instead of routing through P2P service RPCs. p2pClient is retained for non-metric operations (GetPeersForCatchup, RecordBytesDownloaded, parallel fetch).
Call peerRegistry.StartBanDecay(ctx) during blockchain service startup so ban scores automatically decay over time (1 point/minute).
… PeerSelector Remove files replaced by the centralized registry in blockchain service: - peer_registry.go, peer_registry_cache.go and their tests - sync_coordinator.go and all related tests - BanManager.go, BanManager_test.go - peer_selector.go, peer_selector_test.go - peer_registry_reputation_test.go Fix remaining references: remove MockPeerBanManager, BanReason refs, use string reason constants, skip tests needing local registry rewrite.
Ban management tests (18 tests in peer_registry_ban_test.go): - AddBanScore: scoring, threshold, decay, peer info sync, config lookup - IsBannedPeer: not banned, banned, auto-unban on expiry - ListBannedPeers: empty, returns only banned - ClearBannedPeers: clears all, resets peer info - ReconsiderBadPeers: old failures reset, recent failures kept, count - decayBanScores: decay over time, zero-score cleanup, banned entry kept - StartBanDecay: context cancellation Catchup poller tests (20 tests in central_registry_poller_test.go): - nextCooldownForPeer: exponential backoff 30s-5min, per-peer tracking - selectBestPeersFromCentralRegistry: height filter, full>pruned sort, wire protocol - pollCentralRegistry: no peers, isCatchingUp skip, cooldown skip, nil hash, error handling, expired cooldown retry
…s wiring - Fix TestCatchup_FSMStateManagement: setFSMCatchingBlocks now calls GetFSMCurrentState to check if already in CATCHINGBLOCKS before transitioning. Update mock to expect RUNNING state first. - Fix TestServerInit* tests: Init() now requires centralRegistry to be set. Add newPermissiveMockRegistry() helper and SetCentralPeerRegistry calls. - Wire P2P ban settings (BanThreshold, BanDuration) from settings.conf to the central registry's BanConfig instead of using 24h default. Fixes TestPeerIDBanExpirationE2E smoketest.
…eer ID encoding
- Add GetFSMCurrentState mock to setupTestCatchupServer (both instances)
for early Step 1.5 FSM transition in catchup
- Add centralRegistry to all 47 Server struct literals in P2P tests
- Fix TestServer_GetPeer: use peerID.String() for mock expectations
(peer.ID("non-existent").String() != "non-existent" due to base58 encoding)
- Fix mockPeerRegistryClient ban methods to actually call m.Called() instead of returning hardcoded values (IsPeerBanned, AddBanScore, etc) - Fix TestIsBannedChecksBothBanSystems: remove extra context arg from IsPeerBanned mock expectation - Fix TestCatchup_FSMStateManagement: filter permissive GetFSMCurrentState before setting ordered .Once() expectations - Fix TestCatchup/Empty_Catchup_Headers: add FSM mocks to standalone Server setup (CatchUpBlocks, GetFSMCurrentState, Run)
- Fix gci formatting in central_registry_test.go - Add FSM mocks (CatchUpBlocks, GetFSMCurrentState, Run) to createServerWithEnhancedCatchup helper in catchup_test.go for TestCatchupIntegrationScenarios/Context_Cancellation_During_Catchup
Must-fix: - Bounded worker pool (4 workers, chan size 256) replaces unbounded fire-and-forget goroutines for central registry updates in P2P - List() now checks ban expiry via banScores instead of stale IsBanned field - TransportType only updated when TransportTypeSet=true (fixes wire peer reset) - blockHashToBytes returns defensive copy to avoid slice aliasing Should-fix: - Throttle updatePeerLastMessageTime (30s cooldown per peer) - Remove duplicate addConnectedPeer (identical to addPeer) - Simplify nextCooldownForPeer with bit shift math - Document single-goroutine invariant on cooldown maps - Add TODO for getPeerIDFromDataHubURL efficiency - Fix waitForLegacyMockCalls race condition with atomic counter - Remove dead shouldSkipDuringSync and its test Nice-to-have: - Remove redundant nil checks in handle_catchup_metrics.go - Improve WireTransport error messages - Rename baseURL to peerEndpoint in CatchupTransport interface - Improve stub RPC logging (Debug -> Info for no-ops)
…-registry # Conflicts: # services/blockchain/Client.go # services/legacy/netsync/manager.go # services/p2p/sync_coordinator.go # services/p2p/sync_coordinator_test.go # services/subtreevalidation/streaming_processor.go # settings/blockchain_settings.go # settings/settings.go
…utines Centrifuge reconnect loop can call logger.Errorf after test teardown begins, racing with testing framework cleanup via t.Logf. Calling ErrorTestLogger.Shutdown() at the start of teardown gates subsequent Errorf calls and prevents the data race flagged by CI.
Legacy wire peers don't send the node_status message that HTTP peers use to advertise storage mode, so legacyPeerToRegistryInfo left Storage empty. The catchup peer selector sorts Storage=="full" peers ahead of others, so legacy peers were ranked last and HTTP teranode peers were picked for every catchup even when legacy peers were higher. Derive Storage from the SFNodeNetwork service flag — peers advertising NODE_NETWORK serve full historical blocks.
Two callers of UpdatePeerMetrics in the legacy server were passing our local accepted height as the peer's height, corrupting the registry entry and dragging the peer's tracked tip down to whatever block we just synced. This caused legacy peers to fall below the catchup selector's minHeight filter and effectively never get picked. - handleUpdatePeerHeights: use sp.LastBlock() (peer's monotonic tracked tip from VERSION + UpdateLastBlockHeight) instead of umsg.newHeight - SetOnBlockAccepted: drop height argument entirely; this callback records interaction success only, height belongs to UpdatePeerHeights
…-registry # Conflicts: # services/p2p/Server.go # services/p2p/peer_registry.go # services/p2p/peer_registry_test.go # services/p2p/server_helpers.go
Add unit tests targeting the newly-introduced central registry helpers in server_helpers.go to lift Sonar coverage on new code above the 80% quality gate threshold: - addConnectedPeer (with/without registry) - updateStorage (with mode, empty mode short-circuit) - addProtocolViolation (success + registry error swallowed) - getPeerIDFromDataHubURL (match, no match, nil registry, list error) - getPeer (found, not found, registry error, nil registry) - InjectPeerForTesting (Storage=full override, nil registry) - enqueueRegistryUpdate (sync fallback, full-channel drop) - centralPeerToLocalPeerInfo (decodable libp2p ID + legacy address fallback)
…t_peers Lift Sonar new-code coverage above the 80% gate. - peer_selection.go: 8 tests covering selectBestPeersForCatchup (nil client, error, empty, height filter, listen-only filter, full population, all-filtered, success-rate log paths) - catchup_status.go: 16 tests covering pure helpers (shortHash, formatInt/Float/Progress/Duration, formatCatchupStatusSummary) plus Server-state paths (no-active, with-previous, nil-context guard, headers/validating/finalizing phases, summary) - get_peers.go: response type JSON shape tests
Lift Sonar new-code coverage further toward the 80% gate. - get_peers.go: covers the NewPeerRegistryClient-fails branch (mock blockchain.Mock so the nil-check passes; bogus gRPC address makes the registry-client construction fail). Also tests PeerInfoResponse/ PeersResponse JSON shape. - get_catchup_status.go: covers happy path (full status), with-previous- attempt path, and BlockValidation client error path.
More Sonar new-code coverage: - tryAlternativePeersForCatchup: 4 tests covering reachable branches without invoking u.catchup (no peers, all excluded, all malicious, selection error) - categorizeWireCatchupError: 5 tests across all switch arms (validation, pruned, peer_misbehavior, network, unknown default) - SetLegacyCatchupClient setter - catchupViaLegacy nil-client guard returns ErrServiceUnavailable
Extract the PeerInfoResponse build loop from GetPeers into a pure peersToResponse function so it can be unit-tested without mocking gRPC dialing. Adds tests for: - empty input - full conversion (all 25+ fields) - nil BlockHash branch Lifts get_peers.go new-code coverage from ~17% toward 80%+, closing the remaining ~0.2% Sonar gate gap.
7 tasks
…rror paths Drives catchupViaLegacy through reachable branches by mocking blockchain.Mock + a scripted legacyCatchupClient that streams progress events and returns an rpcErr. - HappyPath_AllPhases: DOWNLOADING_HEADERS + DOWNLOADING_BLOCKS + COMPLETE phases; verifies blocksValidated counter at end. - FailedPhase: FAILED progress with category=validation yields BlockInvalid via categorizeWireCatchupError. - GRPCStreamError: empty progress stream + rpc error wrapped as NetworkError. - LockAlreadyHeld: pre-existing isCatchingUp blocks acquireCatchupLock. Lifts catchup_via_legacy.go new-code coverage from ~25% toward 80%+.
Upstream PR bsv-blockchain#812 added tests for startPeerRegistryCleanup which exercises the local peerRegistry / NewPeerRegistry / peerRegistryCleanupTimer machinery. This branch deleted that local registry in favour of the centralized peer registry in the blockchain service, so the new tests reference symbols that no longer exist and break the build. Drop them — the central registry has its own coverage in central_registry_test.go and server_helpers_central_test.go.
|
Contributor
Author
|
Continues in #832 |
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.
Summary
RegisterPeer/UpdatePeerMetrics/RemovePeer/ListPeers/GetPeer), JSON persistence, and ban management with decay.DelegateCatchupserver-streaming gRPC. Legacy runs its existing headers-first sync and streams progress back. If Legacy's ownstartSyncis already running when the request arrives, Legacy attaches to that sync rather than starting a duplicate.LEGACYSYNCING; both legacy and HTTP catchup now useCATCHINGBLOCKS. BlockValidation owns the CATCHINGBLOCKS transition during catchup.blocksValidated/blocksFetchedand current height populated from the delegated progress stream so the existing catchup banner works for wire syncs too.Architecture
Key Design Decisions
DelegateCatchupstream lets Legacy use its battle-tested wire pipeline.RUNtransitions are suppressed viasm.delegated.activeso BlockValidation owns the state machine.Test Plan
make testpassesmake smoketestpassesLEGACYSYNCINGreferences remain in Go/proto sourceslegacy:/Bitcoin SV:x.x.x/entries with reputation rising on accepted blocks