Skip to content

backport: bitcoin#16807, #21825, #22257, #23577, #24259, #24420, #24339, #24441, #24735, #25125, #25171 - #7492

Open
knst wants to merge 11 commits into
dashpay:developfrom
knst:bp-v22-23-24
Open

backport: bitcoin#16807, #21825, #22257, #23577, #24259, #24420, #24339, #24441, #24735, #25125, #25171#7492
knst wants to merge 11 commits into
dashpay:developfrom
knst:bp-v22-23-24

Conversation

@knst

@knst knst commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

What was done?

Regular backports from Bitcoin Core v22, v23, v24

How Has This Been Tested?

Run unit & functional tests

Breaking Changes

  • remove -deprecatedrpc=exclude_coinbase logic

  • The validateaddress RPC now returns an error_locations array for invalid
    addresses, with the indices of invalid character locations in the address (if
    known). For example, this will attempt to locate up to two Bech32 errors, and
    return their locations if successful. Success and correctness are only guaranteed
    if fewer than two substitution errors have been made.
    The error message returned in the error field now also returns more specific
    errors when decoding fails.

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

142e2da net: add I2P seeds to chainparamsseeds (Jon Atack)
e01f173 contrib: add a few I2P seed nodes (Jon Atack)
ea269c7 contrib: parse I2P addresses in generate-seeds.py (Jon Atack)

Pull request description:

  Follow-up to bitcoin#21560 that updated the fixed seeds infra for BIP155 addresses and then added Tor v3 ones:

  - Update contrib/generate-seeds.py to parse I2P addresses

  - Add a few I2P nodes to contrib/seeds/nodes_main.txt

  - Run generate-seeds.py and add the I2P seeds to chainparamsseeds.h

  Reviewers, see contrib/seeds/README.md for more info and feel free to use the following CLI one-liner to check for and propose additional seeds for contrib/seeds/nodes_main.txt. You can also see how many I2P peers your node knows with cli -addrinfo.

  ```rake
  bitcoin-cli getnodeaddresses 0 | jq '.[] | (select(.address | contains(".b32.i2p"))) | .address' | sort
  ```

  I verified the I2P addresses are correctly BIP155-serialized/deserialized by building with all seeds removed from chainparamsseeds.h except those added here, restarting with `-datadir=newdir -dnsseed=0` and running rpc ` getnodeaddresses 0` that initially returns only the new I2P addresses.

ACKs for top commit:
  laanwj:
    ACK 142e2da
  vasild:
    ACK 142e2da

Tree-SHA512: 040576012d5f1f034e2bd566ad654a6fdfd8ff7f6b12fa40c9fda1e948ebf8417fcea64cfc14938a41439370aa4669bab3e97274f9d4f9a6906fa9520afa9cf8

Co-authored-by: W. J. van der Laan <laanwj@protonmail.com>
@knst knst added this to the 24 milestone Jul 28, 2026
@thepastaclaw

thepastaclaw commented Jul 28, 2026

Copy link
Copy Markdown

⛔ Blockers found — Sonnet deferred (commit 1f1bacd)
Canonical validated blockers: 1

@github-actions

Copy link
Copy Markdown

Potential PR merge conflicts

This is advisory only. It does not block CI, but it marks PRs that will likely need a rebase depending on merge order.

If this PR merges first

These open PRs will likely need a rebase:

@coderabbitai

coderabbitai Bot commented Jul 28, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: a3aff447-1644-40ab-848e-76dae099a6b5

📥 Commits

Reviewing files that changed from the base of the PR and between c81f21c and 1f1bacd.

📒 Files selected for processing (16)
  • ci/test/00_setup_env_native_valgrind.sh
  • ci/test/00_setup_env_s390x.sh
  • contrib/valgrind.supp
  • doc/bips.md
  • doc/release-notes-16807.md
  • src/key_io.cpp
  • src/key_io.h
  • src/rpc/blockchain.cpp
  • src/rpc/output_script.cpp
  • src/test/fuzz/script_format.cpp
  • src/test/versionbits_tests.cpp
  • src/wallet/rpc/coins.cpp
  • src/wallet/rpc/transactions.cpp
  • test/functional/rpc_invalid_address_message.py
  • test/functional/rpc_verifyislock.py
  • test/functional/wallet_listreceivedby.py
🚧 Files skipped from review as they are similar to previous changes (11)
  • ci/test/00_setup_env_s390x.sh
  • ci/test/00_setup_env_native_valgrind.sh
  • test/functional/rpc_verifyislock.py
  • doc/bips.md
  • contrib/valgrind.supp
  • src/rpc/output_script.cpp
  • src/test/fuzz/script_format.cpp
  • src/wallet/rpc/coins.cpp
  • src/rpc/blockchain.cpp
  • test/functional/wallet_listreceivedby.py
  • src/wallet/rpc/transactions.cpp

Walkthrough

The change expands address decoding diagnostics and adds error_locations to validateaddress, with functional coverage for Bech32, Platform, Base58, and malformed inputs. Wallet received-transaction RPCs remove deprecated exclude_coinbase handling. I2P seed parsing, CI exclusions, Valgrind suppressions, documentation, fuzz bounds, test fixtures, RPC help text, and transaction parsing helpers are also updated.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant validateaddress
  participant DecodeDestination
  participant LocateErrors
  validateaddress->>DecodeDestination: decode address and collect error locations
  DecodeDestination->>LocateErrors: locate Bech32 errors
  LocateErrors-->>DecodeDestination: error text and indices
  DecodeDestination-->>validateaddress: error and error_locations
Loading

Possibly related PRs

  • dashpay/dash#7177: Both changes concern exclusions or behavior related to the feature_init functional test.
  • dashpay/dash#7452: Both changes involve bech32::LocateErrors and Bech32 error-location reporting.

Suggested reviewers: udjinm6

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 6.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title correctly identifies this as a backport and the referenced PRs match the changeset.
Description check ✅ Passed The description matches the backported changes and highlights the same breaking RPC updates.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🤖 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/seeds/generate-seeds.py`:
- Line 19: Update the I2P seed format example in the relevant documentation or
generator template from “<i2p>.b32.i2p:<port>” to “<i2p>.b32.i2p:0”, explicitly
reflecting that only port 0 is valid for I2P addresses.
- Around line 56-57: Update the address parsing branch in the seed generator to
recognize the .b32.i2p suffix case-insensitively, matching the core parser’s
behavior and accepting uppercase variants such as .b32.I2P while preserving the
existing base32 decoding logic.

In `@doc/bips.md`:
- Around line 41-42: Update the BIP 157/158 entry so the PR references use one
balanced parenthetical group around both links, removing the extra opening
wrapper before PR `#14121` while preserving all link text and URLs.

In `@test/functional/rpc_verifyislock.py`:
- Line 6: Remove the unused CTransaction symbol from the import statement in
rpc_verifyislock.py, while retaining tx_from_hex, hash256, ser_compact_size, and
ser_string.
🪄 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 Plus

Run ID: dc0de3bd-8e71-44f6-a9bc-178dbd1237c9

📥 Commits

Reviewing files that changed from the base of the PR and between 585856a and e23d643.

📒 Files selected for processing (18)
  • ci/test/00_setup_env_native_valgrind.sh
  • ci/test/00_setup_env_s390x.sh
  • contrib/seeds/generate-seeds.py
  • contrib/valgrind.supp
  • doc/bips.md
  • doc/release-notes-16807.md
  • src/key_io.cpp
  • src/key_io.h
  • src/rpc/blockchain.cpp
  • src/rpc/output_script.cpp
  • src/test/fuzz/script_format.cpp
  • src/test/versionbits_tests.cpp
  • src/wallet/rpc/coins.cpp
  • src/wallet/rpc/transactions.cpp
  • test/functional/interface_zmq_dash.py
  • test/functional/rpc_invalid_address_message.py
  • test/functional/rpc_verifyislock.py
  • test/functional/wallet_listreceivedby.py

<ip>:<port>
[<ipv6>]:<port>
<onion>.onion:<port>
<i2p>.b32.i2p:<port>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Document the I2P port restriction.

<port> implies that arbitrary ports are valid, but I2P addresses are accepted only with port 0; non-zero ports are rejected by src/test/evo_netinfo_tests.cpp. Document this as <i2p>.b32.i2p:0 to avoid generating invalid seed entries.

🤖 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 `@contrib/seeds/generate-seeds.py` at line 19, Update the I2P seed format
example in the relevant documentation or generator template from
“<i2p>.b32.i2p:<port>” to “<i2p>.b32.i2p:0”, explicitly reflecting that only
port 0 is valid for I2P addresses.

Comment on lines +56 to +57
elif addr.endswith('.b32.i2p'):
vchAddr = b32decode(addr[0:-8] + '====', True)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Match the core parser’s case-insensitive suffix handling.

This rejects valid addresses such as ...b32.I2P, while src/netaddress.cpp lowercases the suffix and src/test/net_tests.cpp explicitly covers uppercase I2P addresses.

Proposed fix
-    elif addr.endswith('.b32.i2p'):
+    elif addr.lower().endswith('.b32.i2p'):
📝 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
elif addr.endswith('.b32.i2p'):
vchAddr = b32decode(addr[0:-8] + '====', True)
elif addr.lower().endswith('.b32.i2p'):
vchAddr = b32decode(addr[0:-8] + '====', True)
🤖 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 `@contrib/seeds/generate-seeds.py` around lines 56 - 57, Update the address
parsing branch in the seed generator to recognize the .b32.i2p suffix
case-insensitively, matching the core parser’s behavior and accepting uppercase
variants such as .b32.I2P while preserving the existing base32 decoding logic.

Comment thread doc/bips.md Outdated
Comment thread test/functional/rpc_verifyislock.py Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e23d643f81

ℹ️ 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".

Comment thread test/functional/interface_zmq_dash.py Outdated
@knst
knst force-pushed the bp-v22-23-24 branch 2 times, most recently from e5bf514 to 07d7f06 Compare July 29, 2026 14:13
@knst knst added the RPC Some notable changes to RPC params/behaviour/descriptions label Jul 29, 2026
knst and others added 10 commits July 30, 2026 01:08
88cc481 Modify copyright header on Bech32 code (Samuel Dobson)
5599813 Add lots of comments to Bech32 (Samuel Dobson)
2eb5792 Add release notes for validateaddress Bech32 error detection (MeshCollider)
42d6a02 Refactor and add more tests for validateaddress (Samuel Dobson)
c4979f7 Add boost tests for bech32 error detection (MeshCollider)
02a7bde Add error_locations to validateaddress RPC (Samuel Dobson)
b62b67e Add Bech32 error location function (Samuel Dobson)
0b06e72 More detailed error checking for base58 addresses (Samuel Dobson)

Pull request description:

  Addresses (partially) bitcoin#16779 - no GUI change in this PR

  Adds a LocateError function the bech32 library, which is then called by `validateaddress` RPC, (and then eventually from a GUI tool too, future work). I think modifying validateaddress is nicer than adding a separate RPC for this.
  Includes tests.

  Based on https://github.com/sipa/bech32/blob/master/ecc/javascript/bech32_ecc.js
  Credit to sipa for that code

ACKs for top commit:
  laanwj:
    Code review and manually tested ACK 88cc481
  ryanofsky:
    Code review ACK 88cc481 with caveat that I only checked the new `LocateErrors` code to try to verify it didn't have unsafe or unexpected operations or loop forever or crash. Did not try to verify behavior corresponds to the spec. In the worst case bugs here should just affect error messages not actual decoding of addresses so this seemed ok.
  w0xlt:
    tACK 88cc481

Tree-SHA512: 9c7fe9745bc7527f80a30bd4c1e3034e16b96a02cc7f6c268f91bfad08a6965a8064fe44230aa3f87e4fa3c938f662ff4446bc682c83cb48c1a3f95cf4186688

Dash note: this is the remaining, Dash-side part of the backport. The Bech32
library part (src/bech32.{cpp,h}, src/test/bech32_tests.cpp) was backported
earlier in ce6d7cb, already including the bitcoin#23577 follow-ups (bd1186e),
so bech32::LocateErrors is used here in its final pair-returning form instead
of the out-arg form this PR introduced upstream.

Dash L1 has no Bech32 addresses: a string using the Bech32m HRP ("dash" on
mainnet, "tdash" on the test chains) can only be a DIP-18 Dash Platform
address, which never encodes an L1 destination. The upstream witness
version/size checks are therefore replaced by DecodePlatformDestination(),
so that a well-formed Platform address, or a Bech32m string with a broken
DIP-18 payload, is reported with a specific reason rather than an empty error
(validateaddress requires isvalid == error.empty()).

The functional test drops the SegWit-only cases for the same reason and
covers the DIP-18 equivalents instead; its Bech32m constants are built from
the DIP-0018 test vectors with the helpers in test_framework/segwit_addr.py.

Co-authored-by: W. J. van der Laan <laanwj@protonmail.com>
a4fe701 Make Bech32 LocateErrors return error list rather than using out-arg (Samuel Dobson)
2fa4fd1 Use std::iota instead of manually pushing range (Samuel Dobson)
405c96f Use bounds-checked array lookups in Bech32 error detection code (Samuel Dobson)
28d9c28 Simplify encoding of e in GF(1024) tables to (1,0) (Samuel Dobson)
14358a0 Replace GF1024 tables and syndrome constants with compile-time generated constexprs. (Samuel Dobson)
63f7b69 Update release note for bech32 error detection (Samuel Dobson)
c8b9a22 Report encoding type in bech32 error message (Samuel Dobson)
92f0caf Improve Bech32 boost tests (Samuel Dobson)
bb4d3e9 Address review comments for Bech32 error validation (Samuel Dobson)

Pull request description:

  A number of follow-ups and improvements to the bech32 error location code, introduced in bitcoin#16807.

  Notably, this removes the hardcoded GF1024 tables in favour of constexpr table generation.

ACKs for top commit:
  laanwj:
    Re-ACK a4fe701

Tree-SHA512: 6312373c20ebd6636f5797304876fa0d70fa777de2f6c507245f51a652b3d1224ebc55b236c9e11e6956c1e88e65faadab51d53587078efccb451455aa2e2276

Dash note: this is the remaining, Dash-side part of the backport; the
src/bech32.{cpp,h} and src/test/bech32_tests.cpp changes were backported
earlier in bd1186e. The "Make Bech32 LocateErrors return error list rather
than using out-arg" hunk for src/key_io.cpp is not needed either, because
Dash never had the intermediate out-arg API.

"Not a valid Bech32 or Base58 encoding" is spelled "Not a valid Bech32m or
Base58 encoding" here: Bech32m (DIP-18 Platform addresses) is the only
Bech32-family encoding Dash accepts for addresses.

Co-authored-by: W. J. van der Laan <laanwj@protonmail.com>
fa4b619 test: Remove unused valgrind suppressions (MarcoFalke)
faccb2d test: Exclude broken feature_init for now (MarcoFalke)
fa086d8 test: Properly skip feature_syscall_sandbox in valgrind (MarcoFalke)

Pull request description:

ACKs for top commit:
  fanquake:
    ACK fa4b619

Tree-SHA512: 5be1a8f288182d386531a033ae7258f753dd655dfa1746a52b65622a0359c2b7143a25b49c0747538308eed606a691847d2f59a5a0382b7751b8de7172adf0d3

Co-authored-by: fanquake <fanquake@gmail.com>
03bc08e doc: Mention missing BIP157 in bips.md (laanwj)
e97e3de doc: Update bips.md for 23.x (laanwj)

Pull request description:

  As far as I know, there have been no new bips implemented in this major release. Update `bips.md` accordingly.
  (if there are, please post below)

ACKs for top commit:
  jonatack:
    ACK 03bc08e
  prayank23:
    ACK bitcoin@03bc08e

Tree-SHA512: d671c37d1aab9f700f9688dabec056acfd2503c83bd3e1612ed1cee4ba92b99db002e34b6f1100b98543fe60f8b757ea2bedcc2ff020243cf30e27b4dd73d04c

Co-authored-by: fanquake <fanquake@gmail.com>
bbbbeaf fuzz: Limit script_format to 100kB (MarcoFalke)

Pull request description:

  The target is still one of the slowest ones, but doesn't seem incredibly important. Especially for sizes larger than the standard tx size.

  Fix that by limiting the script size.

ACKs for top commit:
  fanquake:
    ACK bbbbeaf

Tree-SHA512: b6cf7248753909ef2f21d8824f187e7c05732dd3b99619c0067f862f3c2b0f9a87779d4ddbbd3a7a4bae5c794280e2f0a223bf835d6bc6ccaba01817d69479a2

Co-authored-by: fanquake <fanquake@gmail.com>
BACKPORT NOTE:
Some changes are missing because they are superseeded by bitcoin#25662
Affected files:
 - ci/test/00_setup_env_native_fuzz_with_valgrind.sh
 - ci/test/00_setup_env_native_valgrind.sh

15893a0 supp: remove Boost Valgrind suppression (fanquake)
b0740fd ci: use DWARF-4 for Valgrind CI job (fanquake)

Pull request description:

  [clang-14 defaults to using DWARF-5](https://releases.llvm.org/14.0.0/tools/clang/docs/ReleaseNotes.html#dwarf-support-in-clang), which breaks vlagrinds (3.18) ability
  to parse debug info. Valgrind [claims to support DWARF-5](https://valgrind.org/docs/manual/dist.news.html) from version
  3.18 onwards, but maybe that only works when compiling with GCC.

  Explicitly use DWARF-4 for now. Note that from 11.0 [GCC also defaults to
  using DWARF-5](https://www.gnu.org/software/gcc/gcc-11/changes.html).

  Also remove a Boost related suppression.

Top commit has no ACKs.

Tree-SHA512: e4f476170ac5ccbb43d26e990b24753bda3985b2ac5c8a32e74d2d1d64d1b3a2d80a90fbab345f0a9e404eac7fbd783c20147379208e615d526657e8a57890ca

Co-authored-by: MarcoFalke <falke.marco@gmail.com>
fae3200 test: Slim down versionbits_tests.cpp (MacroFake)

Pull request description:

  Seems confusing to spin up a full chainman that isn't even used.

  Fix that by only spinning up logging. Also, remove the chainman include and comment.

ACKs for top commit:
  fanquake:
    ACK fae3200

Tree-SHA512: 35261e9116c0c276f807453db3d635d83916ec2ffd99cf5641f8732736a30a542213096dcec550ef4522d97b3cafe384fdc6068138bc0b577c66fa61256719f8

Co-authored-by: fanquake <fanquake@gmail.com>
…base` logic

a4703ce doc: add release notes about removal of the `deprecatedrpc=exclude_coinbase` (Sebastian Falbesoner)
ef0aa74 rpc: wallet: remove `-deprecatedrpc=exclude_coinbase` logic (Sebastian Falbesoner)

Pull request description:

  Including coinbase transactions in `receivedby` RPCs and adding the `-deprecatedrpc=exclude_coinbase` was done in PR bitcoin#14707 (released in v23.0). For the next release v24.0, this configuration option can be removed.

ACKs for top commit:
  fanquake:
    ACK a4703ce

Tree-SHA512: 97cd4e78501e64f678c78d2ebb5be5376688c023e34fced71dd24e432d27aa31a74b5483545f49ba0bdf48656d8b8b7bee74e3db26cf6daf112613f1caa4dfa4

Co-authored-by: MacroFake <falke.marco@gmail.com>
…utput types

BACKPORT NOTE:
has been done previously; applied missing changes in src/rpc/blockchain.cpp

c821ab8 Use `GetAllOutputTypes` in `getblock` RPC function (Kiminuo)
d970a85 Move `GetAllOutputTypes` function from `rpc/rawtransaction.cpp` to `rpc/util.{h|cpp}` (Kiminuo)

Pull request description:

  This PR attempts to replicate https://github.com/bitcoin/bitcoin/blob/0ccf9b2e5594581deef2f60174c3651a57f93b64/src/rpc/rawtransaction.cpp#L547 to one other place (at the moment) so that users have better idea what RPC methods can actually return.

  I created this PR as a follow-up to the idea mentioned here bitcoin#23320 (comment) (resolved).

ACKs for top commit:
  kristapsk:
    re-ACK c821ab8

Tree-SHA512: 5ff66a41ad7c43ec769f4a99933d2d070feea7c617286d94b6f9bfa1a2547a42211915778210a89074ad4b14d99f34852cc6871efed5e6f1e2ffedd40d669386

Co-authored-by: MarcoFalke <falke.marco@gmail.com>

@thepastaclaw thepastaclaw left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Preliminary review — Codex only

The backport is generally well adapted, but the bitcoin#24259 Valgrind change is incomplete. It removes Ubuntu-20.04-specific suppressions while leaving the Valgrind job on Ubuntu 20.04, which can cause Valgrind reports to fail CI.

Validated blockers were found in the Codex precheck. Sonnet is deferred until a fresh Codex revalidation clears the blocker gate.

Review provenance

  • Codex reviewers: gpt-5.6-sol — general (completed), gpt-5.6-sol — dash-core-commit-history (completed), gpt-5.6-sol — backport-reviewer (completed)
  • Verifier: gpt-5.6-sol — verifier
  • Sonnet: not run (deferred by blocker gate)

🔴 1 blocking

🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `ci/test/00_setup_env_native_valgrind.sh`:
- [BLOCKING] ci/test/00_setup_env_native_valgrind.sh:8-9: Missing prerequisite hunk: bitcoin#24259's Ubuntu 22.04 Valgrind image
  Upstream commit fa4b61911d54840e9a24bfcabafec159f013ee9a changes this task's image from `ubuntu:20.04` to `ubuntu:22.04` in the same commit that removes the `wcsnrtombs` and Boost suppressions. Dash applied the suppression removals and changed `contrib/valgrind.supp` to document Ubuntu 22.04 as the tested environment, but omitted the image override. `ci/dash/matrix.sh` sources `ci/test/00_setup_env.sh` first, so this task still inherits its `ubuntu:20.04` default. The job therefore runs Ubuntu 20.04 system libraries without their corresponding suppressions, and Valgrind's `--error-exitcode=1` can turn those reports into CI failures. Add the upstream task-specific Ubuntu 22.04 override.

@@ -9,7 +9,7 @@ export LC_ALL=C.UTF-8
export PACKAGES="valgrind clang llvm libclang-rt-dev python3-zmq libevent-dev bsdmainutils libboost-dev libdb5.3++-dev libminiupnpc-dev libzmq3-dev"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔴 Blocking: Missing prerequisite hunk: bitcoin#24259's Ubuntu 22.04 Valgrind image

Upstream commit fa4b619 changes this task's image from ubuntu:20.04 to ubuntu:22.04 in the same commit that removes the wcsnrtombs and Boost suppressions. Dash applied the suppression removals and changed contrib/valgrind.supp to document Ubuntu 22.04 as the tested environment, but omitted the image override. ci/dash/matrix.sh sources ci/test/00_setup_env.sh first, so this task still inherits its ubuntu:20.04 default. The job therefore runs Ubuntu 20.04 system libraries without their corresponding suppressions, and Valgrind's --error-exitcode=1 can turn those reports into CI failures. Add the upstream task-specific Ubuntu 22.04 override.

Suggested change
export PACKAGES="valgrind clang llvm libclang-rt-dev python3-zmq libevent-dev bsdmainutils libboost-dev libdb5.3++-dev libminiupnpc-dev libzmq3-dev"
export DOCKER_NAME_TAG="ubuntu:22.04"
export PACKAGES="valgrind clang llvm libclang-rt-dev python3-zmq libevent-dev bsdmainutils libboost-dev libdb5.3++-dev libminiupnpc-dev libzmq3-dev"

source: ['codex']

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

RPC Some notable changes to RPC params/behaviour/descriptions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants