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 anchor data hash checks to remaining governance action commands #915

Merged
merged 6 commits into from
Sep 25, 2024
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
2 changes: 2 additions & 0 deletions .gitattributes
Original file line number Diff line number Diff line change
@@ -1,2 +1,4 @@
cardano-cli/test/cardano-cli-golden/files/input/example_anchor_data.txt -text
cardano-cli/test/cardano-cli-golden/files/input/example_anchor_data2.txt -text
cardano-cli/test/cardano-cli-test/files/input/example_anchor_data.txt -text
cardano-cli/test/cardano-cli-test/files/input/example_anchor_data2.txt -text
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ data GovernanceActionUpdateCommitteeCmdArgs era
, returnAddress :: !StakeIdentifier
, proposalUrl :: !ProposalUrl
, proposalHash :: !(L.SafeHash L.StandardCrypto L.AnchorData)
, checkProposalHash :: !(MustCheckHash ProposalUrl)
, oldCommitteeVkeySource :: ![VerificationKeyOrHashOrFileOrScriptHash CommitteeColdKey]
, newCommitteeVkeySource :: ![(VerificationKeyOrHashOrFileOrScriptHash CommitteeColdKey, EpochNo)]
, requiredThreshold :: !Rational
Expand All @@ -68,8 +69,10 @@ data GovernanceActionCreateConstitutionCmdArgs era
, mPrevGovernanceActionId :: !(Maybe (TxId, Word16))
, proposalUrl :: !ProposalUrl
, proposalHash :: !(L.SafeHash L.StandardCrypto L.AnchorData)
, checkProposalHash :: !(MustCheckHash ProposalUrl)
, constitutionUrl :: !ConstitutionUrl
, constitutionHash :: !(L.SafeHash L.StandardCrypto L.AnchorData)
, checkConstitutionHash :: !(MustCheckHash ConstitutionUrl)
, constitutionScript :: !(Maybe ScriptHash)
, outFile :: !(File () Out)
}
Expand Down Expand Up @@ -97,6 +100,7 @@ data GovernanceActionCreateNoConfidenceCmdArgs era
, returnStakeAddress :: !StakeIdentifier
, proposalUrl :: !ProposalUrl
, proposalHash :: !(L.SafeHash L.StandardCrypto L.AnchorData)
, checkProposalHash :: !(MustCheckHash ProposalUrl)
, mPrevGovernanceActionId :: !(Maybe (TxId, Word16))
, outFile :: !(File () Out)
}
Expand Down Expand Up @@ -128,6 +132,7 @@ data GovernanceActionTreasuryWithdrawalCmdArgs era
, returnAddr :: !StakeIdentifier
, proposalUrl :: !ProposalUrl
, proposalHash :: !(L.SafeHash L.StandardCrypto L.AnchorData)
, checkProposalHash :: !(MustCheckHash ProposalUrl)
, treasuryWithdrawal :: ![(StakeIdentifier, Lovelace)]
, constitutionScriptHash :: !(Maybe ScriptHash)
, outFile :: !(File () Out)
Expand All @@ -143,6 +148,7 @@ data GovernanceActionHardforkInitCmdArgs era
, mPrevGovernanceActionId :: !(Maybe (TxId, Word16))
, proposalUrl :: !ProposalUrl
, proposalHash :: !(L.SafeHash L.StandardCrypto L.AnchorData)
, checkProposalHash :: !(MustCheckHash ProposalUrl)
, protVer :: !L.ProtVer
, outFile :: !(File () Out)
}
Expand All @@ -165,6 +171,7 @@ data UpdateProtocolParametersConwayOnwards era
, returnAddr :: !StakeIdentifier
, proposalUrl :: !ProposalUrl
, proposalHash :: !(L.SafeHash L.StandardCrypto L.AnchorData)
, checkProposalHash :: !(MustCheckHash ProposalUrl)
, governanceActionId :: !(Maybe (TxId, Word16))
, constitutionScriptHash :: !(Maybe ScriptHash)
}
Expand Down
59 changes: 59 additions & 0 deletions cardano-cli/src/Cardano/CLI/EraBased/Options/Common.hs
Original file line number Diff line number Diff line change
Expand Up @@ -3569,6 +3569,65 @@ pAnchorDataHash =
, Opt.help "Proposal anchor data hash (obtain it with \"cardano-cli hash anchor-data ...\")"
]

data HashCheckParamInfo anchorData
Copy link
Contributor

Choose a reason for hiding this comment

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

This type has incredibly poor type safety.

= HashCheckParamInfo
{ flagSuffix :: String
, dataName :: String
, hashParamName :: String
, urlParamName :: String
}

pMustCheckHash :: HashCheckParamInfo anchorData -> Parser (MustCheckHash anchorData)
pMustCheckHash
( HashCheckParamInfo
{ flagSuffix = flagSuffix'
, dataName = dataName'
, hashParamName = hashParamName'
, urlParamName = urlParamName'
}
) =
asum
[ Opt.flag' CheckHash $
mconcat
[ Opt.long ("check-" ++ flagSuffix')
, Opt.help
( "Check the "
++ dataName'
++ " hash (from "
++ hashParamName'
++ ") by downloading "
++ dataName'
++ " data (from "
++ urlParamName'
++ ")."
)
]
, Opt.flag' TrustHash $
mconcat
[ Opt.long ("trust-" ++ flagSuffix')
, Opt.help
("Do not check the " ++ dataName' ++ " hash (from " ++ hashParamName' ++ ") and trust it is correct.")
]
]

proposalHashCheckInfo :: HashCheckParamInfo ProposalUrl
Copy link
Contributor

Choose a reason for hiding this comment

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

Please turn these into stand alone parsers. You can create a function that creates a parser which is the usual style we have.

proposalHashCheckInfo =
HashCheckParamInfo
{ flagSuffix = "anchor-data"
, dataName = "proposal"
, hashParamName = "--anchor-data-hash"
, urlParamName = "--anchor-url"
}

constitutionHashCheckInfo :: HashCheckParamInfo ConstitutionUrl
constitutionHashCheckInfo =
HashCheckParamInfo
{ flagSuffix = "constitution-hash"
, dataName = "constitution"
, hashParamName = "--constitution-hash"
, urlParamName = "--constitution-url"
}

pPreviousGovernanceAction :: Parser (Maybe (TxId, Word16))
pPreviousGovernanceAction =
optional $
Expand Down
25 changes: 8 additions & 17 deletions cardano-cli/src/Cardano/CLI/EraBased/Options/Governance/Actions.hs
Original file line number Diff line number Diff line change
Expand Up @@ -74,26 +74,10 @@ pGovernanceActionNewInfoCmd era = do
<*> pStakeIdentifier (Just "deposit-return")
<*> pAnchorUrl
<*> pAnchorDataHash
<*> pMustCheckProposalHash
<*> pMustCheckHash proposalHashCheckInfo
<*> pFileOutDirection "out-file" "Path to action file to be used later on with build or build-raw "
)
$ Opt.progDesc "Create an info action."
where
pMustCheckProposalHash :: Parser (MustCheckHash ProposalUrl)
pMustCheckProposalHash =
asum
[ Opt.flag' CheckHash $
mconcat
[ Opt.long "check-anchor-data"
, Opt.help
"Check the proposal hash (from --anchor-data-hash) by downloading anchor data (from --anchor-url)."
]
, Opt.flag' TrustHash $
mconcat
[ Opt.long "trust-anchor-data"
, Opt.help "Do not check the proposal hash (from --anchor-data-hash) and trust it is correct."
]
]

pGovernanceActionNewConstitutionCmd
:: CardanoEra era
Expand All @@ -111,8 +95,10 @@ pGovernanceActionNewConstitutionCmd era = do
<*> pPreviousGovernanceAction
<*> pAnchorUrl
<*> pAnchorDataHash
<*> pMustCheckHash proposalHashCheckInfo
<*> pConstitutionUrl
<*> pConstitutionHash
<*> pMustCheckHash constitutionHashCheckInfo
<*> optional pConstitutionScriptHash
<*> pFileOutDirection "out-file" "Output filepath of the constitution."
)
Expand Down Expand Up @@ -142,6 +128,7 @@ pUpdateCommitteeCmd eon =
<*> pStakeIdentifier (Just "deposit-return")
<*> pAnchorUrl
<*> pAnchorDataHash
<*> pMustCheckHash proposalHashCheckInfo
<*> many pRemoveCommitteeColdVerificationKeySource
<*> many
( (,)
Expand All @@ -167,6 +154,7 @@ pGovernanceActionNoConfidenceCmd era = do
<*> pStakeIdentifier (Just "deposit-return")
<*> pAnchorUrl
<*> pAnchorDataHash
<*> pMustCheckHash proposalHashCheckInfo
<*> pPreviousGovernanceAction
<*> pFileOutDirection "out-file" "Output filepath of the no confidence proposal."
)
Expand All @@ -188,6 +176,7 @@ pUpdateProtocolParametersPostConway conwayOnwards =
<*> pStakeIdentifier (Just "deposit-return")
<*> pAnchorUrl
<*> pAnchorDataHash
<*> pMustCheckHash proposalHashCheckInfo
<*> pPreviousGovernanceAction
<*> optional pConstitutionScriptHash

Expand Down Expand Up @@ -395,6 +384,7 @@ pGovernanceActionTreasuryWithdrawalCmd era = do
<*> pStakeIdentifier (Just "deposit-return")
<*> pAnchorUrl
<*> pAnchorDataHash
<*> pMustCheckHash proposalHashCheckInfo
<*> some ((,) <$> pStakeIdentifier (Just "funds-receiving") <*> pTreasuryWithdrawalAmt)
<*> optional pConstitutionScriptHash
<*> pFileOutDirection "out-file" "Output filepath of the treasury withdrawal."
Expand Down Expand Up @@ -438,6 +428,7 @@ pGovernanceActionHardforkInitCmd era = do
<*> pPreviousGovernanceAction
<*> pAnchorUrl
<*> pAnchorDataHash
<*> pMustCheckHash proposalHashCheckInfo
<*> pPV
<*> pFileOutDirection "out-file" "Output filepath of the hardfork proposal."
)
Expand Down
69 changes: 52 additions & 17 deletions cardano-cli/src/Cardano/CLI/EraBased/Run/Governance/Actions.hs
Original file line number Diff line number Diff line change
Expand Up @@ -101,18 +101,7 @@ runGovernanceActionInfoCmd
, L.anchorDataHash = proposalHash
}

case checkProposalHash of
CheckHash -> do
anchorData <-
L.AnchorData
<$> fetchURLErrorToGovernanceActionError
ProposalCheck
(getByteStringFromURL httpsAndIpfsSchemas $ L.anchorUrl proposalAnchor)
let hash = L.hashAnchorData anchorData
when (hash /= L.anchorDataHash proposalAnchor) $
left $
GovernanceActionsProposalMismatchedHashError ProposalCheck proposalHash hash
TrustHash -> pure ()
carryHashChecks checkProposalHash proposalAnchor ProposalCheck

let sbe = conwayEraOnwardsToShelleyBasedEra eon
govAction = InfoAct
Expand All @@ -121,10 +110,10 @@ runGovernanceActionInfoCmd
firstExceptT GovernanceActionsCmdWriteFileError . newExceptT $
conwayEraOnwardsConstraints eon $
writeFileTextEnvelope outFile (Just "Info proposal") proposalProcedure
where
fetchURLErrorToGovernanceActionError
:: AnchorDataTypeCheck -> ExceptT FetchURLError IO a -> ExceptT GovernanceActionsError IO a
fetchURLErrorToGovernanceActionError adt = withExceptT (GovernanceActionsProposalFetchURLError adt)

fetchURLErrorToGovernanceActionError
:: AnchorDataTypeCheck -> ExceptT FetchURLError IO a -> ExceptT GovernanceActionsError IO a
fetchURLErrorToGovernanceActionError adt = withExceptT (GovernanceActionsProposalFetchURLError adt)
palas marked this conversation as resolved.
Show resolved Hide resolved

-- TODO: Conway era - update with new ledger types from cardano-ledger-conway-1.7.0.0
runGovernanceActionCreateNoConfidenceCmd
Expand All @@ -139,6 +128,7 @@ runGovernanceActionCreateNoConfidenceCmd
, Cmd.returnStakeAddress
, Cmd.proposalUrl
, Cmd.proposalHash
, Cmd.checkProposalHash
, Cmd.mPrevGovernanceActionId
, Cmd.outFile
} = do
Expand All @@ -152,6 +142,8 @@ runGovernanceActionCreateNoConfidenceCmd
, L.anchorDataHash = proposalHash
}

carryHashChecks checkProposalHash proposalAnchor ProposalCheck

let sbe = conwayEraOnwardsToShelleyBasedEra eon
previousGovernanceAction =
MotionOfNoConfidence $
Expand Down Expand Up @@ -185,9 +177,11 @@ runGovernanceActionCreateConstitutionCmd
, Cmd.mPrevGovernanceActionId
, Cmd.proposalUrl
, Cmd.proposalHash
, Cmd.checkProposalHash
, Cmd.constitutionUrl
, Cmd.constitutionHash
, Cmd.constitutionScript
, Cmd.checkConstitutionHash
, Cmd.outFile
} = do
depositStakeCredential <-
Expand All @@ -200,6 +194,8 @@ runGovernanceActionCreateConstitutionCmd
, L.anchorDataHash = proposalHash
}

carryHashChecks checkProposalHash proposalAnchor ProposalCheck

let prevGovActId =
L.maybeToStrictMaybe $
shelleyBasedEraConstraints sbe $
Expand All @@ -217,6 +213,8 @@ runGovernanceActionCreateConstitutionCmd
sbe = conwayEraOnwardsToShelleyBasedEra eon
proposalProcedure = createProposalProcedure sbe networkId deposit depositStakeCredential govAct proposalAnchor

carryHashChecks checkConstitutionHash constitutionAnchor ConstitutionCheck

firstExceptT GovernanceActionsCmdWriteFileError . newExceptT $
conwayEraOnwardsConstraints eon $
writeFileTextEnvelope
Expand All @@ -238,6 +236,7 @@ runGovernanceActionUpdateCommitteeCmd
, Cmd.returnAddress
, Cmd.proposalUrl
, Cmd.proposalHash
, Cmd.checkProposalHash
, Cmd.oldCommitteeVkeySource
, Cmd.newCommitteeVkeySource
, Cmd.requiredThreshold
Expand All @@ -257,6 +256,8 @@ runGovernanceActionUpdateCommitteeCmd
, L.anchorDataHash = proposalHash
}

carryHashChecks checkProposalHash proposalAnchor ProposalCheck

oldCommitteeKeyHashes <- forM oldCommitteeVkeySource $ \vkeyOrHashOrTextFile ->
modifyError GovernanceActionsCmdReadFileError $
readVerificationKeyOrHashOrFileOrScriptHash
Expand Down Expand Up @@ -343,6 +344,7 @@ runGovernanceActionCreateProtocolParametersUpdateCmd eraBasedPParams' = do
returnAddr
proposalUrl
proposalHash
checkProposalHash
mPrevGovActId
mConstitutionalScriptHash <-
hoistMaybe (GovernanceActionsValueUpdateProtocolParametersNotFound anyEra) $
Expand All @@ -362,7 +364,10 @@ runGovernanceActionCreateProtocolParametersUpdateCmd eraBasedPParams' = do
{ L.anchorUrl = unProposalUrl proposalUrl
, L.anchorDataHash = proposalHash
}
govAct =

carryHashChecks checkProposalHash proposalAnchor ProposalCheck

let govAct =
UpdatePParams
prevGovActId
updateProtocolParams
Expand Down Expand Up @@ -419,6 +424,7 @@ runGovernanceActionTreasuryWithdrawalCmd
, Cmd.returnAddr
, Cmd.proposalUrl
, Cmd.proposalHash
, Cmd.checkProposalHash
, Cmd.treasuryWithdrawal
, Cmd.constitutionScriptHash
, Cmd.outFile
Expand All @@ -429,6 +435,8 @@ runGovernanceActionTreasuryWithdrawalCmd
, L.anchorDataHash = proposalHash
}

carryHashChecks checkProposalHash proposalAnchor ProposalCheck

depositStakeCredential <-
firstExceptT GovernanceActionsReadStakeCredErrror $
getStakeCredentialFromIdentifier returnAddr
Expand Down Expand Up @@ -469,6 +477,7 @@ runGovernanceActionHardforkInitCmd
, Cmd.mPrevGovernanceActionId
, Cmd.proposalUrl
, Cmd.proposalHash = anchorDataHash
, Cmd.checkProposalHash
, Cmd.protVer
, Cmd.outFile
} = do
Expand All @@ -482,6 +491,8 @@ runGovernanceActionHardforkInitCmd
, L.anchorDataHash
}

carryHashChecks checkProposalHash proposalAnchor ProposalCheck

let sbe = conwayEraOnwardsToShelleyBasedEra eon
govActIdentifier =
L.maybeToStrictMaybe $
Expand All @@ -497,3 +508,27 @@ runGovernanceActionHardforkInitCmd
firstExceptT GovernanceActionsCmdWriteFileError . newExceptT $
conwayEraOnwardsConstraints eon $
writeFileTextEnvelope outFile (Just "Hardfork initiation proposal") proposalProcedure

-- | Check the hash of the anchor data against the hash in the anchor if
-- checkHash is set to CheckHash.
carryHashChecks
palas marked this conversation as resolved.
Show resolved Hide resolved
:: MustCheckHash a
-- ^ Whether to check the hash or not (CheckHash for checking or TrustHash for not checking)
-> L.Anchor L.StandardCrypto
-- ^ The anchor data whose hash is to be checked
-> AnchorDataTypeCheck
-- ^ The type of anchor data to check (for error reporting purpouses)
-> ExceptT GovernanceActionsError IO ()
carryHashChecks checkHash anchor checkType =
case checkHash of
CheckHash -> do
anchorData <-
L.AnchorData
<$> fetchURLErrorToGovernanceActionError
checkType
(getByteStringFromURL httpsAndIpfsSchemas $ L.anchorUrl anchor)
let hash = L.hashAnchorData anchorData
when (hash /= L.anchorDataHash anchor) $
left $
GovernanceActionsProposalMismatchedHashError checkType (L.anchorDataHash anchor) hash
TrustHash -> pure ()
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,11 @@ instance Error GovernanceActionsError where
<+> "hash:"
<+> pretty (displayException fetchErr)

data AnchorDataTypeCheck = ProposalCheck
data AnchorDataTypeCheck
Copy link
Contributor

Choose a reason for hiding this comment

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

This needs to be commented.

= ProposalCheck
| ConstitutionCheck
deriving Show

anchorDataTypeCheckName :: AnchorDataTypeCheck -> String
anchorDataTypeCheckName ProposalCheck = "proposal"
anchorDataTypeCheckName ConstitutionCheck = "constitution"
Loading
Loading