util: remove Boost posix_time usage from GetTime* #21110

1. 
   fanquake commented at 9:46 am on February 8, 2021: member

   I have a followup that should remove the last of our boost:posix_time usage in ParseISO8601DateTime, but that will likely need more cross-platform testing/discussion, so have just split them up as this change is straight forward.
2. fanquake added the label Refactoring on Feb 8, 2021
3. fanquake added the label Utils/log/libs on Feb 8, 2021
4. 
   maflcko commented at 9:53 am on February 8, 2021: member

   Duplicate of #18282?
5. 
   fanquake commented at 3:07 am on February 10, 2021: member

   Duplicate of #18282?

   Do you want to reopen that and include the epoch assertion/sanity check? Otherwise I can update this PR.

6. 
   maflcko commented at 7:27 am on February 10, 2021: member

   My plan was to wait till c++20, because it means less code to write.
7. 
   fanquake commented at 7:39 am on February 10, 2021: member

   My plan was to wait till c++20, because it means less code to write.

   I’m not sure that waiting the many years until the project will actually be able to use C++20, while preventing the removal of Boost, and use of std library features is worth not having to write a few additional lines of code. Especially since the assumption we have to make here is just a Unix time epoch. If it was something more controversial, or involved having to write hundreds of lines of additional code, which would incur a high maintenance overhead, I might agree, but that shouldn’t be the case here.

8. 
   maflcko commented at 7:50 am on February 10, 2021: member

   Obviously no objection to adding the sanity check now and removing it when we switch to c++20. I just meant to say that I am not going to write (rebase) it.
9. 
   laanwj commented at 8:59 am on February 10, 2021: member

   Concept ACK

   Let’s go with a sanity check. I know I disagreed on this in https://github.com/bitcoin/bitcoin/pull/9566/files#r96350569 but as we age we get tired of the same old things and just want to move forward.

10. 
    practicalswift commented at 9:29 am on February 11, 2021: contributor

    With the simple runtime sanity check from #9566 (https://github.com/bitcoin/bitcoin/pull/9566/commits/9bd32ce6aa50df95fd5aeaece92b8b908c582c73) we’re guaranteed that we get UTC.

    Concept ACK (assuming the sanity check is added of course)

11. 
    ajtowns commented at 4:35 am on February 12, 2021: contributor

    Concept ACK; agree with adding the sanity check and not waiting for C++20.
12. laanwj added the label Waiting for author on Feb 15, 2021
13. time: add runtime sanity check

    std::chrono::system_clock.time_since_epoch and time_t(0) are not guaranteed
    to use the Unix epoch timestamp, but in practice they almost certainly will.
    Any differing behavior will be assumed to be an error, unless certain
    platforms prove to consistently deviate, at which point we'll cope with it
    by adding offsets.
    
    Do a quick runtime check to verify that
    time_t(0) == std::chrono::system_clock's epoch time == unix epoch.
    
    Co-authored-by: Anthony Towns <aj@erisian.com.au>

    3c2e16be22
14. util: Use std::chrono for time getters 9266f7497f
15. fanquake removed the label Waiting for author on Feb 17, 2021
16. fanquake force-pushed on Feb 17, 2021
17. 
    fanquake commented at 5:49 am on February 17, 2021: member

    I’ve changed this to pull in @theuni’s sanity check (with some adjustments by @ajtowns , and some review comments from #9566 addressed). I’ve also dropped my commit in favour of @MarcoFalke change from #18282.
18. 
    practicalswift commented at 9:59 am on February 17, 2021: contributor

    Tested ACK 9266f7497f256d780178829e0f3a29ddaeb794ba

    Startup fails as expected if the sanity check doesn’t pass:

    0$ src/bitcoind
    1Error: Clock epoch mismatch. Aborting.
    2Error: Initialization sanity check failed. Bitcoin Core is shutting down.
    

    After this PR is merged the only remaining user of boost::posix_time is ParseISO8601DateTime. @elichai wrote a Boost-free replacement for ParseISO8601DateTime (then called DecodeDumpTime) two years ago in #17245. That replacement could be re-submitted as a new PR to allow for permanent removal of the boost/date_time/posix_time/posix_time.hpp dependency 🎉

19. 
    laanwj commented at 7:33 pm on February 17, 2021: member

    Code review ACK 9266f7497f256d780178829e0f3a29ddaeb794ba
20. laanwj merged this on Feb 17, 2021
21. laanwj closed this on Feb 17, 2021

    ---

22. sidhujag referenced this in commit 958325fd58 on Feb 18, 2021
23. fanquake deleted the branch on Feb 18, 2021
24. nuttycom referenced this in commit a423c2af8c on Jul 7, 2022
25. Fabcien referenced this in commit 06a488e349 on Jul 22, 2022
26. Fabcien referenced this in commit 7ba2b08340 on Jul 22, 2022
27. Fabcien referenced this in commit a208aeb229 on Jul 22, 2022
28. bitcoin locked this on Aug 16, 2022
29. 
    hebasto commented at 8:47 pm on January 4, 2024: member

    @fanquake

    I have a followup that should remove the last of our boost:posix_time usage in ParseISO8601DateTime…

    I’m not sure that waiting the many years until the project will actually be able to use C++20…

    As std::chrono::parse is available now, do you mind submitting your follow up? If not, I can do it :)

30. 
    fanquake commented at 10:32 am on January 5, 2024: member

    As std::chrono::parse is available now,

    I’m not sure if that is true? See discussion in #29081 (comment). Happy for you to submit a PR in any case.

Contributors
fanquake maflcko laanwj practicalswift ajtowns hebasto

Labels
Refactoring Utils/log/libs