IntersectMBO / cardano-node-emulator

Apache License 2.0
3 stars 4 forks source link

Additional utils around values and addition of a missing export in mock wallets #32

Closed mmontin closed 1 month ago

mmontin commented 1 month ago

This PR extends two aspects of cardano-node-emulator:

These two elements could have been split into 2 PRs, as they are technically disjoint. I can perform this change if needed. Since those 2 were pretty small, I figured one PR for both could be manageable.

All these utilities were previously defined in cooked-validators are are used extensively within the tool and projects using this tool.

An open question about this PR is that is add optics-core as a dependency while some other packages from this repo use lens. I don't see any issue in that but I would understand if others do. Feel free to give me any feedback.

mmontin commented 1 month ago

Hm it seems the CI failed on one quickcheck generated test, which I cannot reproduce on my machine. I've ran them a few times, and all 40 succeeded every time. Since this PR is only adding a few helpers, I doubt it has anything to do with the failure. This is likely that somehow quickcheck spawned a test that uncover a bug in the Escrow contract.

smelc commented 1 month ago

@mmontin> indeed restarting the failed checks made them pass, so I think this is not an issue introduced by this PR :+1:

Personally, I tend to keep dependencies as small as reasonably possible, but it's fine adding optics-core if you deem it necessary. So I'm approving and merging.

mmontin commented 1 month ago

Thanks for merging so quickly @smelc ! I believe there are 2 actions to take/consider later on based on this PR: