Skip to content

Tsan regression perf - #10

Open
knst wants to merge 13 commits into
developfrom
tsan-regression-perf
Open

knst wants to merge 13 commits into
developfrom
tsan-regression-perf

Conversation

@knst

@knst knst commented Oct 18, 2026

Copy link
Copy Markdown
Owner

Please remove the italicized help prompts before submitting or merging

Provide a general summary of your changes in the Title above. The title must
follow the format described in
CONTRIBUTING.md.

Do not manually line wrap text; allow the GitHub UI to dynamically wrap lines.

CONTRIBUTING.md
describes what is expected of a pull request, including tests, refactoring and
backports.

Pull requests without a rationale and clear improvement may be closed
immediately.

Please provide clear motivation for your patch and explain how it improves
Dash Core user experience or Dash Core developer experience
significantly:

Issue being fixed or feature implemented

  • Why is this change required? What problem does it solve?
  • If it fixes an open issue, please link to the issue here.

What was done?

Describe your changes in detail. For a backport or backports, name the upstream PRs and
explain every omitted hunk.

How Has This Been Tested?

Please describe in detail how you tested your changes: which existing tests
cover the change, which new tests were added and what they observe, or why
no test applies. State only what was actually run.

Include details of your testing environment, and the tests you ran
to see how your change affects other areas of the code, etc.

Breaking Changes

Please describe any breaking changes your code introduces, or state that
there are none.

Checklist:

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • I have assigned this pull request to a milestone (for repository code-owners and collaborators only)

…sgproc_mutex to Peer

3a060ae scripted-diff: Rename nUnconnectingHeaders and fPreferHeaders (dergoegge)
279c53d [net processing] Move m_recently_announced_invs from CNodeState to Peer (dergoegge)
938a8e2 [net processing] Annotate m_recently_announced_invs as guarded by g_msgproc_mutex (dergoegge)
8a2cb1f [net processing] Move fPreferHeaders from CNodeState to Peer (dergoegge)
3605011 [net processing] Annotate fPreferHeaders as guarded by g_msgproc_mutex (dergoegge)
4b84e50 [net processing] Move m_headers_sync_timeout from CNodeState to Peer (dergoegge)
689b747 [net processing] Annotate m_headers_sync_timeout as guarded by g_msgproc_mutex (dergoegge)
d8c0d1c [net processing] Move nUnconnectingHeaders from CNodeState to Peer (dergoegge)
5f80d8d [net processing] Annotate nUnconnectingHeaders as guarded by g_msgproc_mutex (dergoegge)
1d87137 [validation] Annotate ChainstateManager::m_best_header as guarded by cs_main (dergoegge)

Pull request description:

  `nUnconnectingHeaders`, `m_headers_sync_timeout`, `fPreferHeaders` and  `m_recently_announced_headers` are currently all `CNodeState` members even though they are only ever accessed from the message processing thread (therefore sufficiently guarded exclusively by `g_msgproc_mutex`). `CNodeState` exists purely to hold validation-specific state guarded by `cs_main` that is accessed by multiple threads.

  This PR adds thread-safety annotations for the above mentioned `CNodeState` members and moves them to `Peer`.

ACKs for top commit:
  glozow:
    code review ACK 3a060ae, as in I am convinced these members shouldn't be guarded by cs_main and belong in Peer/TxRelay. clang checked the annotations for me.
  hebasto:
    ACK 3a060ae

Tree-SHA512: 2db27c03f2c6ed36ad7dfbb4f862eeed3c3e57f845cf8abb9e7cada36f976257311892020bbcff513fbe662a881c93270e3a126946ceb0c3f94213b546bcaa81

Dash adaptations:
- src/net_processing.cpp: FindTxForGetData takes (const Peer&, const uint256& txid) not GenTxid; Dash has no wtxid relay and keeps its asset-unlock instance-hash lookup
- src/net_processing.cpp: upstream's new `const TxRelay* GetTxRelay() const` is dropped; Dash already has one (respecting m_can_tx_relay) and m_tx_relay_mutex is already mutable
- src/net_processing.cpp: HandleFewUnconnectingHeaders keeps Dash's GETHEADERS/GETHEADERS2 choice and the msg_type in its log line, now using the cs_main-snapshotted best_header
- src/net_processing.cpp: UpdatePeerStateForReceivedHeaders call keeps Dash's GetHeadersLimit(pfrom, uses_compressed) in place of MAX_HEADERS_RESULTS
- src/net_processing.cpp: Dash's DSTX-aware tx announcement loop inserts announce_hash into tx_relay->m_recently_announced_invs, not the CNodeState copy
- src/net_processing.cpp: Dash-only fPreferHeadersCompressed (SENDHEADERS2) and m_recent_object_requests stay in CNodeState; upstream only moves fPreferHeaders
- src/net_processing.cpp: the headers-sync timeout check keeps Dash's max_tip_age condition; only state.m_headers_sync_timeout becomes peer->m_headers_sync_timeout
- test/functional/p2p_sendheaders_compressed.py: same MAX_UNCONNECTING_HEADERS -> MAX_NUM_UNCONNECTING_HEADERS_MSGS rename as in Dash's fork of p2p_sendheaders.py
a2bef80 kernel: update m_assumed_* chain params for 25.x (fanquake)
4128e01 kernel: update chainTxData for 25.x (fanquake)
00b2b11 kernel: update nMinimumChainWork & defaultAssumeValid for 25.x (fanquake)
07fcc0a doc: update references to kernel/chainparams.cpp (fanquake)

Pull request description:

  Update chainparams pre `25.x` branch off.
  Co-Author in the commits as a PR (bitcoin#27223) had previously been opened too-early to do the same.

  Note: Remember that some variance is expected in the `m_assumed_*` sizes.

ACKs for top commit:
  achow101:
    ACK a2bef80
  josibake:
    ACK bitcoin@a2bef80
  gruve-p:
    ACK bitcoin@a2bef80
  dergoegge:
    ACK a2bef80 on the new mainnet params

Tree-SHA512: 0b19c2ef15c6b15863d6a560a1053ee223057c7bfb617ffd3400b1734cee8f75bc6fd7f04d8f8e3f5af6220659a1987951a1b36945d6fe17d06972004fd62610

Dash adaptations:
- doc/release-process.md: fixes the chainparams.cpp path in both of Dash's checklist items, since Dash splits upstream's single item into minor/major-release steps
- src/kernel/chainparams.cpp: keeps Dash's own nMinimumChainWork, defaultAssumeValid, m_assumed_* and chainTxData values instead of upstream's Bitcoin 25.x values

Not applicable to Dash (intentionally omitted):
- src/kernel/chainparams.cpp: Bitcoin 25.x chain data (main/test/signet chainwork, assumevalid, sizes, txstats); Dash has its own chain, values and release-time updates, and no signet
@knst
knst force-pushed the tsan-regression-perf branch from 7b22165 to 98903cf Compare October 18, 2026 06:48
@knst

knst commented Oct 18, 2026 •

Copy link
Copy Markdown
Owner Author

6m 5s

no sanitizer to secp + no deadlock-stack

@knst

knst commented Oct 18, 2026 •

Copy link
Copy Markdown
Owner Author

16m 36s

only sanitizer` to secp ; deadlock stack is reverted

@knst
knst force-pushed the tsan-regression-perf branch from 98903cf to adab4d4 Compare October 18, 2026 07:45
290ab76 Merge bitcoin#27482: kernel: chainparams updates for 25.x (DCG-Claude)
bf546a2 Merge bitcoin#26140: refactor: Move CNodeState members guarded by g_msgproc_mutex to Peer (DCG-Claude)

Pull request description:

  <!-- backportsys v1 -->
  > 🔎 **Needs a careful look.** Commits are ordered by how much they were adapted: 🔴 heavy first, then 🟡 light (at most two invented lines, or up to six lines counting edits of upstream functional-test or doc lines), then 🟢 faithful (identical to upstream apart from context).

  ## Issue being fixed or feature implemented

  Backports 2 Bitcoin Core v0.26 pull request(s) that the Dash queue selected, including any discovered prerequisites: bitcoin#26140, bitcoin#27482.

  ## What was done?

  | upstream | commit | review | gates | notes |
  |---|---|---|---|---|
  | bitcoin#26140 | `bf546a200b` | 🔴 heavy | build:pass mech:warn pick:pass tests:pass tree:pass verify:pass | Backports bitcoin#26140 in full: it moves the unconnecting-headers counter, headers sync timeout, prefers-head |
  | bitcoin#27482 | `290ab768cb` | 🔴 heavy | build:pass ci_fork:pass ci_upstream:warn mech:warn pick:pass tests:pass tree:pass verify:pass | Backports the doc/comment path updates from src/chainparams.cpp to src/kernel/chainparams.cpp, including both  |

  Each commit keeps the upstream subject (`partial Merge …` only where a hunk is deferred to a prerequisite still to be backported, named in the commit body; a hunk Dash intentionally never wants is recorded in the commit body or the reviewer's note above and does not make the backport partial). Conflicts were resolved commit by commit; commits that needed no resolution were cherry-picked unchanged.

  ## How Has This Been Tested?

  Recorded per commit, at that commit's own sha, not once for the branch:

  - built at each commit, reusing the worktree build cache
  - the test suites touched by each diff run at that commit, where the diff selected one
  - a mechanical diff-of-diffs against the upstream patch (every upstream hunk present, no added line without an upstream counterpart, no Dash-specific line dropped)
  - an independent verification pass on 2 commit(s) that needed adaptation
  - CI green on the fork gate PR thepastaclaw#101 at `290ab768cb`, which is this branch's head

  Gates that did not come back clean — please weigh these:

  - bitcoin#26140 `mech`: **warn** — 13 invented line(s); 1 partial/prereq/low-risk finding(s)
  - bitcoin#27482 `mech`: **warn** — 2 invented line(s); 2 partial/prereq/low-risk finding(s)

  ## Breaking Changes

  None beyond the upstream changes themselves.

  ## Checklist:

  _Left for the reviewer; backportsys does not tick boxes on its own behalf._

  - [ ] I have performed a self-review of my own code
  - [ ] I have made corresponding changes to the documentation
  - [ ] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

  ---

  ### Maintainer controls

  <!-- backportsys controls -->
  Tick a box and backportsys acts on it within a few minutes, then clears the box. For anything else — a hunk to drop, a resolution to redo, a question — just leave a review comment; nothing here needs a box.

  - [ ] 🔒 **Hands off** — stop every automated update to this branch <!-- bps:hands_off -->
  - [ ] 🔄 **Rebase** onto current `develop` <!-- bps:rebase -->
  - [ ] ❌ **Close** — abandon this batch and release its items <!-- bps:close -->

  <!-- /backportsys controls -->

  <!-- human notes below; the bot never edits this section -->

ACKs for top commit:
  knst:
    utACK 290ab76

Tree-SHA512: 2cb61a803d7a938e1761378bd0b3de615f1c6934c0e08f8186260f5902a7d5ae0e0909c29e08a005b948efe6153cca65be5881505ff53af9a6af8a8db3b678d7
@knst

knst commented Oct 18, 2026

Copy link
Copy Markdown
Owner Author

6m15

reverted only sanitizer's to secp

DCG-Claude and others added 10 commits October 18, 2026 16:00
…ere possible

aaaa3ae test: Use TestNode *_path properties where possible (MarcoFalke)
dddd899 test: Allow pathlib.Path as RPC argument via authproxy (MarcoFalke)
fa41614 scripted-diff: Use wallets_path and chain_path where possible (MarcoFalke)
fa493fa test: Use wallet_dir lambda in wallet_multiwallet test where possible (MarcoFalke)

Pull request description:

  It seems inconsistent, fragile and verbose to:

  * Call `get_datadir_path` to recreate the path that already exists as field in TestNode
  * Call `os.path.join` with the hardcoded chain name or `self.chain` to recreate the TestNode `chain_path` property
  * Sometimes even use the hardcoded node dir name (`"node0"`)

  Fix all issues by using the TestNode properties.

ACKs for top commit:
  willcl-ark:
    re-ACK aaaa3ae
  theStack:
    Code-review ACK aaaa3ae 🌊

Tree-SHA512: e4720278085beb8164e1fe6c1aa18f601558a9263494ce69a83764c1487007de63ebb51d1b1151862dc4d5b49ded6162a5c1553cd30ea1c28627d447db4d8e72

Dash adaptations:
- test/functional/test_framework/test_node.py: chain_path keeps Dash's get_chain_folder() (devnet folder naming), now built on the new datadir_path; imports serialization_fallback, Dash's name for EncodeDecimal
- test/functional/test_framework/util.py: no change, Dash already moved EncodeDecimal into authproxy as serialization_fallback; the authproxy pathlib.Path hunk applied to serialization_fallback
- test/functional/wallet_backup.py: the Dash-only rmtree of evodb, llmq and indexes also uses chain_path, alongside upstream's blocks and chainstate
- test/functional/wallet_hd.py: the Dash-only rmtree of evodb, llmq and indexes (both places) also uses chain_path, alongside upstream's blocks and chainstate
- test/functional/wallet_backwards_compatibility.py: keeps Dash's version < 160000 cutoff and has no v16 wallets dir line, because Dash's set of old nodes is master/v19/v17
- test/functional/wallet_upgradewallet.py: converts Dash's v18_2 and v16_1 wallet paths to wallets_path in place of upstream's v16_3/v15_2/splithd lines, which Dash's test does not have
- test/functional/wallet_descriptor.py: keeps Dash's explicit sqlite3 connect/close structure; only the wallet_db path moves to wallets_path
- test/functional/rpc_whitelist.py: keeps Dash's extra 'quorum sign' in never_allowed while moving the conf write into run_test with restart_node
- test/functional/rpc_blockchain.py: keeps Dash's log wording 'Test that getblock with invalid verbosity type...' while taking upstream's reordering and chain_path use
- test/functional/wallet_migration.py: keeps 'import os', which upstream drops, because Dash's own test_wallet_name_with_slashes still uses os.path

Not applicable to Dash (intentionally omitted):
- test/functional/feature_txindex_compatibility.py: Dash already removed this test (merge of bitcoin#28753) along with the legacy -txindex check; none of the listed Dash-only tests has the same os.path.join(datadir, chain) pattern
- test/functional/wallet_inactive_hdchains.py: test comes from bitcoin#23304 (inactive HD chains via sethdseed); Dash's HD wallet has no sethdseed or inactive-chain code
- test/functional/feature_fee_estimation.py: edits the 'fresh estimation / RBF estimates' block from bitcoin#22539; Dash has no RBF and lacks that block; Dash already uses chain_path elsewhere
fafe43c scripted-diff: Use blocks_path where possible (MarcoFalke)
fa060c1 test: Add blocks_path property to TestNode (MarcoFalke)
faba4fc test: Drop 22.x node from TxindexCompatibilityTest (MarcoFalke)
fa7f65b test: Use clean chain in MempoolCompatibilityTest (MarcoFalke)

Pull request description:

  The node in this test was never really needed, because the compatibility tests shouldn't be used to test previous releases. (The test suite of the previous release itself should be used for that). So remove it.

  Also, other test changes. (See individual commits)

ACKs for top commit:
  theStack:
    Code-review ACK fafe43c

Tree-SHA512: 289f54695bf5310663ab38ebf1aa457f53d0aafae56e6657be0e75bf96b303165bad417dc7eaf4c40f0639aa92ce139e5bacb318a2eabab1f8e23a811cabe0cc

Dash adaptations:
- test/functional/mempool_compatibility.py: drops Dash's shutil.rmtree of node0's cached regtest dir (and the os/shutil imports); setup_clean_chain=True means no cached chain dir exists, so rmtree would raise
- test/functional/mempool_compatibility.py: does not add upstream's COINBASE_MATURITY import context; Dash's version generates 100 blocks literally and never imported it
- test/functional/feature_txindex_compatibility.py: upstream hunk not applied; Dash already deleted this test via bitcoin#28753, so the file stays deleted

Not applicable to Dash (intentionally omitted):
- test/functional/feature_txindex_compatibility.py: Dash already removed this test in merge bitcoin#28753 (upstream's later removal of it), so there is no file to edit
… is only supported on regtest chain

ee5a036 test: ensure acceptstalefeeestimates is supported only on regtest chain (ismaelsadeeq)
22d5d4b tx fees, policy: doc: update and delete unnecessary comment (ismaelsadeeq)

Pull request description:

  This PR Follow up comments from [bitcoin#27622](bitcoin#27622)

  It test that the new `regtest-only` option `acceptstalefeeestimates` is not supported on [main, signet and test chains](https://github.com/bitcoin/bitcoin/pull/27622/files#r1235218268), removes an unnecessary [comment](https://github.com/bitcoin/bitcoin/pull/27622/files#r1235204323), and update fee estimator  `MAXFILEAGE` [description comment](https://github.com/bitcoin/bitcoin/pull/27622/files#r1233887314).

ACKs for top commit:
  jonatack:
    ACK ee5a036
  glozow:
    utACK ee5a036

Tree-SHA512: 4755f25b08db62f37614ea768272b12580ee0d481fb7fa339379901a6132c66828777c6747d3fe67490ceace3a6ff248bf13bdf65720f6e5ba8642eb762acd3c
…8157

d2600bc Merge bitcoin#28157: test doc: tests `acceptstalefeeestimates` option is only supported on regtest chain (DCG-Claude)
5bd92ee Merge bitcoin#28070: test: Drop 22.x node from TxindexCompatibilityTest (DCG-Claude)
0976752 Merge bitcoin#27884: test: Use TestNode datadir_path or chain_path where possible (DCG-Claude)

Pull request description:

  <!-- backportsys v1 -->
  > 🔎 **Needs a careful look.** Commits are ordered by how much they were adapted: 🔴 heavy first, then 🟡 light (at most two invented lines, or up to six lines counting edits of upstream functional-test or doc lines), then 🟢 faithful (identical to upstream apart from context).

  ## Issue being fixed or feature implemented

  Backports 3 Bitcoin Core v0.26 pull request(s) that the Dash queue selected, including any discovered prerequisites: bitcoin#27884, bitcoin#28070, bitcoin#28157.

  ## What was done?

  | upstream | commit | review | gates | notes |
  |---|---|---|---|---|
  | bitcoin#27884 | `61144e66a8` | 🔴 heavy | build:pass mech:warn pick:pass tests:pass tree:pass verify:pass | Backports bitcoin#27884: adds the TestNode datadir_path/wallets_path properties and lets authproxy serialize p |
  | bitcoin#28070 | `c7fcabbc03` | 🔴 heavy | build:pass mech:warn pick:pass tests:pass tree:pass verify:pass | Backports bitcoin#28070, which adds the TestNode.blocks_path helper, switches tests to it with upstream's scri |
  | bitcoin#28157 | `4c719b396c` | 🔴 heavy | build:pass ci_fork:pass ci_upstream:pass mech:warn pick:pass tests:pass tree:pass verify:pass | Backports bitcoin#28157: fee-estimator doc comment cleanups plus a functional test confirming -acceptstalefeee |

  Each commit keeps the upstream subject (`partial Merge …` only where a hunk is deferred to a prerequisite still to be backported, named in the commit body; a hunk Dash intentionally never wants is recorded in the commit body or the reviewer's note above and does not make the backport partial). Conflicts were resolved commit by commit; commits that needed no resolution were cherry-picked unchanged.

  ## How Has This Been Tested?

  Recorded per commit, at that commit's own sha, not once for the branch:

  - built at each commit, reusing the worktree build cache
  - the test suites touched by each diff run at that commit, where the diff selected one
  - a mechanical diff-of-diffs against the upstream patch (every upstream hunk present, no added line without an upstream counterpart, no Dash-specific line dropped)
  - an independent verification pass on 3 commit(s) that needed adaptation
  - CI green on the fork gate PR thepastaclaw#95 at `4c719b396c`, which is this branch's head

  Gates that did not come back clean — please weigh these:

  - bitcoin#27884 `mech`: **warn** — 16 invented line(s); 6 Dash-feature line(s) removed; 11 partial/prereq/low-risk finding(s)
  - bitcoin#28070 `mech`: **warn** — 1 partial/prereq/low-risk finding(s)
  - bitcoin#28157 `mech`: **warn** — 7 invented line(s); 1 edited line(s)

  ## Breaking Changes

  None beyond the upstream changes themselves.

  ## Checklist:

  _Left for the reviewer; backportsys does not tick boxes on its own behalf._

  - [ ] I have performed a self-review of my own code
  - [ ] I have made corresponding changes to the documentation
  - [ ] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

  ---

  ### Maintainer controls

  <!-- backportsys controls -->
  Tick a box and backportsys acts on it within a few minutes, then clears the box. For anything else — a hunk to drop, a resolution to redo, a question — just leave a review comment; nothing here needs a box.

  - [ ] 🔒 **Hands off** — stop every automated update to this branch <!-- bps:hands_off -->
  - [ ] 🔄 **Rebase** onto current `develop` <!-- bps:rebase -->
  - [ ] ❌ **Close** — abandon this batch and release its items <!-- bps:close -->

  <!-- /backportsys controls -->

  <!-- human notes below; the bot never edits this section -->

Top commit has no ACKs.

Tree-SHA512: fc4896bca24c4cd827fd39c95d72bc2dc4e9043660ab714927c84a19f2279db3fa61d04f901c1c7c15f3c3aec758cc29a9eb0545de6870623b3bb9345bf7b608
d799ea2 doc: rewrite explanation for -par= (fanquake)

Pull request description:

  The negative bound for script threads comes from the machine which generates the man pages, so may only be correct for that machine. Any other placeholder value will also be wrong for some machines. Fix this be removing the value. This also fixes help2man incorrectly bolding the value, as if it were a paramater.

  Closes bitcoin#28850.

ACKs for top commit:
  maflcko:
    lgtm ACK d799ea2
  theStack:
    ACK d799ea2

Tree-SHA512: 2eec0086faf4cc64bbf46b22949662f84d8546d2322c3d507fc44a4e1f64d228a2901af4fa4535c0771e3e14600be8308fc5dbd407b66ae6ae4f8878d8372c0a

Dash adaptations:
- src/init.cpp: -par help text was already changed by Dash (4e5cc4b, no -GetNumCores() bound, 'max: %d'); swapped in upstream's '0 = auto, up to %d, ...' wording on that line
- src/init.cpp: applied the same wording to Dash-only -parbls, which shares -par's help-text format and was changed alongside it in 4e5cc4b
- src/init.cpp: kept Dash's -minimumchainwork line (devnet instead of signet) from the conflict hunk unchanged
faa769d Fix bugprone-lambda-function-name errors (MarcoFalke)

Pull request description:

  Inside a lambda, `__func__` will evaluate to something like `"operator()"`. Fix this by either removing it, or by using the real name.

  https://clang.llvm.org/extra/clang-tidy/checks/bugprone/lambda-function-name.html

ACKs for top commit:
  TheCharlatan:
    ACK faa769d
  darosior:
    utACK faa769d

Tree-SHA512: 0b562bd4ebd7f46ca3ebabeee67851ad30bd522fa57e5010e833b163664e51f5df645ff9ca35d22c3479fb27d9267d4e5d0d417d42729bf3ccf80d7944970e4e

Dash adaptations:
- src/validation.cpp: takes upstream's new rename-error LogPrintf but keeps Dash's AbortNode(); Dash lacks the GetNotifications().fatalError() kernel-notifications refactor
- src/rpc/masternode.cpp: masternode_payments RPC lambda logs self.m_name instead of __func__, same fix upstream applied to getmempoolancestors
- src/evo/specialtxman.cpp: drops __func__ from the ForEachMN lambda log in FinishListForBlock and writes the function name as literal text
- src/instantsend/signing.cpp: drops __func__ from the log_refusal lambda in CheckCanLockAssetUnlock and writes the function name as literal text
- src/net_processing.cpp: drops __func__ from the ISDLOCK-skip log inside both RelayInvFiltered ForEachNode lambdas and writes the function name as literal text
- src/qt/guiutil.cpp: drops __func__ from the loadStyleSheet loadFiles lambda's exception message and writes the function name as literal text
- src/qt/guiutil_font.cpp: drops __func__ from the loadFonts importFont lambda's qDebug message and writes the function name as literal text
…write followups

9b3da70 [test] DisconnectedBlockTransactions::DynamicMemoryUsage (glozow)
b2d0447 bugfix: correct DisconnectedBlockTransactions memory usage (stickies-v)
f4254e2 assume duplicate transactions are not added to `iters_by_txid` (ismaelsadeeq)
29eb219 move only: move implementation code to disconnected_transactions.cpp (ismaelsadeeq)
81dfedd refactor: update `MAX_DISCONNECTED_TX_POOL` from kb to bytes (ismaelsadeeq)

Pull request description:

  This PR is a follow-up to fix review comments and a bugfix from bitcoin#28385

  The PR

  - Updated `DisconnectedBlockTransactions`'s `MAX_DISCONNECTED_TX_POOL` from kb to bytes.
  - Moved `DisconnectedBlockTransactions` implementation code to `kernel/disconnected_transactions.cpp`.
  - `AddTransactionsFromBlock` now assume duplicate transactions are not passed by asserting after inserting each transaction to `iters_by_txid`.
  - Included a Bug fix: In the current master we are underestimating the memory usage of `DisconnectedBlockTransactions`.

      * When adding and subtracting `cachedInnerUsage` we call `RecursiveDynamicUsage` with `CTransaction` which invokes this [`RecursiveDynamicUsage(const CTransaction& tx)`](https://github.com/bitcoin/bitcoin/blob/6e721c923c87abdb8d99674093d08d8c17bc52c2/src/core_memusage.h#L32) version of `RecursiveDynamicUsage`, the output of that call only account for the memory usage of the inputs and outputs of the `CTransaction`, this omits the memory usage of the `CTransaction` object and the control block.
      * This PR fixes this bug by calling `RecursiveDynamicUsage` with `CTransactionRef` when adding and subtracting `cachedInnerUsage` which invokes [`RecursiveDynamicUsage(const std::shared_ptr<X>& p)`](https://github.com/bitcoin/bitcoin/blob/6e721c923c87abdb8d99674093d08d8c17bc52c2/src/core_memusage.h#L67) version of `RecursiveDynamicUsage` the output of the calculation accounts for the` CTransaction` object, the control blocks, inputs and outputs memory usage.
      * see  [comment ](bitcoin#28385 (comment))
  - Added test for DisconnectedBlockTransactions memory limit.

ACKs for top commit:
  stickies-v:
    ACK 9b3da70 - nice work!
  BrandonOdiwuor:
    re ACK 9b3da70
  glozow:
    ACK 9b3da70

Tree-SHA512: 69b9595d09f4d0209038f97081d790cea92ccf63efb94e9e372749979fcbe527f7f17a8e454720cedd12021be0c8e11cf99874625d3dafd9ec602b12dbeb4098

Dash adaptations:
- src/Makefile.test.include: test/disconnected_transactions.cpp added after descriptor_tests.cpp, keeping Dash's dynamic_activation_thresholds and evo_* test entries
- src/validation.cpp: Dash-only Chainstate::MarkConflictingBlock now uses MAX_DISCONNECTED_TX_POOL_BYTES instead of MAX_DISCONNECTED_TX_POOL_SIZE * 1000 (constant renamed upstream)
- src/test/validation_chainstatemanager_tests.cpp: Dash-only chainstatemanager_evodb_snapshot_only_flush_restart test now uses MAX_DISCONNECTED_TX_POOL_BYTES (constant renamed upstream)
…n order consistent

c1144f0 tests: Reset node context members on ~BasicTestingSetup (TheCharlatan)
9759af1 shutdown: Destroy kernel last (TheCharlatan)

Pull request description:

  The destruction/resetting of node context members in the tests should roughly follow the behavior of the `Shutdown` function in `init.cpp`.

  This was originally requested by MarcoFalke in this [comment](bitcoin#25065 (comment)) in response to the [original pull request](bitcoin#25065) introducing the `kernel::Context`.

ACKs for top commit:
  maflcko:
    ACK c1144f0 🗣
  achow101:
    ACK c1144f0
  ryanofsky:
    Code review ACK c1144f0. No code changes since last review, just updated commits and descriptions

Tree-SHA512: 819bb85ff82a5c6c60e429674d5684f3692fe9062500d00a87b361cc59e6bda145be21b5a4466dee6791faed910cbde4d26baab325bf6daa1813af13a63588ff

Dash adaptations:
- src/init.cpp: moved node.kernel.reset() after node.scheduler.reset() inside Dash's split Shutdown() (part 2), keeping PrepareShutdown/RestartRequested logic intact
- src/test/util/setup_common.cpp: reordered ChainTestingSetup dtor to mempool, fee_estimator, chainman, scheduler after Dash's mn_sync reset; Dash already reset fee_estimator and scheduler last
…8530, bitcoin#28224

2031251 Merge bitcoin#28224: shutdown: Destroy kernel last, make test shutdown order consistent (DCG-Claude)
204d8a8 Merge bitcoin#28530: tests, bug fix: DisconnectedBlockTransactions rewrite followups (DCG-Claude)
d6d11e7 Merge bitcoin#28737: doc: Fix bugprone-lambda-function-name errors (DCG-Claude)
bddd9bb Merge bitcoin#28858: doc: rewrite explanation for `-par=` (DCG-Claude)

Pull request description:

  <!-- backportsys v1 -->
  > 🔎 **Needs a careful look.** Commits are ordered by how much they were adapted: 🔴 heavy first, then 🟡 light (at most two invented lines, or up to six lines counting edits of upstream functional-test or doc lines), then 🟢 faithful (identical to upstream apart from context).

  ## Issue being fixed or feature implemented

  Backports 4 Bitcoin Core v0.27 pull request(s) that the Dash queue selected, including any discovered prerequisites: bitcoin#28858, bitcoin#28737, bitcoin#28530, bitcoin#28224.

  ## What was done?

  | upstream | commit | review | gates | notes |
  |---|---|---|---|---|
  | bitcoin#28858 | `bddd9bb8cc` | 🔴 heavy | build:pass mech:warn pick:pass tests:warn tree:pass verify:pass | Backports bitcoin#28858 by changing the -par help text to upstream's '0 = auto, up to N, <0 = leave that many  |
  | bitcoin#28737 | `d6d11e7e11` | 🔴 heavy | build:pass mech:warn pick:pass tests:pass tree:pass verify:pass | Every upstream hunk is present: the bugprone-lambda-function-name clang-tidy check, NOLINT guards around the c |
  | bitcoin#28224 | `2031251127` | 🔴 heavy | build:pass ci_fork:pass ci_upstream:pass mech:warn pick:pass tests:warn tree:pass verify:pass | Backports bitcoin#28224: the kernel context is now destroyed last in Shutdown() and reset in ~BasicTestingSetu |
  | bitcoin#28530 | `204d8a84a0` | 🟢 faithful | build:pass mech:pass pick:pass tests:pass tree:pass verify:pass | This backports bitcoin#28530: the DisconnectedBlockTransactions rewrite followups, which move the implementati |

  Each commit keeps the upstream subject (`partial Merge …` only where a hunk is deferred to a prerequisite still to be backported, named in the commit body; a hunk Dash intentionally never wants is recorded in the commit body or the reviewer's note above and does not make the backport partial). Conflicts were resolved commit by commit; commits that needed no resolution were cherry-picked unchanged.

  ## How Has This Been Tested?

  Recorded per commit, at that commit's own sha, not once for the branch:

  - built at each commit, reusing the worktree build cache
  - the test suites touched by each diff run at that commit, where the diff selected one
  - a mechanical diff-of-diffs against the upstream patch (every upstream hunk present, no added line without an upstream counterpart, no Dash-specific line dropped)
  - an independent verification pass on 4 commit(s) that needed adaptation
  - CI green on the fork gate PR thepastaclaw#115 at `2031251127`, which is this branch's head

  Gates that did not come back clean — please weigh these:

  - bitcoin#28858 `tests`: **warn** — no test selected for this diff
  - bitcoin#28858 `mech`: **warn** — 1 invented line(s); 1 partial/prereq/low-risk finding(s)
  - bitcoin#28737 `mech`: **warn** — 9 invented line(s); 2 Dash-feature line(s) removed
  - bitcoin#28224 `tests`: **warn** — no test selected for this diff
  - bitcoin#28224 `mech`: **warn** — 1 partial/prereq/low-risk finding(s)

  ## Breaking Changes

  None beyond the upstream changes themselves.

  ## Checklist:

  _Left for the reviewer; backportsys does not tick boxes on its own behalf._

  - [ ] I have performed a self-review of my own code
  - [ ] I have made corresponding changes to the documentation
  - [ ] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

  ---

  ### Maintainer controls

  <!-- backportsys controls -->
  Tick a box and backportsys acts on it within a few minutes, then clears the box. For anything else — a hunk to drop, a resolution to redo, a question — just leave a review comment; nothing here needs a box.

  - [ ] 🔒 **Hands off** — stop every automated update to this branch <!-- bps:hands_off -->
  - [ ] 🔄 **Rebase** onto current `develop` <!-- bps:rebase -->
  - [ ] ❌ **Close** — abandon this batch and release its items <!-- bps:close -->

  <!-- /backportsys controls -->

  <!-- human notes below; the bot never edits this section -->

ACKs for top commit:
  knst:
    utACK 2031251

Tree-SHA512: 0f42dc8c0ba8d9e09ac1d90fc8d8aa6ff10d0411f3156f703eaf3c68085f88cef45ff9797bb10e181c27996c61255911f1a4de8f83fcf498b35ea118af77c384
It is part of bitcoin#28875, backported in dashpay#7503.
Since that merge every sanitizer job compiles libsecp256k1 with -fsanitize, so the TSan job instruments all of secp256k1's ECDSA code.
It reverts _only_ libsecp256k1 sanitizer, other C code in the repo is still profiled.

The UBSan suppression lines that came with the backport are kept: they are harmless without the instrumentation.
@knst
knst force-pushed the tsan-regression-perf branch from adab4d4 to ed00641 Compare October 21, 2026 06:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants