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.

Sign up for GitHub

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

Disambiguate 2-n flags in governance new-committee action #302

Merged
merged 1 commit into from
Sep 21, 2023
Merged

Disambiguate 2-n flags in governance new-committee action #302

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
107 changes: 62 additions & 45 deletions cardano-cli/src/Cardano/CLI/EraBased/Options/Common.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,14 @@ command' c descr p =
, metavar c
]

-- | @prefixFlag Nothing bar@ is @bar@, while @prefixFlag (Just "foo") bar@ is @foo-bar@.
-- This function is used to optionally prefix some long flags
prefixFlag :: Maybe String -> String -> String
prefixFlag prefix longFlag =
case prefix of
Nothing -> longFlag
Just prefix' -> prefix' <> "-" <> longFlag

pNetworkId :: EnvCli -> Parser NetworkId
pNetworkId envCli = asum $ mconcat
[ [ Opt.flag' Mainnet $ mconcat
Expand Down Expand Up @@ -200,7 +208,7 @@ pStakeIdentifier = asum

pStakeVerifier :: Parser StakeVerifier
pStakeVerifier = asum
[ StakeVerifierKey <$> pStakeVerificationKeyOrFile
[ StakeVerifierKey <$> pStakeVerificationKeyOrFile Nothing
, StakeVerifierScriptFile <$> pScriptFor "stake-script-file" Nothing "Filepath of the staking script."
]

Expand All @@ -219,10 +227,11 @@ parseStakeAddress = do
Nothing -> fail $ "invalid address: " <> Text.unpack str'
Just addr -> pure addr

pStakeVerificationKeyOrFile :: Parser (VerificationKeyOrFile StakeKey)
pStakeVerificationKeyOrFile =
VerificationKeyValue <$> pStakeVerificationKey
<|> VerificationKeyFilePath <$> pStakeVerificationKeyFile
-- | First argument is the optional prefix
pStakeVerificationKeyOrFile :: Maybe String -> Parser (VerificationKeyOrFile StakeKey)
pStakeVerificationKeyOrFile prefix =
VerificationKeyValue <$> pStakeVerificationKey prefix
<|> VerificationKeyFilePath <$> pStakeVerificationKeyFile prefix

pScriptFor :: String -> Maybe String -> String -> Parser ScriptFile
pScriptFor name Nothing help' =
Expand All @@ -240,10 +249,11 @@ pScriptFor name (Just deprecated) help' =
<> Opt.internal
)

pStakeVerificationKey :: Parser (VerificationKey StakeKey)
pStakeVerificationKey =
-- | The first argument is the optional prefix.
pStakeVerificationKey :: Maybe String -> Parser (VerificationKey StakeKey)
pStakeVerificationKey prefix =
Opt.option (readVerificationKey AsStakeKey) $ mconcat
[ Opt.long "stake-verification-key"
[ Opt.long $ prefixFlag prefix "stake-verification-key"
, Opt.metavar "STRING"
, Opt.help "Stake verification key (Bech32 or hex-encoded)."
]
Expand All @@ -266,17 +276,18 @@ readVerificationKey asType =
first (Text.unpack . renderInputDecodeError) $
deserialiseInput (AsVerificationKey asType) keyFormats (BSC.pack str')

pStakeVerificationKeyFile :: Parser (VerificationKeyFile In)
pStakeVerificationKeyFile =
-- | The first argument is the optional prefix.
pStakeVerificationKeyFile :: Maybe String -> Parser (VerificationKeyFile In)
pStakeVerificationKeyFile prefix =
File <$> asum
[ Opt.strOption $ mconcat
[ Opt.long "stake-verification-key-file"
[ Opt.long $ prefixFlag prefix "stake-verification-key-file"
, Opt.metavar "FILE"
, Opt.help "Filepath of the staking verification key."
, Opt.completer (Opt.bashCompleter "file")
]
, Opt.strOption $ mconcat
[ Opt.long "staking-verification-key-file"
[ Opt.long $ prefixFlag prefix "staking-verification-key-file"
, Opt.internal
]
]
Expand Down Expand Up @@ -425,32 +436,35 @@ parseLovelace = do
then fail $ show i <> " lovelace exceeds the Word64 upper bound"
else return $ Lovelace i

pStakePoolVerificationKeyOrFile :: Parser (VerificationKeyOrFile StakePoolKey)
pStakePoolVerificationKeyOrFile =
-- | The first argument is the optional prefix.
pStakePoolVerificationKeyOrFile :: Maybe String -> Parser (VerificationKeyOrFile StakePoolKey)
pStakePoolVerificationKeyOrFile prefix =
asum
[ VerificationKeyValue <$> pStakePoolVerificationKey
, VerificationKeyFilePath <$> pStakePoolVerificationKeyFile
[ VerificationKeyValue <$> pStakePoolVerificationKey prefix
, VerificationKeyFilePath <$> pStakePoolVerificationKeyFile prefix
]

pStakePoolVerificationKey :: Parser (VerificationKey StakePoolKey)
pStakePoolVerificationKey =
-- | The first argument is the optional prefix.
pStakePoolVerificationKey :: Maybe String -> Parser (VerificationKey StakePoolKey)
pStakePoolVerificationKey prefix =
Opt.option (readVerificationKey AsStakePoolKey) $ mconcat
[ Opt.long "stake-pool-verification-key"
[ Opt.long $ prefixFlag prefix "stake-pool-verification-key"
, Opt.metavar "STRING"
, Opt.help "Stake pool verification key (Bech32 or hex-encoded)."
]

pStakePoolVerificationKeyFile :: Parser (VerificationKeyFile In)
pStakePoolVerificationKeyFile =
-- | The first argument is the optional prefix.
pStakePoolVerificationKeyFile :: Maybe String -> Parser (VerificationKeyFile In)
pStakePoolVerificationKeyFile prefix =
File <$> asum
[ Opt.strOption $ mconcat
[ Opt.long "cold-verification-key-file"
[ Opt.long $ prefixFlag prefix "cold-verification-key-file"
, Opt.metavar "FILE"
, Opt.help "Filepath of the stake pool verification key."
, Opt.completer (Opt.bashCompleter "file")
]
, Opt.strOption $ mconcat
[ Opt.long "stake-pool-verification-key-file"
[ Opt.long $ prefixFlag prefix "stake-pool-verification-key-file"
, Opt.internal
]
]
Expand Down Expand Up @@ -525,10 +539,11 @@ pGenesisDelegateVerificationKey =
. deserialiseFromRawBytesHex (AsVerificationKey AsGenesisDelegateKey)
. BSC.pack

pColdVerificationKeyOrFile :: Parser ColdVerificationKeyOrFile
pColdVerificationKeyOrFile =
-- | The first argument is the optional prefix.
pColdVerificationKeyOrFile :: Maybe String -> Parser ColdVerificationKeyOrFile
pColdVerificationKeyOrFile prefix =
asum
[ ColdStakePoolVerificationKey <$> pStakePoolVerificationKey
[ ColdStakePoolVerificationKey <$> pStakePoolVerificationKey prefix
, ColdGenesisDelegateVerificationKey <$> pGenesisDelegateVerificationKey
, ColdVerificationKeyFile <$> pColdVerificationKeyFile
]
Expand Down Expand Up @@ -786,29 +801,30 @@ pGovActionDeposit =
]


pStakeVerificationKeyOrHashOrFile :: Parser (VerificationKeyOrHashOrFile StakeKey)
pStakeVerificationKeyOrHashOrFile = asum
[ VerificationKeyOrFile <$> pStakeVerificationKeyOrFile
, VerificationKeyHash <$> pStakeVerificationKeyHash
-- | First argument is the optional prefix
pStakeVerificationKeyOrHashOrFile :: Maybe String -> Parser (VerificationKeyOrHashOrFile StakeKey)
pStakeVerificationKeyOrHashOrFile prefix = asum
[ VerificationKeyOrFile <$> pStakeVerificationKeyOrFile prefix
, VerificationKeyHash <$> pStakeVerificationKeyHash prefix
]

pStakeVerificationKeyHash :: Parser (Hash StakeKey)
pStakeVerificationKeyHash =
-- | First argument is the optional prefix
pStakeVerificationKeyHash :: Maybe String -> Parser (Hash StakeKey)
pStakeVerificationKeyHash prefix =
Opt.option (pHexHash AsStakeKey) $ mconcat
[ Opt.long "stake-key-hash"
[ Opt.long $ prefixFlag prefix "stake-key-hash"
, Opt.metavar "HASH"
, Opt.help $ mconcat
[ "Stake verification key hash (hex-encoded)."
]
, Opt.help "Stake verification key hash (hex-encoded)."
]


-- | The first argument is the optional prefix.
pStakePoolVerificationKeyOrHashOrFile
:: Parser (VerificationKeyOrHashOrFile StakePoolKey)
pStakePoolVerificationKeyOrHashOrFile =
:: Maybe String -> Parser (VerificationKeyOrHashOrFile StakePoolKey)
pStakePoolVerificationKeyOrHashOrFile prefix =
asum
[ VerificationKeyOrFile <$> pStakePoolVerificationKeyOrFile
, VerificationKeyHash <$> pStakePoolVerificationKeyHash
[ VerificationKeyOrFile <$> pStakePoolVerificationKeyOrFile prefix
, VerificationKeyHash <$> pStakePoolVerificationKeyHash prefix
]

pCombinedStakePoolVerificationKeyOrHashOrFile
Expand Down Expand Up @@ -2092,10 +2108,11 @@ pAddress =
, Opt.help "A Cardano address"
]

pStakePoolVerificationKeyHash :: Parser (Hash StakePoolKey)
pStakePoolVerificationKeyHash =
-- | First argument is the prefix to use
pStakePoolVerificationKeyHash :: Maybe String -> Parser (Hash StakePoolKey)
pStakePoolVerificationKeyHash prefix =
Opt.option (pBech32KeyHash AsStakePoolKey <|> pHexHash AsStakePoolKey) $ mconcat
[ Opt.long "stake-pool-id"
[ Opt.long $ prefixFlag prefix "stake-pool-id"
, Opt.metavar "STAKE_POOL_ID"
, Opt.help $ mconcat
[ "Stake pool ID/verification key hash (either Bech32-encoded or hex-encoded). "
Expand Down Expand Up @@ -2347,7 +2364,7 @@ pStakePoolRegistrationParserRequirements
:: EnvCli -> Parser StakePoolRegistrationParserRequirements
pStakePoolRegistrationParserRequirements envCli =
StakePoolRegistrationParserRequirements
<$> pStakePoolVerificationKeyOrFile
<$> pStakePoolVerificationKeyOrFile Nothing
<*> pVrfVerificationKeyOrFile
<*> pPoolPledge
<*> pPoolCost
Expand Down Expand Up @@ -2692,7 +2709,7 @@ pVoterType =

-- TODO: Conway era include "normal" stake keys
pVotingCredential :: Parser (VerificationKeyOrFile StakePoolKey)
pVotingCredential = pStakePoolVerificationKeyOrFile
pVotingCredential = pStakePoolVerificationKeyOrFile Nothing

pVoteDelegationTarget :: Parser VoteDelegationTarget
pVoteDelegationTarget =
Expand Down
25 changes: 13 additions & 12 deletions cardano-cli/src/Cardano/CLI/EraBased/Options/Governance/Actions.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ pGovernanceActionNewInfoCmd era = do
NewInfoCmd
<$> pNetwork
<*> pGovActionDeposit
<*> pAnyStakeIdentifier
<*> pAnyStakeIdentifier Nothing
<*> pProposalUrl
<*> pProposalHashSource
<*> pFileOutDirection "out-file" "Path to action file to be used later on with build or build-raw "
Expand All @@ -71,7 +71,7 @@ pGovernanceActionNewConstitutionCmd era = do
NewConstitutionCmd
<$> pNetwork
<*> pGovActionDeposit
<*> pAnyStakeIdentifier
<*> pAnyStakeIdentifier Nothing
<*> pPreviousGovernanceAction
<*> pProposalUrl
<*> pProposalHashSource
Expand Down Expand Up @@ -99,11 +99,11 @@ pNewCommitteeCmd =
NewCommitteeCmd
<$> pNetwork
<*> pGovActionDeposit
<*> pAnyStakeIdentifier
<*> pAnyStakeIdentifier Nothing
<*> pProposalUrl
<*> pProposalHashSource
<*> many pAnyStakeIdentifier
<*> many ((,) <$> pAnyStakeIdentifier <*> pEpochNo "Committee member expiry epoch")
<*> many (pAnyStakeIdentifier (Just "remove-cc"))
Copy link
Contributor

@carbolymer carbolymer Sep 21, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I think we could use IsString here:

instance IsString a => IsString (Maybe a) where fromString = Just . fromString

this way we could just write "remove-cc" instead without Just

Copy link
Contributor Author

@smelc smelc Sep 21, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I'm not sure, generally speaking, this instance looks surprising to me (in other words: I'd be surprised to have in my context 🙂).

Also the occurrences of Just "prefix" are actually quite rare, most of this PR is just passing the prefix in context around.

<*> many ((,) <$> pAnyStakeIdentifier (Just "add-cc") <*> pEpochNo "Committee member expiry epoch")
<*> pRational "quorum" "Quorum of the committee that is necessary for a successful vote."
<*> pPreviousGovernanceAction
<*> pOutputFile
Expand All @@ -121,7 +121,7 @@ pGovernanceActionNoConfidenceCmd era = do
NoConfidenceCmd
<$> pNetwork
<*> pGovActionDeposit
<*> pAnyStakeIdentifier
<*> pAnyStakeIdentifier Nothing
<*> pProposalUrl
<*> pProposalHashSource
<*> pTxId "governance-action-tx-id" "Previous txid of `NoConfidence` or `NewCommittee` governance action."
Expand All @@ -130,10 +130,11 @@ pGovernanceActionNoConfidenceCmd era = do
)
$ Opt.progDesc "Create a no confidence proposal."

pAnyStakeIdentifier :: Parser AnyStakeIdentifier
pAnyStakeIdentifier =
asum [ AnyStakePoolKey <$> pStakePoolVerificationKeyOrHashOrFile
, AnyStakeKey <$> pStakeVerificationKeyOrHashOrFile
-- | The first argument is the optional prefix.
pAnyStakeIdentifier :: Maybe String -> Parser AnyStakeIdentifier
pAnyStakeIdentifier prefix =
asum [ AnyStakePoolKey <$> pStakePoolVerificationKeyOrHashOrFile prefix
, AnyStakeKey <$> pStakeVerificationKeyOrHashOrFile prefix
]

pGovernanceActionProtocolParametersUpdateCmd
Expand Down Expand Up @@ -321,10 +322,10 @@ pGovernanceActionTreasuryWithdrawalCmd era = do
TreasuryWithdrawalCmd
<$> pNetwork
<*> pGovActionDeposit
<*> pAnyStakeIdentifier
<*> pAnyStakeIdentifier Nothing
<*> pProposalUrl
<*> pProposalHashSource
<*> many ((,) <$> pAnyStakeIdentifier <*> pTransferAmt)
<*> many ((,) <$> pAnyStakeIdentifier Nothing <*> pTransferAmt) -- TODO we should likely pass a prefix here, becaus pAnyStakeIdentiefier is used twice
Copy link
Contributor

Choose a reason for hiding this comment

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

I agree, I think this should be included in this pr.

Copy link
Contributor Author

@smelc smelc Sep 21, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I would do a separate PR, because it's a different command. I can do this other PR soon after this one is merged.

<*> pFileOutDirection "out-file" "Output filepath of the treasury withdrawal."
)
$ Opt.progDesc "Create a treasury withdrawal."
Expand Down
2 changes: 1 addition & 1 deletion cardano-cli/src/Cardano/CLI/EraBased/Options/Governance/Vote.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,5 +52,5 @@ pAnyVote cOnwards =
pAnyVotingStakeVerificationKeyOrHashOrFile :: Parser AnyVotingStakeVerificationKeyOrHashOrFile
pAnyVotingStakeVerificationKeyOrHashOrFile =
asum [ AnyDRepVerificationKeyOrHashOrFile <$> pDRepVerificationKeyOrHashOrFile
, AnyStakePoolVerificationKeyOrHashOrFile <$> pStakePoolVerificationKeyOrHashOrFile
, AnyStakePoolVerificationKeyOrHashOrFile <$> pStakePoolVerificationKeyOrHashOrFile Nothing
]
2 changes: 1 addition & 1 deletion cardano-cli/src/Cardano/CLI/EraBased/Options/Node.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ pKeyHashVRF =
pNewCounter :: Parser (NodeCmds era)
pNewCounter =
NodeNewCounter
<$> pColdVerificationKeyOrFile
<$> pColdVerificationKeyOrFile Nothing
<*> pCounterValue
<*> pOperatorCertIssueCounterFile

Expand Down
6 changes: 3 additions & 3 deletions cardano-cli/src/Cardano/CLI/EraBased/Options/Query.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ pAllStakePoolsOrOnly = pAll <|> pOnly
, Opt.help "Query for all stake pools"
]
pOnly :: Parser (AllOrOnly [Hash StakePoolKey])
pOnly = Only <$> many pStakePoolVerificationKeyHash
pOnly = Only <$> many (pStakePoolVerificationKeyHash Nothing)

pQueryStakeSnapshot :: EnvCli -> Parser (QueryCmds era)
pQueryStakeSnapshot envCli =
Expand All @@ -187,7 +187,7 @@ pQueryPoolState envCli =
<$> pSocketPath envCli
<*> pConsensusModeParams
<*> pNetworkId envCli
<*> many pStakePoolVerificationKeyHash
<*> many (pStakePoolVerificationKeyHash Nothing)

pQueryTxMempool :: EnvCli -> Parser (QueryCmds era)
pQueryTxMempool envCli =
Expand Down Expand Up @@ -217,7 +217,7 @@ pLeadershipSchedule envCli =
<*> pConsensusModeParams
<*> pNetworkId envCli
<*> pGenesisFile "Shelley genesis filepath"
<*> pStakePoolVerificationKeyOrHashOrFile
<*> pStakePoolVerificationKeyOrHashOrFile Nothing
<*> pVrfSigningKeyFile
<*> pWhichLeadershipSchedule
<*> pMaybeOutputFile
Expand Down
6 changes: 3 additions & 3 deletions cardano-cli/src/Cardano/CLI/EraBased/Options/StakeAddress.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ pStakeAddressKeyHashCmd era = do
$ subParser "key-hash"
$ Opt.info
( StakeAddressKeyHashCmd w
<$> pStakeVerificationKeyOrFile
<$> pStakeVerificationKeyOrFile Nothing
<*> pMaybeOutputFile
)
$ Opt.progDesc "Print the hash of a stake address key"
Expand Down Expand Up @@ -121,7 +121,7 @@ pStakeAddressStakeDelegationCertificateCmd era = do
$ Opt.info
( StakeAddressStakeDelegationCertificateCmd w
<$> pStakeIdentifier
<*> pStakePoolVerificationKeyOrHashOrFile
<*> pStakePoolVerificationKeyOrHashOrFile Nothing
<*> pOutputFile
)
$ Opt.progDesc
Expand All @@ -140,7 +140,7 @@ pStakeAddressStakeAndVoteDelegationCertificateCmd era = do
$ Opt.info
( StakeAddressStakeAndVoteDelegationCertificateCmd w
<$> pStakeIdentifier
<*> pStakePoolVerificationKeyOrHashOrFile
<*> pStakePoolVerificationKeyOrHashOrFile Nothing
<*> pVoteDelegationTarget
<*> pOutputFile
)
Expand Down
Loading