[hackobi]

This carries the last #1094 commit, which was pushed to that branch a few minutes after #1094 merged and so never reached stabilisation.

Change

mergePeerlist in Sync.ts treated addPeer's [added, reason] tuple as a boolean. A non-empty array is always truthy, so every peer in a block's peerlist was reported as merged, including refused ones. That list also drives the L2PS participation exchange. It now unpacks the tuple, as broadcastManager.ts does since #1094.

Tests

• New src/libs/blockchain/routines/Sync.mergePeerlist.test.ts: it fails with the Sync.ts change reverted and passes with it.
• Added to SUITE_FILES in scripts/test-consensus.sh, next to the list fix(launch): lost commit-lock file, go/no-go validator set and caps, double-sign alerts #1095 introduced. The cherry-pick conflicted there; I kept fix(launch): lost commit-lock file, go/no-go validator set and caps, double-sign alerts #1095's list and added this one line.
• Local bash scripts/test-consensus.sh at this head: 68 files, 0 failed. bun run type-check passes.

Summary by CodeRabbit

• Bug Fixes
  • Peer lists now include only peers successfully accepted by the peer manager.
• Tests
  • Added coverage for peer list merging when some peers are refused.

[claude]

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.

_{Tip: disable this comment in your organization's Code Review settings.}

[qodo-code-review]

ⓘ Qodo reviews are paused because the subscription is no longer active. Ask your workspace admin to reactivate the subscription to resume reviews. Manage billing

[coderabbitai]

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: cdd2e092-420a-458d-9a52-a5f5cd2ec6ad

Commits

Reviewing files that changed from the base of the PR and between aba1089 and 07b7e3a.

Files selected for processing: 3

• scripts/test-consensus.sh
• src/libs/blockchain/routines/Sync.mergePeerlist.test.ts
• src/libs/blockchain/routines/Sync.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

Walkthrough

mergePeerlist now adds a peer identity to its result only when PeerManager.addPeer reports success. A test checks accepted and refused peers, and the consensus test script includes the new test.

Changes

Peer-list merge

Layer / File(s)	Summary
Merge accepted peers src/libs/blockchain/routines/Sync.ts, src/libs/blockchain/routines/Sync.mergePeerlist.test.ts, scripts/test-consensus.sh	mergePeerlist checks the addPeer success flag before adding a peer identity. The test checks that only the accepted peer appears in the result. The consensus test script includes the test file.

Priority: Normal

Estimated code review effort: 2 (Simple) | ~8 minutes

Change: Bug fix

Suggested reviewers: shitikyan

Merge Risk: Minimal · up to 07b7e

Peer-list merges now exclude refused peers. No material merge risk was identified.

Security architecture risk: Minimal · up to 07b7e

The change makes synchronization respect existing peer-admission decisions. Accepted peers retain their existing behavior, while refused peers are excluded from the merged result. No material security risk introduced or worsened by this change was identified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The changed behavior is bounded to the local node's reporting of newly admitted peers and the existing L2PS exchange target selection. It removes refused identities from that flow without granting additional admission authority or expanding the destination set.

Trust Boundaries and Controls

• observed — Block-listed peer objects still pass through addPeer without owner-signed authority. Its existing controls reject placeholder identities, invalid URLs, self-directed URLs for other identities, and unsigned claims on URLs bound to another identity. The head now propagates those refusals into the merged result rather than treating the refusal tuple as success.

Pre-merge checks: 5

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main change: counting only accepted peers in mergePeerlist.
Docstring Coverage		Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 3 files.

Finishing Touches

📝 Generate docstrings

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Warning

Some tools did not complete. Review the errors below.

ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install failed due to a network error.

_{A rabbit checks each peer in line
One joins the list; one stays behind
The merge now reads the success sign
A test confirms the rule is fine
Then hops along, content and benign}

_{Comment @coderabbitai help to get the list of available commands.}

[greptile-apps]

Confidence Score: 5/5

[Medium impact] This PR appears safe to merge.

Summary

mergePeerlist now checks whether addPeer accepted a peer instead of treating its return array as a boolean.

• Block sync counts only accepted peers as merged.

_{Reviews (1) · Last reviewed commit: "fix(sync): count only accepted peers in ..." · Reviewed by Greptile}