feat: Split quorum contrib data out of evodb

[UdjinM6]

Issue being fixed or feature implemented

Quorum data is not stored on-chain, it's a temporary data produced during dkg and should not be a part of evodb.

What was done?

Use new db in llmq/ to store quorum data, migrate old data to it.

How Has This Been Tested?

Run tests, run a node on testnet

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)

[github-actions]

This pull request has conflicts, please rebase.

[github-actions]

This pull request has conflicts, please rebase.

[github-actions]

This pull request has conflicts, please rebase.

[github-actions]

This pull request has conflicts, please rebase.

[thephez]

Is this still targeting v21?

[PastaPastaPasta]

utACK f6c7c20; this is just an implementation detail; as such I don't think it needs any release notes, and it doesn't introduce any backwards incompatibility that would require a reindex on downgrade.

[PastaPastaPasta]

Please get clang-diff happy :)

[UdjinM6]

Clang format differences found:
--- src/llmq/context.cpp	(before formatting)
+++ src/llmq/context.cpp	(after formatting)
@@ -18,[14](https://github.com/dashpay/dash/actions/runs/10372523552/job/28715446613?pr=5481#step:4:15) +18,17 @@
 #include <llmq/signing_shares.h>
 
 LLMQContext::LLMQContext(CChainState& chainstate, CConnman& connman, CDeterministicMNManager& dmnman, CEvoDB& evo_db,
-                         CMasternodeMetaMan& mn_metaman, CMNHFManager& mnhfman, CSporkManager& sporkman, CTxMemPool& mempool,
-                         const CActiveMasternodeManager* const mn_activeman, const CMasternodeSync& mn_sync,
-                         const std::unique_ptr<PeerManager>& peerman, bool unit_tests, bool wipe) :
+                         CMasternodeMetaMan& mn_metaman, CMNHFManager& mnhfman, CSporkManager& sporkman,
+                         CTxMemPool& mempool, const CActiveMasternodeManager* const mn_activeman,
+                         const CMasternodeSync& mn_sync, const std::unique_ptr<PeerManager>& peerman, bool unit_tests,
+                         bool wipe) :
     is_masternode{mn_activeman != nullptr},
     bls_worker{std::make_shared<CBLSWorker>()},
     dkg_debugman{std::make_unique<llmq::CDKGDebugManager>()},
     quorum_block_processor{std::make_unique<llmq::CQuorumBlockProcessor>(chainstate, dmnman, evo_db, peerman)},
-    qdkgsman{std::make_unique<llmq::CDKGSessionManager>(*bls_worker, chainstate, connman, dmnman, *dkg_debugman, mn_metaman, *quorum_block_processor, mn_activeman, sporkman, peerman, unit_tests, wipe)},
+    qdkgsman{std::make_unique<llmq::CDKGSessionManager>(*bls_worker, chainstate, connman, dmnman, *dkg_debugman,
+                                                        mn_metaman, *quorum_block_processor, mn_activeman, sporkman,
+                                                        peerman, unit_tests, wipe)},
     qman{std::make_unique<llmq::CQuorumManager>(*bls_worker, chainstate, connman, dmnman, *qdkgsman, evo_db,
                                                 *quorum_block_processor, mn_activeman, mn_sync, sporkman, unit_tests,
                                                 wipe)},
@@ -41,7 +44,8 @@
         llmq::quorumInstantSendManager = std::make_unique<llmq::CInstantSendManager>(*llmq::chainLocksHandler, chainstate, connman, *qman, *sigman, *shareman, sporkman, mempool, mn_sync, peerman, is_masternode, unit_tests, wipe);
         return llmq::quorumInstantSendManager.get();
     }()},
-    ehfSignalsHandler{std::make_unique<llmq::CEHFSignalsHandler>(chainstate, mnhfman, *sigman, *shareman, mempool, *qman, sporkman, peerman)}
+    ehfSignalsHandler{std::make_unique<llmq::CEHFSignalsHandler>(chainstate, mnhfman, *sigman, *shareman, mempool,
+                                                                 *qman, sporkman, peerman)}
 {
     // NOTE: we use this only to wipe the old db, do NOT use it for anything else
     // TODO: remove it in some future version

both suggestions are unrelated imo

EDIT: hmm.. there is a 3rd one, I think I did not have it locally, let me check

EDIT2: I have it locally but it's unrelated too :D

[PastaPastaPasta]

Yes; but you changed these files, so I think it's still best to add a fmt: run clang-format commit in this PR which makes it happy

[UdjinM6]

I'd prefer to keep this PR focused - clang diff script shouldn't be suggesting changes to unrelated lines in the first place, it's more of a bug in this case imo.

[PastaPastaPasta]

how are these changes not related?

 #include <llmq/signing_shares.h>
 
 LLMQContext::LLMQContext(CChainState& chainstate, CConnman& connman, CDeterministicMNManager& dmnman, CEvoDB& evo_db,
-                         CMasternodeMetaMan& mn_metaman, CMNHFManager& mnhfman, CSporkManager& sporkman, CTxMemPool& mempool,
-                         const CActiveMasternodeManager* const mn_activeman, const CMasternodeSync& mn_sync,
-                         const std::unique_ptr<PeerManager>& peerman, bool unit_tests, bool wipe) :
+                         CMasternodeMetaMan& mn_metaman, CMNHFManager& mnhfman, CSporkManager& sporkman,
+                         CTxMemPool& mempool, const CActiveMasternodeManager* const mn_activeman,
+                         const CMasternodeSync& mn_sync, const std::unique_ptr<PeerManager>& peerman, bool unit_tests,
+                         bool wipe) :
     is_masternode{mn_activeman != nullptr},
     bls_worker{std::make_shared<CBLSWorker>()},
     dkg_debugman{std::make_unique<llmq::CDKGDebugManager>()},
     quorum_block_processor{std::make_unique<llmq::CQuorumBlockProcessor>(chainstate, dmnman, evo_db, peerman)},
-    qdkgsman{std::make_unique<llmq::CDKGSessionManager>(*bls_worker, chainstate, connman, dmnman, *dkg_debugman, mn_metaman, *quorum_block_processor, mn_activeman, sporkman, peerman, unit_tests, wipe)},
+    qdkgsman{std::make_unique<llmq::CDKGSessionManager>(*bls_worker, chainstate, connman, dmnman, *dkg_debugman,
+                                                        mn_metaman, *quorum_block_processor, mn_activeman, sporkman,
+                                                        peerman, unit_tests, wipe)},
     qman{std::make_unique<llmq::CQuorumManager>(*bls_worker, chainstate, connman, dmnman, *qdkgsman, evo_db,
                                                 *quorum_block_processor, mn_activeman, mn_sync, sporkman, unit_tests,
                                                 wipe)},
@@ -41,7 +44,8 @@

[UdjinM6]

I never touched these lines while implementing the feature. Why should I refactor them? 🤷‍♂️

[PastaPastaPasta]

you touched these? https://github.com/dashpay/dash/pull/5481/files#diff-3ce892d5580c8044e0b4ebc4d1fe596d919788682fc38ccd66913508ff8fc43dL29-R31

[UdjinM6]

yes, and it doesn't complain about this one nvm, I was thinking in terms of lines 🙈

[PastaPastaPasta]

utACK 97d08e8

range-diff looks good

 -:  ---------- >  1:  fb00431b7c Merge bitcoin/bitcoin#22385: refactor: Use DeploymentEnabled to hide VB deployments
 -:  ---------- >  2:  8928146bfa Merge bitcoin/bitcoin#22550: test: improve `test_signing_with_{csv,cltv}` subtests (speed, prevent timeout)
 -:  ---------- >  3:  cbd2be8e18 Merge bitcoin/bitcoin#22597: consensus/params: simplify ValidDeployment check to avoid gcc warning
 -:  ---------- >  4:  fc25503cbc Merge bitcoin/bitcoin#22632: test: Set regtest.BIP66Height = 102 to speed up tests
 -:  ---------- >  5:  71af8816ef Merge bitcoin/bitcoin#22907: test: Avoid intermittent test failure in feature_csv_activation.py
 -:  ---------- >  6:  101a863399 Merge bitcoin/bitcoin#16333: test: Set BIP34Height = 2 for regtest
 -:  ---------- >  7:  adcf095ab9 Merge bitcoin/bitcoin#22718: doc: Add missing PR 16333 release note
 -:  ---------- >  8:  65b92fa093 Merge bitcoin/bitcoin#21862: test: Set regtest.BIP65Height = 111 to speed up tests
 -:  ---------- >  9:  e4e7c440f4 fix: use proper chain instead using ActiveChain for test framework
 -:  ---------- > 10:  cb3ac4656b ci: add more hosts to Github Actions
 -:  ---------- > 11:  3ec0c8ca0a fix: persist coinjoin denoms and sessions options from gui over restarts
 -:  ---------- > 12:  bf377d47e5 fix: correct is_snapshot_cs in VerifyDB
 1:  f6c7c20070 = 13:  59da9bc157 feat: Split quorum contribution db out of evodb
 -:  ---------- > 14:  97d08e8b1b refactor: make clang-format happy

[UdjinM6]

ping @knst

[UdjinM6]

ping @knst

ping x 2 @knst

[knst]

LGTM overall, but check comments, see 6b9fb14

%d is to fix, others are nits

[knst]

utACK ad35c1a

[PastaPastaPasta]

utACK ad35c1a