feat: mnauth always use basic scheme by PastaPastaPasta · Pull Request #6467 · dashpay/dash

PastaPastaPasta · 2024-12-08T21:10:05Z

Issue being fixed or feature implemented

Currently, mnauth has to check status of v19 hard fork. While this isn't soo terrible, it's not needed anymore. On mainnet or testnet, any mn you possible connect to will have it's TIP past v19 HF, meaning in practice it will only ever send you basic scheme anyhow. Let's just harden it. I initially guarded this behind a new protocol version, but I do not think that is needed.

What was done?

How Has This Been Tested?

Breaking Changes

This is potentially a breaking change for devnets, which are moving past the v19 hard fork, but on develop v19 activates at block 2 for devnets sooooo, this shouldn't be noticed.

Checklist:

Go over all the following points, and put an x in all the boxes that apply.

I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have added or updated relevant unit/integration/functional/e2e tests
I have made corresponding changes to the documentation
I have assigned this pull request to a milestone (for repository code-owners and collaborators only)

knst

Looks good overall, but it is incomplete because RPC mnauth still uses V19 deployment in rpc/misc.cpp to decide if that's legacy or basic public key.

This PR is subset of my local branch to fire up v19 on RegTest from block 1, but so far as this PR is almost ready and my local branch with v19 changes is still blocked by #6325, let's get this one merged first.

Though, consider to include df1d707

knst · 2024-12-09T07:11:56Z

nit: drop also unused ConstCBLSPublicKeyVersionWrapper - see 68ee12db683d454b85ca7c8454934e9483e013e2

PastaPastaPasta · 2024-12-09T20:17:47Z

Code reviews applied, squashed into top commit.

knst · 2024-12-09T20:28:12Z

nit: also

-class CChain;

knst · 2024-12-09T20:29:33Z

haven't you forgot to update net_processing.cpp?
CMNAuth::ProcessMessage changed signature

probably 🙈

fixup: drop CChain from mnauth ProcessMessage feat: let RPC mnauth to generate only BASIC bls messages and fixes for rpc_mnauth.py and p2p_quorum_data.py refactor: drop unused ConstCBLSPublicKeyVersionWrapper

UdjinM6

utACK 09058e0

knst · 2024-12-10T13:34:30Z

Thread sanitizer failed with error that addressed in my PR.
Failure: https://gitlab.com/dashpay/dash/-/jobs/8592007713
PR: #6470

logs for reference

test  2024-12-09T21:45:07.972000Z TestFramework (ERROR): 
 node0 stderr Assertion failed: detected inconsistent lock order for 'cs_main' in node/blockstorage.cpp:775 (in thread 'httpworker.3'), details in debug log.
==================
WARNING: ThreadSanitizer: signal-unsafe call inside of a signal (pid=10145)
    #0 operator new(unsigned long) <null> (dashd+0x1ab367) (BuildId: caa064d989fe6f8ddf115c37f17007602b71b63d)
    #1 allocate /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/ext/new_allocator.h:127:27 (dashd+0x20caed) (BuildId: caa064d989fe6f8ddf115c37f17007602b71b63d)
    #2 allocate /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/alloc_traits.h:464:20 (dashd+0x20caed)
    #3 _M_create /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/basic_string.tcc:153:14 (dashd+0x20caed)
    #4 std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>::reserve(unsigned long) /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/basic_string.tcc:291:23 (dashd+0x20caed)
    #5 std::__cxx11::basic_stringbuf<char, std::char_traits<char>, std::allocator<char>>::overflow(int) <null> (libstdc++.so.6+0x142952) (BuildId: e37fe1a879783838de78cbc8c80621fa685d58a2)
    #6 format src/./tinyformat.h:528:13 (dashd+0x1b031c) (BuildId: caa064d989fe6f8ddf115c37f17007602b71b63d)
    #7 tinyformat::detail::formatImpl(std::ostream&, char const*, tinyformat::detail::FormatArg const*, int) src/./tinyformat.h:907:17 (dashd+0x1b031c)
    #8 vformat src/./tinyformat.h:1055:5 (dashd+0x1aeb83) (BuildId: caa064d989fe6f8ddf115c37f17007602b71b63d)
    #9 format<const char *> src/./tinyformat.h:1065:5 (dashd+0x1aeb83)
    #10 std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> tinyformat::format<char const*>(char const*, char const* const&) src/./tinyformat.h:1074:5 (dashd+0x1aeb83)
    #11 HandlePosixSignal(int) src/stacktraces.cpp:776:27 (dashd+0x1413d7f) (BuildId: caa064d989fe6f8ddf115c37f17007602b71b63d)
    #12 __tsan::CallUserSignalHandler(__tsan::ThreadState*, bool, bool, int, __sanitizer::__sanitizer_siginfo*, void*) tsan_interceptors_posix.cpp.o (dashd+0x12ba9f) (BuildId: caa064d989fe6f8ddf115c37f17007602b71b63d)
    #13 potential_deadlock_detected(std::pair<void*, void*> const&, std::__debug::vector<std::pair<void*, CLockLocation>, std::allocator<std::pair<void*, CLockLocation>>> const&, std::__debug::vector<std::pair<void*, CLockLocation>, std::allocator<std::pair<void*, CLockLocation>>> const&) src/sync.cpp:130:9 (dashd+0x14225cc) (BuildId: caa064d989fe6f8ddf115c37f17007602b71b63d)
    #14 push_lock<std::recursive_mutex> src/sync.cpp:190:13 (dashd+0x14258ab) (BuildId: caa064d989fe6f8ddf115c37f17007602b71b63d)
    #15 void EnterCritical<std::recursive_mutex>(char const*, char const*, int, std::recursive_mutex*, bool) src/sync.cpp:214:5 (dashd+0x14258ab)
    #16 UniqueLock<AnnotatedMixin<std::recursive_mutex>, std::unique_lock<std::recursive_mutex>>::Enter(char const*, char const*, int) src/./sync.h:166:9 (dashd+0x20cc7c) (BuildId: caa064d989fe6f8ddf115c37f17007602b71b63d)
    #17 UniqueLock src/./sync.h:190:13 (dashd+0x64b339) (BuildId: caa064d989fe6f8ddf115c37f17007602b71b63d)
    #18 operator() src/node/blockstorage.cpp:775:33 (dashd+0x64b339)
    #19 ReadBlockFromDisk(CBlock&, CBlockIndex const*, Consensus::Params const&) src/node/blockstorage.cpp:775:33 (dashd+0x64b339)
    #20 GetNonNullCoinbaseChainlock(CBlockIndex const*) src/evo/cbtx.cpp:463:10 (dashd+0xc73125) (BuildId: caa064d989fe6f8ddf115c37f17007602b71b63d)
    #21 llmq::utils::GetHashModifier(Consensus::LLMQParams const&, gsl::not_null<CBlockIndex const*>) src/llmq/utils.cpp:83:21 (dashd+0x46b664) (BuildId: caa064d989fe6f8ddf115c37f17007602b71b63d)
    #22 ComputeQuorumMembers src/llmq/utils.cpp:188:27 (dashd+0x462ae1) (BuildId: caa064d989fe6f8ddf115c37f17007602b71b63d)
    #23 llmq::utils::GetAllQuorumMembers(Consensus::LLMQType, CDeterministicMNManager&, gsl::not_null<CBlockIndex const*>, bool) src/llmq/utils.cpp:167:25 (dashd+0x462ae1)
    #24 llmq::CQuorumManager::BuildQuorumFromCommitment(Consensus::LLMQType, gsl::not_null<CBlockIndex const*>, bool) const src/llmq/quorums.cpp:410:20 (dashd+0x30d76b) (BuildId: caa064d989fe6f8ddf115c37f17007602b71b63d)
    #25 llmq::CQuorumManager::GetQuorum(Consensus::LLMQType, gsl::not_null<CBlockIndex const*>, bool) const src/llmq/quorums.cpp:672:12 (dashd+0x3142a9) (BuildId: caa064d989fe6f8ddf115c37f17007602b71b63d)
    #26 llmq::CQuorumManager::RequestQuorumData(CNode*, Consensus::LLMQType, CBlockIndex const*, unsigned short, uint256 const&) const src/llmq/quorums.cpp:494:9 (dashd+0x312053) (BuildId: caa064d989fe6f8ddf115c37f17007602b71b63d)
    #27 operator() src/rpc/quorums.cpp:821:31 (dashd+0x9a38a9) (BuildId: caa064d989fe6f8ddf115c37f17007602b71b63d)
    #28 __invoke_impl<bool, (lambda at rpc/quorums.cpp:820:36) &, CNode *> /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/invoke.h:61:14 (dashd+0x9a38a9)
    #29 __invoke_r<bool, (lambda at rpc/quorums.cpp:820:36) &, CNode *> /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/invoke.h:114:9 (dashd+0x9a38a9)
    #30 std::_Function_handler<bool (CNode*), quorum_getdata()::$_0::operator()(RPCHelpMan const&, JSONRPCRequest const&) const::'lambda'(CNode*)>::_M_invoke(std::_Any_data const&, CNode*&&) /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/std_function.h:290:9 (dashd+0x9a38a9)
    #31 operator() /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/std_function.h:590:9 (dashd+0x511210) (BuildId: caa064d989fe6f8ddf115c37f17007602b71b63d)
    #32 CConnman::ForNode(long, std::function<bool (CNode const*)>, std::function<bool (CNode*)>) src/net.cpp:5109:47 (dashd+0x511210)
    #33 ForNode<(lambda at rpc/quorums.cpp:820:36)> src/./net.h:1329:16 (dashd+0x9a3388) (BuildId: caa064d989fe6f8ddf115c37f17007602b71b63d)
    #34 operator() src/rpc/quorums.cpp:820:20 (dashd+0x9a3388)
    #35 __invoke_impl<UniValue, (lambda at rpc/quorums.cpp:793:9) &, const RPCHelpMan &, const JSONRPCRequest &> /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/invoke.h:61:14 (dashd+0x9a3388)
    #36 __invoke_r<UniValue, (lambda at rpc/quorums.cpp:793:9) &, const RPCHelpMan &, const JSONRPCRequest &> /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/invoke.h:114:9 (dashd+0x9a3388)
    #37 std::_Function_handler<UniValue (RPCHelpMan const&, JSONRPCRequest const&), quorum_getdata()::$_0>::_M_invoke(std::_Any_data const&, RPCHelpMan const&, JSONRPCRequest const&) /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/std_function.h:290:9 (dashd+0x9a3388)
    #38 operator() /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/std_function.h:590:9 (dashd+0x133e9d3) (BuildId: caa064d989fe6f8ddf115c37f17007602b71b63d)
    #39 RPCHelpMan::HandleRequest(JSONRPCRequest const&) const src/rpc/util.cpp:540:12 (dashd+0x133e9d3)
    #40 CRPCCommand::CRPCCommand(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>, RPCHelpMan (*)())::'lambda'(JSONRPCRequest const&, UniValue&, bool)::operator()(JSONRPCRequest const&, UniValue&, bool) const src/./rpc/server.h:107:91 (dashd+0x7faace) (BuildId: caa064d989fe6f8ddf115c37f17007602b71b63d)
    #41 __invoke_impl<bool, (lambda at ./rpc/server.h:107:15) &, const JSONRPCRequest &, UniValue &, bool> /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/invoke.h:61:14 (dashd+0x7fa98d) (BuildId: caa064d989fe6f8ddf115c37f17007602b71b63d)
    #42 __invoke_r<bool, (lambda at ./rpc/server.h:107:15) &, const JSONRPCRequest &, UniValue &, bool> /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/invoke.h:114:9 (dashd+0x7fa98d)
    #43 std::_Function_handler<bool (JSONRPCRequest const&, UniValue&, bool), CRPCCommand::CRPCCommand(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>, RPCHelpMan (*)())::'lambda'(JSONRPCRequest const&, UniValue&, bool)>::_M_invoke(std::_Any_data const&, JSONRPCRequest const&, UniValue&, bool&&) /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/std_function.h:290:9 (dashd+0x7fa98d)
    #44 operator() /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/std_function.h:590:9 (dashd+0xa1eb0c) (BuildId: caa064d989fe6f8ddf115c37f17007602b71b63d)
    #45 ExecuteCommand src/rpc/server.cpp:622:20 (dashd+0xa1eb0c)
    #46 ExecuteCommands(std::__debug::vector<CRPCCommand const*, std::allocator<CRPCCommand const*>> const&, JSONRPCRequest const&, UniValue&, std::__debug::multimap<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>, std::__debug::vector<UniValue, std::allocator<UniValue>>, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>>, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const, std::__debug::vector<UniValue, std::allocator<UniValue>>>>> const&) src/rpc/server.cpp:511:13 (dashd+0xa1eb0c)
    #47 CRPCTable::execute(JSONRPCRequest const&) const src/rpc/server.cpp:543:13 (dashd+0xa1de0b) (BuildId: caa064d989fe6f8ddf115c37f17007602b71b63d)
    #48 HTTPReq_JSONRPC(std::variant<std::monostate, std::reference_wrapper<ArgsManager>, std::reference_wrapper<NodeContext>, std::reference_wrapper<WalletContext>, std::reference_wrapper<CTxMemPool>, std::reference_wrapper<ChainstateManager>, std::reference_wrapper<CBlockPolicyEstimator>, std::reference_wrapper<LLMQContext>> const&, HTTPRequest*) src/httprpc.cpp:244:40 (dashd+0xe18b8b) (BuildId: caa064d989fe6f8ddf115c37f17007602b71b63d)
    #49 operator() src/httprpc.cpp:338:80 (dashd+0xe16d7d) (BuildId: caa064d989fe6f8ddf115c37f17007602b71b63d)
    #50 __invoke_impl<bool, (lambda at httprpc.cpp:338:23) &, HTTPRequest *, const std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > &> /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/invoke.h:61:14 (dashd+0xe16d7d)
    #51 __invoke_r<bool, (lambda at httprpc.cpp:338:23) &, HTTPRequest *, const std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > &> /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/invoke.h:114:9 (dashd+0xe16d7d)
    #52 std::_Function_handler<bool (HTTPRequest*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&), StartHTTPRPC(std::variant<std::monostate, std::reference_wrapper<ArgsManager>, std::reference_wrapper<NodeContext>, std::reference_wrapper<WalletContext>, std::reference_wrapper<CTxMemPool>, std::reference_wrapper<ChainstateManager>, std::reference_wrapper<CBlockPolicyEstimator>, std::reference_wrapper<LLMQContext>> const&)::$_0>::_M_invoke(std::_Any_data const&, HTTPRequest*&&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&) /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/std_function.h:290:9 (dashd+0xe16d7d)
    #53 operator() /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/std_function.h:590:9 (dashd+0xe32681) (BuildId: caa064d989fe6f8ddf115c37f17007602b71b63d)
    #54 HTTPWorkItem::operator()() src/httpserver.cpp:52:9 (dashd+0xe32681)
    #55 WorkQueue<HTTPClosure>::Run() src/httpserver.cpp:124:13 (dashd+0xe34e26) (BuildId: caa064d989fe6f8ddf115c37f17007602b71b63d)
    #56 HTTPWorkQueueRun(WorkQueue<HTTPClosure>*, int) src/httpserver.cpp:372:12 (dashd+0xe2a70b) (BuildId: caa064d989fe6f8ddf115c37f17007602b71b63d)
    #57 __invoke_impl<void, void (*)(WorkQueue<HTTPClosure> *, int), WorkQueue<HTTPClosure> *, int> /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/invoke.h:61:14 (dashd+0xe3680d) (BuildId: caa064d989fe6f8ddf115c37f17007602b71b63d)
    #58 __invoke<void (*)(WorkQueue<HTTPClosure> *, int), WorkQueue<HTTPClosure> *, int> /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/invoke.h:96:14 (dashd+0xe3680d)
    #59 _M_invoke<0UL, 1UL, 2UL> /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/std_thread.h:259:13 (dashd+0xe3680d)
    #60 operator() /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/std_thread.h:266:11 (dashd+0xe3680d)
    #61 std::thread::_State_impl<std::thread::_Invoker<std::tuple<void (*)(WorkQueue<HTTPClosure>*, int), WorkQueue<HTTPClosure>*, int>>>::_M_run() /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/std_thread.h:211:13 (dashd+0xe3680d)
    #62 <null> <null> (libstdc++.so.6+0xdc252) (BuildId: e37fe1a879783838de78cbc8c80621fa685d58a2)
SUMMARY: ThreadSanitizer: signal-unsafe call inside of a signal (/builds/dashpay/dash/build-ci/dashcore-linux64_tsan/src/dashd+0x1ab367) (BuildId: caa064d989fe6f8ddf115c37f17007602b71b63d) in operator new(unsigned long)
================== Early exiting after test failure

knst

utACK 09058e0

tsan failure unrelated, see #6470

…ustom changes Assertion failed: detected inconsistent lock order for 'cs_main' in node/blockstorage.cpp:775 (in thread 'httpworker.2'), details in debug log. Previous lock order was: (2) 'cs_main' in rpc/net.cpp:666 (in thread 'httpworker.3') (1) 'm_nodes_mutex' in net.cpp:4754 (in thread 'httpworker.3') Current lock order is: (1) 'm_nodes_mutex' in net.cpp:5102 (in thread 'httpworker.2') (2) 'cs_main' in node/blockstorage.cpp:775 (in thread 'httpworker.2') node0 2024-12-09T07:46:49.907123Z (mocktime: 2014-12-04T18:34:20Z) [httpworker.2] [stacktraces.cpp:528] [PrintCrashInfo] Posix Signal: Aborted ... 15#: (0x5764EB18DE42) sync.cpp:123 - potential_deadlock_detected 16#: (0x5764EB194A7E) sync.cpp:190 - push_lock<std::recursive_mutex> 17#: (0x5764EB194A7E) sync.cpp:214 - void EnterCritical<std::recursive_mutex>(char const*, char const*, int, std::recursive_mutex*, bool) 18#: (0x5764EA928642) unique_lock.h:150 - std::unique_lock<std::recursive_mutex>::try_lock() 19#: (0x5764EA928642) sync.h:168 - UniqueLock<AnnotatedMixin<std::recursive_mutex>, std::unique_lock<std::recursive_mutex> >::Enter(char const*, char const*, int) 20#: (0x5764EA928642) sync.h:190 - UniqueLock<AnnotatedMixin<std::recursive_mutex>, std::unique_lock<std::recursive_mutex> >::UniqueLock(AnnotatedMixin<std::recursive_mutex>&, char const*, char const*, int, bool) 21#: (0x5764EAAD9C3D) chain.h:214 - CBlockIndex::GetBlockPos() const 22#: (0x5764EAAD9C3D) blockstorage.cpp:775 - operator() 23#: (0x5764EAAD9C3D) blockstorage.cpp:775 - ReadBlockFromDisk(CBlock&, CBlockIndex const*, Consensus::Params const&) 24#: (0x5764EADB117A) stl_vector.h:1126 - std::vector<std::shared_ptr<CTransaction const>, std::allocator<std::shared_ptr<CTransaction const> > >::operator[](unsigned long) 25#: (0x5764EADB117A) cbtx.cpp:467 - GetNonNullCoinbaseChainlock(CBlockIndex const*) 26#: (0x5764EA9FE4F8) utils.cpp:84 - GetHashModifier 27#: (0x5764EAA05291) utils.cpp:189 - ComputeQuorumMembers 28#: (0x5764EAA05291) utils.cpp:167 - llmq::utils::GetAllQuorumMembers(Consensus::LLMQType, CDeterministicMNManager&, gsl::not_null<CBlockIndex const*>, bool) 29#: (0x5764EA9906FC) stl_vector.h:1258 - std::vector<std::shared_ptr<CDeterministicMN const>, std::allocator<std::shared_ptr<CDeterministicMN const> > >::data() 30#: (0x5764EA9906FC) span.h:164 - Span<std::shared_ptr<CDeterministicMN const> >::Span<std::vector<std::shared_ptr<CDeterministicMN const>, std::allocator<std::shared_ptr<CDeterministicMN const> > > >(std::vector<std::shared_ptr<CDeterministicMN const>, std::allocator<std::shared_ptr<CDeterministicMN const> > >&, std::enable_if<((!Span<std::shared_ptr<CDeterministicMN const> >::is_Span<std::vector<std::shared_ptr<CDeterministicMN const>, std::allocator<std::shared_ptr<CDeterministicMN const> > > >::value)&&std::is_convertible<std::remove_pointer<decltype ((((declval<std::vector<std::shared_ptr<CDeterministicMN const>, std::allocator<std::shared_ptr<CDeterministicMN const> > >&>)()).data)())>::type (*) [], std::shared_ptr<CDeterministicMN const> (*) []>::value)&&std::is_convertible<decltype ((((declval<std::vector<std::shared_ptr<CDeterministicMN const>, std::allocator<std::shared_ptr<CDeterministicMN const> > >&>)()).size)()), unsigned long>::value, decltype(nullptr)>::type) 31#: (0x5764EA9906FC) quorums.cpp:412 - llmq::CQuorumManager::BuildQuorumFromCommitment(Consensus::LLMQType, gsl::not_null<CBlockIndex const*>, bool) const 32#: (0x5764EA99198B) shared_ptr_base.h:1540 - std::__shared_ptr<llmq::CQuorum const, (__gnu_cxx::_Lock_policy)2>::__shared_ptr<llmq::CQuorum, void>(std::__shared_ptr<llmq::CQuorum, (__gnu_cxx::_Lock_policy)2>&&) 33#: (0x5764EA99198B) shared_ptr.h:369 - std::shared_ptr<llmq::CQuorum const>::shared_ptr<llmq::CQuorum, void>(std::shared_ptr<llmq::CQuorum>&&) 34#: (0x5764EA99198B) quorums.cpp:672 - llmq::CQuorumManager::GetQuorum(Consensus::LLMQType, gsl::not_null<CBlockIndex const*>, bool) const 35#: (0x5764EA991BD6) shared_ptr_base.h:1070 - std::__shared_count<(__gnu_cxx::_Lock_policy)2>::~__shared_count() 36#: (0x5764EA991BD6) shared_ptr_base.h:1524 - std::__shared_ptr<llmq::CQuorum const, (__gnu_cxx::_Lock_policy)2>::~__shared_ptr() 37#: (0x5764EA991BD6) shared_ptr.h:175 - std::shared_ptr<llmq::CQuorum const>::~shared_ptr() 38#: (0x5764EA991BD6) quorums.cpp:494 - llmq::CQuorumManager::RequestQuorumData(CNode*, Consensus::LLMQType, CBlockIndex const*, unsigned short, uint256 const&) const ...

…with custom changes 1b0af99 fix: wrong lock order, observed locally on top of #6467 with custom changes (Konstantin Akimov) Pull request description: ## Issue being fixed or feature implemented Assertion failed: detected inconsistent lock order for 'cs_main' in node/blockstorage.cpp:775 (in thread 'httpworker.2'), details in debug log. Previous lock order was: (2) 'cs_main' in rpc/net.cpp:666 (in thread 'httpworker.3') (1) 'm_nodes_mutex' in net.cpp:4754 (in thread 'httpworker.3') Current lock order is: (1) 'm_nodes_mutex' in net.cpp:5102 (in thread 'httpworker.2') (2) 'cs_main' in node/blockstorage.cpp:775 (in thread 'httpworker.2') node0 2024-12-09T07:46:49.907123Z (mocktime: 2014-12-04T18:34:20Z) [httpworker.2] [stacktraces.cpp:528] [PrintCrashInfo] Posix Signal: Aborted ... 15#: (0x5764EB18DE42) sync.cpp:123 - potential_deadlock_detected 16#: (0x5764EB194A7E) sync.cpp:190 - push_lock<std::recursive_mutex> 17#: (0x5764EB194A7E) sync.cpp:214 - void EnterCritical<std::recursive_mutex>(char const*, char const*, int, std::recursive_mutex*, bool) 18#: (0x5764EA928642) unique_lock.h:150 - std::unique_lock<std::recursive_mutex>::try_lock() 19#: (0x5764EA928642) sync.h:168 - UniqueLock<AnnotatedMixin<std::recursive_mutex>, std::unique_lock<std::recursive_mutex> >::Enter(char const*, char const*, int) 20#: (0x5764EA928642) sync.h:190 - UniqueLock<AnnotatedMixin<std::recursive_mutex>, std::unique_lock<std::recursive_mutex> >::UniqueLock(AnnotatedMixin<std::recursive_mutex>&, char const*, char const*, int, bool) 21#: (0x5764EAAD9C3D) chain.h:214 - CBlockIndex::GetBlockPos() const 22#: (0x5764EAAD9C3D) blockstorage.cpp:775 - operator() 23#: (0x5764EAAD9C3D) blockstorage.cpp:775 - ReadBlockFromDisk(CBlock&, CBlockIndex const*, Consensus::Params const&) 24#: (0x5764EADB117A) stl_vector.h:1126 - std::vector<std::shared_ptr<CTransaction const>, std::allocator<std::shared_ptr<CTransaction const> > >::operator[](unsigned long) 25#: (0x5764EADB117A) cbtx.cpp:467 - GetNonNullCoinbaseChainlock(CBlockIndex const*) 26#: (0x5764EA9FE4F8) utils.cpp:84 - GetHashModifier 27#: (0x5764EAA05291) utils.cpp:189 - ComputeQuorumMembers 28#: (0x5764EAA05291) utils.cpp:167 - llmq::utils::GetAllQuorumMembers(Consensus::LLMQType, CDeterministicMNManager&, gsl::not_null<CBlockIndex const*>, bool) 29#: (0x5764EA9906FC) stl_vector.h:1258 - std::vector<std::shared_ptr<CDeterministicMN const>, std::allocator<std::shared_ptr<CDeterministicMN const> > >::data() 30#: (0x5764EA9906FC) span.h:164 - Span<std::shared_ptr<CDeterministicMN const> >::Span<std::vector<std::shared_ptr<CDeterministicMN const>, std::allocator<std::shared_ptr<CDeterministicMN const> > > >(std::vector<std::shared_ptr<CDeterministicMN const>, std::allocator<std::shared_ptr<CDeterministicMN const> > >&, std::enable_if<((!Span<std::shared_ptr<CDeterministicMN const> >::is_Span<std::vector<std::shared_ptr<CDeterministicMN const>, std::allocator<std::shared_ptr<CDeterministicMN const> > > >::value)&&std::is_convertible<std::remove_pointer<decltype ((((declval<std::vector<std::shared_ptr<CDeterministicMN const>, std::allocator<std::shared_ptr<CDeterministicMN const> > >&>)()).data)())>::type (*) [], std::shared_ptr<CDeterministicMN const> (*) []>::value)&&std::is_convertible<decltype ((((declval<std::vector<std::shared_ptr<CDeterministicMN const>, std::allocator<std::shared_ptr<CDeterministicMN const> > >&>)()).size)()), unsigned long>::value, decltype(nullptr)>::type) 31#: (0x5764EA9906FC) quorums.cpp:412 - llmq::CQuorumManager::BuildQuorumFromCommitment(Consensus::LLMQType, gsl::not_null<CBlockIndex const*>, bool) const 32#: (0x5764EA99198B) shared_ptr_base.h:1540 - std::__shared_ptr<llmq::CQuorum const, (__gnu_cxx::_Lock_policy)2>::__shared_ptr<llmq::CQuorum, void>(std::__shared_ptr<llmq::CQuorum, (__gnu_cxx::_Lock_policy)2>&&) 33#: (0x5764EA99198B) shared_ptr.h:369 - std::shared_ptr<llmq::CQuorum const>::shared_ptr<llmq::CQuorum, void>(std::shared_ptr<llmq::CQuorum>&&) 34#: (0x5764EA99198B) quorums.cpp:672 - llmq::CQuorumManager::GetQuorum(Consensus::LLMQType, gsl::not_null<CBlockIndex const*>, bool) const 35#: (0x5764EA991BD6) shared_ptr_base.h:1070 - std::__shared_count<(__gnu_cxx::_Lock_policy)2>::~__shared_count() 36#: (0x5764EA991BD6) shared_ptr_base.h:1524 - std::__shared_ptr<llmq::CQuorum const, (__gnu_cxx::_Lock_policy)2>::~__shared_ptr() 37#: (0x5764EA991BD6) shared_ptr.h:175 - std::shared_ptr<llmq::CQuorum const>::~shared_ptr() 38#: (0x5764EA991BD6) quorums.cpp:494 - llmq::CQuorumManager::RequestQuorumData(CNode*, Consensus::LLMQType, CBlockIndex const*, unsigned short, uint256 const&) const ... ## What was done? Refactored call of GetQuorum out of `llmq::CQuorumManager::RequestQuorumData`. It also optimize `CQuorumManager::StartQuorumDataRecoveryThread` because avoid double call of `GetQuorum` ## How Has This Been Tested? Run with and without this changes. ## Breaking Changes You may observe error `RPC_INVALID_PARAMETER, "quorum not found"` while calling RPC `quorum getdata` instead returning `false` with log output only in case of request for non-existing quorum. The output is not documented at the moment anyway. ## Checklist: - [x] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have added or updated relevant unit/integration/functional/e2e tests - [ ] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone ACKs for top commit: UdjinM6: utACK 1b0af99 PastaPastaPasta: utACK 1b0af99 Tree-SHA512: b1955c7a2caad81f6c68299df513b4b83ff43f1d829d91769ac7d2a7b88985b5e7a86b765cfe90739ced9bac97da8fea23cd5c87cde1fca039d8b3f1a9c91084

…nauth test ec00c37 test: fix off-by-one in dynamically_add_masternode (UdjinM6) 6519856 test: don't add legacy bls mn on start (UdjinM6) 3db20e3 test: actually use masternode with basic bls pubkey in mnauth test (UdjinM6) Pull request description: ## Issue being fixed or feature implemented `rpc_manuth` is failing in ~50% cases locally because we still use legacy pubkeys (not in 100% cases because sometimes they look like basic ones). In CI it fails too but we retry failed tests a few times so it's less noticeable. Example of "unlucky" tests: https://gitlab.com/dashpay/dash/-/jobs/8613271300#L1867. #6467 follow-up ## What was done? Add another masternode after v19 activaition to actually use basic bls pubkey ## How Has This Been Tested? run tests ## Breaking Changes n/a ## Checklist: - [ ] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have added or updated relevant unit/integration/functional/e2e tests - [ ] I have made corresponding changes to the documentation - [ ] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ ACKs for top commit: knst: utACK ec00c37 PastaPastaPasta: utACK ec00c37 Tree-SHA512: 850a02ea1bd943762cdb0be706f3703742944c294ee9603b1f9ab95a6b10fb827bc9376e03333177d956b2c13df7384cfe0eb2ffef4d05ff3ec239caa8318d24

PastaPastaPasta added this to the 22.1 milestone Dec 8, 2024

PastaPastaPasta requested review from UdjinM6 and knst December 8, 2024 21:10

UdjinM6 reviewed Dec 8, 2024

View reviewed changes

Comment thread src/evo/mnauth.cpp Outdated

knst previously approved these changes Dec 9, 2024

View reviewed changes

PastaPastaPasta dismissed knst’s stale review via 844f242 December 9, 2024 20:17

PastaPastaPasta force-pushed the always-mnauth-basic branch from fb9fdf9 to 844f242 Compare December 9, 2024 20:17

PastaPastaPasta requested review from UdjinM6 and knst December 9, 2024 20:17

knst reviewed Dec 9, 2024

View reviewed changes

Comment thread src/evo/mnauth.h Outdated

Copy link
Copy Markdown

Collaborator

knst Dec 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: also

-class CChain;

knst reviewed Dec 9, 2024

View reviewed changes

feat: mnauth always use basic scheme

09058e0

fixup: drop CChain from mnauth ProcessMessage feat: let RPC mnauth to generate only BASIC bls messages and fixes for rpc_mnauth.py and p2p_quorum_data.py refactor: drop unused ConstCBLSPublicKeyVersionWrapper

PastaPastaPasta force-pushed the always-mnauth-basic branch from 844f242 to 09058e0 Compare December 9, 2024 20:36

PastaPastaPasta requested a review from knst December 9, 2024 20:44

UdjinM6 approved these changes Dec 10, 2024

View reviewed changes

knst approved these changes Dec 10, 2024

View reviewed changes

PastaPastaPasta merged commit b197ffb into dashpay:develop Dec 10, 2024

PastaPastaPasta deleted the always-mnauth-basic branch December 10, 2024 15:40

UdjinM6 mentioned this pull request Dec 11, 2024

test: actually use masternode with basic bls pubkey in mnauth test #6481

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: mnauth always use basic scheme#6467

feat: mnauth always use basic scheme#6467
PastaPastaPasta merged 1 commit into
dashpay:developfrom
PastaPastaPasta:always-mnauth-basic

PastaPastaPasta commented Dec 8, 2024

Uh oh!

Uh oh!

knst left a comment •

edited

Loading

Uh oh!

knst Dec 9, 2024

Uh oh!

PastaPastaPasta commented Dec 9, 2024

Uh oh!

knst Dec 9, 2024

Uh oh!

knst Dec 9, 2024

Uh oh!

PastaPastaPasta Dec 9, 2024

Uh oh!

UdjinM6 left a comment

Uh oh!

knst commented Dec 10, 2024

Uh oh!

knst left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

PastaPastaPasta commented Dec 8, 2024

Issue being fixed or feature implemented

What was done?

How Has This Been Tested?

Breaking Changes

Checklist:

Uh oh!

Uh oh!

knst left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

knst Dec 9, 2024

Choose a reason for hiding this comment

Uh oh!

PastaPastaPasta commented Dec 9, 2024

Uh oh!

knst Dec 9, 2024

Choose a reason for hiding this comment

Uh oh!

knst Dec 9, 2024

Choose a reason for hiding this comment

Uh oh!

PastaPastaPasta Dec 9, 2024

Choose a reason for hiding this comment

Uh oh!

UdjinM6 left a comment

Choose a reason for hiding this comment

Uh oh!

knst commented Dec 10, 2024

Uh oh!

knst left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

knst left a comment •

edited

Loading