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.

Sign up for GitHub

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

Jump to bottom

fix: adjust incorrect parameter description that says there is a default that doesn't exist #6104

Merged
merged 1 commit into from
Jul 8, 2024
Merged

fix: adjust incorrect parameter description that says there is a default that doesn't exist #6104

merged 1 commit into from
Jul 8, 2024

Conversation

PastaPastaPasta
Copy link
Member

Issue being fixed or feature implemented

This default doesn't actually exist in code.

What was done?

Remove default from text

How Has This Been Tested?

Breaking Changes

None

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)

@PastaPastaPasta PastaPastaPasta added this to the 21.1 milestone Jul 8, 2024
knst
knst reviewed Jul 8, 2024
View reviewed changes
src/chainparamsbase.cpp
@@ -25,7 +25,7 @@ void SetupChainParamsBaseOptions(ArgsManager& argsman)
argsman.AddArg("-highsubsidyblocks=<n>", "The number of blocks with a higher than normal subsidy to mine at the start of a chain. Block after that height will have fixed subsidy base. (default: 0, devnet-only)", ArgsManager::ALLOW_ANY, OptionsCategory::CHAINPARAMS);
argsman.AddArg("-highsubsidyfactor=<n>", "The factor to multiply the normal block subsidy by while in the highsubsidyblocks window of a chain (default: 1, devnet-only)", ArgsManager::ALLOW_ANY, OptionsCategory::CHAINPARAMS);
argsman.AddArg("-llmqchainlocks=<quorum name>", "Override the default LLMQ type used for ChainLocks. Allows using ChainLocks with smaller LLMQs. (default: llmq_devnet, devnet-only)", ArgsManager::ALLOW_ANY, OptionsCategory::CHAINPARAMS);
argsman.AddArg("-llmqdevnetparams=<size>:<threshold>", "Override the default LLMQ size for the LLMQ_DEVNET quorum (default: 3:2, devnet-only)", ArgsManager::ALLOW_ANY, OptionsCategory::CHAINPARAMS);
argsman.AddArg("-llmqdevnetparams=<size>:<threshold>", "Override the default LLMQ size for the LLMQ_DEVNET quorum (devnet-only)", ArgsManager::ALLOW_ANY, OptionsCategory::CHAINPARAMS);
Copy link
Collaborator

@knst knst Jul 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

default is 12:6, no?

    LLMQ_DEVNET = 101, // 12 members, 6 (50%) threshold, one per hour. Params might differ when -llmqdevnetparams is used

Copy link
Member Author

Choose a reason for hiding this comment

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

well; that's the default quorum; but it's not the default for this param strictly. Plus I'd prefer avoiding duplicating the locations where we reference this in case we change llmq_devnet sizes in the future.

knst
knst approved these changes Jul 8, 2024
View reviewed changes
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 60e36b7

better no value rather than wrong value, that's for sure!

UdjinM6
UdjinM6 approved these changes Jul 8, 2024
View reviewed changes
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 60e36b7

@PastaPastaPasta PastaPastaPasta merged commit 154bdd4 into dashpay:develop Jul 8, 2024
9 of 10 checks passed
@PastaPastaPasta PastaPastaPasta deleted the fix-text-llmqdevnetparams branch July 8, 2024 23:53
PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this pull request Jul 15, 2024
@PastaPastaPasta
Merge dashpay#6104: fix: adjust incorrect parameter description that …
a45e6df 
…says there is a default that doesn't exist

60e36b7 fix: adjust incorrect parameter description that says there is a default that doesn't exist (pasta)

Pull request description:

  ## Issue being fixed or feature implemented
  This default doesn't actually exist in code.

  ## What was done?
  Remove default from text

  ## How Has This Been Tested?

  ## Breaking Changes
  None

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

ACKs for top commit:
  UdjinM6:
    utACK 60e36b7

Tree-SHA512: 658eb2cf101d0450619461b7ffe6069de5c04c1fdcca216713f9374cc1e60413ec9b96c3a2931fb69a4c2f8277dd6677100270960a58197da3b92dffb1e9e327
PastaPastaPasta added a commit that referenced this pull request Jul 15, 2024
@PastaPastaPasta
Merge #6115: backport: v21.0.x rc.2
b0b6ba1 
db82817 Merge #6106: feat: create new composite quorum-command platformsign (pasta)
a45e6df Merge #6104: fix: adjust incorrect parameter description that says there is a default that doesn't exist (pasta)
7330982 Merge #6100: feat: make whitelist works with composite commands for platform needs (pasta)
9998ffd Merge #6096: feat: split type of error in submitchainlock - return enum in CL verifying code (pasta)
cdf7a25 Merge #6095: fix: createwallet to require 'load_on_startup' for descriptor wallets (pasta)
c1c2c55 Merge #6092: fix: mixing for partially unlocked descriptor wallets (pasta)
1175486 Merge #6073: feat: add logging for RPC HTTP requests: command, user, http-code, time of running (pasta)

Pull request description:

  ## Issue being fixed or feature implemented
  Backports a set of 6 PRs needed in rc.2

  ## What was done?
  Backported PRs with labels

  ## How Has This Been Tested?

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

ACKs for top commit:
  kwvg:
    LGTM, utACK db82817
  UdjinM6:
    utACK db82817

Tree-SHA512: 1b242c5db04bd5873ef622543bc2a25e29567f15962c677ea51ff05cb784291968d18f419bf611c206b912e8f15d687208ae75af33aab89038b6f0167d99c4bf
@PastaPastaPasta
Copy link
Member Author

back ported in #6115

PastaPastaPasta added a commit that referenced this pull request Jul 29, 2024
@PastaPastaPasta
Merge #6161: chore: merge master 21.0.0 back into develop
7ebd778 
98a3393 chore: set release to true (pasta)
cd0a3a6 Merge #6154: chore: remove trailing whitespaces in release notes (pasta)
6bc60a7 Merge #6151: chore: update seeds for v21 release (pasta)
88e949a Merge #6146: chore: bump assumevalid, minchainwork, checkpoints, chaintxdata (pasta)
cc14427 Merge #6144: docs: release notes for v21.0.0 (pasta)
0a8ece1 Merge #6122: chore: translations 2024-07 (pasta)
146d244 Merge #6140: feat: harden all sporks on mainnet to current values (pasta)
024d272 Merge #6126: feat: enable EHF activation of MN_RR on mainnet (pasta)
e780b3d Merge #6125: docs: update manpages for 21.0 (pasta)
5ede23c Merge #6118: docs: add release notes notifying change of default branch to `develop` (pasta)
1b6fe9c Merge #6117: docs: update supported versions in SECURITY.md (pasta)
27d20be Merge #6116: fix: mitigate crashes associated with some upgradetohd edge cases (pasta)
db82817 Merge #6106: feat: create new composite quorum-command platformsign (pasta)
a45e6df Merge #6104: fix: adjust incorrect parameter description that says there is a default that doesn't exist (pasta)
7330982 Merge #6100: feat: make whitelist works with composite commands for platform needs (pasta)
9998ffd Merge #6096: feat: split type of error in submitchainlock - return enum in CL verifying code (pasta)
cdf7a25 Merge #6095: fix: createwallet to require 'load_on_startup' for descriptor wallets (pasta)
c1c2c55 Merge #6092: fix: mixing for partially unlocked descriptor wallets (pasta)
1175486 Merge #6073: feat: add logging for RPC HTTP requests: command, user, http-code, time of running (pasta)

Pull request description:

  ## Issue being fixed or feature implemented
  Suppressed changes from be83865 so the diff is empty.

  ## What was done?

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

ACKs for top commit:
  PastaPastaPasta:
    ACK 158cf86; no diff
  knst:
    ACK 158cf86

Tree-SHA512: 3310a39fbcb45bdf09f885fe77ba769c0a715869a3bb287eaf0f2cf54b35a7e1f832c88df3bd31097eabf2d375515c1b87ff05e0c3282cef642833a154c42bbe
@UdjinM6 UdjinM6 modified the milestones: 21.1, 21.2 Aug 8, 2024
@UdjinM6 UdjinM6 modified the milestones: 21.2, 22 Oct 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@UdjinM6 UdjinM6 UdjinM6 approved these changes

@knst knst knst approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
22
Development

Successfully merging this pull request may close these issues.
3 participants
@PastaPastaPasta @knst @UdjinM6