There is an intermitted fail in rpc_net.py when adding addresses to the addrman via addpeeraddress. This is happening due to some really uncommon collisions in the address manager’s tried table (see https://github.com/bitcoin/bitcoin/issues/28964).
The way the addrman works is that, if a new address wants to be added, first it is added to its new table and then, if successful, it may be moved to tried. However, addpeeraddress return value is based on whether the address was added to new, even if it fails to be added to tried. This can happen in very unlikely occasions if there is a collision when trying to add the address to tried, meaning that the method may return true in two cases: if the address has been added to new and moved to tried , and if the address has been added to new and couldn’t be added to tried.
This PR reworks the return of the method to make the boolean consistent with whether the address was being added to both tables (if requested: tried=true ) and to report an error based on where the addition fail, if applicable.
There is not much going on with this, it is a simple PR, the only issues I can see with it boil down to how data is added to the addrman which makes some path non-testable from the RPC API:
tried it need to be moved from new .tried and we try to add it again, the failure will happen when adding the address to new , meaning that we cannot really test tried collisions on demandThe current approach is not being as exhaustive as it could be testing the failure paths, which I’ve pointed out in the review. Currently only trying to add a new address, when that address is already in tried is being tested. However, we should also test trying to add a new address to new when it already is in new .
rpc: addpeeraddress tried return error on failure by 0xB10C · Pull Request #28998 · bitcoin/bitcoin