aboutsummaryrefslogtreecommitdiff
AgeCommit message (Collapse)Author
2024-01-16build: always set -g -O2 in CORE_CXXFLAGSfanquake
This avoids cases of missing -O2, when *FLAGS has been overriden. Removes the need for duplicate code to clear autoconf defaults. Also, move CORE_CXXFLAGS before DEBUG_CXXFLAGS, so that -O2 is always overriden if debugging etc.
2024-01-16Merge bitcoin/bitcoin#29185: build: remove `--enable-lto`fanquake
2d1b1c7daeeada3f737e62ceb2db7484cde5ff4e build: remove --enable-lto (fanquake) Pull request description: This has outlived its usefulness, doesn't gel well with newer compilers & `-flto` related options, i.e thin vs full, or `=auto`, and having `-flto` as the only option means that sometimes this just needs to be worked around, i.e in oss-fuzz: https://github.com/google/oss-fuzz/blob/master/projects/bitcoin-core/build.sh. While it was convenient when `-flto` was newer, support for `-flto` is now in all compilers we use, and there's also no-longer any real need for us to treat `-flto` different to any other optimization option. Remove it, to remove build complexity, and so there's no need to port a similar option to CMake. Note that the LTO option remains in depends, because we still a way to build packages that have LTO specific patches/options. ACKs for top commit: TheCharlatan: ACK 2d1b1c7daeeada3f737e62ceb2db7484cde5ff4e hebasto: ACK 2d1b1c7daeeada3f737e62ceb2db7484cde5ff4e. Tree-SHA512: 91812de7da35346f51850714a188fcffbac478bc8b348bf756c2555fcbde86ba622ac2fb77d294dea0378c741d3656f06121ef3a795aeed63fd170fc31bfa5af
2024-01-15Merge bitcoin/bitcoin#29227: log mempool loading progressfanquake
eb78ea4eebfe150bc1746282bfdad6eb0f764e3c [log] mempool loading (glozow) Pull request description: Motivated by #29193. Currently, we only log something (non-debug) when we fail to load the file and at the end of importing all the transactions. That means it's hard to tell what's happening if it's taking a long time to load. This PR adds a maximum of 10 new unconditional log lines: - When we start to load transactions. - Our progress percentage when it advances by at least 10% from the last time we logged. Percentage is based on the number of transactions. If there are lots of transactions in the mempool, the logs will look like this: ``` 2024-01-11T11:36:30.410726Z Loading 401 mempool transactions from disk... 2024-01-11T11:36:30.423374Z Progress loading mempool transactions from disk: 10% (tried 41, 360 remaining) 2024-01-11T11:36:30.435539Z Progress loading mempool transactions from disk: 20% (tried 81, 320 remaining) 2024-01-11T11:36:30.447874Z Progress loading mempool transactions from disk: 30% (tried 121, 280 remaining) 2024-01-11T11:36:30.460474Z Progress loading mempool transactions from disk: 40% (tried 161, 240 remaining) 2024-01-11T11:36:30.473731Z Progress loading mempool transactions from disk: 50% (tried 201, 200 remaining) 2024-01-11T11:36:30.487806Z Progress loading mempool transactions from disk: 60% (tried 241, 160 remaining) 2024-01-11T11:36:30.501739Z Progress loading mempool transactions from disk: 70% (tried 281, 120 remaining) 2024-01-11T11:36:30.516334Z Progress loading mempool transactions from disk: 80% (tried 321, 80 remaining) 2024-01-11T11:36:30.531309Z Progress loading mempool transactions from disk: 90% (tried 361, 40 remaining) 2024-01-11T11:36:30.549019Z Imported mempool transactions from disk: 401 succeeded, 0 failed, 0 expired, 0 already there, 400 waiting for initial broadcast ``` If there are 0 or 1 transactions, progress logs aren't printed. ACKs for top commit: kevkevinpal: Concept ACK [eb78ea4](https://github.com/bitcoin/bitcoin/pull/29227/commits/eb78ea4eebfe150bc1746282bfdad6eb0f764e3c) ismaelsadeeq: ACK eb78ea4eebfe150bc1746282bfdad6eb0f764e3c dergoegge: Code review ACK eb78ea4eebfe150bc1746282bfdad6eb0f764e3c theStack: re-ACK eb78ea4eebfe150bc1746282bfdad6eb0f764e3c mzumsande: tested ACK eb78ea4eebfe150bc1746282bfdad6eb0f764e3c Tree-SHA512: ae4420986dc7bd5cb675a7ebc76b24c8ee60007f0296ed37e272f1c3415764d44963bea84c51948da319a65661dca8a95eac2a59bf7e745519b6fcafa09812cf
2024-01-15Merge bitcoin/bitcoin#29237: depends: Allow PATH with spaces in directory names.fanquake
4756114e505cff8848fb6344ef9a48d8822066c1 [depends] Allow PATH with spaces in directory names. (Mark Friedenbach) Pull request description: The goal of this PR is to help close https://github.com/bitcoin/bitcoin/pull/28733. I reverted the change on `depends/config.guess` based on the feedback provided in the previous PR. I've also incorporated the test mentioned by maflcko ACKs for top commit: maflcko: lgtm ACK 4756114e505cff8848fb6344ef9a48d8822066c1 hebasto: ACK 4756114e505cff8848fb6344ef9a48d8822066c1, successfully built depends on Ubuntu 22.04. TheCharlatan: ACK 4756114e505cff8848fb6344ef9a48d8822066c1 Tree-SHA512: ee257f6efd235839156bc236384f08d77b91debc3c257168368a71e70742639f28a3289572b8693609c1109062dc9968e461103d1f4f5679906506e94b54e649
2024-01-15Merge bitcoin/bitcoin#29241: doc: Add missing backtick in developer notes ↵fanquake
logging section c003562120c193c296ac754b4bb8cff02bbbe4dc doc: Add missing backtick in developer notes logging section (Fabian Jahr) Pull request description: Newly added logging section from https://github.com/bitcoin/bitcoin/pull/28318 is missing a single backtick. Also fixes some minor punctuation errors in that section. ACKs for top commit: jonatack: ACK c003562120c193c296ac754b4bb8cff02bbbe4dc alfonsoromanz: ACK c003562120c193c296ac754b4bb8cff02bbbe4dc Tree-SHA512: 2f75f9472d212ce7c7ebf3e7404f86b3bd8c695f63e2a7447d7a55bb54dcdb5e3bfd15e8eac5b92efbdcf1216c4e8d699cae0250d021f72b9d1c32a7db91989d
2024-01-15Merge bitcoin/bitcoin#29243: wallet: Reset chain notifications handler if ↵fanquake
AttachChain fails ea2551e55d260854a5cca8aa95034970d4adca1c wallet: Reset chain notifications handler if AttachChain fails (Ava Chow) Pull request description: AttachChain will create the chain notifications handler which contains a reference to the wallet's shared_ptr. If AttachChain fails, the wallet needs to be unloaded, and this is expected to happen with its custom deleter ReleaseWallet. However, if the chain notifications handler is still set, then the shared_ptr is still referenced by something, so the wallet is never actually released. This behavior can also be verified by looking at the debug.log file. When the wallet is released, the line "Releasing wallet" should appear in the debug.log file. However the failing test does not contain that line, indicating that the problem is that the `CWallet` object is not being destroyed. After this PR, that log line now appears, and the test also passes. Fixes #29234 ACKs for top commit: jamesob: ACK https://github.com/bitcoin/bitcoin/pull/29243/commits/ea2551e55d260854a5cca8aa95034970d4adca1c murchandamus: ACK ea2551e55d260854a5cca8aa95034970d4adca1c TheCharlatan: ACK ea2551e55d260854a5cca8aa95034970d4adca1c furszy: Code review ACK ea2551e5 BrandonOdiwuor: Code Review ACK ea2551e55d260854a5cca8aa95034970d4adca1c Tree-SHA512: 73d676068c699303d9bcf70c9288ecb05f5f2e400ff3b7201367658d39d2fab63def97ab5ce4a742a6f2ca5e325f598fdbb6ce8f157a0423c07abc9a19bd5c81
2024-01-12wallet: Reset chain notifications handler if AttachChain failsAva Chow
AttachChain will create the chain notifications handler which contains a reference to the wallet's shared_ptr. If AttachChain fails, the wallet needs to be unloaded, and this is expected to happen with its custom deleter ReleaseWallet. However, if the chain notifications handler is still set, then the shared_ptr is still referenced by something, so the wallet is never actually released.
2024-01-12doc: Add missing backtick in developer notes logging sectionFabian Jahr
Also fix some minor punctuation error in the section.
2024-01-12[log] mempool loadingglozow
Log at the top before incrementing so that this log isn't printed when there's only 1 tx.
2024-01-12Merge bitcoin/bitcoin#29235: doc: refer to "Node relay options" in policy/READMEglozow
0d627c4ca8684653ddef3bb4041ad1e129ed3d4d doc: refer to "Node relay options" in policy/README (djschnei21) Pull request description: Fixed up #29095, to refer to `-help`, rather than listing every option. ACKs for top commit: stickies-v: ACK 0d627c4ca8684653ddef3bb4041ad1e129ed3d4d glozow: lgtm ACK 0d627c4ca8684653ddef3bb4041ad1e129ed3d4d Tree-SHA512: 37d36ffa48297371eb0032ed48dce28802f862f6c18bdb50207555a228ce252e51a93a6fdef86b3e596d486c5107594d64db89f077b77fc885fe84cecb1dadc3
2024-01-12Merge bitcoin/bitcoin#28885: mempool / rpc: followup to ↵glozow
getprioritisedtransactions and delete a mapDeltas entry when delta==0 0eebd6fe7d01ddc7f6b7f13a6ed6e705c7aeae4e test: Assert that a new tx with a delta of 0 is never added (kevkevin) cfdbcd19b32fd63954d7947dcc639aef291fb6b2 rpc: exposing modified_fee in getprioritisedtransactions (kevkevin) 252a86729a15e47ed168d8da7c4a8d6113673909 rpc: renaming txid -> transactionid (kevkevin) 2fca6c2dd03c3955d86efb0b8d2a7961e42115fd rpc: changed prioritisation-map -> "" (kevkevin) 3a118e19e100110300d3290d4c1434f963721d94 test: Directly constructing 2 entry map for getprioritisedtransactions (kevkevin) Pull request description: In this PR I am addressing some comments in https://github.com/bitcoin/bitcoin/pull/27501 as a followup. - changed `prioritisation-map` in the `RPCResult` to `""` - Directly constructing 2 entry map for getprioritisedtransactions in functional tests - renamed `txid` to `transactionid` in `RPCResult` to be more consistent with naming elsewhere - exposed the `modified_fee` field instead of having it be a useless arg - Created a new test that asserts when `prioritisedtransaction` is called with a fee_delta of 0 it is not added to mempool ACKs for top commit: glozow: reACK 0eebd6fe7d01ddc7f6b7f13a6ed6e705c7aeae4e, only change is the doc suggestion Tree-SHA512: e99056e37a8b1cfc511d87c83edba7c928b50d2cd6c2fd7c038976779850677ad37fddeb2b983e8bc007ca8567eb21ebb78d7eae9b773657c2b297299993ec05
2024-01-12Merge bitcoin/bitcoin#29208: build: Bump clang minimum supported version to 14fanquake
aaaace2fd1299939c755c281b787df0bbf1747a0 fuzz: Assume presence of __builtin_*_overflow, without checks (MarcoFalke) fa223ba5eb764fe822229a58d4d44d3ea83d0793 Revert "build: Fix undefined reference to __mulodi4" (MarcoFalke) fa7c751bd923cd9fb4790fe7fb51fafa2faa1db6 build: Bump clang minimum supported version to 14 (MarcoFalke) Pull request description: Most supported operating systems ship with clang-14 (or later), so bump the minimum to that and allow new code to drop workarounds for previous clang bugs. For reference: * https://packages.debian.org/bookworm/clang (`clang-14`) * https://packages.ubuntu.com/jammy/clang (`clang-14`) * CentOS-like 8/9 Stream: All Clang versions from 15 to 17 * FreeBSD 12/13: All Clang versions from 15 to 16 * OpenSuse Tumbleweed ships with https://software.opensuse.org/package/clang (`clang17`); No idea about OpenSuse Leap On operating systems where the clang version is not shipped by default, the user would have to use GCC, or install clang in a different way. For example: * https://packages.debian.org/bullseye/g++ (g++-10) * https://packages.ubuntu.com/focal/g++-10 * https://apt.llvm.org/, or nix, or guix, or compile clang from source, ... ACKs for top commit: fanquake: ACK aaaace2fd1299939c755c281b787df0bbf1747a0 Tree-SHA512: 81d066b14cc568d27312f1cc814b09540b038a10a0a8e9d71fc9745b024fb6c32a959af673e6819b817ea7cef98da4abfa63dff16cffb7821b40083016b0291f
2024-01-12Merge bitcoin/bitcoin#29218: ci: Rename tasks (previous releases, macOS cross)fanquake
fa0c594b33970e12d97e6879ab4ca57045453492 ci: Rename tasks (previous releases, macOS cross) (MarcoFalke) Pull request description: The previous releases task no longer uses the qt5 dev package, but the depends package, so fix that in the name. Also, remove a detail from the macOS cross task name, because anyone can look it up in the source, if they really want to. Otherwise, it may go out of date in the name. Also, rename the two tasks' config file to reflect the same. ACKs for top commit: fanquake: ACK fa0c594b33970e12d97e6879ab4ca57045453492 Tree-SHA512: e6f1d04128d35462c49367c98a0227988695d75add88c569804551e3fd30c22292d22b88fa19c54f02fab0c9784c77a078447de0280553a3fc7162dcf992d7ae
2024-01-11[depends] Allow PATH with spaces in directory names.Mark Friedenbach
added test and update code based on feedback
2024-01-11doc: refer to "Node relay options" in policy/READMEdjschnei21
2024-01-11Merge bitcoin/bitcoin#29212: Fix -netinfo backward compat with getpeerinfo ↵Ava Chow
pre-v26 5fa74609b833643334dfb5519f2023119984267b Fix -netinfo backward compat with getpeerinfo pre-v26 (Jon Atack) Pull request description: Commit fb5bfed26a564014b83ccfc96ff00b630930fc61 in #29058 will cause `-netinfo` to break when calling it on a node that is running pre-v26 bitcoind, as `getpeerinfo` doesn't yet return a "transport_protocol_type" field. Fix this by adding an `IsNull()` check, as already done for other recent getpeerinfo fields, and also in the same commit: a) avoid checking for the full string "detecting", and instead do the cheaper check for the most frequent case of the string starting with "v" b) drop displaying the "v" prefix in all the rows, as it doesn't add useful information, and instead use "v" for the column header c) display nothing when a value isn't determined yet, like for the -netinfo mping and ping columns (as `*` already has a separate meaning in this dashboard, and `?` might look like there is a bug) ACKs for top commit: mzumsande: Code Review ACK 5fa74609b833643334dfb5519f2023119984267b achow101: ACK 5fa74609b833643334dfb5519f2023119984267b kristapsk: ACK 5fa74609b833643334dfb5519f2023119984267b Tree-SHA512: 4afc513dc669b95037180008eb4c57fc0a0d742c02f24b236562d6b8daad5c120eb1ce0d90e51696e0f9b8361a72fc930c0b64f04902cf96fb48c8e042e58624
2024-01-11Merge bitcoin/bitcoin#29034: test: detect OS in functional tests ↵Ava Chow
consistently using `platform.system()` 878d914777a03a04ecb84217152e8b7fd73a5062 doc: test: mention OS detection preferences in style guideline (Sebastian Falbesoner) 4c65ac96f8b021c107783adce3e8afe4f8edee6e test: detect OS consistently using `platform.system()` (Sebastian Falbesoner) 37324ae3dfb0e50daaf752dc863a880559fa4637 test: use `skip_if_platform_not_linux` helper where possible (Sebastian Falbesoner) Pull request description: There are at least three ways to detect the operating system in Python3: - `os.name` (https://docs.python.org/3.9/library/os.html#os.name) - `sys.platform` (https://docs.python.org/3.9/library/sys.html#sys.platform) - `platform.system()` (https://docs.python.org/3.9/library/platform.html#platform.system) We are currently using all of them in functional tests (both in individual tests and shared test framework code), which seems a bit messy. This PR consolidates into using `platform.system()`, as it appears to be one most consistent and easy to read (see also [IRC discussion](https://bitcoin-irc.chaincode.com/bitcoin-core-dev/2023-12-08#989301;) and table below). `sys.platform` is inconsistent as it has the major version number encoded for BSD systems, which doesn't make much sense for e.g. OpenBSD, where there is no concept of major versions, but instead the version is simply increased by 0.1 on each release. Note that `os.name` is still useful to detect whether we are running a POSIX system (see `BitcoinTestFramework.skip_if_platform_not_posix`), so for this use-case it is kept as only exception. The following table shows values for common operating systems, found via ``` $ python3 -c "import os; import sys; import platform; print(os.name, sys.platform, platform.system())" ``` | OS | os.name | sys.platform | platform.system() | |--------------|---------|--------------|--------------------| | Linux 6.2.0 | posix | linux | Linux | | MacOS* | posix | darwin | Darwin | | OpenBSD 7.4 | posix | openbsd7 | OpenBSD | | Windows* | nt | win32 | Windows | \* = I neither have a MacOS nor a Windows machine available, so I extracted the values from documentation and our current code. Also I'm relying on CI for testing the relevant code-paths. Having reviewers to this this locally would be very appreciated, if this gets Concept ACKed. ACKs for top commit: kevkevinpal: ACK [878d914](https://github.com/bitcoin/bitcoin/pull/29034/commits/878d914777a03a04ecb84217152e8b7fd73a5062) achow101: ACK 878d914777a03a04ecb84217152e8b7fd73a5062 hebasto: ACK 878d914777a03a04ecb84217152e8b7fd73a5062, I have reviewed the code and it looks OK. pablomartin4btc: tACK 878d914777a03a04ecb84217152e8b7fd73a5062 Tree-SHA512: 24513d493e47f572028c843260b81c47c2c29bfb701991050255c9f9529cd19065ecbc7b3b6e15619da7f3f608b4825c345ce6fee30d8fd1eaadbd08cff400fc
2024-01-11Merge bitcoin/bitcoin#29222: doc: update Bitcoin Core license to 2024fanquake
1f8450f066724dfbb5c5bc4060843e2f3340ed88 doc: upgrade Bitcoin Core license to 2024 (22388o⚡️) Pull request description: See https://github.com/bitcoin/bitcoin/pull/26748. Cherry-picked these commits from 22388o and then squashed them. ACKs for top commit: fanquake: ACK 1f8450f066724dfbb5c5bc4060843e2f3340ed88 Tree-SHA512: 6d12f24a6c7cd421f7d975d7e445de4583144a4d0902d4e68e7648395074ca804e3ee585b2d3f307d193690e2ed2f9fbd8e7938f8fb8af263888b8701993782a
2024-01-11Merge bitcoin/bitcoin#28870: depends: Include `config.guess` and ↵fanquake
`config.sub` into `meta_depends` ff3f51b402efe6dc0b4bd14aecb9b58c2815c6e4 depends: Include `config.guess` and `config.sub` into `meta_depends` (Hennadii Stepanov) Pull request description: ACKs for top commit: theuni: ACK ff3f51b402efe6dc0b4bd14aecb9b58c2815c6e4. Tree-SHA512: e8575473d3fca2293181131c76bd6d43017fe753d2e670c53227a646b64b069dc542a0fc50a77b43e74bc6a0c0159ffa2fb1c3ff3aef9625684e0f78c16ad960
2024-01-11ci: Rename tasks (previous releases, macOS cross)MarcoFalke
2024-01-11Merge bitcoin/bitcoin#28838: test: add assumeutxo wallet testAva Chow
997b9a73e5166b4244f7c5b4fe144d524f3005f4 test: add assumeutxo wallet test (Sjors Provoost) Pull request description: Extracted from #28616, this adds a (very) basic wallet test for assume utxo. It checks some circumstances where a backup can and can't be loaded. ACKs for top commit: maflcko: lgtm ACK 997b9a73e5166b4244f7c5b4fe144d524f3005f4 achow101: ACK 997b9a73e5166b4244f7c5b4fe144d524f3005f4 theStack: Code-review ACK 997b9a73e5166b4244f7c5b4fe144d524f3005f4 Tree-SHA512: 69474e56c6a46bb4f30fc54f8e5844766ac2a5f8226bb0b168d11ae1e3d4eae58570c1f1b4cc2b2f6f51b5d0e055bbe2bbd11684265215e01d4eb81ab4b7b0bb
2024-01-11Merge bitcoin/bitcoin#29186: ci, iwyu: Drop backported mappingsfanquake
a395218d8cdffba375006590edca58c844126b16 ci, iwyu: Drop backported mappings (Hennadii Stepanov) Pull request description: See https://github.com/include-what-you-use/include-what-you-use/pull/1026. Split from https://github.com/bitcoin/bitcoin/pull/27710 as a non-controversial change. ACKs for top commit: fanquake: ACK a395218d8cdffba375006590edca58c844126b16 Tree-SHA512: ae7955a99396ab4f62ab7c989dba59c26448837f0ba4436bf3fddebe4099b5d3c03492e22a8497104c6afcceede1bb1b81f9d71c7c7e43692e6d70dcdfc11e7c
2024-01-11Merge bitcoin/bitcoin#29225: ci: move CMake into base packagesfanquake
f3ca6db8d349941f6bad6e8b328033222af8f063 ci: move CMake into base packages (fanquake) Pull request description: This is already used in multiple CIs, and will soon become a requirement for most CIs, i.e when we migrate depends packages to use CMake, for example: https://github.com/bitcoin/bitcoin/pull/21778#issuecomment-1885576324. Some of the CIs in 21778 are failing because CMake isn't available, so just break this out and make CMake globally available. ACKs for top commit: maflcko: lgtm ACK f3ca6db8d349941f6bad6e8b328033222af8f063 Tree-SHA512: b3daa82c1ead29600168b3f064dc65c8e632baa6e7efb5f2e87ba1e9130a48c31becdf0a89e6ede479c5b08dd97eb4c9cfd4cc10207235c601dd089d5b808b59
2024-01-11test: Assert that a new tx with a delta of 0 is never addedkevkevin
2024-01-11rpc: exposing modified_fee in getprioritisedtransactionskevkevin
Instead of having modified_fee be hidden we are now exposing it to avoid having useless code
2024-01-11Merge bitcoin/bitcoin#29219: fuzz: Improve fuzzing stability for ↵fanquake
ellswift_roundtrip harness 154fcce55c84c251fad8d280eafb3c0a5284fcd4 [fuzz] Improve fuzzing stability for ellswift_roundtrip harness (dergoegge) Pull request description: See #29018 ACKs for top commit: sipa: utACK 154fcce55c84c251fad8d280eafb3c0a5284fcd4 brunoerg: crACK 154fcce55c84c251fad8d280eafb3c0a5284fcd4 Tree-SHA512: 1e1ee47467a4a0d3a4e79f672018b440d8b3ccafba7428d37b9d0b8d3afd07e3f64f53ee668ed8a6a9ad1919422b5970814eaf857890acae7546951d8cb141d6
2024-01-11ci: move CMake into base packagesfanquake
This is already used in multiple CIs, and will soon become a requirement for most CIs, i.e when we migrate depends packages to use CMake, for example: https://github.com/bitcoin/bitcoin/pull/21778#issuecomment-1885576324. Some of the CIs in 21778 are failing because CMake isn't available, so just break this out and make CMake globally available.
2024-01-11Merge bitcoin/bitcoin#29127: Use hardened runtime on macOS release builds.fanquake
4fdd836db92e789c98b9e68398ca931a968cc9c3 Use hardened runtime on macOS release builds. (Mark Friedenbach) Pull request description: The Apple notary service requires submitted app bundles to be configured to use the hardened runtime libraries. This is configured at signing time, and supported by the signapple tool Bitcoin Core uses for reproduceable signed binaries. We simply need to pass "--hardened-runtime" when the signature is created. Once attached to the bundle, the resulting codesigned binary can be successfully submitted to the Apple binary notarization service by any Apple Developer. This partially resolves #15774. The release maintainer, or any authorized Apple Developer, will need to run `xcrun notarytool` to prevent gatekeeper warnings on macOS. Using `xcrun staple` to generate a binary that doesn't call home on first launch would be bonus, but at least this would massively improve the user experience. ACKs for top commit: fanquake: ACK 4fdd836db92e789c98b9e68398ca931a968cc9c3 - we can move ahead with this, and figure out notarisation / stapling as a followup. Tree-SHA512: 7b8ba50030fb230d44bd63d12ed082537e8eaaa61396114c5df715f8dd6772fd8d84b00dc819f88d9a463996c2170a84981fce1bde7f7999b4bdb914fbcdfdac
2024-01-10doc: upgrade Bitcoin Core license to 202422388o⚡️
2024-01-10Merge bitcoin/bitcoin#29204: test: wallet migration, add coverage for tx ↵Ava Chow
extra data 016cc807f77a9128d430a0df1edd133521628a33 test: wallet migration, add coverage for tx extra data (furszy) Pull request description: Quick follow-up to #28610, coming from https://github.com/bitcoin/bitcoin/pull/28610#pullrequestreview-1802823938. Verifying that the 'replaced_by_txid' and 'replaces_txid' tx data is preserved after migration, as well as the extra tx comments. ACKs for top commit: jamesob: Nice, ACK https://github.com/bitcoin/bitcoin/pull/29204/commits/016cc807f77a9128d430a0df1edd133521628a33 achow101: ACK 016cc807f77a9128d430a0df1edd133521628a33 pablomartin4btc: ACK 016cc807f77a9128d430a0df1edd133521628a33 BrandonOdiwuor: lgtm ACK 016cc807f77a9128d430a0df1edd133521628a33 Tree-SHA512: 697cabece730cbe5c5947bf98455e80a8877c0352fbe2a66362ce5ea530b67882b0bec561a67d48fee200cdad717cd62c57fd809e2a94ff83c3fad30021e1d9e
2024-01-10Merge bitcoin/bitcoin#29211: fuzz: fix `connman` initializationAva Chow
e84dc36733687fffe08ae4621b571cc66afc047d fuzz: fix `connman` initialization (brunoerg) Pull request description: Fixes https://github.com/bitcoin/bitcoin/pull/29172#issuecomment-1883547121 ACKs for top commit: achow101: ACK e84dc36733687fffe08ae4621b571cc66afc047d Tree-SHA512: e5f3c378cfe367cc4c387fa1b13663a74d8b667a5d130d62919e21455861cfb9383b63ef4ebe56daab7b2c09e3b5031acc463065455f71607c5fb9e3c370d3ad
2024-01-10Merge bitcoin/bitcoin#28318: logging: Simplify API for level based loggingAva Chow
e60fc7d5d34f23cccbff6e4f5f3d716fa8dad50c logging: Replace uses of LogPrintfCategory (Anthony Towns) f7ce5ac08c669ac763e275bb7c82dcfb2b1b6c33 logging: add LogError, LogWarning, LogInfo, LogDebug, LogTrace (Anthony Towns) fbd7642c8e5b70327e019382320f5ef0a651ecc5 logging: add -loglevelalways=1 option (Anthony Towns) 782bb6a05663ad7a53908e910d0f42b49b881e09 logging: treat BCLog::ALL like BCLog::NONE (Anthony Towns) 667ce3e3297645527b07314e1d5a82275fb25845 logging: Drop BCLog::Level::None (Anthony Towns) ab34dc6012351e7b8aab871dd9d2b38ade1cd9bc logging: Log Info messages unconditionally (Anthony Towns) dfe98b6874da04e45f68d17575c1e8a5431ca9bc logging: make [cat:debug] and [info] implicit (Anthony Towns) c5c76dc615677d226c9f6b3f2b66d833315d40da logging: refactor: pull prefix code out (Anthony Towns) Pull request description: Replace `LogPrint*` functions with severity based logging functions: * `LogInfo(...)`, `LogWarning(...)`, `LogError(...)` for unconditional (uncategorised) logging (replaces `LogPrintf`) * `LogDebug(CATEGORY, ...)` and `LogTrace(CATEGORY, ...)` for conditional logging (replaces `LogPrint`) * `LogPrintLevel(CATEGORY, LEVEL, ...)` for when the level isn't known in advance, or a category needs to be added for an info/warning/error log message (mostly unchanged, but rarely needed) Logs look roughly as they do now with `LogInfo` not having an `[info]` prefix, and `LogDebug` having a `[cat]` prefix, rather than a `[cat:debug]` prefix. This removes `BCLog::Level::None` entirely -- for `LogFlags::NONE` just use `Level::Info`, for any actual category, use `Level::Debug`. Adds docs to developer-notes about when to use which level. Adds `-loglevelalways=1` option so that you get `[net:debug]`, `[all:info]`, `[all:warning]` etc, which might be helpful for automated parsing, or just if you like everything to be consistent. Defaults to off to reduce noise in the default config, and to avoid unnecessary changes on upgrades. Changes the behaviour of `LogPrintLevel(CATEGORY, BCLog::Level::Info, ...)` to be logged unconditionally, rather than only being an additional optional logging level in addition to trace and debug. Does not change the behaviour of `LogPrintLevel(NONE, Debug, ...)` and `LogPrintLevel(NONE, Trace, ...)` being no-ops. ACKs for top commit: maflcko: re-ACK e60fc7d5d34f23cccbff6e4f5f3d716fa8dad50c 🌚 achow101: ACK e60fc7d5d34f23cccbff6e4f5f3d716fa8dad50c stickies-v: ACK e60fc7d5d34f23cccbff6e4f5f3d716fa8dad50c jamesob: ACK e60fc7d5d34f23cccbff6e4f5f3d716fa8dad50c ([`jamesob/ackr/28318.1.ajtowns.logging_simplify_api_for`](https://github.com/jamesob/bitcoin/tree/ackr/28318.1.ajtowns.logging_simplify_api_for)) Tree-SHA512: e7a4588779b148242495b7b6f64198a00c314cd57100affab11c43e9d39c9bbf85118ee2002792087fdcffdea08c84576e20844b3079f27083e26ddd7ca15d7f
2024-01-10Merge bitcoin/bitcoin#29215: test: assumeutxo: spend coin from snapshot ↵glozow
chainstate after loading 931575418e082af37b88b1819125b0d0f0fabbd5 test: assumeutxo: spend coin from snapshot chainstate after loading (Sebastian Falbesoner) Pull request description: This PR extends the AssumeUTXO functional test by submitting a spending transaction for an UTXO that is only available in a the snapshot chainstate (after loading via `loadtxoutset`), i.e. it hasn't been seen in a block before. With that we can verify that snapshot coins are visible to the mempool. Note that we unfortunately can't use MiniWallet here, as the only available UTXO to spend from the snapshot chainstate is at height 200, where a P2PKH created from the test framework's deterministic private key is used (see `TestNode.generate(...)` and the `PRIV_KEYS` array). Coinbase outputs with smaller heights (<= 199) would be part of the pre-generated chain and hence not qualify for the "UTXO is only in snapshot chainstate and has never been seen in a block" scenario, coinbase outputs with larger heights (>= 201) can't be spent due to immaturity, as the snapshot chainstate block height is 299. One could of course mine a different chain with outputs that MiniWallet supports (e.g. taproot anyone-can-spend), but this would change the hardcoded AssumeUTXO hash, colliding with other PRs like #28838, so I wanted to avoid that. ACKs for top commit: maflcko: lgtm ACK 931575418e082af37b88b1819125b0d0f0fabbd5 jamesob: ACK https://github.com/bitcoin/bitcoin/pull/29215/commits/931575418e082af37b88b1819125b0d0f0fabbd5 Tree-SHA512: 0665868e1e91fe74f408d0a239cc264bbbc11a6b55bcc0e86cc8b4b2ec1f44977884b817dbe9065a7c768332cab464636656858bc8b9c8e7d7810498e0a17d78
2024-01-10[fuzz] Improve fuzzing stability for ellswift_roundtrip harnessdergoegge
`CPubKey::VerifyPubKey` uses rng internally which leads to instability in the fuzz test. We fix this by avoiding `VerifyPubKey` in the test and verifying the decoded public key with a fuzzer chosen message instead.
2024-01-10test: assumeutxo: spend coin from snapshot chainstate after loadingSebastian Falbesoner
Check that an UTXO that is only available in the snapshot chainstate is also visible to the mempool by submitting a spending transaction.
2024-01-09Fix -netinfo backward compat with getpeerinfo pre-v26Jon Atack
CLI -netinfo will currently break when calling it on a node that is running pre-v26 bitcoind, as `getpeerinfo` doesn't yet return a transport_protocol_type field. Fix this by adding an `IsNull()` check as already done for other fields, and also: - avoid checking for the full string "detecting", and instead do the cheaper check for the most frequent case of the string starting with "v" - drop displaying the "v" prefix in all the rows, as it doesn't add useful information, and instead use "v" for the column header - display nothing during peer setup, like for the -netinfo mping and ping columns
2024-01-09fuzz: fix `connman` initializationbrunoerg
2024-01-09Merge bitcoin/bitcoin#29058: net, cli: use v2transport for manual/addrfetch ↵Ava Chow
connections, add to -netinfo fb5bfed26a564014b83ccfc96ff00b630930fc61 cli: add transport protcol column to -netinfo (Martin Zumsande) 9eed22e870e650cadf5f65650917da21836d2bb0 net: attempt v2 transport for addrfetch connections if we support it (Martin Zumsande) 770c0311ef5e35444efe4fd26f7bb5782624cf2c net: attempt v2 transport for manual connections if we support it (Martin Zumsande) Pull request description: Some preparations before enabling `-v2transport` as the default: * Use v2 for `-connect`, `-addnode` config arg and `-seednode` if `-v2transport` is enabled. Our peer may or may not support v2, but I don't think an extra option is necessary for any of these (we have that for the `addnode` rpc), because we have the reconnection mechanism that will try again with `v1` if our peer doesn't support `v2`. * Add a column for the transport protocol to `-netinfo`. I added it next to the `net` column because I thought it looked nice there, but if people prefer it somewhere else I'm happy to move it. ![Screenshot from 2023-12-11 17-51-22](https://github.com/bitcoin/bitcoin/assets/48763452/b4f5dfcb-16be-4d8f-9303-9d342123deec) ACKs for top commit: sipa: utACK fb5bfed26a564014b83ccfc96ff00b630930fc61 achow101: ACK fb5bfed26a564014b83ccfc96ff00b630930fc61 stratospher: tested ACK fb5bfed. addrfetch + manual connections aren't frequent and it would be useful to have this for transition to v2 one day. theStack: ACK fb5bfed26a564014b83ccfc96ff00b630930fc61 kristapsk: ACK fb5bfed26a564014b83ccfc96ff00b630930fc61 Tree-SHA512: c4575ad11b99613870b342acae369fa08f877ac79e6e04eb62e94ad7a92d528e289183c0963c78aa779ba11cb91e2a6fad7c8b0d813126c46c3e5b54bd962c26
2024-01-09Merge bitcoin/bitcoin#29200: net: create I2P sessions using both ↵fanquake
ECIES-X25519 and ElGamal encryption 9d728916b27e18efc6f8839770ed5ec14789fc08 net: create I2P sessions with both ECIES-X25519 and ElGamal encryption (Jon Atack) Pull request description: A Bitcoin Core node may only connect to a peer destination via I2P if both sides have sessions with the same encryption type. Encryption type is a property of the session, not the destination. Sessions may support multiple encryption types. As Bitcoin Core is not currently setting the encryption type when creating I2P sessions, it uses the older default, ElGamal (type 0). This pull updates our I2P session creation to use both ECIES-X25519 and ElGamal (types 4 and 0, respectively). This allows to connect to I2P peers of either type, and the newer, faster ECIES-X25519 will be preferred. See also: - discussion around https://github.com/qbittorrent/qBittorrent/issues/19625#issuecomment-1879582395 - recently updated "Signature and Encryption Types" in https://geti2p.net/en/docs/api/samv3 Thank you and credit to zzzi2p for reporting and to vort for the patch. Closes https://github.com/bitcoin/bitcoin/issues/29197. ACKs for top commit: zzzi2p: ACK 9d728916b27e18efc6f8839770ed5ec14789fc08 recursive-rat4: ACK 9d728916b27e18efc6f8839770ed5ec14789fc08 kristapsk: cr utACK 9d728916b27e18efc6f8839770ed5ec14789fc08 brunoerg: crACK 9d728916b27e18efc6f8839770ed5ec14789fc08 shaavan: crACK 9d728916b27e18efc6f8839770ed5ec14789fc08 Tree-SHA512: 0912fc01af9706914a7854f7479b9d82fc86c9530466cad8674e30f7eb4894d90d514efbc1aee8b7ea690faa6ff4a23b62cf5de8737cffdbc463300082c9b917
2024-01-09Merge bitcoin/bitcoin#29203: build: Drop `ALLOW_HOST_PACKAGES` support in ↵fanquake
depends 080763a058f399583deb9dea6687e87fc1c097a9 build: Drop `ALLOW_HOST_PACKAGES` support in depends (Hennadii Stepanov) Pull request description: The `ALLOW_HOST_PACKAGES` variable was introduced in bitcoin#10508 "to speed up build and avoid timeout". It is no longer the case for our CI infrastructure, which uses self- hosted persistent workers and depends caching. In the current circumstances, it does not seem worth porting this feature to the upcoming [CMake-based](https://github.com/bitcoin/bitcoin/issues/28607) build system. ACKs for top commit: fanquake: ACK 080763a058f399583deb9dea6687e87fc1c097a9 - I can't imagine this option got any use outside our CI. It's also mostly just at odds with the idea of a self-contained dependency builder. TheCharlatan: ACK 080763a058f399583deb9dea6687e87fc1c097a9 Tree-SHA512: 36f52690be913479c5d12be36760b8de1a6e891fe7c2cf98a7b8d6561006a6b18631e431351d79e97edb9409f9902d032aedf7b963aa7615e54b59fc2a58f7d6
2024-01-09fuzz: Assume presence of __builtin_*_overflow, without checksMarcoFalke
2024-01-09Revert "build: Fix undefined reference to __mulodi4"MarcoFalke
This reverts commit e4c8bb62e4a6873c45f42d0d2a24927cb241a0ea.
2024-01-09build: Bump clang minimum supported version to 14MarcoFalke
2024-01-09Merge bitcoin/bitcoin#29195: build: Fix `-Xclang -internal-isystem` optionfanquake
d742be3d3f5d5063d7160f72422bce2fec953f38 ci: Switch native macOS CI job to Xcode 15.0 (Hennadii Stepanov) 8decc5c726caca2381cffbd1b3585862421f5b8e build: Fix `-Xclang -internal-isystem` option (Hennadii Stepanov) Pull request description: This PR: - addresses https://github.com/bitcoin/bitcoin/pull/29165#discussion_r1439433156 - fixes https://github.com/bitcoin/bitcoin/issues/29174 ACKs for top commit: fanquake: ACK d742be3d3f5d5063d7160f72422bce2fec953f38. The same as what was done in #27328. Tree-SHA512: 4788a0511e9fac638edab8e4f7ec62c5e08aeb07e518ab62fd53074ab3dd4eca1f62dc17c2af2b535bad12e77a7437e5c1c714cd03ce711e5d5e5c87d4620358
2024-01-09Merge bitcoin/bitcoin#29172: fuzz: set `nMaxOutboundLimit` in connman targetfanquake
e5b9ee0221ec8aa238fb5720fcd6faa01b09fe46 fuzz: set `nMaxOutboundLimit` in connman target (brunoerg) Pull request description: Setting `nMaxOutboundLimit` (`-maxuploadtarget`) will make fuzz to reach more coverage in connman target. This value is used in `GetMaxOutboundTimeLeftInCycle`, `OutboundTargetReached` and `GetOutboundTargetBytesLeft`. ACKs for top commit: dergoegge: utACK e5b9ee0221ec8aa238fb5720fcd6faa01b09fe46 jonatack: ACK e5b9ee0221ec8aa238fb5720fcd6faa01b09fe46 Tree-SHA512: d19c83602b0a487e6da0e3be539aa2abc95b8bbf36cf9a3e391a4af53b959f68ca38548a96d27d56742e3b772f648da04e2bf8973dfc0ab1cdabf4f2e8d44de6
2024-01-08rpc: renaming txid -> transactionidkevkevin
renamed to transactionid because it is named this way in getrawmempool and getmempoolancestors
2024-01-08rpc: changed prioritisation-map -> ""kevkevin
prioritisation-map gets eaten by the help generator to be "" so we are setting to "" to begin with
2024-01-08build: Drop `ALLOW_HOST_PACKAGES` support in dependsHennadii Stepanov
The `ALLOW_HOST_PACKAGES` variable was introduced in bitcoin#10508 "to speed up build and avoid timeout". It is no longer the case for our CI infrastructure, which uses self- hosted persistent workers and depends caching. In the current circumstances, it does not seem worth porting this feature to the upcoming CMake-based build system.
2024-01-08test: wallet migration, add coverage for tx extra datafurszy
Verifying that the 'replaced_by_txid' and 'replaces_txid' tx data is preserved after migration, as well as the extra tx comments.
2024-01-08Merge bitcoin/bitcoin#28610: wallet: Migrate entire address book entries to ↵fanquake
watchonly and solvables too 406b71abcb72f234ddf9245a3f57e748343c774f wallet: Migrate entire address book entries (Andrew Chow) Pull request description: Not all of the data in an address book entry was being copied to the watchonly and solvables wallets. This includes information such as whether the address was previously spent, and any receive requests that may exist. A test has been added to check that the previously spent information is copied, although it passes without the changes in this PR since this information is also regenerated when a transaction is loaded/added into a wallet. ACKs for top commit: ryanofsky: Code review ACK 406b71abcb72f234ddf9245a3f57e748343c774f. Just suggested change since last review furszy: Code review ACK 406b71ab Tree-SHA512: 13de42b16a1d8524fe0555764744139566b2e7d29741ceffc1158a905dd537136b762330568b3b5cac28cbee1bfd363a20de97d0a6c5296738cb3aa99133945b