Deser fix for Update + pre-Alonzo blocks support #28
Conversation
Update's ProtocolVersion was generated as an array due to cddl-codegen not supporting occurencies so it treated [protocol_version] as an array rather than a single one as all other occurences of an array with a single element were * occurences in the cddl. As of Alozno, Blocks have a mandatory invalid_transactions field which makes it one of the 2 non-backwards-compatible (the other being transaction which we already support deserializing both) era changes to the CDDL. This includes a fix for this by ignoring it if it's not present.
| @@ -3069,15 +3042,23 @@ impl Deserialize for Block { | |||
| let auxiliary_data_set = (|| -> Result<_, DeserializeError> { | |||
| Ok(AuxiliaryDataSet::deserialize(raw)?) | |||
| })().map_err(|e| e.annotate("auxiliary_data_set"))?; | |||
| let invalid_present = match len { | |||
There was a problem hiding this comment.
I made this permissive deserialization just for now. When we do the complete re-generation I can add in a ShelleyBlock type and have the Block type that works for Alonzo separately, same for Transaction if you think that's the right decision. We can always make it optional as well instead of separate structures. We don't have them as separate in any of the backwards-compatible things like TransactionMetadata/AuxiliaryData, or Value and TransactionOutput and those are semantically similar. Or do you think it's better to be separate since then you are explicitly aware you're passing blocks from the incorrect era?
There was a problem hiding this comment.
If we do have the blocks separate, but don't have separate structs for TransactionMetadata/AuxiliaryData, Value and TransactionOutput, then users would think those would be there but that's not the case in the library since you could have a pre-Alonzo block with datum hashes in the TransactionOutputs which is not allowed by the CDDL/protocol. It's a bit inconsistent.
Update's ProtocolVersion was generated as an array due to cddl-codegen
not supporting occurencies so it treated [protocol_version] as an array
rather than a single one as all other occurences of an array with a
single element were * occurences in the cddl.
As of Alozno, Blocks have a mandatory invalid_transactions field which
makes it one of the 2 non-backwards-compatible (the other being
transaction which we already support deserializing both) era changes to
the CDDL. This includes a fix for this by ignoring it if it's not
present.