Skip to content

Merged elements to 29.4#1574

Open
tomt1664 wants to merge 406 commits into
ElementsProject:elements-29.xfrom
tomt1664:merged-elements-29.x
Open

Merged elements to 29.4#1574
tomt1664 wants to merge 406 commits into
ElementsProject:elements-29.xfrom
tomt1664:merged-elements-29.x

Conversation

@tomt1664

Copy link
Copy Markdown
Member

Upstream merges from Bitcoin 29.x to 29.4.

Additional fixes for CI:

1. Merge setup & CI infrastructure

  • Merge script sanity check — confirmed all 42 upstream backport PRs (e9e6825b8c..3fc0865963) were merge commits with no stray non-merge first-parent commits; merge-prs.sh pointed at elements-29.x.
  • CI migration, Cirrus CI → GitHub Actions — resolved Docker/host environment conflicts, macOS path divergence (~/.elements vs ~/Library/Application Support/Elements), subtree lint failures (secp256k1, minisketch), test-each-commit FETCH_DEPTH issues, Windows MSVC job config.
  • ci/test/03_test_script.sh — removed dead autotools-era code paths (./autogen.sh, ./configure), rewrote to match upstream's CMake-only flow.
  • doc/man generation — resolved merge conflicts from upstream's doc/man/bitcoin-*.1 vs Elements' elements-*.1 naming; regenerate via contrib/devtools/gen-manpages.py at release time instead of merging.
  • contrib/devtools/check-deps.sh — added ~30 missing SUPPRESS[...] entries recording Elements-specific cross-module dependencies (confidential_validation, pegins, mainchainrpc, pak, dynafed, block_proof, versionbits) that upstream's dependency-graph checker had no knowledge of.
  • CentOS/depends/GUI job — "No space left on device" — infrastructure issue (disk/ccache exhaustion on the runner), not a code bug; not something fixable via source changes.
  • --exclude feature_dbcrashfeature_dbcrash.py is commented out of Elements' own test_runner.py test list (too disk/time-intensive for GHA); excluding an already-absent test is now a hard error in test_runner.py, not a no-op. Removed from the exclude list in ci/test/00_setup_env_native_previous_releases.sh.
  • Same file — --extended,feature_fee_estimation,... argparse corruption

2. RPATH — "libsecp256k1.so.6: cannot open shared object file" bug

top-level CMakeLists.txt sets CMAKE_SKIP_BUILD_RPATH TRUE globally (deliberate upstream policy, tied to Guix reproducible builds). This is invisible under a normal (static-secp256k1) build, but any build enabling libbitcoinkernel forces secp256k1 to build as a shared library — and every other executable target then fails at runtime unless it gets the same opt-out.

  • elements-tx, elements-util (src/CMakeLists.txt)
  • test_elements (src/test/CMakeLists.txt)
  • bench_bitcoin (src/bench/CMakeLists.txt)
  • elements-qt (src/qt/CMakeLists.txt)
  • test_elements-qt (src/qt/test/CMakeLists.txt)
  • elementsd (src/CMakeLists.txt)
  • elements-cli (src/CMakeLists.txt)

3. Code/sanitizer bugs

  • CTxOut 2-arg vs 3-arg constructor — ported Bitcoin Core test/IPC code used upstream's CTxOut(amount, script); fixed to Elements' CTxOut(CAsset(), amount, script) in transaction_tests.cpp, test/ipc_test.cpp.
  • checkqueue.h::Loop() ownership bug — failing/skipped-after-failure checks never deleted in Elements' pointer-based (T*) CCheckQueue; real leak reachable from production validation code.
  • spend.cppvtxinwit/vin invariant violationvtxinwit.emplace_back() only ran inside a conditional branch; moved to run unconditionally alongside every vin.emplace_back().
  • wallet.cpp::SignPSBT — missing tx.witness.vtxinwit.resize(tx.vin.size()) (had the vtxoutwit equivalent but not the vtxinwit one); found via gdb backtrace.
  • coins.h::AddFlagsAssume(flags & (DIRTY|FRESH)) didn't account for Elements' third PEGIN flag; fixed to Assume(flags & (DIRTY|FRESH|PEGIN)).
  • kernel/chainparams.h::CChainParams — missing virtual destructor; new-delete-type-mismatch deleting a CCustomParams through unique_ptr<const CChainParams>.
  • validation.cpp::CheckInputScripts synchronous path — leaked the heap-allocated CScriptCheck on every call (missing delete).
  • test/fuzz/deserialize.cpp — generic round-trip fuzz check inappropriately applied to Coin, which asserts !IsSpent() on serialize; fuzzed bytes trivially violate this.
  • test/fuzz/package_eval.cpp — duplicate tx_mut.vin.push_back(in) — an earlier fix (adding vtxinwit.emplace_back() alongside the push_back) was applied additively instead of replacing the original line, leaving every selected input pushed into vin twice. Traced via reproducible fuzz crash + gdb + targeted fprintf instrumentation to a outpoints.insert(in.prevout).second assertion failure; root-caused to the duplicate push, not (as initially suspected) OutpointsUpdater or cross-iteration state.
  • kernel/chainparams.cpp::CTestNet4Params — entire Elements-specific initialization block (enforce_pak, accept_unlimited_issuances, multi_data_permitted, accept_discount_ct, create_discount_ct, pegin_subsidy, pegin_minimum, anyonecanspend_aremine, consensus.has_parent_chain, consensus.genesis_subsidy, consensus.connect_genesis_outputs, consensus.subsidy_asset, g_signed_blocks) missing entirely — testnet4 is a brand-new upstream 29.x chain type, and the new class was adapted from upstream's version (which has no concept of any of these fields) without ever adding Elements' equivalent block. Caught independently by both UBSan (invalid-bool-load) and MSan (use-of-uninitialized-value) in two separate CI jobs.
  • validation.cpp::GuessVerificationProgress — unguarded division (pindex->nHeight / (pindex->nHeight + moreBlocksExpected)) with a denominator that can hit exactly zero when a block's timestamp is far enough ahead of "now" (e.g. during feature_taproot.py's test-vector generation). Added a guard clause returning 1.0 when the extrapolation is degenerate.
  • wallet/rpc/spend.cpppsbtbumpfee not-mine-input pathFillPSBT(...) call omitted Elements' imbalance_ok parameter (defaults to false), causing a legitimate CT-balance-check failure for external/not-owned inputs to trip a CHECK_NONFATAL(!err) that assumed the call could never fail. Every other Elements-specific FillPSBT call site in the same file already passes /*imbalance_ok=*/true; this one didn't. Root-caused by diffing against upstream (confirmed the CHECK_NONFATAL pattern itself is unmodified) and against Elements' own FillPSBT declaration.

4. Type/size mismatches

Several issues traced to CoinsCachePair/Coin/CTxOut being substantially larger than upstream's equivalents (confidential asset/value/nonce commitments), breaking constants and assumptions tuned for upstream's smaller structs:

  • CTxMemPoolEntry::discountSizeWithAncestors — declared uint64_t while its three sibling ancestor-state accumulators are int64_t; the odd type caused a UBSan implicit-integer-sign-change when a negative delta was added during removal. Retyped to int64_t to match siblings.
    • Follow-up compile fix: GetDiscountTxSize() returns size_t; brace-init into the now-int64_t member needed an explicit static_cast<int64_t>(...).
    • Confirmed the six external call sites (node/miner.h, node/miner.cpp, txmempool.h) did not need changes — they only ever read the post-assert, always-non-negative value into uint64_t, which is a value-preserving conversion UBSan doesn't flag.
  • coins.h::CCoinsMapMemoryResourcePoolResource's generic default constructor hardcodes a 256 KiB chunk, sized for upstream's smaller CoinsCachePair; no longer big enough to hold 1000 of Elements' larger nodes without overflowing into a second chunk. Fixed by passing an explicit chunk_size_bytes scaled to sizeof(CoinsCachePair).
    • Follow-up: validation_flush_tests.cpp's getcoinscachesizestate test hardcoded MAX_COINS_CACHE_BYTES = 262144 + 512 based on the (now-stale) "PoolResource defaults to 256 KiB" assumption; updated to compute the same way as the fixed default.
  • 32-bit ARM validation_flush_tests.cpp — the test's own "unsupported architecture" fallback path hardcoded 1000 coins as "obviously enough" to cross the CRITICAL cache-size threshold; doesn't hold on 32-bit ARM where COIN_SIZE is smaller (32-bit pointers). Fixed by computing the coin count from MAX_COINS_CACHE_BYTES / COIN_SIZE instead of a fixed constant.

5. Build configuration / CMake

  • src/CMakeLists.txt — simplicity subtree warnings-Wconditional-uninitialized/-Wimplicit-fallthrough (Clang) and -Wtype-limits (GCC/mingw) in vendored dag.c/rsort.c; suppression was gated if(APPLE) only, extended to all Clang toolchains plus a GCC-specific suppression.
  • src/qt/CMakeLists.txtWITH_MULTIPROCESS block called import_plugins(bitcoin-gui), never renamed to Elements' elements-gui target.
  • src/kernel/CMakeLists.txtlibbitcoinkernel missing several source files (addresstype.cpp, key.cpp, script/miniscript.cpp, common/{config,settings}.cpp, chainparamsbase.cpp, mainchainrpc.cpp) and libevent linkage, needed because pegins.cpp (pulled into the kernel lib) calls IsConfirmedBitcoinBlock unconditionally.
  • Windows cross-compilation — secp256k1 __imp_ linker symbols (fixed via PUBLIC CMake linkage); pegins.cpp/confidential_validation.cpp needed to move from bitcoin_node to bitcoin_common to satisfy the dependency graph; NSIS installer asset staging.

6. Toolchain / fuzz harness

  • test/fuzz/simplicity_tx.cpp — missing SeedRandomStateForTest(SeedRand::ZEROS); as the first line of the FUZZ_TARGET body, required by the CheckGlobals reproducibility guard whenever a target touches the global RNG state. Also needed the corresponding #include <test/util/random.h>

7. Python functional-test gaps

  • test/functional/p2p_headers_sync_with_minchainwork.py and p2p_headers_presync.py — both call self.nodes[0].createwallet(...) unconditionally (an Elements-specific addition needed because Elements' generatetoaddress-based mining approach requires a wallet address, unlike upstream's self.generate()), but neither had the standard self.skip_if_no_wallet() guard that every other wallet-touching test in the suite already has, causing Method not found (-32601) failures in no-wallet CI builds. Both fixed by adding the guard as the first line of run_test(). Flagged for a broader sweep (grep -rL skip_if_no_wallet against files that call wallet methods) since both were adapted from the same original source and the same gap may recur elsewhere.

8. Static analysis / clang-tidy

Batch of ~9 findings from a -warnings-as-errors clang-tidy run,

  • rpc/util.cpp — two hand-written trivial default constructors → = default.
  • qt/networkstyle.cpp — legitimate bounded single-level recursion (instantiate(), guarded by an assert immediately before the recursive call) → NOLINT(misc-no-recursion) rather than a rewrite.
  • wallet/coinselection.cpp — two occurrences of auto input_set/set = result.GetInputSet(); copy-constructed but only used by reference (.find()/.end() need to operate on the same instance, so the copy can't simply be inlined away either) → const auto&. One occurrence was caught by CI in a second pass after the first fix (§10 recurrence).
  • assetsdir.cpp — range-for copying each string → const std::string&.
  • primitives/bitcoin/merkleblock.cpp — three inherently-recursive Merkle-tree traversal functions, straight from unmodified upstream → NOLINT(misc-no-recursion) rather than rewriting a correctness-sensitive algorithm.
  • validation.cppemplace_back in a loop of known size without reserve().
  • bitcoin-tx.cpp — three occurrences of the same unnecessary-copy pattern as coinselection.cpp.
  • test/blind_tests.cpppush_back in a fixed 6-iteration loop without reserve().

glozow and others added 30 commits August 20, 2025 10:19
…t/kvB

Let's say an attacker wants to use/exhaust the network's bandwidth, and
has the choice between renting resources from a commercial provider and
getting the network to "spam" itself it by sending unconfirmed
transactions. We'd like the latter to be more expensive than the former.

The bandwidth for relaying a transaction across the network is roughly
its serialized size (plus relay overhead) x number of nodes. A 1000vB
transaction is 1000-4000B serialized. With 100k nodes, that's 0.1-0.4GB
If the going rate for commercial services is 10c/GB, that's like 1-4c per kvB
of transaction data, so a 1000vB transaction should pay at least $0.04.

At a price of 120k USD/BTC, 100sat is about $0.12. This price allows us
to tolerate a large decrease in the conversion rate or increase in the
number of nodes.

Github-Pull: #33106
Rebased-From: 6da5de5
Release notes are from 18720bc
…for rc2

0034dcf [doc] man pages for 29.1rc2 (glozow)
eb1574a [build] bump version to 29.1rc2 (glozow)
f9f1ca5 [doc] update release notes (glozow)
9dd7efc [policy] lower default minrelaytxfee and incrementalrelayfee to 100sat/kvB (glozow)
bbdab3e [prep/test] make wallet_fundrawtransaction's minrelaytxfee assumption explicit (glozow)
da30ca0 [prep/util] help MockMempoolMinFee handle more precise feerates (glozow)
a0ae3fc [prep/test] replace magic number 1000 with respective feerate vars (glozow)
1c1970f [miner] lower default -blockmintxfee to 1sat/kvB (glozow)
3a7e093 [doc] assert that default min relay feerate and incremental are the same (glozow)
567c3ee [test] explicitly check default -minrelaytxfee and -incrementalrelayfee (glozow)
6b5396c [test] RBF rule 4 for various incrementalrelayfee settings (glozow)
03da7af [test] check bypass of minrelay for various minrelaytxfee settings (glozow)
4e3cfa6 [test] check miner doesn't select 0fee transactions (glozow)

Pull request description:

  Backports #33106 and includes final changes for 29.1rc2. Based on current network conditions (in which nodes rejecting 0.1-1sat/vB are missing many transactions), it is recommended to change these policy settings.

  I did not include #32750 because it causes #33177 and I don't foresee any problems; it was just a nice to have.
  For reviewers: the backport is unclean but fairly straightforward. I just had to adapt a test that is no longer in master (#32973) and include `-datacarriersize` in order to pad transaction size (#32406).

ACKs for top commit:
  dergoegge:
    utACK 0034dcf
  marcofleon:
    ACK  0034dcf
  murchandamus:
    crACK 0034dcf
  brunoerg:
    crACK 0034dcf

Tree-SHA512: 1b7540ac3fec5b15cf36926dbf633054f14549d76aa445a2bf042b5667e8637db4f9c21c869af25a0c3f8c7cca6c585d17896d2f7e95a6264c1ff59817446694
…1rc2

65dc198 doc: update example bitcoin conf for 29.1rc2 (fanquake)

Pull request description:

  Followup to #33226.

ACKs for top commit:
  dergoegge:
    ACK 65dc198
  willcl-ark:
    ACK 65dc198

Tree-SHA512: b2924783dd98890bd031dbca8c9c126cd3ab45c3cc8d2f14dd5b5f940fcc7061f3d1f73e2d36482afceaae786f3087b59baab98db0f10bc0d19e3f016f52851a
The committed state of an index should never
be ahead of the flushed chainstate. Otherwise, in the case
of an unclean shutdown, the blocks necessary to revert
from the prematurely committed state would not be
available, which would corrupt the coinstatsindex in particular.
Instead, the index state will be committed with the next
ChainStateFlushed notification.

Github-Pull: #33212
Rebased-From: 01b95ac
This test fails without the previous commit.

Github-Pull: #33212
Rebased-From: a602f6f
fcac802 test: index with an unclean restart after a reorg (Martin Zumsande)
16b1710 index: don't commit state in BaseIndex::Rewind (Martin Zumsande)

Pull request description:

  Backports #33212 to 29.x

ACKs for top commit:
  achow101:
    ACK fcac802
  stickies-v:
    ACK fcac802
  mzumsande:
    Code Review ACK fcac802

Tree-SHA512: eeb9213f03bbb1d48c3ccb12121a6e475f436895d314b5171007e7e4ee457c74b312fa7f0d1808d6221dc22b192700a93ea21c4e9e04689da7dde7e1f79e9569
Remove it in feerate.

Fix it in the other places.

Github-Pull: #33236
Rebased-From: 966666d
Github-Pull: #33261
Rebased-From: 509ffea
084c95a doc: update manual pages for v29.1 (fanquake)
37d115c build: bump version to v29.1 final (fanquake)
b0d88bc doc: finalise release notes for 29.1 (fanquake)
99ab2e7 ci: return to using dash in CentOS job (fanquake)
6448ebb doc: Remove wrong and redundant doxygen tag (MarcoFalke)

Pull request description:

  Backports:
  * #33236
  * #33261

  Since `rc2`, #33212 was also backported in #33251.

ACKs for top commit:
  glozow:
    ACK 084c95a
  willcl-ark:
    ACK 084c95a

Tree-SHA512: 0698e5b2d12f7328bf5af8dbbd92b0049de401c0a4af27fda2209f9aab35d827c5ac65eb9268aa1fae241e3adf0d3dd89324bb288655ead8af2b5584aae1f6d2
Also, use update-alternatives to avoid having to manually specify
clang-${APT_LLVM_V} or llvm-symbolizer-${APT_LLVM_V} everywhere.

Github-Pull: #32999
Rebased-From: fad040a
Qt is disabled, as the build is now taking a very long time.

Github-Pull: #33099
Rebased-From: b09af2c
Github-Pull: #33099
Rebased-From: 7aa5b67
Github-Pull: #33258
Rebased-From: 4cf0ae4
7c6be9a doc: update release notes for 29.x (fanquake)
ea40fa9 ci: use LLVM 21 (fanquake)
5513516 ci: remove DEBUG_LOCKORDER from TSAN job (fanquake)
f9939cd ci: instrument libc++ in TSAN job (fanquake)
0fba5ae ci: allow libc++ instrumentation other than msan (fanquake)
10cbf22 ci: Use APT_LLVM_V in msan task (MarcoFalke)

Pull request description:

  Backports:
  * #32999
  * #33099 (added `ninja-build`)
  * #33258

ACKs for top commit:
  marcofleon:
    ACK 7c6be9a, looks okay to me

Tree-SHA512: 928882d505ed8101a6d4123947252a84d40bd350383408926b5c37aed56dc3359067d1d14c443c51351a6958a8dd9e141bb7713665295ff1f1ad86c5f8a36df0
The `SHA256AutoDetect` return output is used, among other use cases, to
name benchmarks. Using a comma breaks the CSV output.

This change replaces the comma with a semicolon, which fixes the issue.

Github-Pull: #33340
Rebased-From: 790b440
Rather than trying to match the apt installed clang version, which is
prone to intermittent issues. i.e #33345.

Github-Pull: #33364
Rebased-From: b736052
Since #29412, we have not allowed mutated blocks to continue
being processed immediately the block is received, but this
is only done for the legacy BLOCK message.

Extend these checks as belt-and-suspenders to not allow
similar mutation strategies to affect relay by honest peers
by applying the check inside
PartiallyDownloadedBlock::FillBlock, immediately before
returning READ_STATUS_OK.

This also removes the extraneous CheckBlock call.

Github-Pull: #32646
Rebased-From: bac9ee4
Previously in debug builds, this would cause an Assume crash if
FillBlock had been called previously. This could happen when multiple
blocktxn messages were received.

Co-Authored-By: Greg Sanders <gsanders87@gmail.com>

Github-Pull: #33296
Rebased-From: 5e585a0
Add test_multiple_blocktxn_response that checks that the peer is
disconnected.

Github-Pull: #33296
Rebased-From: 8b62647
tomt1664 and others added 30 commits July 14, 2026 10:17
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.