Time moves during fork choice spec test

[hcnam]

In the fork choice spec test, it uses tick to set the internal time of Prysm. However, Pyrsm’s Tick function resets the genesis time to now()-tick and adjusts the slot to the appropriate value. Therefore, now() is taken as time.Now(), which represents the real time and changes with the program’s flow. This results in a potential unintended slot change during a test (e.g., when tick=(ticks_in_new_slot - 1)), which may or may not affect the forkchoice decision and non-deterministically lead to a different result (PASS/FAIL), which does not happen in other clients like Lighthouse, Teku, or Nimbus.

For example, when steps.yaml defined as below, and attestations were

- {tick: 0}
- {tick: 12}
   # block_0xAAA was proposed
- {tick: 24}
   # block_0xBBB was proposed
- {tick: 36}
   # block_0xCCC was proposed
- {tick: 48}
   # block_0xDDD was proposed
- {tick: 59}
- {block: block_0xAAA}   # Receive all blocks right before next slot
- {block: block_0xBBB}
- {block: block_0xCCC}
- {block: block_0xDDD}
- {attestation: attestation_0x000}   # Assume attestations are from future (from right next slot)
- {attestation: attestation_0x111}   
- {attestation: attestation_0x222}
- {attestation: attestation_0x333}
- {attestation: attestation_0x444}
- {attestation: attestation_0x555}
- {tick: 60}
…

Expect: All attestations from 0x000 to 0x555 will be rejected (due to onAttestation itself is stateless in spec test)
Result: Sometimes, some of attestations are accepted and affect to weight (due to time passed, became next slot)

Reference:
In here and here onTick is technically implemented according to the specs, but in fact when prysmTime.Now() is called it shows the real current time, which will change as time goes. This doesn’t happen in Teku, Nimbus or Lighthouse

[potuz]

This is correct, however, all spectests have ticks for all slot boundaries and thus this error can not be triggered. This is a low urgency/no urgency issue as if the situation happens by definition it will be caught during spectesting and it will need to be addressed accordingly (most probably modifying the spectest). I do not know how is this handled in other clients, my guess as to why this isn't triggered in other clients is that they run on_tick backwards if they get a tick and realized that they had to call the previous ticks for the slot boundary. Prysm does not do this on its codebase because we run on_tick clock based instead of event based. Testing the scenario you are depicting would require modifying our core functions to adapt to the test itself or to have a separate path to deal only with tests, both are not something we are willing to do lightly as it is more important to test the actual production code. I'm closing this issue with this comment, but feel free to open it if you think there are viable alternatives that would not touch our core code.