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: allow resigning for EHF #6175

Merged
merged 1 commit into from
Aug 7, 2024

Conversation

PastaPastaPasta
Copy link
Member

@PastaPastaPasta PastaPastaPasta commented Aug 5, 2024

Issue being fixed or feature implemented

Please see: https://www.dash.org/forum/index.php?threads/ehf-activation-issues.55146/ for a description of this issue

What was done?

Allow re-signing specifically for EHF messages, this is important as due to rotation, and duplicate requestIDs a member will refuse to re-sign when they participate in a later quorum.

How Has This Been Tested?

Devnet deployed with a mix of this version, older versions and current v21.0

Breaking Changes

This doesn't introduce any breaking changes

Checklist:

Go over all the following points, and put an x in all the boxes that apply.

  • 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)

@DashCoreAutoGuix
Copy link

Guix Automation has began to build this PR tagged as v21.0.2-devpr6175.0903883c. A new comment will be made when the image is pushed.

Copy link
Collaborator

@knst knst left a comment

Choose a reason for hiding this comment

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

LGTM 0903883

Do you want to pick extra logs from #6170 ?

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 0903883

@DashCoreAutoGuix
Copy link

Guix Automation has completed; a release should be present here: https://github.com/dashpay/dash-dev-branches/releases/tag/v21.0.2-devpr6175.0903883c. The image should be on dockerhub soon.

@PastaPastaPasta PastaPastaPasta changed the title Feat resign ehf feat: allow resigning for EHF Aug 7, 2024
LogPrintf("CSigningManager::%s -- already voted for id=%s and msgHash=%s. Not voting on conflicting msgHash=%s\n", __func__,
id.ToString(), prevMsgHash.ToString(), msgHash.ToString());
return false;
if (allowDiffMsgHashSigning) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: have a TODO here to drop it with v22 upgrade when requestID is changed to proper one?

@PastaPastaPasta PastaPastaPasta added this to the 21.1 milestone Aug 7, 2024
@PastaPastaPasta PastaPastaPasta marked this pull request as ready for review August 7, 2024 10:50
@DashCoreAutoGuix
Copy link

Guix Automation has began to build this PR tagged as v21.0.2-devpr6175.6f82a561. A new comment will be made when the image is pushed.

@knst knst removed the guix-build label Aug 7, 2024
@PastaPastaPasta PastaPastaPasta merged commit 3c6ef31 into dashpay:develop Aug 7, 2024
4 of 6 checks passed
@PastaPastaPasta PastaPastaPasta deleted the feat-resign-ehf branch August 7, 2024 11:06
PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this pull request Aug 7, 2024
6f82a56 feat: add option to AsyncSignIfMember to allow signing same requestID on different msgHashes (pasta)

Pull request description:

  ## Issue being fixed or feature implemented
  Please see: https://www.dash.org/forum/index.php?threads/ehf-activation-issues.55146/ for a description of this issue

  ## What was done?
  Allow re-signing specifically for EHF messages, this is important as due to rotation, and duplicate requestIDs a member will refuse to re-sign when they participate in a later quorum.

  ## How Has This Been Tested?
  Devnet deployed with a mix of this version, older versions and current v21.0

  ## Breaking Changes
  This doesn't introduce any breaking changes

  ## Checklist:
    _Go over all the following points, and put an `x` in all the boxes that apply._
  - [ ] 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)_

Top commit has no ACKs.

Tree-SHA512: a758e6363901624d8540983ad09085d47a33fe61e8e576ec4317b051a0f988efb983bd06d9894bcce19c9f587ad703022ce00ceddd3760d7c9d382878f40c2d7
PastaPastaPasta added a commit that referenced this pull request Aug 7, 2024
8e9dd12 chore: bump version in configure.ac (pasta)
afb8dc0 docs: add v21.1.0 release notes and archive v21.0.2 (pasta)
269dd02 Merge #6179: chore: update manpages v21.1 (pasta)
a8cb643 Merge #6175: feat: allow resigning for EHF (pasta)
de5cc22 Merge #6178: chore: bump protocol version to 70233 (pasta)
2de4ce5 Merge #6176: test: reduce BRRHeight in regtest (pasta)
840175e Merge #6174: fix: stop trying to sign pending txes when they are no longer non-locked (pasta)

Pull request description:

  ## Issue being fixed or feature implemented
  Backports for v21.1.0, add release notes and

  ## What was done?
  See commits

  ## How Has This Been Tested?
  Been on devnet

  ## Breaking Changes
  None

  ## Checklist:
    _Go over all the following points, and put an `x` in all the boxes that apply._
  - [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)_

ACKs for top commit:
  knst:
    utACK  8e9dd12

Tree-SHA512: 3c968c4f0e06bc88f63bf10f742d39595c45c7fcd4aef1891d2e76d9f443cae1ddd3cb84c787faa189ef3a189a9dd63a7f75e69f5787a47d1281d5b6a445a2a6
@DashCoreAutoGuix
Copy link

Guix Automation has completed; a release should be present here: https://github.com/dashpay/dash-dev-branches/releases/tag/v21.0.2-devpr6175.6f82a561. The image should be on dockerhub soon.

@thephez thephez mentioned this pull request Aug 7, 2024
4 tasks
PastaPastaPasta added a commit that referenced this pull request Aug 9, 2024
8e9dd12 chore: bump version in configure.ac (pasta)
afb8dc0 docs: add v21.1.0 release notes and archive v21.0.2 (pasta)
269dd02 Merge #6179: chore: update manpages v21.1 (pasta)
a8cb643 Merge #6175: feat: allow resigning for EHF (pasta)
de5cc22 Merge #6178: chore: bump protocol version to 70233 (pasta)
2de4ce5 Merge #6176: test: reduce BRRHeight in regtest (pasta)
840175e Merge #6174: fix: stop trying to sign pending txes when they are no longer non-locked (pasta)

Pull request description:

  ## Issue being fixed or feature implemented

  ## What was done?

  ## 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)_

ACKs for top commit:
  PastaPastaPasta:
    utACK 058ff45
  kwvg:
    utACK 058ff45

Tree-SHA512: b619f9754258cece012f817a523f5849128e044ffbe17a28e12f1e8424d11f6a4c8d011a554fce0b106e1d6efee2aa306befdc034b8e47be077ad28c7f39c975
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.

4 participants