feat: generalize ehf activation by panleone · Pull Request #5824 · dashpay/dash

panleone · 2024-01-14T11:10:25Z

Issue being fixed or feature implemented

Try to sign/mine any ehf deployment and not only mn_rr. As asked in the review this decouples (and improves) commit e24cb23 from PR #5799

What was done?

See commit description

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

src/llmq/ehf_signals.cpp

knst · 2024-01-14T11:46:27Z

you can add also changes from test/functional/test_framework/test_framework.py related to s/activate_mn_rr/activate_ehf/ to this PR

knst

utACK

UdjinM6

LGTM in general 👍 however I'd like to squash few lines to make no-whitespaces diff look a bit more compact

src/llmq/ehf_signals.cpp

UdjinM6

👍

utACK

knst

re-utACK

src/llmq/ehf_signals.cpp

UdjinM6

re-utACK

PastaPastaPasta · 2024-01-31T17:01:54Z

@knst Can you please help to coordinate a devnet test with this build and 20.0.4 ensuring that both activate EHF as expected and no issues are detected?

DashCoreAutoGuix · 2024-01-31T17:24:59Z

Guix Automation has failed due to the HEAD commit not being signed by an authorized core-team member. Please rebase and sign or push a new empty signed commit to allow Guix build to happen.

PastaPastaPasta · 2024-01-31T17:30:27Z

Can't push a commit here due to lack of permission issues @panleone; please check this box; or I'd have to create a new PR in order to get a build here

panleone · 2024-01-31T19:28:54Z

Can't push a commit here due to lack of permission issues @panleone; please check this box; or I'd have to create a new PR in order to get a build here

hmm looks this option is not available since I forked dash from an organisation (https://stackoverflow.com/questions/75596018/allow-edits-from-maintainers-does-not-work-when-the-pr-is-created-by-a-group)

github-actions · 2024-02-03T05:16:55Z

This pull request has conflicts, please rebase.

Try to sign any ehf deployment that can be activated and that hasn't been mined on chain yet. Also when receiving a new recovered signature try to match it with any ehf deployment which hasn't been mined on chain yet

To avoid copy and pasting future ehf deployments activation functions

DashCoreAutoGuix · 2024-02-08T15:56:05Z

Guix Automation has failed due to the HEAD commit not being signed by an authorized core-team member. Please rebase and sign or push a new empty signed commit to allow Guix build to happen.

panleone · 2024-02-08T16:11:25Z

had to rebase since there was a conflict
relevant diff is here
https://github.com/dashpay/dash/compare/0fc3599cf65ef3fc6c191e6345604d13de34354b..1821d92b66ad570508b9c4318283fba7e21ecfc7
(in the file ehf_signals.cpp )

UdjinM6

rebase looks clean, re-utACK

knst

tACK

Details here: #5857

PastaPastaPasta

utACK for merge via merge commit

knst reviewed Jan 14, 2024

View reviewed changes

src/llmq/ehf_signals.cpp Outdated Show resolved Hide resolved

src/llmq/ehf_signals.cpp Outdated Show resolved Hide resolved

knst added this to the 20.1 milestone Jan 14, 2024

panleone force-pushed the generalized_ehf branch from 5f12f6d to dac9cb3 Compare January 14, 2024 12:47

knst previously approved these changes Jan 14, 2024

View reviewed changes

knst requested review from PastaPastaPasta and UdjinM6 January 14, 2024 19:16

UdjinM6 requested changes Jan 14, 2024

View reviewed changes

src/llmq/ehf_signals.cpp Outdated Show resolved Hide resolved

src/llmq/ehf_signals.cpp Outdated Show resolved Hide resolved

src/llmq/ehf_signals.cpp Outdated Show resolved Hide resolved

panleone dismissed knst’s stale review via 778d2bc January 15, 2024 07:42

panleone force-pushed the generalized_ehf branch 2 times, most recently from 778d2bc to 721770e Compare January 15, 2024 07:58

UdjinM6 previously approved these changes Jan 15, 2024

View reviewed changes

knst previously approved these changes Jan 15, 2024

View reviewed changes

PastaPastaPasta requested changes Jan 28, 2024

View reviewed changes

src/llmq/ehf_signals.cpp Outdated Show resolved Hide resolved

src/llmq/ehf_signals.cpp Outdated Show resolved Hide resolved

panleone dismissed stale reviews from knst and UdjinM6 via 0fc3599 January 30, 2024 14:14

panleone force-pushed the generalized_ehf branch from 721770e to 0fc3599 Compare January 30, 2024 14:14

UdjinM6 previously approved these changes Jan 30, 2024

View reviewed changes

PastaPastaPasta added the guix-build label Jan 31, 2024

github-actions bot added the needs rebase label Feb 3, 2024

panleone mentioned this pull request Feb 3, 2024

feat: implement the dash analogue to BIP143 #5860

Open

5 tasks

panleone added 2 commits February 8, 2024 16:55

feat(consensus): Generalize ehf activation

de38dca

Try to sign any ehf deployment that can be activated and that hasn't been mined on chain yet. Also when receiving a new recovered signature try to match it with any ehf deployment which hasn't been mined on chain yet

test: add activate_ehf_by_name

1821d92

To avoid copy and pasting future ehf deployments activation functions

panleone dismissed UdjinM6’s stale review via 1821d92 February 8, 2024 15:56

panleone force-pushed the generalized_ehf branch from 0fc3599 to 1821d92 Compare February 8, 2024 15:56

github-actions bot removed the needs rebase label Feb 8, 2024

UdjinM6 approved these changes Feb 8, 2024

View reviewed changes

knst approved these changes Feb 16, 2024

View reviewed changes

knst mentioned this pull request Feb 16, 2024

Guix Build for Generalized ehf #5857

Closed

5 tasks

knst requested a review from PastaPastaPasta February 16, 2024 22:10

PastaPastaPasta approved these changes Feb 29, 2024

View reviewed changes

PastaPastaPasta merged commit 7701948 into dashpay:develop Feb 29, 2024

Conversation

panleone commented Jan 14, 2024 • edited by knst Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Issue being fixed or feature implemented

What was done?

How Has This Been Tested?

Breaking Changes

Checklist:

Uh oh!

Uh oh!

Uh oh!

knst commented Jan 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

knst left a comment

Choose a reason for hiding this comment

Uh oh!

UdjinM6 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

UdjinM6 left a comment

Choose a reason for hiding this comment

Uh oh!

knst left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

UdjinM6 left a comment

Choose a reason for hiding this comment

Uh oh!

PastaPastaPasta commented Jan 31, 2024

Uh oh!

DashCoreAutoGuix commented Jan 31, 2024

Uh oh!

PastaPastaPasta commented Jan 31, 2024

Uh oh!

panleone commented Jan 31, 2024

Uh oh!

github-actions bot commented Feb 3, 2024

Uh oh!

DashCoreAutoGuix commented Feb 8, 2024

Uh oh!

panleone commented Feb 8, 2024

Uh oh!

UdjinM6 left a comment

Choose a reason for hiding this comment

Uh oh!

knst left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PastaPastaPasta left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

panleone commented Jan 14, 2024 •

edited by knst

Loading

knst commented Jan 14, 2024 •

edited

Loading

knst left a comment •

edited

Loading