feat: Superblock creation (Sentinel elimination) #5525

ogabrielides · 2023-08-03T13:59:46Z

Issue being fixed or feature implemented

Implementation of issue https://github.com/dashpay/dash-issues/issues/43

What was done?

Masternode will try to create, sign and submit a Superblock (GovTrigger) during the nSuperblockMaturityWindow.

How Has This Been Tested?

Breaking Changes

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)

src/chain.h

src/governance/governance.cpp

src/governance/classes.cpp

UdjinM6 · 2023-08-09T20:51:17Z

Note:

Check Merge Fast-Forward Only / check_merge (pull_request) Failing after 23s

needs rebase

UdjinM6

pls see 97ec634

NOTE: I'm not 100% sure about 97ec634628#diff-6787065e28efe13420b2f5e85125894595bec4c84b0b917d952dc86abd879033R534-R538 but we need a way to prevent mns from creating triggers all at the same time. Sentinel does this by introducing random delays. I don't think we can use delays in Core so limiting it to the current payee only is one possible solution that might work I think. This means it's only nSuperblockMaturityWindow MNs that could participate (1662 on mainnet). The chance all of them are broken/malicious and won't create any trigger is pretty low I think. But maybe there is a better idea.

github-actions · 2023-08-18T09:50:20Z

This pull request has conflicts, please rebase.

ogabrielides · 2023-08-18T11:42:46Z

pls see 97ec634

NOTE: I'm not 100% sure about 97ec634628#diff-6787065e28efe13420b2f5e85125894595bec4c84b0b917d952dc86abd879033R534-R538 but we need a way to prevent mns from creating triggers all at the same time. Sentinel does this by introducing random delays. I don't think we can use delays in Core so limiting it to the current payee only is one possible solution that might work I think. This means it's only nSuperblockMaturityWindow MNs that could participate (1662 on mainnet). The chance all of them are broken/malicious and won't create any trigger is pretty low I think. But maybe there is a better idea.

@UdjinM6 Well technically this means that most of the time, always the first payee during the nSuperblockMaturityWindow will manage to create the trigger. Since others MNs will vote for it.

UdjinM6 · 2023-08-18T15:30:19Z

LGTM but c5f6915 should be dropped and instead this PR should be rebased after #5538 is merged.

…l snapshot (#5538) ## Issue being fixed or feature implemented `-1` will only mean `not initialized` from now Should fix crashes like https://gitlab.com/dashpay/dash/-/jobs/4893359925 This fix applied on top of #5525 https://gitlab.com/UdjinM6/dash/-/pipelines/972154075 ## What was done? Introduce and use `m_initial_snapshot_index` instead of re-using `nHeight`. Added a couple of asserts to make sure: 1. we never create mn lists with `nHeight` set to `-1` _explicitly_ (but it's ok for ctor with no params to do so) 2. we never set `nHeight` to `-1` for an existing mn list 3. we never try to get a height for a non-initialized list 4. `GetListForBlockInternal` never returns non-initialized mn lists ## How Has This Been Tested? Run tests, run regtest/testnet wallets. ## Breaking Changes We never stored snapshots with `nHeight == -1`, should be no breaking changes I think. ## Checklist: - [x] 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 - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ogabrielides · 2023-08-22T08:25:05Z

@UdjinM6 Done

UdjinM6 · 2023-08-22T20:20:21Z

pls see b25de6d

ogabrielides · 2023-08-23T09:17:10Z

pls see b25de6d

@UdjinM6 done

UdjinM6

Looks good now I think. Sentinel does have some additional logic for picking "best" triggers and deleting ones with "worse" hashes. But that's only for triggers that had no our vote yet as far as I can tell which never actually happens now that we vote asap, so it should be safe to skip this part imo.

utACK

src/governance/classes.cpp

PastaPastaPasta

a few small comments

src/governance/governance.cpp

Co-Authored-By: PastaPastaPasta <[email protected]>

PastaPastaPasta

utACK for squash merge

UdjinM6

utACK

…5552) ## Issue being fixed or feature implemented With #5525 , MNs shouldn't use Sentinel anymore. ## What was done? In order to force them to remove Sentinel: - `gobject submit` RPC won't accept triggers anymore. - `gobject vote-conf` RPC isn't available anymore. ## How Has This Been Tested? `feature_governance.py` and `feature_governance_object.py` ## Breaking Changes Normally, only Sentinel should be broken. ## Checklist: - [x] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [x] I have added or updated relevant unit/integration/functional/e2e tests - [x] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ --------- Co-authored-by: UdjinM6 <[email protected]>

…et (#5560) ## Issue being fixed or feature implemented Since #5525, MNs during the maturity window, will propose new triggers. In `CGovernanceManager::CreateSuperblockCandidate`, SuperBlock creation is skipped when the bellow check is true: `if (nHeight % Params().GetConsensus().nSuperblockCycle < Params().GetConsensus().nSuperblockCycle - Params().GetConsensus().nSuperblockMaturityWindow) return std::nullopt; ` Hence, the value of `nSuperblockMaturityWindow` must be less than `nSuperblockCycle` and greater than 0. ## What was done? Changed `nSuperblockMaturityWindow` for devnet and Testnet chain parameters to the following values: `nSuperblockCycle` = 24 `nSuperblockMaturityWindow` = 8 ## How Has This Been Tested? ## Breaking Changes ## Checklist: - [x] 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 - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

## Issue being fixed or feature implemented ## What was done? Added release notes for #5525. ## How Has This Been Tested? ## Breaking Changes ## Checklist: - [x] 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 - [x] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ --------- Co-authored-by: PastaPastaPasta <[email protected]> Co-authored-by: UdjinM6 <[email protected]>

ogabrielides marked this pull request as draft August 3, 2023 14:00

ogabrielides force-pushed the superblock_creation branch from dd3a114 to 6b06e28 Compare August 3, 2023 15:48

ogabrielides marked this pull request as ready for review August 4, 2023 15:58

ogabrielides marked this pull request as draft August 4, 2023 18:00

UdjinM6 reviewed Aug 9, 2023

View reviewed changes

src/chain.h Outdated Show resolved Hide resolved

src/governance/governance.cpp Outdated Show resolved Hide resolved

src/governance/governance.cpp Show resolved Hide resolved

src/governance/governance.cpp Show resolved Hide resolved

ogabrielides marked this pull request as ready for review August 9, 2023 15:21

ogabrielides requested review from UdjinM6 and PastaPastaPasta August 9, 2023 15:21

UdjinM6 reviewed Aug 9, 2023

View reviewed changes

src/governance/classes.cpp Outdated Show resolved Hide resolved

ogabrielides force-pushed the superblock_creation branch from 4a99d60 to b4b24fa Compare August 10, 2023 07:47

ogabrielides requested a review from UdjinM6 August 16, 2023 12:11

UdjinM6 added this to the 20 milestone Aug 17, 2023

UdjinM6 requested changes Aug 17, 2023

View reviewed changes

github-actions bot added the needs rebase label Aug 18, 2023

ogabrielides force-pushed the superblock_creation branch from c2f1c2c to c5f6915 Compare August 18, 2023 09:55

github-actions bot removed the needs rebase label Aug 18, 2023

ogabrielides requested a review from UdjinM6 August 18, 2023 11:14

UdjinM6 mentioned this pull request Aug 18, 2023

fix: Do not use nHeight when trying to identify the very first/initial snapshot #5538

Merged

5 tasks

ogabrielides and others added 7 commits August 22, 2023 11:04

Masternode Superblock creation

85ee3d2

suggestions

700c826

do not copy

444e14d

comment

c83f381

linter fix

495497f

don't lock twice

900d017

Voting for triggers

560e0cb

UdjinM6 and others added 4 commits August 22, 2023 11:04

some suggestions

f330c3b

more suggestions

b44d205

fix

145bfbd

revert

05970c4

ogabrielides force-pushed the superblock_creation branch from c5f6915 to 05970c4 Compare August 22, 2023 08:22

more fixes and tests

8469265

UdjinM6 requested a review from knst August 23, 2023 13:12

UdjinM6 previously approved these changes Aug 23, 2023

View reviewed changes

knst reviewed Aug 24, 2023

View reviewed changes

src/governance/classes.cpp Show resolved Hide resolved

PastaPastaPasta reviewed Aug 28, 2023

View reviewed changes

src/governance/governance.cpp Outdated Show resolved Hide resolved

src/governance/governance.cpp Outdated Show resolved Hide resolved

suggestions

c5107ed

Co-Authored-By: PastaPastaPasta <[email protected]>

ogabrielides dismissed UdjinM6’s stale review via c5107ed August 28, 2023 14:28

ogabrielides requested a review from UdjinM6 August 28, 2023 14:33

PastaPastaPasta approved these changes Aug 28, 2023

View reviewed changes

UdjinM6 approved these changes Aug 29, 2023

View reviewed changes

PastaPastaPasta merged commit ceb84d5 into dashpay:develop Aug 29, 2023
9 checks passed

ogabrielides deleted the superblock_creation branch August 29, 2023 15:37

thephez mentioned this pull request Aug 29, 2023

Any interest in obsoleting this repo? dashpay/sentinel#77

Closed

ogabrielides mentioned this pull request Aug 30, 2023

feat(rpc): Disable submission of triggers and gobject vote-conf RPC #5552

Merged

5 tasks

PastaPastaPasta added the Needs Release Notes This PR includes breaking changes for which release notes have not yet been merged. label Aug 30, 2023

thephez mentioned this pull request Aug 31, 2023

feat(dashmate): remove sentinel service dashpay/platform#1354

Merged

5 tasks

This was referenced Aug 31, 2023

docs: Added release notes for #5525 #5557

Merged

fix: adjusted nSuperblockMaturityWindow to new logic for devnet/testnet #5560

Merged

thephez mentioned this pull request Sep 5, 2023

docs: v20 sentinel deprecation dashpay/docs#299

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Superblock creation (Sentinel elimination) #5525

feat: Superblock creation (Sentinel elimination) #5525

ogabrielides commented Aug 3, 2023 •

edited by PastaPastaPasta

Loading

UdjinM6 commented Aug 9, 2023

UdjinM6 left a comment

github-actions bot commented Aug 18, 2023

ogabrielides commented Aug 18, 2023

UdjinM6 commented Aug 18, 2023

ogabrielides commented Aug 22, 2023

UdjinM6 commented Aug 22, 2023

ogabrielides commented Aug 23, 2023

UdjinM6 left a comment

PastaPastaPasta left a comment

PastaPastaPasta left a comment

UdjinM6 left a comment

feat: Superblock creation (Sentinel elimination) #5525

feat: Superblock creation (Sentinel elimination) #5525

Conversation

ogabrielides commented Aug 3, 2023 • edited by PastaPastaPasta Loading

Issue being fixed or feature implemented

What was done?

How Has This Been Tested?

Breaking Changes

Checklist:

UdjinM6 commented Aug 9, 2023

UdjinM6 left a comment

Choose a reason for hiding this comment

github-actions bot commented Aug 18, 2023

ogabrielides commented Aug 18, 2023

UdjinM6 commented Aug 18, 2023

ogabrielides commented Aug 22, 2023

UdjinM6 commented Aug 22, 2023

ogabrielides commented Aug 23, 2023

UdjinM6 left a comment

Choose a reason for hiding this comment

PastaPastaPasta left a comment

Choose a reason for hiding this comment

PastaPastaPasta left a comment

Choose a reason for hiding this comment

UdjinM6 left a comment

Choose a reason for hiding this comment

ogabrielides commented Aug 3, 2023 •

edited by PastaPastaPasta

Loading