fix: no need to relay quorum commitment in case of block undo

It fixes potentiall deadlock:
    Assertion failed: detected inconsistent lock order for 'peer.m_tx_relay->m_tx_inventory_mutex' in net_processing.cpp:971 (in thread 'httpworker.0'), details in debug log.
    Posix Signal: Aborted
       0#: (0x5BE0A9B78F04) stl_vector.h:115        - std::_Vector_base<unsigned long, std::allocator<unsigned long> >::_Vector_impl_data::_M_copy_data(std::_Vector_base<unsigned long, std::allocator<unsigned long> >::_Vector_impl_data const&)
       1#: (0x5BE0A9B78F04) stl_vector.h:127        - std::_Vector_base<unsigned long, std::allocator<unsigned long> >::_Vector_impl_data::_M_swap_data(std::_Vector_base<unsigned long, std::allocator<unsigned long> >::_Vector_impl_data&)
       2#: (0x5BE0A9B78F04) stl_vector.h:1959       - std::vector<unsigned long, std::allocator<unsigned long> >::_M_move_assign(std::vector<unsigned long, std::allocator<unsigned long> >&&, std::integral_constant<bool, true>)
       3#: (0x5BE0A9B78F04) stl_vector.h:768        - std::vector<unsigned long, std::allocator<unsigned long> >::operator=(std::vector<unsigned long, std::allocator<unsigned long> >&&)
       4#: (0x5BE0A9B78F04) stacktraces.cpp:777     - HandlePosixSignal
       5#: (0x733859C42990) libc_sigaction.c        - ???
       6#: (0x733859C99A1B) pthread_kill.c:44       - __pthread_kill_implementation
       7#: (0x733859C99A1B) pthread_kill.c:78       - __pthread_kill_internal
       8#: (0x733859C99A1B) pthread_kill.c:89       - __GI___pthread_kill
       9#: (0x733859C428E6) raise.c:27              - __GI_raise
      10#: (0x733859C268B7) abort.c:81              - __GI_abort
      11#: (0x5BE0A9B7EA06) basic_string.h:390      - std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_check_length(unsigned long, unsigned long, char const*) const
      12#: (0x5BE0A9B7EA06) basic_string.h:1461     - std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::append(char const*)
      13#: (0x5BE0A9B7EA06) basic_string.h:1365     - std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::operator+=(char const*)
      14#: (0x5BE0A9B7EA06) sync.cpp:114            - potential_deadlock_detected
      15#: (0x5BE0A9B8548E) sync.cpp:188            - push_lock<std::recursive_mutex>
      16#: (0x5BE0A9B8548E) sync.cpp:212            - void EnterCritical<std::recursive_mutex>(char const*, char const*, int, std::recursive_mutex*, bool)
      17#: (0x5BE0A935C582) unique_lock.h:150       - std::unique_lock<std::recursive_mutex>::try_lock()
      18#: (0x5BE0A935C582) sync.h:162              - UniqueLock<AnnotatedMixin<std::recursive_mutex>, std::unique_lock<std::recursive_mutex> >::Enter(char const*, char const*, int)
      19#: (0x5BE0A935C582) sync.h:183              - UniqueLock<AnnotatedMixin<std::recursive_mutex>, std::unique_lock<std::recursive_mutex> >::UniqueLock(AnnotatedMixin<std::recursive_mutex>&, char const*, char const*, int, bool)
      20#: (0x5BE0A9487A92) net_processing.cpp:972  - PushInv
      21#: (0x5BE0A94896E5) shared_ptr_base.h:1070  - std::__shared_count<(__gnu_cxx::_Lock_policy)2>::~__shared_count()
      22#: (0x5BE0A94896E5) shared_ptr_base.h:1524  - ~__shared_ptr
      23#: (0x5BE0A94896E5) shared_ptr.h:175        - ~shared_ptr
      24#: (0x5BE0A94896E5) net_processing.cpp:2276 - operator()
      25#: (0x5BE0A94896E5) net.h:1051              - ForEachNode<CConnman::CFullyConnectedOnly, (anonymous namespace)::PeerManagerImpl::RelayInv(CInv&, int)::<lambda(CNode*)>&>
      26#: (0x5BE0A94896E5) net.h:1058              - ForEachNode<(anonymous namespace)::PeerManagerImpl::RelayInv(CInv&, int)::<lambda(CNode*)> >
      27#: (0x5BE0A94896E5) net_processing.cpp:2269 - RelayInv
      28#: (0x5BE0A98B7C03) blockprocessor.cpp:683  - llmq::CQuorumBlockProcessor::AddMineableCommitment(llmq::CFinalCommitment const&)
      29#: (0x5BE0A98BE2E1) blockprocessor.cpp:338  - llmq::CQuorumBlockProcessor::UndoBlock(CBlock const&, gsl::not_null<CBlockIndex const*>)
      30#: (0x5BE0A9809EA9) specialtxman.cpp:264    - CSpecialTxProcessor::UndoSpecialTxsInBlock(CBlock const&, CBlockIndex const*, std::optional<MNListUpdates>&)
      31#: (0x5BE0A96FFB84) validation.cpp:1693     - CChainState::DisconnectBlock(CBlock const&, CBlockIndex const*, CCoinsViewCache&)
      32#: (0x5BE0A9701481) validation.cpp:2726     - CChainState::DisconnectTip(BlockValidationState&, DisconnectedBlockTransactions*)
This commit is contained in:
Konstantin Akimov 2024-07-15 02:08:56 +07:00
parent 0431a33919
commit f4cb0fbfe1
No known key found for this signature in database
GPG Key ID: 2176C4A5D01EA524
3 changed files with 14 additions and 6 deletions

View File

@ -138,7 +138,7 @@ PeerMsgRet CQuorumBlockProcessor::ProcessMessage(const CNode& peer, std::string_
LogPrint(BCLog::LLMQ, "CQuorumBlockProcessor::%s -- received commitment for quorum %s:%d, validMembers=%d, signers=%d, peer=%d\n", __func__, LogPrint(BCLog::LLMQ, "CQuorumBlockProcessor::%s -- received commitment for quorum %s:%d, validMembers=%d, signers=%d, peer=%d\n", __func__,
qc.quorumHash.ToString(), ToUnderlying(qc.llmqType), qc.CountValidMembers(), qc.CountSigners(), peer.GetId()); qc.quorumHash.ToString(), ToUnderlying(qc.llmqType), qc.CountValidMembers(), qc.CountSigners(), peer.GetId());
AddMineableCommitment(qc); AddMineableCommitmentAndRelay(qc);
return {}; return {};
} }
@ -636,7 +636,7 @@ bool CQuorumBlockProcessor::HasMineableCommitment(const uint256& hash) const
return minableCommitments.count(hash) != 0; return minableCommitments.count(hash) != 0;
} }
void CQuorumBlockProcessor::AddMineableCommitment(const CFinalCommitment& fqc) std::optional<uint256> CQuorumBlockProcessor::AddMineableCommitment(const CFinalCommitment& fqc)
{ {
const uint256 commitmentHash = ::SerializeHash(fqc); const uint256 commitmentHash = ::SerializeHash(fqc);
@ -662,9 +662,15 @@ void CQuorumBlockProcessor::AddMineableCommitment(const CFinalCommitment& fqc)
return false; return false;
}(); }();
return relay ? std::make_optional(commitmentHash) : std::nullopt;
}
void CQuorumBlockProcessor::AddMineableCommitmentAndRelay(const CFinalCommitment& fqc)
{
const auto commitmentHashOpt = AddMineableCommitment(fqc);
// We only relay the new commitment if it's new or better then the old one // We only relay the new commitment if it's new or better then the old one
if (relay) { if (commitmentHashOpt) {
CInv inv(MSG_QUORUM_FINAL_COMMITMENT, commitmentHash); CInv inv(MSG_QUORUM_FINAL_COMMITMENT, *commitmentHashOpt);
Assert(m_peerman)->RelayInv(inv); Assert(m_peerman)->RelayInv(inv);
} }
} }

View File

@ -59,7 +59,7 @@ public:
bool ProcessBlock(const CBlock& block, gsl::not_null<const CBlockIndex*> pindex, BlockValidationState& state, bool fJustCheck, bool fBLSChecks) EXCLUSIVE_LOCKS_REQUIRED(cs_main); bool ProcessBlock(const CBlock& block, gsl::not_null<const CBlockIndex*> pindex, BlockValidationState& state, bool fJustCheck, bool fBLSChecks) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
bool UndoBlock(const CBlock& block, gsl::not_null<const CBlockIndex*> pindex) EXCLUSIVE_LOCKS_REQUIRED(cs_main); bool UndoBlock(const CBlock& block, gsl::not_null<const CBlockIndex*> pindex) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
void AddMineableCommitment(const CFinalCommitment& fqc); void AddMineableCommitmentAndRelay(const CFinalCommitment& fqc);
bool HasMineableCommitment(const uint256& hash) const; bool HasMineableCommitment(const uint256& hash) const;
bool GetMineableCommitmentByHash(const uint256& commitmentHash, CFinalCommitment& ret) const; bool GetMineableCommitmentByHash(const uint256& commitmentHash, CFinalCommitment& ret) const;
std::optional<std::vector<CFinalCommitment>> GetMineableCommitments(const Consensus::LLMQParams& llmqParams, int nHeight) const EXCLUSIVE_LOCKS_REQUIRED(cs_main); std::optional<std::vector<CFinalCommitment>> GetMineableCommitments(const Consensus::LLMQParams& llmqParams, int nHeight) const EXCLUSIVE_LOCKS_REQUIRED(cs_main);
@ -74,6 +74,8 @@ public:
std::vector<std::pair<int, const CBlockIndex*>> GetLastMinedCommitmentsPerQuorumIndexUntilBlock(Consensus::LLMQType llmqType, const CBlockIndex* pindex, size_t cycle) const; std::vector<std::pair<int, const CBlockIndex*>> GetLastMinedCommitmentsPerQuorumIndexUntilBlock(Consensus::LLMQType llmqType, const CBlockIndex* pindex, size_t cycle) const;
std::optional<const CBlockIndex*> GetLastMinedCommitmentsByQuorumIndexUntilBlock(Consensus::LLMQType llmqType, const CBlockIndex* pindex, int quorumIndex, size_t cycle) const; std::optional<const CBlockIndex*> GetLastMinedCommitmentsByQuorumIndexUntilBlock(Consensus::LLMQType llmqType, const CBlockIndex* pindex, int quorumIndex, size_t cycle) const;
private: private:
//! it returns hash of commitment if it should be relay, otherwise nullopt
std::optional<uint256> AddMineableCommitment(const CFinalCommitment& fqc);
static bool GetCommitmentsFromBlock(const CBlock& block, gsl::not_null<const CBlockIndex*> pindex, std::multimap<Consensus::LLMQType, CFinalCommitment>& ret, BlockValidationState& state) EXCLUSIVE_LOCKS_REQUIRED(cs_main); static bool GetCommitmentsFromBlock(const CBlock& block, gsl::not_null<const CBlockIndex*> pindex, std::multimap<Consensus::LLMQType, CFinalCommitment>& ret, BlockValidationState& state) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
bool ProcessCommitment(int nHeight, const uint256& blockHash, const CFinalCommitment& qc, BlockValidationState& state, bool fJustCheck, bool fBLSChecks) EXCLUSIVE_LOCKS_REQUIRED(cs_main); bool ProcessCommitment(int nHeight, const uint256& blockHash, const CFinalCommitment& qc, BlockValidationState& state, bool fJustCheck, bool fBLSChecks) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
static bool IsMiningPhase(const Consensus::LLMQParams& llmqParams, const CChain& active_chain, int nHeight) EXCLUSIVE_LOCKS_REQUIRED(cs_main); static bool IsMiningPhase(const Consensus::LLMQParams& llmqParams, const CChain& active_chain, int nHeight) EXCLUSIVE_LOCKS_REQUIRED(cs_main);

View File

@ -570,7 +570,7 @@ void CDKGSessionHandler::HandleDKGRound()
auto finalCommitments = curSession->FinalizeCommitments(); auto finalCommitments = curSession->FinalizeCommitments();
for (const auto& fqc : finalCommitments) { for (const auto& fqc : finalCommitments) {
quorumBlockProcessor.AddMineableCommitment(fqc); quorumBlockProcessor.AddMineableCommitmentAndRelay(fqc);
} }
} }