Fix onchain logic #5
Conversation
|
Thanks Riley for creating this PR. I've been dealing with a really sick child and have not done anything since I chatted to you last time on Discord. Here's a summary of the PR:
|
| getOutput txOuts asset = | ||
| case [o | o <- txOuts, containsClass o asset] of | ||
| [x] -> x | ||
| _ -> traceError "Fail here." |
There was a problem hiding this comment.
This is the part where it doesn't return a TxOut, instead it calls traceError. If we don't always return a result, we should be returning a Maybe (or something similar) instead of bailing with traceError.
| getOutputPDatum :: TxInfo -> [TxOut] -> TxOut | ||
| getOutputPDatum info txOuts = case [o | o <- txOuts, containsPot info o] of | ||
| [x] -> x | ||
| _ -> traceError "Fail here." |
There was a problem hiding this comment.
This is the part where it doesn't return a TxOut, instead it calls traceError. If we don't always return a result, we need to return a maybe instead of bailing with traceError.
| _ -> traceError "Fail here." | ||
| {-# INLINABLE findOutputForClass #-} | ||
| findOutputForClass :: AssetClass -> [TxOut] -> Maybe TxOut | ||
| findOutputForClass asset = find $ \o -> containsClass o asset |
There was a problem hiding this comment.
The naming convention for getting a value (if possible) from a collection is find so I named it this way, it also corresponds with the find call in the implementation.
| in case d of | ||
| Just PotDatum -> True | ||
| _ -> False | ||
| case findBountyDatum info o of |
There was a problem hiding this comment.
Since Haskell is expression based, you don't need to assign value to a variable and then pattern match on that variable, you can directly pattern match on the value/expression.
|
|
||
| policy :: AssetClass -> Scripts.MintingPolicy | ||
| policy asset = | ||
| mkMintingPolicyScript $ | ||
| $$(PlutusTx.compile [||Scripts.wrapMintingPolicy . mkPolicy||]) | ||
| `PlutusTx.applyCode` PlutusTx.liftCode asset | ||
| `PlutusTx.applyCode` | ||
| PlutusTx.liftCode asset |
There was a problem hiding this comment.
I don't understand this formatting 🤷♂️ . However, setting it like this will stop this formatting from getting into every PR as noise. I've asked about this particular issue on IOG's tech channel, this seems to be stylish-haskell's fault, well, I guess we need to live with it.
| -- validateUseOfPot bounty mPotTxOut mpot mcollection = case mpot of | ||
| validateUseOfPot bounty (Just potTxOut) (Just _) (Just (CollectionDatum c)) = | ||
| solidCollection bounty c && correctCollection potTxOut c | ||
| validateUseOfPot _ _ _ _ = False |
There was a problem hiding this comment.
I refactored this part so that it's clearer in that line 157 lists the case for which we want to run the validator logic. For all other cases, we evaluate to False straight away.
When we write it in nested style like the removed code above, it's can be less obvious what cases (branches) have been covered and what have not.
| bVoters :: ![PubKeyHash], | ||
| bRequiredVotes :: !Integer, | ||
| bCollectionMakerClass :: !AssetClass, | ||
| bCollectionToken :: !AssetClass |
There was a problem hiding this comment.
It's common practice to prefix record field names with data type initials. In this case one letter prefix is enough, however usually with several record data structures, two-letter prefixes are more common.
This has been one of the biggest issues with Record in Haskell. I think this particular variable scoping issue has been fixed in GHC 9.x, I've not tried it myself though so can't comment much on that.
|
It doesn't make much sense for me to review my own code, but I've commented on a few changes hoping to make the intentions clear. |
This is a direct PR from Brians fork to our own, I have yet to read through this completely and I will be leaving my own comments below. I have requested review from Brian, Siegfried, and Dz.
Thanks all.