block.timestamp compatible Checkpoints

[jaketimothy]

🧐 Motivation

Current implementation of Checkpoints is built for block.number based checkpointing. Would love to generalize the utility to support block.timestamp.

Would not be surprised if this has been discussed internally wrt timestamp based Governor and Votes features. I see this as laying some groundwork to enable those features.

📝 Details

I see the following limitations to address

• uint32 creates unix epoch overflow in year 2106. While not urgent, uint64 would future-proof this. uint64 is also compatible with Timers, increasing composability.
• getAtBlock, getAtProbablyRecentBlock, and push use block.number internally.

Would this help next-v5.0?

With #2962 coming, is this going too far?

    struct Checkpoint {
        Timer _timer;
        uint192 _value;
    }

[Amxx]

Hello @jaketimothy

As you can see, the current checkpoint implementation include the Trace224 and Trace160 structures. They already provide the checkpoint mechanisms in a way that is not linked to "block.number".

Before we can have block.timestamp based governor we need to figure out the way Governor and Tokens are going to work together on using the same reference. My personal opinion is that the governor should not ever lookup the time, and should ask the token for that information. That way the governor would be oblivious, and the clock would be set by the token.

This is what I started to formalize in EIP-5805.

The uint32 limitation doesn't feel like its an actual issue. I honestly don't think that 80 years is anything we should be concerned with. Ethereum did not exist 10 years ago, and the computers from 80 years ago where electromechanical enigma-braking machines. I'm sure we all want ethereum still be relevant in 80 years ... but if the name ethereum is, the reality it represent will definitely not be what we have right now, and I'd argue it would be foolish to build contracts today hoping the runtime they are based on is still the same in 80 years. (just try to install/start a windows game from the late 1990's and you'll see what I mean)

for the getAtBlock, getAtProbablyRecentBlock, etc, functions. we should use the new Trace224 with a lookup to the public now() function defined by EIP-5805. Anyone will be able to "just" overidde the now() function to use timestamp, block numbers, or anything they want ... and it should be good.

So the next step are:

• clean up EIP-5805
• have it "accepted" (not exactly sure what that means, finalization will take a long time, we may want to move forward faster than that)
• update Votes to implement EIP-5805
• update Governor to use EIP-5805 from the voting contract.

[jaketimothy]

Before we can have block.timestamp based governor we need to figure out the way Governor and Tokens are going to work together on using the same reference. My personal opinion is that the governor should not ever lookup the time, and should ask the token for that information. That way the governor would be oblivious, and the clock would be set by the token.

Agreed. Since the token must be time aware, this design isolates timekeeping.

This is what I started to formalize in ethereum/EIPs#5805.

Thank you for writing this EIP!

The uint32 limitation doesn't feel like its an actual issue...

Fair! There was no such thing as unix epoch 80 years ago... Our grandchildren can sort it out.

for the getAtBlock, getAtProbablyRecentBlock, etc, functions. we should use the new Trace224 with a lookup to the public now() function defined by EIP-5805. Anyone will be able to "just" overidde the now() function to use timestamp, block numbers, or anything they want ... and it should be good.

I propose this become the scope of this feature request as a part of "update Votes to implement EIP-5805".

[jaketimothy]

What do you favor between passing in a reference to now() vs removing the now() check? Something like

function getAtTime(Trace224 storage self, IERC5805Now timekeeper, uint256 timepoint) internal view returns (uint256) {
    require(timepoint < timekeeper.now(), "Checkpoints: timepoint in past");
    uint32 key = SafeCast.toUint32(timepoint);

vs

function getAtTime(Trace224 storage self, uint256 timepoint) internal view returns (uint256) {
    uint32 key = SafeCast.toUint32(timepoint);

[frangio]

I believe this was fixed in #3934.