Skip to content

Checkpoint timestamps come from filesystem mtime but are compared against block time #4563

Description

@PastaPastaPasta

load_current_checkpoints takes each checkpoint's timestamp from the directory's filesystem modification time:

// packages/rs-drive/src/open/load_current_checkpoints.rs
let timestamp_ms: TimestampMillis = std::fs::metadata(&path)
    .and_then(|m| m.modified())

and should_checkpoint compares that against block time:

// packages/rs-drive-abci/src/execution/platform_events/block_end/should_checkpoint/v0/mod.rs
let most_recent_checkpoint_interval_time =
    block_time - block_time % checkpoint_interval_milliseconds;

let should_checkpoint = match current_checkpoints_guard.last_key_value() {
    None => true,
    Some((_height, checkpoint_info)) => {
        checkpoint_info.timestamp_ms < most_recent_checkpoint_interval_time
            && block_time >= most_recent_checkpoint_interval_time
    }
};

Wall clock on one side, consensus time on the other.

Effect

On a node following the tip the two are within a block or so of each other and the comparison behaves as intended.

On a node replaying history they are months apart. After any restart the loaded timestamps are ~now while block_time is far in the past, so checkpoint_info.timestamp_ms < most_recent_checkpoint_interval_time is false and the node silently stops checkpointing until block time catches up to the moment of the restart.

Note this is not the same as the fix in #4553, which deliberately skips checkpoints while replaying. This is about a node that is supposed to checkpoint and does not, and it applies to a restarted node at any height where block time trails the restart.

How it was found

Measuring #4553. The first A/B showed no difference between skipping and not skipping, because the snapshot the two arms started from carried checkpoint directories whose mtimes were "now" — so neither arm checkpointed. Removing those directories, which is the state a node syncing from genesis is actually in, reproduced the 12.1 ms/block difference immediately.

Why this is an issue rather than a PR

The fix needs a decision about where the timestamp comes from, and the layering is awkward:

  • The natural source is the checkpoint's own platform_state.bin, whose last_committed_block_time_ms is exactly the right value — but load_current_checkpoints lives in rs-drive, which does not know about PlatformState.
  • Alternatives are a sidecar timestamp file, encoding the block time in the directory name (currently the block height), or having drive-abci fix up the timestamps after load, where checkpoint_platform_states is already populated.

Each is a small on-disk format decision, and picking one unilaterally on the back of a performance sweep seemed wrong.


🤖 Posted autonomously by Claude on behalf of pasta.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions