Update PTryFrom instances for Plutarch 1.2
#520
Conversation
- PMaybeData - PCredential - PPubKeyHash - PValidatorHash
…-ptryFrom-instances
- recovering PValue - recovering all the types that belong to PValue - test all the recoveries succeed with the correct values with property tests - new module Extra.ByteSTring that exports convenience functions
|
Thanks! |
Plutarch/Api/V1/Tx.hs
Outdated
| instance PTryFrom PData (PAsData PTxId) where | ||
| type PTryFromExcess PData (PAsData PTxId) = Flip Term PTxId | ||
| ptryFrom' opq = runTermCont $ do | ||
| opq' <- tcont . plet $ pasConstr # opq | ||
| dataBs <- tcont $ \f -> | ||
| pif | ||
| (pfstBuiltin # opq' #== 0 #&& plength # (psndBuiltin # opq') #== 1) | ||
| (f $ phead #$ psndBuiltin # opq') | ||
| (ptraceError "ptryFrom(TxId): bad constructor") | ||
| unwrapped <- tcont . plet $ ptryFrom @(PAsData PByteString) dataBs snd | ||
| tcont $ \f -> | ||
| pif (plengthBS # unwrapped #== 28) (f ()) (ptraceError "ptryFrom(TxId): must be 28 bytes long") | ||
| pure (punsafeCoerce opq, pcon . PTxId $ pdcons # pdata unwrapped # pdnil) |
There was a problem hiding this comment.
@L-as Is this the most ergonomic way to implement PTryFrom for data sum stuff currently? Surely users shouldn't be expected to perform structure validation when the target invariant is only for one of the fields within - I assume there is a better solution utilizing PTryFromExcess?
There was a problem hiding this comment.
From my other comment:
Optimally you should be able to first verify it's a PDataSum, but because it doesn't return any excess, it's not efficient.
Plutarch/Api/V1/Tx.hs
Outdated
| ptryFrom' opq cont = ptryFrom @(PAsData PTxId) opq (cont . first punsafeCoerce) | ||
|
|
||
| instance PTryFrom PData (PAsData PTxId) where | ||
| type PTryFromExcess PData (PAsData PTxId) = Flip Term PTxId |
There was a problem hiding this comment.
There's no point in having this excess, since it's the same as the output.
Plutarch/Api/V1/Tx.hs
Outdated
| opq' <- tcont . plet $ pasConstr # opq | ||
| dataBs <- tcont $ \f -> | ||
| pif | ||
| (pfstBuiltin # opq' #== 0 #&& plength # (psndBuiltin # opq') #== 1) |
There was a problem hiding this comment.
This is inefficient. You're traversing the list twice.
There was a problem hiding this comment.
Optimally you should be able to first verify it's a PDataSum, but because it doesn't return any excess, it's not efficient.
There was a problem hiding this comment.
This seems like a big drawback, why doesn't PDataSum return an excess with fields?
There was a problem hiding this comment.
(Human) laziness. I'm sure there's a way to do it, but I'd rather rethink the whole excess field bullshit.
Plutarch/Builtin.hs
Outdated
| @@ -3,6 +3,7 @@ | |||
| {-# OPTIONS_GHC -Wno-orphans #-} | |||
|
|
|||
| module Plutarch.Builtin ( | |||
| Flip, | |||
There was a problem hiding this comment.
maybe we should add it to Extra.TryFrom, otherwise everybody declares their own Flip? Maybe it's not an issue though.
There was a problem hiding this comment.
it really does feel weird for everyone to have to define it inline - even if it's supposed to be a "type lambda".
+1 to having it exposed in plutarch-extra at this point
|
something something name not much shorter than definition.
Rather than doing
```haskell
import Something (Flip)
```
You just do
```haskell
newtypo Flip f b a = Flip (f a b) deriving stock Generic
```
It is barely any shorter to do the derive, and it's barely code reuse. Inlining is good to help people understand the code.
|
Updates based on #438