Adding kernel_mmr_size and output_mmr_size to BlockHeaderPrintable#3329
Conversation
antiochp
left a comment
There was a problem hiding this comment.
I think if this is simply adding additional fields to the json it should be fine.
@quentinlesceller Any concerns here regarding the api?
👍 from me.
|
Strictly speaking it would require a minor version bump, some json bindings expect the exact match with their schema, speaking from my personal experience. However taking into consideration the nature of hard fork I'm not sure if we should bother to bump the version. |
|
@hashmap Wow, that's a horrible idea to expect an exact match, but whatever. Are you referring to an API version of some sort though? I wasn't aware we had such a thing. |
|
Yeah, I was quite surprised too to learn one day that I broke a java client by adding a filed, it's still a common case according to quick google search https://www.baeldung.com/jackson-deserialize-json-unknown-properties Technically we have v1 and v2 apis, I suspect it could be a bit of pain to support v2 and v2.1, so I'd yank v2 and s/v2/v2.1/ |
quentinlesceller
left a comment
There was a problem hiding this comment.
@antiochp I think that we should assume that adding new field is okay. However changing or deleting existing fields is not. Most clients are okay with additional fields. So 👍 on my side (especially with the new HF version).
|
I'd be in favour of merging this now. What do you think @antiochp ? |
This is needed to completely rebuild a core::BlockHeader from JSON.