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(rpc): quorum dkginfo rpc #5853

Merged
merged 8 commits into from
Feb 1, 2024

Conversation

ogabrielides
Copy link
Collaborator

@ogabrielides ogabrielides commented Jan 31, 2024

Issue being fixed or feature implemented

Dashmate wanted a way to know if it is safe to restart the masternode.
This new RPC indicates the number of active DKG sessions, and the number of blocks until next potential DKG.

What was done?

Examples of responses:
{'active_dkgs': 0, 'next_dkg': 22}

How Has This Been Tested?

feature_llmq_rotation.py was updated

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 January 31, 2024 14:21
@ogabrielides ogabrielides marked this pull request as ready for review January 31, 2024 14:34
@ogabrielides ogabrielides added this to the 20.1 milestone Jan 31, 2024
@ogabrielides ogabrielides added RPC Some notable changes to RPC params/behaviour/descriptions guix-build labels Jan 31, 2024
@DashCoreAutoGuix
Copy link

Guix Automation has began to build this PR tagged as v20.1.0-devpr5853.8386a3d6. A new comment will be made when the image is pushed.

@DashCoreAutoGuix
Copy link

Guix Automation has completed; a release should be present here: https://github.com/dashpay/dash-dev-branches/releases/tag/v20.1.0-devpr5853.8386a3d6. The image should be on dockerhub soon.

@qwizzie
Copy link

qwizzie commented Jan 31, 2024

Note that this RPC is allowed only for masternodes

Does this mean this new RPC can not be used for Evonodes ? Is there a different approach to masternodes (this pull request) and evonodes (adds a check in stopNodeTaskFactory through a Platform pull request). Or are both pull requests focused just on masternodes for now ? See : dashpay/platform#1683

@thephez
Copy link
Collaborator

thephez commented Jan 31, 2024

Note that this RPC is allowed only for masternodes

Does this mean this new RPC can not be used for Evonodes ? Is there a different approach to masternodes (this pull request) and evonodes (adds a check in stopNodeTaskFactory through a Platform pull request). Or are both pull requests focused just on masternodes for now ? See : dashpay/platform#1683

I don't think so. In my mind (and I think how it's referred to here), an evonode is just a particular type of masternode. Regular MNs and evonodes are both subsets of the generic "masternode".

@kxcd
Copy link

kxcd commented Jan 31, 2024

WOW! This could be really useful, if it works !

@kxcd
Copy link

kxcd commented Jan 31, 2024

It is not better to slot this RPC under quorum ?

@ogabrielides
Copy link
Collaborator Author

It is not better to slot this RPC under quorum ?

Yes, you are right.

@ogabrielides ogabrielides changed the title feat(rpc): dkginfo rpc feat(rpc): quorum dkginfo rpc Jan 31, 2024
@thephez
Copy link
Collaborator

thephez commented Jan 31, 2024

It could be valuable to include nextDKG always That way you'd always know how far in the future the next DKG was (even if in the middle of DKG already).

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.

few suggestions, see below (all of them squashed 44467c0)

doc/release-notes-5853.md Outdated Show resolved Hide resolved
doc/release-notes-5853.md Outdated Show resolved Hide resolved
src/rpc/quorums.cpp Outdated Show resolved Hide resolved
src/rpc/quorums.cpp Outdated Show resolved Hide resolved
src/rpc/quorums.cpp Outdated Show resolved Hide resolved
test/functional/feature_llmq_rotation.py Outdated Show resolved Hide resolved
doc/release-notes-5853.md Outdated Show resolved Hide resolved
src/rpc/quorums.cpp Outdated Show resolved Hide resolved
src/rpc/quorums.cpp Outdated Show resolved Hide resolved
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.

👍

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 f5b7aa0 into dashpay:develop Feb 1, 2024
9 checks passed
@ogabrielides ogabrielides deleted the dkginfo_rpc branch February 1, 2024 15:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RPC Some notable changes to RPC params/behaviour/descriptions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants