Skip to content

Conversation

@UdjinM6
Copy link

@UdjinM6 UdjinM6 commented Apr 23, 2025

Issue being fixed or feature implemented

What was done?

ff-ed master 21.1.0 -> 21.1.2, merging it back into develop now

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)

PastaPastaPasta and others added 18 commits February 14, 2025 13:19
… when in discreet mode

21504ae fix: use proper units, show 0 rounds to reuse existing translation (UdjinM6)
5329b7d fix(qt): avoid leaking balance and CJ info in GUI when in discreet mode (UdjinM6)

Pull request description:

  ## Issue being fixed or feature implemented
  develop:
  <img width="721" alt="Screenshot 2025-02-10 at 16 14 52" src="https://github.com/user-attachments/assets/742dc502-64c4-4c99-adb2-97009d387143" />

  this PR:
  <img width="715" alt="Screenshot 2025-02-10 at 16 17 52" src="https://github.com/user-attachments/assets/854a8de9-2f5b-447b-b618-593ce8cf9225" />

  ## 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 21504ae
  knst:
    utACK 21504ae

Tree-SHA512: 6ac57e171f256f322479644fced8e6223eeac2813bea3d1c1e4debdb9586e3e46265fa36b5f196256dbe470a5c32ef54753a3024722b3410d9eeafa5e378b97d
…ific flags

17f0aa3 fix: ReconnectionInfo should also store Dash-specific flags (UdjinM6)

Pull request description:

  ## Issue being fixed or feature implemented
  Mixing with p2p v2 protocol enabled on p2p v1 masternodes triggers reconnection which "forgets" about Dash-specific flags so these connections aren't dropped on mixing session timeouts like they should. Most nodes on testnet are upgraded so the issue wasn't noticeable there unfortunately.

  ## What was done?
  Store Dash-specific flags in `ReconnectionInfo` and use them in `PerformReconnections()` later

  ## How Has This Been Tested?

  ## Breaking Changes
  n/a

  ## 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:
  kwvg:
    ACK 17f0aa3
  PastaPastaPasta:
    utACK 17f0aa3

Tree-SHA512: eb69716e79acc29529b900900d284ddd2d6565d847b1039194c8764fa4575a7a8f51e95b733dc58a550819f9cf5be11de759ceb6884ce44723ede912231cc574
4298d73 chore: bump to 22.1.1 (pasta)
fc65a16 chore: release notes for 22.1.1 (pasta)
38762f7 Merge dashpay#6574: fix: ReconnectionInfo should also store Dash-specific flags (pasta)
580b74c Merge dashpay#6566: fix(qt): avoid leaking balance and CJ info in GUI when in discreet mode (pasta)

Pull request description:

  ## Issue being fixed or feature implemented
  See commits; backport to and prep for release 22.1.1

  ## What was done?

  ## How Has This Been Tested?
  built locally

  ## 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
  - [x] I have assigned this pull request to a milestone

ACKs for top commit:
  UdjinM6:
    LGTM, utACK 4298d73
  kwvg:
    utACK 4298d73

Tree-SHA512: 4e07dd80cfda8c05645a2089fdffe1ddccc6179283c2b2448292fa8d8577a6db794cc46a389a401e7a8a14ac987ebea9e76330c78c7efc08cb4c4bd9a7a91cf2
…en comparing to the default / null object; speedup CDeterministicMNList::AddMN by avoiding check to IsValid when a nullcheck is sufficient

0cf8a46 suggestions (UdjinM6)
56ac184 fmt: apply clang-format suggestions (pasta)
e18f621 fix: improper default check; add tests; use in more dml places (pasta)
ada6f2b perf: speedup of CBLSLazyPublicKey::operator== when comparing to the default / null object; speedup CDeterministicMNList::AddMN by avoiding check to IsValid when a nullcheck is sufficient (pasta)

Pull request description:

  ## Profiling Analysis
  before
  <img width="1926" alt="Pasted Graphic 11" src="https://github.com/user-attachments/assets/3d333419-eea3-4ba3-a0e1-daa1670e4d52" />

  after
  <img width="1909" alt="Pasted Graphic" src="https://github.com/user-attachments/assets/b019b62d-5033-4ed9-9fdb-34e4e1d9e76a" />

  ## Methods
  Below, is some analysis on the results of running the `protx diff` RPC 500 times. The diffs had a start block between MIN and MAX as defined below; and an end block no more than MAX_DIFF from the selected start block. We then perform some statistical analysis on the data.

  MIN_VALUE = 1500050
  MAX_VALUE = 2000050
  MAX_DIFF = 50000
  
  ## Statistical Analysis, outliers included

  ### Before
  Five-Number Summary of Execution Times:
  Min:    0.024492 sec
  Q1:     0.124626 sec
  Median: 0.243000 sec
  Q3:     0.358459 sec
  Max:    15.583948 sec

  Mean Execution Time: 0.428296 sec
  Standard Deviation: 0.933486 sec

  Linear Regression Results:
  y = 0.000001 * x + 0.308662
  R-squared: 0.008160 (Goodness of Fit)
  ![Observed Data](https://github.com/user-attachments/assets/12fdd894-0f2c-4cba-9945-5e9d084c9b24)
  ![Pasted Graphic 6](https://github.com/user-attachments/assets/97706786-73b1-417d-9036-caaa3add1290)

  ### After
  Five-Number Summary of Execution Times:
  Min:    0.038174 sec
  Q1:     0.121363 sec
  Median: 0.158175 sec
  Q3:     0.215866 sec
  Max:    16.587903 sec

  Mean Execution Time: 0.239239 sec
  Standard Deviation: 0.762387 sec

  Linear Regression Results:
  y = 0.000001 * x + 0.151169
  R-squared: 0.006918 (Goodness of Fit)
  P-value: 0.063105 (Significance)
  ![Observed Data](https://github.com/user-attachments/assets/d50e5b5b-1a67-47c9-980c-564adab083ba)
  ![Observed Data](https://github.com/user-attachments/assets/cecc0394-f484-410a-ad21-1a9bbd5a1dc7)

  ## Statistical Analysis, outliers excluded

  ### Before
  removed 76 data points
  Five-Number Summary of Execution Times (After Outlier Removal):
  Min:    0.035916 sec
  Q1:     0.211060 sec
  Median: 0.319278 sec
  Q3:     0.357963 sec
  Max:    0.572785 sec

  Mean Execution Time: 0.289764 sec
  Standard Deviation: 0.101140 sec

  Linear Regression Results (After Outlier Removal):
  y = 0.0000000199 * x + 0.286447
  R-squared: 0.000496 (Goodness of Fit)
  ![Pasted Graphic 10](https://github.com/user-attachments/assets/1f0f6389-dc8f-48a5-80b7-9b5ccb3869dc)

  ### After

  removed 32 data points
  Five-Number Summary of Execution Times (After Outlier Removal):
  Min:    0.038174 sec
  Q1:     0.119880 sec
  Median: 0.151724 sec
  Q3:     0.205017 sec
  Max:    0.355078 sec

  Mean Execution Time: 0.164165 sec
  Standard Deviation: 0.060919 sec

  Linear Regression Results (After Outlier Removal):
  y = 0.0000003119 * x + 0.111002
  R-squared: 0.399298 (Goodness of Fit)

  ![Observed Data](https://github.com/user-attachments/assets/91e97214-6afb-4c3a-a98a-cd7e5704e724)

  ## How Has This Been Tested?
  Ran unit tests locally; reindexing currently, going to let CI run functional tests

  ## Breaking Changes
  Should be none; but please think through the diff specifically related to https://github.com/dashpay/dash/compare/develop...PastaPastaPasta:dash:perf-build-simplified-mn-list-diff-bls-compare-to-null?expand=1#diff-0998f8dfc4c1089e90cbaafe9607b361035b904cd103df31e3c2339a3cbf790dR480

  ## 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:
    LGTM, utACK 0cf8a46

Tree-SHA512: 14eb1dd3bb85c271c1d8a381554b1d774c573336123e97cffbf3222efbbe78e201ef9f331046f8f3b9147fda13fc2ea70250a46e87adcc7da5ae3301c555eddd
…mplified mn list diff output

fab006d refactor: add var name comment (UdjinM6)
50e4004 fix: Do not assert special tx type for cbtx in simplified mn list difff output (UdjinM6)

Pull request description:

  ## Issue being fixed or feature implemented
  Fixes an edge case for debug builds, release builds aren't affected.

  ## What was done?

  ## How Has This Been Tested?

  ## Breaking Changes
  n/a

  ## 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:
  kwvg:
    utACK fab006d
  knst:
    utACK fab006d
  PastaPastaPasta:
    utACK fab006d

Tree-SHA512: ffe8439e782582eda14d10c80d628272b49fea9e39f8b95186d475b13e1a373573afa68a830639f31138e51f24b763a7fb61df1bc29eb83d3835ba8117e2d228
2f5a466 fix: resolve potential deadlock in coinjoin_tests (UdjinM6)

Pull request description:

  ## Issue being fixed or feature implemented
  `ScanForWalletTransactions` should be called outside of `cs_wallet` lock scope which is not the case for `CTransactionBuilderTestSetup ` ctor in `coinjoin_tests.cpp` atm.

  Should fix tsan test failures like https://github.com/PastaPastaPasta/dash/actions/runs/13467587625/job/37636500963#step:8:1.

  ## 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 2f5a466; thanks for looking into it!
  kwvg:
    utACK 2f5a466

Tree-SHA512: 06a3b5d8406d6675f1a9271618dbb5b5839983b90c50c8895fce755639c8f90608748c5e5f56aecd8640420b15536c9e0b4d065b8b32eb6a2f3f731f132f1b59
…r in COPYING and debian's package

c0d3abb fix: follow-up dashpay#6546 to bump copyright year in COPYING and debian's package (Konstantin Akimov)

Pull request description:

  ## What was done?
  Bump copyright for 2025 for leftover files

  ## How Has This Been Tested?
  N/A

  ## Breaking Changes
  N/A

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

ACKs for top commit:
  UdjinM6:
    utACK c0d3abb
  PastaPastaPasta:
    utACK c0d3abb

Tree-SHA512: 7d9e915b4a05966c72b080a388676da1b112ad9c91b17e945a4834551e71ff0b76921a20cb1c1832677d1896123e6952352d0df5865cd091d6a0afd178e6e1a3
…4 LTS (`jammy`), pin QEMU to avoid segfault

20524e4 fix: pin version of QEMU to avoid segfault (Kittywhiskers Van Gogh)
c8ab705 revert: update containers and CI to use Ubuntu 24.04 LTS (`noble`) (Kittywhiskers Van Gogh)
2bcc90a revert: use default non-root user `ubuntu` introduced in Ubuntu 22.10 (Kittywhiskers Van Gogh)

Pull request description:

  ## Additional Information

  * There is a regression in `noble` that has caused a build failure in trying to package our release container ([build](https://github.com/dashpay/dash/actions/runs/13376032021)) linked to [docker/setup-qemu-action#198](docker/setup-qemu-action#198).

  * In light of this, all non-CI containers and jobs have been moved back to Ubuntu 22.04 (`jammy`) though it seems that downgrading alone may be insufficient (see [actions/runner-images#11471](actions/runner-images#11471 (comment))).
    * To remedy this we are pinning the version of QEMU as suggested in [tonistiigi/binfmt#240](tonistiigi/binfmt#240 (comment))

  ## Checklist:

  - [x] I have performed a self-review of my own code
  - [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)**
  - [x] I have added or updated relevant unit/integration/functional/e2e tests **(note: N/A)**
  - [x] I have made corresponding changes to the documentation **(note: N/A)**
  - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  UdjinM6:
    utACK 20524e4

Tree-SHA512: 26bb2cd55a0267b56f86938d97ddfa32f0cdd8a2786c0366eedbcddf706e38b6af93cd29ab98ba420cbdbd112561ded61e2dba906c4b233ad737f24730f58ddc
03c15a3 fix: actually bump protocol version (pasta)
606719a nit: s/use_leagcy_construction/use_legacy_construction/ (pasta)
7432879 fmt: apply clang-format suggestions (pasta)
bdfd597 feat: only use efficient qrinfo construction for new shared protocol version (pasta)
71112dc refactor: more clang-format (UdjinM6)
ff16e68 refactor: clang-format (UdjinM6)
1f16cf4 fix: sort indexes in GetLastBaseBlockHash (UdjinM6)
810ecd8 fix: don't use `baseBlockIndexes.back()` for the tip (UdjinM6)
ec78465 fix: move BuildSimplifiedMNListDiff for block h (UdjinM6)
2a12ff6 bump guix (Konstantin Akimov)
e8bbfd2 fix(perf): build mnlistdiffs in rotation info using dynamically the highest known base block (Odysseas Gabrielides)

Pull request description:

  ## Issue being fixed or feature implemented
  See dashpay#6587 for history

  ## Breaking Changes
  This should remain backwards compatible

  ## 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 03c15a3

Tree-SHA512: d378b519fe614f047b031509b0ec764160ae8674f7417ef571ce7e6fd98d6c94966e6c94681e7bb9c66ac334ce3640492948aae6bb45737dfdf480f6f58b4472
…edge cases, add tests

8b9a728 chore: drop redundant debug logging in tests (UdjinM6)
4f57afa chore: fix clang format (UdjinM6)
275ed8e test: a few trivial checks for `quorum rotationinfo` RPC (UdjinM6)
9c98f99 fix: off-by-one: do not ignore the base block which matches the requested block (UdjinM6)
422a695 fix: default to genesis block as the last base block (UdjinM6)

Pull request description:

  ## Issue being fixed or feature implemented
  Before this PR you could get `baseBlockHash` as all 0s for some cases. Also, a known `baseBlockHash` that matched the `cycleHash` was ignored which means that results included quorum changes from an older known `baseBlockHash` till the `cycleHash`(which is the same as the `baseBlockHash` in question) i.e. these changes were already known to the requester.

  ## What was done?
  Pls see individual commits

  ## How Has This Been Tested?
  Run tests

  ## Breaking Changes
  n/a, results should simply be a bit more meaningful and compact

  ## 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 8b9a728

Tree-SHA512: 88e5463ca0a40e3decb5cfcab948d7be2febc3583f7a24d455a5a93eb730e34c25246a1384fcfeea887a91348651e6250e226afa02b73f4403d8d10624a84d55
…g block of the signing quorum

d150a65 docs: add release notes (UdjinM6)
85c6b58 feat: bump PROTOCOL_VERSION/MIN_MASTERNODE_PROTO_VERSION to 70237 (UdjinM6)
c20dfd8 fix: `cycleHash` should represent a cycle starting block of the signing quorum (UdjinM6)

Pull request description:

  ## Issue being fixed or feature implemented
  Currently `cycleHash` is calculated based on the current chain height we are signing `isdlock` at. This however can result in `cycleHash` to be a block hash of a new cycle where DKG is still in progress which can be confusing to verifiers.

  ## What was done?
  Select a signing quorum first and then calculate `cycleHash` based on the quorum's base block index instead of using the current chain height.

  ## How Has This Been Tested?
  Running a testnet MN for some time. It's creating new `isdlock`s and processing `isdlock`s from other nodes with no issues.

  ## Breaking Changes
  None, `cycleHash` is simply going to be more accurate now.

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

ACKs for top commit:
  PastaPastaPasta:
    utACK d150a65
  knst:
    utACK d150a65

Tree-SHA512: ff7e8a9f37aded9ac330185a34db0eef346c6a3f4a6e22f11a6081ff074a789d260fbdf9efc1d8590d4013ecd95a273e07126dfcb5a7f6d6ebebdb45a543f053
docs: move release notes into release-notes.md

docs: add release notes for 6587 and 6625

fixup: add ody as a contributor

Co-authored-by: UdjinM6 <[email protected]>

fixup: release notes to refer to 6622

Co-authored-by: thephez <[email protected]>
8b4ab03 fix: suppress MIN_MASTERNODE_PROTO_VERSION bump in 6608 (pasta)
aca04d1 chore: bump build to 22.1.2 (pasta)
d9d8c24 docs: add release notes for 22.1.2 (pasta)
fb45240 Merge dashpay#6608: fix: `cycleHash` should represent a cycle starting block of the signing quorum (pasta)
9d1498c Merge dashpay#6625: fix: adjust quorum rotation data results in some edge cases, add tests (pasta)
dfc1119 Merge dashpay#6622: fix: efficient build mnlistdiffs in rotation info (pasta)
6fd626b Merge dashpay#6586: fix: revert deployment images back to Ubuntu 22.04 LTS (`jammy`), pin QEMU to avoid segfault (pasta)
affa9d1 Merge dashpay#6599: fix: follow-up dashpay#6546 to bump copyright year in COPYING and debian's package (pasta)
f6163a2 Merge dashpay#6593: fix: resolve potential deadlock in coinjoin_tests (pasta)
243e0ab Merge dashpay#6585: fix: Do not assert special tx type for cbtx in simplified mn list diff output (pasta)
497f95c Merge dashpay#6581: perf: speedup of CBLSLazyPublicKey::operator== when comparing to the default / null object; speedup CDeterministicMNList::AddMN by avoiding check to IsValid when a nullcheck is sufficient (pasta)

Pull request description:

  ## Issue being fixed or feature implemented
  Backports for a new version, v22.1.2

  ## What was done?
  See release notes

  ## 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 8b4ab03
  knst:
    utACK 8b4ab03

Tree-SHA512: 9f2f65e315940197cc2b75a6b0a858d624256cbe668272ff6dfa216eceda1ba9338484d47afc569202b3b5cc75b4dcb825209efe3a3d3ccec57c741f75c40577
@UdjinM6 UdjinM6 added this to the 23 milestone Apr 23, 2025
@coderabbitai
Copy link

coderabbitai bot commented Apr 23, 2025

Walkthrough

The changes introduce updates and additions to the release notes documentation for Dash Core. The main release notes file was revised to reflect version v22.1.2, with a focus on bug fixes and performance improvements, and several previous sections were replaced or removed in favor of new ones that detail technical updates such as quorum rotation optimizations, deployment and CI fixes, and P2P protocol changes. Notable technical changes documented include optimizations in quorum rotation RPCs and P2P messages, fixes to baseBlockHash handling, QEMU version pinning, performance improvements in deterministic masternode list management, a deadlock fix in CoinJoin tests, and a P2P protocol version bump due to changes in the isdlock message. Additionally, release notes for versions v22.1.0 and v22.1.1 were added as new markdown files, each summarizing their respective releases, key changes, upgrade instructions, and contributor acknowledgments. No changes were made to exported or public code entities.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (7)
doc/release-notes/dash/release-notes-22.1.1.md (2)

1-4: Refine the intro wording for clarity.

The current phrasing is wordy and could be simplified to improve readability. Consider:

- This is a new minor version release, bringing various bugfixes.
- This release is **optional** for all nodes, although recommended.
+ This minor release brings various bugfixes and is **optional** for all nodes, though recommended.
🧰 Tools
🪛 LanguageTool

[style] ~3-~3: This is not the usual sequence for adjectives that have no special emphasis.
Context: # Dash Core version v22.1.1 This is a new minor version release, bringing various bugfi...

(EN_ADJ_ORDER)


29-38: Ensure consistent capitalization and hyphenation.

For technical consistency and readability:

  • Change “QT” to “Qt”.
  • Capitalize “CoinJoin” properly.
  • Hyphenate “CJ-related” when used as a compound adjective.
- Minor QT Coinjoin Fixes
+ Minor Qt CoinJoin Fixes

- Avoid leaking CJ related balances in discrete mode (dash#6566)
+ Avoid leaking CJ-related balances in discrete mode (dash#6566)
doc/release-notes/dash/release-notes-22.1.0.md (1)

1-4: Refine the intro wording for consistency.

The opening sentence can be streamlined for clarity:

- This is a new minor version release, bringing new features, and various bugfixes.
+ This minor release introduces new features and brings various bugfixes.
🧰 Tools
🪛 LanguageTool

[style] ~3-~3: This is not the usual sequence for adjectives that have no special emphasis.
Context: # Dash Core version v22.1.0 This is a new minor version release, bringing new features,...

(EN_ADJ_ORDER)

doc/release-notes.md (4)

1-4: Simplify the release introduction.

A more concise phrasing improves readability:

- This is a new minor version release, bringing various bugfixes and performance improvements.
+ This minor release brings various bugfixes and performance improvements.
🧰 Tools
🪛 LanguageTool

[style] ~3-~3: This is not the usual sequence for adjectives that have no special emphasis.
Context: # Dash Core version v22.1.2 This is a new minor version release, bringing various bugfi...

(EN_ADJ_ORDER)


32-33: Add definite articles for clarity in RPC and P2P descriptions.

Including “the” before technical terms enhances readability:

- Optimized `quorum rotationinfo` RPC and `GETQUORUMROTATIONINFO` P2P message by constructing diffs progressively from oldest to newest, reducing redundancy and improving efficiency (dash#6622).
+ Optimized the `quorum rotationinfo` RPC and the `GETQUORUMROTATIONINFO` P2P message by constructing diffs progressively from oldest to newest, reducing redundancy and improving efficiency (dash#6622).

- Fixed incorrect `baseBlockHash` handling, eliminating unnecessary quorum changes in responses and improving result accuracy and compactness (dash#6625).
+ Fixed the incorrect `baseBlockHash` handling, eliminating unnecessary quorum changes in responses and improving result accuracy and compactness (dash#6625).

54-54: Add missing article before “COPYING file.”

For grammatical correctness, include “the”:

- Updated copyright notices to 2025 in COPYING file and Debian packaging metadata (dash#6599).
+ Updated copyright notices to 2025 in the COPYING file and Debian packaging metadata (dash#6599).
🧰 Tools
🪛 LanguageTool

[uncategorized] ~54-~54: You might be missing the article “the” here.
Context: ... - Updated copyright notices to 2025 in COPYING file and Debian packaging metadata (das...

(AI_EN_LECTOR_MISSING_DETERMINER_THE)


59-60: Use active voice and articles in P2P change description.

Rephrase for consistency with other bullets:

- `cycleHash` field in `isdlock` message will now represent a DKG cycle starting block of the signing quorum instead of a DKG cycle starting block corresponding to the current chain height. While this is fully backwards compatible with older versions of Dash Core, other implementations might not be expecting this, so the P2P protocol version was bumped to 70237. (#6608)
+ The `cycleHash` field in the `isdlock` message now represents the DKG cycle starting block of the signing quorum instead of the cycle corresponding to the current chain height. While fully backwards compatible with older versions of Dash Core, other implementations might not expect this, so the P2P protocol version was bumped to 70237. (#6608)
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between db5d000 and 73fa780.

📒 Files selected for processing (3)
  • doc/release-notes.md (5 hunks)
  • doc/release-notes/dash/release-notes-22.1.0.md (1 hunks)
  • doc/release-notes/dash/release-notes-22.1.1.md (1 hunks)
🧰 Additional context used
🪛 LanguageTool
doc/release-notes/dash/release-notes-22.1.1.md

[style] ~3-~3: This is not the usual sequence for adjectives that have no special emphasis.
Context: # Dash Core version v22.1.1 This is a new minor version release, bringing various bugfi...

(EN_ADJ_ORDER)

doc/release-notes/dash/release-notes-22.1.0.md

[style] ~3-~3: This is not the usual sequence for adjectives that have no special emphasis.
Context: # Dash Core version v22.1.0 This is a new minor version release, bringing new features,...

(EN_ADJ_ORDER)

doc/release-notes.md

[style] ~3-~3: This is not the usual sequence for adjectives that have no special emphasis.
Context: # Dash Core version v22.1.2 This is a new minor version release, bringing various bugfi...

(EN_ADJ_ORDER)


[uncategorized] ~54-~54: You might be missing the article “the” here.
Context: ... - Updated copyright notices to 2025 in COPYING file and Debian packaging metadata (das...

(AI_EN_LECTOR_MISSING_DETERMINER_THE)

⏰ Context from checks skipped due to timeout of 90000ms (10)
  • GitHub Check: linux64_multiprocess-build / Build source
  • GitHub Check: linux64_tsan-build / Build source
  • GitHub Check: linux64_ubsan-build / Build source
  • GitHub Check: linux64_fuzz-build / Build source
  • GitHub Check: linux64-build / Build source
  • GitHub Check: linux64_sqlite-build / Build source
  • GitHub Check: win64-build / Build source
  • GitHub Check: linux64_nowallet-build / Build source
  • GitHub Check: x86_64-apple-darwin / Build depends
  • GitHub Check: arm-linux-gnueabihf / Build depends

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.

utACK 73fa780; gonna merge as this changes docs only

@PastaPastaPasta PastaPastaPasta merged commit cff9ef0 into dashpay:develop Apr 23, 2025
30 of 31 checks passed
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.

2 participants