chore: merge master 23.1.5 back into develop#7379
Conversation
…ssing changes from bitcoin#19607 eb769ab fix: remove cs_main from PeerManagerImpl::MaybePunishNodeForTx, missing changes from bitcoin#19607 (Konstantin Akimov) Pull request description: ## Issue being fixed or feature implemented Helper MaybePunishNodeForTx has been introduced when bitcoin#19607 is done, it caused missing changes. ## What was done? Removed cs_main from MaybePunishNodeForTx ## 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 _(for repository code-owners and collaborators only)_ ACKs for top commit: PastaPastaPasta: utACK eb769ab Tree-SHA512: ce9b26d5dd3036d1b8b5ff7f302e813b6c3e776f0132d8cdca5ccabbd9f485fd93cd55629c73d576362723f6f35fcc528c4be0ad3d3ae64aca6dd4546d913cbd
…et-pow calculation much faster e8435d3 test: add regression test for faster division of base_uint<256> to uint32_t (Konstantin Akimov) de1e56a perf: optimize division to uint32 to make re-target-pow calculation much faster (Konstantin Akimov) Pull request description: ## Issue being fixed or feature implemented Dash retargets difficulty every block via DarkGravityWave (24-block loop), so every header processed during sync runs several 256-bit divisions. Bitcoin only retargets once per 2016 blocks. Every division in DGW is by a small integer (nCountBlocks+1) * nTargetTimespan (3600). But the code routes through the only available overload, `operator/=(const base_uint&)`, which does bit-by-bit long division: ~224 iterations, each doing a full-width `div >>= 1` During header sync up to 35% of CPU is wasted on this division on release build (accordingly `perf`): ``` 20.36% d-msghand dash-qt [.] base_uint<256u>::operator>>=(unsigned int) 15.00% d-msghand dash-qt [.] base_uint<256u>::operator/=(base_uint<256u> const&) ``` ## What was done? Implemented optimization for division to unsigned values that could fit inside uint32_t. ## How Has This Been Tested? Got GUIX build from changes in this PR and develop: ``` [develop] 5af9f57/src/qt/dash-qt -datadir=/tmp/dd -reindex real 7m42.152s user 7m0.396s sys 0m3.328s [PR] 8608a3c3bbcd/src/qt/dash-qt -datadir=/tmp/dd -reindex real 4m17.294s user 2m55.474s sys 0m2.731s ``` Perf stats are updated as expected: ``` - 1.61% 0.06% d-msghand dash-qt [.] base_uint<256u>::operator/=(base_uint<256u> const&) - 1.55% base_uint<256u>::operator/=(base_uint<256u> const&) 0.92% base_uint<256u>::operator>>=(unsigned int) + 1.48% 0.57% d-msghand dash-qt [.] base_uint<256u>::operator>>=(unsigned int) ``` So total header sync just a half of the time. Please note, that header sync will be slightly slower due to dashpay#7320 so overall win is smaller. ## Breaking Changes N/A ## Checklist: - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [x] 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 e8435d3 Tree-SHA512: 797c446cbfad6a1eecb98bee893d530e88db7576d0d15faa36c0d9ccb6d3872313c29f977194ba42cfdedf837a7227018dc750fa87b1f376106831204b08f650
It also prevent potential crash in assert in case of huge re-organizations of blockchain when buried fork is re-validated in BlockUndo
c8801b2 test: add test for non-standard asset-locks (Konstantin Akimov) 9ad2afb feat: make v2 asset locks and huge asset locks as non-standard (Konstantin Akimov) Pull request description: ## Issue being fixed or feature implemented Platform ignores asset-locks with amount of inputs more than 100 or with size of transaction bigger than 20480 bytes. If user will create too big asset-lock transaction that platform (L2) can not recognize it may cause to fund lost. Issue is worsened once user will have ability to create asset-lock tx himself by Dash Core after dashpay#7294 It's an addition to this PR: https://github.com/dashpay/platform/pull/3491/files ## What was done? Asset locks that have more than 100 inputs are non-standard txes Asset locks that have size bigger than 20480 bytes are non-standard txes Asset locks that have v2 payload are non-standard txes. Once v24 is activated, platform may be not ready to process them. This non-standard limitation is a feature for soft enabling v2 asset-locks without 2nd fork. ## How Has This Been Tested? See updates in functional test. ## Breaking Changes Some special transactions are now valid for consensus if they are mined in block but they are non-standard and won't be relayed to network. ## Checklist: - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [x] I have added or updated relevant unit/integration/functional/e2e tests - [x] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone Top commit has no ACKs. Tree-SHA512: ce2a6221f66367d839696ce2bfd93941a74eae1469276703fae06e1ccb3f979beaf14ca4dc05c75d03496cdabb5ae58d6b87b902c0a7bfb16e42d558cbb10d96
…onInterface` dtor 8714052 fix: drop `virtual` specifier for `CZMQNotificationInterface` dtor (UdjinM6) Pull request description: ## Issue being fixed or feature implemented ``` In file included from zmq/zmqnotificationinterface.cpp:5: ./zmq/zmqnotificationinterface.h:22:13: error: virtual method '~CZMQNotificationInterface' is inside a 'final' class and can never be overridden [-Werror,-Wunnecessary-virtual-specifier] 22 | virtual ~CZMQNotificationInterface(); | ^ 1 error generated. ``` ## 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: knst: utACK 8714052 Tree-SHA512: 9026c7902b7cc7db51925e97212cf54ef98aca98d862911cbdc85cc9828bec07163e9b7258b91d1af4752c2df7962ec85f22933abcf13337506f3b6a78ea279d
10b6309 docs: use standard bug fixes wording (PastaClaw) 0495b47 docs: refine v23.1.4 release intro (PastaClaw) 3826688 docs: fix v23.1.4 release notes title (PastaClaw) Pull request description: # Update v23.1.4 release notes title and intro ## Issue being fixed or feature implemented The current v23.1.x release notes describe the v23.1.4 changelog, but the top-level title still said `v23.1.3`. Review also noted that the intro should say this release brings performance improvements and bug fixes, not GUI improvements or the one-word `bugfixes` spelling. ## What was done? - Updated the `doc/release-notes.md` title from `Dash Core version v23.1.3` to `Dash Core version v23.1.4`. - Updated the intro text to say the release brings performance improvements and bug fixes. ## How Has This Been Tested? - Ran `git diff --check`. - Verified the first line of `doc/release-notes.md` is now `# Dash Core version v23.1.4`. - Verified the intro now says `performance improvements and bug fixes`. - Ran pre-PR code review and got `ship`. ## Breaking Changes None. ## 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 - [x] I have made corresponding changes to the documentation - [ ] I have assigned this pull request to a milestone ACKs for top commit: UdjinM6: utACK 10b6309 Tree-SHA512: 524de34601f1fcdd1d835d6cef11c2d6483a1b34466a76449144941bb5b86b31e48bd9d199141e42e9d305737de3d8d9ad5fff271920c465adb82d8f05ba904d
c3e3b9d fix: correct v23.1.4 checkpoint hash (PastaClaw) Pull request description: # Fix v23.1.4 checkpoint hash ## Issue being fixed or feature implemented The v23.1.4 mainnet checkpoint at height 2487500 was accidentally populated with the `nMinimumChainWork` value instead of the block hash. Checkpoint lookup treats these entries as block hashes, so the new checkpoint could not be found in the block index. ## What was done? Updated the height 2487500 checkpoint entry to use the same block hash already configured for `consensus.defaultAssumeValid` at that height: `00000000000000119fe42827219e0686d3f7b494ae65f823194c740c5dbab492` ## How Has This Been Tested? - Ran `git diff --check`. - Ran a local Python consistency check verifying: - the 2487500 checkpoint equals `defaultAssumeValid`; - the 2487500 checkpoint does not equal `nMinimumChainWork`. - Ran pre-PR code review and got `ship`. - Cross-checked height 2487500 block hash against Dash explorer data. ## Breaking Changes None. ## 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 - [ ] I have assigned this pull request to a milestone ACKs for top commit: UdjinM6: utACK c3e3b9d Tree-SHA512: 7b708a545a9adc7a3db3faeb08d7023ed1d28a3c338c9df280820d2b5ae0c1459262a4a3e7080aa0c320eb8d405e18d9152b1982fa54dbde680edd3124690354
01640b9 chore: prepare v23.1.5 release (PastaClaw) Pull request description: ## Issue being fixed or feature implemented Prepare the `v23.1.x` branch for Dash Core v23.1.5. This release-prep PR is intentionally minimal. The v23.1.5 release notes only cover the commits after `9adc0b16f93d15fe065692cbe77f3950419db0cb` currently on `v23.1.x`. ## What was done? - Bumped the release version from v23.1.4 to v23.1.5 in `configure.ac`. - Added v23.1.5 to the Flatpak metainfo release list. - Regenerated manpages from freshly built v23.1.5 binaries. - Generated `contrib/debian/examples/dash.conf` with `contrib/devtools/gen-dash-conf.sh`. - Archived the current v23.1.4 release notes to `doc/release-notes/dash/release-notes-23.1.4.md`. - Replaced `doc/release-notes.md` with scoped v23.1.5 notes covering: - dash#7368: corrected the checkpoint hash for height 2487500. - dash#7369: updated v23.1.4 release notes wording. ## How Has This Been Tested? Validation performed locally: - `git diff --check` - Full-feature Linux container build (`--with-gui=qt5`, ZMQ, UPnP, NAT-PMP) of `dashd`, `dash-cli`, `dash-tx`, `dash-wallet`, `dash-util`, and `dash-qt`. - `contrib/devtools/gen-manpages.py` using the freshly built v23.1.5 binaries. - `contrib/devtools/gen-dash-conf.sh` using the freshly built v23.1.5 `dashd`. - `python3 test/lint/lint-whitespace.py` - `python3 test/lint/lint-files.py` - `python3 test/lint/lint-python.py` - Confirmed `doc/release-notes/dash/release-notes-23.1.4.md` is an exact copy of the previous `doc/release-notes.md` from `upstream/v23.1.x`. - Parsed `contrib/flatpak/org.dash.dash-core.metainfo.xml` with Python `xml.etree.ElementTree`. - Reviewed the post-`9adc0b16f93d15fe065692cbe77f3950419db0cb` commit range to keep the v23.1.5 notes scoped to the two minor changes requested. - Ran the mandatory pre-PR `code-review` gate: recommendation `ship`. The generated artifacts were rebuilt in an Ubuntu 24.04 container from the current PR head. ## Breaking Changes None. ## 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 - [x] 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: UdjinM6: utACK 01640b9 Tree-SHA512: 102b2d979aebfe914c8a6759e1b00f44905991a0706179cd5b44012aee0a8f7fac05e8e913586c1e50c62bbe31d3c124303be9ee1f475655585e028b23b80be9
|
@coderabbitai review |
✅ No Merge Conflicts DetectedThis PR currently has no conflicts with other open PRs. |
✅ Action performedReview finished.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (10)
✅ Files skipped from review due to trivial changes (5)
🚧 Files skipped from review as they are similar to previous changes (5)
WalkthroughUpdated release metadata and generated documentation to v23.1.5, refreshed mainnet chain parameters and fixed seed nodes, changed MNHF validation and quorum message handling, and extended ProTx mempool conflict tracking for duplicate platform node IDs with functional test coverage. Sequence Diagram(s)sequenceDiagram
participant CheckMNHFTx
participant MNHFTxPayload
participant CQuorumManager
participant VerifyInsecure
CheckMNHFTx->>MNHFTxPayload: IsTriviallyValid(state)
alt payload valid
CheckMNHFTx->>CQuorumManager: GetQuorum(...)
CheckMNHFTx->>VerifyInsecure: verify mnhfTx.signal.sig
else payload invalid
MNHFTxPayload-->>CheckMNHFTx: invalid TxValidationState
end
sequenceDiagram
participant Peer
participant NetSigningProcessMessage
participant CSigSharesInv
participant BanNode
Peer->>NetSigningProcessMessage: QSIGSHARESINV / QGETSIGSHARES
NetSigningProcessMessage->>CSigSharesInv: deserialize invs
alt std::ios_base::failure
CSigSharesInv-->>NetSigningProcessMessage: failure
NetSigningProcessMessage->>BanNode: BanNode(pfrom.GetId())
NetSigningProcessMessage-->>Peer: rethrow exception
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 markdownlint-cli2 (0.22.1)doc/release-notes.mdmarkdownlint-cli2 wrapper config was not available before execution doc/release-notes/dash/release-notes-23.1.4.mdmarkdownlint-cli2 wrapper config was not available before execution 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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@contrib/flatpak/org.dash.dash-core.metainfo.xml`:
- Around line 24-25: The Flatpak metainfo release entry for v23.1.4 is
inconsistent with the release notes date, so reconcile the `release` metadata in
`org.dash.dash-core.metainfo.xml` with the date used in `doc/release-notes.md`.
Update the affected `<release>` entry so `23.1.4` matches the canonical release
date, keeping the surrounding release history entries in sync.
In `@doc/man/dash-qt.1`:
- Around line 131-136: The man page wording for the -par and -parbls options
hardcodes a fixed negative lower bound, but the actual limit in init settings is
derived dynamically from GetNumCores(). Update the documentation near the -par
and -parbls option descriptions to describe the lower bound as “-number of
cores” or otherwise dynamic, and remove the build-machine-specific hardcoded
value. Use the option names and the init.cpp-derived thread range behavior as
the references when editing the man page text.
In `@doc/man/dashd.1`:
- Line 882: The dashd man page’s -debug category list is missing the active lock
category, so update the documented categories in dashd.1 to include lock
alongside the existing entries such as mempool and llmq. Keep the list
consistent with the categories defined in the logging implementation (for
example, the categories maintained in logging.cpp) so the manual matches the
available runtime options.
In `@doc/release-notes.md`:
- Around line 53-54: The release metadata for v23.1.4 and v23.1.3 is
inconsistent between release notes and Flatpak metainfo, so verify the correct
publication dates and make them match everywhere. Update the release date
entries in the release-notes documents and the corresponding metainfo.xml
values, and also check any other release metadata sources tied to these versions
to keep them aligned. Use the versioned release note entries for v23.1.4 and
v23.1.3 as the reference points when making the corrections.
In `@doc/release-notes/dash/release-notes-23.1.4.md`:
- Around line 49-52: Add the missing PR reference to the “Early bail-out for
invalid oversized llmq messages” bullet in the release notes section so it
matches the other entries. Update the release-notes-23.1.4 markdown item in this
changelog block to include the corresponding dash PR number alongside the LLMQ
bailout note for consistency and traceability.
In `@src/chainparams.cpp`:
- Around line 349-352: The regeneration hint in the chainTxData comment is stale
and still references the old checkpoint base, so update the helper comment in
getchaintxstats/chainTxData to point at the current block/hash used for
regeneration. Make sure the comment near the chainTxData values reflects the
latest base block so future refreshes are reproduced from the correct
checkpoint.
In `@src/llmq/net_signing.cpp`:
- Around line 106-108: The diagnostic log in NetSigning::ProcessQBSigShares is
reporting the wrong count for the limit check: it logs msgs.size() even though
the ban condition is based on totalSigsCount. Update the LogPrint call in the
MAX_MSGS_TOTAL_BATCHED_SIGS branch to report the signature count that actually
triggered the failure, using totalSigsCount alongside the existing limit and
node id context.
In `@src/txmempool.cpp`:
- Around line 689-690: The platform-node conflict tracking in txmempool is
mixing two different identifiers: the owner ProTx hash and the conflicting
transaction hash. Update the logic around mapProTxPlatformNodeIDs, the removal
path near the mempool txid lookup, and the protx-dup comparison in the check
around opt_proTx->proTxHash so the platformNodeID map stores the owning
masternode’s ProTx hash instead of the ProUpServTx tx hash, while still keeping
the conflicting txid available where the mempool needs it. This will let the
same masternode submit a later update-service tx without incorrectly rejecting
it as a duplicate when platformNodeID stays unchanged.
In `@test/functional/test_framework/messages.py`:
- Line 2479: The __slots__ tuple in the message class needs to be reordered to
satisfy Ruff’s RUF023 check. Update the __slots__ declaration in the relevant
messages.py class so the slot names are sorted consistently (for example, in
alphabetical order) while keeping the same entries and behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 133e0ece-b658-4dfd-8f0d-03c2a182b653
📒 Files selected for processing (25)
configure.accontrib/flatpak/org.dash.dash-core.metainfo.xmlcontrib/seeds/nodes_main.txtcontrib/seeds/nodes_test.txtdoc/man/dash-cli.1doc/man/dash-qt.1doc/man/dash-tx.1doc/man/dash-util.1doc/man/dash-wallet.1doc/man/dashd.1doc/release-notes.mddoc/release-notes/dash/release-notes-23.1.3.mddoc/release-notes/dash/release-notes-23.1.4.mdsrc/chainparams.cppsrc/chainparamsseeds.hsrc/evo/chainhelper.cppsrc/evo/mnhftx.cppsrc/evo/mnhftx.hsrc/llmq/net_signing.cppsrc/llmq/signing_shares.hsrc/txmempool.cppsrc/txmempool.htest/functional/p2p_quorum_data.pytest/functional/rpc_netinfo.pytest/functional/test_framework/messages.py
| - [v23.1.4](https://github.com/dashpay/dash/blob/master/doc/release-notes/dash/release-notes-23.1.4.md) released Jun/18/2026 | ||
| - [v23.1.3](https://github.com/dashpay/dash/blob/master/doc/release-notes/dash/release-notes-23.1.3.md) released May/28/2026 |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Date discrepancies between release notes and Flatpak metainfo for both v23.1.4 and v23.1.3.
- v23.1.4: release-notes says
Jun/18/2026but metainfo.xml has2026-06-13(5 days earlier) - v23.1.3: release-notes says
May/28/2026but metainfo.xml has2026-05-15(13 days earlier)
Please verify and align all release dates across release notes, metainfo, and any other release metadata.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@doc/release-notes.md` around lines 53 - 54, The release metadata for v23.1.4
and v23.1.3 is inconsistent between release notes and Flatpak metainfo, so
verify the correct publication dates and make them match everywhere. Update the
release date entries in the release-notes documents and the corresponding
metainfo.xml values, and also check any other release metadata sources tied to
these versions to keep them aligned. Use the versioned release note entries for
v23.1.4 and v23.1.3 as the reference points when making the corrections.
| a potential assertion crash during large blockchain re-organizations when a | ||
| buried fork is re-validated in `BlockUndo`. | ||
| - Early bail-out for invalid oversized llmq messages. | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Missing PR reference for the LLMQ message bail-out item.
Line 51 lists "Early bail-out for invalid oversized llmq messages." without a (dash#XXXX) reference, while all other items in this section include PR references. Please add the corresponding PR number for consistency and traceability.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@doc/release-notes/dash/release-notes-23.1.4.md` around lines 49 - 52, Add the
missing PR reference to the “Early bail-out for invalid oversized llmq messages”
bullet in the release notes section so it matches the other entries. Update the
release-notes-23.1.4 markdown item in this changelog block to include the
corresponding dash PR number alongside the LLMQ bailout note for consistency and
traceability.
|
@coderabbitai review |
✅ Action performedReview finished.
|
7f09437 to
22a3321
Compare
|
Asked in slack for agent to resolve issues; but @coderabbitai this pr, must only merge back into develop. Do NOT flag things not related to the merge. Only merge deficiencies are what I care about. |
|
Tip For best results, initiate chat on the files or code changes.
(•ㅅ•)ゞ ✏️ Learnings added
|
|
Dropped the CodeRabbit-response commits from this branch so #7379 is back to being only the The only CodeRabbit item I found worth tracking separately is the |
chore: merge master 23.1.5 back into develop
Issue being fixed or feature implemented
Backmerge current
masterintodevelopso the active development branchincludes the
v23.1.4andv23.1.5release-line changes.What was done?
Merged
upstream/master(v23.1.5,5dde61050d20c23c3e179c34e663a025f56843f3)into
upstream/develop.Carried forward release metadata and generated artifacts for
v23.1.4andv23.1.5, including version metadata, man pages, release notes, checkpoints,chainTxData, minimum chain work, assume-valid data, and seed updates.
Resolved conflicts in Dash-specific code while preserving develop-only refactors:
CChainstateHelperownership layout andSuperblockManagerintegration.
removed
CMNHFManagerquorum-manager member.NetSigningsplit.functional test coverage.
How Has This Been Tested?
git diff --check upstream/develop...HEAD.CMNHFManagerconstructor call after conflictresolution.
code-reviewgate against the backmerge branch;final recommendation was
shipwith no significant issues found.No local build was run; this is a release backmerge and CI should run the full
Dash Core PR build/test matrix.
Breaking Changes
None expected. This is a backmerge from the release branch into
develop.Checklist
(for repository code-owners and collaborators only)