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

ContractModel: Extend the Escrow contract model #296

Merged

Conversation

rjmh
Copy link
Contributor

@rjmh rjmh commented Feb 3, 2022

Extend Escrow tests, add negative testing
Add NoLockedFunds to Escrow tests

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

@rjmh
Copy link
Contributor Author

rjmh commented Feb 3, 2022

Do you mind having a quick look at this? @sjoerdvisscher

&& Nothing /= (s ^. contractState . contributions . at w)
Pay w v -> Nothing == s ^. contractState . contributions . at w
Pay w v -> (True || Nothing == s ^. contractState . contributions . at w) -- disabled
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this disabled?

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's disabled so that we can also test paying the amount due in several installments. The contract allows that, so there's no reason not to test it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(should have removed the condition altogether, of course).

Extend Escrow tests, add negative testing
Add NoLockedFunds to Escrow tests
@MaximilianAlgehed MaximilianAlgehed force-pushed the PR-escrow-contract-model-improvements branch from 96ef2b8 to 284c6f2 Compare February 11, 2022 11:05
@MaximilianAlgehed
Copy link
Contributor

@sjoerdvisscher I think this is ready to merge now :)

@sjoerdvisscher sjoerdvisscher merged commit 975f0ab into IntersectMBO:main Feb 14, 2022
@MaximilianAlgehed MaximilianAlgehed deleted the PR-escrow-contract-model-improvements branch February 16, 2022 14:32
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