Actually check the CollectCom transition consumes consistent PTs/STs #249
Conversation
| @@ -440,17 +441,23 @@ alterRedeemers fn = \case | |||
| in TxBodyScriptData dats (Ledger.Redeemers newRedeemers) | |||
|
|
|||
| -- | Remove redeemer for given `TxIn` from the transaction's redeemers map. | |||
There was a problem hiding this comment.
Should: update this outdated doc.
| @@ -80,11 +80,12 @@ genCommitMutation (tx, _utxo) = | |||
| , SomeMutation MutateCommittedValue <$> do | |||
| mutatedValue <- genValue `suchThat` (/= committedOutputValue) | |||
| let mutatedOutput = modifyTxOutValue (const mutatedValue) committedTxOut | |||
| pure $ ChangeInput committedTxIn mutatedOutput | |||
|
|
|||
There was a problem hiding this comment.
Could: remove redundant new line
| & filter (/= adaSymbol) | ||
| & \case | ||
| [headPolicyId] -> headPolicyId | ||
| _ -> traceError "malformed thread token, expected single asset" |
There was a problem hiding this comment.
Nice code. We should use consistent names though.
Should: As it seems not to be the thread token we retrieve, but this head's currency symbol, please rename threadToken.
Could: More minor, the term headPolicyId is not consistent with what plutus is calling it (ref symbols and adaSymbol). So maybe we should also be consistent in that regards?
There was a problem hiding this comment.
As it seems not to be the thread token we retrieve, but this head's currency symbol
Yup, I told myself the same thing yesterday when I self reviewed the code. It's not a thread token indeed.
| @@ -145,6 +168,13 @@ genCollectComMutation (tx, utxo) = | |||
| [ ChangeHeadDatum $ Head.Initial c moreParties | |||
| , ChangeOutput 0 $ mutatedPartiesHeadTxOut moreParties | |||
| ] | |||
| , SomeMutation MutateHeadId <$> do | |||
| illedHeadResolvedInput <- | |||
There was a problem hiding this comment.
😂 is that even a word? Maybe sickHeadResolvedInput is also an option?
No need to change, though.
There was a problem hiding this comment.
I thought that as a German-speaker, you would appreciate our creativity on words 🤓
The idea is to cause a mutation to force the inspection of participation tokens in the collect-com. Currently, tokens are only counted (and even... we only truly count commits). Yet, we do want in the end to look for token which have the same policy id as the thread token, and interpret them as PTs. Thus, by mutating the policy id of the thread token, we should in principle invalidate all PTs based on a different policy id. Which, for now, does not happen, hence the failing mutation test.
Make validator more efficient by avoiding constructing and traversing a list of PTs. And, this also reflects a bit more the logic itself.
So that, collect-com can validate properly.. but, we are running out of budget.
We need to optimize the collect-com! ... and probably other validators too.
Fortunately, this is a cost-free change! 🎉.
This is a temporary measure because, if we indeed want to run this on THE testnet, we need the execution budget to be smaller. So optimizing collect-com is a must (or reviewing the approach to do smaller steps via sequential collect of commits). → #254
Discovered in a flaky test which saw didn't see enough initials/commits because their TxIns were conflicting
Arbitrary TxIn, TxId, VerificationKey and Hashes thereof should not conflict as this would lead to surprising test failures
Fixes #241