getaddednodeinfo RPC call returns JSON object with duplicate keys #3581

As the title states, the getaddednodeinfo RPC call, when invoked with the dns flag set to false, returns a JSON object with duplicate keys:

bitcoind getaddednodeinfo false { "addednode" : "1.2.3.4", "addednode" : "5.6.7.8" }

Most parsers will not handle this correctly. For example, paste {"addednode":"1.2.3.4","addednode":"5.6.7.8"} into http://jsonlint.com/ and look at the result.

I would suggest that is changed to an array of strings ["1.2.3.4","5.6.7.8"].

15:48 < gavinandresen> davec: can you file a bug?  That should definitely get fixed.
15:49 < davec> gavinandresen: will do
15:51 < gavinandresen> davec: you sure it is broken?  According to the help, getaddednodeinfo returns an array of objects
15:54 < davec> > bitcoind getaddednodeinfo false
15:54 < davec> > {
15:54 < davec> >    "addednode" : "1.2.3.4",
15:54 < davec> >    "addednode" : "5.6.7.8"
15:54 < davec> > }
15:54 < davec> gavinandresen: with the dns flag set to true, it is an array of objs
15:55 < gavinandresen> davec: ah, yeah.  Definitely file a bug, getaddednodeinfo false should just return an aray of IP strings.

According to the help https://github.com/bitcoin/bitcoin/blob/master/src/rpcnet.cpp#L202 it's supposed to return a record per node with at least an IP address so

[
{"addednode" : "1.2.3.4"},
{"addednode" : "5.6.7.8"}
]

But it somehow creates one object with multiple keys. This is indeed a bug.

See #3586

I'm glad to see this addressed so quickly, but I would like to point out I don't think the change is quite right.

Perhaps I read the help differently, but I see "If false, only a list of added nodes will be provided," meaning it should be an array of IP address strings. Also, returning an array of single entry objects is inconsistent with the rest of the RPC methods. For example, look at the addresses field of decoderawtransaction and decodescript. In all other cases where there is only a single entry, the JSON is just an array of the entries versus an array of objects containing a single entry.

Finally, per the quoted conversation above with Gavin quoted he indicated it should "return an aray of IP strings".

Edit:

To make it clear what I think the final JSON should look like: ["1.2.3.4", "5.6.7.8"]

This is how I interpreted it. Keeps the output format in both cases consistent and makes it possible to add fields later.

Anyway, you know how to change it now feel free to do it differently.