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

Generators necessary for creating transactions and scenarios. #22

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

Riley-Kilgore
Copy link
Member

@Riley-Kilgore Riley-Kilgore commented Oct 16, 2024

Some of the work in this PR is upstreamed from https://github.com/CardanoSolutions/zhuli
Changes to the upstreamed work include a small fix made to any_value_extending, and else branches are now supported in scenario.ak.

Additionally a number of new functionalities are introduced such as generating constrained values, inputs, outputs, and a rudimentary balancing function for transactions.

Example usage of the new tx generator functions can be found here: https://gist.github.com/Riley-Kilgore/47a938e5cba59f0dfaa1e61a06d14e97
Example usage of scenarios can be found here: https://github.com/CardanoSolutions/zhuli/blob/main/validators/zhuli.test.ak

@waalge
Copy link
Collaborator

waalge commented Nov 1, 2024

Hey @Riley-Kilgore

Apologies for leaving this hanging.

I defer to @KtorZ for his judgement on these things. He's got some blame on these commits and I'm guessing there's been some discussion on design and design choices that are out of (my) band.

There's quite a lot here. I'd very much to see these features added. I have questions.

From a browse all the 'gen' methods in modules named gen_* are called any_* while the only module with methods gen_ are in fuzz/scenario.ak.
I think there's room for both these terms, but any suggests, well, 'any'.
Some of these generators are not really close to any.

test prop_any_ada_only_value(v via any_ada_only_value()) {
 let n = assets.lovelace_of(v)
 label(
   if n <= 0 {
     @"[-255; 0]"
   } else if n < 256 {
     @"]0; 255]"
   } else {
     @"n > 255"
   },
 )
 True
}

Results in

    ┍━ cardano/gen_assets ━━━━━━━━━━━━━━━━━━━━━━━━━━
    │ PASS [after 100 tests] prop_any_ada_only_value
    │ · with coverage
    │ | ]0; 255] 100.0%
    ┕ with --seed=4048650851 → 1 tests | 1 passed | 0 failed

      Summary 100 checks, 0 errors, 0 warnings

Quite unrepresentative of ada values in utxos in the wild.

Other methods such as any_assets are capped at at most 2 policy ids.
I might think my dapp works great until some user with an NFT saturated utxo gets their assets stranded.

any_constrained_ is a bit of an oxymoron - not to say its not fine, it just causes a double take.
Is there a way we can expose the internals in more systematic way?
Its a little opaque what the output distribution of the value will be for a given list (without reading the source code).
If for some reason i have, say, 5 possible asset classes, but I want precisely two of them will be present in a test,
this is a constraint not accommodated.
Pathological you may say, but its very dapp specific the things I think need to be tested.

Digression: This is a thread @KtorZ and I were discussing many moons ago.
https://github.com/waalge/aiken-fuzz/blob/main/lib/fuzz/transaction/credential.ak
My hunch is when i'm testing a dapp I care about very specific things.
Give me 'default' distributions of all the aspects that i'm pretty confident don't have any bearing.
Or at least I have no prior as to where i 'ought' to be looking for problems.
But give me fine-grained control in regions I'm pretty concerned about.

Address looks like a genuine any.
Any thoughts on a aliases gens for hashes eg fn any_script_hash() { bytearray(28,28) }, likewise for any_verification_key_hash ? i don't think / know that there is a performance penalty
And Pointers are dead, right??

Not familiar enough with certs to digest that module. Looks fine.

@Riley-Kilgore
Copy link
Member Author

Hey @waalge , thanks for the feedback. I'll try to address some of the concerns brought up in a commit I am now working on.

I have also started working on a bench-marking tool, which will initially use the existing property based tests that users define, but will also accommodate to SizedFuzzer<a> = fn(Int, PRNG) -> Option<(PRNG, a)>. I am not sure if we want to include functions helpful to users for writing these new fuzzers in this library or separately given that they are primarily intended to be used for bench-marking.

@waalge
Copy link
Collaborator

waalge commented Nov 1, 2024

@Riley-Kilgore,

Sounds good.
I'm not sure where we got to on drawing the boundary of this repo wrt others.
There was a desire to have fuzzers that were defined here upstreamed to the stdlib and prevent circular dependencies.

In these constrained environments, I see benchmarking as relevant as fuzzing.
No good having "provably terminates" if, in practice, it has to exceed some hard limit of the ledger to do so.
It should certainly go somewhere.

I don't really have an intuition as to how benchmarking is / will work.
I defer to the acting product owner @KtorZ as to what should go where.


Update: 'Consensus' is: circular dependencies allowed at a lib level (... I'm guessing not at module level? Not so important)

https://github.com/aiken-lang/stdlib/blob/main/lib/aiken/cbor.test.ak

Fuzz depends on stdlib, and any tests in stdlib depend on fuzz.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants