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: keep llmq_50_60 enabled in Devnets #6183

Merged
merged 3 commits into from
Oct 7, 2024

Conversation

ogabrielides
Copy link
Collaborator

@ogabrielides ogabrielides commented Aug 9, 2024

Issue being fixed or feature implemented

LLMQ_50_60 needs to be enabled in Devnets when deploying a larger one.

What was done?

How Has This Been Tested?

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)

src/llmq/options.cpp Outdated Show resolved Hide resolved
knst
knst previously approved these changes Aug 9, 2024
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.

utACK a95aaf1

@PastaPastaPasta PastaPastaPasta modified the milestones: 21.2, 22 Aug 11, 2024
UdjinM6
UdjinM6 previously approved these changes Aug 12, 2024
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 a95aaf1

The title is a bit misleading btw - llmq_50_60 are enabled already but only prior to full dip24 activation. This PR simply keeps them enabled after dip24 activation.

@ogabrielides ogabrielides changed the title feat: enable llmq_50_60 in Devnets feat: keep llmq_50_60 enabled in Devnets Aug 12, 2024
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.

We need to merge in #6184 first

PastaPastaPasta added a commit that referenced this pull request Sep 25, 2024
32ef5f8 chore: bump version in core to v22 (pasta)

Pull request description:

  ## Issue being fixed or feature implemented
  Platform is requesting #6183 be merged into develop, so that they can stop using hacky custom builds (which end up being out of date).

  ## What was done?
  Bump version to v22.0, allow for breaking changes to be merged in. My plan here is to basically have v22.0 be what v21.1 would have originally been, a large minor version, however now, we can merge in breaking changes too.

  Breaking changes can now be merged in

  I don't have a firm timeline yet, but I want this optional smaller v22 to be released relatively quickly compared to normal major versions.

  ## How Has This Been Tested?
  NA

  ## Breaking Changes
  None, yet!

  ## 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
  - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  knst:
    utACK 32ef5f8
  UdjinM6:
    utACK 32ef5f8

Tree-SHA512: 5695c076f4fc5666300492f734dd1ef860a6c7cbbfc7647dfb998c6a2188fddbe69f11c38b8f26b82d6711ef0ccb19d64cf2f6768035800f74120aa271eed4ad
@PastaPastaPasta
Copy link
Member

@ogabrielides can you add release notes and rebase?

@ogabrielides ogabrielides dismissed stale reviews from UdjinM6 and knst via de4238e September 28, 2024 09:03
@ogabrielides
Copy link
Collaborator Author

@PastaPastaPasta done

@ogabrielides ogabrielides requested review from UdjinM6 and knst October 3, 2024 12:13
knst
knst previously approved these changes Oct 3, 2024
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.

utACK de4238e

@UdjinM6
Copy link

UdjinM6 commented Oct 3, 2024

Clang format complains:

--- src/llmq/options.cpp	(before formatting)
+++ src/llmq/options.cpp	(after formatting)
@@ -[13](https://github.com/dashpay/dash/actions/runs/11082690782/job/30796045092?pr=6183#step:4:14)1,8 +131,8 @@
         case Consensus::LLMQType::LLMQ_DEVNET:
             return true;
         case Consensus::LLMQType::LLMQ_50_60:
-            return !fDIP0024IsActive || !fHaveDIP0024Quorums ||
-                    Params().NetworkIDString() == CBaseChainParams::TESTNET || Params().NetworkIDString() == CBaseChainParams::DEVNET;
+            return !fDIP0024IsActive || !fHaveDIP0024Quorums || Params().NetworkIDString() == CBaseChainParams::TESTNET ||
+                   Params().NetworkIDString() == CBaseChainParams::DEVNET;
         case Consensus::LLMQType::LLMQ_TEST_INSTANTSEND:
             return !fDIP00[24](https://github.com/dashpay/dash/actions/runs/11082690782/job/30796045092?pr=6183#step:4:25)IsActive || !fHaveDIP0024Quorums ||
                     consensusParams.llmqTypeDIP0024InstantSend == Consensus::LLMQType::LLMQ_TEST_INSTANTSEND;

@ogabrielides
Copy link
Collaborator Author

@UdjinM6 done.

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 e3a2cbb

@ogabrielides ogabrielides requested a review from knst October 3, 2024 16:47
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.

utACK e3a2cbb

@PastaPastaPasta PastaPastaPasta merged commit fa6132f into dashpay:develop Oct 7, 2024
29 of 31 checks passed
@ogabrielides ogabrielides deleted the enable_50_60_devnet branch October 7, 2024 20:00
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