Skip to content

fix: compare serialized timestamps in seconds#58

Closed
thepastaclaw wants to merge 2 commits into
PastaPastaPasta:codex/backport-25786from
thepastaclaw:fix-7459-chrono-overflow
Closed

fix: compare serialized timestamps in seconds#58
thepastaclaw wants to merge 2 commits into
PastaPastaPasta:codex/backport-25786from
thepastaclaw:fix-7459-chrono-overflow

Conversation

@thepastaclaw

Copy link
Copy Markdown

Issue being fixed or feature implemented

The Dash-specific adaptation in dashpay#7459 compares serialized int64_t timestamps in seconds with native-resolution NodeClock::time_point values. The implicit common-duration conversion scales peer-controlled values to microseconds or nanoseconds and can overflow signed integers before the comparison.

What was done?

  • Normalize adjusted time to whole seconds before comparing it with serialized spork and governance timestamps.
  • Make the governance vote-rate boundary check safe at extreme clock values without subtracting potentially unrepresentable durations.
  • Preserve existing wire and disk timestamp types and encodings.
  • Add focused regression coverage for signed extreme spork timestamps and a validly signed governance vote with an extreme future timestamp.

How Has This Been Tested?

Tested on macOS/aarch64 with an undefined-behavior-sanitized build:

  • CCACHE_DISABLE=1 make -C src -j15 test/test_dash
  • UBSAN_OPTIONS=halt_on_error=1:print_stacktrace=1 ./src/test/test_dash --run_test=chrono_overflow_tests --log_level=message
  • UBSAN_OPTIONS=halt_on_error=1:print_stacktrace=1 ./src/test/test_dash --run_test=chrono_overflow_tests/governance_vote_future_time_extreme --log_level=test_suite
  • UBSAN_OPTIONS=halt_on_error=1:print_stacktrace=1 ./src/test/test_dash --run_test=chrono_overflow_tests,governance_vote_wire_tests,governance_validators_tests,coinjoin_queue_tests,ratecheck_tests --log_level=message
  • clang-format --dry-run --Werror src/test/chrono_overflow_tests.cpp
  • git diff --check f5e834c0f9011e523725684c543b055a7ea86621..HEAD

All tests and checks passed.

Breaking Changes

None.

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have made corresponding changes to the documentation
  • I have assigned this pull request to a milestone (for repository code-owners and collaborators only)

PastaPastaPasta and others added 2 commits July 14, 2026 02:08
Convert nearby CoinJoin, governance, spork, LLMQ, RPC, and test code to chrono time points and durations. Preserve serialized, signed, hashed, and RPC-facing timestamps as integer Unix seconds with explicit conversions at those boundaries.
@coderabbitai

coderabbitai Bot commented Jul 14, 2026

Copy link
Copy Markdown

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e5ff2281-4532-4580-81e8-37b78149d6ca

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@thepastaclaw

Copy link
Copy Markdown
Author

Closing as superseded: the target branch was force-updated to 40e554c and now contains the serialized-timestamp overflow fixes and focused regression coverage directly. This child PR is therefore redundant and conflicted. Fresh CI/review continues on dashpay#7459 at that head.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants