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

feat: mnlistdiff v20 CL sig quorums #5377

Merged

Conversation

ogabrielides
Copy link
Collaborator

@ogabrielides ogabrielides commented May 17, 2023

Issue being fixed or feature implemented

Implementation of Randomness Beacon Part 3.

Starting from v20 activation fork, members for quorums are sorted using (if available) the best CL signature found in Coinbase.
If no CL signature is present yet, then the usual way is used (By using Blockhash instead)

The actual new way to shuffle is already implemented in #5366.

SPV clients also need to calculate members, but they only know block headers.
Since Coinbase is in the actual block, then they lack the required information to correctly calculate quorum members.

What was done?

  • Message MNLISTIDFF is enriched with a new field quorumsCLSigs. This field holds the Chainlock Signature required for each set of indexes corresponding to quorums in field newQuorums.
  • Protocol version has been bumped to 70230.
  • Clients with protocol version greater or equal to 70230 will receive the new field quorumsCLSigs.
  • The same field is returned in protx diff RPC.

Note:

  • Field quorumsCLSigs will populated only after v20 activation
  • If for one or more quorums, no non-null CL sig was found in CbTx then a null signature is returned in quorumsCLSigs.

How Has This Been Tested?

  • Functional test mininode's protocol version was bumped to 70230.
  • feature_llmq_rotation.py checks that quorumsCLSigs match in both P2P and RPC messages.

Breaking Changes

No

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)

@ogabrielides ogabrielides marked this pull request as draft May 17, 2023 18:00
@ogabrielides ogabrielides added RPC Some notable changes to RPC params/behaviour/descriptions P2P Some notable changes on p2p level labels May 17, 2023
@ogabrielides ogabrielides added this to the 20 milestone May 17, 2023
@ogabrielides ogabrielides force-pushed the mnlistdiff_quorums_clsigs_spv branch from c351674 to c0b8dfb Compare May 17, 2023 20:36
@ogabrielides ogabrielides marked this pull request as ready for review May 17, 2023 20:38
src/evo/simplifiedmns.cpp Outdated Show resolved Hide resolved
src/evo/simplifiedmns.cpp Outdated Show resolved Hide resolved
src/evo/simplifiedmns.cpp Outdated Show resolved Hide resolved
src/evo/simplifiedmns.h Outdated Show resolved Hide resolved
src/version.h Show resolved Hide resolved
src/evo/simplifiedmns.h Outdated Show resolved Hide resolved
@github-actions
Copy link

This pull request has conflicts, please rebase.

Copy link
Collaborator

@thephez thephez left a comment

Choose a reason for hiding this comment

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

A few minor suggestions

doc/release-notes-5377.md Outdated Show resolved Hide resolved
doc/release-notes-5377.md Outdated Show resolved Hide resolved
doc/release-notes-5377.md Outdated Show resolved Hide resolved
doc/release-notes-5377.md Outdated Show resolved Hide resolved
@ogabrielides ogabrielides force-pushed the mnlistdiff_quorums_clsigs_spv branch from 1306dcd to 8ebf00d Compare May 18, 2023 20:48
@ogabrielides ogabrielides requested review from UdjinM6 and thephez May 19, 2023 06:48
UdjinM6
UdjinM6 previously approved these changes May 20, 2023
Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

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

utACK

Copy link
Collaborator

@thephez thephez left a comment

Choose a reason for hiding this comment

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

Doc ACK 🙂

@github-actions
Copy link

This pull request has conflicts, please rebase.

doc/release-notes-5377.md Outdated Show resolved Hide resolved
@ogabrielides ogabrielides requested a review from UdjinM6 June 16, 2023 13:13
UdjinM6
UdjinM6 previously approved these changes Jun 16, 2023
Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

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

re-utACK

Copy link
Member

@PastaPastaPasta PastaPastaPasta left a comment

Choose a reason for hiding this comment

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

Please see f838f95 c02bac5 and 63efe7f

Then, I am concerned about testing for this, I tried to break the test but couldn't really. I think any of these changes should break the tests

00f1a42
7f3f385
5c04684

@ogabrielides ogabrielides force-pushed the mnlistdiff_quorums_clsigs_spv branch from 681b519 to cdd8e8e Compare June 28, 2023 16:22
@ogabrielides
Copy link
Collaborator Author

@PastaPastaPasta @UdjinM6

With the latest pushed change, quorum verification captures the disturbed returned CL caused by 7f3f385 5c04684.

Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

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

utACK

Copy link
Member

@PastaPastaPasta PastaPastaPasta left a comment

Choose a reason for hiding this comment

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

utACK for squash merge

@PastaPastaPasta PastaPastaPasta merged commit 494b5c7 into dashpay:develop Jul 10, 2023
@ogabrielides ogabrielides deleted the mnlistdiff_quorums_clsigs_spv branch July 11, 2023 13:04
thephez added a commit to thephez/docs-core that referenced this pull request Aug 16, 2023
thephez added a commit to dashpay/docs-core that referenced this pull request Aug 30, 2023
* docs: deprecate MSG_LEGACY_TXLOCK_REQUEST

Aligns with dashpay/dash#5483

* chore: update link to prev version of docs

* docs: update mnlistdiff nversion location

Relates to dashpay/dash#5450

* docs(p2p): update mnlistdiff

Relates to dashpay/dash#5377

* docs: update cbtx for v3

Relates to dashpay/dash#5262

* docs: update mnhf with details of final implementation

Relates to dashpay/dash#5469 and dashpay/dash#5505

* docs: note removal of NODE_GETUTXO

Relates to dashpay/dash#5500

* chore: revert "docs: update mnhf with details of final implementation"

This reverts commit 8e4bf6c since there
may still be additional changes to the implementation (it's not merged)
thephez added a commit to thephez/docs-core that referenced this pull request Sep 27, 2023
* docs: deprecate MSG_LEGACY_TXLOCK_REQUEST

Aligns with dashpay/dash#5483

* chore: update link to prev version of docs

* docs: update mnlistdiff nversion location

Relates to dashpay/dash#5450

* docs(p2p): update mnlistdiff

Relates to dashpay/dash#5377

* docs: update cbtx for v3

Relates to dashpay/dash#5262

* docs: update mnhf with details of final implementation

Relates to dashpay/dash#5469 and dashpay/dash#5505

* docs: note removal of NODE_GETUTXO

Relates to dashpay/dash#5500

* chore: revert "docs: update mnhf with details of final implementation"

This reverts commit 8e4bf6c since there
may still be additional changes to the implementation (it's not merged)
thephez added a commit to thephez/docs-core that referenced this pull request Oct 3, 2023
thephez added a commit to dashpay/docs-core that referenced this pull request Oct 3, 2023
* docs(rpc): update protx diff

Relates to dashpay/dash#5377

* docs(rpc): add getindexinfo rpc

Relates to dashpay/dash#5492

* docs(rpc): add gettxchainlocks

Relates to dashpay/dash#5578

* docs(rpc): update getblock
thephez added a commit to dashpay/docs-core that referenced this pull request Nov 15, 2023
* docs: deprecate MSG_LEGACY_TXLOCK_REQUEST

Aligns with dashpay/dash#5483

* chore: update link to prev version of docs

* docs: update mnlistdiff nversion location

Relates to dashpay/dash#5450

* docs(p2p): update mnlistdiff

Relates to dashpay/dash#5377

* docs: update cbtx for v3

Relates to dashpay/dash#5262

* docs: update mnhf with details of final implementation

Relates to dashpay/dash#5469 and dashpay/dash#5505

* docs: note removal of NODE_GETUTXO

Relates to dashpay/dash#5500

* chore: revert "docs: update mnhf with details of final implementation"

This reverts commit 8e4bf6c since there
may still be additional changes to the implementation (it's not merged)
thephez added a commit to dashpay/docs-core that referenced this pull request Nov 15, 2023
* docs(rpc): update protx diff

Relates to dashpay/dash#5377

* docs(rpc): add getindexinfo rpc

Relates to dashpay/dash#5492

* docs(rpc): add gettxchainlocks

Relates to dashpay/dash#5578

* docs(rpc): update getblock
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2P Some notable changes on p2p level RPC Some notable changes to RPC params/behaviour/descriptions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants