chore: prepare v23.1.3 release#7321
Draft
thepastaclaw wants to merge 6 commits into
Draft
Conversation
…hen using external signer 3af06ee Merge bitcoin-core/gui#555: Restore Send button when using external signer (Hennadii Stepanov) Pull request description: ## Issue being fixed or feature implemented Fixes bitcoin-core/gui#551 For the simplest use case of a wallet with one external signer and "PSBT Controls" disabled in settings (the default), the send dialog will behave the same as when using a wallet with private keys. I.e. there's no "Create Unsigned" button. When PSBT controls are turned on, you can now actually make a PSBT with signing it; before this PR that button would trigger a sign event and would broadcast the transaction. In case a multisig, the Send button will sign on the device, and then fall back to presenting a PSBT (same behavior as before dashpay#441). This PR starts with two refactoring commits to move some stuff into a helper function for improved readability in general, and to make the main commit easier to review. ## What was done? Backport bitcoin-core/gui#555 ## How Has This Been Tested? ## 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: UdjinM6: utACK 3af06ee Tree-SHA512: ec0cb2a2daad54d30b5ec9b82e0fb3b59f1bcfd049b036dc52875923b197ceee66aa2b15374c3c83ac894e5c7343b37d1e97e361dc573786db54c9d46f1785b1 (cherry picked from commit bcc0092)
…th watchquorums f6bb02d fix: keep sending ISDLOCK invs to non-MN peers that want recsigs (Konstantin Akimov) Pull request description: ## Issue being fixed or feature implemented PR dashpay#6994 made masternodes skip ISDLOCK inv announcements to any peer with m_wants_recsigs set, on the premise that such peers can reconstruct the ISLOCK from the recsig. It works for MN peers but it does not work for quorum observers running with -watchquorums: those nodes also opt in to recsigs via QSENDRECSIGS but they don't have a signer worker running, so they cannot reconstruct an ISDLOCK from a recsig and they still need the inv. nodes[0] runs with -watchquorums and had progressively sent QSENDRECSIGS to all four MN peers; by the third call every MN saw nodes[0].m_wants_recsigs=true and skipped the inv to it. ## What was done? The ISDLOCK is skipped now only on the peer being verified masternode. Move the policy from PushInv (called for every inv type) to the three sites that actually relay MSG_ISDLOCK and have CNode in scope. ## How Has This Been Tested? Run functional test interface_dash_zmq.py multiple times. This PR drops failure rate from 50% to 0. ## 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: PastaPastaPasta: utACK f6bb02d Tree-SHA512: 14be2cfaad6dd0cb359bedca9e65e176b3d52de109f1c9e606892ee284f8079860d01117d538efb19bcf695e5da0cae747f08dcb98c209624ca28d6cf7d1e34d (cherry picked from commit 67683f3)
…taddressbalances c27ba87 fix: field name in listaddresses (Konstantin Akimov) 53cd792 fix: pass uses_wallet correctly to fix coverage bug (Konstantin Akimov) ff4302f Merge bitcoin#33064: test: fix RPC coverage check (Konstantin Akimov) 2b3cdfb refactor: move listaddressbalances to wallet/rpc/coins.cpp where it belongs to (Konstantin Akimov) 4b1a335 test: add coverage for RPC listaddressbalances (Konstantin Akimov) f6c66c8 test: add exception for importelectrumwallet RPC coverage (Konstantin Akimov) afa9b91 fix: RPC doc for listaddressbalances (Konstantin Akimov) Pull request description: ## Issue being fixed or feature implemented - listaddressbalances returns results that is not matched with its doc and it causes failure on debug builds such as: ``` $ listaddressbalances Internal bug detected: std::any_of(m_results.m_results.begin(), m_results.m_results.end(), [&ret](const RPCResult& res) { return res.MatchesType(ret); }) rpc/util.cpp:530 (HandleRequest) Dash Core v23.1.2-320-ge9b93e3b87be-dirty Please report this issue here: https://github.com/dashpay/dash/issues (code -1) ``` Further investigation discovered the bug, that exclude _all_ wallet RPCs from coverage linter. ## What was done? - fixes doc for RPC `listaddressbalances` - implementation of `listaddressbalances` moved between files to where it is supposed to be - adds functional tests for `listaddressbalances` - fixes coverage linter to add all wallet RPCs - backport bitcoin#33064 (fixes coverage linter and add test for `abortrescan` RPC) ## Backport candidate only this commit is considerable useful for backporting: [fix: RPC doc for listaddressbalances](dashpay@afa9b91) ## How Has This Been Tested? See updates in functional tests ## 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 c27ba87 Tree-SHA512: 538077f3d4c1d177a819c6cc1a87f5e9b529b39b3ef5b341d4b091c23df776948225a568e0f529c374e9bc94731e74e6e2f025d8de101129accf265c09e304f4 (cherry picked from commit 2b831c1)
…attempts" + tests c1cdb75 test: assert QSENDRECSIGS handshake is symmetric under spork21 (Konstantin Akimov) 0e33aa2 revert: "fix: avoid improper dual way connection attempts" (Konstantin Akimov) Pull request description: ## Issue being fixed or feature implemented dashpay#7255 Intermittent failure at `feature_llmq_signing.py --spork21` File "feature_llmq_signing.py", line 111, in run_test wait_for_sigs(True, False, True, 15) File "feature_llmq_signing.py", line 60, in wait_for_sigs self.wait_until(lambda: check_sigs(hasrecsigs, isconflicting1, isconflicting2), timeout = timeout) AssertionError: Predicate '' not true after <timeout> seconds ## What was done? This reverts a57a811 from PR dashpay#6967. `relayMembers` and `connections` in EnsureQuorumConnections are not interchangeable. * `connections`: who this node should connect TO. For each pair (A, B) only the deterministic-outbound side is listed, so the pair results in one TCP connection. * `relayMembers`: who this node should ask to push recovered sigs. For every already-connected MN in the set we send QSENDRECSIGS=true, and the peer then flips m_wants_recsigs=true on its side. For the handshake to happen in both directions, the set must list every other quorum member -- not just the outbound half. After a57a811, only the outbound half is listed, so on the inbound half of each pair m_wants_recsigs stays false. RelayRecoveredSig only pushes QSIGREC to peers with m_wants_recsigs=true, so half of all proactive recovered-sig pushes are silently dropped. This only triggers with spork21 on (IsAllMembersConnectedEnabled returns true). In this case both the path that uses the half-mesh outbound subset and the path that relies on proactive QSIGREC. ## How Has This Been Tested? This fixes the functional test `feature_llmq_signing.py --spork21`, which times out in wait_for_sigs. It reduced amount of failures on my localhost from ~50% to almost 0. Added new functional test in feature_llmq_signing.py for --spork21 case; which fails as expected without this patch. ## 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 c1cdb75 Tree-SHA512: 81730edf32317f3ac81a35ef72dbe556b03120cc91eba3b809af252ea2909d04561efa8bcbbb8bf73d4cf91508b74f9869c5e7946d904fd9c9b21401bae36381 (cherry picked from commit 8c42e82)
… for invalid blocks 76e6645 fix: intermittent incorrect logging of CheckQueue for invalid blocks (Konstantin Akimov) Pull request description: ## Issue being fixed or feature implemented ConnectBlock may return no-named failure instead proper error code such as: 2024-09-23T13:13:10Z ERROR: ConnectBlock: CheckQueue failed ## What was done? This PR fixes this intermittent failure ## 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 76e6645 Tree-SHA512: 5d2b2d40ae65086082dc81c4d0faf9dc2895db1e97cfe1d47857efc6ec5050ea930ca501eb8ef56ebc70ff7e0a14622b69979e63a8d3c1d6703875da06121e8b (cherry picked from commit f4f0b49)
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Author
|
✅ Review complete (commit 3026972) |
WalkthroughThis pull request prepares Dash Core v23.1.3, a patch release that addresses InstantSend relay behavior, external signer wallet workflows, and LLMQ quorum connectivity. The changes span version metadata, consensus parameters, networking protocol improvements, Qt wallet enhancements, a new RPC method, and test framework updates. All version strings across build configuration, documentation, and package metadata are synchronized to v23.1.3. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@src/chainparams.cpp`:
- Around line 418-419: Update the Testnet consensus parameters in
src/chainparams.cpp by replacing the current consensus.nMinimumChainWork and
consensus.defaultAssumeValid values with the PR-specified values and adjust the
associated comment height to 1457000; specifically, set
consensus.nMinimumChainWork to the chainwork hex "03d210973a682f3c" and
consensus.defaultAssumeValid to the block hash
"000000c50584fa861b930e2e773cc095e9e4602d57a91687544168304702d66d", and change
the trailing comment that currently says 1450000 to 1457000 so the variables
(consensus.nMinimumChainWork and consensus.defaultAssumeValid) and their comment
match the PR objectives.
In `@src/wallet/rpc/coins.cpp`:
- Around line 184-205: The RPC handler for listing address balances reads wallet
state without syncing to the current chain; before calling GetAddressBalances in
the lambda (the listaddressbalances RPC) call BlockUntilSyncedToCurrentChain()
while holding the wallet lock (or immediately before LOCK(pwallet->cs_wallet))
so the wallet is synchronized to the latest tip; ensure you reference
BlockUntilSyncedToCurrentChain() and keep the call placement consistent with
other balance-reading RPCs in this file (e.g., getbalance/getunconfirmedbalance)
to avoid stale results.
🪄 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: 2127518f-98bf-4ec8-9ca4-639d1a87ed99
📒 Files selected for processing (25)
configure.accontrib/flatpak/org.dash.dash-core.metainfo.xmldoc/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.2.mdsrc/chainparams.cppsrc/llmq/utils.cppsrc/net_processing.cppsrc/qt/sendcoinsdialog.cppsrc/qt/sendcoinsdialog.hsrc/validation.cppsrc/wallet/rpc/coins.cppsrc/wallet/rpc/wallet.cpptest/functional/create_cache.pytest/functional/feature_llmq_signing.pytest/functional/test_framework/test_framework.pytest/functional/test_framework/test_node.pytest/functional/test_runner.pytest/functional/wallet_balance.pytest/functional/wallet_transactiontime_rescan.py
Comment on lines
+418
to
+419
| consensus.nMinimumChainWork = uint256S("0x00000000000000000000000000000000000000000000000003eb87177577500b"); // 1450000 | ||
| consensus.defaultAssumeValid = uint256S("0x000000fa4225fa022d4216eabb176848d61d03027df12e0685f82d8b13c60f03"); // 1450000 |
There was a problem hiding this comment.
Critical mismatch: Testnet consensus parameters do not match PR objectives.
The code shows testnet parameters for height 1450000, but the PR objectives specify height 1457000 with different hash and chainwork values:
Code (current):
- Height: 1450000
- Hash:
000000fa4225fa022d4216eabb176848d61d03027df12e0685f82d8b13c60f03 - Chainwork:
03eb87177577500b
PR objectives (expected):
- Height: 1457000
- Hash:
000000c50584fa861b930e2e773cc095e9e4602d57a91687544168304702d66d - Chainwork:
03d210973a682f3c
🔧 Proposed fix to match PR objectives
- consensus.nMinimumChainWork = uint256S("0x00000000000000000000000000000000000000000000000003eb87177577500b"); // 1450000
- consensus.defaultAssumeValid = uint256S("0x000000fa4225fa022d4216eabb176848d61d03027df12e0685f82d8b13c60f03"); // 1450000
+ consensus.nMinimumChainWork = uint256S("0x00000000000000000000000000000000000000000000000003d210973a682f3c"); // 1457000
+ consensus.defaultAssumeValid = uint256S("0x000000c50584fa861b930e2e773cc095e9e4602d57a91687544168304702d66d"); // 1457000📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| consensus.nMinimumChainWork = uint256S("0x00000000000000000000000000000000000000000000000003eb87177577500b"); // 1450000 | |
| consensus.defaultAssumeValid = uint256S("0x000000fa4225fa022d4216eabb176848d61d03027df12e0685f82d8b13c60f03"); // 1450000 | |
| consensus.nMinimumChainWork = uint256S("0x00000000000000000000000000000000000000000000000003d210973a682f3c"); // 1457000 | |
| consensus.defaultAssumeValid = uint256S("0x000000c50584fa861b930e2e773cc095e9e4602d57a91687544168304702d66d"); // 1457000 |
🤖 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 `@src/chainparams.cpp` around lines 418 - 419, Update the Testnet consensus
parameters in src/chainparams.cpp by replacing the current
consensus.nMinimumChainWork and consensus.defaultAssumeValid values with the
PR-specified values and adjust the associated comment height to 1457000;
specifically, set consensus.nMinimumChainWork to the chainwork hex
"03d210973a682f3c" and consensus.defaultAssumeValid to the block hash
"000000c50584fa861b930e2e773cc095e9e4602d57a91687544168304702d66d", and change
the trailing comment that currently says 1450000 to 1457000 so the variables
(consensus.nMinimumChainWork and consensus.defaultAssumeValid) and their comment
match the PR objectives.
Author
There was a problem hiding this comment.
Not applying this one as-is: the PR body/source data currently specify the testnet checkpoint at height 1450000, and the code matches those documented values. If we want to advance the testnet checkpoint to 1457000, that should be a deliberate source-data update to both the PR body and chainparams after verifying the checkpoint data.
There was a problem hiding this comment.
@thepastaclaw, understood — thanks for the clarification. Since the code at height 1450000 matches your PR body's source data, my concern doesn't apply. I'll disregard this comment. If you ever intentionally advance the testnet checkpoint to 1457000 (or any future height), that should indeed be a deliberate coordinated update to both the source data and chainparams.cpp together.
( ˘ ³˘)🐇
🧠 Learnings used
Learnt from: UdjinM6
Repo: dashpay/dash PR: 7265
File: src/wallet/wallet.cpp:3381-3382
Timestamp: 2026-04-03T14:17:17.656Z
Learning: In Dash Core, `strprintf()` uses `tinyformat` (type-safe printf). Do not flag format-specifier mismatches for integer types when using `strprintf()`: `%d` is correct for any integer type, including `CAmount`/`int64_t` arguments. Only treat `%d`/integer combinations as a formatting issue if they occur in a non-`strprintf()` (e.g., libc `printf`) context or there is evidence the format string is not being handled by `tinyformat`.
Learnt from: knst
Repo: dashpay/dash PR: 7275
File: src/wallet/scriptpubkeyman.cpp:2152-2157
Timestamp: 2026-04-21T07:05:38.615Z
Learning: In dashpay/dash, the CMake configuration strips `-DNDEBUG`/`/DNDEBUG` from release-mode compiler flags (per `src/secp256k1/CMakeLists.txt`), so `assert()` remains enabled in all build types. Therefore, in this repository, do not treat bare `assert()` usage as “unsafe-in-release-builds” or a release-only issue; only flag `assert()` concerns when there is a different, concrete problem (e.g., side effects, incorrect assumptions, or thread-safety), not based solely on release/build-type behavior.
thepastaclaw
force-pushed
the
chore/v23.1.3-release-prep
branch
from
121f68d to
640ac3c
Compare
✅ No Merge Conflicts DetectedThis PR currently has no conflicts with other open PRs. |
thepastaclaw
force-pushed
the
chore/v23.1.3-release-prep
branch
from
60a7d51 to
8133ca3
Compare
knst
reviewed
May 15, 2026
| ## Tests | ||
|
|
||
| - Renamed `bitcoin-util` manpage and test references to `dash-util` (dash#7221). | ||
| - Added RPC coverage linter fixes and coverage for `listaddressbalances` (dash#7279). |
Collaborator
There was a problem hiding this comment.
Added RPC coverage linter fixes and coverage for -- coverage linter is irrelevant for release notes; it's a devtool.
Probalby you just need to mention here fixes for RPC listaddressbalances help only; see if any other relevant changes for RPC inside PR
knst
reviewed
May 15, 2026
| # Dash Core version v23.1.3 | ||
|
|
||
| This is a new patch version release, bringing GUI improvements, new features, bugfixes, and performance optimizations. | ||
| This is a new patch version release, bringing GUI improvements, bugfixes, and test improvements. |
Collaborator
There was a problem hiding this comment.
test improvements also irrelevant for release notes
knst
reviewed
May 15, 2026
thepastaclaw
force-pushed
the
chore/v23.1.3-release-prep
branch
2 times, most recently
from
05036e0 to
00d2cb0
Compare
Author
|
Addressed release-notes comments in |
thepastaclaw
force-pushed
the
chore/v23.1.3-release-prep
branch
from
00d2cb0 to
3026972
Compare
Author
|
Addressed chainparams feedback in |
thepastaclaw
commented
May 15, 2026
Author
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
I did not find a concrete correctness regression in the queued 3026972a0f7f984d737c5d2036ddd89b212b1211 release-prep patch. The risky backports in LLMQ relay handling, ISDLOCK announcement filtering, wallet RPC help/result metadata, and external-signer send flow are internally consistent and have matching functional-test updates. I was not able to execute the touched functional tests in this checkout because test/config.ini is absent, so this assessment is based on static review of the exact queued diff and surrounding code.
Reviewed commit: 3026972
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Release Preparation for v23.1.3
Prepares Dash Core v23.1.3 on the
v23.1.xbranch.Changes
configure.ac)listaddressbalancesfixeswatchquorumsblocks
nMinimumChainWork,defaultAssumeValid,checkpointData, andchainTxData; testnet isintentionally unchanged for this patch release
Chainparams source data
Mainnet was updated from a synced node using a chainlocked block two blocks back
from tip:
000000000000001a19ad7270422a00f86123ea94e0b295a3a796d6861bd7b03200000000000000000000000000000000000000000000b9040746437784aaec47getchaintxstats 17280:time: 1778832687txcount: 69379403txrate: 0.1476929741159368Testnet chainparams are left unchanged from v23.1.x.
Backport label follow-up
Once this PR lands, the
backport-candidate-23.1.xlabel can be removed fromdash#7271, dash#7279, dash#7289, dash#7293, and dash#7312.
Existing older candidate labels that appear already included in the v23.1 line
and may be stale:
backport-candidate-22.1.x: dash#6879backport-candidate-23.0.x: dash#7064, dash#7069, dash#7087, dash#7126Validation
git diff --check upstream/v23.1.x..HEADpython3 test/lint/lint-whitespace.pypython3 test/lint/lint-files.pypython3 test/lint/lint-python.py(skipped Python linting becauseflake8is not installed)