passing a singletonSlotLengths to realBlockchainTime is almost certainly incorrect if an adopted update proposal ever changed the slot duration protocol parameter

[nfrisby]

While preparing for #1514 (add update proposals to RealPBFT tests), I noticed two things.

First, the slot duration is one of the protocol parameters that an update proposal can change -- see ppuSlotDuration in the cardano-ledger repo.

Second, the current Byron implementation via ouroboros-consensus and ouroboros-consensus-byron implicitly assume that no update has ever changed the slot duration.

Node.run includes

    realBlockchainTime ...
        (nodeStartTime (Proxy @blk) cfg)
        (focusSlotLengths (knownSlotLengths (configBlock cfg)))

but knownSlotLengths ignores any adopted update proposals, whether they be in the anticipatable future (see Issue #1637) or in the known past (this Issue).

instance LedgerDerivedInfo ByronBlock where
  knownSlotLengths =
        singletonSlotLengths
      . slotLengthFromMillisec
      . (fromIntegral :: Natural -> Integer)
      . CC.ppSlotDuration
      . Gen.configProtocolParameters
      . byronGenesisConfig

I suppose the above could possibly achieve the correct behavior if the nodeStartTime were somewhat abused and referred to the time at which that latest slot length was adopted, but that's not what's currently happening (unless I'm taking the name GenesisData too literally; it is data from the genesis block, right?).

  nodeStartTime             = const
                            $ SystemStart
                            . Genesis.gdStartTime
                            . Genesis.configGenesisData
                            . byronGenesisConfig
                            . configBlock

---

Therefore, unless no update proposal will ever change the slot duration, we likely need some changes. Two more notable observations.

• In order for knownSlotLengths to be fully correct, it would need an up-to-date immutable database and to read the entire history therein of updates to the slot duration protocol parameter. (If a node realizes it started with a partial history, then it may need to restart again after syncing more.)

• The slot duration seems to be the only updatable parameters that, in the current implementation, requires a node restart in order to adopt a new value. (Edit:) Also, the max block size insomuch as it affects the max size of the mempool, requires a restarts (what else is there?). For all other parameters, I've only seen their values read from the latest ChainValidationState as needed (they're used in cardano-ledger when validating various things).

If we alter the node so that it need not restart on changes to slot duration, would update proposals that only change protocol parameters (and not software version) then never require the node to restart? (Edit: No. I've found at least one more value that's cached on start-up: the mempool's max size may depend on the current value of ppMaxBlockSize.)

[nfrisby]

Ah. I have now found IntersectMBO/cardano-ledger#642

However, it's not quite clear to me what the ultimately intended correct behavior is. For example, has the parameter ever changed in the mainnet history? Should it ever be able to change in the future? In the remaining Byron era? In the Shelley era? Etc.

[edsko]

Closing this as WONTFIX, as this value is not actually updatable through the update system. You are nonetheless correct that passing singletonSlotLengths is incorrect, as the slot lengths can change as part of hard forks (and will: going down from 20secs in Byron to 2 or even 1 in Shelley), but it's not otherwise updatable. The same goes for the epoch length. I am addressing both of those things in my work on the hard fork combinator however, and will only become relevant for the Shelley mainnet release.