[ADP-3226] Use even more IsRecentEra.

[jonathanknowles]

Issues

ADP-3226 (Follow-on from #4422)

Description

This PR adjusts even more functions to use an IsRecentEra era constraint instead of a RecentEra era parameter.

[HeinrichApfelmus]

I would like to see all applications @era that do not involve recentEra @era removed, and replaced by a type annotation on the result.

The type application for recentEra @era is shorthand for recentEra :: RecentEra era, which I find acceptable.

[HeinrichApfelmus]

Suggested change

	W.constructTransaction @n @era
	W.constructTransaction @n

The correct solution is to give a type annotation to the return value, here (unbalancedTx :: Cardano.TxBody (Write.CardanoApiEra era)).

In this case, annotating the type is necessary to avoid ambiguity — at some point, we have to specify the era in which the transaction is being created, specifically that this era is specific to the result obtained from the PParams.

See also the comment involving certificateFromDelegationAction.

[jonathanknowles]

@HeinrichApfelmus If I've interpreted your suggestion correctly, this would result in a patch similar to:

- unbalancedTx <-
-     liftHandler $
-     W.constructTransaction @n @era
+ unbalancedTx :: Cardano.TxBody (Write.CardanoApiEra era) <-
+     liftHandler $
+     W.constructTransaction @n
          db transactionCtx3
          PreSelection { outputs = outs <> mintingOuts }

The correct solution is to give a type annotation to the return value, here (unbalancedTx :: Cardano.TxBody (Write.CardanoApiEra era)).

My apologies, I don't follow your reasoning here.

Could you explain why you believe that this solution (of using a type annotation) is more "correct" than the current solution (of using a type application)?

Thanks!

[HeinrichApfelmus]

Could you explain why you believe that this solution (of using a type annotation) is more "correct" than the current solution (of using a type application)?

Two reasons:

• The current solution uses a language extension TypeApplications. No reasoning has been given on how this language extension improves our code (compared to plain Haskell2010) — the current solution has no strong basis. If you question a new solution, you have to question the status quo as well.
• A type annotation always adds documentation, making it more clear which value has which type — the only reason not to add a type annotation is brevity. As a side effect, this type annotation also disambiguates. In contrast, the type application only disambiguates, but provides no additional benefit in terms of documentation.

[jonathanknowles]

@HeinrichApfelmus thanks for your comment!

I'll try to reply to your points:

The current solution uses a language extension TypeApplications.
No reasoning has been given on how this language extension improves our code (compared to plain Haskell2010)

The proposed solution uses ScopedTypeVariables, which AFAIK is also not part of Haskell2010. We've used both of these extensions within our code base for several years.

Nevertheless, if no reasoning has been given for how TypeApplications improves our code, then on what basis can we advocate for recentEra @era over recentEra :: RecentEra era, as advocated for in an earlier comment:

The type application for recentEra @era is shorthand for recentEra :: RecentEra era, which I find acceptable.

I'm guessing there are some circumstances in which you find this extension acceptable, and other circumstances where you find it to be less acceptable?

Additionally, I'm still wondering about my original question, which is about correctness:

Could you explain why you believe that this solution (of using a type annotation) is more "correct" than the current solution (of using a type application)?

I asked the above question in response to the following claim (see original comment):

The correct solution is to give a type annotation to the return value, here (unbalancedTx :: Cardano.TxBody (Write.CardanoApiEra era)).

Apologies in advance if I've misunderstood, but I would interpret the use of a definite article here ("the correct solution") as implying that only the proposed solution is correct (i.e., that the original solution is incorrect in some way).

But I haven't seen any arguments (as yet) for why the original solution is incorrect.

I accept there is some debate to be had about the relative ability of type annotations versus type applications to provide documentation, but it seems to me that these are two points on a design spectrum that can be considered on a case-by-case basis. It's certainly not clear to me that one is strictly better than the other. (Or else we would presumably not prefer recentEra @era over recentEra :: RecentEra era.)

However... 😄

I'd like to make a proposal to you:

1. We move any discussion about the relative merits of TypeApplications versus ScopedTypeVariables to another forum. While I do agree with you that this is a worthwhile discussion, I suspect that moving it to a separate forum would be more productive (and perhaps enlightening). Perhaps it could be a future development meeting topic?

2. If we agree that this PR has already met its goals, then I would advocate that we merge this PR without further changes.

What do you think?

For the two remaining usages of @era (outside of recentEra), I would prefer to keep them as is, for the following reasons:

1. The current solutions are much more concise than the proposed alternatives. In particular, for the second example (the call to constructUnbalancedSharedTransaction), attempting to add a type annotation to the result leads to a very awkward multi-line tuple, which I find rather unreadable. Of course, there are several ways to ameliorate this, but I suspect these are all rather verbose in some way.

2. The functions in question both already use TypeApplications, and in particular the @n type application: adding an @era type application does not increase the idiomatic burden in any way.

[HeinrichApfelmus]

The proposed solution uses ScopedTypeVariables, which AFAIK is also not part of Haskell2010.

The type application @era uses both ScopedTypeVariables and TypeApplications. The claim that the proposed solution uses an orthogonal extension is not correct — instead, the proposed solution uses strictly less extensions.

We've used both of these extensions within our code base for several years.

Appealing to habit is not sufficient reasoning for the use of TypeApplications. Other people have smoked for several years.

Nevertheless, if no reasoning has been given for how TypeApplications improves our code, then on what basis can we advocate for recentEra @era over recentEra :: RecentEra era, as advocated for in an #4439 (review):

I personally prefer recentEra :: RecentEra era, but in this particular instance I would accept the shorthand recentEra @era. I am not advocating the use of the type application, but in this case, I would have no objection to it — as recentEra is a special case, it works as documentation of the result type.

But I haven't seen any arguments (as yet) for why the original solution is incorrect.

I have presented an argument based on the notion of "types as documentation" that favors the proposed solution over the original solution. To summarize my argument why the original solution is incorrect:

• It uses two extensions TypeApplications and ScopedTypeVariables that are outside of Haskell2010. Of those two, ScopedTypeVariables is more in line with the spirit of Haskell2010.
• The "spirit of Haskell2010" is that types are light-weight and compiler-checked documentation. This suggests the the "correct" fix is to add a type annotation to unsealedTx.

1. The current solutions are much more concise than the proposed alternatives.

… but it pays a price in terms of documentation. I believe that the type of unsealedTx should be documented in the code.

2. The functions in question both already use TypeApplications, and in particular the @n type application: adding an @era type application does not increase the idiomatic burden in any way.

But removing @era type applications does reduce the idiomatic burden. This pull request adds a type application @era where I put a value application era previously.

If we agree that this PR has already met its goals,

I'm afraid, I do believe that the merit of this PR depends on the merits of TypeApplications.

[Anviking]

Regardless of type- application vs annotation, my vote in this particular case (and similar ones) is to keep the value level RecentEra era like on master, as it makes the requirement to specify the era clear and transparent.

[jonathanknowles]

@HeinrichApfelmus

I am happy to discuss different points in the design space with you.

However, I do object to your labelling of my original solution as "incorrect". After reading your responses, it seems that this designation stems from your personal opinion rather than objective fact. While I really do empathise with your desire to find the best solution, I believe it's also important to be clear about what is objective and what is subjective.

Let's examine what you have written:

To summarize my argument why the original solution is incorrect:

• It uses two extensions TypeApplications and ScopedTypeVariables that are outside of Haskell2010. Of those two, ScopedTypeVariables is more in line with the spirit of Haskell2010.
• The "spirit of Haskell2010" is that types are light-weight and compiler-checked documentation.

Forgive me if I've misunderstood, but you seem to be implying that Haskell2010 (and the set of extensions it includes) represents some kind of ideal, one that people writing Haskell should pursue by default, and that justification should be given before using extensions not found in Haskell2010. Is that a fair appraisal of your position?

I am willing to grant you that some Haskellers do prefer to restrict their code to Haskell2010. (I have also encountered those that would prefer Haskell98, for various reasons, such as compatibility with compilers other than GHC.) However, it's also true that this opinion is not universally held within the Haskell community at large. Evidence:

• In the most recent State of Haskell Survey, the question "Which language extensions would you like to be enabled by default?", TypeApplications was the one of the most highly-voted-for extensions (very few people voted against).
• The GHC2021 specification (admittedly, a GHC specification rather than a Haskell language standard per se), includes this extension by default, along with the qualifying statement:

  GHC blesses a number of extensions, beyond Haskell2010, to be suitable to turned on by default. These extensions are considered to be stable and conservative.)
  Note the emphasis on conservative.

Of course, we could debate the merits of the above survey, and we could question whether the GHC development team made the right choice to include TypeApplications in their GHC2021 specification. However, this would not change the fact that "Haskell2010 should be preferred":

• is a subjective opinion.
• is not an objective fact.

(As a side point, while I do acknowledge your argument about documentation, I disagree somewhat with the idea that type applications do not have documentation benefits. But let's agree to disagree on that point.)

My primary concern lies with the approach this review seems to take, one that seems to characterise subjective opinion as objective truth, as exemplified by the assertion that my original solution is "incorrect." I believe this approach is unproductive, and even harmful.

I believe it's unproductive, because the non-threaded format of GitHub comments makes it rather impractical to keep track of the sort of long and nuanced discussion that a decision on the utility of TypeApplications would probably require. While we could have such a discussion, it was certainly not my goal in creating this PR.

As for the harmfulness of this approach: have you considered the difference between:

• "The correct solution is ... / the original solution is incorrect"
• "Thanks for making this PR! I see you have used solution X. Have you considered solution Y, for reasons Z?"

While the former approach may be suitable for objectively incorrect implementations, such as an implementation that violates a specification, I feel that in this context, where multiple design options exist with different trade-offs, the approach of labelling someone's solution as "incorrect" seems unnecessarily condescending, and actually hinders collaboration.

I feel that starting with the latter approach might lead to a more positive and fruitful review process, and possibly expedite the process of achieving consensus. What do you think?

[HeinrichApfelmus]

Forgive me if I've misunderstood, but you seem to be implying that Haskell2010 (and the set of extensions it includes) represents some kind of ideal, one that people writing Haskell should pursue by default, and that justification should be given before using extensions not found in Haskell2010. Is that a fair appraisal of your position?

In essence, yes, with the qualification that I use Haskell2010 as an umbrella term for the point in the design space that I would label "spirit of Haskell2010" and which I would summarize as: "The spirit of Haskell2010 is that types are light-weight and compiler-checked documentation for terms."

My primary concern lies with the approach this review seems to take, one that seems to characterise subjective opinion as objective truth, as exemplified by the assertion that my original solution is "incorrect." I believe this approach is unproductive, and even harmful.

My objection to TypeApplications is on the grounds that it deviates too much from the point in the design space I tried to summarize above. Importantly, this is an objective criterion. Perhaps "incorrect" is too strong a word, but the deviation from the "spirit of Haskell2010" as defined above is factual, not opinion — it's "incorrect" in the sense of being outside of the "spirit of Haskell2010". (It's tautologically true that TypeApplications is not part of Haskell2010, the question is whether it's still within the point of the design space that I like to call "the spirit of Haskell2010" or outside.)

I contrast, I find that your arguments in favor of TypeApplications err on the side of opinion. The result of a survey is pure opinion. The inclusion in a set of stable extensions in more objective, but only as far as the criterion of stability is concerned, not the effect of expressiveness on code.

The important questions for any extensions are:

• Which new forms of expression does the extension enable that significantly simplify code?
• Is this simplification worth the cognitive overhead?

On both counts, TypeApplications does not favor well. Did you consider these two questions before using the extension? What were your conclusions?

[HeinrichApfelmus]

Suggested change

	W.constructUnbalancedSharedTransaction @n @era
	W.constructUnbalancedSharedTransaction @n

[jonathanknowles]

@HeinrichApfelmus If I've interpreted your comment correctly, you're proposing to replace this:

(unbalancedTx, scriptLookup) <- liftHandler $
    W.constructUnbalancedSharedTransaction @n @era
    db txCtx PreSelection {outputs = outs}

with (something like):

(unbalancedTx :: Cardano.TxBody (Write.CardanoApiEra era)
    , scriptLookup
    ) <- liftHandler $
        W.constructUnbalancedSharedTransaction @n
        db txCtx PreSelection {outputs = outs}

resulting in a patch similar to:

-         (unbalancedTx, scriptLookup) <-
+         (unbalancedTx :: Cardano.TxBody (Write.CardanoApiEra era)
+             , scriptLookup) <-
                  liftHandler $
-                 W.constructUnbalancedSharedTransaction @n @era
+                 W.constructUnbalancedSharedTransaction @n
                      db txCtx PreSelection {outputs = outs}

Of course, there are probably nicer ways to format this (so that we don't need a multi-line tuple).

But I'm not sure what we gain by replacing the type application (on the function) with a type annotation (on the result) here. If anything, the result is certainly less concise.

[HeinrichApfelmus]

Same argument as for the previous comment.

The type annotation for unbalancedTx adds documentation — in this case, telling us in which era this unbalanced transaction is contructed (namely the one in scope). This information is important, as it also disambiguates the era — even the compiler complains about that.

In contrast, the type application does not add documentation, as one still has to inspect the type of constructUnbalancedSharedTransaction @n @era in order to compute the type of unbalancedTx.

[jonathanknowles]

In contrast, the type application does not add documentation, as one still has to inspect the type of constructUnbalancedSharedTransaction @n @era in order to compute the type of unbalancedTx.

Thanks.

Though I disagree with the notion that the @era type application adds no documentation.

When I read this code, I see the @era type application as documentation of two facts:

1. That the construction of a transaction is era-dependent.
2. That we're constructing an (unbalanced) transaction for the era type that is currently in scope.

[HeinrichApfelmus]

When I read this code, I see the @era type application as documentation of two facts:

1. That the construction of a transaction is era-dependent.
2. That we're constructing an (unbalanced) transaction for the era type that is currently in scope.

Both of these items depend on knowledge of the type of constructUnbalancedSharedTransaction. In principle, the implementation of this function could choose to ignore the type argument.

In contrast, the type signature on unbalancedTx provides compiler-checked documentation that the result of constructUnbalancedSharedTransaction indeed satisfies these two facts. The facts being documentation are the same, but the type signature is superior in that

• it pertains to the result of constructUnbalancedSharedTransaction, not to the arguments

The point of the type signature on unbalancedTx is that these two facts are both documented and compiler-checked, without the reader needing to consult the implementation or even the type of constructUnbalancedSharedTransaction.

[paolino]

I'm all for singletons and I'm not happy with the necessity of signing to disambiguate on the era. So I'm mostly out of the discussion 🤷