Make UTCTime test generator generate values without leap seconds

[carbolymer]

Description

This PR changes UTCTime generator, which is only used in genKesPeriodInfoOutput generator for round-trip testing of JSON decoding/encoding of the QueryKesPeriodInfoOutput type.

Previous UTCTime generator was using SystemTime and was generating values in the range [0; 2^32-1] for representing nanoseconds part of the time. The maximum allowed value of nanoseconds in SystemTime is 2E9-1, where values from range (1E9;2E9-1] are used to represent leap seconds. Therefore, the previous generator was able to generate values above 2E9-1 , introducing this way extra leap seconds (up to 4), where UTC standard supports only one leap second (see ITU recommendation for UTC).

Unfortunately, in the chain of functions generate Word32 -> create SystemTime -> convert to UTCTime -> encode into JSON -> decode from JSON this leap seconds condition is checked only at the JSON decoding stage, which results in a test failure there.

Observed failure: https://ci.zw3rk.com/build/2171231/nixlog/2

This fix changes the generation method to use unix time to create a valid UTC timestamp.

See also: haskell/time#242

Checklist

• Commit sequence broadly makes sense and commits have useful messages
• New tests are added if needed and existing tests are updated. These may include:
  • golden tests
  • property tests
  • roundtrip tests
  • integration tests
    See Runnings tests for more details
• Any changes are noted in the CHANGELOG.md for affected package
• The version bounds in .cabal files are updated
• CI passes. See note on CI. The following CI checks are required:
  • Code is linted with hlint. See .github/workflows/check-hlint.yml to get the hlint version
  • Code is formatted with stylish-haskell. See .github/workflows/stylish-haskell.yml to get the stylish-haskell version
  • Code builds on Linux, MacOS and Windows for ghc-8.10.7 and ghc-9.2.7
• Self-reviewed the diff

[njd42]

My concern is that this is not how the Cardano system interprets time. We made the decision to stick with UTC (and live with the issues of leap seconds) rather than some form of strict montonic clock (such as TAI or some offset from it).

We are using UTC “on chain” and we should not use other clocks in testing - otherwise other issues will creep in like TTLs not working as expected and/or nodes creating blocks early/late as leap second effect accumulate

We just have to accept (and recognise as false positives) related test failures. So this is not way that this issue can be resolved - sorry to be bearer of bad news

[njd42]

I wanted to stop this being merged (see comment above) - I'm not a code owner, so I converted it to a DRAFT so we can discuss this before it ends up in master (which, as you can see from my comment above) I don't think it should @disassembler @Jimbo4350

[dcoutts]

I can't see that this has anything to do with leap seconds, as the PR description says.

Can you clarify?

As far as I can see it's just replacing a bogus generator with a sane one.

[newhoggy]

LGTM

Note this generator is used to generate a time to do a round trip serialisation/deserialisation test and not for any timing related tests and we could not use this generator for timing related tests anyway because it generates values for all possible times and we would not be able to do timing tests with values from such a large range.

[njd42]

GIven that we've resolved this not to be about UTC v other time models but one of an incorrect generator of values, now made 'sane'. LGTM

[carbolymer]

I've updated the description with your remarks.
@njd42

We are using UTC “on chain” and we should not use other clocks in testing - otherwise other issues will creep in like TTLs not working as expected and/or nodes creating blocks early/late as leap second effect accumulate

This generator is only using for round-trip json serialization/deserialization, so it shouldn't affect the rest of the compontents.

@dcoutts

I can't see that this has anything to do with leap seconds, as the PR description says.

The range (1E9; 2E9) is used to represent a leap second. The values above 2E9 would indicate more than one second, which is not in the UTC standard, which results in the test failure, because deserialization relies on the fact that there could be only one leap second.