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

Fix missing redeemers in certificate delegation and deregistration #306

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
15 changes: 8 additions & 7 deletions cardano-cli/src/Cardano/CLI/EraBased/Run/Transaction.hs
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ import Data.Type.Equality (TestEquality (..))
import Lens.Micro ((^.))
import qualified System.IO as IO


runTransactionCmds :: TransactionCmds era -> ExceptT TxCmdError IO ()
runTransactionCmds cmd =
case cmd of
Expand Down Expand Up @@ -173,13 +174,13 @@ runTxBuildCmd
readFileTextEnvelope AsCertificate (File certFile))
| (CertificateFile certFile, mSwit) <- certFilesAndMaybeScriptWits
]
withdrawalsAndMaybeScriptWits <- firstExceptT TxCmdScriptWitnessError
$ readScriptWitnessFilesThruple era wdrls
txMetadata <- firstExceptT TxCmdMetadataError
. newExceptT $ readTxMetadata era metadataSchema metadataFiles
withdrawalsAndMaybeScriptWits <- firstExceptT TxCmdScriptWitnessError $
readScriptWitnessFilesThruple era wdrls
txMetadata <- firstExceptT TxCmdMetadataError . newExceptT $
readTxMetadata era metadataSchema metadataFiles
valuesWithScriptWits <- readValueScriptWitnesses era $ fromMaybe mempty mValue
scripts <- firstExceptT TxCmdScriptFileError $
mapM (readFileScriptInAnyLang . unScriptFile) scriptFiles
mapM (readFileScriptInAnyLang . unScriptFile) scriptFiles
txAuxScripts <- hoistEither $ first TxCmdAuxScriptsValidationError $ validateTxAuxScripts era scripts

mProp <- forM mUpProp $ \(UpdateProposalFile upFp) ->
Expand Down Expand Up @@ -517,12 +518,12 @@ runTxBuild
dummyFee = Just $ Lovelace 0
inputsThatRequireWitnessing = [input | (input,_) <- inputsAndMaybeScriptWits]

-- Pure
let allReferenceInputs = getAllReferenceInputs
inputsAndMaybeScriptWits
(snd valuesWithScriptWits)
certsAndMaybeScriptWits
withdrawals readOnlyRefIns
withdrawals
readOnlyRefIns

validatedCollateralTxIns <- hoistEither $ validateTxInsCollateral era txinsc
validatedRefInputs <- hoistEither $ validateTxInsReference era allReferenceInputs
Expand Down
6 changes: 4 additions & 2 deletions cardano-cli/src/Cardano/CLI/Read.hs
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ import GHC.IO.Handle.FD (openFileBlocking)
import qualified Options.Applicative as Opt
import System.IO (IOMode (ReadMode))


-- Metadata

data MetadataError
Expand Down Expand Up @@ -288,9 +289,9 @@ readScriptWitness era (PlutusScriptWitnessFiles
case script' of
PlutusScript version pscript -> do
datum <- firstExceptT ScriptWitnessErrorScriptData
$ readScriptDatumOrFile datumOrFile
$ readScriptDatumOrFile datumOrFile
redeemer <- firstExceptT ScriptWitnessErrorScriptData
$ readScriptRedeemerOrFile redeemerOrFile
$ readScriptRedeemerOrFile redeemerOrFile
return $ PlutusScriptWitness
langInEra version (PScript pscript)
datum
Expand Down Expand Up @@ -620,6 +621,7 @@ data SomeSigningWitness
| AGenesisUTxOSigningWitness (SigningKey GenesisUTxOKey)
| ADRepSigningWitness (SigningKey DRepKey)
| ACommitteeSigningWitness (SigningKey CommitteeColdKey)
deriving Show


-- | Data required for constructing a Shelley bootstrap witness.
Expand Down
2 changes: 1 addition & 1 deletion cardano-cli/src/Cardano/CLI/Types/Errors/TxCmdError.hs
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ import Cardano.Api.Shelley

import Cardano.CLI.Read
import Cardano.CLI.Types.Common
import Cardano.CLI.Types.Errors.ProtocolParamsError
import Cardano.CLI.Types.Errors.BootstrapWitnessError
import Cardano.CLI.Types.Errors.ProtocolParamsError
import Cardano.CLI.Types.Errors.TxValidationError
import Cardano.CLI.Types.Output
import Cardano.CLI.Types.TxFeature
Expand Down
71 changes: 45 additions & 26 deletions cardano-cli/src/Cardano/CLI/Types/Errors/TxValidationError.hs
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
{-# OPTIONS_GHC -Wno-incomplete-uni-patterns #-}
Copy link
Contributor

Choose a reason for hiding this comment

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

👍


{-# LANGUAGE DataKinds #-}
{-# LANGUAGE FlexibleContexts #-}
{-# LANGUAGE GADTs #-}
Expand Down Expand Up @@ -37,6 +35,7 @@ module Cardano.CLI.Types.Errors.TxValidationError
) where

import Cardano.Api
import qualified Cardano.Api.Ledger as L
import Cardano.Api.Shelley

import Prelude
Expand Down Expand Up @@ -247,30 +246,50 @@ validateTxCertificates era certsAndScriptWitnesses = cardanoEraConstraints era $
reqWits = Map.fromList $ mapMaybe convert certsAndScriptWitnesses
pure $ TxCertificates supported certs $ BuildTxWith reqWits
where
-- We get the stake credential witness for a certificate that requires it.
-- NB: Only stake address deregistration and delegation requires
-- witnessing (witness can be script or key)
deriveStakeCredentialWitness
:: Certificate era
-> Maybe StakeCredential
deriveStakeCredentialWitness _cert = Nothing
-- case cert of
-- -- TODO: Conway era
-- -- StakeAddressDeregistrationCertificate sCred -> Just sCred
-- -- StakeAddressPoolDelegationCertificate sCred _ -> Just sCred
-- _ -> Nothing

convert
:: (Certificate era, Maybe (ScriptWitness WitCtxStake era))
-> Maybe (StakeCredential, Witness WitCtxStake era)
convert (cert, mScriptWitnessFiles) = do
sCred <- deriveStakeCredentialWitness cert
case mScriptWitnessFiles of
Just sWit -> do
Just ( sCred
, ScriptWitness ScriptWitnessForStakeAddr sWit
)
Nothing -> Just (sCred, KeyWitness KeyWitnessForStakeAddr)
-- We get the stake credential witness for a certificate that requires it.
-- NB: Only stake address deregistration and delegation requires
-- witnessing (witness can be script or key)
deriveStakeCredentialWitness
:: Certificate era
-> Maybe StakeCredential
deriveStakeCredentialWitness = fmap fromShelleyStakeCredential . \case
ShelleyRelatedCertificate stbEra shelleyCert -> shelleyToBabbageEraConstraints stbEra $
case shelleyCert of
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't we have this function in cardano-api?

Copy link
Contributor Author

@carbolymer carbolymer Oct 3, 2023

Choose a reason for hiding this comment

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

These are similar ones, but are not the same: https://github.com/input-output-hk/cardano-api/blob/151eff181cb1782f9db46dbea36d22e8d40f0545/cardano-api/internal/Cardano/Api/Certificate.hs#L670

Should we extract stake creds only for deregistration and delegation like the comment says here, or should we use stake credential witness for the cases when it's available?

Copy link
Contributor

Choose a reason for hiding this comment

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

We can leave it as it for now. We should follow the specification and document it here

L.RegTxCert _sCred -> Nothing -- not required
L.UnRegTxCert sCred -> Just sCred
L.DelegStakeTxCert sCred _ -> Just sCred
L.RegPoolTxCert _ -> Nothing
L.RetirePoolTxCert _ _ -> Nothing
L.MirTxCert _ -> Nothing
L.GenesisDelegTxCert{} -> Nothing

ConwayCertificate cEra conwayCert -> conwayEraOnwardsConstraints cEra $
case conwayCert of
L.RegPoolTxCert _ -> Nothing
L.RetirePoolTxCert _ _ -> Nothing
L.RegTxCert _ -> Nothing
L.UnRegTxCert sCred -> Just sCred
L.RegDepositTxCert _ _ -> Nothing
L.UnRegDepositTxCert sCred _ -> Just sCred
L.DelegTxCert sCred _ -> Just sCred
L.RegDepositDelegTxCert sCred _ _ -> Just sCred
L.AuthCommitteeHotKeyTxCert{} -> Nothing
L.ResignCommitteeColdTxCert _ -> Nothing
L.RegDRepTxCert{} -> Nothing
L.UnRegDRepTxCert{} -> Nothing
L.UpdateDRepTxCert{} -> Nothing

convert
:: (Certificate era, Maybe (ScriptWitness WitCtxStake era))
-> Maybe (StakeCredential, Witness WitCtxStake era)
convert (cert, mScriptWitnessFiles) = do
sCred <- deriveStakeCredentialWitness cert
case mScriptWitnessFiles of
Just sWit -> do
Just ( sCred
, ScriptWitness ScriptWitnessForStakeAddr sWit
)
Nothing -> Just (sCred, KeyWitness KeyWitnessForStakeAddr)

newtype TxProtocolParametersValidationError
= ProtocolParametersNotSupported AnyCardanoEra
Expand Down