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.

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

Long Living Masternode Quorums - Part 3, DKG #2722

Merged
merged 34 commits into from
Feb 16, 2022

Conversation

random-zebra
Copy link

@random-zebra random-zebra commented Jan 14, 2022

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

As outlined in DIP 6, 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").

@furszy
Copy link

furszy commented Jan 19, 2022

Can rebase it on top of 2719 once more. Ready to start reviewing this beauty :)

@random-zebra
Copy link
Author

Rebased on #2728 (and including #2719)

@furszy
Copy link

furszy commented Jan 20, 2022

Have added test coverage for #2719 here: furszy@b1c7c78

If you cherry-pick it, and reorder the commits to be placed on top of 7334c3f, i could close #2719.

@random-zebra
Copy link
Author

If you cherry-pick it, and reorder the commits

done.

@random-zebra
Copy link
Author

Rebased on master.

furszy added a commit that referenced this pull request Jan 25, 2022
… 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
Copy link

@furszy furszy left a comment

Choose a reason for hiding this comment

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

First round, left some comments

src/llmq/quorums_dkgsession.cpp Outdated Show resolved Hide resolved
src/llmq/quorums_dkgsession.cpp Outdated Show resolved Hide resolved
src/llmq/quorums_dkgsessionhandler.cpp Outdated Show resolved Hide resolved
src/llmq/quorums_dkgsessionhandler.cpp Outdated Show resolved Hide resolved
src/llmq/quorums_dkgsessionhandler.cpp Outdated Show resolved Hide resolved
src/llmq/quorums_dkgsessionmgr.cpp Outdated Show resolved Hide resolved
src/llmq/quorums_debug.cpp Outdated Show resolved Hide resolved
src/rpc/rpcquorums.cpp Show resolved Hide resolved
@random-zebra
Copy link
Author

Thanks for the review, furszy. Feedback addressed, and rebased on master fixing conflicts.

@random-zebra random-zebra force-pushed the 202201_LLMQs branch 2 times, most recently from 94e75f9 to 59ad88d Compare January 31, 2022 08:49
@furszy
Copy link

furszy commented Jan 31, 2022

Great thanks. Second round: code wise is looking good.
Going to rebase my new GUI branch on top and see how the PoSe behaves for the next days.

@random-zebra
Copy link
Author

Added error simulation and relative functional test.

Copy link

@furszy furszy left a comment

Choose a reason for hiding this comment

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

awesome new test!👌

random-zebra and others added 19 commits February 15, 2022 17:49
>>> adapted from dash@d157718243222479c2e0e98208dff168e2a16bef +
                 dash@f44dd06eef21a5083528295aeb534cc3fa131f2b

* llqm: Fix thread handling in CDKGSessionManager and CDKGSessionHandler

* llmq: Removed unused thread_pool from CDKGSessionManager

* Tweak `CDKGSessionHandler::StartThread()`

* llmq: Simplify CDKGSessionHandler's thread naming

* llmq: Make sure CDKGSessionHandler uses a valid LLMQ type

* Having "const char*" leads to undefined behaviour if the "const char*"
is deallocated before the thread used it.


Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
* -Wrange-loop-analysis (VerifyLLMQCommitment)
* -Wpessimizing-move (CDKGPendingMessages::PopPendingMessages)
to set the error type and rate to simulate during the DKG of LLMQ_TEST
and refactor check_final_commitment out of mine_quorum
@random-zebra
Copy link
Author

Rebased due to conflicts with recently merged PRs

Copy link

@furszy furszy left a comment

Choose a reason for hiding this comment

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

rebase utACK f8a7dd8

As soon as this one gets merged, will push the GUI PR 🤘 .

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.

couple minor nits, otherwise ACK

src/rpc/misc.cpp Show resolved Hide resolved
src/cxxtimer.hpp Show resolved Hide resolved
@random-zebra
Copy link
Author

couple minor nits, otherwise ACK

Ok. Will fix them in a subsequent PR, so reviews don't get dismissed.

@Fuzzbawls
Copy link
Collaborator

sounds good :)

@random-zebra random-zebra merged commit 74cc314 into PIVX-Project:master Feb 16, 2022
furszy added a commit that referenced this pull request Feb 17, 2022
e2d3a4a Trivial: rename cxxtimer.hpp to cxxtimer.h (random-zebra)
21dedda RPC: fix mnconnect when first param is missing (random-zebra)

Pull request description:

  Tackling last nits left in #2722 (review)

ACKs for top commit:
  furszy:
    utACK e2d3a4a
  Fuzzbawls:
    ACK e2d3a4a

Tree-SHA512: cdda59b60914516b98436ce79649556d318b450690fd476c1cb9435842c3c2b510641465ac94bcdbdd398ac307ffed79ccf27cacfb6d53547bec673ca8994375
furszy added a commit that referenced this pull request Feb 23, 2022
… selection algorithm

b4e7fe1 Refactor: remove unneeded lambda in llmq::GetQuorumRelayMembers (random-zebra)
68c7d71 Refactor: remove code duplication in net_quorum_tests (random-zebra)
3d65f84 Refactor: assert forMemberIndex in GetQuorumRelayMembers and simplify (random-zebra)
2858d50 LLMQ: GetQuorumRelayMembers returning a single element for the special case of quorum size of two members. (furszy)
34a80fa Refactor: no need to double check the proTx in GetQuorumRelayMembers (random-zebra)
fb1c533 Fix for GetQuorumRelayMembers infinite loop. (furszy)
69cd4cc Test: validate GetQuorumRelayMembers algorithm and prove infinite loop. (furszy)
4bc864d Remove extra 'GetAllQuorumMembers' call from 'GetQuorumConnections' (furszy)
6e2f536 Faster and cleaner GetQuorumRelayMembers calculation. (furszy)
a854b89 CDKGSessionHandler: No need to walk-backwards through the chain to get the last quorum index. (furszy)
f8ea90c CDKGSession: remove not used messageHandlerPool field (furszy)
023c6ba CDKGSessionHandler::GetPhaseAndQuorumHash(), make return object more descriptive and readable. (furszy)

Pull request description:

  Follow-up work to my on-going review of #2722 (review). Starts in eeefe79.

  The current intra-quorum relay members selection algorithm receives a quorum member and selects a number of relay connections equals to the quorum size bits count.
  So, for example: each DMN, in a quorum of 400 members, deterministically selects 9 other members and mark them as intra-quorum-relay members.

  In the test LLMQ params case, with a quorum of 2, this algorithm falls into an infinite loop. Blocking the node's LLMQ session forever.
  The process cannot select the input member for intra-quorum relay and expects to return a minimum of 2 elements.

  Added a test case commit first so the issue can be easily verified/tested.

  Aside from that, added some other performance improvements and cleanups for the LLMQ sessions:

  1. Faster and cleaner GetQuorumRelayMembers calculation.
      1. No need to re-calculate the quorum members internally, the function' callers already have calculated the DMN list.
      2. As 'onlyOutbound' argument is always true. There is no need to loop over all the quorum members. The function only calculates the local DMN outbound relay connections.
  2.  Inside the `CDKGSessionHandler::UpdateBlockTip`: No need to walk-backwards through the chain to get the last quorum index. Can just look it up on chainActive`.
  3.  GetPhaseAndQuorumHash() returning an struct instead of a pair.
  4. Remove unused 'messageHandlerPool' field in the CDKGSession` class.

ACKs for top commit:
  random-zebra:
    obviously, ACK b4e7fe1

Tree-SHA512: 25c5795ecd1c721b4e29514bb17c5def0d41d45797fb4f2c700c5b9e04d339af07aa0632acd7f1c0559ecfabbf26375520a6d7bef929bbcd21181a62029ec709
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants