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(dashmate): remove sentinel service #1354

Merged
merged 12 commits into from
Sep 4, 2023
Merged

Conversation

strophy
Copy link
Collaborator

@strophy strophy commented Aug 31, 2023

Issue being fixed or feature implemented

Sentinel is now implemented as part of DashCore so sentinel service is deprecated and needs to be removed

What was done?

  • Remove sentinel
  • Add deprecatedrpc to dash.conf

How Has This Been Tested?

Locally with fullnodes/masternodes

Breaking Changes

None

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

For repository code-owners and collaborators only

  • I have assigned this pull request to a milestone

@strophy strophy added this to the v0.24.x milestone Aug 31, 2023
@strophy strophy requested a review from pshenmic August 31, 2023 06:50
Copy link
Member

@shumkov shumkov left a comment

Choose a reason for hiding this comment

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

We want to disable sentinel even if the node is a masternode?

@strophy
Copy link
Collaborator Author

strophy commented Aug 31, 2023

Yes, that was the request

@strophy strophy changed the base branch from master to v0.25-dev August 31, 2023 10:41
@strophy strophy changed the base branch from v0.25-dev to master August 31, 2023 10:49
@strophy strophy changed the base branch from master to v0.25-dev August 31, 2023 11:09
@strophy strophy changed the title feat: add sentinel enable toggle to config chore: remove sentinel Aug 31, 2023
@strophy strophy requested a review from shumkov August 31, 2023 12:14
@thephez
Copy link
Collaborator

thephez commented Aug 31, 2023

We want to disable sentinel even if the node is a masternode?

Yes, this corresponds to changes in Core v20 to pull sentinel functionality directly into Core (dashpay/dash#5525)

pshenmic
pshenmic previously approved these changes Sep 1, 2023
Copy link
Collaborator

@pshenmic pshenmic left a comment

Choose a reason for hiding this comment

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

LGTM, tested locally

@pshenmic pshenmic changed the title chore: remove sentinel feat(dashmate): remove sentinel Sep 1, 2023
pshenmic
pshenmic previously approved these changes Sep 1, 2023
Copy link
Collaborator

@ogabrielides ogabrielides left a comment

Choose a reason for hiding this comment

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

LGTM

antouhou
antouhou previously approved these changes Sep 1, 2023
Copy link
Collaborator

@antouhou antouhou left a comment

Choose a reason for hiding this comment

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

Looks good to me 👍

packages/dashmate/docker-compose.yml Outdated Show resolved Hide resolved
@strophy strophy dismissed stale reviews from pshenmic and antouhou via d4ad370 September 4, 2023 12:01
Copy link
Member

@shumkov shumkov left a comment

Choose a reason for hiding this comment

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

👍

@shumkov shumkov changed the title feat(dashmate): remove sentinel feat(dashmate): remove sentinel service Sep 4, 2023
@strophy strophy merged commit 8b9e35a into v0.25-dev Sep 4, 2023
12 checks passed
@strophy strophy deleted the feat/sentinel-config branch September 4, 2023 12:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants