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

Filling in more Alonzo API extensions for the Tx and TxBody #2738

Merged
merged 8 commits into from
May 28, 2021

Conversation

dcoutts
Copy link
Contributor

@dcoutts dcoutts commented May 24, 2021

No description provided.

@dcoutts dcoutts marked this pull request as draft May 24, 2021 13:14
@dcoutts dcoutts added hydra-dont-build WIP Work In Progress (cannot be merged yet) labels May 24, 2021
@dcoutts
Copy link
Contributor Author

dcoutts commented May 24, 2021

For the moment this extends #2642. It will need to be rebased once that is merged.

@Jimbo4350 Jimbo4350 force-pushed the dcoutts/alonzo-api-tx-body branch 2 times, most recently from b07ba16 to 6043b00 Compare May 24, 2021 14:38
@dcoutts dcoutts force-pushed the dcoutts/alonzo-api-tx-body branch 2 times, most recently from 4d9a3ec to 6043b00 Compare May 24, 2021 18:04
dcoutts and others added 7 commits May 27, 2021 01:18
This is a further step along the path to Alonzo support in the API.

The TxBody's ShelleyTxBody constructor holds all the info that goes with
the tx body. This includes scripts and aux data. For Alonzo it now also
has to include the script data, which the ledger calls the redeemers.
This is a big mapping of each use of a script to the script redeemer
data and execution units.

This patch adds it to the TxBody but does not yet do the main
conversion step of constructing the redeemer pointer mapping.
Based on the script witnesses used within the tx body.

Co-authored-by: Jordan Millar <jordan.millar@iohk.io>
The getTransactionBodyContent function and the functions it was calling
were doing unnecessary checks that made them partial. We do not need to
validate when converting from a ledger transaction, only when building
one. By eliminating unnecessary checks and making this pure we can now
introduce a TxBody pattern that lets one get at the TxBodyContent. This
makes for a nicer API.

Also eliminate a couple deprecated functions that will be awkward to
update for new features, and are no longer used.
It will be needed for the Alonzo tx construction.

Co-authored-by: Duncan Coutts <duncan@well-typed.com>
Needed to construct the hash of the script-relevant protocol params and
the redeemers.

It is only supplied when building the transaction and is not available
when viewing it.
For now always fill it in with Nothing. This is a rarely used feature
that we can complete at the end.

It is a bit annoying however: it needs a Shelley network identifier,
which is not the same as the API NetworkId type, since that also
contains the NetworkMagic. That means we cannot use that type since we
would not be able to round-trip it to/from a ledger transaction.

So this will need some extension and/or refactoring of the NetworkId
type and utils.

Co-authored-by: Duncan Coutts <duncan@well-typed.com>
These specify that extra key witnesses are required, and this fact is
visible to Plutus scripts. It thereby provides a mechaism for Plutus
scripts to check that the transaction has been signed by a particular
key.

Co-authored-by: Duncan Coutts <duncan@well-typed.com>
@dcoutts dcoutts force-pushed the dcoutts/alonzo-api-tx-body branch from 6043b00 to ecfaa63 Compare May 27, 2021 01:35
@dcoutts dcoutts removed WIP Work In Progress (cannot be merged yet) hydra-dont-build labels May 27, 2021
@dcoutts dcoutts marked this pull request as ready for review May 27, 2021 01:40
@newhoggy newhoggy force-pushed the dcoutts/alonzo-api-tx-body branch 2 times, most recently from 909e496 to 06906ac Compare May 27, 2021 06:35
@@ -1173,6 +1186,7 @@ deserialiseShelleyBasedTxBody mkTxBody bs =
(CBOR.runAnnotator txbody fbs)
(map (`CBOR.runAnnotator` fbs) txscripts)
(CBOR.runAnnotator <$> txmetadata <*> pure fbs)
(error "deserialiseShelleyBasedTxBody: Redeemer pointer map")
Copy link
Contributor

Choose a reason for hiding this comment

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

Yep, I wasn't sure how we wanted to handle serialisation.

Copy link
Contributor

Choose a reason for hiding this comment

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

This bit is causing test failures currently.

Copy link
Contributor

Choose a reason for hiding this comment

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

    prop_roundtrip_txbody_shelley_CBOR:                        FAIL
        ✗ prop_roundtrip_txbody_shelley_CBOR failed at test/Test/Cardano/Api/Typed/CBOR.hs:156:3
          after 1 test.
        
              ┏━━ test/Test/Cardano/Api/Typed/CBOR.hs ━━━
          152 ┃ roundtrip_CBOR
          153 ┃   :: (SerialiseAsCBOR a, Eq a, Show a)
          154 ┃   => AsType a -> Gen a -> Property
          155 ┃ roundtrip_CBOR typeProxy gen =
          156 ┃   H.property $ do
          157 ┃     val <- H.forAll gen
          158 ┃     H.tripping val serialiseToCBOR (deserialiseFromCBOR typeProxy)
              ┃     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
              ┃     │ ━━━ Exception (ErrorCall) ━━━
              ┃     │ deserialiseShelleyBasedTxBody: Redeemer pointer map
              ┃     │ CallStack (from HasCallStack):
              ┃     │   error, called at src/Cardano/Api/TxBody.hs:1219:12 in cardano-api-1.27.0-inplace:Cardano.Api.TxBody
        
          This failure can be reproduced by running:
          > recheck (Size 0) (Seed 16246837987535537937 6040485580684346801) prop_roundtrip_txbody_shelley_CBOR
        
      Use '--hedgehog-replay "Size 0 Seed 16246837987535537937 6040485580684346801"' to reproduce.
    prop_roundtrip_txbody_allegra_CBOR:                        FAIL
        ✗ prop_roundtrip_txbody_allegra_CBOR failed at test/Test/Cardano/Api/Typed/CBOR.hs:156:3
          after 1 test.
        
              ┏━━ test/Test/Cardano/Api/Typed/CBOR.hs ━━━
          152 ┃ roundtrip_CBOR
          153 ┃   :: (SerialiseAsCBOR a, Eq a, Show a)
          154 ┃   => AsType a -> Gen a -> Property
          155 ┃ roundtrip_CBOR typeProxy gen =
          156 ┃   H.property $ do
          157 ┃     val <- H.forAll gen
          158 ┃     H.tripping val serialiseToCBOR (deserialiseFromCBOR typeProxy)
              ┃     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
              ┃     │ ━━━ Exception (ErrorCall) ━━━
              ┃     │ deserialiseShelleyBasedTxBody: Redeemer pointer map
              ┃     │ CallStack (from HasCallStack):
              ┃     │   error, called at src/Cardano/Api/TxBody.hs:1219:12 in cardano-api-1.27.0-inplace:Cardano.Api.TxBody
        
          This failure can be reproduced by running:
          > recheck (Size 0) (Seed 5526808619565871689 17014309501333539577) prop_roundtrip_txbody_allegra_CBOR
        
      Use '--hedgehog-replay "Size 0 Seed 5526808619565871689 17014309501333539577"' to reproduce.
    prop_roundtrip_txbody_mary_CBOR:                           FAIL
        ✗ prop_roundtrip_txbody_mary_CBOR failed at test/Test/Cardano/Api/Typed/CBOR.hs:156:3
          after 1 test.
        
              ┏━━ test/Test/Cardano/Api/Typed/CBOR.hs ━━━
          152 ┃ roundtrip_CBOR
          153 ┃   :: (SerialiseAsCBOR a, Eq a, Show a)
          154 ┃   => AsType a -> Gen a -> Property
          155 ┃ roundtrip_CBOR typeProxy gen =
          156 ┃   H.property $ do
          157 ┃     val <- H.forAll gen
          158 ┃     H.tripping val serialiseToCBOR (deserialiseFromCBOR typeProxy)
              ┃     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
              ┃     │ ━━━ Exception (ErrorCall) ━━━
              ┃     │ deserialiseShelleyBasedTxBody: Redeemer pointer map
              ┃     │ CallStack (from HasCallStack):
              ┃     │   error, called at src/Cardano/Api/TxBody.hs:1219:12 in cardano-api-1.27.0-inplace:Cardano.Api.TxBody
        
          This failure can be reproduced by running:
          > recheck (Size 0) (Seed 2450485195492175484 8218421726347182459) prop_roundtrip_txbody_mary_CBOR
        
      Use '--hedgehog-replay "Size 0 Seed 2450485195492175484 8218421726347182459"' to reproduce.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'll push a fix

Copy link
Contributor

@Jimbo4350 Jimbo4350 left a comment

Choose a reason for hiding this comment

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

LGTM!




-- TODO: Better name?
Copy link
Contributor

Choose a reason for hiding this comment

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

Leftover TODO here.

--correct thing in the final tx. Also wrap in an Either
mintingRedeemerPtr
:: TxMintValue BuildTx era
-- BuildTxWith build (Map PolicyId (Witness WitCtxMint era))
Copy link
Contributor

Choose a reason for hiding this comment

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

Leftover comment (from me 😄 )

@dcoutts dcoutts force-pushed the dcoutts/alonzo-api-tx-body branch 2 times, most recently from 69f8eb6 to 72a0567 Compare May 27, 2021 14:44
@dcoutts dcoutts force-pushed the dcoutts/alonzo-api-tx-body branch from 72a0567 to 6544ca8 Compare May 27, 2021 15:18
Copy link
Contributor

@Jimbo4350 Jimbo4350 left a comment

Choose a reason for hiding this comment

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

LGTM!

@dcoutts
Copy link
Contributor Author

dcoutts commented May 28, 2021

bors merge

@iohk-bors
Copy link
Contributor

iohk-bors bot commented May 28, 2021

@iohk-bors iohk-bors bot merged commit 5430355 into master May 28, 2021
@iohk-bors iohk-bors bot deleted the dcoutts/alonzo-api-tx-body branch May 28, 2021 09:07
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