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

Add getTransactionBodyContent #2663

Merged
merged 1 commit into from
May 6, 2021
Merged

Conversation

cblp
Copy link
Contributor

@cblp cblp commented Apr 27, 2021

No description provided.

@cblp cblp force-pushed the cblp/getTransactionBodyContent branch 2 times, most recently from 2aa6fcb to 760fd2e Compare April 29, 2021 15:45
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.

Nice work, some comments below.

cardano-api/src/Cardano/Api/TxBody.hs Outdated Show resolved Hide resolved
cardano-api/src/Cardano/Api/TxBody.hs Outdated Show resolved Hide resolved
cardano-api/src/Cardano/Api/TxBody.hs Outdated Show resolved Hide resolved
cardano-api/src/Cardano/Api/TxBody.hs Outdated Show resolved Hide resolved
cardano-api/src/Cardano/Api/TxBody.hs Outdated Show resolved Hide resolved
cardano-api/src/Cardano/Api/TxBody.hs Outdated Show resolved Hide resolved
cardano-api/src/Cardano/Api/TxBody.hs Outdated Show resolved Hide resolved
cardano-api/src/Cardano/Api/TxBody.hs Outdated Show resolved Hide resolved
cardano-api/src/Cardano/Api/TxBody.hs Show resolved Hide resolved
@cblp cblp force-pushed the cblp/getTransactionBodyContent branch from 2f46d62 to b841473 Compare May 5, 2021 17:02
@Jimbo4350
Copy link
Contributor

Jimbo4350 commented May 6, 2021

@cblp So following the style of cardano-api this is what we are aiming for. You're heading in the right direction but we can further refactor to the following:

getTxBodyContent :: TxBody era -> TxBodyContent ViewTx era
getTxBodyContent (ByronTxBody annot) = getByronTxBodyContent annot
getTxBodyContent (ShelleyTxBody sbe txBody scripts mAuxData) =
  fromLedgerTxBody sbe txBody

fromLedgerTxBody
  :: ShelleyBasedEra era
  -> Ledger.TxBody (ShelleyLedgerEra era)
  -> TxBodyContent ViewTx era
fromLedgerTxBody sbe txBod =
  TxBodyContent
    { txIns = fromLedgerTxIns sbe txBod
    , txOuts = error ""
    , txFee = error ""
    , txValidityRange = error ""
    , txMetadata = error ""
    , txAuxScripts = error ""
    , txWithdrawals = error ""
    , txCertificates = error ""
    , txUpdateProposal = error ""
    , txMintValue = error ""
    }

fromLedgerTxIns
  :: ShelleyBasedEra era
  -> Ledger.TxBody (ShelleyLedgerEra era)
  -> [(TxIn,BuildTxWith ViewTx (Witness WitCtxTxIn era))]
fromLedgerTxIns sbe ledTxBody =
  case sbe of
    ShelleyBasedEraShelley ->
      [ (fromShelleyTxIn input, ViewTx)
      | input <- toList $ Shelley._inputs ledTxBody
      ]
    ShelleyBasedEraAllegra ->
      [ (fromShelleyTxIn input, ViewTx)
      | input <- toList $ Allegra.inputs' ledTxBody
      ]
    ShelleyBasedEraMary ->
      [ (fromShelleyTxIn input, ViewTx)
      | input <- toList $ Mary.inputs' ledTxBody
      ]

We can also remove the error checking as we already do that when constructing a TxBody era i.e :

makeShelleyTransactionBody :: ShelleyBasedEra era
                           -> TxBodyContent BuildTx era
                           -> Either (TxBodyError era) (TxBody era)                       

The type signature of fromLedgerTxBody will need to be updated with the other things e.g Maybe (Ledger.AuxiliaryData (ShelleyLedgerEra era)) but this is the general format.

@cblp
Copy link
Contributor Author

cblp commented May 6, 2021

restored txMetadata code

@cblp
Copy link
Contributor Author

cblp commented May 6, 2021

@Jimbo4350 transposed code by field-then-era

@cblp cblp requested a review from Jimbo4350 May 6, 2021 14:29
@cblp cblp force-pushed the cblp/getTransactionBodyContent branch from 37bb275 to 72f28e1 Compare May 6, 2021 16:41
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.

Nice work! LGTM

@cblp
Copy link
Contributor Author

cblp commented May 6, 2021

bors merge

@iohk-bors
Copy link
Contributor

iohk-bors bot commented May 6, 2021

@iohk-bors iohk-bors bot merged commit 4d52323 into master May 6, 2021
@iohk-bors iohk-bors bot deleted the cblp/getTransactionBodyContent branch May 6, 2021 20:42
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.

2 participants