Change Details

PR description > Rationale: > > The height based polling in the GUI is an issue on chain reorgs. Any new tip signal with the same height as the one that it's cached in the model was not triggering the GUI update (interpreting it as the same same block when it could receive a different one). Ending up with bad information presented in the GUI. > > This PR essentially changes the last cached height to be a last cached block hash. https://github.com/bitcoin/bitcoin/pull/17993/commits/2f867203b0c7a4438ce484be4cfa2b29dbf1abf0 > [ClientModel] best header/block hash cached. https://github.com/bitcoin/bitcoin/pull/19132/commits/f46b678acff0b2e75e26aa50b14d935b3d251a2a > qt: lock cs_main, m_cached_tip_mutex in that order > > Always lock the mutexes `cs_main` and `m_cached_tip_mutex` in > the same order: `cs_main`, `m_cached_tip_mutex`. Otherwise we may end up > in a deadlock. > > `ClientModel::m_cached_tip_blocks` is protected by > `ClientModel::m_cached_tip_mutex`. There are two access paths that > lock the two mutexes in opposite order: > > ``` > validation.cpp:2868 CChainState::ActivateBestChain(): lock cs_main > validation.cpp:2916 CChainState::ActivateBestChain(): call uiInterface.NotifyBlockTip() > ui_interface.cpp:52 CClientUIInterface::NotifyBlockTip(): go deep in boost > ... > qt/clientmodel.cpp:255 BlockTipChanged(): lock m_cached_tip_mutex > ``` > > and > > ``` > qt/clientmodel.cpp:119 ClientModel::getBestBlockHash(): lock m_cached_tip_mutex > qt/clientmodel.cpp:121 ClientModel::getBestBlockHash(): call m_node.getBestBlockHash() > interfaces/node.cpp:200 NodeImpl::getBestBlockHash(): lock cs_main > ``` > > From `debug.log`: > > ``` > POTENTIAL DEADLOCK DETECTED > Previous lock order was: > m_cs_chainstate validation.cpp:2851 > (1) cs_main validation.cpp:2868 > ::mempool.cs validation.cpp:2868 > (2) clientmodel->m_cached_tip_mutex qt/clientmodel.cpp:255 > Current lock order is: > (2) m_cached_tip_mutex qt/clientmodel.cpp:119 > (1) ::cs_main interfaces/node.cpp:200 > ``` > > The possible deadlock was introduced in > https://github.com/bitcoin/bitcoin/pull/17993 This is a backport of Core [[https://github.com/bitcoin/bitcoin/pull/17993 | PR17993]] [1/2] and [[https://github.com/bitcoin/bitcoin/pull/19132 | PR19132]]

PR description > Rationale: > > The height based polling in the GUI is an issue on chain reorgs. Any new tip signal with the same height as the one that it's cached in the model was not triggering the GUI update (interpreting it as the same same block when it could receive a different one). Ending up with bad information presented in the GUI. > > This PR essentially changes the last cached height to be a last cached block hash. https://github.com/bitcoin/bitcoin/pull/17993/commits/2f867203b0c7a4438ce484be4cfa2b29dbf1abf0 > [ClientModel] best header/block hash cached. https://github.com/bitcoin/bitcoin/pull/19132/commits/f46b678acff0b2e75e26aa50b14d935b3d251a2a > qt: lock cs_main, m_cached_tip_mutex in that order > > Always lock the mutexes `cs_main` and `m_cached_tip_mutex` in > the same order: `cs_main`, `m_cached_tip_mutex`. Otherwise we may end up > in a deadlock. > > `ClientModel::m_cached_tip_blocks` is protected by > `ClientModel::m_cached_tip_mutex`. There are two access paths that > lock the two mutexes in opposite order: > > ``` > validation.cpp:2868 CChainState::ActivateBestChain(): lock cs_main > validation.cpp:2916 CChainState::ActivateBestChain(): call uiInterface.NotifyBlockTip() > ui_interface.cpp:52 CClientUIInterface::NotifyBlockTip(): go deep in boost > ... > qt/clientmodel.cpp:255 BlockTipChanged(): lock m_cached_tip_mutex > ``` > > and > > ``` > qt/clientmodel.cpp:119 ClientModel::getBestBlockHash(): lock m_cached_tip_mutex > qt/clientmodel.cpp:121 ClientModel::getBestBlockHash(): call m_node.getBestBlockHash() > interfaces/node.cpp:200 NodeImpl::getBestBlockHash(): lock cs_main > ``` > > From `debug.log`: > > ``` > POTENTIAL DEADLOCK DETECTED > Previous lock order was: > m_cs_chainstate validation.cpp:2851 > (1) cs_main validation.cpp:2868 > ::mempool.cs validation.cpp:2868 > (2) clientmodel->m_cached_tip_mutex qt/clientmodel.cpp:255 > Current lock order is: > (2) m_cached_tip_mutex qt/clientmodel.cpp:119 > (1) ::cs_main interfaces/node.cpp:200 > ``` > > The possible deadlock was introduced in > https://github.com/bitcoin/bitcoin/pull/17993 This is a backport of Core [[https://github.com/bitcoin/bitcoin/pull/17993 | PR17993]] [1/2] and [[https://github.com/bitcoin/bitcoin/pull/19132 | PR19132]]

PR description > Rationale: > > The height based polling in the GUI is an issue on chain reorgs. Any new tip signal with the same height as the one that it's cached in the model was not triggering the GUI update (interpreting it as the same same block when it could receive a different one). Ending up with bad information presented in the GUI. > > This PR essentially changes the last cached height to be a last cached block hash. https://github.com/bitcoin/bitcoin/pull/17993/commits/2f867203b0c7a4438ce484be4cfa2b29dbf1abf0 > [ClientModel] best header/block hash cached. https://github.com/bitcoin/bitcoin/pull/19132/commits/f46b678acff0b2e75e26aa50b14d935b3d251a2a > qt: lock cs_main, m_cached_tip_mutex in that order > > Always lock the mutexes `cs_main` and `m_cached_tip_mutex` in > the same order: `cs_main`, `m_cached_tip_mutex`. Otherwise we may end up > in a deadlock. > > `ClientModel::m_cached_tip_blocks` is protected by > `ClientModel::m_cached_tip_mutex`. There are two access paths that > lock the two mutexes in opposite order: > > ``` > validation.cpp:2868 CChainState::ActivateBestChain(): lock cs_main > validation.cpp:2916 CChainState::ActivateBestChain(): call uiInterface.NotifyBlockTip() > ui_interface.cpp:52 CClientUIInterface::NotifyBlockTip(): go deep in boost > ... > qt/clientmodel.cpp:255 BlockTipChanged(): lock m_cached_tip_mutex > ``` > > and > > ``` > qt/clientmodel.cpp:119 ClientModel::getBestBlockHash(): lock m_cached_tip_mutex > qt/clientmodel.cpp:121 ClientModel::getBestBlockHash(): call m_node.getBestBlockHash() > interfaces/node.cpp:200 NodeImpl::getBestBlockHash(): lock cs_main > ``` > > From `debug.log`: > > ``` > POTENTIAL DEADLOCK DETECTED > Previous lock order was: > m_cs_chainstate validation.cpp:2851 > (1) cs_main validation.cpp:2868 > ::mempool.cs validation.cpp:2868 > (2) clientmodel->m_cached_tip_mutex qt/clientmodel.cpp:255 > Current lock order is: > (2) m_cached_tip_mutex qt/clientmodel.cpp:119 > (1) ::cs_main interfaces/node.cpp:200 > ``` > > The possible deadlock was introduced in > https://github.com/bitcoin/bitcoin/pull/17993 This is a backport of Core [[https://github.com/bitcoin/bitcoin/pull/17993 | PR17993]] [1/2] and [[https://github.com/bitcoin/bitcoin/pull/19132 | PR19132]]