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

protocolInfoCardano: only allow to change the TriggerHardForkAtEpoch HF trigger #1281

Closed
amesgen opened this issue Oct 11, 2024 · 0 comments · Fixed by #1282
Closed

protocolInfoCardano: only allow to change the TriggerHardForkAtEpoch HF trigger #1281

amesgen opened this issue Oct 11, 2024 · 0 comments · Fixed by #1282
Assignees

Comments

@amesgen
Copy link
Member

amesgen commented Oct 11, 2024

The goal of this ticket is to further (#319) improve protocolInfoCardano (by simplifying and improving misuse resistance) for the HFC triggers.


Callers of protocolInfoCardano need to provide CardanoHardForkTriggers

newtype CardanoHardForkTriggers = CardanoHardForkTriggers {
getCardanoHardForkTriggers :: NP (K TriggerHardFork) (CardanoShelleyEras StandardCrypto)
}

(we also provide a more convenient pattern synonym) which is used by the HFC to determine when (i.e. at which epoch) to hardfork into the specific era.

Concretely, there are three possible cases:

-- | The trigger condition that will cause the hard fork transition.
--
-- This type is only intended for use as part of a
-- 'Ouroboros.Consensus.Ledger.Basics.LedgerCfg', which means it is "static":
-- it cannot change during an execution of the node process.
data TriggerHardFork =
-- | Trigger the transition when the on-chain protocol major version (from
-- the ledger state) reaches this number.
--
-- Note: The HFC logic does not require the trigger version for one era to
-- be the successor of the trigger version for the previous era.
TriggerHardForkAtVersion !Word16
-- | For testing only, trigger the transition at a specific hard-coded
-- epoch, irrespective of the ledger state.
| TriggerHardForkAtEpoch !EpochNo
-- | Ledger states in this era cannot determine when the hard fork
-- transition will happen.
--
-- It's crucial to note that this option does /not/ imply that "the era
-- will never end". Instead, the era cannot end within this node process
-- before it restarts with different software and/or configuration for this
-- era.
| TriggerHardForkNotDuringThisExecution

  • TriggerHardForkNotDuringThisExecution is currently only used for the last era, and it shouldn't be used for anything else (if one wants to disable an era, one can use the max major protocol version check by setting cardanoProtocolVersion appropriately).
  • TriggerHardForkAtVersion should always be set to the version mandated by Ledger via its eraProtVerLow. Any other configuration is not mainnet-compatible, and also somewhat dubious as Ledger expects the ledger protocol versions to be in the correct range for any given era. I am also not aware of any case (other than our tests, which isn't fundamental) that set these to different values (also see Slack).
  • TriggerHardForkAtEpoch is often used to skip to a specific era immediately at the start (see Determine the downstream reliance on TriggerHardForkAtEpoch #395 for more use cases). This needs to stay configurable, as the era to start from differs between different (local) testnets.

Therefore, only TriggerHardForkAtEpoch actually needs to be configurable; the default should always be TriggerHardForkAtVersion for the corresponding Ledger version.

One concrete idea would be to change CardanoHardForkTriggers from the above to

 newtype CardanoHardForkTriggers = CardanoHardForkTriggers { 
     getCardanoHardForkTriggers :: NP (Maybe :.: K EpochNo) (CardanoShelleyEras StandardCrypto) 
   } 

(or using OptNP) where supplying

  • Just epochNo would result in TriggerHardForkAtEpoch epochNo and
  • Nothing would result in TriggerHardForkAtVersion (L.getVersion (L.eraProtVerLow @era))

where ShelleyBlock proto era is the current index.


See IntersectMBO/cardano-node#6009 (comment) for a recent example where this would have helped.

@amesgen amesgen self-assigned this Oct 14, 2024
github-merge-queue bot pushed a commit that referenced this issue Oct 16, 2024
Closes #1281

Instead of letting the user provide several `TriggerHardFork`s, only let
them provide `CardanoHardForkTrigger`s, a restricted version that should
make `protocolInfoCardano` more straightforward and less error-prone.

```haskell
data CardanoHardForkTrigger blk =
    -- | Trigger the hard fork when the ledger protocol version is updated to
    -- the default for that era (@'L.eraProtVerLow' \@('ShelleyBlockLedgerEra'
    -- blk)@). Also see 'TriggerHardForkAtVersion'.
    CardanoTriggerHardForkAtDefaultVersion
  |
    -- | Trigger the hard fork at the given epoch. For testing only. Also see
    -- 'TriggerHardForkAtEpoch'.
    CardanoTriggerHardForkAtEpoch EpochNo
```

It is (intentionally) no longer possible to directly (though still
manually, also see the changelog entry) to use a non-default version
trigger. However, this feature was used in the Cardano ThreadNet test
(as Byron had an intra-era HF), which we resolve by modifying the
initial Byron protocol version (see the corresponding Haddocks).

In the node, this will result in the removal of the (unused)
`TestXxxHardForkAtVersion` config fields.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: ✅ Done
Development

Successfully merging a pull request may close this issue.

1 participant