Skip to content

Conversation

@DashCoreAutoGuix
Copy link
Owner

Backports bitcoin#26118

Original commit: 2397574

Changes:

  • Replace GetTimeMicros() with SteadyClock::now() for benchmarking durations
  • Use type-safe Ticks<MillisecondsDouble>() and Ticks<SecondsDouble>() helpers instead of manual MILLI * value and MICRO * value conversions
  • Add MillisecondsDouble type alias to src/util/time.h
  • Preserve Dash-specific BCLog::BENCHMARK category (Bitcoin uses BCLog::BENCH)
  • Preserve all Dash-specific timing variables for masternode, InstantSend, and governance validation

Scope:

  • Bitcoin: 169 lines changed across 5 files
  • Dash: 251 lines changed across 5 files (149% - includes additional Dash-specific timing code updates)

fabf1cd Use steady clock for bench logging (MacroFake)
faed342 scripted-diff: Rename time symbols (MacroFake)

Pull request description:

  Instead of using `0.001` and similar constants to "convert" an int64_t to milliseconds, use the type-safe `Ticks<>` helper. Also, use steady clock instead of system clock, since the durations are used for benchmarking.

ACKs for top commit:
  fanquake:
    ACK fabf1cd - validation bench output still looks sane.

Tree-SHA512: e6525b5fdad6045ca500c56014897d7428ad288aaf375933d3b5939feddf257f6910d562eb66ebcde9186bef9a604ee8d763a318253838318d59df2a285be7c2
@coderabbitai
Copy link

coderabbitai bot commented Dec 2, 2025

Warning

Rate limit exceeded

@DashCoreAutoGuix has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 10 minutes and 58 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between 93d8697 and f4131c9.

📒 Files selected for processing (5)
  • src/node/miner.cpp (3 hunks)
  • src/policy/fees.cpp (1 hunks)
  • src/rpc/mining.cpp (1 hunks)
  • src/util/time.h (1 hunks)
  • src/validation.cpp (23 hunks)
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch backport-0.25-batch-469-pr-26118

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 and usage tips.

@DashCoreAutoGuix
Copy link
Owner Author

✅ Verification Complete - APPROVED

Summary

This PR successfully backports Bitcoin Core PR bitcoin#26118, which replaces system time () with steady clock for bench logging to prevent timing issues from system clock adjustments.

Bitcoin Commit

Verification Results

📊 Statistics

  • Files Changed: 5 (matches Bitcoin)
  • Size Ratio: 148.5% (PASS - within 80-150% range)
  • CI Status: All 53 checks passing ✅

📝 File-by-File Analysis

All changes are faithful to the Bitcoin commit with appropriate Dash adaptations:

  1. src/util/time.h: ✅ Added MillisecondsDouble type alias
  2. src/node/miner.cpp: ✅ Converted timing to steady clock in CreateNewBlock()
  3. src/policy/fees.cpp: ✅ Converted timing in FlushUnconfirmed()
  4. src/rpc/mining.cpp: ✅ Renamed nStart to time_start
  5. src/validation.cpp: ✅ Extensive conversion of all benchmark timing variables from int64_t to SteadyClock::duration

🎯 Dash-Specific Adaptations

The backport correctly handles Dash-specific code:

  • Additional timing metrics for Dash features (InstantSend filter, masternode payee validation, special tx processing, etc.) properly converted
  • BCLog::BENCHBCLog::BENCHMARK (standard Dash adaptation)
  • Additional logging and metrics code properly adapted
  • All Dash-specific timing variables converted consistently

✨ Key Changes

  • Removed hardcoded MICRO and MILLI macros
  • Replaced int64_t timing variables with SteadyClock::duration
  • Used Ticks<MillisecondsDouble> and Ticks<SecondsDouble> for converting durations to numeric values
  • Renamed nBlocksTotalnum_blocks_total (in Bitcoin portions)
  • Dash retained nBlocksTotal naming for its own code sections

Conclusion

The backport is complete, faithful, and correct. All Bitcoin changes have been properly applied with appropriate Dash adaptations. The increased size ratio (148.5%) is due to Dash's additional timing metrics for masternode and governance features, which have been correctly adapted to use steady clock.

Status: ✅ APPROVED

@DashCoreAutoGuix DashCoreAutoGuix added the verified Backport verification passed - ready for merge label Dec 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

verified Backport verification passed - ready for merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants