enforce unique map keys in CBOR decoding, ver. 9 #3277
Conversation
There was a problem hiding this comment.
There is a more efficient way to implement stricter Map decoder, which I suggested in a comment.
Also, maybe you wanna try adding a quickcheck property test to hspec test suite? I think it would be a good exercise to get first hand experience with hspec. This is just a suggestion, so feel free to ignore it.
Hedgehog definitely works, so the test that is already included in this PR will work just fine, but since we mostly use QuckCheck, I thought it would be better if we stick to one tool for consistency. However, if you prefer Hedgehog there is nothing wrong with using both of the random property testing libraries.
libs/cardano-ledger-binary/src/Cardano/Ledger/Binary/Decoding/Decoder.hs
Outdated
Show resolved
Hide resolved
libs/cardano-ledger-binary/test/Test/Cardano/Ledger/Binary/Vintage/Failure.hs
Outdated
Show resolved
Hide resolved
libs/cardano-ledger-binary/test/Test/Cardano/Ledger/Binary/Vintage/Failure.hs
Outdated
Show resolved
Hide resolved
JaredCorduan
force-pushed
the
jc/cbor-enforce-unique-map-keys
branch
from
d0c78b0 to
cef1bec
Compare
I only used Hedgehog since that was what was already being used in the |
|
|
Starting in major version 9, we know raise a decoding error if a CBOR map contains duplicate keys. resolves #2965
JaredCorduan
force-pushed
the
jc/cbor-enforce-unique-map-keys
branch
from
cef1bec to
ff7ea5a
Compare
|
@lehins I redid the tests in QuickCheck. let me know if this is what you had in mind. |
|
What was the behaviour when decoding a map with duplicate keys? Like, what value for the duplicate key was used? |
it was overwriting (so right biased). |
Description
Starting in major version 9, we raise a CBOR decoding error if a map contains duplicate keys.
resolves #2965
Checklist
fourmolu(which can be run withscripts/fourmolize.sh