Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[TierTwo] Long Living Masternode Quorums - Part 2, network layer #2647

Merged
merged 33 commits into from
Jan 19, 2022
Merged

[TierTwo] Long Living Masternode Quorums - Part 2, network layer #2647

merged 33 commits into from
Jan 19, 2022

Conversation

furszy
Copy link

@furszy furszy commented Nov 24, 2021 •
edited
Loading

Built on top of #2607, #2632 and #2639. Focused on implementing the Masternode quorums network communication layer.

This layer is in charge of:

  1. Open MN-to-MN, MN-to-Quorum, MN-to-MN_iqr (intra-quorum relay) and MN-to-MN-probe connections.
  2. Implement a challenge-based connection authentication for MNs (new network message mnauth introduced).
  3. Cache and store MN connections metadata such as the last successful connection time and the last connection attempt time.
  4. Protect verified MN connections from eviction while they are part of the active quorum.
  5. Send interest in plain LLMQ recovered sigs (QSENDRECSIGS net message).

Plus, added a new functional test p2p_quorum_connect.py which basically verifies the proper functioning of most of this points (except the eviction process).

And finally, bumped the protocol version to 70925 so peers can know to whom they can, or not, send the new p2p messages.

@furszy furszy self-assigned this Nov 24, 2021
@furszy furszy force-pushed the 2021_net_llmq branch 2 times, most recently from e4d3fa6 to 459a71f Compare November 28, 2021 17:26
@furszy furszy mentioned this pull request Nov 28, 2021
Merged
@furszy furszy mentioned this pull request Nov 29, 2021
Merged
@furszy
Copy link
Author

furszy commented Nov 30, 2021

rebased on top of #2607, #2631, #2632, #2639, #2649 and #2657. Plus worked further on the circular dependencies problems and removed most of the masternode-sync.h includes that were all around the sources.
Restructuring the masternode synchronization, decoupling the state into a separate class tiertwo_sync_state which is only in charge of storing and providing the tier two synchronization status, without any synchronization logic (the sync logic remains inside the masternode-sync class).

@furszy furszy force-pushed the 2021_net_llmq branch 3 times, most recently from 24aa4e8 to f38f521 Compare December 1, 2021 19:13
@random-zebra random-zebra added this to the 6.0.0 milestone Dec 9, 2021
@furszy furszy mentioned this pull request Dec 14, 2021
Merged
furszy added a commit that referenced this pull request Dec 16, 2021
@furszy
Merge #2657: [TierTwo] Fix more circular dependencies.
1709f6b 
a02faf0 lint: remove fixed circular dependencies. (furszy)
16396fd net_processing, fix mnpayments-net_processing circ dependency. (furszy)
d57f060 net_processing: fix budgetman-net_processing circ dependency. (furszy)
1836bc7 net_processing, fix circ dependency: evo/mnauth -> llmq/quorums_utils -> spork -> net_processing -> evo/mnauth (furszy)
8bf4add net_processing, fix circ dependency: evo/mnauth -> masternode-sync -> masternodeman -> net_processing -> evo/mnauth. (furszy)
f85663a budget-wallet CreateBudgetFeeTX remove circ dependency. (furszy)

Pull request description:

  Fixed some circular dependencies over the tier two and net_processing due the cs_main/Misbehaving calls.

  Making this now, and not waiting to post-v6.0 (where we can directly remove the entire masternode manager file) because it affects the `mnauth` workflow introduced in #2647 which adds other circular dependencies.

ACKs for top commit:
  random-zebra:
    ACK a02faf0
  Fuzzbawls:
    ACK a02faf0

Tree-SHA512: ac04284420c0f6c2ae6742e53837d7f36c82ab0d5a8c011390a8d123790ddec71aaee8efdd9b4d94fde98930aed84761cc5dd100b56450ce4a5fa8e0cbec7ff7
@furszy furszy mentioned this pull request Dec 17, 2021
Merged
@furszy furszy force-pushed the 2021_net_llmq branch 2 times, most recently from d1cd285 to b99493c Compare December 27, 2021 18:18
furszy added a commit that referenced this pull request Dec 29, 2021
@furszy
Merge #2684: [Refactor] Move tier two managers initialization from in…
ab30cd2 
…it.cpp to tiertwo/init.cpp

d1fffb5 [Move-only] Move Masternodes thread creation from init.cpp to tiertwo/init.cpp (furszy)
8f7868f Remove completely broken "fLiteMode" (furszy)
c78bcd7 [Move-only] Move active masternode initialization from init.cpp to tiertwo/init.cpp (furszy)
a04bf92 [Move-only] Move tier two managers dump from init.cpp to a separate "tiertwo/init.cpp" (furszy)
77d8dcb [Move-only] Move tier two managers initialization from init.cpp to a separate "tiertwo/init.cpp" (furszy)

Pull request description:

  It's primarily a move-only refactoring, breaking up a bit more the long monolithic `init.cpp`. Placing the tier two components initialization in a separate `tiertwo/init.cpp` file.

  This will make future inclusions easier, like #2647 introducing the new tier two MN metadata manager and the network sync requests manager.

  And, as an extra cleanup, removed the broken `litemode` flag. Because, as is now, it simply makes no sense.. it's a "turn this on to fork your node".

ACKs for top commit:
  random-zebra:
    diff utACK d1fffb5
  Fuzzbawls:
    ACK d1fffb5

Tree-SHA512: ad2edd1dbc7007562fa55e61b73a50552d56aa186180b56cba3598435f607fcde2721fe0f7edb575ab4bc4ac907bcebfdf0e30a74139f99a3427dd790b959275
@furszy
Copy link
Author

furszy commented Jan 3, 2022

rebased on top of the recently rebased #2690. Once that one gets merged, this is ready to go.

@furszy furszy force-pushed the 2021_net_llmq branch 2 times, most recently from 6712c7f to d6a257e Compare January 3, 2022 20:52
furszy added a commit that referenced this pull request Jan 5, 2022
@furszy
Merge #2690: [Refactoring] Decouple and encapsulate tier two synchron…
017953e 
…ization state

3e05c82 [TierTwo] Request missing mnw signer during initial sync (random-zebra)
0ad6cd8 TierTwoSyncState: make every getter function const. (furszy)
c63d51c TierTwo: Do not lock g_best_block_mutex too often, only update chain sync status every 30 seconds. (furszy)
e90e27d Fix several compiler warning. (furszy)
1eb58ea Clean corrected masternode-sync circular dependencies. (furszy)
7e2f332 masternodesync: move seen maps to tiertwo_sync_state class and remove most of the masternode-sync dependencies. (furszy)
f29f9af [Refactoring] Unify AddedMasternode* functions (random-zebra)
c1091ab masternodesync: move sync asset phase to tiertwo_sync_state class. (furszy)
ca03c76 mnsync: decouple IsBlockchainSync into a publicly available g_tiertwo_sync_state class which only stores the final value. (furszy)
8ac4d91 mnsync: move sync reset due an hibernation client to CMasternodeSync::Process function. (furszy)

Pull request description:

  Work decoupled from #2647 which has grew a lot lately. So it can get reviewed and merged in parallel of #2607.

  Essentially have encapsulated the tier two synchronization status in a separate object which is only in charge of storing and retrieving the sync status members values in a synchronic manner.

  Under this new architecture, every manager/object that needs access to it can add a small dependency to the new `g_tiertwo_sync_state` object only (which has no other dependencies), without depending on the whole `masternodesSync` object (which is a bad name for the tier two sync manager, but.. renaming will come in another PR), which depends on every other tier two manager and lot other files.
  Plus, to not generate confusions, the `g_tiertwo_sync_state` members update are only, and exclusively, done by the tier two sync manager.

  Fixing in this way, at least (because there are surely more), the following problems:

  1) Circular dependencies:

     * "activemasternode -> masternode-sync -> activemasternode".
     * "budget/budgetmanager -> masternode-sync -> budget/budgetmanager"
     * "masternode -> masternode-sync -> masternode"
     * "masternode-payments -> masternode-sync -> masternode-payments"
     * "masternode-sync -> validation -> masternode-sync"
     * "evo/deterministicmns -> masternode -> masternode-sync -> evo/deterministicmns"

  2) Allow and guard concurrent access. (Prior to this work, the word "concurrency" was inexistent for this code).

ACKs for top commit:
  random-zebra:
    ACK 3e05c82 again. Let's get this merged.
  Fuzzbawls:
    ACK 3e05c82

Tree-SHA512: 77bbf9542265139a726c3bedda9274a24e8ee64a2e69c53984cafa7cb226fb6b4d86d1b0d6923e79d126a98a1cddd0aac3ab0db2657acaea82781a2a2c3918e7
random-zebra
random-zebra reviewed Jan 5, 2022
View reviewed changes
Copy link

@random-zebra random-zebra left a comment

Choose a reason for hiding this comment

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

Code ACK up to 3d9b2c1. Really nice job. Only nits and minor things so far.

src/tiertwo/init.cpp Outdated Show resolved Hide resolved
src/tiertwo/net_masternodes.cpp Outdated Show resolved Hide resolved
src/tiertwo/net_masternodes.cpp Outdated Show resolved Hide resolved
src/net.cpp Show resolved Hide resolved
src/rpc/client.cpp Outdated Show resolved Hide resolved
src/rpc/misc.cpp Outdated Show resolved Hide resolved
src/rpc/client.cpp Outdated Show resolved Hide resolved
src/rpc/client.cpp Outdated Show resolved Hide resolved
src/rpc/misc.cpp Outdated Show resolved Hide resolved
src/rpc/misc.cpp Outdated Show resolved Hide resolved
@furszy
net_mn: decouple g_args from the MN networking sources.
01ba363
@furszy
protocol: introduce QSENDRECSIGS net message.
1c16a2f
@furszy
net_mn: cache MN quorum relay members + let them know that we are int…
3897ae5 
…erested in plain LLMQ recovered signatures.
@furszy
RPC: add verified DMN connection info to 'getpeerinfo'
acefaa8
@furszy
bump protocol version to 70925
80d7d6b
@furszy
Test: add MN-to-MN, MN-to-quorum, MN-to-relay_members and probe MN co…
72ef11d 
…nnection test coverage
@furszy
net_mn: use string connection instead of addr to bypass the single IP…
13440cc 
… conn requirement.
@furszy
Node stats: add m_masternode_iqr_connection and m_masternode_probe_co…
6b4d616 
…nnection.
@furszy
net_mn: don't try to connect to ourselves.
a2b478f
@furszy
net_processing: try to update iqr member field if "qsendrecsigs" is r…
747f838 
…eceived.

Plus add spam protection for it as well and, as the message is relayed right after the verack and before the mnauth, move the code block above the mnauth first message check.
@furszy
net_processing: add missing return to the process verack msg flow.
de436d3 
And don't count getspork/spork messages in the mnauth flow.
@furszy
add -pushversion help message
a38fdf3
@furszy
net_mn: extract cs_main/Misbehaving from mnauth::ProcessMessage to ne…
038191c 
…t_processing.

fixes a circular dependency.
@furszy
mnsync: decouple IsBlockchainSync into a publicly available g_tiertwo…
6015d90 
…_sync_state class which only stores the final value.
@furszy
mn_net: store local DMN protxhash instead of access the activeMnManag…
693bff6 
…er object.

Removing the following circular dependency:
"activemasternode -> net -> tiertwo/net_masternodes -> activemasternode"
@furszy
Speed up functional tests decreasing the regtest tierTwo chain sync c…
b1a9637 
…heck up to the minimum.
@furszy
Define constants for MN metadata manager filename and magic string.
cb34b51 
Clean not needed mnconnect RPC command string args conversions.

And tackle variable names feedback.
@furszy
p2p: do not send MN connection flag and challenge in the version msg …
758e19f 
…if the connection is not meant to be a DMN-to-DMN.
@furszy
test: add coverage for regular peers disconnecting after receiving an…
b39f275 
… authenticated connection.
@furszy
RPC: document getpeerinfo quorum members values
ac014fb 
And add mnconnect arguments type checks and small command description.
@furszy
Copy link
Author

furszy commented Jan 17, 2022

Done, feedback tackled.

random-zebra
random-zebra approved these changes Jan 18, 2022
View reviewed changes
Copy link

@random-zebra random-zebra left a comment

Choose a reason for hiding this comment

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

utACK ac014fb

Fuzzbawls
Fuzzbawls approved these changes Jan 19, 2022
View reviewed changes
Copy link
Collaborator

@Fuzzbawls Fuzzbawls left a comment

Choose a reason for hiding this comment

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

ACK ac014fb

@furszy furszy merged commit b40facd into PIVX-Project:master Jan 19, 2022
furszy added a commit that referenced this pull request Jan 25, 2022
@furszy
Merge #2723: Finish moving tier two initialization code from init.cpp…
1eaa10c 
… to tiertwo/init.cpp and add disabledkg init flag

c445333 init: add flag to disable the DKG processes. (furszy)
6035112 init: move tier two objects initializers and the Masternodes collateral transactions' output lock in wallet to the tiertwo/init.cpp file. (furszy)
38b92b9 init: improve dmn collateral locking. (furszy)
918db4e init: move Masternode arguments inside tiertwo/init GetTierTwoHelpString (furszy)

Pull request description:

  Follow-up to #2684, built on top of #2647. Starting in 296e6fa.

  Focused on the following points:

  1) Move init arguments help messages to a new `GetTierTwoHelpString()`, the tier two objects initializers and the Masternodes collateral output locking process to the tiertwo/init files.
  2) Improve DMN collateral locking process:
     * Walking through the DMN list only once instead of one-time per wallet.
     * Removing the wallet dependency on `evo/deterministicmns.h`.
  3) Add `-disabledkg` init argument so the `p2p_quorum_connect.py` functional test does not get affected by the automatic DKG sessions processes (coming in #2722). The same flag will be used in other future tests that perform manual operations as well.

ACKs for top commit:
  random-zebra:
    utACK c445333
  Fuzzbawls:
    ACK c445333

Tree-SHA512: ed879a1b2ce840777bc9144297bfe76aa3184d6b3aede925c6bb2a8f6854a4b87b5acfa734d8f5da128cf5880a8927999f9c5798e17174f97b26f3a392e1b8cb
random-zebra added a commit that referenced this pull request Feb 16, 2022
@random-zebra
Merge #2722: Long Living Masternode Quorums - Part 3, DKG
74cc314 
f8a7dd8 [Trivial] Log quorum public keys using bech encoding (random-zebra)
cb8fc38 DKG: avoid extra member lookup. (furszy)
bf11238 QA: Implement tiertwo_dkg_errors functional test (random-zebra)
2e1d0c9 QA: fine grained control over expected messages in wait_for_dkg_phase (random-zebra)
f5d1940 Don't lie to yourself :) (Alexander Block)
b0b8f23 RPC: Implement quorumdkgsimerror command (random-zebra)
375b0dc RPC/Utils: add utility to parse UniValue arg as floating-point double (random-zebra)
57eb2a4 [Cleanup] Drop unused hash argument in CDKGSession::PreVerifyMessage (random-zebra)
25fe8d7 [Validation] Verify qfc in CDKGSession::FinalizeCommitments (random-zebra)
76497b7 Refactor: fix compiler warnings (random-zebra)
3ff7366 [Refactoring] Fix thread handling in CDKGSessionManager/Handler (xdustinface)
3588ead QA: Unify dmn setup code into test_framework (random-zebra)
79af97b Trivial: fix signed-unsigned comparisons in quorums_dkgsession (random-zebra)
2dbfa19 QA: add missing average durations and re-sort test_runner lists (random-zebra)
6280cc0 QA: Add tiertwo_dkg_pose functional test (random-zebra)
e993a7a Params: speed-up llmq_test (random-zebra)
839f14d P2P: Ensure intra-quorum connections and only relay to members (random-zebra)
87e962a RPC: Implement getquorummembers command (random-zebra)
29a9554 RPC: Implement getminedcommitment command (random-zebra)
ad874ba Refactor: remove net_processing dependency in quorums_dkgsessionmgr (random-zebra)
aae961c RPC: Implement quorumdkgstatus command (random-zebra)
8341489 QA: fix cs_main-lock for chainActive in deterministicmns_tests (random-zebra)
310c515 [TierTwo] Implement Quorums debug class (random-zebra)
e593cf5 [Trivial] Prefix all bls/quorum threads with pivx (random-zebra)
bd83342 Implement LLMQ DKG (Alexander Block)
50df807 [Net] Introduce NetMsgType for inter-quorum communication messages (random-zebra)
5c923cf [Refactoring] ActiveMasternode: get proTxHash and pointer to op key (random-zebra)
a99bdd8 [Utils] Add tiny header-only timer class (random-zebra)
90dd6ed [Utils] Introduce CBatchedLogger (random-zebra)
13b5c1b [Rand] Implement GetRandBool with rate (random-zebra)
9c0be19 Cleanup: Remove unused log categories (random-zebra)
918467b QA: Fix random timeout failures in p2p_quorum_connect functional test (random-zebra)
7dc448f net_mn: test regular connection refresh after selecting peer as quorum member (furszy)
70642d8 Net: refresh DMN connections when they are selected as quorum members (random-zebra)

Pull request description:

  This PR continues the work started with #2607 and #2647, implementing the distributed key generation protocol.

  As outlined in [DIP 6](https://github.com/dashpay/dips/blob/master/dip-0006.md), this is a decentralized BLS M-of-N threshold scheme, based on Shamir's Secrete Sharing, and it consists in 7 phases:

  1. **Initialization**: the members of the new quorum are determined.
  2. **Contribution**: each member generates his own contribution (verification vector and secret key-contribution) and receives/validates the contributions of all other members.
  3. **Complaining**: each member sends a complaint message with information about missing/invalid contributions in the previous phase, and receives/validates the complaint messages of all other members.
  4. **Justification**: each member that was previously complained about can justify for a valid contribution.
  5. **Commitment**: the members create their own threshold secret key share from the secret key-contributions received and build the quorum verification vector. Then each member relays a premature commitment message containing the quorum public key, the hash of the verification vector, and a bitset of the valid members.
  6. **Finalization**: each member collects the premature commitments received by all other (valid) members, aggregate them into a "final commitment", and relay it to all the nodes of the network (including non-masternodes).
  7. **Mining**: miners receive the final commitment message, build a `LLMQComm` special transaction, and mine a block with it.

  The 7-th phase has been already introduced in #2607.

  The communication between the participants of the DKG (phases 1 to 6) is supported by the following new P2P message types (which are exchanged only between the quorum members):
  - `qcontrib`
  - `qcomplaint`
  - `qjustify`
  - `qpcommit`

  Sessions of the DKG are implemented in the `CDKGSession` class and the flow of message calls is identical for all phases:
   - create/send own message
   - pre-verify incoming messages
   - collect preverified messages and perform batched BLS signature verification
   - call `ReceiveMessage`, which does additional validation

  Sessions are owned and called by `CDKGSessionHandler`, which manages multiple sequential sessions of one specific LLMQ type (there is one instance of this class per LLMQ type). It is responsible for starting the phase handler thread  which constantly loops, processing the received messages, calling the required function for the current phase (`CDKGSession::Contribute`, `CDKGSession::VerifyAndComplain`, `CDKGSession::VerifyAndJustify`, `CDKGSession::VerifyAndCommit`), and waiting for the next one if needed.

  A global `CDKGSessionManager` object keeps track of of all session handlers.

  New functional tests are introduced to check the status (messages sent and received) of the nodes during the phases of the DKG (`CDKGDebugSessionStatus` accessed via `quorumdkgstatus` RPC) and to verify the DKG effectiveness as proof-of-service (non participating nodes get eventually "PoSe banned").

ACKs for top commit:
  furszy:
    rebase utACK f8a7dd8

Tree-SHA512: a10223718caacbf67de67d70f9aeeea6ab91a584dd3b7c65f283f5e5c72a2ca15b1a1d380691128584805bc50237870f60ecc81acaa8698a273c20fba61b1e48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@random-zebra random-zebra random-zebra approved these changes

@Fuzzbawls Fuzzbawls Fuzzbawls approved these changes

Assignees

@furszy furszy

Labels
Masternode Network P2P
Projects
None yet
Milestone
6.0.0
Development

Successfully merging this pull request may close these issues.
3 participants
@furszy @Fuzzbawls @random-zebra