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.

Sign up for GitHub

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

Jump to bottom

Make Byron constructors and functions propagate `ByronEraOnly` eon. Delete `CardanoEraStyle` #335

Merged

newhoggy merged 9 commits into main from newhoggy/make-byron-constructor-and-functions-propagate-ByronEraOnly

Oct 28, 2023

Collaborator

newhoggy commented Oct 22, 2023 •

edited

Loading

Changelog

- description: |
    In `Tx`, `ByronTx` now carries additional `ByronEraOnly` witness
    In `TxBody`, `TxBodyByron` now carries additional `ByronEraOnly` witness
    Delete `CardanoEraStyle` because eons solve the same problem more generally
    Delete `cardanoEraStyle`
# uncomment types applicable to the change:
  type:
  # - feature        # introduces a new feature
  - breaking       # the API has changed in a breaking way
  # - compatible     # the API has changed but is non-breaking
  # - optimisation   # measurable performance improvements
  # - improvement    # QoL changes e.g. refactoring
  # - bugfix         # fixes a defect
  # - test           # fixes/modifies tests
  # - maintenance    # not directly related to the code
  # - release        # related to a new release preparation
  # - documentation  # change in code docs, haddocks...

Context

Modify Byron constructors and functions propagate ByronEraOnly eon because that makes the code more compatible with eon support.

Delete CardanoEraStyle because it is unnecessary as eons solve the same problem more generally.

How to trust this PR

Highlight important bits of the PR that will make the review faster. If there are commands the reviewer can run to observe the new behavior, describe them.

Checklist

Commit sequence broadly makes sense and commits have useful messages
New tests are added if needed and existing tests are updated. See Running tests for more details
Self-reviewed the diff

newhoggy commented

View reviewed changes

cardano-api/internal/Cardano/Api/Tx.hs

@@ @@ -101,8 +102,9 @@ import Lens.Micro @@
               data Tx era where
                    ByronTx
-                     :: Byron.ATxAux ByteString
-                     -> Tx ByronEra

Collaborator Author

newhoggy Oct 22, 2023

Everywhere, instead of using ByronEra directly, we leave it parameterised and add a eon witness to prove it is Byron. This makes the code compatible with eon support.

newhoggy commented

View reviewed changes

cardano-api/internal/Cardano/Api/TxBody.hs

                                       <*> return (TxOutDatumHash w dHash)
                                       <*> return ReferenceScriptNone
-              fromByronTxOut :: Byron.TxOut -> TxOut ctx ByronEra
-              fromByronTxOut (Byron.TxOut addr value) =
+              fromByronTxOut :: ByronEraOnly era -> Byron.TxOut -> TxOut ctx era

Collaborator Author

newhoggy Oct 22, 2023

Everywhere, instead of using ByronEra directly, we leave it parameterised and add a eon witness to prove it is Byron. This makes the code compatible with eon support.

newhoggy commented

View reviewed changes

cardano-api/internal/Cardano/Api/TxBody.hs

-                     -> TxBody ByronEra
+                     :: ByronEraOnly era
+                     -> Annotated Byron.Tx ByteString
+                     -> TxBody era

Collaborator Author

newhoggy Oct 22, 2023

Everywhere, instead of using ByronEra directly, we leave it parameterised and add a eon witness to prove it is Byron. This makes the code compatible with eon support.

newhoggy commented

View reviewed changes

cardano-api/internal/Cardano/Api/TxBody.hs

+              getByronTxBodyContent :: ()
+                => ByronEraOnly era
+                -> Annotated Byron.Tx ByteString
+                -> TxBodyContent ViewTx era

Collaborator Author

newhoggy Oct 22, 2023

Everywhere, instead of using ByronEra directly, we leave it parameterised and add a eon witness to prove it is Byron. This makes the code compatible with eon support.

newhoggy commented

View reviewed changes

cardano-api/internal/Cardano/Api/TxBody.hs

-                                 (TxOutAdaOnly ByronToAllegraEraByron value) _ _)
-                    | value < 0 = TxBodyOutputNegative (lovelaceToQuantity value) (txOutInAnyEra ByronEra txout)
-                    | otherwise = TxBodyOutputOverflow (lovelaceToQuantity value) (txOutInAnyEra ByronEra txout)
+              classifyRangeError :: ByronEraOnly era -> TxOut CtxTx era -> TxBodyError

Collaborator Author

newhoggy Oct 22, 2023

Everywhere, instead of using ByronEra directly, we leave it parameterised and add a eon witness to prove it is Byron. This makes the code compatible with eon support.

newhoggy commented

View reviewed changes

cardano-api/internal/Cardano/Api/TxBody.hs

+              makeByronTransactionBody :: ()
+                => ByronEraOnly era
+                -> TxBodyContent BuildTx era
+                -> Either TxBodyError (TxBody era)

Collaborator Author

newhoggy Oct 22, 2023

Everywhere, instead of using ByronEra directly, we leave it parameterised and add a eon witness to prove it is Byron. This makes the code compatible with eon support.

newhoggy commented

View reviewed changes

cardano-api/internal/Cardano/Api/TxBody.hs

                 TxOut
                   (AddressInEra ByronAddressInAnyEra (ByronAddress addr))
                   (TxOutAdaOnly ByronToAllegraEraByron (fromByronLovelace value))
                    TxOutDatumNone ReferenceScriptNone
-              toByronTxOut :: TxOut ctx ByronEra -> Maybe Byron.TxOut
-              toByronTxOut (TxOut (AddressInEra ByronAddressInAnyEra (ByronAddress addr))
+              toByronTxOut :: ByronEraOnly era -> TxOut ctx era -> Maybe Byron.TxOut

Collaborator Author

newhoggy Oct 22, 2023

Everywhere, instead of using ByronEra directly, we leave it parameterised and add a eon witness to prove it is Byron. This makes the code compatible with eon support.

newhoggy changed the title ~~Make Byron constructors and functions propagate ByronEraOnly eon~~ Make Byron constructors and functions propagate ByronEraOnly eon. Delete CardanoEraStyle

newhoggy marked this pull request as ready for review

October 22, 2023 05:22

newhoggy requested review from dcoutts, Jimbo4350, erikd, carbolymer, CarlosLopezDeLara and disassembler as code owners

October 22, 2023 05:22

Jimbo4350 approved these changes

View reviewed changes

carbolymer approved these changes

View reviewed changes

newhoggy added 9 commits

October 28, 2023 18:24


          Make Byron constructors and functions propagate ByronEraOnly eon

35014c2


          Simplify deserialiseFromCBOR for Tx era

37c6cd7


          Simplify deserialiseTx

14bb6d9


          Simplify requireShelleyBasedEra with eons

f84b8e8


          Simplify createAndValidateTransactionBody

9c8ffe3


          Simplify deserialiseFromTextEnvelopeCddlAnyOf

c8ed7de


          Simplify anyAddressInEra

93d71ad


          Remove remaining uses of CardanoEraStyle

db42dc1


          Delete CardanoEraStyle and cardanoEraStyle

3edfb23

newhoggy force-pushed the newhoggy/make-byron-constructor-and-functions-propagate-ByronEraOnly branch from 94f6b4f to 3edfb23 Compare

October 28, 2023 07:25

newhoggy added this pull request to the merge queue

Merged via the queue into main with commit dd6369b

20 checks passed

newhoggy deleted the newhoggy/make-byron-constructor-and-functions-propagate-ByronEraOnly branch

October 28, 2023 08:54

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Jimbo4350 Jimbo4350 approved these changes

carbolymer carbolymer approved these changes

dcoutts Awaiting requested review from dcoutts

erikd Awaiting requested review from erikd

CarlosLopezDeLara Awaiting requested review from CarlosLopezDeLara

disassembler Awaiting requested review from disassembler

Labels

None yet