chore: Merge master 23.1.3 back into develop#7337
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)
293f778 doc: trim v23.1.3 release note test detail (PastaClaw) 2c0015a chore: prepare v23.1.3 release (PastaClaw) d08fdd7 Merge dashpay#7312: fix: intermittent incorrect logging of CheckQueue for invalid blocks (pasta) 4616073 Merge dashpay#7289: revert: "fix: avoid improper dual way connection attempts" + tests (pasta) 5e0af0b Merge dashpay#7279: test: RPC coverage linter + related fixes for listaddressbalances (pasta) 5bc0bd5 Merge dashpay#7293: fix: keep sending ISDLOCK invs to non-MN peers with watchquorums (pasta) 1b52c85 Merge dashpay#7271: Merge bitcoin-core/gui#555: Restore Send button when using external signer (pasta) Pull request description: # Release Preparation for v23.1.3 Prepares Dash Core v23.1.3 on the `v23.1.x` branch. ## Changes - **Version bump:** 23.1.2 → 23.1.3 (`configure.ac`) - **Backports included:** - dash#7271 — Restore Send button when using external signer - dash#7279 — RPC coverage linter + `listaddressbalances` fixes - dash#7289 — Revert improper dual-way connection attempts + tests - dash#7293 — Keep sending ISDLOCK invs to non-MN peers with `watchquorums` - dash#7312 — Fix intermittent incorrect logging of CheckQueue for invalid blocks - **Release notes:** Archived v23.1.2 notes and wrote v23.1.3 notes - **Flatpak metainfo:** Added v23.1.3 release entry dated 2026-05-15 - **Manpages:** Regenerated for v23.1.3 / May 2026 - **Chainparams:** Refreshed mainnet `nMinimumChainWork`, `defaultAssumeValid`, `checkpointData`, and `chainTxData`; testnet is intentionally unchanged for this patch release ## Chainparams source data Mainnet was updated from a synced node using a chainlocked block two blocks back from tip: - Height: 2471728 - Hash: `000000000000001a19ad7270422a00f86123ea94e0b295a3a796d6861bd7b032` - Chainwork: `00000000000000000000000000000000000000000000b9040746437784aaec47` - `getchaintxstats 17280`: - `time`: 1778832687 - `txcount`: 69379403 - `txrate`: 0.1476929741159368 Testnet chainparams are left unchanged from v23.1.x. ## Backport label follow-up Once this PR lands, the `backport-candidate-23.1.x` label can be removed from dash#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#6879 - `backport-candidate-23.0.x`: dash#7064, dash#7069, dash#7087, dash#7126 ## Validation - `git diff --check upstream/v23.1.x..HEAD` - `python3 test/lint/lint-whitespace.py` - `python3 test/lint/lint-files.py` - `python3 test/lint/lint-python.py` (skipped Python linting because `flake8` is not installed) - Pre-PR code review gate: no significant issues found; recommendation: ship ACKs for top commit: UdjinM6: utACK 293f778 UdjinM6: re-utACK 293f778 Tree-SHA512: 1052f3ecb7cdc9b0e035c25c34a083d6e913d7080bc8128f90a0a15cbb51c6199112274b82fcf431125badcaaf2bfa64adff0f4215d5f702a6f0916025029f4b
✅ No Merge Conflicts DetectedThis PR currently has no conflicts with other open PRs. |
|
✅ Review complete (commit 0f821f4) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0f821f4fbc
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| define(_CLIENT_VERSION_MAJOR, 23) | ||
| define(_CLIENT_VERSION_MINOR, 1) | ||
| define(_CLIENT_VERSION_BUILD, 2) | ||
| define(_CLIENT_VERSION_BUILD, 3) |
There was a problem hiding this comment.
Do not advertise v23.1.3 without its fixes
This bumps binaries built from this commit to 23.1.3, but the reviewed commit is a single-parent release-prep commit and does not include the actual v23.1.3 fix commits listed in the new release notes; git log 69c8b8b5..0b416e55 contains only this change, while the release branch includes code changes such as #7271, #7293, #7289, and #7312. In this state users and automation would see a 23.1.3 build that still has the pre-23.1.3 behavior, so the version bump should only land together with the merged fix commits.
Useful? React with 👍 / 👎.
|
Actionable comments posted: 0 |
knst
left a comment
There was a problem hiding this comment.
lgtm, CI failed due to unrelated issue.
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
Routine v23.1.3 back-merge into develop: version bumps, release notes archive, Flatpak metainfo entry, and standard mainnet chainparams updates (nMinimumChainWork, defaultAssumeValid, new checkpoint, chainTxData) at block 2471728. Verified the new checkpoint hash matches defaultAssumeValid and the chainTxData comment references the same block. No code-logic changes and no Dash-subsystem impact. Both agents independently concluded the merge is clean; the single nitpick was a self-acknowledged non-defect about a routine release-engineering value and is dropped.
Issue being fixed or feature implemented
What was done?
How Has This Been Tested?
Breaking Changes
Checklist: