getblockchaininfo: make bip9_softforks an object, not an array. #7863

We can't change "softforks", but it seems far more logical to use tags in an object rather than using an "id" field in an array.

For example, to get the csv status before, you need to iterate the array to find the entry with 'id' field equal to "csv":

jq '.bip9_softforks | map(select(.id == "csv"))[] | .status'

Now: jq '.bip9_softforks.csv.status'

There is no issue with fork names being incompatible with JSON tags, since we're selecting them ourselves.

Signed-off-by: Rusty Russell rusty@rustcorp.com.au

it probably broke py tests

The array also implies that id may not be unique.

utACK bc1ebf5.

I don't strongly oppose, but using an 'id' is easier for parsers who use class reflection (every statically typed language does it) for parsing the JSON. (In .NET for example, you don't have to parse by hand, you just create your class with an "id" field and that's it)

Thanks @NicolasDorier it did break tests. It's exactly this kind of code which is why I think the format is ins... sub-optimal:

-    for row in info['bip9_softforks']:
-        if row['id'] == key:
-            return row
-    raise IndexError ('key:"%s" not found' % key)
+    return info['bip9_softforks'][key]

@rustyrussell agreed, for selecting it changes the lookup complexity to O(1) (not that performance matters, but, it certainly looks/feels nicer) @NicolasDorier surely those languages can derive their field from the key, maybe through a custom constructor?

@dcousens static languages derive their field the shape of the JSON. Changing it that way will mean there will be one different field for each fork proposal instead of a simple non typed array.

But by thinking about it is in reality it is not so much a problem, because the list of bip9 is static anyway, so it makes sense for a static language to have a different field for each different fork proposal.

So now I'm rather neutral about this change.

@NicolasDorier keeping the id field seems like it would entirely be a special case just for libraries that automatically parse the JSON into class objects. For those that might access the JSON using a more traditional approach json["key"]["sub-key"] (etc), it just becomes a tedium.

I've only ever used the RPC using Python/Javascript and C++, all of which that traditional approach ruled supreme, and with which searching the JSON Array would be a tedium.

Travis error unrelated: #7846

@dcousens well, when one solution improve the life of some people, it makes more difficult the life of other. Should we make life easier for hackish tool to parse json or make easier for parser library for static language is a question without responses.

What you call "traditional approach" is different for both of us, as we code in different environments.

But by thinking about it I don't think it makes life much difficult for parser libs with generated class objects after all.

OK, found another place in the tests which cut & paste the same function. And added BTCDrak's error cleanup as suggested by @MarcoFalke as a separate commit on top.

tACK d12760b

utACK

Concept ACK

Removing label 'Needs backport'. Please discuss in #8186 about the backport.