Add PlutusLedgerApi QuickCheck instances #6624

t4ccer · 2024-11-01T18:25:35Z

Add Arbitrary, CoArbitrary and Function instances from QuickCheck for every PlutusLedgerApi type. This is a port of instances from https://github.com/Plutonomicon/plutarch-plutus

Pre-submit checklist:

Branch
- Tests are provided (if possible)
- Commit sequence broadly makes sense
- Key commits have useful messages
- Changelog fragments have been written (if appropriate)
- Relevant tickets are mentioned in commit messages
- Formatting, PNG optimization, etc. are updated
PR
- (For external contributions) Corresponding issue exists and is linked in the description
- Targeting master unless this is a cherry-pick backport
- Self-reviewed the diff
- Useful pull request description
- Reviewer requested

kozross · 2024-11-05T18:16:31Z

@effectfully - would you mind taking a look at this?

effectfully · 2024-11-05T19:03:49Z

It's on my list, but I can't do it this week, sorry, too many commitments. I'll do my best to review it the next week. Thanks for the reminder.

effectfully

Shrinking is broken, coverage is poor, invariants aren't respected. 3k lines of code.

Thank you for opening the PR! We'll process it, split into smaller PRs, fix issues etc.

effectfully · 2024-11-08T23:32:57Z

plutus-ledger-api/testlib/PlutusLedgerApi/Test/Common/QuickCheck/Utils.hs

+  arbitrary =
+    UnsafeSizedByteString . BS.pack <$> do
+      let !len = fromIntegral . natVal $ Proxy @n
+      vectorOf len arbitrary


vectorOf has to force len, so you don't really need a bang here, but it's OK to keep it, I'm only saying it as FYI.

effectfully · 2024-11-12T19:34:17Z

plutus-ledger-api/testlib/PlutusLedgerApi/Test/Common/QuickCheck/Utils.hs

+
+instance KnownNat n => Arbitrary (SizedByteString n) where
+  {-# INLINEABLE arbitrary #-}
+  arbitrary =


Note to self: when we generate ByteStrings normally we use Text.encodeUtf8 <$> arbitrary. Why? Should we do the same here?

This guarantees that you get ByteStrings that are UTF-8 encodings of text, which is probably what you often want. However, in a lot of situations here, we're generating arbitrary hashes, which most certainly do not follow this rule.

Then we should change this rule in our code in the first place. Thanks!

effectfully · 2024-11-12T19:36:30Z

plutus-ledger-api/testlib/PlutusLedgerApi/Test/Orphans/PlutusTx.hs

+instance Arbitrary PlutusTx.BuiltinByteString where
+  {-# INLINEABLE arbitrary #-}
+  arbitrary = PlutusTx.toBuiltin @ByteString <$> arbitrary
+  {-# INLINEABLE shrink #-}
+  shrink = fmap (PlutusTx.toBuiltin @ByteString) . shrink . PlutusTx.fromBuiltin


@Unisay once you're done figuring out what we're supposed to do with ByteString, could you just suggest what we should do here? Is it already fine?

So this instance delegates to the Arbitrary ByteString instance.
Assuming the latter is fine, this one should be also fine.

plutus-ledger-api/testlib/PlutusLedgerApi/Test/Orphans/PlutusTx.hs

effectfully · 2024-11-12T19:40:18Z

plutus-ledger-api/testlib/PlutusLedgerApi/Test/Orphans/PlutusTx.hs

+{- | This is a very general instance, able to produce 'PlutusTx.BuiltinData' of
+basically any shape. You probably want something more focused than this.
+-}
+instance Arbitrary PlutusTx.BuiltinData where


This is just a reimplementation (a likely worse one) of the already existing Arbitrary Data instance.

effectfully · 2024-11-12T20:25:28Z

plutus-ledger-api/testlib/PlutusLedgerApi/Test/Orphans/V1/Value.hs

+this can have zero values, and does not treat the Ada symbol or token name
+specially.
+-}
+instance Arbitrary Value where


We have that one as well, except more fine-grained.

Should we instead use instance that was in plutus-ledger-api/testlib/PlutusLedgerApi/Test/V1/Value.hs?

We should merge ours with yours as both have good and bad things about them. I assume MLabs isn't paid by IOG to do this work?

No, current Plutarch work is part of Catalyst project

Good to know, thanks. It'll probably take us a lot of time to adopt all of this code tbh, due to other things being considered higher priority. We'll tag you on relevant PRs if we have any improvement suggestions over what you have here. Thanks again for creating this PR, we'll keep it open for now and we'll see how it goes with adopting your code.

effectfully · 2024-11-12T20:26:03Z

plutus-ledger-api/testlib/PlutusLedgerApi/Test/Orphans/V1/Value.hs

+2. It is difficult to work within the size limit (32 bytes) when generating
+   UTF-8.
+-}
+instance Arbitrary TokenName where


This one we definitely need to steal.

plutus-ledger-api/testlib/PlutusLedgerApi/Test/Orphans/V1/Value.hs

effectfully · 2024-11-12T20:29:50Z

plutus-ledger-api/testlib/PlutusLedgerApi/Test/Orphans/V1/Value.hs

+described above: failing to do so means all guarantees of this type are off
+the table.
+-}
+newtype MintValue = MintValue Value


We now have our own MintValue type with its generator.

Could you point me to that please? This is also "old" mint value, i.e. for V1 and V2 that has zero ADA entry, our V3 MintValue has that omitted

PlutusLedgerApi.V3.MintValue.MintValue and the generator is in PlutusLedgerApi.Test.V3.MintValue. Ours is probably broken though, we need to fix it.

effectfully · 2024-11-12T20:40:30Z

plutus-ledger-api/testlib/PlutusLedgerApi/Test/V1/Value.hs

-    arbitrary = error "No such 'Arbitrary' instance"
-    shrink _ = []
-
-instance Arbitrary Value where


That's bold haha.

Such intervals are provided by cardano-node, see: <https://github.com/IntersectMBO/plutus/blob/c082e28591990a14a9f4f91700b684590ac252be/plutus-ledger-api/src/PlutusLedgerApi/V1/Interval.hs#L369-L393>

This is a backport of <Plutonomicon/plutarch-plutus@6c8db66>

t4ccer added 4 commits November 1, 2024 12:14

Add V1 PlutusLedgerApi QuickCheck instances

a5aa618

Add V2 PlutusLedgerApi QuickCheck instances

6029d31

Add V3 PlutusLedgerApi QuickCheck instances

4bda6e1

Remove obsolete Arbitrary instances

6ed9dd3

t4ccer temporarily deployed to github-pages November 1, 2024 18:26 — with GitHub Actions Inactive

t4ccer added 2 commits November 1, 2024 12:54

Fix build with GHC8

33de71e

Add changelog fragment

7f28295

t4ccer temporarily deployed to github-pages November 1, 2024 18:56 — with GitHub Actions Inactive

Remove main changelog entry

b2ad5e8

t4ccer temporarily deployed to github-pages November 1, 2024 18:58 — with GitHub Actions Inactive

effectfully mentioned this pull request Nov 9, 2024

Ledger types testing #6618

Closed

11 tasks

Add UnsortedAssocMap

79d543c

t4ccer temporarily deployed to github-pages November 10, 2024 23:34 — with GitHub Actions Inactive

effectfully reviewed Nov 12, 2024

View reviewed changes

t4ccer added 5 commits November 13, 2024 12:50

Shrink hashes towards all zeros

9597cf9

Shrink fields one by one

2deda45

Implement Arbitrary BuiltinData in terms of Data

076343d

Shrink Extended towards Finite 0

dbd6dac

Generate arbitrary closure bounds at infinities

60ebef7

Such intervals are provided by cardano-node, see: <https://github.com/IntersectMBO/plutus/blob/c082e28591990a14a9f4f91700b684590ac252be/plutus-ledger-api/src/PlutusLedgerApi/V1/Interval.hs#L369-L393>

t4ccer temporarily deployed to github-pages November 13, 2024 23:19 — with GitHub Actions Inactive

Generate large lovelace entries in FeeValue

e077140

t4ccer temporarily deployed to github-pages November 14, 2024 02:09 — with GitHub Actions Inactive

t4ccer added 2 commits November 13, 2024 19:37

Merge branch 'master' into t4/quickcheck

3586aba

Fix post-merge duplicate Arbitrary instances

655c729

t4ccer temporarily deployed to github-pages November 14, 2024 03:24 — with GitHub Actions Inactive

Fix V3.MintValue generator

cdcac03

This is a backport of <Plutonomicon/plutarch-plutus@6c8db66>

t4ccer temporarily deployed to github-pages November 15, 2024 05:06 — with GitHub Actions Inactive

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add PlutusLedgerApi QuickCheck instances #6624

Add PlutusLedgerApi QuickCheck instances #6624

t4ccer commented Nov 1, 2024

kozross commented Nov 5, 2024

effectfully commented Nov 5, 2024

effectfully left a comment

effectfully Nov 8, 2024

effectfully Nov 12, 2024

kozross Nov 13, 2024 •

edited

Loading

effectfully Nov 13, 2024

effectfully Nov 12, 2024

Unisay Nov 13, 2024

effectfully Nov 12, 2024

effectfully Nov 12, 2024

t4ccer Nov 14, 2024

effectfully Nov 14, 2024

t4ccer Nov 15, 2024

effectfully Nov 15, 2024

effectfully Nov 12, 2024

effectfully Nov 12, 2024

t4ccer Nov 14, 2024 •

edited

Loading

effectfully Nov 14, 2024 •

edited

Loading

effectfully Nov 12, 2024

Add PlutusLedgerApi QuickCheck instances #6624

Are you sure you want to change the base?

Add PlutusLedgerApi QuickCheck instances #6624

Conversation

t4ccer commented Nov 1, 2024

kozross commented Nov 5, 2024

effectfully commented Nov 5, 2024

effectfully left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kozross Nov 13, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

t4ccer Nov 14, 2024 • edited Loading

Choose a reason for hiding this comment

effectfully Nov 14, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kozross Nov 13, 2024 •

edited

Loading

t4ccer Nov 14, 2024 •

edited

Loading

effectfully Nov 14, 2024 •

edited

Loading