Avoid branching on translateUpperBoundForPlutusScripts protocol version

[aniketd]

Description

Closes #5126

Checklist

• Commits in meaningful sequence and with useful messages.
• Tests added or updated when needed.
• CHANGELOG.md files updated for packages with externally visible changes.
  NOTE: New section is never added with the code changes. (See RELEASING.md).
• Versions updated in .cabal and CHANGELOG.md files when necessary, according to the
  versioning process.
• Version bounds in .cabal files updated when necessary.
  NOTE: If bounds change in a cabal file, that package itself must have a version increase. (See RELEASING.md).
• Code formatted (use scripts/fourmolize.sh).
• Cabal files formatted (use scripts/cabal-format.sh).
• CDDL files are up to date (use scripts/gen-cddl.sh)
• hie.yaml updated (use scripts/gen-hie.sh).
• Self-reviewed the diff.

[teodanciu]

Looks good to me, some minor suggestions and what Joosp said about not invoking the test with @ALONZOERA instead of @era (though the test still passes and i believe it's technically correct, albeit confusing!)

Wait, I can't explain the failure of the golden translation tests! So I'm dismissing my review until I understand - it doesn't seem to me like they should be failing!

[lehins]

There is a golden test failure on CI, which @teodanciu can give you a suggestion on how resolve.
Other than that the PR is good.

[teodanciu]

There is a golden test failure on CI, which @teodanciu can give you a suggestion on how resolve.

@aniketd , I think there is is a mistake in the way the golden files were generated!
For reasons that I cannot remember (but likely as simple as: my stupidity), the TranslationInstances that get serialized in the golden files, use an arbitrary protocol version, here: https://github.com/IntersectMBO/cardano-ledger/blob/master/eras/alonzo/impl/testlib/Test/Cardano/Ledger/Alonzo/Translation/TranslatableGen.hs#L77.

So I believe what happened is:

• because of this arbitrary protocol version, the Boolean flag in the translated Interval was set in the golden files to True or False, equally arbitrarily!
• because - until your change - the same protocol version was used to check the golden files, the check would always pass.
• now, with your change, the switching is based on the era rather than on this protocol version, so the checks are failing.

That's my theory at least - i got curious because I don't remember seeing these tests fail before.
Hope this helps - if what I said doesn't makes sense or you would like to talk about it, let me know please!

PS: If my theory is correct, and the golden files are, indeed, wrong - i think it would be good to fix them in a different PR, so that it's clear that your change doesn't alter them - so basically to not regenerate the files AND make changes that could potentially change the files in the same PR.

[aniketd]

@teodanciu Thanks a lot for your investigation into this matter. It helped a great deal in confirming my solution. Please take a second look at this one, now that I have addressed the review comments 😁 and let me know what you think 👍

[aniketd]

@aniketd I need to look into the conformance tests and also split the commit into one that changes the code and another that updates the golden files.

[teodanciu]

Hey @aniketd , FIY: I had a similar failure on my PR, and the good news is that @neilmayhew's PR has solved mine, so perhaps it will help you too!

[lehins]

Looks good. Both @teodanciu and @Soupstraw rightfully so pointed out inferior setup for the test that check this functionality. Albeit enough to convince that changes in this PR are sound, but they can be adjusted to make their quality much better.
I've created #5249, which @aniketd will tackle as a subsequent PR