Rebase to latest and update to libbitcoinpqc 0.4#2
Open
cryptoquick wants to merge 3957 commits into
Open
Conversation
…ng UB d635993 validation: check invariants when inserting into m_blocks_unlinked (stratospher) 0852925 test/doc: remove misleading comment and improve tests (stratospher) ca4a380 test: add coverage for UB caused by FindMostWorkChain (stratospher) c787b3b validation: avoid duplicates in m_blocks_unlinked (stratospher) Pull request description: This is joint work with @ mzumsande. note: this requires a pruned node with deep reorgs to trigger. still it breaks assumptions in the codebase and is good to fix. A similar UB was fixed in bitcoin#34521. This PR prevents duplicate insertions into `m_blocks_unlinked` in `FindMostWorkChain`. There are 3 ways to insert into `m_blocks_unlinked`: 1. `LoadBlockIndex` - not problematic, as each block index is processed only once. 2. `ReceivedBlockTransactions` - not problematic, as this is usually only called once per block when it is first accepted in `AcceptBlock`. in the rare case it’s triggered again after pruning, the block would have been removed from `m_blocks_unlinked` when it was initially pruned, so duplicates still can’t arise. 3. `FindMostWorkChain` - problematic when multiple candidate tips share common chains of ancestors, traversals from each tip to the fork point may insert duplicate (`pprev`, `pindex`) entries for blocks whose parents have been pruned. When the missing parent is later received and `ReceivedBlockTransactions` processes `m_blocks_unlinked`, the same entry may be processed multiple times. This can result in the block being re-added to `setBlockIndexCandidates` with a modified `nSequenceId`, violating its ordering invariants and leading to undefined behavior. So avoid duplicate insertions into `m_blocks_unlinked` in `FindMostWorkChain`. ### how to test: use the updated `feature_pruning.py` which adds coverage for this scenario. - on master: the test (with the below diff) fails since `nSequenceId` is being modified for an entry in `setBlockIndexCandidates` - on this branch: the test (with the below diff) passes ```diff --git a/src/validation.cpp b/src/validation.cpp --- a/src/validation.cpp +++ b/src/validation.cpp @@ -3814,6 +3814,12 @@ void ChainstateManager::ReceivedBlockTransactions(const CBlock& block, CBlockInd pindex->nHeight, pindex->m_chain_tx_count, prev_tx_sum(*pindex), CLIENT_NAME, FormatFullVersion(), CLIENT_BUGREPORT); } pindex->m_chain_tx_count = prev_tx_sum(*pindex); + for (const auto& c : m_chainstates) { + if (c->setBlockIndexCandidates.contains(pindex)) { + LogInfo("### pindex UB = %s", pindex); + assert(false); + } + } pindex->nSequenceId = nBlockSequenceId++; for (const auto& c : m_chainstates) { c->TryAddBlockIndexCandidate(pindex); ``` ACKs for top commit: sedited: Re-ACK d635993 marcofleon: crACK d635993 stringintech: ACK d635993 mzumsande: Sure - Code Review ACK [d635993](bitcoin@d635993) Tree-SHA512: bb21adc2d92fe1865bbbcebf775a850ca3eccac6fe83d7bca10b78eee4c0abf782e44fa0ddfec9d9a70f42fa40bdc49b68a1d1b4905cdc371ba29117d3120619
Also remove raii_event_tests.
This avoids hardcoding xprvs and xpubs in the test. Also use key's parent fingerprint and derivation path while creating descriptors so that all of it is self-documenting and not dependent on hardcoded values, making them easy to read and update. Also use descsum_create function more to avoid hardcoding the descriptor checksum.
Each FuzzedSock used to own its mocked steady clock and call MockableSteadyClock::SetMockTime() directly. Hold the clock by reference to an externally provided FakeSteadyClock instead, so that several FuzzedSock instances sharing a test case (e.g. one per peer, or one created via Accept()) advance a single mocked clock, and the mocking goes through the FakeSteadyClock RAII helper that resets mocktime on destruction. FakeSteadyClock is a LimitOne type, so each fuzz target constructs one instance per iteration and passes it to ConsumeSock / ConsumeNode / the FuzzedSock constructor.
…, reject sendtoaddress/sendmany 2fe3480 wallet: reject sendtoaddress and sendmany for external signers (Sjors Provoost) bd5a32f doc: add taproot descriptor to getdescriptors example (woltx) 7131c82 doc: clarify which commands receive --chain, --fingerprint and --stdin (woltx) 4fdd4d8 doc: replace stale signtransaction wording with current signtx flow (woltx) fab9225 doc, rpc: document enumerate model field and fingerprint deduplication (woltx) Pull request description: This PR aligns the external signer documentation with current behavior, and makes one previously implicit behavior explicit. Per review feedback, each commit fixes a limited set of issues: * **doc, rpc: document enumerate model field and fingerprint deduplication** — the `enumerate` response uses the optional `model` field, which Bitcoin Core maps to the `name` field of the `enumeratesigners` RPC result. Duplicate fingerprints are skipped, and wallet operations require exactly one connected signer. * **doc: replace stale signtransaction wording with current signtx flow** — spending from an external signer wallet uses `send`/`sendall` (and `bumpfee` for fee-bumping), which invoke `<cmd> --stdin` and pass the `signtx` subcommand and PSBT over stdin. * **doc: clarify which commands receive --chain, --fingerprint and --stdin** — mark `--chain` and `--fingerprint` as required except for `enumerate`, keep `--stdin` required for protocol flexibility, and match the order and form of the actual invocations in the usage examples. * **doc: add taproot descriptor to getdescriptors example** — show the BIP86 `tr()` descriptor alongside the other address types. * **wallet: reject sendtoaddress and sendmany for external signers** — return a specific error instead of the misleading "Private keys are disabled for this wallet", with functional test coverage. Cherry-picked from bitcoin#33112 (thanks Sjors). How the documentation went stale: * The `enumerate` example has shown a `name` field since external signer support landed in bitcoin#16546, but the implementation has always read `model`. * `sendtoaddress`/`sendmany` external signer support was effectively precluded by bitcoin#21201, which was merged a few days before bitcoin#16546, so the interaction was missed in review and the documented `signtransaction` flow never existed in this form. * Fingerprint deduplication was added in bitcoin#35251. * The documentation was last updated in bitcoin#33765. ACKs for top commit: Sjors: ACK 2fe3480 optout21: ACK 2fe3480 naiyoma: ACK 2fe3480 Tree-SHA512: 86859d2f81ac337f3b4b6578c6ee0151ffb76b8374dfa58e28e00ce4eb69dc200cd6bd2d0a99f73d0475c3824d6ac1cb9e2542b119ca124dd835132dc95cd023
146b3ad doc: remove libevent (fanquake) 96d7f55 vcpkg: remove libevent (fanquake) 0443943 ci: remove libevent (fanquake) a0ca249 depends: remove libevent (fanquake) 35d2d06 cmake: remove libevent (fanquake) Pull request description: This builds on all the work done by fjahr and pinheadmz to fully remove libevent from the codebase. Closes bitcoin#31194. ACKs for top commit: fjahr: ACK 146b3ad dergoegge: ACK 146b3ad pinheadmz: ACK 146b3ad sedited: ACK 146b3ad Tree-SHA512: ecd14be93d11603d7c373a41474a7df1734b48550b12cd37933b604860913a77d42ee08bc187610881bec239b0834c2486f8fe52299cd3315a57b79c2e95929d
…artup defaults) b847626 test: refresh MiniWallet after node restart (Sjors Provoost) f4e643c test: merge mining options in package feerate check (Sjors Provoost) 280ce6a miner: ensure block_max_weight is flattened before limit checks (Sjors Provoost) 65bd316 mining: clarify test_block_validity comment (Sjors Provoost) 978e721 test: use shared default_ipc_timeout (Sjors Provoost) Pull request description: This implement the suggested followups from bitcoin#33966. Each commit links to the original comment. The most important change is the extra asserts added in `miner: ensure block_max_weight is flattened before limit checks`. ACKs for top commit: achow101: ACK b847626 enirox001: tACK b847626 sedited: ACK b847626 w0xlt: ACK b847626 Tree-SHA512: 47678eaed604228269bd892ccf8ff58804745bbc7675b4a93528da9a9292a2eb1e0562cdb8341edac77178563420885b48282bb9e5c2b997b28f2fc64ceeff3d
Rename the `CCoinsViewDB` async compaction wrapper to `CompactFullAsync()` so it is distinct from the blocking `CDBWrapper::CompactFull()` primitive it calls.
Exercise `CCoinsViewDB::CompactFullAsync()` from the `coins_view_db` fuzz target so the new chainstate compaction wrapper can run concurrently with ordinary coins view operations. The fuzz operation only schedules compaction, matching production; outstanding work is waited for by the `CCoinsViewDB` destructor at the end of the fuzz input.
…etation abc33ff test: announce field must be 0 or 1 in sendcmpct (brunoerg) 2d0dce0 net_processing: fix BIP152 first integer interpretation (brunoerg) Pull request description: Fixes bitcoin#35542 According to the BIP152, the first integer in `sendcmpct` message shall be interpreted as a boolean (and MUST have a value of either 1 or 0). We currently correctly interpret it as boolean, however, we accept any value >=1 and treat it as `true`, deviating from the specification. This PR fixes it. ACKs for top commit: edilmedeiros: utACK abc33ff davidgumberg: crACK bitcoin@abc33ff Seems reasonable to comply with BIP152 strictly, test looks good as well. Sjors: ACK abc33ff jonatack: re-ACK abc33ff achow101: ACK abc33ff w0xlt: ACK abc33ff Tree-SHA512: 77fed86d4de81f7c35ff002b6e1b2a90882ea55f159075da4d34a619d1075f625fca34f929cdd981f1b2eb06f76b64f42cda46502dcd1b4a634c690b0882ec7c
Libevent log category was partially removed in 39e9099, and this commit extends that with the following changes: - Stops showing libevent in the list of supported log categories in `bitcoind -help` and `bitcoin-cli help logging` output. - Stops returning `"libevent": false` in `logging` RPC output. It's not good to treat libevent as a supported log category when it can't be enabled and trying to enable it results in warnings. There's also no need to define an unused LIBEVENT constant value and keep more complicated logic for dealing with deprecated log categories, so this change also simplifies code internally. Co-authored-by: David Gumberg <davidzgumberg@gmail.com> Co-authored-by: l0rinc <pap.lorinc@gmail.com>
703a671 fuzz: compact coins view db during fuzzing (Lőrinc) 0868c85 refactor: rename async coin compaction (Lőrinc) Pull request description: **Problem:** bitcoin#35465 added async chainstate compaction, but the `coins_view_db` fuzz target did not exercise scheduling compaction alongside ordinary coins view operations. The async wrapper also shared the `CompactFull()` name with the blocking `CDBWrapper` primitive. **Fix:** Rename the coins DB wrapper to `CompactFullAsync()` and let `coins_view_db` randomly schedule it under `cs_main` (like in production). The fuzz operation only starts compaction and any running job is joined by the `CCoinsViewDB` destructor at the end of the fuzz input. ACKs for top commit: sedited: ACK 703a671 andrewtoth: ACK 703a671 Tree-SHA512: 9854c3acbaace795155e7469cb10938fbd872726cbb8a4b4ef71d6d352d00824498747215aee1b237e64452eae2e690a3f7d7daa6b6b0030e75a6f2ccc0802fb
…t after migrating 0cdd817 add release note (Pol Espinasa) 517d37c test: tests wallet migration with load_wallet disabled (Pol Espinasa) b98dd63 rpc: Add load_wallet argument to migratewallet RPC (Pol Espinasa) 4acd063 wallet: make loading the wallet after migrating optional (Pol Espinasa) 97d08d6 refactor: store wallet names to MigrationResult (Pol Espinasa) Pull request description: This PR is motivated by this [Stack Exchange question](https://bitcoin.stackexchange.com/questions/130713/bitcoin-core-quickest-method-legacy-descriptor-wallet-migration). Long story short, someone who has a node pruned before his legacy wallet birthday, is unable to migrate the wallet as it is not possible to load it. Loading is not necessary for migration, and migrating without wanting to use the wallet in that node is a valid use-case. This PR adds a new RPC argument to `migratewallet` that allow the user disabling the wallet loading. Second commits adds tests for it. Follow-up: Add an option to the GUI to not load the wallet after migrating. ACKs for top commit: achow101: ACK 0cdd817 w0xlt: ACK 0cdd817 pablomartin4btc: ACK bitcoin@0cdd817 Tree-SHA512: 8389599e63603b1a532e1bfba0b6c652653386c001f5a881bd49843302b74ff4dbaa4131b5b377c24f483d42e0e70a92b96f760244e3c2e2b44ce08cd04ca1e0
Cursor iteration is only supported by the coins database view. Pass `CCoinsViewDB` pointers to UTXO stats code and use the concrete DB pointer in the coins_view fuzz target before removing the abstract hook. Co-authored-by: sedited <seb.kung@gmail.com>
`CCoinsView` does not need to expose cursor iteration now that the few cursor users take `CCoinsViewDB` directly. Keep `Cursor()` on `CCoinsViewDB`, where iteration is supported, and remove the empty, forwarding, and throwing base-view overrides. This also removes the coins_view fuzz target probe that only asserted the unsupported `CCoinsViewCache::Cursor()` throw path. Co-authored-by: sedited <seb.kung@gmail.com>
The UTXO stats helpers require a non-null coins DB view after cursor users were narrowed to `CCoinsViewDB`. Use references for the DB view in the UTXO stats helpers and for local DB-view/block-manager variables at call sites, so the path no longer spells a nullable contract. Co-authored-by: sedited <seb.kung@gmail.com>
…d workarounds 2bab6bc refactor: Drop support for FreeBSD < 14 (Hennadii Stepanov) 91b5c8a refactor: Remove FreeBSD-specific workaround (Hennadii Stepanov) 56701ff doc: Clarify supported *BSD releases (Hennadii Stepanov) Pull request description: This PR establishes a baseline for the oldest *BSD releases supported by Bitcoin Core. Clarifying these minimum requirements paves the way for dropping compatibility code and workarounds for unsupported versions. The obsolete FreeBSD-specific workaround and version check have been dropped. ACKs for top commit: maflcko: lgtm ACK 2bab6bc willcl-ark: ACK 2bab6bc theStack: lgtm ACK 2bab6bc sedited: ACK 2bab6bc Tree-SHA512: 6d9ca0ff881a60c33fe3aa18a03726426f07f2896b2f56b12804865acfa910aca7efdc1312eb4055e35aab8423d0c2326b89c1da448e01b4fa213f73dfd2b118
…ble error code a8223bb wallet: Introduce WalletError with machine-readable error code (pseudoramdom) Pull request description: Per discussion in bitcoin#35436 (comment), `WalletError` is split out so that it can be reused by multiple wallet interface changes (bitcoin#34861 in particular) ---- Introduce a `wallet::WalletError`, a generic wallet-layer error type that contains - a machine-readable `WalletErrorCode` for programmatic handling - a translated user-facing `bilingual_str` message The initial enum is intentionally small. `WALLET_ERROR` is used for generic failures that callers should display to the user. The intention is to have more specific codes only when the callers can handle the condition differently. ACKs for top commit: achow101: ACK a8223bb davidgumberg: ACK a8223bb polespinasa: ACK a8223bb Tree-SHA512: 15fedf96cb5c3e8167a236bfa1a4d01d94f258a22943e53330ab432ad04a31f81f2eac8749f38390b2d34156d283d3bf76f293c640724bd0b090dc494123b230
…rm with explicit class members fed3cf6 wallet: Replace CWalletTx's vOrderForm with specific fields (Ava Chow) 4f8823e wallet: Drop vOrderForm from CommitTransaction (Ava Chow) a2b0bfc wallet: Drop mapValue from CWalletTx (Ava Chow) cb99864 wallet: Throw if unknown entry is found in mapValue (Ava Chow) 98d5cda wallet: Make CWalletTx "replaces_txid" and "replaced_by_txid" member variables (Ava Chow) 7ef8a6e wallet: Make CWalletTx "comment" and "to" member variables (Ava Chow) 2155e91 wallet: Make CWalletTx "from" and "message" member variables (Ava Chow) c6ba98d wallet: Drop mapValue from CommitTransaction (Ava Chow) 00abb17 wallet: Pass comment and comment_to to CommitTransaction (Ava Chow) 1a219a3 wallet: Pass replaces_txid to CommitTransaction outside of mapValue (Ava Chow) Pull request description: `mapValue` and `vOrderForm` are opaque data structures that contain transaction metadata. It is hard to determine what actual data each field contains, and they can ostensibly be misused where metadata is added in the future without developers realizing that such metadata exists. It's much clearer to have all of that metadata live in their own explicit member variables within `CWalletTx`. This PR implements that change. Since the serialization format of `CWalletTx` depends on `mapValue` and `vOrderForm`, the serialization remains unchanged, so when serializing these new members, they need to be shoved/extracted from a temporary `mapValue` or `vOrderForm`. This does end up breaking forwards compatibility as unknown fields in `mapValue` and `vOrderForm` are stripped out if the record is rewritten. However, I don't expect that we would continue to use these fields for future metadata, so I think that risk is low. ACKs for top commit: ajtowns: reACK fed3cf6 w0xlt: ACK fed3cf6 with above nits/caveats. Eunovo: ACK bitcoin@fed3cf6 Tree-SHA512: c7deab5aaeac13656012f8b13c0161fd420d2a5348eebd7649310e78ccb1216995aa6a7cbd506ac8d11d7b46b0856d6e6a897bc39965b51cfcf2268356ace261
…_FUZZ_BINARY db35b92 ipc # build: Fix fuzz target CMakeLists.txt for external libmultiprocess (Ryan Ofsky) Pull request description: CMake `WITH_EXTERNAL_LIBMULTIPROCESS` and `BUILD_FUZZ_BINARY` options stopped working together recently due to bitcoin#35118 commit 037ad77 because an non-namespaced `Libmultiprocess::multiprocess` target name was referenced. Fix by specifying the full target name which is better for readability anyway. ACKs for top commit: sedited: ACK db35b92 hebasto: ACK db35b92, I have reviewed the code and it looks OK. Tree-SHA512: 7cfb19a66dd4ccdcd6dfdedc3000fd20f488851f3235852717fd31698a7e8efb5c4e14705b0dc0c1ea506b38268f1ab030bca7865ddfdd85144598f435581247
…let is at the tip 9e62e4b test: slow down rescaning process (Pol Espinasa) 336f5a7 wallet: reserve walletrescan before checking wallet is at the tip (Pol Espinasa) Pull request description: `ImportDescriptors` rpc has a race condition where two imports running in parallel can both succeed or fail one of them. The race happens when there are two threads A and B trying to importdescriptors at the same time. 1. Thread A calls `BlockUntilSyncedToCurrentChain()` (holding `cs_wallet` fast, no contention) and then `reserve()`, acquiring the `WalletRescanReserver`. It proceeds to `ProcessDescriptorImport()`, which holds `cs_wallet` for an extended time (specially on slow machines) while importing descriptors. 2. B reaches `BlockUntilSyncedToCurrentChain()`, which internally does `WITH_LOCK(cs_wallet, ...)`. Since A holds `cs_wallet`, B blocks here for the entire duration of Thread A's descriptors import. 3. Then A finishes importing, releases `cs_wallet`, rescans (fast in regtest), and sets `fScanningWallet = false`. 4. B can now continue in `BlockUntilSyncedToCurrentChain()` acquiring `cs_wallet`, and then calls `reserve()` which succeeds because `fScanningWallet` is already `false`. Both imports succeed. I don't think the behavior is problematic at all from a usability PoV, but it can be a bad UX if some imports fails and other's no. It also makes testing difficult as race conditions are not easy to test. This PR fixes it by calling `reserver.reserve()` before `cs_wallet` is locked, so multiple threads will be aware of currently imports before being stuck at any point. So only one `importdescriptor` call can be done at the same time. I think this should fix bitcoin#35544 (comment) ACKs for top commit: achow101: ACK 9e62e4b nebula-21: ACK 9e62e4b w0xlt: lgtm reACK 9e62e4b Tree-SHA512: be0027e1a7b77252ed9fb514c3b3311d6905903d4b0bfc1021a1e1c2bb06872ef599647ff8a4536929240717ae9db62fb75374f629cd3046c7192e2b8b4d7344
d164a04 node: smooth oversized `-dbcache` warnings (Lőrinc) Pull request description: **Problem:** The oversized `-dbcache` warning threshold has a sharp formula cliff when detected RAM crosses the cutoff used by the warning logic. This was reported during review of [bitcoin#34641](bitcoin#34641 (comment)), where an earlier version could jump from the auto default at `4095 MiB` RAM to `75%` of RAM at `4096 MiB`. That made `1 MiB` of extra detected RAM raise the warning threshold from about `511 MiB` to `3072 MiB`. The surviving warning-only code has the same shape at a different boundary. Below `2 GiB` RAM the cap is `DEFAULT_DB_CACHE` (`450 MiB`), but at `2 GiB` it switches to `75%` of total RAM, so a tiny increase in detected RAM can suddenly raise the warning threshold from `450 MiB` to about `1536 MiB`. <img width="1484" height="881" alt="Image" src="https://github.com/user-attachments/assets/b51d5d24-31b8-4a2a-8f70-e7536481f855" /> **Fix:** Base the warning on a reserved non-dbcache memory budget instead: ```math \text{warn if } \mathit{dbcache} > \max\left(\mathit{DEFAULT\_DB\_CACHE}, 0.75 \cdot \max(\text{total RAM} - \mathit{DBCACHE\_WARNING\_RESERVED\_RAM}, 0)\right) ``` `DBCACHE_WARNING_RESERVED_RAM` is `2 GiB`, so the fixed `DEFAULT_DB_CACHE` cap remains the floor below that reserve and the warning threshold grows monotonically above it. This keeps the warning conservative around low-memory boundaries and avoids treating a boundary-crossing `1 MiB` RAM difference as a reason to allow a much larger explicit `-dbcache`. **Quick reference:** | System RAM | Previous warning cap | New warning cap | | ---------- | -------------------- | --------------- | | 1 GiB | 450 MiB | 450 MiB | | 2 GiB | 1536 MiB | 450 MiB | | 3 GiB | 2304 MiB | 768 MiB | | 4 GiB | 3072 MiB | 1536 MiB | | 8 GiB | 6144 MiB | 4608 MiB | | 16 GiB | 12288 MiB | 10752 MiB | | 32 GiB | 24576 MiB | 23040 MiB | On 32-bit builds, effective `-dbcache` values are still capped to `1024 MiB` before the warning check, so thresholds above that cap are not reachable there. ACKs for top commit: optout21: reACK d164a04 sedited: Re-ACK d164a04 w0xlt: reACK d164a04 Tree-SHA512: deb81f0e192261f01dda6f1575a7b2e147f1e07e813d0833c7aeaf6a3fbd7594c0145ec1c98ded74efee9005108d229942949279c9458cbec4046913387db845
…penBSD a54ec37 depends: Build `qt` and `qrencode` packages for OpenBSD hosts (Hennadii Stepanov) Pull request description: This PR enables the building of GUI dependencies ( `qt` and `qrencode`) natively on OpenBSD: <img width="1280" height="960" alt="VirtualBox_OpenBSD Desktop_01_06_2026_00_57_50" src="https://github.com/user-attachments/assets/9ab20ad2-8812-42c7-9858-65ee6a7f6a26" /> --- Build logs in the GHA environment are available here: - OpenBSD 7.8: https://github.com/hebasto/bitcoin-core-nightly/actions/runs/26852431726/job/79187479855 - OpenBSD 7.9: https://github.com/hebasto/bitcoin-core-nightly/actions/runs/26852431726/job/79187479827 --- The [branch](https://github.com/hebasto/bitcoin/tree/260531-openbsd-qt-cross) that is compatible with [cross-compiling](bitcoin#35397) from Linux to OpenBSD is still a WIP. ACKs for top commit: fanquake: ACK a54ec37 - changes look fine, didn't build or test. Tree-SHA512: 2d674b8b5677b2d1f91337243f1173dba4beb12bb8aa2ee6bad71de50dd403d6dd50009f1836c22abad63ec54ebb94bcfc9c664a8fc811f900b4ddef4623626e
Disable this for now, until issues with llvm-ranlib, and gui deps are fixed: ```bash libtool: install: chmod 644 /home/runner/work/_temp/depends/work/staging/x86_64-unknown-openbsd/fontconfig/2.12.6-a12d0a13377/home/runner/work/_temp/depends/x86_64-unknown-openbsd/lib/libfontconfig.a libtool: install: llvm-ranlib-22 -t /home/runner/work/_temp/depends/work/staging/x86_64-unknown-openbsd/fontconfig/2.12.6-a12d0a13377/home/runner/work/_temp/depends/x86_64-unknown-openbsd/lib/libfontconfig.a llvm-ranlib-22: error: Invalid option: '-t' make[4]: *** [Makefile:539: install-libLTLIBRARIES] Error 1 ```
b0e0951 ci: disable Qt build in OpenBSD cross job (fanquake) Pull request description: Disable this for now, until issues with `llvm-ranlib`, and gui deps are fixed: ```bash libtool: install: chmod 644 /home/runner/work/_temp/depends/work/staging/x86_64-unknown-openbsd/fontconfig/2.12.6-a12d0a13377/home/runner/work/_temp/depends/x86_64-unknown-openbsd/lib/libfontconfig.a libtool: install: llvm-ranlib-22 -t /home/runner/work/_temp/depends/work/staging/x86_64-unknown-openbsd/fontconfig/2.12.6-a12d0a13377/home/runner/work/_temp/depends/x86_64-unknown-openbsd/lib/libfontconfig.a llvm-ranlib-22: error: Invalid option: '-t' make[4]: *** [Makefile:539: install-libLTLIBRARIES] Error 1 ``` Followup to bitcoin#35427. ACKs for top commit: maflcko: lgtm ACK b0e0951 hebasto: ACK b0e0951. Tree-SHA512: fab7bb503e4f729ced1a986535a05db6e9218077e3a55c24741cd956f759f42fa8e11f56952f657dfa002ac4f42e8207d7d6a7f19cf091307cf9350ec36e964c
e8de5c7b68 Merge bitcoin-core/libmultiprocess#305: refactor: memcpy to std::ranges::copy to work around ubsan warn 9307e68e5a Merge bitcoin-core/libmultiprocess#306: doc: Bump version 12 > 13 fac7b9b7f6 refactor: memcpy to std::ranges::copy to work around ubsan warn 1bd7025609 Merge bitcoin-core/libmultiprocess#297: test: add map serialization round-trip coverage 438fdd243d doc: Bump version 12 > 13 463d073cb8 test: rename vBool to vector_bool 85df233845 test: add mapStringInt to foo.capnp to cover map serialization and deserialization git-subtree-dir: src/ipc/libmultiprocess git-subtree-split: e8de5c7b68e0ae21c94ae92aa22e5c3b213f9c12
HEAD is the prior subtree update commit 'a9d1b652f324126ef7e80d9ab0b9e4f60019dade'
This is a style cleanup. The general pattern to use `LIMITED_WHILE`, which all other fuzz tests use, has some benefits: * When no data is available, a simple and single (let's say) 64 value in the fuzz input will not result in 64 loops over the same body with the same default/fallback values. * When no data is available, `ConsumeBool` falls back to `false` and breaks the loop early. * When further data is available, the overhead is just a single byte, making it also possibly easier for the fuzz engine to mutate the data, as a single int that influences the whole remainder of the fuzz input can lead to the 'havoc' effect. * When a crash is reduced, deleting bytes will directly influence the execution length, so byte-length of the fuzz input roughly corresponds to run-time length.
This is a whitespace-only clang-format change. To verify it, one can run: ```sh (git show | git apply --reverse ) && ( git diff -U0 | ./contrib/devtools/clang-format-diff.py -p1 -i -v ) && git diff HEAD ``` A few minor, non-macro formatting adjustments were made in touched files: * `src/wallet/test/fuzz/crypter.cpp`: Removed a redundant double semicolon * `src/test/fuzz/txorphan.cpp`: Corrected indentation on an `else if` block. * `src/test/fuzz/mini_miner.cpp`: Removed an unnecessary empty line.
…tck_TransactionInput 6667dc4 kernel: expose scriptSig for btck_TransactionInput (Peter Zafonte) e6de3a2 kernel: expose witness stack for btck_TransactionInput (Peter Zafonte) Pull request description: Silent payments scanning needs the public key from every input. For SegWit inputs it is in the witness stack. For P2PKH inputs it is in scriptSig. Without these new functions, callers must deserialize the raw transaction themselves to reach that data, which is difficult and error-prone. Introduces a `btck_WitnessStack` type and adds the following functions: **Witness stack:** `btck_transaction_input_get_witness_stack`: returns a non-owning `const btck_WitnessStack* `view `btck_witness_stack_count_items `: item count `btck_witness_stack_get_item_at `: single item by index via btck_WriteBytes `btck_witness_stack_copy` / `btck_witness_stack_destroy`: lifecycle for owned copies **scriptSig:** `btck_transaction_input_get_script_sig`: full scriptSig via btck_WriteBytes All functions are exposed in the C++ wrapper via `WitnessStackView`, `WitnessStack`, and `WitnessStackApi` , and `GetScriptSig()`. ACKs for top commit: sedited: ACK 6667dc4 musaHaruna: ACK [6667dc](bitcoin@6667dc4) stickies-v: ACK 6667dc4 Tree-SHA512: b5e9d32ec87a5f5a9fea5652ed69737eae1d1f9cfb777544b96d703bbd057c114e0b7afae4e9e9d09b8b546694b14fb923b747c04a8f0211ab6caa015d06967d
900a778 ci: use Ubuntu 26.04 for lint container (fanquake) 058a73a ci: use uv 11.x (fanquake) 14f4ddc ci: mypy 2.3.0 (fanquake) Pull request description: Container: Ubuntu `24.04` -> `26.04`. `uv`: `10.x` -> `11.x` mypy: `1.19.1` -> `2.3.0` ACKs for top commit: hebasto: ACK 900a778. willcl-ark: ACK 900a778 Tree-SHA512: fc664b9cd16257ae023a4a60566bb512bb9b12d72a81e34a4a6659490a9eb84922cef6e44c622d4b52048788ce880d71a9b5168591aadbb83eae62fe54f54fcb
…z test workaround fab8eee fuzz: clang-format LIMITED_WHILE (MarcoFalke) fa0d777 fuzz: Clang-format LIMITED_WHILE like while (MarcoFalke) fa1a9bd fuzz: Remove unused workaround after fix in libmultiprocess byte-span serializer (MarcoFalke) fa55385 fuzz: Use LIMITED_WHILE over for-loop with consumed size integral (MarcoFalke) 6d5f753 Squashed 'src/ipc/libmultiprocess/' changes from 28e056576a..e8de5c7b68 (MarcoFalke) Pull request description: Includes several changes, to first update the subtree. Then, modify the fuzz test to address review comments: * bitcoin#35118 (comment) * bitcoin#35118 (comment) ACKs for top commit: ryanofsky: Code review ACK fab8eee. Just fuzz test clang-format cleanups added since last review, which seem nice Tree-SHA512: 0836628f8ee54adf02571025456211a74f63d05058b72280b10111ecfbb93d30945f4a48f30cc790774de4e2b489907313e86b0fa6729f3480c64850c94848b4
Remove the `Mutex` from the `coins_view` and `coinscache_sim` pool startup helpers. Fuzz targets are entered sequentially within a process and parallel fuzzing uses separate processes/forks, which each have their own copy of the global thread pool. Therefore, a mutex to prevent two in-process callers from racing to start the pool isn't needed.
240d5f7 fuzz: Drop unnecessary mutexes (marcofleon) Pull request description: Quick cleanup addressing bitcoin#35295 (comment). This follows the same logic as 48df093 from bitcoin#35521. ACKs for top commit: maflcko: lgtm ACK 240d5f7 nervana21: ACK 240d5f7 l0rinc: code review ACK 240d5f7 Tree-SHA512: 869127dc8cec4fc0688f92d67dc22f643cf3741c62d645eb4a959a9d8f46590efe19765f7df97ee36f9a3ca759c50fe983a3e6655e40a53ea99904e7a1ec00f6
… values 3ae3a94 wallet: avoid call bumpfeediscount with negative values (Pol Espinasa) Pull request description: in bitcoin#34232 dergoegge reported an assertion fail in `SetBumpFeeDiscount`. **Context** The bump-fee discount in the savings that we can have when multiple UTXOs that we selected for our transaction share a common unconfirmed ancestor transaction. We know we can have some savings because we first calculate the `summed_bump_fees` and then compare to the `combined_bump_fee`. For context: - `bump_fees`: Extra fees that the new transaction must pay to contribute to get his ancestor confirmed. - `summed_bump_fees`: The sum of each input ancestor `bump_fee`. If we have two inputs with unconfirmed ancestors A and B and both have a `bump_fee = 100` then `summed_bump_fees = 200`. - `combined_bump_fee`: Is the total `bump_fees` summed of all inputs, but taking into account shared ancestors. If A and B share the transaction ancestor then `combined_bump_fee = 100` not `200` as the transaction must be bumped only once. If `summed_bumpfees` > `combined_bump_fee` we are overestimating the `bump_fee` as we are counting multiple times the same ancestors so we can discount it using `SetBumpFeeDiscount(summed_bump_fees - combined_bump_fees)`. **Problem** To calculate `summed_bump_fees` and `combined_bump_fee` we use two different fresh MiniMiner snapshots of the mempool. Because they are called in different moments the two snapshots of the mempool might be different. An artificial feerate decrease of an ancestor using `prioritizesettransaction` can make `combined_bump_fee > summed_bump_fees` creating a negative discount. This cause calling `SetBumpFeeDiscount` with a negative vaule triggering an assertion `discount >= 0`. **Fix** This PR fixes it by ensuring not only that a discount exist but also that is greater the 0. **Test** It is hard to manually trigger this race condition. dergoegge coded a patch and test to trigger it that can be used to test the fix. polespinasa@5320e2f ACKs for top commit: achow101: ACK 3ae3a94 pablomartin4btc: ACK 3ae3a94 Tree-SHA512: e76693eb66c4883ed3ef5edf1baa972657b0d532487803706312d486b4d4a4997f5dcbc64781a1a21b7b5628b06fcc97a6633382509061734d0f5615e851bef1
72db4ac coins: drop stale cursor null checks (Lőrinc) 3d2f2d8 coins: pass UTXO stats view by reference (Lőrinc) 35aedb2 coins: drop cursor from base view (Lőrinc) c6fbe2f coins: pass DB view to cursor users (Lőrinc) Pull request description: **Problem:** `CCoinsView::Cursor()` makes cursor iteration look like a generic coins view operation, but cursor iteration is only supported by the DB-backed coins view. The cache override only threw, and the `coins_view` fuzz target only asserted that deterministic unsupported throw path. **Fix:** Make cursor iteration a `CCoinsViewDB` operation. Cursor users now take the DB-backed view directly, `CCoinsView` no longer exposes `Cursor()`, and the fuzz target keeps DB-backed cursor coverage while dropping the unsupported cache throw probe. The UTXO stats path is also tightened to pass the non-null DB view by reference, and stale null handling for DB cursors is removed. This was extracted from review discussion in bitcoin#35295 (comment) and extended based on bitcoin#35562 (comment). ACKs for top commit: achow101: ACK 72db4ac sedited: Re-ACK 72db4ac w0xlt: ACK 72db4ac andrewtoth: ACK 72db4ac Tree-SHA512: 12a81330a6ec1b91a7e4393f3761ea9ed4702ecb24312f1defa5a9a079a396ce921fc52f74fe296e5ac7ab20d5b5a8a84e858c96847f333c58b7fa9de9e8143e
…rate response 4c9de7d external_signer: validate fingerprint from enumerate response (Kyle 🐆) Pull request description: `enumeratesigners` takes the `fingerprint` field from the external signer's `enumerate` output and stores it without checking it. That value is later handed back to the signer command as `--fingerprint <value>` (e.g. in `displayaddress`), so a malformed value propagates unchecked. A master key fingerprint is 4 bytes, i.e. 8 hex characters. This adds a check that the reported fingerprint is exactly 8 hex characters and throws a clear error otherwise. A functional test covers empty, wrong-length, and non-hex inputs. ACKs for top commit: Sjors: utACK 4c9de7d achow101: ACK 4c9de7d sedited: ACK 4c9de7d Tree-SHA512: 7c3303b24e234e13a4c20c0b93552145b9ccffc29d1bae42ce8a2faf548377f051e52f8ffb3924679065b27d15fc7bf3859e5ae32a2bb185738cc29bc0ade486
dab7f2c test: cover -externalip/onlynet interaction in functional test (will) 657a5aa test: cover -externalip bypassing -onlynet (will) 8c87e32 net: let -externalip bypass -onlynet (will) f4af02e net: add an add_even_if_unreachable argument to AddLocal (will) Pull request description: `-onlynet` is documented to restrict automatic outbound connections, but it also currently prevents `-externalip` addresses from being advertised when their network is not in the `-onlynet` set. This happens because `AddLocal()` rejects addresses outside `g_reachable_nets`, regardless of whether the address was explicitly configured by the user. Previous attempts to fix this (bitcoin#24835 and bitcoin#25690) removed the `g_reachable_nets` check from `AddLocal()`. This PR instead adds an explicit `add_even_if_unreachable` argument to `AddLocal()`. The argument defaults to `false`, and is set to `true` only when adding addresses from `-externalip`. As a result, explicitly configured `-externalip` addresses can still be advertised even when their network is excluded by `-onlynet`, while discovered, mapped, bound, Tor control, and I2P SAM addresses continue to use the existing reachable-network filter. This keeps the fix scoped to `-externalip` and addresses the concern raised in bitcoin#25690: > I think it might also be weird for a user to activate -onlynet and keep on advertising their clearnet address to the network The branch adds unit coverage for `AddLocal()` and functional coverage in `p2p_addr_selfannouncement.py` for `-onlynet=ipv4 -externalip=<onion>`. Fixes: bitcoin#25336 Fixes: bitcoin#25669 ACKs for top commit: achow101: ACK dab7f2c mzumsande: re-ACK dab7f2c w0xlt: ACK dab7f2c Tree-SHA512: a4ac9334b85da8b6902d3850e21d3a1c9d7dce70bcb79182448c8d5684e24462cd6e440385af7aa4420d9582e4dff9dc9e827ca7a6da0363fff2d3c531784d9b
Bring p2mr up to date with Bitcoin Core master while preserving BIP 360 P2MR consensus, policy, descriptor, and test coverage.
32 tasks
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
20 participants
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.
Should be no surprises here, just a lot of commits to sync up