rpc: addpeeraddress: throw on invalid IP #33430

1. 
   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.
2. 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
3. DrahtBot added the label RPC/REST/ZMQ on Sep 18, 2025
4. 
   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.

5. vasild approved
6. 
   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.

7. 
   sipa commented at 12:47 pm on September 23, 2025: member

   utACK 316a0c513278d53cb25f42ea502d20691962aad6
8. 
   pablomartin4btc commented at 3:10 pm on September 24, 2025: member

   tACK 316a0c513278d53cb25f42ea502d20691962aad6

   master:

   0./build/bin/bitcoin-cli -regtest -datadir=/tmp/btc addpeeraddress "" 8883
   1{
   2  "success": false
   3}
   

   This branch:

   0./build/bin/bitcoin-cli -regtest -datadir=/tmp/btc addpeeraddress "" 8883
   1error code: -30
   2error message:
   3Invalid 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?

9. 
   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.

10. 
    BrunoSampaioDev commented at 0:15 am on September 25, 2025: none

    LGTM
11. 
    achow101 commented at 10:37 pm on September 25, 2025: member

    ACK 316a0c513278d53cb25f42ea502d20691962aad6
12. achow101 merged this on Sep 25, 2025
13. achow101 closed this on Sep 25, 2025

    ---

Contributors
john-moffett DrahtBot vasild sipa pablomartin4btc BrunoSampaioDev achow101

Labels
RPC/REST/ZMQ