-
Notifications
You must be signed in to change notification settings - Fork 156
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Cleanup address deserialization #3218
Conversation
bc1ccbd
to
f3b7266
Compare
f3b7266
to
52f1426
Compare
52f1426
to
487e4b5
Compare
libs/cardano-ledger-core/test/Test/Cardano/Ledger/AddressSpec.hs
Outdated
Show resolved
Hide resolved
libs/cardano-ledger-binary/test/Test/Cardano/Ledger/Binary/RoundTripSpec.hs
Show resolved
Hide resolved
libs/cardano-ledger-binary/testlib/Test/Cardano/Ledger/Binary/RoundTrip.hs
Outdated
Show resolved
Hide resolved
libs/cardano-ledger-binary/testlib/Test/Cardano/Ledger/Binary/RoundTrip.hs
Outdated
Show resolved
Hide resolved
libs/cardano-ledger-binary/testlib/Test/Cardano/Ledger/Binary/RoundTrip.hs
Show resolved
Hide resolved
487e4b5
to
f071445
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
First two commits could be probably squashed into one, to make each remaining commit work correctly, and same with next two commits, but don't think it matters too much, with all the moving around.
* Also add roundtrip for CompactAddr
…er. Deprecate the former
f071445
to
7236776
Compare
I was unable to squash last two commits, since we then loose git history, however squashing the subsequent two worked out fine. |
Description
Currently there are 4 separate implementations for deserializing an
Addr
. I am not gonna go into history why this is the case, but this PR moves old implementations into tests and uses them for ensuring that deserialization doesn't break accidentally over time and also as a baseline for checking performance. Summary:CompactAddr
andAddr
CompactAddr
is no longer exposed, making it impossible to create invalid and dangerous binary representations for addressesCardano.Ledger.CompactAddress
intoCardano.Ledger.Address
, while preserving git historyfromCBOR
inBabbageTxBody
andBabbagetxOut
for address. Now possible thanks to versioned deserializationChecklist
scripts/ormolise.sh