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. |
| @@ -139,7 +141,8 @@ BOOST_FIXTURE_TEST_CASE(chainstate_update_tip, TestChain100Setup) | |||
| // g_best_block should be unchanged after adding a block to the background | |||
| // 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 fa7dd1d, which also fixes up the outdated comment.
| @@ -303,7 +304,7 @@ void StopRPC(const std::any& context) | |||
| LogDebug(BCLog::RPC, "Stopping RPC\n"); | |||
| 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 fadd531, which does the above.
|
Let's go with #30967. |
Based on post-merge comments on #30409.