-
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
Fix MultiAsset generation #3347
Conversation
e29049b
to
9bfd2e2
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 great to me, thank you @aniketd !
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.
Great job on investigating this problem and figuring out the value of 910. Taking a quick peek at the compaction implementation it does seem to be the correct value indeed.
There are some problems that I commented on in the PR itself, but the overall direction is totally correct.
We are still missing fixes to the deserializer, but I am sure you knew this already anyways.
c6807d3
to
e20ec44
Compare
e20ec44
to
368b1a2
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.
More work is needed in order to get this right.
libs/cardano-ledger-binary/testlib/Test/Cardano/Ledger/Binary/RoundTrip.hs
Outdated
Show resolved
Hide resolved
ea040f7
to
dcd5ed6
Compare
b22ea27
to
88c70d6
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.
Couple of minor changes are still needed, but other than that this looks awesome!
88c70d6
to
e37889d
Compare
e37889d
to
4228466
Compare
4228466
to
f362139
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.
Two final comments. This is awesome stuff! Thank you Aniket!
f362139
to
1a77632
Compare
Serialization version inside the test differed from the one used in `setMinCoinTxOut`. Both should be drawing the protocol version from the supplied PParams
1a77632
to
a018a68
Compare
Description
Closes #3312
Checklist
CHANGELOG.md
for affected packagefourmolu
(usescripts/fourmolize.sh
)scripts/cabal-format.sh
)hie.yaml
has been updated (usescripts/gen-hie.sh
)