IntersectMBO / cardano-ledger

The ledger implementation and specifications of the Cardano blockchain.
Apache License 2.0
255 stars 156 forks source link

Mention if duplicates are allowed in maps in the CDDL files #4335

Open itsfarseen opened 3 months ago

itsfarseen commented 3 months ago

In conway.cddl, there are many types which are maps. As per CDDL spec, maps are allowed to have duplicate keys. But the node doesn't allow duplicate keys in some places, and allows in some other places.

For example, the multiasset type is defined as a map: https://github.com/IntersectMBO/cardano-ledger/blob/28ab3884cac8edbb7270fd4b8628a16429d2ec9e/eras/conway/impl/cddl-files/conway.cddl#L564

The mint type, which is an instance of multiasset, can have duplicate keys. https://github.com/IntersectMBO/cardano-ledger/blob/28ab3884cac8edbb7270fd4b8628a16429d2ec9e/eras/conway/impl/cddl-files/conway.cddl#L576

But the multiasset field of the value type can't have. https://github.com/IntersectMBO/cardano-ledger/blob/28ab3884cac8edbb7270fd4b8628a16429d2ec9e/eras/conway/impl/cddl-files/conway.cddl#L574

It would be helpful for library/tools authors to have comments in the CDDL file around map types, describing whether duplicate keys are allowed or not.

lehins commented 3 months ago

Duplicate keys are not allowed in any of the maps in cbor, simply because the mapping must be unique from keys to values. The difference is in some cases we fail hard on duplicates while in others we discard the duplicate mapping from the map. It is a bit unfortunate, but it has to do with the limitation of current implementation of serialization that we have. Although we do have strong plans on fixing it throughout all of the ledger types. As far as the tooling is concerned, IMHO it should not allow duplicate keys, because that is the ultimate goal. That being said, I am totally for documenting this discrepancy for every map/set occurrence in the cddl