-
Notifications
You must be signed in to change notification settings - Fork 483
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
[Builtins] Added 'Data' as a built-in type #3385
[Builtins] Added 'Data' as a built-in type #3385
Conversation
plutus-core/plutus-core/test/Pretty/Golden/Readable/StdLib/Data/List/FoldrList.plc.golden
Outdated
Show resolved
Hide resolved
...ore/untyped-plutus-core/test/Evaluation/Machines/Memory/StdLib/Data/List/FoldList.plc.golden
Outdated
Show resolved
Hide resolved
...re/untyped-plutus-core/test/Evaluation/Machines/Memory/StdLib/Data/List/FoldrList.plc.golden
Outdated
Show resolved
Hide resolved
...us-core/untyped-plutus-core/test/Evaluation/Machines/Memory/StdLib/Data/List/List.plc.golden
Outdated
Show resolved
Hide resolved
932ff8a
to
cfd2f2f
Compare
47ec54b
to
e2aea26
Compare
instance Serialise a => Flat (AsSerialize a) where | ||
encode = encode . serialise . unAsSerialize | ||
decode = AsSerialize . deserialise <$> decode | ||
size = error "Implement me" |
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.
@michaelpj what about size
?
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.
We're encoding a lazy bytestring, so presumably size @ByteString
. Or you could call https://hackage.haskell.org/package/flat-0.4.4/docs/Flat-Encoder-Strict.html#v:sLazyBytes which is how size
is implemented for lazy bytestrings.
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.
This:
newtype AsSerialize a = AsSerialize
{ unAsSerialize :: a
} deriving newtype (Serialise)
instance Serialise a => Flat (AsSerialize a) where
encode = encode . serialise
decode = deserialise <$> decode
size = size . serialise
? Also pinging @kwxm and @raduom who I think also did something with flat
(for context: we're trying to write a Flat
instance for things that already have a Serialize
instance).
e2aea26
to
bff5501
Compare
Ready for review. |
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.
Generally looks good.
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 OK, and it'll be interesting to see what happens when it's integrated with the plugin. Presumably it'll be faster and smaller: would it be easy to generate some random examples to try it on right now before we do it for real? It's probably not worth the bother, but if it's something that could be done in 20 minutes if might be interesting to check.
I worry a little bit about costing, since if if we get this in the plugin and we don't have any budgeting in place it'll mess up the cost model (although now that I think about it, it looks as if builtins typically only take up 2 or 3% of the execution time, so if reading data's really quick it might be safe to ignore it for a bit). I couldn't see any issues for this, so if there's nothing relevant maybe we should make one and think about it a bit (there's an issue about how lists and Data advertise their size to the budgeting mechanism, for example). On the other hand, I should try to finalise the cost model in its current form before we start adding complicated new stuff. I hope to get that done by the end of this sprint.
@@ -85,6 +88,15 @@ implementations for them (if they have any constructors reserved for future use) | |||
By default, Flat does not use any space to serialise `()`. | |||
-} | |||
|
|||
newtype AsSerialize a = AsSerialize | |||
{ unAsSerialize :: a |
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.
[Comment deleted: I didn't understand what was going on here, but then spotted the previous conversation with @michaelpj.]
I wonder if doing it this way is significantly more expensive or larger than having a special instance?
Also, how are Data
objects going to be transmitted? I had a vague idea that they were going to be Flat wrapped in CBOR, but the earlier conversation above suggests that there's already a full CBOR instance. What is that for? If it's for transmission, why do we need a Flat instance at all? Because we might have programs with Data
objects inside them?
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.
I've no idea what I'm doing, I only implemented @michaelpj's suggestion.
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.
Data
has a CBOR instance, because it's going to appear in a number of other places in transactions etc, and also it's literally designed to be a subset of CBOR. People may want to generate it using other tools etc.- We need all types in the builtin universe to have
Flat
instances, soData
also needs aFlat
instance. As Kenneth says, indeed this is so that we could serialize a program with aData
literal in it. For example, the fully-applied scripts produced by theplutus-use-cases-scripts
thing will be like that. - We could give
Data
a newFlat
instance, but given that we already have a way of serializing it, we might as well just dump it in as an opaque serialized bytestring. That has the advantage that you could in principle fish it out and work with it separately, or whatever.
plutus-core/untyped-plutus-core/test/Evaluation/Builtins/Definition.hs
Outdated
Show resolved
Hide resolved
Just realised we will almost certainly need |
…to effectfully/builtins/add-data
…fectfully/plutus-prototype into effectfully/builtins/add-data
…fectfully/plutus-prototype into effectfully/builtins/add-data
…fectfully/plutus-prototype into effectfully/builtins/add-data
It's green! Comments addressed. Remaining stuff is parsing and pretty-printing of newtype AsSerialize a = AsSerialize
{ unAsSerialize :: a
} deriving newtype (Serialise)
instance Serialise a => Flat (AsSerialize a) where
encode = encode . serialise
decode = deserialise <$> decode
size = size . serialise is correct (in particular, the definition of |
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.
Honestly, the main part is surprisingly painless!
-- | ||
-- > /\(a :: *) -> \(f : a -> a) (p : pair a a) -> | ||
-- > biconstPair {a} {a} (f (fst {a} {a} p)) (f (snd {a} {a} p)) p | ||
obothPair :: TermLike term TyName Name DefaultUni (Either DefaultFun ExtensionFun) => term () |
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.
Could we have uncurry
too? I think that's probably the most useful function for me, since I'll want to write builtinPairToHaskellPair
which is more-or-less uncurryBuiltin haskellPairConstructor
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.
Oh, it's in the stdlib bit. I don't really understand why this isn't there, then? Whatever.
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.
So stdlib
is for people to use, examples
is for me to write tests. Monomorphic mapping is a weird thing, so I don't want people to use it (unless they really need to, in which case it should be moved to stdlib
), hence it lives in examples
.
its @Data@ argument and chooses the appropriate branch (type instantiations and strictness concerns | ||
are omitted for clarity): | ||
|
||
chooseData |
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.
This makes me wonder whether we should have chooseList
instead of null
for consistency (noting that ifThenElse
is exactly chooseBool
). That would give us a consistent story for "how to do built-in datatypes": have a chooseFoo
function and then an unsafeUnConstrX
function for each constructor.
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.
I kinda like how if null xs then z else f (head xs) (tail xs)
reads, but I wouldn't mind using chooseList
. But let's do that separately then.
@@ -85,6 +88,15 @@ implementations for them (if they have any constructors reserved for future use) | |||
By default, Flat does not use any space to serialise `()`. | |||
-} | |||
|
|||
newtype AsSerialize a = AsSerialize | |||
{ unAsSerialize :: a |
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.
Data
has a CBOR instance, because it's going to appear in a number of other places in transactions etc, and also it's literally designed to be a subset of CBOR. People may want to generate it using other tools etc.- We need all types in the builtin universe to have
Flat
instances, soData
also needs aFlat
instance. As Kenneth says, indeed this is so that we could serialize a program with aData
literal in it. For example, the fully-applied scripts produced by theplutus-use-cases-scripts
thing will be like that. - We could give
Data
a newFlat
instance, but given that we already have a way of serializing it, we might as well just dump it in as an opaque serialized bytestring. That has the advantage that you could in principle fish it out and work with it separately, or whatever.
The entire thing would've been completely painless if we didn't support polymorphic functions over polymorphic builtins, 'cause it's the part that requires all the |
Don't look here yet.