-
Notifications
You must be signed in to change notification settings - Fork 16
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
Integrate latest ledger, consensus and api for 8.6.0 #385
Changes from all commits
a15d1cf
97bbd95
0672d09
87e92b1
022b657
4303fbf
5b2d25d
486d011
eda3544
10440c0
f1928f9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2077,30 +2077,62 @@ pInvalidBefore = fmap SlotNo $ asum | |
] | ||
] | ||
|
||
pInvalidHereafter :: Parser SlotNo | ||
pInvalidHereafter = | ||
pLegacyInvalidHereafter :: Parser SlotNo | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But we also have legacy commands that don't understand eras the same way. In the legacy era, options are always there regardless of era and these need validation. This is why these parse plain types, not era sensitive types. This means we may receive values from the CLI parser that are invalid for our era and hence we need validation. |
||
pLegacyInvalidHereafter = | ||
fmap SlotNo $ asum | ||
[ Opt.option (bounded "SLOT") $ mconcat | ||
[ Opt.long "invalid-hereafter" | ||
, Opt.metavar "SLOT" | ||
, Opt.help "Time that transaction is valid until (in slots)." | ||
] | ||
, Opt.option (bounded "SLOT") $ mconcat | ||
[ Opt.long "upper-bound" | ||
, Opt.metavar "SLOT" | ||
, Opt.help $ mconcat | ||
[ "Time that transaction is valid until (in slots) " | ||
, "(deprecated; use --invalid-hereafter instead)." | ||
[ Opt.option (bounded "SLOT") $ mconcat | ||
[ Opt.long "invalid-hereafter" | ||
, Opt.metavar "SLOT" | ||
, Opt.help "Time that transaction is valid until (in slots)." | ||
] | ||
, Opt.option (bounded "SLOT") $ mconcat | ||
[ Opt.long "upper-bound" | ||
, Opt.metavar "SLOT" | ||
, Opt.help $ mconcat | ||
[ "Time that transaction is valid until (in slots) " | ||
, "(deprecated; use --invalid-hereafter instead)." | ||
] | ||
, Opt.internal | ||
] | ||
, Opt.option (bounded "SLOT") $ mconcat | ||
[ Opt.long "ttl" | ||
, Opt.metavar "SLOT" | ||
, Opt.help "Time to live (in slots) (deprecated; use --invalid-hereafter instead)." | ||
, Opt.internal | ||
] | ||
, Opt.internal | ||
] | ||
, Opt.option (bounded "SLOT") $ mconcat | ||
[ Opt.long "ttl" | ||
, Opt.metavar "SLOT" | ||
, Opt.help "Time to live (in slots) (deprecated; use --invalid-hereafter instead)." | ||
, Opt.internal | ||
] | ||
] | ||
|
||
pInvalidHereafter :: () | ||
=> CardanoEra era | ||
-> Parser (TxValidityUpperBound era) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In era-based commands we want to bring the era-sensitive types closer to the CLI. This means we parse This allows the parser to determine if the options are valid for the given era. For example In the Byron era, |
||
pInvalidHereafter = | ||
caseByronOrShelleyBasedEra | ||
(pure . TxValidityNoUpperBound) | ||
(\eon -> | ||
fmap (TxValidityUpperBound eon) $ asum | ||
[ fmap (Just . SlotNo) $ Opt.option (bounded "SLOT") $ mconcat | ||
[ Opt.long "invalid-hereafter" | ||
, Opt.metavar "SLOT" | ||
, Opt.help "Time that transaction is valid until (in slots)." | ||
] | ||
, fmap (Just . SlotNo) $ Opt.option (bounded "SLOT") $ mconcat | ||
[ Opt.long "upper-bound" | ||
, Opt.metavar "SLOT" | ||
, Opt.help $ mconcat | ||
[ "Time that transaction is valid until (in slots) " | ||
, "(deprecated; use --invalid-hereafter instead)." | ||
] | ||
, Opt.internal | ||
] | ||
, fmap (Just . SlotNo) $ Opt.option (bounded "SLOT") $ mconcat | ||
[ Opt.long "ttl" | ||
, Opt.metavar "SLOT" | ||
, Opt.help "Time to live (in slots) (deprecated; use --invalid-hereafter instead)." | ||
, Opt.internal | ||
] | ||
, pure Nothing | ||
] | ||
) | ||
|
||
pTxFee :: Parser Lovelace | ||
pTxFee = | ||
|
@@ -2560,7 +2592,6 @@ pProtocolParametersUpdate = | |
<*> optional pPoolInfluence | ||
<*> optional pMonetaryExpansion | ||
<*> optional pTreasuryExpansion | ||
<*> pure Nothing | ||
<*> pure mempty | ||
<*> optional pExecutionUnitPrices | ||
<*> optional pMaxTxExecutionUnits | ||
|
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.
Have a look at how the governance action parsers handle anchors. E.g
In particular the
proposalUrl
andproposalHashSource
fields.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.
@Jimbo4350 So you prefer to flatten these two fields , url and hash.
But what to do about
ProposalUrl
andProposalHashSource
which are not accurately describing this Anchor, no - since it's not related to proposal, but to resignation.. Create new types for them?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.
So you can construct an
Anchor
as follows: https://github.com/input-output-hk/cardano-cli/blob/e86aa60f4481dcdc035d2173d61090bd284ad089/cardano-cli/src/Cardano/CLI/EraBased/Run/Governance/Actions.hs#L98There 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.
Hm, I don't understand exactly what the suggestion is (probably because I'm not familiar with the codebase).
Would you like me to represent Anchor in
GovernanceCommitteeCreateColdKeyResignationCertificateCmdArgs
as two different fields, similar to the example you gave above, so as :instead of the field that I have added:
?
Did i understand correctly?
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.
Yes exactly 👍I see what you're saying now.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.
Both of them will have to be
Maybe
I suppose, since the field is optional. I hope it won't be too messy, if say one is there and the other one not 🤔