Skip to content
This repository has been archived by the owner on Dec 2, 2024. It is now read-only.

Fix mustSatisfyAnyOf logic (fix #101). #166

Merged
1 commit merged into from
Dec 8, 2021
Merged

Fix mustSatisfyAnyOf logic (fix #101). #166

1 commit merged into from
Dec 8, 2021

Conversation

ghost
Copy link

@ghost ghost commented Dec 3, 2021

should fix #101

Pre-submit checklist:

  • Branch
    • Tests are provided (if possible)
    • Commit sequence broadly makes sense
    • Key commits have useful messages
    • Relevant tickets are mentioned in commit messages
    • Formatting, materialized Nix files, PNG optimization, etc. are updated
  • PR
    • Self-reviewed the diff
    • Useful pull request description
    • Reviewer requested

@ghost ghost requested review from sjoerdvisscher, j-mueller and silky December 6, 2021 04:45
Copy link
Contributor

@sjoerdvisscher sjoerdvisscher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Would be nice to use any in the implementation of MustSatisfyAnyOf though 😄

@@ -107,7 +108,7 @@ checkTxConstraint ctx@ScriptContext{scriptContextTxInfo} = \case
$ V.findDatum dvh scriptContextTxInfo == Just dv
MustSatisfyAnyOf xs ->
traceIfFalse "Ld" -- "MustSatisfyAnyOf"
$ any (checkTxConstraint ctx) xs
$ or $ map (all (checkTxConstraint ctx)) xs
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's just any (all (checkTxConstraint ctx)) xs isn't it?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahah, yeah, missed that, thanks!

@ghost ghost force-pushed the fix-mustsatisfyanyof branch from d3bbde0 to 5891f08 Compare December 6, 2021 12:41
@silky
Copy link
Contributor

silky commented Dec 6, 2021

Seems fine @ak3n ; but I note that the PIR gets larger, and some of the golden outputs (amounts) change? Is that because the PIR got larger? Is that expected?

@ghost
Copy link
Author

ghost commented Dec 7, 2021

@silky I think they get changed each time in renderGuess.txt.

At least it was true for 63123ff, and for others commits in the history of that file.

@silky
Copy link
Contributor

silky commented Dec 7, 2021

@ak3n I'm happy then if you're happy :)

add contract test

Use any instead of or and map
@ghost ghost force-pushed the fix-mustsatisfyanyof branch from 5891f08 to 40b63ed Compare December 8, 2021 07:48
@ghost ghost merged commit 804a84b into main Dec 8, 2021
@ghost ghost deleted the fix-mustsatisfyanyof branch December 8, 2021 08:21
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

mustSatisfyAnyOf fails the expected logic
2 participants