Skip to content

Commit

Permalink
Merge bitcoin#26477: validation: fix broken maxtipage comparison
Browse files Browse the repository at this point in the history 
e4be0e9 test: add -maxtipage test for the maximum allowable value (James O'Beirne)
a451e83 fix: validation: cast now() to seconds for maxtipage comparison (James O'Beirne)

Pull request description:

  Since bitcoin@faf4487, the maxtipage comparison in IsInitialBlockDownload() has been broken, since the NodeClock::now() time_point is in the system's native denomination (nanoseconds).

  Without this patch, specifying the maximum allowable -maxtipage (9223372036854775807) results in a SIGABRT crash:

  ```
  % gdb --args ./src/bitcoind -maxtipage=9223372036854775207 -minimumchainwork=0x00 -stopatheight=30000
  ...
  2022-11-09T15:55:17Z [dnsseed] dnsseed thread exit
  [Thread 0x7fff937fe640 (LWP 69883) exited]

  Thread 29 "b-msghand" received signal SIGABRT, Aborted.
  [Switching to Thread 0x7fff91ffb640 (LWP 69886)]
  __pthread_kill_implementation (threadid=<optimized out>, signo=signo@entry=6, no_tid=no_tid@entry=0) at ./nptl/pthread_kill.c:44
  44      ./nptl/pthread_kill.c: No such file or directory.
  (gdb) bt
  #0  __pthread_kill_implementation (threadid=<optimized out>, signo=signo@entry=6, no_tid=no_tid@entry=0) at ./nptl/pthread_kill.c:44
  #1  0x00007ffff768989f in __pthread_kill_internal (signo=6, threadid=<optimized out>) at ./nptl/pthread_kill.c:78
  #2  0x00007ffff763da52 in __GI_raise (sig=sig@entry=6) at ../sysdeps/posix/raise.c:26
  #3  0x00007ffff7628469 in __GI_abort () at ./stdlib/abort.c:79
  #4  0x00007ffff7cf79a4 in __mulvdi3 () from /lib/x86_64-linux-gnu/libgcc_s.so.1
  #5  0x00005555558d13ab in std::chrono::__duration_cast_impl<std::chrono::duration<long, std::ratio<1l, 1000000000l> >, std::ratio<1000000000l, 1l>, long, false, true>::__cast<long, std::ratio<1l, 1l> > (__d=...) at /usr/include/c++/12/bits/chrono.h:521
  #6  std::chrono::duration_cast<std::chrono::duration<long, std::ratio<1l, 1000000000l> >, long, std::ratio<1l, 1l> > (__d=...)
      at /usr/include/c++/12/bits/chrono.h:260
  #7  std::chrono::duration<long, std::ratio<1l, 1000000000l> >::duration<long, std::ratio<1l, 1l>, void> (__d=..., this=<optimized out>)
      at /usr/include/c++/12/bits/chrono.h:514
  #8  std::chrono::operator-<long, std::ratio<1l, 1000000000l>, long, std::ratio<1l, 1l> > (__rhs=..., __lhs=...)
      at /usr/include/c++/12/bits/chrono.h:650
  #9  std::chrono::operator-<NodeClock, std::chrono::duration<long, std::ratio<1l, 1000000000l> >, long, std::ratio<1l, 1l> > (__rhs=...,
      __lhs=...) at /usr/include/c++/12/bits/chrono.h:1020
  #10 Chainstate::IsInitialBlockDownload (this=0x555556071940) at ./src/validation.cpp:1545
  #11 0x00005555556efd1e in operator() (__closure=<optimized out>) at ./src/net_processing.cpp:3369
  #12 (anonymous namespace)::PeerManagerImpl::ProcessMessage (this=0x555556219be0, pfrom=..., msg_type=..., vRecv=..., time_received=...,
      interruptMsgProc=...) at ./src/net_processing.cpp:3369
  #13 0x00005555556f75cc in (anonymous namespace)::PeerManagerImpl::ProcessMessages (this=0x555556219be0, pfrom=<optimized out>,
      interruptMsgProc=std::atomic<bool> = { false }) at ./src/net_processing.cpp:4985
  #14 0x00005555556a83c9 in CConnman::ThreadMessageHandler (this=0x5555560ebc70) at ./src/net.cpp:2014
  #15 0x0000555555c4d5d6 in std::function<void ()>::operator()() const (this=0x7fff91ffadb0) at /usr/include/c++/12/bits/std_function.h:591
  bitcoin#16 util::TraceThread(std::basic_string_view<char, std::char_traits<char> >, std::function<void ()>) (
      thread_name="0\255\377\221\377\177\000\000\v\000\000\000\000\000\000\000TraceThread\000\000\000\000\000P\255\377\221\377\177\000\000\017\000\000\000\000\000\000\000util/thread.cpp\000\000\000\000\000\000\000\000\000\000ihB鵿6\000\000\000\000\000\000\000\000\260\255\377\221\377\177\000\000\277\211\321UUU\000\000p\324\304UUU\000\000\002\000\000\000\000\000\000\000\240xh\367\377\177\000\000\000\000\000\000\000\000\000\000]\340iUUU\000\000p\274\016VUU\000\000\000\000\000\000\000\000\000\000\300\303iUUU\000\000p\206jUUU", '\000' <repeats 11 times>, "ihB鵿6\200\251!VUU\000\000"..., thread_func=...) at util/thread.cpp:21
  bitcoin#17 0x000055555569e05d in std::__invoke_impl<void, void (*)(std::basic_string_view<char>, std::function<void()>), char const*, CConnman::Start(CScheduler&, const Options&)::<lambda()> > (__f=<optimized out>) at /usr/include/c++/12/bits/invoke.h:61
  bitcoin#18 std::__invoke<void (*)(std::basic_string_view<char>, std::function<void()>), char const*, CConnman::Start(CScheduler&, const Options&)::<lambda()> > (__fn=<optimized out>) at /usr/include/c++/12/bits/invoke.h:96
  bitcoin#19 std::thread::_Invoker<std::tuple<void (*)(std::basic_string_view<char, std::char_traits<char> >, std::function<void()>), char const*, CConnman::Start(CScheduler&, const Options&)::<lambda()> > >::_M_invoke<0, 1, 2> (this=<optimized out>) at /usr/include/c++/12/bits/std_thread.h:252
  bitcoin#20 std::thread::_Invoker<std::tuple<void (*)(std::basic_string_view<char, std::char_traits<char> >, std::function<void()>), char const*, CConnman::Start(CScheduler&, const Options&)::<lambda()> > >::operator() (this=<optimized out>) at /usr/include/c++/12/bits/std_thread.h:259
  bitcoin#21 std::thread::_State_impl<std::thread::_Invoker<std::tuple<void (*)(std::basic_string_view<char, std::char_traits<char> >, std::function<void()>), char const*, CConnman::Start(CScheduler&, const Options&)::<lambda()> > > >::_M_run(void) (this=<optimized out>)
      at /usr/include/c++/12/bits/std_thread.h:210
  bitcoin#22 0x00007ffff7ad43d3 in ?? () from /lib/x86_64-linux-gnu/libstdc++.so.6
  bitcoin#23 0x00007ffff7687b27 in start_thread (arg=<optimized out>) at ./nptl/pthread_create.c:435
  bitcoin#24 0x00007ffff770a78c in clone3 () at ../sysdeps/unix/sysv/linux/x86_64/clone3.S:81
  (gdb)
  ```

ACKs for top commit:
  MarcoFalke:
    review ACK e4be0e9 🏽

Tree-SHA512: d892d6264a284d952a68a8631a6301277373b8df939dafd9e2652f2f22ab60712cde63b90c27c67ea2d05f02443452e3e4e1b9f25479bfaca00d4c4de13b9fbd
  • Loading branch information
@fanquake
fanquake committed Dec 13, 2022
2 parents 6061eb6 + e4be0e9 commit 968f03e
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 10 deletions.
2 changes: 1 addition & 1 deletion src/validation.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1542,7 +1542,7 @@ bool Chainstate::IsInitialBlockDownload() const
if (m_chain.Tip()->nChainWork < m_chainman.MinimumChainWork()) {
return true;
}
if (m_chain.Tip()->Time() < NodeClock::now() - m_chainman.m_options.max_tip_age) {
if (m_chain.Tip()->Time() < Now<NodeSeconds>() - m_chainman.m_options.max_tip_age) {
return true;
}
LogPrintf("Leaving InitialBlockDownload (latching to false)\n");
Expand Down
23 changes: 14 additions & 9 deletions test/functional/feature_maxtipage.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,23 +22,24 @@ def set_test_params(self):
self.setup_clean_chain = True
self.num_nodes = 2

def test_maxtipage(self, maxtipage, set_parameter=True):
def test_maxtipage(self, maxtipage, set_parameter=True, test_deltas=True):
node_miner = self.nodes[0]
node_ibd = self.nodes[1]

self.restart_node(1, [f'-maxtipage={maxtipage}'] if set_parameter else None)
self.connect_nodes(0, 1)

# tips older than maximum age -> stay in IBD
cur_time = int(time.time())
node_ibd.setmocktime(cur_time)
for delta in [5, 4, 3, 2, 1]:
node_miner.setmocktime(cur_time - maxtipage - delta)
self.generate(node_miner, 1)
assert_equal(node_ibd.getblockchaininfo()['initialblockdownload'], True)

if test_deltas:
# tips older than maximum age -> stay in IBD
node_ibd.setmocktime(cur_time)
for delta in [5, 4, 3, 2, 1]:
node_miner.setmocktime(cur_time - maxtipage - delta)
self.generate(node_miner, 1)
assert_equal(node_ibd.getblockchaininfo()['initialblockdownload'], True)

# tip within maximum age -> leave IBD
node_miner.setmocktime(cur_time - maxtipage)
node_miner.setmocktime(max(cur_time - maxtipage, 0))
self.generate(node_miner, 1)
assert_equal(node_ibd.getblockchaininfo()['initialblockdownload'], False)

Expand All @@ -51,6 +52,10 @@ def run_test(self):
self.log.info(f"Test IBD with maximum tip age of {hours} hours (-maxtipage={maxtipage}).")
self.test_maxtipage(maxtipage)

max_long_val = 9223372036854775807
self.log.info(f"Test IBD with highest allowable maximum tip age ({max_long_val}).")
self.test_maxtipage(max_long_val, test_deltas=False)


if __name__ == '__main__':
MaxTipAgeTest().main()

0 comments on commit 968f03e

Please sign in to comment.