Skip to content
This repository has been archived by the owner on Aug 11, 2022. It is now read-only.

tasty-plutus: excessive type safety could hide bugs #173

Open
AriFordsham opened this issue Jan 31, 2022 · 6 comments
Open

tasty-plutus: excessive type safety could hide bugs #173

AriFordsham opened this issue Jan 31, 2022 · 6 comments
Labels
bug Something isn't working question Further information is requested

Comments

@AriFordsham
Copy link
Contributor

Since 6.0, TestData has encoded the token-minting instructions as a (list of) MintingPolicyTask which is constructed with separate functions for burn and mint, which cannot be passed negative values. This ensures the correct action is always performed.

However, forgetting to account for the wrong sign is a understandable source of bugs with potentially devastating scope. It would be more useful if property testing by default tested with positive and negative numbers to ensure these are caught.

@kozross
Copy link
Contributor

kozross commented Jan 31, 2022

What do you mean by 'forgetting to account for the wrong sign' here? I'm not quite following what kind of issue you're expecting to catch.

@AriFordsham
Copy link
Contributor Author

Naively allowing e.g. liberal permissions for token burning while forgetting to check that the quantity is not positive.

@AriFordsham
Copy link
Contributor Author

I kind of just ran into this.

@kozross kozross added the bug Something isn't working label Jan 31, 2022
@sergesku
Copy link
Contributor

@AriFordsham I`m not sure what you mean.

Blockchain MintingPolicy doesn't have separate actions for minting and burning. It works with positive and negative Integers.

When constructing we specify a MintingPolicyTask with a MintingAction and a Positive amount of Tokens

data MintingPolicyTask = MPTask MintingPolicyAction Tokens

data Tokens = Tokens TokenName Positive

data MintingPolicyAction = BurnAction | MintAction

When we create a ScriptContext we process the MintingPolicyTask and specify an Integer sign

processMPTask :: MintingPolicyTask -> (TokenName, Integer)
processMPTask (MPTask action (Tokens tn pos)) =
  let i = case action of
        MintAction -> getPositive pos
        BurnAction -> negate $ getPositive pos
   in (tn, i)

If I missed something, could you provide a detailed example.

@AriFordsham
Copy link
Contributor Author

I just wrote a minting policy which had restrictive mint conditions, but liberal burn. I accidentally omitted the sign check on the burn action, allowing basically unrestricted minting. If the testing default would be to test positive and negative numbers I think I would have caught it faster.

@sergesku
Copy link
Contributor

sergesku commented Feb 2, 2022

I still believe, that our implementation covers all cases.

As I understand it, by mint conditions you mean something like (AssetClass, 5), and by burn conditions something like (AssetClass, -5).
And it doesn't make sense to check the sign of the burn action because it's always negative. If the action has the positive sign, it is a mint action.

I can be very wrong. Therefore, please give a detailed example (code) of a MintingPolicy with a description of which case we cannot cover with our model. Catching abstract bugs is very difficult for me.

@re-xyr re-xyr added the question Further information is requested label Feb 20, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working question Further information is requested
Projects
None yet
Development

No branches or pull requests

4 participants