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

Make --fee mandatory in transaction build-raw #768

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ data TransactionBuildRawCmdArgs era = TransactionBuildRawCmdArgs
-- ^ Transaction validity lower bound
, mValidityUpperBound :: !(TxValidityUpperBound era)
-- ^ Transaction validity upper bound
, fee :: !(Maybe Coin)
, fee :: !Coin
-- ^ Transaction fee
, certificates :: ![(CertificateFile, Maybe (ScriptWitnessFiles WitCtxStake))]
-- ^ Certificates with potential script witness
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ pTransactionBuildRaw era =
<*> optional (pMintMultiAsset ManualBalance)
<*> optional pInvalidBefore
<*> pInvalidHereafter era
<*> optional pTxFee
<*> pTxFee
<*> many (pCertificateFile ManualBalance )
<*> many (pWithdrawal ManualBalance)
<*> pTxMetadataJsonSchema
Expand Down
22 changes: 11 additions & 11 deletions cardano-cli/src/Cardano/CLI/EraBased/Run/Transaction.hs
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,7 @@ runTransactionBuildEstimateCmd
certsAndMaybeScriptWits
withdrawalsAndMaybeScriptWits
requiredSigners
Nothing
0
txAuxScripts
txMetadata
txUpdateProposal
Expand All @@ -360,15 +360,15 @@ runTransactionBuildEstimateCmd
poolsToDeregister = Set.fromList $ catMaybes [getPoolDeregistrationInfo cert | (cert,_) <- certsAndMaybeScriptWits]
totCol = fromMaybe 0 plutusCollateral
pScriptExecUnits = Map.fromList [ (sWitIndex, execUnits)
| (sWitIndex, (AnyScriptWitness (PlutusScriptWitness _ _ _ _ _ execUnits))) <- collectTxBodyScriptWitnesses sbe txBodyContent
| (sWitIndex, AnyScriptWitness (PlutusScriptWitness _ _ _ _ _ execUnits)) <- collectTxBodyScriptWitnesses sbe txBodyContent
]

BalancedTxBody _ balancedTxBody _ _ <-
hoistEither $ first TxCmdFeeEstimationError $
estimateBalancedTxBody eon txBodyContent (unLedgerProtocolParameters ledgerPParams) poolsToDeregister
stakeCredentialsToDeregisterMap drepsToDeregisterMap
pScriptExecUnits totCol shelleyWitnesses (fromMaybe 0 mByronWitnesses)
(fromMaybe 0 (unReferenceScriptSize <$> totalReferenceScriptSize)) (anyAddressInShelleyBasedEra sbe changeAddr)
(maybe 0 unReferenceScriptSize totalReferenceScriptSize) (anyAddressInShelleyBasedEra sbe changeAddr)
totalUTxOValue

let noWitTx = makeSignedTransaction [] balancedTxBody
Expand Down Expand Up @@ -576,7 +576,7 @@ runTxBuildRaw :: ()
-- ^ Tx lower bound
-> TxValidityUpperBound era
-- ^ Tx upper bound
-> Maybe L.Coin
-> L.Coin
-- ^ Tx fee
-> (Value, [ScriptWitness WitCtxMint era])
-- ^ Multi-Asset value(s)
Expand All @@ -597,13 +597,13 @@ runTxBuildRaw sbe
readOnlyRefIns txinsc
mReturnCollateral mTotCollateral txouts
mLowerBound mUpperBound
mFee valuesWithScriptWits
fee valuesWithScriptWits
certsAndMaybeSriptWits withdrawals reqSigners
txAuxScripts txMetadata mpparams txUpdateProposal votingProcedures proposals = do

txBodyContent <- constructTxBodyContent sbe mScriptValidity (unLedgerProtocolParameters <$> mpparams) inputsAndMaybeScriptWits readOnlyRefIns txinsc
mReturnCollateral mTotCollateral txouts mLowerBound mUpperBound valuesWithScriptWits
certsAndMaybeSriptWits withdrawals reqSigners mFee txAuxScripts txMetadata txUpdateProposal
certsAndMaybeSriptWits withdrawals reqSigners fee txAuxScripts txMetadata txUpdateProposal
votingProcedures proposals

first TxCmdTxBodyError $ createAndValidateTransactionBody sbe txBodyContent
Expand Down Expand Up @@ -636,7 +636,7 @@ constructTxBodyContent
-- ^ Withdrawals
-> [Hash PaymentKey]
-- ^ Required signers
-> Maybe L.Coin
-> L.Coin
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm unsure about this bit. I guess it makes sense to say in the function signature, that the fee is required and let users specify 0 if needed.

-- ^ Tx fee
-> TxAuxScripts era
-> TxMetadataInEra era
Expand All @@ -647,7 +647,7 @@ constructTxBodyContent
constructTxBodyContent sbe mScriptValidity mPparams inputsAndMaybeScriptWits readOnlyRefIns txinsc
mReturnCollateral mTotCollateral txouts mLowerBound mUpperBound
valuesWithScriptWits certsAndMaybeScriptWits withdrawals
reqSigners mFee txAuxScripts txMetadata txUpdateProposal
reqSigners fee txAuxScripts txMetadata txUpdateProposal
votingProcedures proposals
= do
let era = toCardanoEra sbe -- TODO: Propagate SBE
Expand All @@ -666,7 +666,7 @@ constructTxBodyContent sbe mScriptValidity mPparams inputsAndMaybeScriptWits rea
<- first TxCmdTotalCollateralValidationError $ validateTxTotalCollateral era mTotCollateral
validatedRetCol
<- first TxCmdReturnCollateralValidationError $ validateTxReturnCollateral era mReturnCollateral
dFee <- first TxCmdTxFeeValidationError . validateTxFee sbe . Just $ fromMaybe (L.Coin 0) mFee
let txFee = TxFeeExplicit sbe fee
validatedLowerBound <- first TxCmdTxValidityLowerBoundValidationError (validateTxValidityLowerBound era mLowerBound)
validatedReqSigners <- first TxCmdRequiredSignersValidationError $ validateRequiredSigners era reqSigners
validatedTxWtdrwls <- first TxCmdTxWithdrawalsValidationError $ validateTxWithdrawals era withdrawals
Expand All @@ -683,7 +683,7 @@ constructTxBodyContent sbe mScriptValidity mPparams inputsAndMaybeScriptWits rea
& setTxOuts txouts
& setTxTotalCollateral validatedTotCollateral
& setTxReturnCollateral validatedRetCol
& setTxFee dFee
& setTxFee txFee
& setTxValidityLowerBound validatedLowerBound
& setTxValidityUpperBound mUpperBound
& setTxMetadata txMetadata
Expand Down Expand Up @@ -809,7 +809,7 @@ runTxBuild
certsAndMaybeScriptWits
withdrawals
reqSigners
Nothing
0
txAuxScripts
txMetadata
txUpdateProposal
Expand Down
3 changes: 2 additions & 1 deletion cardano-cli/src/Cardano/CLI/Legacy/Run/Transaction.hs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import Cardano.CLI.Types.Errors.TxValidationError
import Cardano.CLI.Types.Governance

import Data.Function
import Data.Maybe

runLegacyTransactionCmds :: LegacyTransactionCmds -> ExceptT TxCmdError IO ()
runLegacyTransactionCmds = \case
Expand Down Expand Up @@ -175,7 +176,7 @@ runLegacyTransactionBuildRawCmd
runTransactionBuildRawCmd
( Cmd.TransactionBuildRawCmdArgs
sbe mScriptValidity txins readOnlyRefIns txinsc mReturnColl
mTotColl reqSigners txouts mValue mLowBound upperBound fee certs wdrls
mTotColl reqSigners txouts mValue mLowBound upperBound (fromMaybe 0 fee) certs wdrls
Copy link
Contributor Author

Choose a reason for hiding this comment

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

As far as I understand, in Byron the fee is still optional?

metadataSchema scriptFiles metadataFiles mProtocolParamsFile mUpdateProposalFile [] []
outFile
)
Expand Down
3 changes: 0 additions & 3 deletions cardano-cli/src/Cardano/CLI/Types/Errors/TxCmdError.hs
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,6 @@ data TxCmdError
| TxCmdAuxScriptsValidationError TxAuxScriptsValidationError
| TxCmdTotalCollateralValidationError TxTotalCollateralValidationError
| TxCmdReturnCollateralValidationError TxReturnCollateralValidationError
| TxCmdTxFeeValidationError TxFeeValidationError
| TxCmdTxValidityLowerBoundValidationError TxValidityLowerBoundValidationError
| TxCmdTxValidityUpperBoundValidationError TxValidityUpperBoundValidationError
| TxCmdRequiredSignersValidationError TxRequiredSignersValidationError
Expand Down Expand Up @@ -220,8 +219,6 @@ renderTxCmdError = \case
prettyError e
TxCmdReturnCollateralValidationError e ->
prettyError e
TxCmdTxFeeValidationError e ->
prettyError e
TxCmdTxValidityLowerBoundValidationError e ->
prettyError e
TxCmdTxValidityUpperBoundValidationError e ->
Expand Down
23 changes: 0 additions & 23 deletions cardano-cli/src/Cardano/CLI/Types/Errors/TxValidationError.hs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
module Cardano.CLI.Types.Errors.TxValidationError
( TxAuxScriptsValidationError(..)
, TxCertificatesValidationError(..)
, TxFeeValidationError(..)
, TxGovDuplicateVotes(..)
, TxProtocolParametersValidationError
, TxScriptValidityValidationError(..)
Expand All @@ -26,7 +25,6 @@ module Cardano.CLI.Types.Errors.TxValidationError
, validateScriptSupportedInEra
, validateTxAuxScripts
, validateTxCertificates
, validateTxFee
, validateRequiredSigners
, validateTxReturnCollateral
, validateTxScriptValidity
Expand Down Expand Up @@ -72,27 +70,6 @@ validateScriptSupportedInEra era script@(ScriptInAnyLang lang _) =
(AnyScriptLanguage lang) (anyCardanoEra $ toCardanoEra era)
Just script' -> pure script'


data TxFeeValidationError
= TxFeatureImplicitFeesE AnyCardanoEra -- ^ Expected an explicit fee
| TxFeatureExplicitFeesE AnyCardanoEra -- ^ Expected an implicit fee
deriving Show

instance Error TxFeeValidationError where
prettyError (TxFeatureImplicitFeesE era) =
"Implicit transaction fee not supported in " <> pretty era
prettyError (TxFeatureExplicitFeesE era) =
"Explicit transaction fee not supported in " <> pretty era

validateTxFee :: ShelleyBasedEra era
-> Maybe L.Coin -- TODO: Make this mandatory in the cli (Remove Maybe)
-> Either TxFeeValidationError (TxFee era)
validateTxFee era = \case
Nothing ->
let cEra = toCardanoEra era
in Left . TxFeatureImplicitFeesE $ cardanoEraConstraints cEra $ AnyCardanoEra cEra
Just fee -> pure (TxFeeExplicit era fee)

newtype TxTotalCollateralValidationError
= TxTotalCollateralNotSupported AnyCardanoEra
deriving Show
Expand Down
14 changes: 7 additions & 7 deletions cardano-cli/test/cardano-cli-golden/files/golden/help.cli
Original file line number Diff line number Diff line change
Expand Up @@ -912,7 +912,7 @@ Usage: cardano-cli shelley transaction build-raw
)]
[--invalid-before SLOT]
[--invalid-hereafter SLOT]
[--fee LOVELACE]
--fee LOVELACE
[--certificate-file FILE
[ --certificate-script-file FILE
[
Expand Down Expand Up @@ -2107,7 +2107,7 @@ Usage: cardano-cli allegra transaction build-raw
)]
[--invalid-before SLOT]
[--invalid-hereafter SLOT]
[--fee LOVELACE]
--fee LOVELACE
[--certificate-file FILE
[ --certificate-script-file FILE
[
Expand Down Expand Up @@ -3294,7 +3294,7 @@ Usage: cardano-cli mary transaction build-raw
)]
[--invalid-before SLOT]
[--invalid-hereafter SLOT]
[--fee LOVELACE]
--fee LOVELACE
[--certificate-file FILE
[ --certificate-script-file FILE
[
Expand Down Expand Up @@ -4616,7 +4616,7 @@ Usage: cardano-cli alonzo transaction build-raw
)]
[--invalid-before SLOT]
[--invalid-hereafter SLOT]
[--fee LOVELACE]
--fee LOVELACE
[--certificate-file FILE
[ --certificate-script-file FILE
[
Expand Down Expand Up @@ -5970,7 +5970,7 @@ Usage: cardano-cli babbage transaction build-raw
)]
[--invalid-before SLOT]
[--invalid-hereafter SLOT]
[--fee LOVELACE]
--fee LOVELACE
[--certificate-file FILE
[ --certificate-script-file FILE
[
Expand Down Expand Up @@ -7772,7 +7772,7 @@ Usage: cardano-cli conway transaction build-raw
)]
[--invalid-before SLOT]
[--invalid-hereafter SLOT]
[--fee LOVELACE]
--fee LOVELACE
[--certificate-file FILE
[ --certificate-script-file FILE
[
Expand Down Expand Up @@ -9164,7 +9164,7 @@ Usage: cardano-cli latest transaction build-raw
)]
[--invalid-before SLOT]
[--invalid-hereafter SLOT]
[--fee LOVELACE]
--fee LOVELACE
[--certificate-file FILE
[ --certificate-script-file FILE
[
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ Usage: cardano-cli allegra transaction build-raw
)]
[--invalid-before SLOT]
[--invalid-hereafter SLOT]
[--fee LOVELACE]
--fee LOVELACE
[--certificate-file FILE
[ --certificate-script-file FILE
[
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ Usage: cardano-cli alonzo transaction build-raw
)]
[--invalid-before SLOT]
[--invalid-hereafter SLOT]
[--fee LOVELACE]
--fee LOVELACE
[--certificate-file FILE
[ --certificate-script-file FILE
[
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ Usage: cardano-cli babbage transaction build-raw
)]
[--invalid-before SLOT]
[--invalid-hereafter SLOT]
[--fee LOVELACE]
--fee LOVELACE
[--certificate-file FILE
[ --certificate-script-file FILE
[
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ Usage: cardano-cli conway transaction build-raw
)]
[--invalid-before SLOT]
[--invalid-hereafter SLOT]
[--fee LOVELACE]
--fee LOVELACE
[--certificate-file FILE
[ --certificate-script-file FILE
[
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ Usage: cardano-cli latest transaction build-raw
)]
[--invalid-before SLOT]
[--invalid-hereafter SLOT]
[--fee LOVELACE]
--fee LOVELACE
[--certificate-file FILE
[ --certificate-script-file FILE
[
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ Usage: cardano-cli mary transaction build-raw
)]
[--invalid-before SLOT]
[--invalid-hereafter SLOT]
[--fee LOVELACE]
--fee LOVELACE
[--certificate-file FILE
[ --certificate-script-file FILE
[
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ Usage: cardano-cli shelley transaction build-raw
)]
[--invalid-before SLOT]
[--invalid-hereafter SLOT]
[--fee LOVELACE]
--fee LOVELACE
[--certificate-file FILE
[ --certificate-script-file FILE
[
Expand Down
Loading