rpc: addpeeraddress: throw on invalid IP #33430

pull john-moffett wants to merge 1 commits into bitcoin:master from john-moffett:rpc-addpeeraddress-error changing 2 files +22 −17

john-moffett commented at 6:52 PM on September 18, 2025: contributor

Right now we return an opaque {"success" : false} in addpeeraddress for an empty or invalid IP. This changes it to throw RPC_CLIENT_INVALID_IP_OR_SUBNET with the error message Invalid IP address. Tests updated to match.

rpc: addpeeraddress: throw on invalid IP

Throw RPC_CLIENT_INVALID_IP_OR_SUBNET when LookupHost(addr, false) fails
in addpeeraddress. This aligns with setban/addconnection and avoids the
opaque {"success": false} result for input errors. The JSON {success,
error?} object remains for addrman outcomes only. Update test to match.

316a0c5132

DrahtBot added the label RPC/REST/ZMQ on Sep 18, 2025
DrahtBot commented at 6:52 PM on September 18, 2025: contributor



The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.



Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/33430.



Reviews

See the guideline for information on the review process.

Type Reviewers

ACK vasild, sipa, pablomartin4btc, achow101

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
vasild approved
vasild commented at 12:22 PM on September 23, 2025: contributor

ACK 316a0c513278d53cb25f42ea502d20691962aad6

As an alternative, we could use the error field in the response instead of throwing an exception. The error field is currently not set in master if an invalid IP address is provided.
sipa commented at 12:47 PM on September 23, 2025: member

utACK 316a0c513278d53cb25f42ea502d20691962aad6
pablomartin4btc commented at 3:10 PM on September 24, 2025: member
tACK 316a0c513278d53cb25f42ea502d20691962aad6

master:
```
./build/bin/bitcoin-cli -regtest -datadir=/tmp/btc addpeeraddress "" 8883
{
  "success": false
}
```
This branch:
```
./build/bin/bitcoin-cli -regtest -datadir=/tmp/btc addpeeraddress "" 8883
error code: -30
error message:
Invalid IP address
```
I agree with @vasild with the suggested alternative, not strong opinion.

Would it be useful for the user to differentiate from an empty address vs a provided invalid IP (which obviously includes an empty value) in the error message?
john-moffett commented at 4:04 PM on September 24, 2025: contributor

Thanks, all! @vasild

As an alternative, we could use the error field in the response instead of throwing an exception.

I tried to use the same pattern as setban does, which I think has a clean boundary between an invalid input and a problem that occurred despite valid input. To me, that category separation makes sense. @pablomartin4btc

Would it be useful for the user to differentiate from an empty address vs a provided invalid IP (which obviously includes an empty value) in the error message?

My gut says it's probably overkill, but I'm certainly happy to change it if others disagree.
BrunoSampaioDev commented at 12:15 AM on September 25, 2025: none

LGTM
achow101 commented at 10:37 PM on September 25, 2025: member

ACK 316a0c513278d53cb25f42ea502d20691962aad6
achow101 merged this on Sep 25, 2025
achow101 closed this on Sep 25, 2025
sedited referenced this in commit 3f14dd4b2a on Oct 7, 2025
sedited referenced this in commit 5f9d179b8d on Oct 8, 2025
sedited referenced this in commit 845b93d99e on Oct 8, 2025
yuvicc referenced this in commit ccce70c31a on Oct 8, 2025
sedited referenced this in commit 3b1e26d5d3 on Oct 10, 2025
sedited referenced this in commit 0fa2fdaca0 on Oct 11, 2025
stringintech referenced this in commit f74b275ffc on Oct 14, 2025
stickies-v referenced this in commit a785fe6759 on Nov 5, 2025
Kino1994 referenced this in commit 8b2ba78bf0 on Jun 28, 2026
BigcoinBGC referenced this in commit 990006ee7d on Jun 30, 2026
Kino1994 referenced this in commit d650ae767a on Aug 19, 2026
bitcoin locked this on Sep 25, 2026