Remove unused GetTimeSeconds #25102

pull MarcoFalke wants to merge 1 commits into bitcoin:master from MarcoFalke:2205-no-GetTimeSeconds-😅 changing 5 files +21 −20
  1. MarcoFalke commented at 11:58 AM on May 10, 2022: member

    Seems confusing to have this helper when it is possible to get the system time in a type-safe way by simply calling std::chrono::system_clock::now (C++11).

    This patch replaces GetTimeSeconds and removes it:

    • in bitcoin-cli.cpp by system_clock
    • in test/fuzz/fuzz.cpp by steady_clock
  2. MarcoFalke added the label Refactoring on May 10, 2022
  3. MarcoFalke force-pushed on May 10, 2022
  4. MarcoFalke force-pushed on May 10, 2022
  5. DrahtBot commented at 7:11 PM on May 10, 2022: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #25101 (Add mockable clock type by MarcoFalke)
    • #24697 (refactor address relay time by MarcoFalke)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

  6. DrahtBot cross-referenced this on May 11, 2022 from issue refactor address relay time by MarcoFalke
  7. in src/bitcoin-cli.cpp:442 in fab9d594b2 outdated
     438 | @@ -433,7 +439,7 @@ class NetinfoRequestHandler : public BaseRequestHandler
     439 |          if (conn_type == "addr-fetch") return "addr";
     440 |          return "";
     441 |      }
     442 | -    const int64_t m_time_now{GetTimeSeconds()};
     443 | +    const int64_t m_time_now{CountSeconds(Now<CliSeconds>())};
    


    laanwj commented at 12:49 PM on May 11, 2022:

    As this is used solely to measure intervals, it may be another good use for steady_clock.


    laanwj commented at 12:56 PM on May 11, 2022:

    Oh, never mind, it compares to the server time.

  8. Remove unused GetTimeSeconds fab9e8a29c
  9. in src/util/time.h:38 in fab9d594b2 outdated
      36 | +template <typename Clock>
      37 | +constexpr int64_t CountSeconds(std::chrono::time_point<Clock, std::chrono::seconds> t)
      38 | +{
      39 | +    return t.time_since_epoch().count();
      40 | +}
      41 |  constexpr int64_t count_seconds(std::chrono::seconds t) { return t.count(); }
    


    laanwj commented at 12:57 PM on May 11, 2022:

    It's confusing to me to have both count_seconds and CountSeconds.


    MarcoFalke commented at 2:40 PM on May 11, 2022:

    Ah, I forgot that function overloading exists. Thanks, fixed.

  10. MarcoFalke force-pushed on May 11, 2022
  11. laanwj commented at 5:03 PM on May 11, 2022: member

    Code review ACK fab9e8a29c2cdeab6cf1ae7c1fc0e0a3af783b17

  12. DrahtBot cross-referenced this on May 12, 2022 from issue Add mockable clock type by MarcoFalke
  13. naumenkogs commented at 7:55 AM on May 12, 2022: member

    ACK fab9e8a29c2cdeab6cf1ae7c1fc0e0a3af783b17

  14. MarcoFalke merged this on May 12, 2022
  15. MarcoFalke closed this on May 12, 2022

  16. MarcoFalke deleted the branch on May 12, 2022
  17. sidhujag referenced this in commit d19364a082 on May 13, 2022
  18. jonatack commented at 3:12 PM on May 16, 2022: contributor

    Post-merge ACK // edit: modulo a few clean-ups addressed in #25155

  19. jonatack cross-referenced this on May 17, 2022 from issue GetTimeSeconds() removal followups by jonatack
  20. bitcoin locked this on May 17, 2023

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-05-20 06:53 UTC