-
Notifications
You must be signed in to change notification settings - Fork 1.2k
backport: bitcoin#16999, #24586, #24661, #24716, #24721, #24724, #24776, #24818, #24841, #24862 #6858
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…saction` 71038a1 rpc: Fix documentation assertion for `getrawtransaction` (laanwj) Pull request description: When `getrawtransaction` is successfully used on a coinbase transaction, there is an assertion error. This is very unlikely but happens in the `interface_usdt_utxocache.py` test in bitcoin#24358. This does the following: - Add missing "coinbase" documentation. - Synchronize documentation between `getrawtransaction` and `decoderawtransaction`, the two users of `TxToUniv` that have detailed documentation. `decodepsbt` and `getblock` also uses it but fortunately elides this block. - Change "vout[].amount" to `STR_AMOUNT` for consistency. - Add maintainer comment to keep the two places synchronized. It might be possible to get smarter with deduplication, but there are some extra fields that prevent the obvious way. ACKs for top commit: jonatack: ACK 71038a1 Tree-SHA512: 962236130455d805190ff9a5c971e4e25c17db35614a90ce340264ec953b0ad7fb814eb33ae430b5073955a8a350f72bdd67ba93e35f9c70e5175b836a767a35
|
WalkthroughAdds substantial logic in contrib/seeds/makeseeds.py: MAX_SEEDS_PER_ASN becomes a per-network dict, MIN_BLOCKS added, parseip typed, resolveasn added, several typed filtering helpers, filterbyasn implementing per-ASN/per-network limits and progress output, ip_stats, and logging; IPs now read from mn['state']['addresses'][0]. Adds DecodeTxDoc(...) in src/rpc/rawtransaction.cpp and replaces duplicated verbose-transaction RPC schemas with calls to it. Tests: BasicTestingSetup switched to ConnmanTestMsg. Documentation updates in doc/REST-interface.md and doc/dependencies.md. The rest of the diff is widespread cosmetic C++ changes standardizing inline named-parameter comment style and optional-field annotations across many RPC/help and call sites. Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (32)
-
contrib/seeds/makeseeds.py(5 hunks) -
doc/REST-interface.md(2 hunks) -
doc/dependencies.md(1 hunks) -
src/bench/addrman.cpp(1 hunks) -
src/bench/mempool_stress.cpp(1 hunks) -
src/bench/rpc_mempool.cpp(1 hunks) -
src/bench/wallet_balance.cpp(1 hunks) -
src/net.cpp(1 hunks) -
src/net_processing.cpp(2 hunks) -
src/node/interfaces.cpp(1 hunks) -
src/qt/peertablemodel.cpp(1 hunks) -
src/qt/rpcconsole.cpp(2 hunks) -
src/qt/transactiontablemodel.cpp(1 hunks) -
src/rpc/blockchain.cpp(9 hunks) -
src/rpc/coinjoin.cpp(1 hunks) -
src/rpc/mining.cpp(1 hunks) -
src/rpc/net.cpp(3 hunks) -
src/rpc/rawtransaction.cpp(18 hunks) -
src/rpc/server.cpp(1 hunks) -
src/rpc/txoutproof.cpp(1 hunks) -
src/test/txpackage_tests.cpp(1 hunks) -
src/test/util/setup_common.cpp(2 hunks) -
src/txdb.cpp(1 hunks) -
src/txmempool.cpp(1 hunks) -
src/wallet/rpc/addresses.cpp(3 hunks) -
src/wallet/rpc/backup.cpp(2 hunks) -
src/wallet/rpc/coins.cpp(4 hunks) -
src/wallet/rpc/spend.cpp(7 hunks) -
src/wallet/rpc/transactions.cpp(10 hunks) -
src/wallet/rpc/wallet.cpp(2 hunks) -
src/wallet/spend.cpp(1 hunks) -
src/wallet/walletdb.cpp(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
src/**/*.{cpp,h,cc,cxx,hpp}
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.{cpp,h,cc,cxx,hpp}: Dash Core C++ codebase must be written in C++20 and require at least Clang 16 or GCC 11.1
Dash uses unordered_lru_cache for efficient caching with LRU eviction
Files:
src/txdb.cppsrc/wallet/spend.cppsrc/txmempool.cppsrc/wallet/rpc/coins.cppsrc/wallet/rpc/backup.cppsrc/bench/wallet_balance.cppsrc/node/interfaces.cppsrc/rpc/net.cppsrc/qt/transactiontablemodel.cppsrc/net_processing.cppsrc/bench/mempool_stress.cppsrc/wallet/rpc/addresses.cppsrc/rpc/txoutproof.cppsrc/test/txpackage_tests.cppsrc/test/util/setup_common.cppsrc/wallet/rpc/spend.cppsrc/wallet/walletdb.cppsrc/rpc/server.cppsrc/rpc/blockchain.cppsrc/rpc/mining.cppsrc/qt/rpcconsole.cppsrc/bench/rpc_mempool.cppsrc/wallet/rpc/wallet.cppsrc/rpc/coinjoin.cppsrc/wallet/rpc/transactions.cppsrc/qt/peertablemodel.cppsrc/net.cppsrc/bench/addrman.cppsrc/rpc/rawtransaction.cpp
doc/**
📄 CodeRabbit inference engine (CLAUDE.md)
Unless specifically prompted, avoid making changes to the doc directory (documentation)
Files:
doc/REST-interface.mddoc/dependencies.md
src/bench/**/*.{cpp,h,cc,cxx,hpp}
📄 CodeRabbit inference engine (CLAUDE.md)
Performance benchmarks should be placed in src/bench/ and use nanobench
Files:
src/bench/wallet_balance.cppsrc/bench/mempool_stress.cppsrc/bench/rpc_mempool.cppsrc/bench/addrman.cpp
src/{test,wallet/test,qt/test}/**/*.{cpp,h,cc,cxx,hpp}
📄 CodeRabbit inference engine (CLAUDE.md)
Unit tests for C++ code should be placed in src/test/, src/wallet/test/, or src/qt/test/ and use Boost::Test or Qt 5 for GUI tests
Files:
src/test/txpackage_tests.cppsrc/test/util/setup_common.cpp
contrib/**
📄 CodeRabbit inference engine (CLAUDE.md)
Unless specifically prompted, avoid making changes to the contrib directory (contributed scripts)
Files:
contrib/seeds/makeseeds.py
🧠 Learnings (4)
📓 Common learnings
Learnt from: kwvg
PR: dashpay/dash#6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.
📚 Learning: 2025-01-14T08:37:16.955Z
Learnt from: kwvg
PR: dashpay/dash#6530
File: src/validation.cpp:360-362
Timestamp: 2025-01-14T08:37:16.955Z
Learning: The UpdateTransactionsFromBlock() method in txmempool.cpp takes parameters in the order: vHashUpdate, ancestor_size_limit, ancestor_count_limit. The size limit comes before the count limit.
Applied to files:
src/txmempool.cpp
📚 Learning: 2025-08-08T07:01:47.332Z
Learnt from: knst
PR: dashpay/dash#6805
File: src/wallet/rpc/wallet.cpp:357-357
Timestamp: 2025-08-08T07:01:47.332Z
Learning: In src/wallet/rpc/wallet.cpp, the upgradetohd RPC now returns a UniValue string message (RPCResult::Type::STR) instead of a boolean, including guidance about mnemonic backup and null-character passphrase handling; functional tests have been updated to assert returned strings in several cases.
Applied to files:
src/wallet/rpc/coins.cppsrc/wallet/rpc/backup.cppsrc/wallet/rpc/addresses.cppsrc/rpc/blockchain.cppsrc/wallet/rpc/wallet.cppsrc/wallet/rpc/transactions.cpp
📚 Learning: 2025-02-14T15:19:17.218Z
Learnt from: kwvg
PR: dashpay/dash#6529
File: src/wallet/rpcwallet.cpp:3002-3003
Timestamp: 2025-02-14T15:19:17.218Z
Learning: The `GetWallet()` function calls in `src/wallet/rpcwallet.cpp` are properly validated with null checks that throw appropriate RPC errors, making additional validation unnecessary.
Applied to files:
src/wallet/rpc/wallet.cpp
🧬 Code graph analysis (9)
src/wallet/spend.cpp (1)
src/wallet/coinselection.cpp (3)
result(70-70)result(174-174)result(285-285)
src/node/interfaces.cpp (1)
src/node/transaction.cpp (2)
BroadcastTransaction(33-118)BroadcastTransaction(33-33)
src/rpc/net.cpp (1)
src/netbase.cpp (2)
GetNetworkNames(133-145)GetNetworkNames(133-133)
src/rpc/txoutproof.cpp (1)
src/validation.cpp (1)
GetTransaction(143-143)
src/test/txpackage_tests.cpp (1)
src/test/util/setup_common.cpp (2)
CreateValidMempoolTransaction(544-591)CreateValidMempoolTransaction(544-550)
src/wallet/rpc/spend.cpp (2)
src/rpc/util.cpp (2)
AmountFromValue(88-98)AmountFromValue(88-88)src/wallet/spend.cpp (2)
FundTransaction(1134-1206)FundTransaction(1134-1134)
src/qt/rpcconsole.cpp (1)
src/qt/guiutil.cpp (2)
ConnectionTypeToQString(1692-1719)ConnectionTypeToQString(1692-1692)
src/bench/rpc_mempool.cpp (1)
src/bench/mempool_eviction.cpp (1)
AddTx(11-11)
src/rpc/rawtransaction.cpp (3)
src/node/transaction.cpp (2)
BroadcastTransaction(33-118)BroadcastTransaction(33-33)src/node/interfaces.cpp (14)
tx(609-612)tx(609-609)tx(732-735)tx(732-732)tx(736-739)tx(736-736)tx(757-760)tx(757-757)tx(1002-1009)tx(1002-1002)tx(1021-1036)tx(1021-1021)chainman(576-578)chainman(1061-1063)src/validation.cpp (2)
ProcessNewPackage(1356-1374)ProcessNewPackage(1356-1357)
🪛 Ruff (0.13.1)
contrib/seeds/makeseeds.py
132-134: Prefer next(...) over single element slice
Replace with next(...)
(RUF015)
135-135: Consider moving this statement to an else block
(TRY300)
136-136: Do not catch blind exception: Exception
(BLE001)
🪛 Flake8 (7.2.0)
contrib/seeds/makeseeds.py
[error] 134-134: continuation line over-indented for hanging indent
(E126)
🪛 markdownlint-cli2 (0.18.1)
doc/dependencies.md
29-29: Heading levels should only increment by one level at a time
Expected: h3; Actual: h4
(MD001, heading-increment)
29-29: Heading style
Expected: setext; Actual: atx
(MD003, heading-style)
38-38: Heading style
Expected: setext; Actual: atx
(MD003, heading-style)
49-49: Heading style
Expected: setext; Actual: atx
(MD003, heading-style)
🪛 GitHub Actions: Clang Diff Format Check
src/rpc/coinjoin.cpp
[error] 1-1: Clang format differences found. The diff-based formatting check failed (exit code 1). Command: 'git diff -U0 origin/develop --
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: linux64_ubsan-build / Build source
- GitHub Check: linux64_sqlite-build / Build source
- GitHub Check: linux64-build / Build source
- GitHub Check: linux64_fuzz-build / Build source
- GitHub Check: mac-build / Build source
- GitHub Check: arm-linux-build / Build source
- GitHub Check: linux64_nowallet-build / Build source
- GitHub Check: win64-build / Build source
- GitHub Check: linux64_tsan-build / Build source
🔇 Additional comments (9)
src/txdb.cpp (1)
91-91: Formatting tweak looks goodThe updated /name=/ comment style keeps the constructor call clear and matches upstream formatting. 👍
src/qt/transactiontablemodel.cpp (1)
35-39: Comment-style alignment looks good.The updated
/*name=*/comment style matches the broader RPC/Qt annotation cleanup in this backport and keeps the column metadata self-documenting without touching behavior.src/txmempool.cpp (1)
311-314: Consistent argument annotation update
The switch to/*entry_count=*/1matches the upstream naming style and keeps the call-site self-documenting without altering behavior. Nice cleanup.doc/dependencies.md (3)
18-22: Required dependency table update looks goodThe expanded columns and package.mk links improve traceability for required dependencies. No further action needed.
26-28: Optional dependency entry reads clearlyThe new Releases column and footnote for libgmp align with the updated layout. Looks good as is.
31-36: GUI dependency table refresh looks consistentIncluding the Releases column and package.mk references clarifies provenance for each GUI dependency. No changes requested.
src/bench/wallet_balance.cpp (1)
53-56: Named argument tags look good.The new comment-style labels keep these wrapper calls readable without touching behavior. 👍
src/net_processing.cpp (2)
2174-2176: Inline flag annotation matches upstream formatting.Tagging
via_compact_blockhere mirrors the upstream style and doesn’t alter control flow. Looks good to me.
4987-4988: Clarifying the network parameter is helpful.The explicit
/*network=*/tag makes this backport easier to diff against upstream while keeping behavior identical.
| # Filter IPv46 by ASN, and limit to max_per_net per network | ||
| result = [] | ||
| asn_count = {} | ||
| net_count: Dict[str, int] = collections.defaultdict(int) | ||
| asn_count: Dict[int, int] = collections.defaultdict(int) | ||
|
|
||
| for i, ip in enumerate(ips_ipv46): | ||
| if len(result) == max_total: | ||
| break | ||
| try: | ||
| asn = asns[i].get() | ||
| if asn not in asn_count: | ||
| asn_count[asn] = 0 | ||
| if asn_count[asn] == max_per_asn: | ||
| continue | ||
| asn_count[asn] += 1 | ||
| result.append(ip) | ||
| except Exception as e: | ||
| sys.stderr.write(f'ERR: Could not resolve ASN for {ip["ip"]}: {e}\n') | ||
|
|
||
| # Add back Onions | ||
| result.extend(ips_onion) | ||
| if i % 10 == 0: | ||
| # give progress update | ||
| print(f"{i:6d}/{len(ips_ipv46)} [{100*i/len(ips_ipv46):04.1f}%]\r", file=sys.stderr, end='', flush=True) | ||
|
|
||
| if net_count[ip['net']] == max_per_net: | ||
| # do not add this ip as we already too many | ||
| # ips from this network | ||
| continue | ||
| asn = lookup_asn(ip['net'], ip['ip']) | ||
| if asn is None or asn_count[asn] == max_per_asn[ip['net']]: | ||
| # do not add this ip as we already have too many | ||
| # ips from this ASN on this network | ||
| continue | ||
| asn_count[asn] += 1 | ||
| net_count[ip['net']] += 1 | ||
| result.append(ip) | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Track ASN usage per network
asn_count is keyed only by ASN, but the limit you compare against (max_per_asn[ip['net']]) is per network. If an ASN already has 4 IPv4 entries, asn_count[asn] hits 4, so the IPv6 branch stops once that combined counter reaches 10. The intended 10 IPv6 slots per ASN are never reachable (you only get 10 - ipv4_count). Please key the counter by (net, asn) (or maintain separate dicts) so IPv4 and IPv6 quotas are enforced independently.
- asn_count: Dict[int, int] = collections.defaultdict(int)
+ asn_count: Dict[tuple[str, int], int] = collections.defaultdict(int)
...
- asn = lookup_asn(ip['net'], ip['ip'])
- if asn is None or asn_count[asn] == max_per_asn[ip['net']]:
+ asn = lookup_asn(ip['net'], ip['ip'])
+ key = (ip['net'], asn) if asn is not None else None
+ if asn is None or asn_count[key] == max_per_asn[ip['net']]:
continue
- asn_count[asn] += 1
+ asn_count[key] += 1📝 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.
| # Filter IPv46 by ASN, and limit to max_per_net per network | |
| result = [] | |
| asn_count = {} | |
| net_count: Dict[str, int] = collections.defaultdict(int) | |
| asn_count: Dict[int, int] = collections.defaultdict(int) | |
| for i, ip in enumerate(ips_ipv46): | |
| if len(result) == max_total: | |
| break | |
| try: | |
| asn = asns[i].get() | |
| if asn not in asn_count: | |
| asn_count[asn] = 0 | |
| if asn_count[asn] == max_per_asn: | |
| continue | |
| asn_count[asn] += 1 | |
| result.append(ip) | |
| except Exception as e: | |
| sys.stderr.write(f'ERR: Could not resolve ASN for {ip["ip"]}: {e}\n') | |
| # Add back Onions | |
| result.extend(ips_onion) | |
| if i % 10 == 0: | |
| # give progress update | |
| print(f"{i:6d}/{len(ips_ipv46)} [{100*i/len(ips_ipv46):04.1f}%]\r", file=sys.stderr, end='', flush=True) | |
| if net_count[ip['net']] == max_per_net: | |
| # do not add this ip as we already too many | |
| # ips from this network | |
| continue | |
| asn = lookup_asn(ip['net'], ip['ip']) | |
| if asn is None or asn_count[asn] == max_per_asn[ip['net']]: | |
| # do not add this ip as we already have too many | |
| # ips from this ASN on this network | |
| continue | |
| asn_count[asn] += 1 | |
| net_count[ip['net']] += 1 | |
| result.append(ip) | |
| # File: contrib/seeds/makeseeds.py | |
| # Filter IPv46 by ASN, and limit to max_per_net per network | |
| result = [] | |
| net_count: Dict[str, int] = collections.defaultdict(int) | |
| asn_count: Dict[tuple[str, int], int] = collections.defaultdict(int) | |
| for i, ip in enumerate(ips_ipv46): | |
| if i % 10 == 0: | |
| # give progress update | |
| print(f"{i:6d}/{len(ips_ipv46)} [{100*i/len(ips_ipv46):04.1f}%]\r", | |
| file=sys.stderr, end='', flush=True) | |
| if net_count[ip['net']] == max_per_net: | |
| # do not add this ip as we already have too many | |
| # ips from this network | |
| continue | |
| - asn = lookup_asn(ip['net'], ip['ip']) | |
| asn = lookup_asn(ip['net'], ip['ip']) | |
| # track counts separately for each (network, ASN) pair | |
| key = (ip['net'], asn) if asn is not None else None | |
| if asn is None or asn_count[key] == max_per_asn[ip['net']]: | |
| # do not add this ip as we already have too many | |
| # ips from this ASN on this network | |
| continue | |
| asn_count[key] += 1 | |
| net_count[ip['net']] += 1 | |
| result.append(ip) |
| # Add back Onions (up to max_per_net) | ||
| result.extend(ips_onion[0:max_per_net]) | ||
| return result |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Restore the global NSEEDS cap
max_per_net is now NSEEDS, so each network can contribute 512 entries and onions add another 512. With three nets this script can emit >1500 seeds, breaking the historical 512-seed contract that downstream tooling assumes. Keep per-net caps if you like, but also enforce the overall NSEEDS ceiling before returning (e.g., slice result to max_total or stop appending once len(result) == NSEEDS).
- result.extend(ips_onion[0:max_per_net])
- return result
+ result.extend(ips_onion[0:max_per_net])
+ return result[:max_per_net]📝 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.
| # Add back Onions (up to max_per_net) | |
| result.extend(ips_onion[0:max_per_net]) | |
| return result | |
| # Add back Onions (up to max_per_net) | |
| result.extend(ips_onion[0:max_per_net]) | |
| return result[:max_per_net] |
🤖 Prompt for AI Agents
In contrib/seeds/makeseeds.py around lines 205-207, the function currently
appends up to NSEEDS per network plus onions which can exceed the historical
global limit of 512; enforce a global NSEEDS cap before returning by truncating
the aggregated result to NSEEDS (e.g., slice result = result[:NSEEDS]) or by
stopping/appending only until len(result) == NSEEDS as you build it so the final
list never exceeds the overall NSEEDS ceiling.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Nitpick comments (7)
doc/dependencies.md (2)
20-22: Use descriptive link text in “Releases” columnReplace generic “[link]” with descriptive text (e.g., “Releases”, “Project site”) to satisfy MD059 and improve accessibility. Examples below.
-| [Boost](../depends/packages/boost.mk) | [link](https://www.boost.org/users/download/) | 1.81.0 | [1.73.0](https://github.com/bitcoin/bitcoin/pull/29066) | No | +| [Boost](../depends/packages/boost.mk) | [Releases](https://www.boost.org/users/download/) | 1.81.0 | [1.73.0](https://github.com/bitcoin/bitcoin/pull/29066) | No | -| [libevent](../depends/packages/libevent.mk) | [link](https://github.com/libevent/libevent/releases) | [2.1.12-stable](https://github.com/bitcoin/bitcoin/pull/21991) | [2.1.8](https://github.com/bitcoin/bitcoin/pull/24681) | No | +| [libevent](../depends/packages/libevent.mk) | [Releases](https://github.com/libevent/libevent/releases) | [2.1.12-stable](https://github.com/bitcoin/bitcoin/pull/21991) | [2.1.8](https://github.com/bitcoin/bitcoin/pull/24681) | No | -| glibc | [link](https://www.gnu.org/software/libc/) | N/A | [2.31](https://github.com/bitcoin/bitcoin/pull/29987) | Yes | +| glibc | [Project site](https://www.gnu.org/software/libc/) | N/A | [2.31](https://github.com/bitcoin/bitcoin/pull/29987) | Yes | -| libgmp | [link](https://gmplib.org/download/gmp/)<sup>[ \* ](#note1)</sup> | 6.3.0 | [6.2.0](https://github.com/dashpay/bls-signatures/pull/92) | No | +| libgmp | [Releases](https://gmplib.org/download/gmp/)<sup>[\*](#note1)</sup> | 6.3.0 | [6.2.0](https://github.com/dashpay/bls-signatures/pull/92) | No | -| [Fontconfig](../depends/packages/fontconfig.mk) | [link](https://www.freedesktop.org/wiki/Software/fontconfig/) | [2.12.6](https://github.com/bitcoin/bitcoin/pull/23495) | 2.6 | Yes | +| [Fontconfig](../depends/packages/fontconfig.mk) | [Project site](https://www.freedesktop.org/wiki/Software/fontconfig/) | [2.12.6](https://github.com/bitcoin/bitcoin/pull/23495) | 2.6 | Yes | -| [FreeType](../depends/packages/freetype.mk) | [link](https://freetype.org) | [2.11.0](https://github.com/bitcoin/bitcoin/commit/01544dd78ccc0b0474571da854e27adef97137fb) | 2.3.0 | Yes | +| [FreeType](../depends/packages/freetype.mk) | [Project site](https://freetype.org) | [2.11.0](https://github.com/bitcoin/bitcoin/commit/01544dd78ccc0b0474571da854e27adef97137fb) | 2.3.0 | Yes | -| [qrencode](../depends/packages/qrencode.mk) | [link](https://fukuchi.org/works/qrencode/) | [4.1.1](https://github.com/bitcoin/bitcoin/pull/27312) | | No | +| [qrencode](../depends/packages/qrencode.mk) | [Project site](https://fukuchi.org/works/qrencode/) | [4.1.1](https://github.com/bitcoin/bitcoin/pull/27312) | | No | -| [Qt](../depends/packages/qt.mk) | [link](https://download.qt.io/official_releases/qt/) | [5.15.3](https://github.com/bitcoin/bitcoin/pull/24668) | [5.11.3](https://github.com/bitcoin/bitcoin/pull/24132) | No | +| [Qt](../depends/packages/qt.mk) | [Releases](https://download.qt.io/official_releases/qt/) | [5.15.3](https://github.com/bitcoin/bitcoin/pull/24668) | [5.11.3](https://github.com/bitcoin/bitcoin/pull/24132) | No | -| [libnatpmp](../depends/packages/libnatpmp.mk) | [link](https://github.com/miniupnp/libnatpmp/) | commit [07004b9...](https://github.com/miniupnp/libnatpmp/tree/07004b97cf691774efebe70404cf22201e4d330d) | | No | +| [libnatpmp](../depends/packages/libnatpmp.mk) | [Project site](https://github.com/miniupnp/libnatpmp/) | commit [07004b9...](https://github.com/miniupnp/libnatpmp/tree/07004b97cf691774efebe70404cf22201e4d330d) | | No | -| [MiniUPnPc](../depends/packages/miniupnpc.mk) | [link](https://miniupnp.tuxfamily.org/) | [2.2.2](https://github.com/bitcoin/bitcoin/pull/20421) | 1.9 | No | +| [MiniUPnPc](../depends/packages/miniupnpc.mk) | [Project site](https://miniupnp.tuxfamily.org/) | [2.2.2](https://github.com/bitcoin/bitcoin/pull/20421) | 1.9 | No | -| [ZeroMQ](../depends/packages/zeromq.mk) | [link](https://github.com/zeromq/libzmq/releases) | 4.3.5 | 4.0.0 | No | +| [ZeroMQ](../depends/packages/zeromq.mk) | [Releases](https://github.com/zeromq/libzmq/releases) | 4.3.5 | 4.0.0 | No | -| [Berkeley DB](../depends/packages/bdb.mk) (legacy wallet) | [link](https://www.oracle.com/technetwork/database/database-technologies/berkeleydb/downloads/index.html) | 4.8.30 | 4.8.x | No | +| [Berkeley DB](../depends/packages/bdb.mk) (legacy wallet) | [Downloads](https://www.oracle.com/technetwork/database/database-technologies/berkeleydb/downloads/index.html) | 4.8.30 | 4.8.x | No | -| [SQLite](../depends/packages/sqlite.mk) | [link](https://sqlite.org) | 3.38.5 | [3.7.17](https://github.com/bitcoin/bitcoin/pull/19077) | No | +| [SQLite](../depends/packages/sqlite.mk) | [Project site](https://sqlite.org) | 3.38.5 | [3.7.17](https://github.com/bitcoin/bitcoin/pull/19077) | No |As per coding guidelines.
Also applies to: 33-36, 41-42, 47-47, 52-53
30-32: Add blank line between headings and tablesInsert a blank line after the “### GUI/Networking/Notifications/Wallet” headings to satisfy MD058 and improve readability.
-### GUI -| Dependency | Releases | Version used | Minimum required | Runtime | +### GUI + +| Dependency | Releases | Version used | Minimum required | Runtime | -### Networking -| Dependency | Releases | Version used | Minimum required | Runtime | +### Networking + +| Dependency | Releases | Version used | Minimum required | Runtime | -### Notifications -| Dependency | Releases | Version used | Minimum required | Runtime | +### Notifications + +| Dependency | Releases | Version used | Minimum required | Runtime | -### Wallet -| Dependency | Releases | Version used | Minimum required | Runtime | +### Wallet + +| Dependency | Releases | Version used | Minimum required | Runtime |As per coding guidelines.
Also applies to: 38-40, 44-46, 49-51
contrib/seeds/makeseeds.py (5)
20-25: Duplicate constant definition for MAX_SEEDS_PER_ASNA scalar is defined at Line 20 and immediately overridden by the dict at Lines 22–25. Keep only the dict to avoid confusion.
-NSEEDS=512 - -MAX_SEEDS_PER_ASN=4 - +NSEEDS = 512 + MAX_SEEDS_PER_ASN = { 'ipv4': 4, 'ipv6': 10, }
27-27: Unused constant MIN_BLOCKSMIN_BLOCKS is defined but never used. Remove it or wire it into filtering if intended.
136-144: Pool lifecycle, picklability, and portability
- Resolver instances are passed to worker processes; these may not be picklable and will break on spawn‑based platforms (macOS/Windows).
- The Pool is never closed/joined, leaking processes until interpreter exit.
Prefer a context manager and construct the resolver inside the worker. Or switch to threads (DNS I/O bound).
- my_resolver = dns.resolver.Resolver() - pool = multiprocessing.Pool(processes=16) + pool_size = 16 + def _task(ip): + resolver = dns.resolver.Resolver() + resolver.nameservers = ['208.67.222.222', '208.67.220.220'] + return resolveasn(resolver, ip) @@ - # OpenDNS servers - my_resolver.nameservers = ['208.67.222.222', '208.67.220.220'] - - # Resolve ASNs in parallel - asns = [pool.apply_async(resolveasn, args=(my_resolver, ip)) for ip in ips_ipv46] + # Resolve ASNs in parallel + with multiprocessing.Pool(processes=pool_size) as pool: + asns = [pool.apply_async(_task, args=(ip,)) for ip in ips_ipv46]Alternatively, use concurrent.futures.ThreadPoolExecutor with the same _task.
155-166: Use >= in quota checks to be robustUse >= instead of == to avoid accidental overshoot if counts are corrupted or pre‑seeded.
- if net_count[ip['net']] == max_per_net: + if net_count[ip['net']] >= max_per_net: continue
208-214: Potential KeyError/IndexError for mn['state']['addresses'][0]If “addresses” is missing or empty, this raises. Since None/invalid entries are filtered later, guard the extraction.
- ips = [parseip(mn['state']['addresses'][0]) for mn in mns] + def _first_addr(mn): + addrs = mn.get('state', {}).get('addresses') or [] + return addrs[0] if addrs else None + ips = [parseip(addr) for mn in mns if (addr := _first_addr(mn))]Please confirm the “protx list valid 1” schema always includes a non‑empty state.addresses list on Mainnet. If guaranteed, we can keep the current form. Based on learnings.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (31)
-
contrib/seeds/makeseeds.py(4 hunks) -
doc/REST-interface.md(2 hunks) -
doc/dependencies.md(1 hunks) -
src/bench/addrman.cpp(1 hunks) -
src/bench/mempool_stress.cpp(1 hunks) -
src/bench/rpc_mempool.cpp(1 hunks) -
src/bench/wallet_balance.cpp(1 hunks) -
src/net.cpp(1 hunks) -
src/net_processing.cpp(2 hunks) -
src/node/interfaces.cpp(1 hunks) -
src/qt/peertablemodel.cpp(1 hunks) -
src/qt/rpcconsole.cpp(2 hunks) -
src/qt/transactiontablemodel.cpp(1 hunks) -
src/rpc/blockchain.cpp(9 hunks) -
src/rpc/coinjoin.cpp(1 hunks) -
src/rpc/mining.cpp(1 hunks) -
src/rpc/net.cpp(3 hunks) -
src/rpc/rawtransaction.cpp(18 hunks) -
src/rpc/server.cpp(1 hunks) -
src/rpc/txoutproof.cpp(1 hunks) -
src/test/util/setup_common.cpp(2 hunks) -
src/txdb.cpp(1 hunks) -
src/txmempool.cpp(1 hunks) -
src/wallet/rpc/addresses.cpp(3 hunks) -
src/wallet/rpc/backup.cpp(2 hunks) -
src/wallet/rpc/coins.cpp(4 hunks) -
src/wallet/rpc/spend.cpp(7 hunks) -
src/wallet/rpc/transactions.cpp(10 hunks) -
src/wallet/rpc/wallet.cpp(2 hunks) -
src/wallet/spend.cpp(1 hunks) -
src/wallet/walletdb.cpp(1 hunks)
✅ Files skipped from review due to trivial changes (4)
- src/bench/mempool_stress.cpp
- src/wallet/walletdb.cpp
- src/rpc/txoutproof.cpp
- src/bench/addrman.cpp
🚧 Files skipped from review as they are similar to previous changes (16)
- src/qt/transactiontablemodel.cpp
- src/wallet/rpc/transactions.cpp
- src/net_processing.cpp
- src/wallet/rpc/backup.cpp
- src/node/interfaces.cpp
- src/rpc/net.cpp
- src/bench/rpc_mempool.cpp
- src/wallet/rpc/spend.cpp
- src/txmempool.cpp
- src/test/util/setup_common.cpp
- src/txdb.cpp
- doc/REST-interface.md
- src/wallet/spend.cpp
- src/wallet/rpc/wallet.cpp
- src/rpc/mining.cpp
- src/qt/peertablemodel.cpp
🧰 Additional context used
📓 Path-based instructions (4)
src/**/*.{cpp,h,cc,cxx,hpp}
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.{cpp,h,cc,cxx,hpp}: Dash Core C++ codebase must be written in C++20 and require at least Clang 16 or GCC 11.1
Dash uses unordered_lru_cache for efficient caching with LRU eviction
Files:
src/qt/rpcconsole.cppsrc/rpc/coinjoin.cppsrc/rpc/server.cppsrc/wallet/rpc/coins.cppsrc/bench/wallet_balance.cppsrc/rpc/rawtransaction.cppsrc/rpc/blockchain.cppsrc/wallet/rpc/addresses.cppsrc/net.cpp
src/bench/**/*.{cpp,h,cc,cxx,hpp}
📄 CodeRabbit inference engine (CLAUDE.md)
Performance benchmarks should be placed in src/bench/ and use nanobench
Files:
src/bench/wallet_balance.cpp
contrib/**
📄 CodeRabbit inference engine (CLAUDE.md)
Unless specifically prompted, avoid making changes to the contrib directory (contributed scripts)
Files:
contrib/seeds/makeseeds.py
doc/**
📄 CodeRabbit inference engine (CLAUDE.md)
Unless specifically prompted, avoid making changes to the doc directory (documentation)
Files:
doc/dependencies.md
🧠 Learnings (2)
📓 Common learnings
Learnt from: kwvg
PR: dashpay/dash#6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.
📚 Learning: 2025-08-08T07:01:47.332Z
Learnt from: knst
PR: dashpay/dash#6805
File: src/wallet/rpc/wallet.cpp:357-357
Timestamp: 2025-08-08T07:01:47.332Z
Learning: In src/wallet/rpc/wallet.cpp, the upgradetohd RPC now returns a UniValue string message (RPCResult::Type::STR) instead of a boolean, including guidance about mnemonic backup and null-character passphrase handling; functional tests have been updated to assert returned strings in several cases.
Applied to files:
src/rpc/blockchain.cppsrc/wallet/rpc/addresses.cpp
🧬 Code graph analysis (1)
src/qt/rpcconsole.cpp (1)
src/qt/guiutil.cpp (2)
ConnectionTypeToQString(1692-1719)ConnectionTypeToQString(1692-1692)
🪛 GitHub Actions: Clang Diff Format Check
src/rpc/coinjoin.cpp
[error] 411-411: Clang format differences found. The diff tool detected formatting changes relative to clang-format. Please run clang-format to fix code style issues in this file. Command step that failed: git diff -U0 origin/develop --
🪛 Ruff (0.13.1)
contrib/seeds/makeseeds.py
118-120: Prefer next(...) over single element slice
Replace with next(...)
(RUF015)
121-121: Consider moving this statement to an else block
(TRY300)
122-122: Do not catch blind exception: Exception
(BLE001)
🪛 Flake8 (7.2.0)
contrib/seeds/makeseeds.py
[error] 120-120: continuation line over-indented for hanging indent
(E126)
🪛 markdownlint-cli2 (0.18.1)
doc/dependencies.md
20-20: Link text should be descriptive
(MD059, descriptive-link-text)
21-21: Link text should be descriptive
(MD059, descriptive-link-text)
22-22: Link text should be descriptive
(MD059, descriptive-link-text)
28-28: Spaces inside link text
(MD039, no-space-in-links)
28-28: Spaces inside link text
(MD039, no-space-in-links)
28-28: Link text should be descriptive
(MD059, descriptive-link-text)
31-31: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
33-33: Link text should be descriptive
(MD059, descriptive-link-text)
34-34: Link text should be descriptive
(MD059, descriptive-link-text)
35-35: Link text should be descriptive
(MD059, descriptive-link-text)
36-36: Link text should be descriptive
(MD059, descriptive-link-text)
39-39: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
41-41: Link text should be descriptive
(MD059, descriptive-link-text)
42-42: Link text should be descriptive
(MD059, descriptive-link-text)
45-45: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
47-47: Link text should be descriptive
(MD059, descriptive-link-text)
50-50: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
52-52: Link text should be descriptive
(MD059, descriptive-link-text)
53-53: Link text should be descriptive
(MD059, descriptive-link-text)
🔇 Additional comments (17)
src/wallet/rpc/addresses.cpp (1)
145-146: Consistent optional annotation updateThanks for standardizing these RPC help annotations to the new
/*optional=*///*priv=*/style. This keeps the wallet RPC docs in sync with the broader backport work and preserves the existing behavior.Also applies to: 435-461, 513-513
src/qt/rpcconsole.cpp (2)
916-916: Consistent named-argument comment.Switch to keep_prompt labeled arg looks good and is consistent with project style.
1330-1330: Consistent named-argument comment.Using /prepend_direction=/true matches GUIUtil::ConnectionTypeToQString signature; no behavior change.
src/rpc/blockchain.cpp (4)
621-622: Help schema optionality aligned with reality.Marking previousblockhash/nextblockhash as optional in getblockheader(s) matches runtime. LGTM.
Also applies to: 696-698
969-969: Address field wording.Using “The Dash address” is consistent with the rest of Dash RPC docs. Good change.
1465-1467: Optional markers standardized in getblockchaininfo.Converting pruning- and softfork-related fields to /optional=/true improves clarity in auto-generated help.
Also applies to: 1471-1471, 1474-1474, 1482-1482, 1491-1491
1993-2027: getblockstats: clarify optional outputs.Marking many stats as optional and using ARR_FIXED for feerate_percentiles matches conditional computation paths. Looks good.
src/rpc/rawtransaction.cpp (6)
47-47: Include for Cat<> utility is correct.Adding <util/vector.h> is required for Cat<std::vector<...>>; avoids ODR/undeclared identifier issues. 👍
748-755: decoderawtransaction help refactor via DecodeTxDoc.Nice deduplication; schema stays uniform with getrawtransaction verbose output.
1064-1064: Named-argument comments on BroadcastTransaction.Adding /relay=/ and /wait_callback=/ improves readability without behavior changes.
1155-1158: Fix confirmed: named-argument comment parsing.The earlier compile issue (
/*test_accept*/=true) is corrected to/*test_accept=*/truein both calls. Build should be fine now.
1259-1355: decodepsbt help: standardized optional markers.The sweep to annotate optional PSBT fields (inputs/outputs, derivs, preimages, proprietary, fee) with /optional=/true aligns docs with actual presence. Looks good.
1688-1690: finalizepsbt help: optional result fields.Marking psbt/hex as optional clarifies mutually exclusive outputs. 👍
doc/dependencies.md (1)
18-22: Good: Releases column added and alignedPrevious header/row misalignment is resolved; the “Releases” column is present and renders correctly.
contrib/seeds/makeseeds.py (3)
215-224: Nice diagnosticsThe progress/stats output is helpful and unobtrusive; keeps stderr cleanly structured.
145-166: ASN quota is enforced across both nets instead of per‑netasn_count is keyed only by ASN while the limit is per net (ipv4 vs ipv6). This couples IPv4 and IPv6 quotas and underfills IPv6 when IPv4 already used slots. Key by (net, asn).
- asn_count: Dict[int, int] = collections.defaultdict(int) + asn_count: Dict[tuple[str, int], int] = collections.defaultdict(int) @@ - asn = asns[i].get() - if asn is None or asn_count[asn] == max_per_asn[ip['net']]: + asn = asns[i].get() + key = (ip['net'], asn) if asn is not None else None + if asn is None or asn_count[key] >= max_per_asn[ip['net']]: # do not add this ip as we already have too many # ips from this ASN on this network continue - asn_count[asn] += 1 + asn_count[key] += 1
168-170: Enforce historical global NSEEDS capWith per‑net caps set to NSEEDS, final output can exceed 512 (IPv4 + IPv6 + onions). Enforce the overall ceiling before returning.
- # Add back Onions (up to max_per_net) - result.extend(ips_onion[0:max_per_net]) - return result + # Add back Onions (up to max_per_net), then enforce global cap + result.extend(ips_onion[0:max_per_net]) + return result[:max_per_net]Also applies to: 170-170
| static std::vector<RPCResult> DecodeTxDoc(const std::string& txid_field_doc) | ||
| { | ||
| return { | ||
| {RPCResult::Type::STR_HEX, "txid", txid_field_doc}, | ||
| {RPCResult::Type::NUM, "size", "The serialized transaction size"}, | ||
| {RPCResult::Type::NUM, "version", "The version"}, | ||
| {RPCResult::Type::NUM, "type", "The type"}, | ||
| {RPCResult::Type::NUM_TIME, "locktime", "The lock time"}, | ||
| {RPCResult::Type::ARR, "vin", "", | ||
| { | ||
| {RPCResult::Type::OBJ, "", "", | ||
| { | ||
| {RPCResult::Type::STR_HEX, "coinbase", /*optional=*/true, "The coinbase value (only if coinbase transaction)"}, | ||
| {RPCResult::Type::STR_HEX, "txid", /*optional=*/true, "The transaction id (if not coinbase transaction)"}, | ||
| {RPCResult::Type::NUM, "vout", /*optional=*/true, "The output number (if not coinbase transaction)"}, | ||
| {RPCResult::Type::OBJ, "scriptSig", /*optional=*/true, "The script (if not coinbase transaction)", | ||
| { | ||
| {RPCResult::Type::STR, "asm", "asm"}, | ||
| {RPCResult::Type::STR_HEX, "hex", "hex"}, | ||
| }}, | ||
| {RPCResult::Type::NUM, "sequence", "The script sequence number"}, | ||
| }}, | ||
| }}, | ||
| {RPCResult::Type::ARR, "vout", "", | ||
| { | ||
| {RPCResult::Type::OBJ, "", "", | ||
| { | ||
| {RPCResult::Type::STR_AMOUNT, "value", "The value in " + CURRENCY_UNIT}, | ||
| {RPCResult::Type::NUM, "n", "index"}, | ||
| {RPCResult::Type::OBJ, "scriptPubKey", "", | ||
| { | ||
| {RPCResult::Type::STR, "asm", "the asm"}, | ||
| {RPCResult::Type::STR, "desc", "Inferred descriptor for the output"}, | ||
| {RPCResult::Type::STR_HEX, "hex", "the hex"}, | ||
| {RPCResult::Type::STR, "type", "The type, eg 'pubkeyhash'"}, | ||
| {RPCResult::Type::STR, "address", /*optional=*/true, "The Bitcoin address (only if a well-defined address exists)"}, | ||
| }}, | ||
| }}, | ||
| }}, | ||
| {RPCResult::Type::NUM, "extraPayloadSize", /*optional=*/true, "Size of DIP2 extra payload. Only present if it's a special TX"}, | ||
| {RPCResult::Type::STR_HEX, "extraPayload", /*optional=*/true, "Hex-encoded DIP2 extra payload data. Only present if it's a special TX"}, | ||
| }; | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two small doc fixes in DecodeTxDoc.
- locktime can be a height or a timestamp; using NUM_TIME suggests “always a time”. Prefer plain NUM with a clarifying description.
- “Bitcoin address” should read “Dash address” for consistency.
Proposed diff:
- {RPCResult::Type::NUM_TIME, "locktime", "The lock time"},
+ {RPCResult::Type::NUM, "locktime", "Raw locktime (block height or UNIX timestamp)"},
@@
- {RPCResult::Type::STR, "address", /*optional=*/true, "The Bitcoin address (only if a well-defined address exists)"},
+ {RPCResult::Type::STR, "address", /*optional=*/true, "The Dash address (only if a well-defined address exists)"},📝 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.
| static std::vector<RPCResult> DecodeTxDoc(const std::string& txid_field_doc) | |
| { | |
| return { | |
| {RPCResult::Type::STR_HEX, "txid", txid_field_doc}, | |
| {RPCResult::Type::NUM, "size", "The serialized transaction size"}, | |
| {RPCResult::Type::NUM, "version", "The version"}, | |
| {RPCResult::Type::NUM, "type", "The type"}, | |
| {RPCResult::Type::NUM_TIME, "locktime", "The lock time"}, | |
| {RPCResult::Type::ARR, "vin", "", | |
| { | |
| {RPCResult::Type::OBJ, "", "", | |
| { | |
| {RPCResult::Type::STR_HEX, "coinbase", /*optional=*/true, "The coinbase value (only if coinbase transaction)"}, | |
| {RPCResult::Type::STR_HEX, "txid", /*optional=*/true, "The transaction id (if not coinbase transaction)"}, | |
| {RPCResult::Type::NUM, "vout", /*optional=*/true, "The output number (if not coinbase transaction)"}, | |
| {RPCResult::Type::OBJ, "scriptSig", /*optional=*/true, "The script (if not coinbase transaction)", | |
| { | |
| {RPCResult::Type::STR, "asm", "asm"}, | |
| {RPCResult::Type::STR_HEX, "hex", "hex"}, | |
| }}, | |
| {RPCResult::Type::NUM, "sequence", "The script sequence number"}, | |
| }}, | |
| }}, | |
| {RPCResult::Type::ARR, "vout", "", | |
| { | |
| {RPCResult::Type::OBJ, "", "", | |
| { | |
| {RPCResult::Type::STR_AMOUNT, "value", "The value in " + CURRENCY_UNIT}, | |
| {RPCResult::Type::NUM, "n", "index"}, | |
| {RPCResult::Type::OBJ, "scriptPubKey", "", | |
| { | |
| {RPCResult::Type::STR, "asm", "the asm"}, | |
| {RPCResult::Type::STR, "desc", "Inferred descriptor for the output"}, | |
| {RPCResult::Type::STR_HEX, "hex", "the hex"}, | |
| {RPCResult::Type::STR, "type", "The type, eg 'pubkeyhash'"}, | |
| {RPCResult::Type::STR, "address", /*optional=*/true, "The Bitcoin address (only if a well-defined address exists)"}, | |
| }}, | |
| }}, | |
| }}, | |
| {RPCResult::Type::NUM, "extraPayloadSize", /*optional=*/true, "Size of DIP2 extra payload. Only present if it's a special TX"}, | |
| {RPCResult::Type::STR_HEX, "extraPayload", /*optional=*/true, "Hex-encoded DIP2 extra payload data. Only present if it's a special TX"}, | |
| }; | |
| } | |
| static std::vector<RPCResult> DecodeTxDoc(const std::string& txid_field_doc) | |
| { | |
| return { | |
| {RPCResult::Type::STR_HEX, "txid", txid_field_doc}, | |
| {RPCResult::Type::NUM, "size", "The serialized transaction size"}, | |
| {RPCResult::Type::NUM, "version", "The version"}, | |
| {RPCResult::Type::NUM, "type", "The type"}, | |
| {RPCResult::Type::NUM, "locktime", "Raw locktime (block height or UNIX timestamp)"}, | |
| {RPCResult::Type::ARR, "vin", "", | |
| { | |
| {RPCResult::Type::OBJ, "", "", | |
| { | |
| {RPCResult::Type::STR_HEX, "coinbase", /*optional=*/true, "The coinbase value (only if coinbase transaction)"}, | |
| {RPCResult::Type::STR_HEX, "txid", /*optional=*/true, "The transaction id (if not coinbase transaction)"}, | |
| {RPCResult::Type::NUM, "vout", /*optional=*/true, "The output number (if not coinbase transaction)"}, | |
| {RPCResult::Type::OBJ, "scriptSig",/*optional=*/true, "The script (if not coinbase transaction)", | |
| { | |
| {RPCResult::Type::STR, "asm", "asm"}, | |
| {RPCResult::Type::STR_HEX, "hex", "hex"}, | |
| }}, | |
| {RPCResult::Type::NUM, "sequence", "The script sequence number"}, | |
| }}, | |
| }}, | |
| {RPCResult::Type::ARR, "vout", "", | |
| { | |
| {RPCResult::Type::OBJ, "", "", | |
| { | |
| {RPCResult::Type::STR_AMOUNT, "value", "The value in " + CURRENCY_UNIT}, | |
| {RPCResult::Type::NUM, "n", "index"}, | |
| {RPCResult::Type::OBJ, "scriptPubKey", "", | |
| { | |
| {RPCResult::Type::STR, "asm", "the asm"}, | |
| {RPCResult::Type::STR, "desc", "Inferred descriptor for the output"}, | |
| {RPCResult::Type::STR_HEX, "hex", "the hex"}, | |
| {RPCResult::Type::STR, "type", "The type, eg 'pubkeyhash'"}, | |
| {RPCResult::Type::STR, "address", /*optional=*/true, "The Dash address (only if a well-defined address exists)"}, | |
| }}, | |
| }}, | |
| }}, | |
| {RPCResult::Type::NUM, "extraPayloadSize", /*optional=*/true, "Size of DIP2 extra payload. Only present if it's a special TX"}, | |
| {RPCResult::Type::STR_HEX, "extraPayload", /*optional=*/true, "Hex-encoded DIP2 extra payload data. Only present if it's a special TX"}, | |
| }; | |
| } |
🤖 Prompt for AI Agents
In src/rpc/rawtransaction.cpp around lines 132 to 175, update the DecodeTxDoc
RPC schema: change the "locktime" field type from RPCResult::Type::NUM_TIME to
RPCResult::Type::NUM and adjust its description to note it can be either a block
height or a timestamp; also change the "The Bitcoin address (only if a
well-defined address exists)" string to "The Dash address (only if a
well-defined address exists)" to reflect the correct currency name.
| Cat<std::vector<RPCResult>>( | ||
| { | ||
| {RPCResult::Type::BOOL, "in_active_chain", /* optional */ true, "Whether specified block is in the active chain or not (only present with explicit \"blockhash\" argument)"}, | ||
| {RPCResult::Type::STR_HEX, "txid", "The transaction id (same as provided)"}, | ||
| {RPCResult::Type::NUM, "size", "The serialized transaction size"}, | ||
| {RPCResult::Type::NUM, "version", "The version"}, | ||
| {RPCResult::Type::NUM, "type", "The type"}, | ||
| {RPCResult::Type::NUM_TIME, "locktime", "The lock time"}, | ||
| {RPCResult::Type::ARR, "vin", "", | ||
| { | ||
| {RPCResult::Type::OBJ, "", "", | ||
| { | ||
| {RPCResult::Type::STR_HEX, "txid", "The transaction id"}, | ||
| {RPCResult::Type::NUM, "vout", "The output number"}, | ||
| {RPCResult::Type::OBJ, "scriptSig", "The script", | ||
| { | ||
| {RPCResult::Type::STR, "asm", "asm"}, | ||
| {RPCResult::Type::STR_HEX, "hex", "hex"}, | ||
| }}, | ||
| {RPCResult::Type::NUM, "sequence", "The script sequence number"}, | ||
| }}, | ||
| }}, | ||
| {RPCResult::Type::ARR, "vout", "", | ||
| { | ||
| {RPCResult::Type::OBJ, "", "", | ||
| { | ||
| {RPCResult::Type::NUM, "value", "The value in " + CURRENCY_UNIT}, | ||
| {RPCResult::Type::NUM, "n", "index"}, | ||
| {RPCResult::Type::OBJ, "scriptPubKey", "", | ||
| { | ||
| {RPCResult::Type::STR, "asm", "the asm"}, | ||
| {RPCResult::Type::STR, "desc", "Inferred descriptor for the output"}, | ||
| {RPCResult::Type::STR, "hex", "the hex"}, | ||
| {RPCResult::Type::STR, "type", "The type, eg 'pubkeyhash'"}, | ||
| {RPCResult::Type::STR, "address", /* optional */ true, "The Dash address (only if a well-defined address exists)"}, | ||
| }}, | ||
| }}, | ||
| }}, | ||
| {RPCResult::Type::NUM, "extraPayloadSize", true /*optional*/, "Size of DIP2 extra payload. Only present if it's a special TX"}, | ||
| {RPCResult::Type::STR_HEX, "extraPayload", true /*optional*/, "Hex-encoded DIP2 extra payload data. Only present if it's a special TX"}, | ||
| {RPCResult::Type::STR_HEX, "hex", "The serialized, hex-encoded data for 'txid'"}, | ||
| {RPCResult::Type::STR_HEX, "blockhash", /* optional */ true, "the block hash"}, | ||
| {RPCResult::Type::BOOL, "in_active_chain", /*optional=*/true, "Whether specified block is in the active chain or not (only present with explicit \"blockhash\" argument)"}, | ||
| {RPCResult::Type::STR_HEX, "blockhash", /*optional=*/true, "the block hash"}, | ||
| {RPCResult::Type::NUM, "height", "The block height"}, | ||
| {RPCResult::Type::NUM, "confirmations", /* optional */ true, "The confirmations"}, | ||
| {RPCResult::Type::NUM_TIME, "blocktime", /* optional */ true, "The block time expressed in " + UNIX_EPOCH_TIME}, | ||
| {RPCResult::Type::NUM, "time", /* optional */ true, "Same as \"blocktime\""}, | ||
| {RPCResult::Type::NUM, "confirmations", /*optional=*/true, "The confirmations"}, | ||
| {RPCResult::Type::NUM_TIME, "blocktime", /*optional=*/true, "The block time expressed in " + UNIX_EPOCH_TIME}, | ||
| {RPCResult::Type::NUM, "time", /*optional=*/true, "Same as \"blocktime\""}, | ||
| {RPCResult::Type::BOOL, "instantlock", "Current transaction lock state"}, | ||
| {RPCResult::Type::BOOL, "instantlock_internal", "Current internal transaction lock state"}, | ||
| {RPCResult::Type::BOOL, "chainlock", "The state of the corresponding block ChainLock"}, | ||
| } | ||
| {RPCResult::Type::STR_HEX, "hex", "The serialized, hex-encoded data for 'txid'"}, | ||
| }, | ||
| DecodeTxDoc(/*txid_field_doc=*/"The transaction id (same as provided)")), | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
getrawtransaction help: make “height” optional to match runtime.
height is only added when a block is known (TxToJSON sets it inside the blockhash branch). Mark it optional like the other block fields.
- {RPCResult::Type::NUM, "height", "The block height"},
+ {RPCResult::Type::NUM, "height", /*optional=*/true, "The block height"},🤖 Prompt for AI Agents
In src/rpc/rawtransaction.cpp around lines 236 to 250, the RPCResult entry for
"height" is currently required but at runtime it is only present when a block is
known; mark it optional like the other block-related fields by setting the
optional flag to true for the "height" RPCResult entry so the help output
matches actual behavior.
bf77fea test: fix incorrect named args in txpackage tests (fanquake) Pull request description: Final non-scripted-diff commit split from bitcoin#24661. Could be tested with: `./autogen.sh && ./configure CC=clang-12 CXX=clang++-12 && make clean && bear make -j9 && ( cd ./src/ && run-clang-tidy-12 -j9 )`. Motivation: > Incorrect named args are source of bugs, like bitcoin#22979. > To allow them being checked by clang-tidy, use a format it can understand. ACKs for top commit: ajtowns: ACK bf77fea Tree-SHA512: a13bfb5fc70424b13fbeec7f164d7a0d3b72b27ebec11dfd4115b7782a0037f26e9349e06eef8a6b17b8f529e0c7f43ae37a9c252bde65706dd164704d207d5f
…guments 37a16ff refactor: fix clang-tidy named args usage (fanquake) Pull request description: > Incorrect named args are source of bugs, like bitcoin#22979. > To allow them being checked by clang-tidy, use a format it can understand. Picks up bitcoin#23545, with some additional changes and some feedback addressed. With these changes invoking `./autogen.sh && ./configure CC=clang-12 CXX=clang++-12 && make clean && bear make -j9 && ( cd ./src/ && run-clang-tidy-12 -j9 )` no-longer results in named argument errors out of `clang-tidy`. Ultimately I think we should just add `clang-tidy-*` jobs to the CI and automate things away. ACKs for top commit: MarcoFalke: cr ACK 37a16ff Tree-SHA512: 9bfc0d006eb187755b4fdb0bd92cee9266fc0816be42065ef7dcd885b9020ff12e3cdd7ca3a831613a56a0206d448e690ee4e1fa37628fa2013860e17f416ff3
c848a45 test: fix connman UB by calling derived constructor (chinggg) Pull request description: Hopefully closes bitcoin#24373 by calling `ConnmanTestMsg` test-constructor to avoid undefined behavior in process_message.cpp after casting `g_setup->m_node.connman`. Top commit has no ACKs. Tree-SHA512: c3dce9dcce33614c7b739edf28e416b600ab3d38d16cdb0430490e8ffc9b64aff9292006ae6fe7c636ab0627893bb21f69435893bdfb129a9a865be92baa6f17
…C help 1d95b5c doc: cleanups to mempool rest endpoints (brunoerg) b941dec docs: update `/rest/chaininfo` doc referring to RPC help (brunoerg) Pull request description: Internally, `/rest/chaininfo` gets the infos from `getblockchaininfo` and I just realized the documentation of it in `REST-interface.md` is outdated compared to the `getblockchaininfo` RPC one. This PR removes the documentation of the fields and adds a reference to the RPC help. ACKs for top commit: jonatack: ACK 1d95b5c Tree-SHA512: 643db202e13e8372105460b0871facb11586dc0ff5e86ec9e105a178bcfeefa3555bb047cd28cfaeb3e747f5a2055e27961813c9e299ba7b2d36151e81049507
abcb876 doc: add more info to dependencies.md (Pavol Rusnak) Pull request description: Follow-up to bitcoin#23565 I added more info to dependencies.md - especially links to `depends/packages/*.mk` files and link to PRs where used versions were bumped. Preview at: https://github.com/prusnak/bitcoin/blob/dependencies/doc/dependencies.md ACKs for top commit: fanquake: ACK abcb876 - I didn't click on or test all of the links, but this looks ok. Tree-SHA512: e91deb639afebeb37f7bf05dddad8f70547b51688e938a30692e59dbd7c9e49d52b7f9bfacb74ef60c98862b6f8f444199d0ae06973c42dc647314bc1ffc22d5
0218171 contrib: Remove invalid nodes from seeds list (Wladimir J. van der Laan) 3b09f2b net: 0.19 hardcoded seeds update (Wladimir J. van der Laan) 801d341 contrib: makeseeds: More fancy output (Wladimir J. van der Laan) ed76299 contrib: makeseeds: Limit per network, instead of total (Wladimir J. van der Laan) c254a9e contrib: makeseeds: dedup by ip,port (Wladimir J. van der Laan) 3314d87 contrib: makeseeds: Factor out ASN lookup (Wladimir J. van der Laan) 301c2b1 contrib: makeseeds: Improve logging and filtering (Wladimir J. van der Laan) Pull request description: - contrib: Improve makeseeds script - net: 0.19 hardcoded seeds update Sources: - http://bitcoin.sipa.be/seeds.txt.gz (Sipa) - https://github.com/bitcoin/bitcoin/files/3671913/dnsseed.dump.tar.gz (Sjors) Output: ``` Initial: IPv4 418690, IPv6 55861, Onion 2747 Skip entries with invalid address: IPv4 418690, IPv6 55861, Onion 2747 After removing duplicates: IPv4 409220, IPv6 54028, Onion 2717 Skip entries from suspicious hosts: IPv4 409219, IPv6 54028, Onion 2717 Enforce minimal number of blocks: IPv4 106719, IPv6 46342, Onion 2621 Require service bit 1: IPv4 106384, IPv6 46241, Onion 2542 Require minimum uptime: IPv4 5300, IPv6 1153, Onion 201 Require a known and recent user agent: IPv4 4642, IPv6 1060, Onion 141 Filter out hosts with multiple bitcoin ports: IPv4 4642, IPv6 1060, Onion 141 Look up ASNs and limit results, both per ASN and globally: IPv4 464, IPv6 48, Onion 141 ``` ACKs for top commit: Sjors: ACK 0218171. I also checked that `chainparamsseeds.h` is generated from `nodes_main.txt`. Sounds like we should look at this script a bit more outside release moments :-) Tree-SHA512: c1f5795fe88d14800c4da918387368d51e85f4319f2ce3c0359851d041767e2883f32b1da371bba22bd5f0b442ac3e5ea7d685c233ad2cc4045c930f973b0aa2
c457fb1 improve clarity and up max ipv6 ASNs (Baas) Pull request description: This PR attempts to address some of the areas of improvement raised in bitcoin#17020 . Concretely, my proposed change is fairly minor but addresses the following changes to [`makeseeds.py`](https://github.com/bitcoin/bitcoin/blob/master/contrib/seeds/makeseeds.py): - Increase max seeds per ASN for IPv6 to 10 as recommended [here](bitcoin#16999 (comment)), while keeping max seeds per ASN for IPv4 at 2. - Bump `MIN_BLOCKS` to 730000. - Improved script clarity: added function types and more docs to functions, added progress indicator when performing ASN lookup, and change string formatting to better align with [bitcoin python style guidelines](https://github.com/bitcoin/bitcoin/blob/master/test/functional/README.md#style-guidelines) With the different ASN limits for IPv4 and IPv6, and the new minimum block requirement, the current stats look look like: ``` IPv4 IPv6 Onion Pass 470689 73238 0 Initial 470689 73238 0 Skip entries with invalid address 470689 73238 0 After removing duplicates 470688 73238 0 Skip entries from suspicious hosts 6098 1676 0 Enforce minimal number of blocks 5252 1443 0 Require service bit 1 3812 898 0 Require minimum uptime 3738 877 0 Require a known and recent user agent 3715 869 0 Filter out hosts with multiple bitcoin ports 512 512 0 Look up ASNs and limit results per ASN and per net ``` The new ASN max seeds of 10 allows for 512 IPv6 addresses to be included, up from the ~150 that was filtered by the previous version. While there is more to do for bitcoin#17020 , these changes I think are fairly isolated from the rest and should make it a bit easier for others to get up to speed with what the functions in the script do. ACKs for top commit: laanwj: Concept and code review ACK c457fb1 Tree-SHA512: 3ed67868443cc50544e23b27e2341758c3a8866997b0dba47b137032d5e1a13428855daaeed682626ed471542b44435635178d54848a2cd6fe73777679428032
…eeds 2f629f8 contrib: Remove suspicious hosts list from makeseeds (laanwj) Pull request description: I have some qualms about maintaining a suspicious hosts list as part as the repository\*. But also, it's stale and irrelevant. I've checked the entire list and none of them is connectable. Only one still appars in `nodes_main.txt` but with low uptime and an old subversion string so it wouldn't be picked in the first place. This change removes the list and the functionality to use it. | IP | 8333 connectable | in `nodes_main.txt` | |------------------|---------------------|-----------------------| | 130.211.129.106 | no | no | | 148.251.238.178 | no | no | | 176.9.46.6 | no | yes: /Satoshi:0.9.2.1/ | | 178.63.107.226 | no | no | | 54.173.72.127 | no | no | | 54.174.10.182 | no | no | | 54.183.64.54 | no | no | | 54.194.231.211 | no | no | | 54.66.214.167 | no | no | | 54.66.220.137 | no | no | | 54.67.33.14 | no | no | | 54.77.251.214 | no | no | | 54.94.195.96 | no | no | | 54.94.200.247 | no | no | | 83.81.130.26 | no | no | | 88.198.17.7 | no | no | ref: bitcoin#17020 (comment) \* besides the commit noise, potential legal issues around accountability and liability that would come with maintaining such a blocklist actively, I don't think we should expose the project to ACKs for top commit: Empact: ACK bitcoin@2f629f8 jonatack: ACK 2f629f8 1440000bytes: ACK bitcoin@2f629f8 Tree-SHA512: 3159d7df7cf66415a5db6058b62e5696efcf6c46b0ec38090e22ba26d9b375eb1a88f510b71769eb7b4f14e7007d2b64e1709cf6b1300ade3f7277d50efb3ddb
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/wallet/rpc/transactions.cpp (1)
248-252: Fix invalid RPC example argumentsThe two
HelpExampleRpc("listreceivedbyaddress", …)strings now read"6, false true, …"which misses the comma betweenfalseandtrue. As written the examples are no longer valid JSON-RPC argument lists. Please restore the comma so the help text shows a runnable example.- + HelpExampleRpc("listreceivedbyaddress", "6, false true, true, \"" + EXAMPLE_ADDRESS[0] + "\"") - + HelpExampleRpc("listreceivedbyaddress", "6, false true, true, \"" + EXAMPLE_ADDRESS[0] + "\", true") + + HelpExampleRpc("listreceivedbyaddress", "6, false, true, true, \"" + EXAMPLE_ADDRESS[0] + "\"") + + HelpExampleRpc("listreceivedbyaddress", "6, false, true, true, \"" + EXAMPLE_ADDRESS[0] + "\", true")contrib/seeds/makeseeds.py (2)
52-56: Also exclude IPv6 localhost (‘::1’)Currently only
'::'is filtered. Add'::1'to the localhost guard.- if m.group(1) in ['::']: # Not interested in localhost + if m.group(1) in ('::', '::1'): # Not interested in localhost/loopback return None
136-141: Avoid passing Resolver objects into processes; manage the pool and set DNS timeouts
dns.resolver.Resolvermay not pickle cleanly undermultiprocessing.Pool, and the pool isn’t context‑managed. Prefer a thread pool here (I/O bound), reuse the resolver safely, and ensure cleanup.- my_resolver = dns.resolver.Resolver() - pool = multiprocessing.Pool(processes=16) + my_resolver = dns.resolver.Resolver() + my_resolver.lifetime = 5.0 # fail fast + my_resolver.timeout = 2.0 + from multiprocessing.pool import ThreadPool + pool = ThreadPool(processes=16) @@ - # OpenDNS servers + # OpenDNS servers my_resolver.nameservers = ['208.67.222.222', '208.67.220.220'] @@ - # Resolve ASNs in parallel - asns = [pool.apply_async(resolveasn, args=(my_resolver, ip)) for ip in ips_ipv46] + # Resolve ASNs in parallel + try: + asns = [pool.apply_async(resolveasn, args=(my_resolver, ip)) for ip in ips_ipv46] + finally: + pool.close(); pool.join()If you prefer processes, instantiate the Resolver inside
resolveasnto avoid pickling it. Based on learnings.
🧹 Nitpick comments (8)
doc/dependencies.md (5)
18-22: Use descriptive link text in “Releases” column (MD059)
Replace generic “[link]” with descriptive text (e.g., “Releases”) so screen readers and markdownlint are happy.-| [Boost](../depends/packages/boost.mk) | [link](https://www.boost.org/users/download/) | 1.81.0 | [1.73.0](https://github.com/bitcoin/bitcoin/pull/29066) | No | +| [Boost](../depends/packages/boost.mk) | [Releases](https://www.boost.org/users/download/) | 1.81.0 | [1.73.0](https://github.com/bitcoin/bitcoin/pull/29066) | No | -| [libevent](../depends/packages/libevent.mk) | [link](https://github.com/libevent/libevent/releases) | [2.1.12-stable](https://github.com/bitcoin/bitcoin/pull/21991) | [2.1.8](https://github.com/bitcoin/bitcoin/pull/24681) | No | +| [libevent](../depends/packages/libevent.mk) | [Releases](https://github.com/libevent/libevent/releases) | [2.1.12-stable](https://github.com/bitcoin/bitcoin/pull/21991) | [2.1.8](https://github.com/bitcoin/bitcoin/pull/24681) | No | -| glibc | [link](https://www.gnu.org/software/libc/) | N/A | [2.31](https://github.com/bitcoin/bitcoin/pull/29987) | Yes | +| glibc | [Releases](https://www.gnu.org/software/libc/) | N/A | [2.31](https://github.com/bitcoin/bitcoin/pull/29987) | Yes |
30-36: Surround table with blank lines (MD058) and use descriptive link text (MD059)
Add a blank line after the “### GUI” heading and replace “[link]” with “Releases”.-### GUI +### GUI + | Dependency | Releases | Version used | Minimum required | Runtime | | --- | --- | --- | --- | --- | -| [Fontconfig](../depends/packages/fontconfig.mk) | [link](https://www.freedesktop.org/wiki/Software/fontconfig/) | [2.12.6](https://github.com/bitcoin/bitcoin/pull/23495) | 2.6 | Yes | +| [Fontconfig](../depends/packages/fontconfig.mk) | [Releases](https://www.freedesktop.org/wiki/Software/fontconfig/) | [2.12.6](https://github.com/bitcoin/bitcoin/pull/23495) | 2.6 | Yes | -| [FreeType](../depends/packages/freetype.mk) | [link](https://freetype.org) | [2.11.0](https://github.com/bitcoin/bitcoin/commit/01544dd78ccc0b0474571da854e27adef97137fb) | 2.3.0 | Yes | +| [FreeType](../depends/packages/freetype.mk) | [Releases](https://freetype.org) | [2.11.0](https://github.com/bitcoin/bitcoin/commit/01544dd78ccc0b0474571da854e27adef97137fb) | 2.3.0 | Yes | -| [qrencode](../depends/packages/qrencode.mk) | [link](https://fukuchi.org/works/qrencode/) | [4.1.1](https://github.com/bitcoin/bitcoin/pull/27312) | | No | +| [qrencode](../depends/packages/qrencode.mk) | [Releases](https://fukuchi.org/works/qrencode/) | [4.1.1](https://github.com/bitcoin/bitcoin/pull/27312) | | No | -| [Qt](../depends/packages/qt.mk) | [link](https://download.qt.io/official_releases/qt/) | [5.15.3](https://github.com/bitcoin/bitcoin/pull/24668) | [5.11.3](https://github.com/bitcoin/bitcoin/pull/24132) | No | +| [Qt](../depends/packages/qt.mk) | [Releases](https://download.qt.io/official_releases/qt/) | [5.15.3](https://github.com/bitcoin/bitcoin/pull/24668) | [5.11.3](https://github.com/bitcoin/bitcoin/pull/24132) | No |
38-42: Networking: add blank line before table (MD058) and descriptive link text (MD059)
Insert a blank line after the heading and replace generic link text.-### Networking +### Networking + | Dependency | Releases | Version used | Minimum required | Runtime | | --- | --- | --- | --- | --- | -| [libnatpmp](../depends/packages/libnatpmp.mk) | [link](https://github.com/miniupnp/libnatpmp/) | commit [07004b9...](https://github.com/miniupnp/libnatpmp/tree/07004b97cf691774efebe70404cf22201e4d330d) | | No | +| [libnatpmp](../depends/packages/libnatpmp.mk) | [Releases](https://github.com/miniupnp/libnatpmp/) | commit [07004b9...](https://github.com/miniupnp/libnatpmp/tree/07004b97cf691774efebe70404cf22201e4d330d) | | No | -| [MiniUPnPc](../depends/packages/miniupnpc.mk) | [link](https://miniupnp.tuxfamily.org/) | [2.2.2](https://github.com/bitcoin/bitcoin/pull/20421) | 1.9 | No | +| [MiniUPnPc](../depends/packages/miniupnpc.mk) | [Releases](https://miniupnp.tuxfamily.org/) | [2.2.2](https://github.com/bitcoin/bitcoin/pull/20421) | 1.9 | No |
44-47: Notifications: add blank line before table (MD058) and fix link text (MD059)
Add a blank line and change “[link]” to “Releases”.-### Notifications +### Notifications + | Dependency | Releases | Version used | Minimum required | Runtime | | --- | --- | --- | --- | --- | -| [ZeroMQ](../depends/packages/zeromq.mk) | [link](https://github.com/zeromq/libzmq/releases) | 4.3.5 | 4.0.0 | No | +| [ZeroMQ](../depends/packages/zeromq.mk) | [Releases](https://github.com/zeromq/libzmq/releases) | 4.3.5 | 4.0.0 | No |
49-53: Wallet: add blank line before table (MD058) and fix link text (MD059)
Insert a blank line after the heading and use descriptive link text.-### Wallet +### Wallet + | Dependency | Releases | Version used | Minimum required | Runtime | | --- | --- | --- | --- | --- | -| [Berkeley DB](../depends/packages/bdb.mk) (legacy wallet) | [link](https://www.oracle.com/technetwork/database/database-technologies/berkeleydb/downloads/index.html) | 4.8.30 | 4.8.x | No | +| [Berkeley DB](../depends/packages/bdb.mk) (legacy wallet) | [Releases](https://www.oracle.com/technetwork/database/database-technologies/berkeleydb/downloads/index.html) | 4.8.30 | 4.8.x | No | -| [SQLite](../depends/packages/sqlite.mk) | [link](https://sqlite.org) | 3.38.5 | [3.7.17](https://github.com/bitcoin/bitcoin/pull/19077) | No | +| [SQLite](../depends/packages/sqlite.mk) | [Releases](https://sqlite.org) | 3.38.5 | [3.7.17](https://github.com/bitcoin/bitcoin/pull/19077) | No |contrib/seeds/makeseeds.py (3)
20-25: Remove shadowing constant definition for clarity
MAX_SEEDS_PER_ASNis defined twice (scalar, then dict). Drop the first to avoid confusion.- MAX_SEEDS_PER_ASN=4 - - MAX_SEEDS_PER_ASN = { +MAX_SEEDS_PER_ASN = { 'ipv4': 4, 'ipv6': 10, }
221-224: Cap wording vs behaviorThe log says “limit results per ASN and per net,” but
max_per_netis passed asNSEEDSand the function then appends onions, exceeding the global limit. Either (a) pass a smaller per‑net cap, or (b) keep per‑net caps and additionally enforce the overallNSEEDSlimit (see prior comment).
27-28: Remove unused constantMIN_BLOCKS
MIN_BLOCKSis defined at contrib/seeds/makeseeds.py:27 but never used—delete it to eliminate dead code.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (32)
contrib/seeds/makeseeds.py(4 hunks)doc/REST-interface.md(2 hunks)doc/dependencies.md(1 hunks)src/bench/addrman.cpp(1 hunks)src/bench/mempool_stress.cpp(1 hunks)src/bench/rpc_mempool.cpp(1 hunks)src/bench/wallet_balance.cpp(1 hunks)src/net.cpp(1 hunks)src/net_processing.cpp(2 hunks)src/node/interfaces.cpp(1 hunks)src/qt/peertablemodel.cpp(1 hunks)src/qt/rpcconsole.cpp(2 hunks)src/qt/transactiontablemodel.cpp(1 hunks)src/rpc/blockchain.cpp(9 hunks)src/rpc/coinjoin.cpp(1 hunks)src/rpc/mining.cpp(1 hunks)src/rpc/net.cpp(3 hunks)src/rpc/rawtransaction.cpp(18 hunks)src/rpc/server.cpp(1 hunks)src/rpc/txoutproof.cpp(1 hunks)src/test/txpackage_tests.cpp(1 hunks)src/test/util/setup_common.cpp(2 hunks)src/txdb.cpp(1 hunks)src/txmempool.cpp(1 hunks)src/wallet/rpc/addresses.cpp(3 hunks)src/wallet/rpc/backup.cpp(2 hunks)src/wallet/rpc/coins.cpp(4 hunks)src/wallet/rpc/spend.cpp(7 hunks)src/wallet/rpc/transactions.cpp(10 hunks)src/wallet/rpc/wallet.cpp(2 hunks)src/wallet/spend.cpp(1 hunks)src/wallet/walletdb.cpp(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- src/test/txpackage_tests.cpp
- src/rpc/coinjoin.cpp
- src/bench/addrman.cpp
🚧 Files skipped from review as they are similar to previous changes (21)
- src/qt/rpcconsole.cpp
- src/txmempool.cpp
- src/rpc/net.cpp
- src/net_processing.cpp
- src/node/interfaces.cpp
- src/qt/transactiontablemodel.cpp
- src/txdb.cpp
- src/bench/rpc_mempool.cpp
- doc/REST-interface.md
- src/bench/wallet_balance.cpp
- src/net.cpp
- src/test/util/setup_common.cpp
- src/wallet/walletdb.cpp
- src/rpc/server.cpp
- src/wallet/rpc/spend.cpp
- src/wallet/rpc/coins.cpp
- src/wallet/rpc/wallet.cpp
- src/wallet/spend.cpp
- src/bench/mempool_stress.cpp
- src/wallet/rpc/addresses.cpp
- src/wallet/rpc/backup.cpp
🧰 Additional context used
📓 Path-based instructions (3)
src/**/*.{cpp,h,cc,cxx,hpp}
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.{cpp,h,cc,cxx,hpp}: Dash Core C++ codebase must be written in C++20 and require at least Clang 16 or GCC 11.1
Dash uses unordered_lru_cache for efficient caching with LRU eviction
Files:
src/qt/peertablemodel.cppsrc/rpc/blockchain.cppsrc/rpc/mining.cppsrc/rpc/rawtransaction.cppsrc/wallet/rpc/transactions.cppsrc/rpc/txoutproof.cpp
doc/**
📄 CodeRabbit inference engine (CLAUDE.md)
Unless specifically prompted, avoid making changes to the doc directory (documentation)
Files:
doc/dependencies.md
contrib/**
📄 CodeRabbit inference engine (CLAUDE.md)
Unless specifically prompted, avoid making changes to the contrib directory (contributed scripts)
Files:
contrib/seeds/makeseeds.py
🧠 Learnings (2)
📓 Common learnings
Learnt from: kwvg
PR: dashpay/dash#6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.
📚 Learning: 2025-08-08T07:01:47.332Z
Learnt from: knst
PR: dashpay/dash#6805
File: src/wallet/rpc/wallet.cpp:357-357
Timestamp: 2025-08-08T07:01:47.332Z
Learning: In src/wallet/rpc/wallet.cpp, the upgradetohd RPC now returns a UniValue string message (RPCResult::Type::STR) instead of a boolean, including guidance about mnemonic backup and null-character passphrase handling; functional tests have been updated to assert returned strings in several cases.
Applied to files:
src/rpc/blockchain.cppsrc/wallet/rpc/transactions.cpp
🧬 Code graph analysis (2)
src/rpc/rawtransaction.cpp (2)
src/node/transaction.cpp (2)
BroadcastTransaction(33-118)BroadcastTransaction(33-33)src/node/interfaces.cpp (14)
tx(609-612)tx(609-609)tx(732-735)tx(732-732)tx(736-739)tx(736-736)tx(757-760)tx(757-757)tx(1002-1009)tx(1002-1002)tx(1021-1036)tx(1021-1021)chainman(576-578)chainman(1061-1063)
src/rpc/txoutproof.cpp (1)
src/validation.cpp (1)
GetTransaction(143-143)
🪛 markdownlint-cli2 (0.18.1)
doc/dependencies.md
20-20: Link text should be descriptive
(MD059, descriptive-link-text)
21-21: Link text should be descriptive
(MD059, descriptive-link-text)
22-22: Link text should be descriptive
(MD059, descriptive-link-text)
28-28: Spaces inside link text
(MD039, no-space-in-links)
28-28: Spaces inside link text
(MD039, no-space-in-links)
28-28: Link text should be descriptive
(MD059, descriptive-link-text)
31-31: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
33-33: Link text should be descriptive
(MD059, descriptive-link-text)
34-34: Link text should be descriptive
(MD059, descriptive-link-text)
35-35: Link text should be descriptive
(MD059, descriptive-link-text)
36-36: Link text should be descriptive
(MD059, descriptive-link-text)
39-39: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
41-41: Link text should be descriptive
(MD059, descriptive-link-text)
42-42: Link text should be descriptive
(MD059, descriptive-link-text)
45-45: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
47-47: Link text should be descriptive
(MD059, descriptive-link-text)
50-50: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
52-52: Link text should be descriptive
(MD059, descriptive-link-text)
53-53: Link text should be descriptive
(MD059, descriptive-link-text)
🪛 Ruff (0.13.1)
contrib/seeds/makeseeds.py
118-120: Prefer next(...) over single element slice
Replace with next(...)
(RUF015)
121-121: Consider moving this statement to an else block
(TRY300)
122-122: Do not catch blind exception: Exception
(BLE001)
🪛 Flake8 (7.2.0)
contrib/seeds/makeseeds.py
[error] 120-120: continuation line over-indented for hanging indent
(E126)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: mac-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_sqlite-build / Build source
- GitHub Check: linux64-build / Build source
- GitHub Check: win64-build / Build source
- GitHub Check: linux64_nowallet-build / Build source
- GitHub Check: arm-linux-build / Build source
- GitHub Check: Lint / Run linters
🔇 Additional comments (6)
src/qt/peertablemodel.cpp (1)
85-85: Consistent named-parameter style
The updated comment style matches the project-wide convention adopted in this backport.src/rpc/rawtransaction.cpp (2)
139-139: Clarify thelocktimeRPC schema entry.
locktimeis encoded as either a block height or a UNIX timestamp, so advertising it asNUM_TIME(and describing it simply as “The lock time”) is misleading for callers. Please switch the type back toNUMand mention both possibilities in the docstring to keep the help output accurate.- {RPCResult::Type::NUM_TIME, "locktime", "The lock time"}, + {RPCResult::Type::NUM, "locktime", "Raw locktime (block height or UNIX timestamp)"},
238-244: Markheightas optional in the verbose result schema.In
TxToJSON,heightis only pushed when we have a block context; mempool transactions omit it entirely. The help currently declares it mandatory, which disagrees with runtime behavior. Please mark this entry as optional like the other block-scoped fields so clients are not misled.- {RPCResult::Type::NUM, "height", "The block height"}, + {RPCResult::Type::NUM, "height", /*optional=*/true, "The block height"},contrib/seeds/makeseeds.py (3)
100-125: Harden ASN lookup: fix runtime TypeError and narrow exceptionsThe except path concatenates a dict to a string and catches
Exceptionbroadly. Also, TXT parsing is fragile and timeouts aren’t set. This can crash or mask DNS errors. The diff below fixes the bug, adds timeouts, and simplifies parsing.-def resolveasn(resolver, ip : Dict) -> Union[int, None]: +def resolveasn(resolver, ip: Dict) -> Union[int, None]: @@ - Returns in integer ASN or None if it could not be found. + Returns an integer ASN or None if it could not be found. @@ - try: + try: if ip['net'] == 'ipv4': ipaddr = ip['ip'] prefix = '.origin' else: # http://www.team-cymru.com/IP-ASN-mapping.html res = str() # 2001:4860:b002:23::68 for nb in ip['ip'].split(':')[:4]: # pick the first 4 nibbles for c in nb.zfill(4): # right padded with '0' res += c + '.' # 2001 4860 b002 0023 ipaddr = res.rstrip('.') # 2.0.0.1.4.8.6.0.b.0.0.2.0.0.2.3 prefix = '.origin6' - asn = int([x.to_text() for x in resolver.resolve('.'.join( - reversed(ipaddr.split('.'))) + prefix + '.asn.cymru.com', - 'TXT').response.answer][0].split('\"')[1].split(' ')[0]) - return asn - except Exception: - sys.stderr.write('ERR: Could not resolve ASN for "' + ip + '"\n') - return None + qname = '.'.join(reversed(ipaddr.split('.'))) + prefix + '.asn.cymru.com' + answers = resolver.resolve(qname, 'TXT', lifetime=2.0) + txt = next(iter(answers)).to_text().strip('"') + asn = int(txt.split()[0]) + except (dns.resolver.NXDOMAIN, dns.resolver.NoAnswer, dns.resolver.Timeout, ValueError) as e: + sys.stderr.write(f'ERR: Could not resolve ASN for "{ip.get("ip")}": {e}\n') + return None + else: + return asnOptional: for full IPv6 fidelity, use all 32 nibbles per Team Cymru guidance.
147-166: Per‑ASN quota must be enforced per network (ipv4/ipv6 separately)
asn_countis keyed only by ASN, but the limit is per network. As soon as IPv4 entries reach their quota, IPv6 from the same ASN is prematurely blocked. Key the counter by(net, asn).- asn_count: Dict[int, int] = collections.defaultdict(int) + asn_count: Dict[Tuple[str, int], int] = collections.defaultdict(int) @@ - asn = asns[i].get() - if asn is None or asn_count[asn] == max_per_asn[ip['net']]: + asn = asns[i].get() + key = (ip['net'], asn) if asn is not None else None + if asn is None or asn_count[key] == max_per_asn[ip['net']]: # do not add this ip as we already have too many # ips from this ASN on this network continue - asn_count[asn] += 1 + asn_count[key] += 1Outside this hunk, add
Tupleto the typing import:-from typing import List, Dict, Union +from typing import List, Dict, Union, Tuple
168-170: Enforce the historical global NSEEDS capAs written, this can emit up to
NSEEDSper network plus onions. Restore a global cap before returning.- result.extend(ips_onion[0:max_per_net]) - return result + result.extend(ips_onion[0:max_per_net]) + return result[:max_per_net]
| # Extract IPs | ||
| ips = [parseip(mn['state']['addresses']['core_p2p'][0]) for mn in mns] | ||
| ips = [parseip(mn['state']['addresses'][0]) for mn in mns] | ||
| for onion in onions: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Guard against empty addresses arrays
Indexing addresses[0] may raise if the array is empty/missing. Skip such MNs safely.
- ips = [parseip(mn['state']['addresses'][0]) for mn in mns]
+ ips = []
+ for mn in mns:
+ addrs = mn['state'].get('addresses') or []
+ if not addrs:
+ continue
+ ips.append(parseip(addrs[0]))📝 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.
| # Extract IPs | |
| ips = [parseip(mn['state']['addresses']['core_p2p'][0]) for mn in mns] | |
| ips = [parseip(mn['state']['addresses'][0]) for mn in mns] | |
| for onion in onions: | |
| # Extract IPs | |
| ips = [] | |
| for mn in mns: | |
| addrs = mn['state'].get('addresses') or [] | |
| if not addrs: | |
| continue | |
| ips.append(parseip(addrs[0])) | |
| for onion in onions: |
| ## Required | ||
|
|
||
| | Dependency | Version used | Minimum required | Runtime | | ||
| | --- | --- | --- | --- | | ||
| | [Boost](https://www.boost.org/users/download/) | 1.81.0 | [1.73.0](https://github.com/bitcoin/bitcoin/pull/29066) | No | | ||
| | [libevent](https://github.com/libevent/libevent/releases) | 2.1.12-stable | [2.1.8](https://github.com/bitcoin/bitcoin/pull/24681) | No | | ||
| | [glibc](https://www.gnu.org/software/libc/) | N/A | [2.31](https://github.com/bitcoin/bitcoin/pull/29987) | Yes | | ||
| | Dependency | Releases | Version used | Minimum required | Runtime | | ||
| | --- | --- | --- | --- | --- | | ||
| | [Boost](../depends/packages/boost.mk) | [link](https://www.boost.org/users/download/) | 1.81.0 | [1.73.0](https://github.com/bitcoin/bitcoin/pull/29066) | No | | ||
| | [libevent](../depends/packages/libevent.mk) | [link](https://github.com/libevent/libevent/releases) | [2.1.12-stable](https://github.com/bitcoin/bitcoin/pull/21991) | [2.1.8](https://github.com/bitcoin/bitcoin/pull/24681) | No | | ||
| | glibc | [link](https://www.gnu.org/software/libc/) | N/A | [2.31](https://github.com/bitcoin/bitcoin/pull/29987) | Yes | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Optional: quick verification script to catch residual “[link]” anchors
Run this to ensure no “[link]” anchors remain and tables are surrounded by blank lines.
Also applies to: 26-36, 38-47, 49-53
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Ensure there are no generic [link](...) anchors left
rg -n '\[link\]\(' doc/dependencies.md || echo "No generic [link] anchors found."
# 2) Spot tables not preceded by a blank line (simple heuristic)
# Prints header lines not preceded by a blank line.
awk '
/^\|[[:space:]]*Dependency[[:space:]]*\|/ {
if (prev_blank != 1) {
printf("Table header missing leading blank line at line %d\n", NR)
}
}
{ prev_blank = ($0 ~ /^[[:space:]]*$/) ? 1 : 0 }
' doc/dependencies.mdLength of output: 2400
Replace generic “[link]” anchors and add blank lines before tables
- doc/dependencies.md still contains generic
[link](…)anchors (e.g. lines 20–22, 28, 33–36, 41–42, 47, 52–53); replace each with descriptive link text. - Ensure every Markdown table is preceded by a blank line for proper rendering.
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
20-20: Link text should be descriptive
(MD059, descriptive-link-text)
21-21: Link text should be descriptive
(MD059, descriptive-link-text)
22-22: Link text should be descriptive
(MD059, descriptive-link-text)
🤖 Prompt for AI Agents
In doc/dependencies.md around lines 16 to 22 (and other occurrences listed in
the comment: lines ~28, 33–36, 41–42, 47, 52–53), replace every generic
"[link](...)" anchor with descriptive link text that indicates the target (e.g.,
"Boost downloads", "libevent releases", "glibc homepage", or the specific
project/release page) while keeping the original URLs, and ensure every Markdown
table in the file is preceded by a single blank line (insert a blank line
immediately before each table header) so tables render correctly.
| | Dependency | Releases | Version used | Minimum required | Runtime | | ||
| | --- | --- | --- | --- | --- | | ||
| | libgmp | [link](https://gmplib.org/download/gmp/)<sup>[ \* ](#note1)</sup> | 6.3.0 | [6.2.0](https://github.com/dashpay/bls-signatures/pull/92) | No | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix footnote formatting (MD039) and descriptive link text (MD059)
Remove spaces inside the footnote link text and use descriptive link text in Releases column.
-| libgmp | [link](https://gmplib.org/download/gmp/)<sup>[ \* ](#note1)</sup> | 6.3.0 | [6.2.0](https://github.com/dashpay/bls-signatures/pull/92) | No |
+| libgmp | [Releases](https://gmplib.org/download/gmp/)<sup>[*](#note1)</sup> | 6.3.0 | [6.2.0](https://github.com/dashpay/bls-signatures/pull/92) | No |📝 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.
| | Dependency | Releases | Version used | Minimum required | Runtime | | |
| | --- | --- | --- | --- | --- | | |
| | libgmp | [link](https://gmplib.org/download/gmp/)<sup>[ \* ](#note1)</sup> | 6.3.0 | [6.2.0](https://github.com/dashpay/bls-signatures/pull/92) | No | | |
| | Dependency | Releases | Version used | Minimum required | Runtime | | |
| | --- | --- | --- | --- | --- | | |
| | libgmp | [Releases](https://gmplib.org/download/gmp/)<sup>[*](#note1)</sup> | 6.3.0 | [6.2.0](https://github.com/dashpay/bls-signatures/pull/92) | No | |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
28-28: Spaces inside link text
(MD039, no-space-in-links)
28-28: Spaces inside link text
(MD039, no-space-in-links)
28-28: Link text should be descriptive
(MD059, descriptive-link-text)
🤖 Prompt for AI Agents
In doc/dependencies.md around lines 26 to 28, the table cell for libgmp uses a
non-descriptive link text ("link") and a footnote label with spaces ("[ \*
](#note1)"), which triggers MD059 and MD039; replace the Releases column link
text with a descriptive label such as "GMP releases" (or "GMP download page")
pointing to https://gmplib.org/download/gmp/ and remove the spaces in the
footnote token so it reads "[*](#note1)"; ensure the resulting markdown uses
descriptive link text and a correctly formatted footnote marker.
UdjinM6
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK f0d5a82
src/rpc/rawtransaction.cpp
Outdated
| {RPCResult::Type::STR_HEX, "txid", /*optional=*/true, "The transaction id (if not coinbase transaction)"}, | ||
| {RPCResult::Type::NUM, "vout", /*optional=*/true, "The output number (if not coinbase transaction)"}, | ||
| {RPCResult::Type::OBJ, "scriptSig", /*optional=*/true, "The script (if not coinbase transaction)", | ||
| { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
24716: nit: extra {, will be dropped in the next commit (pr 24721), no need to fix it here
kwvg
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK f0d5a82
…]` help text, drop extra `generate`s from test, resolve macOS GID issue 39e847c fix: do not create group if GID is already taken (Kittywhiskers Van Gogh) 36db83a fix: annotate `getblockchaininfo[softforks]` as `OBJ_DYN` (Kittywhiskers Van Gogh) e4ace44 fix: repair IP extraction in `makeseeds.py`, read from `core_p2p` arr (Kittywhiskers Van Gogh) 9ef8c53 test: drop extra `generate`s from `wallet_signrawtransactionwithwallet` (Kittywhiskers Van Gogh) Pull request description: ## Additional Information * The help text for `getblockchaininfo['softforks']` was introduced in [bitcoin#16060](bitcoin#16060) (backported in [dash#5255](#5255)) _but_ the `RPCResult` help text was introduced in [bitcoin#17809](bitcoin#17809), backported _before_ [bitcoin#16060](bitcoin#16060). This resulted in the help text for `softforks` deviating from upstream without the output to match (which is aligned with upstream). This has since been resolved as the output structure hasn't changed, only the help text. * [bitcoin#22818](bitcoin#22818) (backported in [dash#6214](#6214)) activated many softforks at height 1, which rendered the block generation calls in `wallet_signrawtransactionwithwallet.py` ([source](https://github.com/dashpay/dash/blob/3e6e8f57c45fb24107816a10854ed8f349c4aa20/test/functional/wallet_signrawtransactionwithwallet.py#L132-L133)) no longer necessary, additionally [bitcoin#22550](bitcoin#22550) (backported in [dash#6189](#6189)) added assertions to validate CSV/CLTV activation that were not included in the backport. This has since been resolved, replacing the `generate` calls with assertions. * The locktime height was dropped from `1000` to `100` (matching with upstream, [source](https://github.com/bitcoin/bitcoin/blob/24.x/test/functional/wallet_signrawtransactionwithwallet.py#L219)) as we are no longer generating enough blocks to reach that height anymore. * `makeseeds.py` is currently mangled because [dash#6666](#6666) updated the script to look for the `core_p2p` purpose even though purpose code support was spun-off into [dash#6674](#6674). This meant, in between [dash#6666](#6666) and [dash#6674](#6674 merger, `makeseeds.py` was broken. This necessitated a fix that was introduced as e714c06 ([dash#6858](#6858)) but meant that after [dash#6674](#6674), `makeseeds.py` is broken again. This has been resolved as part of this PR. * macOS users who want to utilise the `develop` container ([source](https://github.com/dashpay/dash/blob/b82450aa7dce2307b13e6fe282f7c4f84b48cbbf/contrib/containers/develop/Dockerfile)) need to set the `USER_ID` and `GROUP_ID` `args` to avoid permissions issues. This creates a slight problem as the GID for `staff` is 20 on macOS but Linux uses GID 20 for `dialout`, so creating a group with GID 20 will _fail_. As we only care about the GID, it is safe to skip the group creation step and assign the user to the group anyways as we do it by GID and _not_ name (the user will be presented as a member of the `dialout` group). This should resolve issues for macOS users trying to build the `develop` container. ## Breaking Changes None expected. ## 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 - [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: PastaPastaPasta: utACK 39e847c knst: utACK 39e847c UdjinM6: utACK 39e847c Tree-SHA512: 0ecb8bb930d54d4b9024a6e94a2fe3b49c7334666e5f67d38920ac58f54332fb8b4f62bd883bd949969d60b6b5183c7a20a3593f24c2e6bbbb0de143b3de8fe1
…ftforks]` help text, drop extra `generate`s from test, resolve macOS GID issue 39e847c fix: do not create group if GID is already taken (Kittywhiskers Van Gogh) 36db83a fix: annotate `getblockchaininfo[softforks]` as `OBJ_DYN` (Kittywhiskers Van Gogh) e4ace44 fix: repair IP extraction in `makeseeds.py`, read from `core_p2p` arr (Kittywhiskers Van Gogh) 9ef8c53 test: drop extra `generate`s from `wallet_signrawtransactionwithwallet` (Kittywhiskers Van Gogh) Pull request description: ## Additional Information * The help text for `getblockchaininfo['softforks']` was introduced in [bitcoin#16060](bitcoin#16060) (backported in [dash#5255](dashpay#5255)) _but_ the `RPCResult` help text was introduced in [bitcoin#17809](bitcoin#17809), backported _before_ [bitcoin#16060](bitcoin#16060). This resulted in the help text for `softforks` deviating from upstream without the output to match (which is aligned with upstream). This has since been resolved as the output structure hasn't changed, only the help text. * [bitcoin#22818](bitcoin#22818) (backported in [dash#6214](dashpay#6214)) activated many softforks at height 1, which rendered the block generation calls in `wallet_signrawtransactionwithwallet.py` ([source](https://github.com/dashpay/dash/blob/3e6e8f57c45fb24107816a10854ed8f349c4aa20/test/functional/wallet_signrawtransactionwithwallet.py#L132-L133)) no longer necessary, additionally [bitcoin#22550](bitcoin#22550) (backported in [dash#6189](dashpay#6189)) added assertions to validate CSV/CLTV activation that were not included in the backport. This has since been resolved, replacing the `generate` calls with assertions. * The locktime height was dropped from `1000` to `100` (matching with upstream, [source](https://github.com/bitcoin/bitcoin/blob/24.x/test/functional/wallet_signrawtransactionwithwallet.py#L219)) as we are no longer generating enough blocks to reach that height anymore. * `makeseeds.py` is currently mangled because [dash#6666](dashpay#6666) updated the script to look for the `core_p2p` purpose even though purpose code support was spun-off into [dash#6674](dashpay#6674). This meant, in between [dash#6666](dashpay#6666) and [dash#6674](dashpay#6674 merger, `makeseeds.py` was broken. This necessitated a fix that was introduced as e714c06 ([dash#6858](dashpay#6858)) but meant that after [dash#6674](dashpay#6674), `makeseeds.py` is broken again. This has been resolved as part of this PR. * macOS users who want to utilise the `develop` container ([source](https://github.com/dashpay/dash/blob/b82450aa7dce2307b13e6fe282f7c4f84b48cbbf/contrib/containers/develop/Dockerfile)) need to set the `USER_ID` and `GROUP_ID` `args` to avoid permissions issues. This creates a slight problem as the GID for `staff` is 20 on macOS but Linux uses GID 20 for `dialout`, so creating a group with GID 20 will _fail_. As we only care about the GID, it is safe to skip the group creation step and assign the user to the group anyways as we do it by GID and _not_ name (the user will be presented as a member of the `dialout` group). This should resolve issues for macOS users trying to build the `develop` container. ## Breaking Changes None expected. ## 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 - [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: PastaPastaPasta: utACK 39e847c knst: utACK 39e847c UdjinM6: utACK 39e847c Tree-SHA512: 0ecb8bb930d54d4b9024a6e94a2fe3b49c7334666e5f67d38920ac58f54332fb8b4f62bd883bd949969d60b6b5183c7a20a3593f24c2e6bbbb0de143b3de8fe1
What was done?
Regular backports from Bitcoin Core v24
How Has This Been Tested?
Run unit & functional tests.
To test
makeseeds.py:Breaking Changes
N/A
Checklist:
Go over all the following points, and put an
xin all the boxes that apply.