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

Fix qKesKesKeyExpiry to not always be null #4909

Merged
merged 1 commit into from
Mar 30, 2023
Merged

Fix qKesKesKeyExpiry to not always be null #4909

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
2 changes: 2 additions & 0 deletions cardano-cli/ChangeLog.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@

- Query protocol parameters from the node in the `transaction build` command ([PR 4431](https://github.com/input-output-hk/cardano-node/pull/4431))

- Fix `qKesKesKeyExpiry` in `kes-period-info` ([PR 4909](https://github.com/input-output-hk/cardano-node/pull/4909))

## 1.35.3 -- August 2022

- Update build and build-raw commands to accept simple reference minting scripts (#4087)
Expand Down
26 changes: 21 additions & 5 deletions cardano-cli/src/Cardano/CLI/Shelley/Run/Query.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -448,7 +448,7 @@ runQueryKesPeriodInfo (AnyConsensusModeParams cModeParams) network nodeOpCertFil
eraHistory <- lift (queryNodeLocalState localNodeConnInfo Nothing eraHistoryQuery)
& onLeft (left . ShelleyQueryCmdAcquireFailure)

let eInfo = toEpochInfo eraHistory
let eInfo = toTentativeEpochInfo eraHistory


-- We get the operational certificate counter from the protocol state and check that
Expand Down Expand Up @@ -515,13 +515,13 @@ runQueryKesPeriodInfo (AnyConsensusModeParams cModeParams) network nodeOpCertFil
opCertNodeAndOnDiskCounters o Nothing = OpCertNoBlocksMintedYet o

opCertExpiryUtcTime
:: EpochInfo (Either Text)
:: Tentative (EpochInfo (Either Text))
-> GenesisParameters
-> OpCertEndingKesPeriod
-> Maybe UTCTime
opCertExpiryUtcTime eInfo gParams (OpCertEndingKesPeriod oCertExpiryKesPeriod) =
let time = epochInfoSlotToUTCTime
eInfo
(tentative eInfo)
(SystemStart $ protocolParamSystemStart gParams)
(fromIntegral $ oCertExpiryKesPeriod * fromIntegral (protocolParamSlotsPerKESPeriod gParams))
in case time of
Expand Down Expand Up @@ -576,7 +576,7 @@ runQueryKesPeriodInfo (AnyConsensusModeParams cModeParams) network nodeOpCertFil
createQueryKesPeriodInfoOutput
:: OpCertIntervalInformation
-> OpCertNodeAndOnDiskCounterInformation
-> EpochInfo (Either Text)
-> Tentative (EpochInfo (Either Text))
-> GenesisParameters
-> O.QueryKesPeriodInfoOutput
createQueryKesPeriodInfoOutput oCertIntervalInfo oCertCounterInfo eInfo gParams =
Expand Down Expand Up @@ -1415,9 +1415,25 @@ queryResult eAcq = pure eAcq

toEpochInfo :: EraHistory CardanoMode -> EpochInfo (Either Text)
toEpochInfo (EraHistory _ interpreter) =
hoistEpochInfo (first (Text.pack . show ) . runExcept)
hoistEpochInfo (first (Text.pack . show) . runExcept)
$ Consensus.interpreterToEpochInfo interpreter

-- | A value that is tentative or produces a tentative value if used. These values
-- are considered accurate only if some future event such as a hard fork does not
-- render them invalid.
newtype Tentative a = Tentative { tentative :: a } deriving (Eq, Show)
Copy link
Contributor

@Jimbo4350 Jimbo4350 Mar 7, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

What other values do we expect to be tentative besides EpochInfo?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Potentially anything derived from Tentative EpochInfo is a candidate.


-- | Get an Epoch Info that computes tentative values. The values computed are
-- tentative because it uses an interpreter that is extended past the horizon.
-- This interpreter will compute accurate values into the future as long as a
-- a hard fork does not happen in the intervening time. Those values are thus
-- "tentative" because they can change in the event of a hard fork.
Copy link
Contributor

Choose a reason for hiding this comment

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

Just to clarify my understanding, they can change in the event of a hardfork occurring within the horizon?

Copy link
Contributor Author

@newhoggy newhoggy Mar 29, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

They can change in the event of a hardfork occurring after the horizon. Normally the interpreter refuses to give an answer beyond the horizon so we construct a different interpreter that ignores the horizon.

Values before the horizon still won't change.

However, because the type allows for values beyond the horizon to change, it is annotated as Tentative regardless of whether the value can change or not.

toTentativeEpochInfo :: EraHistory CardanoMode -> Tentative (EpochInfo (Either Text))
toTentativeEpochInfo (EraHistory _ interpreter) =
Tentative
$ hoistEpochInfo (first (Text.pack . show) . runExcept)
$ Consensus.interpreterToEpochInfo (Consensus.unsafeExtendSafeZone interpreter)

obtainLedgerEraClassConstraints
:: ShelleyLedgerEra era ~ ledgerera
=> Api.ShelleyBasedEra era
Expand Down