Followups for #30409 waitTipChanged() #30966
Conversation
An earlier version of this code was written to handle the case where now + timeout calculation would overflow, basically assuming both values were unsigned integer values and overflow behavior would be well defined. But since timeout is a floating point number where "overflow" would just result in inf being set, trying to deal with it this way doesn't make sense. Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
This will print a more usable error message. EnsureAnyNodeContext is intended for RPC method implementations.
Co-authored-by: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. Code CoverageFor detailed information about the code coverage, see the test coverage report. ReviewsSee the guideline for information on the review process. |
| // validation chain. | ||
| BOOST_CHECK(block_added); | ||
| BOOST_CHECK_EQUAL(curr_tip, m_node.notifications->m_tip_block); | ||
| new_tip = WITH_LOCK(m_node.notifications->m_tip_block_mutex, return m_node.notifications->m_tip_block;); |
There was a problem hiding this comment.
Seems verbose to lock this every time. Also, the comment above is still wrong. It would be good to drop this commit, or replace it with fa7dd1d072dc36997e2d5d702e2da529a093c90f, which also fixes up the outdated comment.
| WITH_LOCK(g_deadline_timers_mutex, deadlineTimers.clear()); | ||
| DeleteAuthCookie(); | ||
| node::NodeContext& node = EnsureAnyNodeContext(context); | ||
| node::NodeContext& node = *Assert(util::AnyPtr<node::NodeContext>(context)); |
There was a problem hiding this comment.
Seems overly complicated to turn a reference into a pointer, then turn it into std::any, then recover it from any, then assert the pointer isn't null.
It would be better to remove all of this and just pass the reference.
So it would be good to drop this commit or replace it with fadd531a57710809df0e4b027ce30b5524e5f40f, which does the above.
|
Let's go with #30967. |
Based on post-merge comments on #30409.