Skip to content

UpdateTime() needs to account for timewarp fix on testnet4  #30614

@Sjors

Description

@Sjors

In (miner.cpp) we set the block (template) timestamp as follows:

pblock->nTime = TicksSinceEpoch<std::chrono::seconds>(NodeClock::now());
...
UpdateTime(pblock, ...)


int64_t UpdateTime(CBlockHeader* pblock, ...)
{
    int64_t nOldTime = pblock->nTime;
    int64_t nNewTime{std::max<int64_t>(pindexPrev->GetMedianTimePast() + 1, TicksSinceEpoch<std::chrono::seconds>(NodeClock::now()))};

    if (nOldTime < nNewTime) {
        pblock->nTime = nNewTime;
    }

In other words, we pick the system time, and if needed bump it to MTP + 1.

This doesn't take the timewarp mitigation for testnet4 into account. The first block must have a timestamp no less than 7200 seconds (2 hours) before the last block. There's discussion about lowering that to 600 from the original proposal.

An adversarial miner could set the timestamp 2 hours in the future. The current code would then set the next block at the current time. With the current value of 7200 this should not cause an issue, even if we immediately find a block. But it seems safer to explicitly account for this and increase the timestamp if needed, especially if we ever lower the value.

We should also add a functional test that uses fake time to test the limit case: we don't want to end up accepting a block exactly 2 hours in the future and then failing to generate a template because there's no possible valid timestamp due to some off-by-one error. (I don't think this can happen though, haven't thought it through)

See also
bitcoin/bips#1658 (comment)

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions