-
Notifications
You must be signed in to change notification settings - Fork 23
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 function collectPlutusScriptHashes
to collect script hashes needed to validate a given transaction
#735
Conversation
-- and return them in a map with their corresponding 'ScriptWitnessIndex' as key. | ||
collectScriptHashes | ||
:: AlonzoEraOnwards era | ||
-> TxBody era |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function is more related to TxBody
rather than to this module, so it should go to Cardano.Api.Tx.Body
I think.
(Cardano.Api.Tx.Body
should get chopped down into smaller modules, but that's a separate discussion...)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried but it generated a cyclic dependency. It would probably be solvable by creating some Type
modules and moving stuff around, but it was not trivial at all
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok lets leave it for now. 👍🏻
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would parameterize this on a the api's Tx era
. TxBody era
will be removed in the future because it is a poor abstraction. It (unsuccessfully) tries to find a middle ground between a full transaction and a tx body as defined in the ledger.
31cd3fe
to
99577e1
Compare
99577e1
to
3767bb3
Compare
3767bb3
to
2d9f907
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Before this is merged and exposed I want to have a crack at where this is called in cardano-cli
.
collectScriptHashes
to collect script hashes needed to validate a given transactioncollectPlutusScriptHashes
to collect script hashes needed to validate a given transaction
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM but one comment needs to be addressed.
-- and return them in a map with their corresponding 'ScriptWitnessIndex' as key. | ||
collectScriptHashes | ||
:: AlonzoEraOnwards era | ||
-> TxBody era |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would parameterize this on a the api's Tx era
. TxBody era
will be removed in the future because it is a poor abstraction. It (unsuccessfully) tries to find a middle ground between a full transaction and a tx body as defined in the ledger.
… validate a given transaction
Co-authored-by: Clément Hurlin <[email protected]>
fe1d56e
to
6e17447
Compare
Addressed here: 6e17447 |
Changelog
Context
This is required functionality by this
cardano-cli
PR: IntersectMBO/cardano-cli#1031How to trust this PR
Ensure the API is in the correct place and has the right structure, that it does the right thing, that the documentation is adequate. Also check in conjunction with the related PRs:
cardano-node
: cardano-testnet: Test plutus script cost calculation command cardano-node#6097cardano-cli
: Add command to calculate plutus script costs from an already constructed transaction cardano-cli#1031Checklist