Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

QuickCheck Arbitrary instances for Ledger types #6210

Open
SeungheonOh opened this issue Jun 13, 2024 · 14 comments · May be fixed by #6624
Open

QuickCheck Arbitrary instances for Ledger types #6210

SeungheonOh opened this issue Jun 13, 2024 · 14 comments · May be fixed by #6624
Labels
Low priority Doesn't require immediate attention status: triaged Test

Comments

@SeungheonOh
Copy link

SeungheonOh commented Jun 13, 2024

Describe the feature you'd like

Currently, each projects need to write some property testing generator and shrinker on per-project biases, making testing not only more difficult but possibly more inconsistent and un-standardized. Quickcheck Arbirary instances for each types that PlutusLedgerApi exports would immensely help when testing scripts, generating some place-holder values, or even maintaining eDSL equivalent for ledger api--for example, Plutarch.

@effectfully
Copy link
Contributor

effectfully commented Jun 18, 2024

That would be great to have indeed. I've brought it to the team and the team agreed.

I'm going to mark this issue as "low priority" though, since it's not a bug or similar.

PR would be very welcome.

@effectfully effectfully added Test Low priority Doesn't require immediate attention status: triaged and removed status: needs triage GH issues that requires triage labels Jun 18, 2024
@aleeusgr
Copy link

aleeusgr commented Jul 12, 2024

Property Testing! cool, It looks like something I could implement.
Mentorship is welcome 😇

My questions are:

  1. What do I start with?
    Common.hs seem to be good to start with, but there are 94 lines of types and functions, and among the types it is likely not all are made equal in terms of how useful it is to to have random value generation for them. Are these good enough candidates?

    See Note [Cost model parameters]
    -}
    PLC.CostModelParams,
    ParamName.toCostModelParams,
    Eval.assertWellFormedCostModelParams,
    ParamName.IsParamName (showParamName, readParamName),
    ParamName.GenericParamName,
    ParamName.CostModelApplyError (..),
    ParamName.CostModelApplyWarn (..),

  2. Where do the instances go?
    It should be something like Property.hs in plutus-ledger-api/testlib/ , right?

Thanks!

@effectfully
Copy link
Contributor

@aleeusgr thank you for volunteering! How about you take the work done by MLabs and upstream it into our repo? While improving everything that you can think of along the way.

Where do the instances go?
It should be something like Property.hs in plutus-ledger-api/testlib/ , right?

We have a generator for Value in plutus-ledger-api/testlib/PlutusLedgerApi/Test/V1/Value.hs. Given that MLabs folks put their generators under V2, it makes sense to just continue with this convention and put generators under the appropriate version in testlib.

@SeungheonOh
Copy link
Author

It would be great if you guys can get our instances upstreamed. That was our original plan, but given we have to complete Plutus Ledger API V3 in time, we had to vendor it temporarily.

Most instances come with all necessary parts with some degree of assurance over invariants(e.g. value's entries being sorted): generator, shrinker, and utilities for function generators. Some of the more tricky ones like PScriptContext and PTxInfo doesn't have shrinker due to their complex invariants and some fields being illogical to shrink. You might be able to improve that. Generators for these "tricky" types are quite slow as well. So, that's another part you can improve.

Rest should be good in terms of quality, documentation, and code structure. We've actually used this to check data encoding for Plutarch ledger API and already caught handful of bugs. Property testing rocks!

@aleeusgr
Copy link

aleeusgr commented Jul 18, 2024

I've been digging into the Value type and its Arbitrary instance in testlib, comparing it to FeeValue and its generator in plutarch-plutus. It looks like in some cases I may disregard wrappers and reuse their generators with minimal code changes.

Now the goal is to find the types that do not have generators in plutus but do have them in plutarch-plutus; 🪫🇪🇬🥵💡☯️

I added a test: https://github.com/aleeusgr/plutus/tree/ledger-types-testing

now I am looking for a way to list all types that need to be covered.

references

@effectfully
Copy link
Contributor

I've been digging into the Value type and its Arbitrary instance in testlib

This one is pretty good when it comes to the distribution of generated trees, but it doesn't attempt to satisfy various invariants like the one by MLabs does. It would be ideal to preserve the current generation of trees and add the invariants logic.

Now I focus on finding the types that do not have generators in plutus but do have them in plutarch-plutus

Thank you for doing this!

@aleeusgr
Copy link

aleeusgr commented Oct 7, 2024

Hello! Sadly I report that I am still stuck with this issue.
I explored the repo and the PR suggested above, and although I made some progress initially at the moment I am struggling with formulating next actionable steps.

My first hunch was to automate reasoning about "Type X has an Arbitrary", at which I partially succeeded. I wrote a test that fails to build if a Type has no RNG. But then I failed to find a way to list all the exported types.

Then I decided to not bother with high level and try to copy and paste an RNG from plutarch. Here I don't understand what my acceptance criteria would look like. To illustrate, lets look at Value.

first I load the module to repl cabal repl plutus-ledger-api and I get "ok, 40 modules loaded"
From there I can :l PlutusLedgerApi.V1.Value and then

ghci> :info PlutusLedgerApi.V1.Value.Value 
type Value :: *
newtype Value
  = Value {getValue :: PlutusTx.AssocMap.Map
                         CurrencySymbol (PlutusTx.AssocMap.Map TokenName Integer)}
        -- Defined at src/PlutusLedgerApi/V1/Value.hs:278:1
instance Eq Value
  -- Defined at src/PlutusLedgerApi/V1/Value.hs:307:10
instance Monoid Value
  -- Defined at src/PlutusLedgerApi/V1/Value.hs:321:10
instance Semigroup Value
  -- Defined at src/PlutusLedgerApi/V1/Value.hs:314:10
instance Show Value
  -- Defined at src/PlutusLedgerApi/V1/Value.hs:279:46

as you can see there is no Arbitrary
Fair enough you say, its not in the main lib but in the testlib
loading it to repl, then I use :info PlutusLedgerApi.Test.V1.Value. and press Tab to show the options.
I get the following:

ghci> :info PlutusLedgerApi.Test.V1.Value.
PlutusLedgerApi.Test.V1.Value.FaceValue               PlutusLedgerApi.Test.V1.Value.unFaceValue
PlutusLedgerApi.Test.V1.Value.NoArbitrary             PlutusLedgerApi.Test.V1.Value.unNoArbitrary
PlutusLedgerApi.Test.V1.Value.genShortHex             PlutusLedgerApi.Test.V1.Value.uniqueNames
PlutusLedgerApi.Test.V1.Value.listsToValue            PlutusLedgerApi.Test.V1.Value.valueToLists
PlutusLedgerApi.Test.V1.Value.toCellCandidatesNumber

There is no Arbitrary there!
but if I go to testlib/PLA/Test/V1/Value.hs its there on Line 83.

I am really confused! Should I not user repl in this case?
Please help me with getting the acceptance criteria!

For example the PR defines Arbitrary

instance Arbitrary PLA.Credential where
  {-# INLINEABLE arbitrary #-}
  arbitrary =
    oneof
      [ PLA.PubKeyCredential <$> arbitrary
      , PLA.ScriptCredential <$> arbitrary
      ]

-- | @since 1.0.0
instance CoArbitrary PLA.Credential where
  {-# INLINEABLE coarbitrary #-}
  coarbitrary = \case
    PLA.PubKeyCredential pkh -> variant (0 :: Int) . coarbitrary pkh
    PLA.ScriptCredential sh -> variant (1 :: Int) . coarbitrary sh

Let me implement it! But how do I know if it needs to be done and when I do it how do I know I am successful?

Thank you!

In case you are open to a pair programming session, lets discuss it! We can schedule a transcribed call and I'll use the transcript to write a docpage or an article about testing in plutus.

@effectfully
Copy link
Contributor

There is no Arbitrary there!
but if I go to testlib/PLA/Test/V1/Value.hs its there on Line 83.

I am really confused! Should I not user repl in this case?

Surprised about that as well, I'd expect GHCi to pick up the instance.

Let me implement it! But how do I know if it needs to be done and when I do it how do I know I am successful?

As for the first part of the question, plutus-ledger-api is not a big library and plutus-ledger-api/testlib is quite small, so if you want to check whether something's in there, you can just grep it. git grep Arbitrary plutus-ledger-api gives me 18 results, you need a minute to figure out from those results what types we already provide an Arbitrary instance for.

The second part of the question is harder though. If you want to make this as painless as possible, just add some very simple yet still helpful test requiring some straightforward Arbitrary instance and make a PR, we'll take it from there. You don't want to create a 2k lines PR and end up having 400+ comments on it (like it happened here for example).

@aleeusgr

This comment was marked as outdated.

@aleeusgr

This comment was marked as outdated.

@aleeusgr
Copy link

Please recommend tools I could use to generate a list of Types a package exports!
I am working on a issue in plutus-ledger-api; my goal is to write Arbitrary generators for the Types the package exports.

@coot says:

There's no such tool that I am aware of. You could write a ghc plugin for that (similar to ghc-tags-plugin).

Thanks coot, I will look into that.

@coot
Copy link
Contributor

coot commented Oct 28, 2024

https://github.com/coot/ghc-tags-plugin

@t4ccer
Copy link
Contributor

t4ccer commented Oct 29, 2024

Hey! Looks like I'll be moving the instances that we have in Plutarch upstream. Is that fine with you to put Arbitrary instances in modules that define given datatype or would you rather have them in testlib to avoid QuickCheck dependency on the main lib?

@effectfully
Copy link
Contributor

There are some in testlib now, so I think we prefer to keep it consistent and add new ones there as well.

@aleeusgr aleeusgr mentioned this issue Oct 31, 2024
11 tasks
@t4ccer t4ccer linked a pull request Nov 1, 2024 that will close this issue
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Low priority Doesn't require immediate attention status: triaged Test
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants