-
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
Tidy up "New calculateMinTxFee function" #490
Conversation
cc: @dnadales |
@@ -197,16 +197,16 @@ estimateTransactionFee sbe nw txFeeFixed txFeePerByte = \case | |||
-- | |||
evaluateTransactionFee :: forall era. () | |||
=> ShelleyBasedEra era | |||
-> UTxO 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.
Is this only the UTXO associated with the inputs in the parameter TxBody era
?
We should probably add a comment specifying this.
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.
Comment updated.
1a1473c
to
d575460
Compare
d575460
to
e0247a8
Compare
evaluateTransactionFee
using ledger's calcMinFeeTx
-- transaction in order to lookup any txins included in the transaction. | ||
-- | ||
-- The only type of witnesses that it cannot figure out reliably is the | ||
-- witnesses needed for satisfying native scripts included in the 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.
"Satisfying" or "validating"?
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.
👍
-> Ledger.PParams (ShelleyLedgerEra era) | ||
-> UTxO era | ||
-> TxBody era | ||
-> Word -- ^ The number of Shelley key witnesses |
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 can be made more clear with something like the following: "The number of Shelley key witnesses needed for native scripts or additional redundant key witnesses". Currently it reads like the total number of key witnesses for the transaction.
Changelog
Context
This improves the accurate of the
evaluateTransactionFee
function.See IntersectMBO/cardano-cli#534 (comment)
Impact on
cardano-cli
cardano-cli
usesevaluateTransactionFee
here.This usage does not have
utxo
in scope which will be required after this change is introduced.A possible solution is to query for the utxo like this.
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