Don't take collateral if validation doesn't fail in phase 2

[MaximilianAlgehed]

Pre-submit checklist:

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

[MaximilianAlgehed]

@sjoerdvisscher here is a PR with that fix. I think we might need a bit of help debugging the PAB issue. Should we make an issue with this too?

[koslambrou]

I don't understand how this actually changed the result of PAB testing? Isn't this a code format change?

[koslambrou]

Oh, just saw your comment on the other PR: "because of how do and infix notation interact"

[koslambrou]

But I guess I don't see the difference...

[MaximilianAlgehed]

This is super subtle, the old code would do catchError on the whole do-block, not just the bits in the brackets (the phase 2 validation).

[sjoerdvisscher]

I have looked at this a bit. The problem seems to be that the test was not ok to begin with. The 3rd test does not fail if you comment out the second test. So there's some state transfer between tests. @koslambrou it seems you wrote these tests, does that sound plausible?

[koslambrou]

Found the error, but I don't know why your fix triggered it.

It's in the test case "PingPong contract state is maintained across PAB instances" in CliSpec.

Change

let newConfig = bumpConfig 50 "db1" pabConfig

to

 let newConfig = bumpConfig 50 "db1" defaultPabConfig

That solved it locally.

Edit:

@sjoerdvisscher Mentionned that a better solution would be let newConfig = bumpConfig 10 "db1" pabConfig so the second test runs with bumps of 50 and 60, and the third runs with bumps of 100 and 110?

P.S. I would create a commit with the fix, but I don't have permission access for your fork.

[UlfNorell]

@koslambrou if the test failure is unrelated to this fix, maybe it makes more sense to fix the test in a separate PR and hold this PR until that has landed? If you want it here you can always make a PR to this branch.

[koslambrou]

@MaximilianAlgehed The fix is on main. You can rebase.

[UlfNorell]

@koslambrou Rebased and tests are passing.