Value encoding #1979
Merged
Value encoding #1979
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
redxaxder
force-pushed
the
redxaxder/cborvalue
branch
from
77d8c6e to
ee7065b
Compare
nc6
reviewed
Nov 16, 2020
|
|
||
| -- ======================================================== | ||
|
|
||
| -- | Remove 0 assets from a map | ||
| prune :: Map (PolicyID era) (Map AssetID Integer) |
There was a problem hiding this comment.
I guess this is perhaps a bit slower than the old method?
redxaxder
force-pushed
the
redxaxder/cborvalue
branch
11 times, most recently
from
69516f9 to
d2d5a7a
Compare
redxaxder
force-pushed
the
redxaxder/cborvalue
branch
2 times, most recently
from
1f689bf to
7360191
Compare
redxaxder
force-pushed
the
redxaxder/cborvalue
branch
from
7360191 to
3b73ac7
Compare
nc6
approved these changes
Nov 25, 2020
| fromCBOR, | ||
| toCBOR, | ||
| ) | ||
| import Cardano.Binary (Decoder, Encoding, FromCBOR, ToCBOR, TokenType (..), decodeInt64, decodeWord64, fromCBOR, peekTokenType, toCBOR) |
There was a problem hiding this comment.
Can we avoid such very long lines?
There was a problem hiding this comment.
Doesn't ormolu do something about this?
There was a problem hiding this comment.
Ormolu basically has two choices: one line, or split them. If you add a random line break somewhere in there, ormolu will then format the whole list. But it allows you to choose not to add any line breaks, as in this case
| @@ -1 +1 @@ | |||
| -c "nix-shell ../../../shell.nix --run \"cabal repl -f development shelley-spec-ledger-test\"" -o ghcid.txt | |||
| -c "nix-shell ../../../shell.nix --run \"cabal repl -f development test:shelley-spec-ledger-test\"" -o ghcid.txt | |||
There was a problem hiding this comment.
probably not, I'll remove it
mrBliss
reviewed
Nov 25, 2020
| fromCBOR, | ||
| toCBOR, | ||
| ) | ||
| import Cardano.Binary (Decoder, Encoding, FromCBOR, ToCBOR, TokenType (..), decodeInt64, decodeWord64, fromCBOR, peekTokenType, toCBOR) |
There was a problem hiding this comment.
Doesn't ormolu do something about this?
Comment on lines
486
to
487
| let assume Nothing = error $ "illegal value in txout: " <> show vl | ||
| assume (Just x) = x |
There was a problem hiding this comment.
Similar suggestion about fromMaybe
JaredCorduan
force-pushed
the
redxaxder/cborvalue
branch
from
a9f6fef to
bb55dfd
Compare
The Value type is used in both TxOuts and in the Mint field of the transaction body, but negative values are only allowed in the latter. The rules already enforce this, but now the serialization does too. Resolves: CAD-1667 and CAD-2148
JaredCorduan
force-pushed
the
redxaxder/cborvalue
branch
from
bb55dfd to
44bb3b2
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Different serializations for Value
The Value type is used in both TxOuts and in the Mint field of the transaction body, but negative values are only allowed in the latter. The rules already enforce this, but now the serialization does too.
Resolves: CAD-1667 and CAD-2148