input-output-hk / cardano-ledger-byron

A re-implementation of the Cardano ledger layer, replacing the Byron release
52 stars 13 forks source link

Incorrect comment on the `headerProtocolVersion` field #731

Open nfrisby opened 4 years ago

nfrisby commented 4 years ago

https://github.com/input-output-hk/cardano-ledger/blob/172b49ff1b6456851f10ae18f920fbfa733be0b0/cardano-ledger/src/Cardano/Chain/Block/Header.hs#L145-L146

That comment is incorrect. The field is only used to endorse parameter update proposals, not to document how the current block was forged.

(I'm not sure if the corresponding headerSoftwareVersion comment is also incorrect; I'm looking into that now.)

nc6 commented 4 years ago

This is kind of a matter of perspective. I have always thought that one endorses a proposal by forging a block with the correct protocol version. E.g. "endorsement" is simply a measure of how many nodes are using that version. So from that perspective the comment is correct. However, we could perhaps elaborate this.

nfrisby commented 4 years ago

with the correct protocol version

using that version

@nc6 By "with/use", do you mean that the nodes have started to use the new proposed protocol parameter values even though the ledger rules have not yet adopted those values? I think this is impossible in general. Consider an update proposal that increases ppMaxBlockSize. If a node has adopted the new value prematurely, it might create a block that the ledger rules' currently-adopted protocol parameter values judge to be invalid.

nc6 commented 4 years ago

Ah, I see. Yes, when you think of protocol parameters, I agree this is incorrect