Repository navigation
backport: v0.26 bitcoin#25862, bitcoin#27499 - #84
Open
DCG-Claude wants to merge 2 commits into
Open
DCG-Claude wants to merge 2 commits into
DCG-Claude wants to merge 2 commits into
Conversation
DCG-Claude
force-pushed
the
backport-0.26-b064-src-node
branch
from
September 26, 2026 20:25
807c261 to
9bc7bd5
Compare
Collaborator
Author
The lint failure was a new circular dependency, dbwrapper -> node/database_args -> dbwrapper. It came from the Dash-only adaptation in bitcoin#25862 that made util::MakeDbWrapper call node::ReadDatabaseArgs from dbwrapper.h. I removed the <node/database_args.h> include and now set DBOptions{.force_compact = gArgs.GetBoolArg("-forcecompactdb", false)} directly, so -forcecompactdb still applies to Dash's databases as it did before the backport. lint-circular-dependencies.py now passes locally and a syntax-only compile of dbwrapper.h with MakeDbWrapper succeeds; I did not run a full build. (folded into the 🤖 backportsys, on behalf of the Dash backport pipeline. |
Collaborator
Author
I tried a fix but it failed local gates in bitcoin#28148 (verify: Upstream bitcoin#28148 does not touch the -maxorphantxsize handling. This replaces the std::max(int64_t{0}, *value) line kept from the parent commit with a new overflow clamp, which changes behaviour and is not in upstream or in the commit's declared Dash adaptations.; Added only to support the invented std::clamp change. Upstream adds no includes.; Added only to support the invented std::numeric_). bitcoin#28148 has had its repair lane and is being dropped from this branch. 🤖 backportsys, on behalf of the Dash backport pipeline. |
DCG-Claude
changed the title
DCG-Claude
force-pushed
the
backport-0.26-b064-src-node
branch
from
September 28, 2026 05:14
9bc7bd5 to
e796595
Compare
Collaborator
Author
|
Branch rewritten at
🤖 backportsys, on behalf of the Dash backport pipeline. |
…rapper and txdb aadd7c5 refactor, validation: Add ChainstateManagerOpts db options (Ryan Ofsky) 0352258 refactor, txdb: Use DBParams struct in CBlockTreeDB (Ryan Ofsky) c00fa1a refactor, txdb: Add CoinsViewOptions struct (Ryan Ofsky) 2eaeded refactor, dbwrapper: Add DBParams and DBOptions structs (Ryan Ofsky) Pull request description: Code in the libbitcoin_kernel library should not be calling `ArgsManager` methods or trying to read options from the command line. Instead it should just get options values from simple structs and function arguments that are passed in externally. This PR removes `gArgs` accesses from `dbwrapper` and `txdb` modules by defining appropriate options structs, and is a followup to PR's bitcoin#25290 bitcoin#25487 bitcoin#25527 which remove other `ArgsManager` calls from kernel modules. This PR does not change behavior in any way. It is a simpler alternative to bitcoin#25623 because the only thing it does is remove `gArgs` references from kernel code. It avoids other unnecessary changes like adding options to the kernel API (they can be added separately later). ACKs for top commit: TheCharlatan: Code review ACK aadd7c5 achow101: ACK aadd7c5 furszy: diff ACK aadd7c5 Tree-SHA512: 46dfd5d99ab3110492e7bba97a87122c831b8344caaf7dd2ebdb6e0ad6aa9174d4d1832d6f3a7465eda9294fe50defaa3c000afbbddc4e72838687df09a63ffd Dash adaptations: - src/kernel/chainstatemanager_opts.h: only `fs::path datadir;` added; upstream's conflict context also carried `adjusted_time_callback`, which Dash already removed with bitcoin#28956, so it is not re-added - src/bitcoin-chainstate.cpp, src/init.cpp (AppInitMain), src/test/util/setup_common.cpp, src/test/validation_chainstatemanager_tests.cpp: added `.datadir = ...GetDataDirNet()` without the `.adjusted_time_callback` line from the upstream context, for the same reason - src/validation.cpp: new CoinsViews(DBParams, CoinsViewOptions) ctor placed after Dash's GetBlockSubsidy(const CBlockIndex*, ...), which is kept - src/dbwrapper.h: DBOptions/DBParams added after Dash's CharCast helper, which is kept - src/test/dbwrapper_tests.cpp: unicodepath test keeps Dash's '∋' path name (Dash replaces '₿') and only takes the DBParams constructor change - src/test/util/setup_common.cpp: the CBlockTreeDB(DBParams{...}) change sits next to Dash's mn_sync/clhandler setup, which is kept - src/dbwrapper.h: Dash-only util::MakeDbWrapper (used by evodb, llmq recsigdb/quorumdb/dkgdb and isdb) now builds DBParams; its options come from node::ReadDatabaseArgs(gArgs, ...), like upstream's BaseIndex::DB, so -forcecompactdb still applies to Dash's databases. Added includes for <node/database_args.h> and <util/system.h> - src/txdb.cpp: Dash-only CBlockTreeDB::MigrateOldIndexData opens chainstate/timestampindex/spentindex/addressindex CDBWrappers; converted to DBParams{.path, .cache_bytes = 0} (same memory/wipe/obfuscate values as before) - src/node/chainstate.cpp: Dash-only RecoverSnapshotCleanup's CCoinsViewDB converted to {{.path = normal, .cache_bytes = 1 << 20, .obfuscate = true}, {}}; obfuscate=true is kept because the old CCoinsViewDB always set it and upstream now leaves it to the caller - src/Makefile.am: node/database_args.cpp added to libdashkernel_la_SOURCES, because Dash's kernel lib includes index/base.cpp (upstream's does not), which now calls node::ReadDatabaseArgs
…rom gArgs 23c7b51 [net processing] Move -capturemessages to PeerManager::Options (dergoegge) bd59bda [net processing] Move -blockreconstructionextratxn to PeerManager::Options (dergoegge) 567c4e0 [net processing] Move -maxorphantx to PeerManager::Options (dergoegge) fa9e6d8 [net processing] Move -txreconciliation to PeerManager::Options (dergoegge) 4cfb7b9 [net processing] Use ignore_incoming_txs from m_opts (dergoegge) 8b87725 [net processing] Introduce PeerManager options (dergoegge) Pull request description: This PR decouples `PeerManager` from our global args manager by introducing `PeerManager::Options`. ACKs for top commit: stickies-v: re-ACK 23c7b51 TheCharlatan: ACK 23c7b51 Tree-SHA512: cd807b36ec018010e11935d3539fa7ed5015fdfb531d13a042a65b54ee8533a35a919a6a6c5fa293b5cba76000e9403c64dfd790fb9c649b7838544929b1fee8 Dash adaptations: - src/net_processing.h: PeerManager::Options has `uint32_t max_orphan_txs_size{DEFAULT_MAX_ORPHAN_TRANSACTIONS_SIZE * 1000000}` instead of upstream's `max_orphan_txs{DEFAULT_MAX_ORPHAN_TRANSACTIONS}`, because Dash limits orphans by total size (-maxorphantxsize, MB), not by count (-maxorphantx). The DEFAULT_MAX_ORPHAN_TRANSACTIONS constant is not added (Dash doesn't have it), and DEFAULT_MAX_ORPHAN_TRANSACTIONS_SIZE is kept - src/net_processing.h: DEFAULT_TXRECONCILIATION_ENABLE moved here from node/txreconciliation.h as upstream does, placed above Dash's DEFAULT_MAX_ORPHAN_TRANSACTIONS_SIZE - src/net_processing.h/.cpp: PeerManager::make and the PeerManagerImpl constructor keep all of Dash's extra parameters (dstxman, mn_metaman, mn_sync, sporkman, chainlocks, clhandler, nodeman, dmnman, cj_walletman, isman, llmq_ctx); only the trailing `bool ignore_incoming_txs` becomes `Options opts`, and the m_opts{opts} initializer goes after m_clhandler - src/net_processing.cpp: the orphan-limit line becomes `m_orphanage.LimitOrphans(m_opts.max_orphan_txs_size)` in place of Dash's inline gArgs -maxorphantxsize*1000000 computation. Dash's ForgetTx() call and comments are kept (upstream's m_txrequest.ForgetTxHash/GetWitnessHash lines are not brought in) - src/net_processing.cpp: AddToCompactExtraTransactions keeps Dash's GetInstanceHash() (upstream uses GetWitnessHash()); only the gArgs read becomes m_opts.max_extra_txs - src/net_processing.cpp: upstream's `-#include <common/args.h>` hunk doesn't apply because Dash's net_processing.cpp never included it. util/system.h stays because Dash still uses gArgs there for -pushversion and the devnet name - src/node/peerman_args.cpp: includes <util/system.h> instead of <common/args.h> (Dash hasn't done the common/args split, bitcoin#27419, and ArgsManager lives in util/system.h); reads -maxorphantxsize and stores clamp(>=0)*1000000 into max_orphan_txs_size, the same value Dash computed inline before; the other three options match upstream - src/init.cpp: peerman_opts is built and ApplyArgsManOptions is called just before Dash's PeerManager::make, whose Dash arguments are unchanged, with peerman_opts as the last argument. The node/txreconciliation.h include becomes node/peerman_args.h, and Dash's node/sync_manager.h include stays - src/test/util/setup_common.{h,cpp}: Dash builds peers through its own MakePeerManager(connman, node, banman, ...) helper, so the helper's last parameter changes from `bool ignore_incoming_txs` to `PeerManager::Options opts`. TestingSetup builds peerman_opts with ApplyArgsManOptions(*m_node.args, ...) as upstream does. setup_common.h now includes <net_processing.h> because Options must be a complete type there (node/context.h only forward-declares PeerManager). Upstream's connman line in this hunk is not taken, because Dash creates connman earlier in ChainTestingSetup - src/test/denialofservice_tests.cpp: the four call sites keep Dash's MakePeerManager helper and pass `{}` instead of `/*ignore_incoming_txs=*/false`, which matches upstream's use of `{}` - src/test/net_peer_connection_tests.cpp (Dash-only caller, not touched upstream): its MakePeerManager call now passes `{}` for the new Options parameter
DCG-Claude
force-pushed
the
backport-0.26-b064-src-node
branch
from
October 1, 2026 11:05
e796595 to
8b9cea6
Compare
Collaborator
Author
The backport's Dash adaptation in node/peerman_args.cpp multiplied -maxorphantxsize by 1000000 in uint32_t, which overflows at startup for -maxorphantxsize=100000 (mempool_package_onemore.py); Dash's old inline code had the same overflow but only ran when an orphan was handled. It now clamps the MB value to [0, UINT32_MAX/1000000] before multiplying, using the std::clamp style upstream uses for -blockreconstructionextratxn, so the value saturates at about 4.29 GB instead of wrapping. (folded into the 🤖 backportsys, on behalf of the Dash backport pipeline. |
Potential PR merge conflictsThis is advisory only. It does not block CI, but it marks PRs that will likely need a rebase depending on merge order. If these PRs merge firstThis PR will likely need a rebase:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Automated Bitcoin Core v0.26 backports, batch
backport-0.26-b064-src-node.d1104162588b9cea6bd0Failed gates (not included):
const ArgsManager& args{EnsureAnyArgsman(...)}in getblockchaininfo. Dash's getblockchaininfo still readsargsfor its devnet chain name (args.IsArgSet("-devnet")/ `args.GetDProvenance
Each commit passed: cherry-pick (adapted by an Opus lane only where conflicts existed), build, touched tests, a mechanical diff-of-diffs check (every upstream hunk landed; no added line without an upstream counterpart), and an independent Opus verification lane where anything was adapted. Gate rows and lane artifacts are in the backportsys DB.