fix: null rounds: pass correct timestamp to the FVM.

[raulk]

Related Issues

Fixes filecoin-project/ref-fvm#1632.

Proposed Changes

This PR calculates the correct timestamp for a null round and passes it to the FVM.

I had two choices here:

1. Calculate from genesis, or
2. Calculate from the parent tipset.

I chose the former because I consider it safer/simpler. it requires exactly two store lookups (one to the metadata store, one to the chain blockstore).

The latter should be OK as well, and may possibly require no disk lookups at all, since the parent tipset is likely to be the heaviest tipset, but it's harder to reason about and the GetTipsetByHeight interface is a bit messy.

Happy to switch if over if folks prefer (2).

Additional Info

Checklist

Before you mark the PR ready for review, please make sure that:

• Commits have a clear commit message.
• PR title is in the form of of <PR type>: <area>: <change being made>
  • example: fix: mempool: Introduce a cache for valid signatures
  • PR type: fix, feat, build, chore, ci, docs, perf, refactor, revert, style, test
  • area, e.g. api, chain, state, market, mempool, multisig, networking, paych, proving, sealing, wallet, deps
• New features have usage guidelines and / or documentation updates in
  • Lotus Documentation
  • Discussion Tutorials
• Tests exist for new functionality or change in behavior
• CI is green

[magik6k]

So I think this doesn't have consensus impact, and if it does we'll pick that up running on mainnet pretty quick.

Tests seem validly angry, may need to change to getting timestamp from parent tipset

Failed
=== RUN   TestConformance/reward/reward--ok-miners-awarded-no-premiums--tape.json/tape
    runner.go:200: failed to apply tipset 0: failed to get genesis when backfilling null rounds: datastore: key not found
        --- FAIL: TestConformance/reward/reward--ok-miners-awarded-no-premiums--tape.json/tape (0.00s)

[magik6k]

(rebase should fix test-itest-fevm fails)

[raulk]

That test vector is generated here. That's a synthetic test vector that advances the chain artificially by adding null rounds, to then verify that the correct reward was dispensed.

Before, processing a null round didn't require accessing chain data; now it does. This would still be a problem even if we accessed just the parent, because that suite does not produce any chain data. (Similarly those suites wouldn't work with lookbacks).

[raulk]

I'm afraid the only solution I can think of is to skip that test vector :-/

[raulk]

So I think this doesn't have consensus impact, and if it does we'll pick that up running on mainnet pretty quick.

How often do we have null rounds on mainnet? We'll definitely pick up regressions on non-null rounds quickly, but not sure about the former.

[Kubuxu]

Around 0.6% of epochs are expected to be null rounds.

[arajasek]

I'm afraid the only solution I can think of is to skip that test vector :-/

Yeah, I think skipping that test vector is fine.

How often do we have null rounds on mainnet? We'll definitely pick up regressions on non-null rounds quickly, but not sure about the former.

Null rounds are pretty common on mainnet, but @magik6k is right that this has no consensus impact today. Definitely no impact today (timestamps aren't provided to the VM in nv17 and earlier), and cron doesn't currently use the timestamp.