Fix for 'debug/blacklist' HTTP API method.#2121
Open
nickeskov wants to merge 4 commits into
Open
Conversation
Adds reponse text and fixes request input format.
nickeskov
requested review from
alexeykiselev,
Copilot and
irina-pereiaslavskaia
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes and refactors the debug/blacklist HTTP endpoint by standardizing peer blacklisting around resolved IPv4 addresses (ignoring ports), renaming the peer manager method to reflect the new behavior, and expanding tests to cover additional input formats.
Changes:
- Exported
proto.ResolveHostToIPsv4()and updated proto parsing to use it. - Updated
App.PeersBlackListto resolve an input host / host:port into one or more IPv4s and blacklist each resolved IP (port ignored). - Updated the HTTP handler to return a success body (
"OK") and expanded tests for IP, IP:port, and leading-slash inputs; renamed peer manager blacklisting method toAddToBlackListByIP.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/proto/proto.go | Exports host→IPv4 resolver and updates internal call site to use the exported name. |
| pkg/node/peers/peer_manager.go | Renames peer manager API to AddToBlackListByIP and keeps blacklist behavior IP-based. |
| pkg/node/peers/mock_peer_manager.go | Updates generated mock to match the renamed peer manager method. |
| pkg/api/peers.go | Refactors blacklist logic to resolve and blacklist by IPv4(s), ignoring ports. |
| pkg/api/peers_test.go | Updates and expands app-level blacklist tests for IP and IP:port inputs. |
| pkg/api/node_api.go | Updates blacklist handler to wrap errors with context and write a 200 OK response body on success. |
| pkg/api/node_api_test.go | Expands handler tests to verify "OK" response body for multiple success cases. |
Files not reviewed (1)
- pkg/node/peers/mock_peer_manager.go: Generated file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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
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.
Fix for #2119.
This pull request refactors the peer blacklisting logic to improve IP resolution, standardize method naming, and enhance test coverage. The main change is to always resolve and blacklist by IPv4 address (ignoring port), simplifying and unifying how peers are blacklisted. Several method and mock names are updated for clarity and consistency, and tests are expanded to cover more scenarios.
Peer blacklisting improvements:
App.PeersBlackListnow resolves the provided address (IP or IP:port) to one or more IPv4 addresses and blacklists each resolved IP, ignoring the port. This ensures consistent blacklisting regardless of input format. (pkg/api/peers.go,pkg/proto/proto.go) [1] [2] [3]resolveAddrToIPsV4is introduced to handle address parsing and resolution. (pkg/api/peers.go)API and handler updates:
NodeApi.PeersBlackListhandler now writes a 200 OK response on success and wraps errors with context for better debugging. (pkg/api/node_api.go)Peer manager interface and implementation:
AddToBlackListByAddrtoAddToBlackListByIPin the interface and implementation, and all usages are updated accordingly. (pkg/node/peers/peer_manager.go) [1] [2]Testing improvements:
pkg/api/node_api_test.go,pkg/api/peers_test.go,pkg/node/peers/mock_peer_manager.go) [1] [2] [3] [4] [5] [6] [7] [8] [9]Proto utility function:
resolveHostToIPsv4is made public asResolveHostToIPsv4for use in the new address resolution logic. (pkg/proto/proto.go)These changes collectively make peer blacklisting more robust, consistent, and easier to maintain.