aboutsummaryrefslogtreecommitdiff
AgeCommit message (Collapse)Author
2023-11-09ci: remove "--exclude banman" for fuzzing in macbrunoerg
2023-11-09fuzz: call lookup functions before calling `Ban`brunoerg
Also, compare banmaps only if there are no invalid entries.
2023-10-30Merge bitcoin/bitcoin#28741: refactor: Fix bugprone-string-constructor warningfanquake
fa56067a8f56701cbda95595592e74934af7d1cd refactor: Fix bugprone-string-constructor warning (MarcoFalke) Pull request description: String literals in C++ have a trailing null character, so the current code is fine to rely on that implicitly. However, * the sqlite documentation explicitly mentions the null character * code readers may wonder if the code is intentional * clang-tidy warns about the code via `bugprone-string-constructor` Address the points by putting the null character into the code and enable the clang-tidy `bugprone-string-constructor` check. ACKs for top commit: stickies-v: ACK fa56067a8f56701cbda95595592e74934af7d1cd Tree-SHA512: da519184d792a885a8151ffc44c8da5781f5aaae12ef768a187cc6d9e542ca8952aebc2ec6c1a05f673f29a86ef44902ee96e7b491af7b4705ad38e14624882e
2023-10-30Merge bitcoin/bitcoin#28348: build: Bump g++ minimum supported version to 10fanquake
fa5423b5b582aaf8c5b7c516806157244cf20c74 refactor: Remove unused gcc-9 workaround in txrequest (MarcoFalke) fa918d397dd97114a00ca3d6297f1bb293a8de08 Always enable -Wsuggest-override (MarcoFalke) faea58eee412d36a020aecf5568e4e3e1f0c5b22 Bump g++ minimum supported version to 10 (MarcoFalke) Pull request description: All supported operating systems ship with g++ 10 (or later), so bumping the minimum should not cause any issues. The bump allows to drop some now-unused workarounds. For reference: * https://packages.debian.org/bullseye/g++ (`g++-10`) * https://packages.ubuntu.com/focal/g++-10 * FreeBSD 12/13 ships with g++ 12 * CentOS-like 9 ships with g++ 11 * OpenSuse Tumbleweed ships with g++ 13 https://software.opensuse.org/package/gcc13-c++ (No idea about OpenSuse Leap) This is for Bitcoin Core 27.0 in 2024 (next year), not the soon upcoming 26.0 next month. ACKs for top commit: fanquake: ACK fa5423b5b582aaf8c5b7c516806157244cf20c74 Tree-SHA512: 6f0697ae4c0f578873591b7872bf158aba3af17f171c3556b593a70ec379bf94c7a9dd7697e8e79173edd4ac3c81a376e0cbbc0cfabde1a1cfe5f9b5eaea6831
2023-10-30refactor: Remove unused gcc-9 workaround in txrequestMarcoFalke
2023-10-30Always enable -Wsuggest-overrideMarcoFalke
2023-10-30Bump g++ minimum supported version to 10MarcoFalke
Also, enable -Werror=maybe-uninitialized in ci/test/00_setup_env_native_qt5.sh
2023-10-30refactor: Fix bugprone-string-constructor warningMarcoFalke
2023-10-30Merge bitcoin/bitcoin#28737: doc: Fix bugprone-lambda-function-name errorsfanquake
faa769db5a4c16fd171e9a39c33e245db4e7c134 Fix bugprone-lambda-function-name errors (MarcoFalke) Pull request description: Inside a lambda, `__func__` will evaluate to something like `"operator()"`. Fix this by either removing it, or by using the real name. https://clang.llvm.org/extra/clang-tidy/checks/bugprone/lambda-function-name.html ACKs for top commit: TheCharlatan: ACK faa769db5a4c16fd171e9a39c33e245db4e7c134 darosior: utACK faa769db5a4c16fd171e9a39c33e245db4e7c134 Tree-SHA512: 0b562bd4ebd7f46ca3ebabeee67851ad30bd522fa57e5010e833b163664e51f5df645ff9ca35d22c3479fb27d9267d4e5d0d417d42729bf3ccf80d7944970e4e
2023-10-30Merge bitcoin/bitcoin#28695: net: Sanity check private keys received from ↵fanquake
SAM proxy 5cf4d266d9b1e7bd9394e7581398de5bc540ae99 [test] Test i2p private key constraints (Vasil Dimov) cf70a8d56510a5f07eff0fd773184cae14b2dcc9 [net] Check i2p private key constraints (dergoegge) Pull request description: Not sanity checking can lead to crashes or worse: ``` ==1715589==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x6140000055c2 at pc 0x5622ed66e7ad bp 0x7ffee547a2c0 sp 0x7ffee547a2b8 READ of size 2 at 0x6140000055c2 thread T0 (b-test) #0 0x5622ed66e7ac in memcpy include/bits/string_fortified.h:29:10 #1 0x5622ed66e7ac in i2p::sam::Session::MyDestination() const src/i2p.cpp:362:5 #2 0x5622ed662e46 in i2p::sam::Session::CreateIfNotCreatedAlready() src/i2p.cpp:414:40 #3 0x5622ed6619f2 in i2p::sam::Session::Listen(i2p::Connection&) src/i2p.cpp:143:9 ``` ACKs for top commit: maflcko: code lgtm ACK 5cf4d266d9b1e7bd9394e7581398de5bc540ae99 stickies-v: re-ACK 5cf4d266d9b1e7bd9394e7581398de5bc540ae99 vasild: ACK 5cf4d266d9b1e7bd9394e7581398de5bc540ae99 Tree-SHA512: 3de3bd396538fa619de67957b9c8a58011ab911f0f51097c387e730c13908278b7322aa3357051fb245a20b15bef34b0e9fadcb1eff8ad751139d2aa634c78ad
2023-10-30Merge bitcoin/bitcoin#28753: test: Remove feature_txindex_compatibility.pyfanquake
897d6dd42b78ab8569cbd7ae5e27f277b9acaeff Remove feature_txindex_compatibility.py in V27 (Brandon Odiwuor) Pull request description: Fixes https://github.com/bitcoin/bitcoin/issues/28421, see [#28195 (comment)](https://github.com/bitcoin/bitcoin/pull/28195/commits/fa8685597e7302fc136f21b6dd3a4b187fa8e251#r1311494362) Remove feature_txindex_compatibility.py in V27, follow up to https://github.com/bitcoin/bitcoin/pull/28195 being merged which is included in v26 ACKs for top commit: maflcko: lgtm ACK 897d6dd42b78ab8569cbd7ae5e27f277b9acaeff theStack: ACK 897d6dd42b78ab8569cbd7ae5e27f277b9acaeff stickies-v: ACK 897d6dd42b78ab8569cbd7ae5e27f277b9acaeff Tree-SHA512: 53102d39f6fdbdcf1bb13b6feb2f446b0e9e8e3fe294c0e6fe37e7731713fb9fd5b048e19b6edf80579f5edbcf762b51d56d57bdcda67ec3527706891dc3572b
2023-10-30[test] Test i2p private key constraintsVasil Dimov
2023-10-30Remove feature_txindex_compatibility.py in V27Brandon Odiwuor
2023-10-29Merge bitcoin/bitcoin#28698: assumeutxo, blockstorage: Prevent core dump on ↵fanquake
invalid hash 811067ca1cbbd4a697791cbe3ecd4bee19fe6193 test: add coverage for snapshot chainstate not matching AssumeUTXO parameters (pablomartin4btc) 4a5be10b928d4ed33d223972537c1cb79163e79c assumeutxo, blockstorage: prevent core dump on invalid hash (pablomartin4btc) Pull request description: While reviewing #27596 (ran `loadtxoutset` in `mainnet` before `m_assumeutxo_data` is empty as [currently](https://github.com/jamesob/bitcoin/blob/434495a8c1496ca23fe35b84499f3daf668d76b8/src/kernel/chainparams.cpp#L175-L177) in master - back to 1b1d711), got a `core dumped`, so it seems there's a potential issue if new releases ever remove snapshot details or a semi-experienced user performs a `loadtxoutset` on a different "customised" binary version (not sure if this is a real use case). ``` 2023-10-18T17:42:52Z [init] Using obfuscation key for /tmp/.test_utxo_2/blocks/index: 0000000000000000 node/blockstorage.cpp:390 LoadBlockIndex: Assertion `GetParams().AssumeutxoForBlockhash(*snapshot_blockhash)' failed. Aborted (core dumped) ``` <details> <summary>This is also happening before IBD is completed (<code>background validation</code> still being performed as it can be seen in rpc <code>getchainstates</code>)</summary> ``` /src/bitcoin-cli -datadir=${AU_DATADIR} getchainstates { "headers": 813097, "chainstates": [ { "blocks": 368249, "bestblockhash": "00000000000000000b7a08224a1cb00d337100ba7a46c03d04b2c2d8964efc37", "difficulty": 52278304845.59168, "verificationprogress": 0.086288278873286, "coins_db_cache_bytes": 7969177, "coins_tip_cache_bytes": 14908338995, "validated": true }, { "blocks": 813097, "bestblockhash": "0000000000000000000270c9fdce7b17db64cca91f90106964b58e33a4d91089", "difficulty": 61030681983175.59, "verificationprogress": 0.999997140098457, "coins_db_cache_bytes": 419430, "coins_tip_cache_bytes": 784649420, "snapshot_blockhash": "00000000000000000002a7c4c1e48d76c5a37902165a270156b7a8d72728a054", "validated": false } ] } ``` </details> <details> <summary>Steps to reproduce the core dump error and its output:</summary> 1. Perform a `loadtxoutset` in `mainnet` on compiled `bitcoind` adding the block hash from Sjors's [commit](https://github.com/Sjors/bitcoin/commit/24deb2022b822f22fba9fcbee201e37a83225eb2). 2. Once step 1 finishes, remove the added code from step 1 and compile again or just compile `master` without any changes on top. 3. Run `bitcoind`, soon it'll crash with: ``` 2023-10-18T17:42:52Z [init] init message: Loading block index… 2023-10-18T17:42:52Z [init] Assuming ancestors of block 00000000000000000001a0a448d6cf2546b06801389cc030b2b18c6491266815 have valid signatures. 2023-10-18T17:42:52Z [init] Setting nMinimumChainWork=000000000000000000000000000000000000000052b2559353df4117b7348b64 2023-10-18T17:42:52Z [init] Prune configured to target 3000 MiB on disk for block and undo files. 2023-10-18T17:42:52Z [init] [snapshot] detected active snapshot chainstate (/tmp/.test_utxo_2/chainstate_snapshot) - loading 2023-10-18T17:42:52Z [init] [snapshot] switching active chainstate to Chainstate [snapshot] @ height -1 (null) 2023-10-18T17:42:52Z [init] Opening LevelDB in /tmp/.test_utxo_2/blocks/index 2023-10-18T17:42:52Z [init] Opened LevelDB successfully 2023-10-18T17:42:52Z [init] Using obfuscation key for /tmp/.test_utxo_2/blocks/index: 0000000000000000 node/blockstorage.cpp:390 LoadBlockIndex: Assertion `GetParams().AssumeutxoForBlockhash(*snapshot_blockhash)' failed. Aborted (core dumped) ``` </details> <details> <summary>After original change, error message output:</summary> ``` 2023-10-20T15:49:12Z [init] init message: Loading block index… 2023-10-20T15:49:12Z [init] Assuming ancestors of block 00000000000000000001a0a448d6cf2546b06801389cc030b2b18c6491266815 have valid signatures. 2023-10-20T15:49:12Z [init] Setting nMinimumChainWork=000000000000000000000000000000000000000052b2559353df4117b7348b64 2023-10-20T15:49:12Z [init] Prune configured to target 3000 MiB on disk for block and undo files. 2023-10-20T15:49:12Z [init] [snapshot] detected active snapshot chainstate (/tmp/.test_utxo_2/chainstate_snapshot) - loading 2023-10-20T15:49:12Z [init] [snapshot] switching active chainstate to Chainstate [snapshot] @ height -1 (null) 2023-10-20T15:49:12Z [init] Opening LevelDB in /tmp/.test_utxo_2/blocks/index 2023-10-20T15:49:12Z [init] Opened LevelDB successfully 2023-10-20T15:49:12Z [init] Using obfuscation key for /tmp/.test_utxo_2/blocks/index: 0000000000000000 2023-10-20T15:49:13Z [init] *** Assumeutxo data not found for the given blockhash '00000000000000000002a7c4c1e48d76c5a37902165a270156b7a8d72728a054'. 2023-10-20T15:49:13Z [init] Error: Assumeutxo data not found for the given blockhash '00000000000000000002a7c4c1e48d76c5a37902165a270156b7a8d72728a054'. Error: Assumeutxo data not found for the given blockhash '00000000000000000002a7c4c1e48d76c5a37902165a270156b7a8d72728a054'. 2023-10-20T15:49:13Z [init] Shutdown requested. Exiting. 2023-10-20T15:49:13Z [init] Shutdown: In progress... 2023-10-20T15:49:13Z [scheduler] scheduler thread exit 2023-10-20T15:49:13Z [shutoff] Flushed fee estimates to fee_estimates.dat. 2023-10-20T15:49:13Z [shutoff] Shutdown: done ``` </details> <details> <summary>Alternative on error handling using <code>return error()</code> instead of <code>return FatalError()</code> used in this PR, which produces a different output and perhaps confusing:</summary> ``` 2023-10-20T21:45:58Z [init] Using obfuscation key for /tmp/.test_utxo_2/blocks/index: 0000000000000000 2023-10-20T21:45:59Z [init] ERROR: Assumeutxo data not found for the given blockhash '00000000000000000002a7c4c1e48d76c5a37902165a270156b7a8d72728a054'. 2023-10-20T21:45:59Z [init] : Error loading block database. Please restart with -reindex or -reindex-chainstate to recover. : Error loading block database. Please restart with -reindex or -reindex-chainstate to recover. 2023-10-20T21:45:59Z [init] Aborted block database rebuild. Exiting. 2023-10-20T21:45:59Z [init] Shutdown: In progress... 2023-10-20T21:45:59Z [scheduler] scheduler thread exit 2023-10-20T21:45:59Z [shutoff] Flushed fee estimates to fee_estimates.dat. 2023-10-20T21:45:59Z [shutoff] Shutdown: done ``` </details> <details> <summary>Current state (including ryanofsky <a href="https://github.com/bitcoin/bitcoin/pull/28698#discussion_r1368635965">suggestion</a>), after code change, error message output:</summary> ``` 2023-10-25T02:29:57Z [init] Using obfuscation key for /home/pablo/.test_utxo_2/regtest/blocks/index: 0000000000000000 2023-10-25T02:29:57Z [init] *** Assumeutxo data not found for the given blockhash 'f09b5835f3f8b39481f2af3257bbc2e82845552d4d2d6d31cf520fc24263ed5b'. 2023-10-25T02:29:57Z [init] Error: A fatal internal error occurred, see debug.log for details Error: A fatal internal error occurred, see debug.log for details 2023-10-25T02:29:57Z [init] Shutdown requested. Exiting. 2023-10-25T02:29:57Z [init] Shutdown: In progress... 2023-10-25T02:29:57Z [scheduler] scheduler thread exit 2023-10-25T02:29:57Z [shutoff] Flushed fee estimates to fee_estimates.dat. 2023-10-25T02:29:57Z [shutoff] Shutdown: done ``` </details> ACKs for top commit: naumenkogs: ACK 811067ca1cbbd4a697791cbe3ecd4bee19fe6193 theStack: ACK 811067ca1cbbd4a697791cbe3ecd4bee19fe6193 ryanofsky: Code review ACK 811067ca1cbbd4a697791cbe3ecd4bee19fe6193. Tree-SHA512: cfc137b0a4f638b99fd7dac2c35cc729ef71ae1166a2a8960a91055ec90841cb33aed589834012cfe0e157937e2a76a88d1020ea1df2bc98e1114eb1fc8eaae4
2023-10-29Merge bitcoin/bitcoin#28727: test: replace random_bytes with random.randbytesfanquake
fe3ac3700d31a6329056fee983d91dd8e4c987c1 test: replace random_bytes with randbytes #28720 (ns-xvrn) Pull request description: With Python upgraded to 3.9 replaced the `random_bytes` function in util of functional tests and replaced it's usage with `random.randbytes`. Closes #28720. ACKs for top commit: maflcko: lgtm ACK fe3ac3700d31a6329056fee983d91dd8e4c987c1 BrandonOdiwuor: ACK fe3ac3700d31a6329056fee983d91dd8e4c987c1 stickies-v: ACK fe3ac3700d31a6329056fee983d91dd8e4c987c1, thanks for picking this up kristapsk: utACK fe3ac3700d31a6329056fee983d91dd8e4c987c1 Tree-SHA512: f65a75e73ebd840c2936eb133d42bccd552f25b717c8ca25c18d06e0593e12f292389cfcc0a0b0759004b67a46ea0c8ac237973ef90f246139778230be1e64e1
2023-10-29Merge bitcoin/bitcoin#28740: refactor: Add LIFETIMEBOUND to all (w)txid gettersfanquake
faec889f938f90e0b887426db27a15ec0d169399 refactor: Add LIFETIMEBOUND to all (w)txid getters (MarcoFalke) Pull request description: Currently some getters return a reference, some don't. Fix this by returning a reference everywhere. Also, add `LIFETIMEBOUND` to all. Then, use the compiler warnings to create copies only where needed. Also, fix iwyu includes while touching the includes. ACKs for top commit: dergoegge: Code review ACK faec889f938f90e0b887426db27a15ec0d169399 stickies-v: ACK faec889f938f90e0b887426db27a15ec0d169399 pablomartin4btc: cr ACK faec889f938f90e0b887426db27a15ec0d169399 Tree-SHA512: 0c2a151f39d0e007b4d33b0b85ad578cc220f3e9dd94890e812b3181c3901545b039325707731cc39a5e89557f59c1154c6320525f78f5de95f119a514d2d23f
2023-10-27refactor: Add LIFETIMEBOUND to all (w)txid gettersMarcoFalke
Then, use the compiler warnings to create copies only where needed. Also, fix iwyu includes while touching the includes.
2023-10-26Merge bitcoin/bitcoin#27116: doc: clarify that LOCK() internally checks ↵Andrew Chow
whether the mutex is held 91d08889218e06631f43a3dab0bae576aa46e43c sync: unpublish LocksHeld() which is used only in sync.cpp (Vasil Dimov) 3df37e0c78c3d5139c963a74eda56c331355ef72 doc: clarify that LOCK() does AssertLockNotHeld() internally (Vasil Dimov) Pull request description: Constructs like ```cpp AssertLockNotHeld(m); LOCK(m); ``` are equivalent to (almost, modulo some logging differences, see below) ```cpp LOCK(m); ``` for non-recursive mutexes, so it is ok to omit `AssertLockNotHeld()` in such cases. Requests to do the former keep coming during review process. `developer-notes.md` explicitly states "Combine annotations in function declarations with run-time asserts in function definitions", but that seems to be too strong or unclear. `LOCK()` is also a run-time assert in this case. Also remove `LocksHeld()` from the public interface in `sync.h` since it is only used in `sync.cpp`. ACKs for top commit: achow101: ACK 91d08889218e06631f43a3dab0bae576aa46e43c hebasto: ACK 91d08889218e06631f43a3dab0bae576aa46e43c, I have reviewed the code and it looks OK. Tree-SHA512: c4b7ef2c0bfeb28d1c4f55f497810f629873137e02f5a92137c02cb1ff603ac76473dcd2171e594491494a5cb87b8c0c803e06b86f190d4acb231791e28e802d
2023-10-26Merge bitcoin/bitcoin#28283: doc: clarify cookie generation in ↵Andrew Chow
JSON-RPC-interface.md bdb2e8d4aef927efd4e74d31b5d4dffe73470d1f Update JSON-RPC-interface.md (iamcarlos94) Pull request description: clarifying when the .cookie file is generated ACKs for top commit: maflcko: lgtm ACK bdb2e8d4aef927efd4e74d31b5d4dffe73470d1f achow101: ACK bdb2e8d4aef927efd4e74d31b5d4dffe73470d1f Tree-SHA512: 5a19b9892917126980bd3260f6035a8b2c5c9a9cfd16261d5364713ffa4816f1604e8bd3298fcf7ca7be072f33cd5a9f4e0a89cfee77ae90dc0b201e4abc0f3f
2023-10-26Merge bitcoin/bitcoin#26078: p2p: return `CSubNet` in `LookupSubNet`Andrew Chow
fb3e812277041f239b97b88689a5076796d75b9b p2p: return `CSubNet` in `LookupSubNet` (brunoerg) Pull request description: Analyzing the usage of `LookupSubNet`, noticed that most cases uses check if the subnet is valid by calling `subnet.IsValid()`, and the boolean returned by `LookupSubNet` hasn't been used so much, see: https://github.com/bitcoin/bitcoin/blob/29d540b7ada890dd588c4825d40c27c5e6f20061/src/httpserver.cpp#L172-L174 https://github.com/bitcoin/bitcoin/blob/29d540b7ada890dd588c4825d40c27c5e6f20061/src/net_permissions.cpp#L114-L116 It makes sense to return `CSubNet` instead of `bool`. ACKs for top commit: achow101: ACK fb3e812277041f239b97b88689a5076796d75b9b vasild: ACK fb3e812277041f239b97b88689a5076796d75b9b theStack: Code-review ACK fb3e812277041f239b97b88689a5076796d75b9b stickies-v: Concept ACK, but Approach ~0 (for now). Reviewed the code (fb3e812277041f239b97b88689a5076796d75b9b) and it all looks good to me. Tree-SHA512: ba50d6bd5d58dfdbe1ce1faebd80dd8cf8c92ac53ef33519860b83399afffab482d5658cb6921b849d7a3df6d5cea911412850e08f3f4e27f7af510fbde4b254
2023-10-26Merge bitcoin/bitcoin#28107: util: Type-safe transaction identifiersAndrew Chow
940a49978c70453e1aaf2c4a0bcb382872b844a5 Use type-safe txid types in orphanage (dergoegge) ed70e6501648466b9ca91a39b83775363e9a726d Introduce types for txids & wtxids (dergoegge) cdb14d79e809bf7d1612b21b554a9fcfb2ab1c91 [net processing] Use HasWitness over comparing (w)txids (dergoegge) Pull request description: We currently have two different identifiers for transactions: `txid` (refering to the hash of a transaction without witness data) and `wtxid` (referring to the hash of a transaction including witness data). Both are typed as `uint256` which could lead to type-safety bugs in which one transaction identifier type is passed where the other would be expected. This PR introduces explicit `Txid` and `Wtxid` types that (if used) would cause compilation errors for such type confusion bugs. (Only the orphanage is converted to use these types in this PR) ACKs for top commit: achow101: ACK 940a49978c70453e1aaf2c4a0bcb382872b844a5 stickies-v: ACK 940a49978c70453e1aaf2c4a0bcb382872b844a5 hebasto: ACK 940a49978c70453e1aaf2c4a0bcb382872b844a5, I have reviewed the code and it looks OK. instagibbs: re-ACK 940a49978c70453e1aaf2c4a0bcb382872b844a5 BrandonOdiwuor: re-ACK 940a49978c70453e1aaf2c4a0bcb382872b844a5 glozow: reACK 940a49978c Tree-SHA512: 55298d1c2bb82b7a6995e96e554571c22eaf4a89fb2a4d7a236d70e0f625e8cca62ff2490e1c179c47bd93153fe6527b56870198f026f5ee7753d64d7a424c92
2023-10-26[net] Check i2p private key constraintsdergoegge
Co-authored-by: Vasil Dimov <vd@FreeBSD.org>
2023-10-26Merge bitcoin/bitcoin#28728: wallet: [bugfix] Mark CNoDestination and ↵Andrew Chow
PubKeyDestination constructor explicit 1111475b41698260cda0f25a96c051fd18d66129 bugfix: Mark CNoDestination and PubKeyDestination constructor explicit (MarcoFalke) fa5ccc4137fdd14a75a6fc860b8ff6fc455cb55d iwyu: Export prevector.h from script.h (MarcoFalke) Pull request description: It seems confusing to allow any script, even one with a corresponding address, to silently convert to `CNoDestination`. Make the converstion `explicit` in the code, and fix any bugs that were previously introduced. In a follow-up, the class can be renamed, or the documentation can be updated to better reflect what the code does. ACKs for top commit: josibake: ACK https://github.com/bitcoin/bitcoin/commit/1111475b41698260cda0f25a96c051fd18d66129 achow101: ACK 1111475b41698260cda0f25a96c051fd18d66129 furszy: Code review ACK 1111475 Tree-SHA512: d8b5f54d0cd8649a31e227ef164bb13e5b81ee9820f1976fd70c7a0de6841fba72d549c2f63e351c8cdda37dceb4763eca203e1c8ef385f46d9da6f1855c39ec
2023-10-26Fix bugprone-lambda-function-name errorsMarcoFalke
Can be reviewed with --color-moved=dimmed-zebra
2023-10-25bugfix: Mark CNoDestination and PubKeyDestination constructor explicitMarcoFalke
This should fix the bug reported in https://github.com/bitcoin/bitcoin/pull/28246#discussion_r1371640502, which caused the GUI to not detect the destination type of recipients, thus picking the wrong change destination type. Also, add missing lifetimebound attribute to a getter method.
2023-10-25Merge bitcoin/bitcoin#28264: test: refactor: support sending funds with ↵Andrew Chow
outpoint result 50d1ac120716ab17f32b28513c0ac9940001a783 test: remove unused `find_output` helper (Sebastian Falbesoner) 73a339abc3c864461c8b8830e139c8ec51570243 test: refactor: support sending funds with outpoint result (Sebastian Falbesoner) Pull request description: In wallet-related functional tests we often want to send funds to an address and use the resulting (non-change) UTXO directly after as input for another transaction. Doing that is currently tedious, as it involves finding the index part of the outpoint manually by calling helpers like `find_vout_for_address` or `find_output` first. This results in two different txid/vout variables which then again have to be combined to a single dictionary `{"txid": ..., "vout": ...}` in order to be specified as input for RPCs like `createrawtransaction` or `createpsbt`. For example: ``` txid1 = node1.sendtoaddress(addr1, value1) vout1 = find_vout_for_address(node1, txid1, addr1) txid2 = node2.sendtoaddress(addr2, value2) vout2 = find_vout_for_address(node2, txid2, addr2) node.createrawtransaction([{'txid': txid1, 'vout': vout1}, {'txid': txid2, 'vout': vout2}], .....) ``` This PR introduces a helper `create_outpoints` to immediately return the outpoint as UTXO dictionary in the common format, making the tests more readable and avoiding unnecessary duplication: ``` utxo1 = self.create_outpoints(node1, outputs=[{addr1: value1}])[0] utxo2 = self.create_outpoints(node2, outputs=[{addr2: value2}])[0] node.createrawtransaction([utxo1, utxo2], .....) ``` Tests are switched to work with UTXO-objects rather than two individual txid/vout variables accordingly. The `find_output` helper is removed, as it seems generally a bad idea to search for an outpoint only based on the output value. If that's really ever needed in the future, it makes probably more sense to add it as an additional parameter to `find_vout_of_address`. Note that `find_output` supported specifying a block-hash for where to look for the transaction (being passed on to the `getrawtransaction` RPC). This seems to be unneeded, as txids are always unique and for the only test that used that parameter (rpc_psbt.py) there was no observed difference in run-time, so it was not reintroduced in the new helper. There are still some `find_vout_of_address` calls remaining, used for detecting change outputs or for whenever the sending happens via `sendrawtransaction` instead, so this PR tackles not all, but the most common case. ACKs for top commit: achow101: ACK 50d1ac120716ab17f32b28513c0ac9940001a783 BrandonOdiwuor: ACK 50d1ac120716ab17f32b28513c0ac9940001a783 maflcko: ACK 50d1ac120716ab17f32b28513c0ac9940001a783 🖨 Tree-SHA512: af2bbf13a56cc840fefc1781390cf51625f1e41b3c030f07fc9abb1181b2d414ddbf795e887db029e119cbe45de14f7c987c0cba72ff0b8953080ee218a7915a
2023-10-25test: add coverage for snapshot chainstate not matching AssumeUTXO parameterspablomartin4btc
Co-authored-by: Russell Yanofsky <russ@yanofsky.org> Co-authored-by: Sebastian Falbesoner <sebastian.falbesoner@gmail.com>
2023-10-25test: replace random_bytes with randbytes #28720ns-xvrn
2023-10-25Merge bitcoin-core/gui#771: Avoid error-prone leading whitespace in ↵Hennadii Stepanov
translatable strings 856325fac17465d102da621f1282b6d8ed02f679 lint: Add `lint-qt-translation.py` (Hennadii Stepanov) 294a018bf5106b03af39a2a8cfa4d5f2ebf6912b qt: Avoid error prone leading spaces in translatable strings (Hennadii Stepanov) d8298e7f069f961fc077ceacff2c332d58734688 qt, refactor: Drop superfluous type conversions (Hennadii Stepanov) Pull request description: While working on the GUI translation via Transifex web interface, I found it error-prone to have leading whitespace in translatable strings. This is because it is very easy to unintentionally drop them in translations unnoticed. Fixed all current cases. Added a linter to prevent similar cases in the future. ACKs for top commit: furszy: utACK 856325f Tree-SHA512: b1ca5effb2db6649e1e99382de79acf3a9f81cc9dad434db5623338489e597897e8addd60c1ab3dcc7506ae62753a7a4ad5a41d7a865f8fcdf94348b54baa7e7
2023-10-25Merge bitcoin-core/gui#742: Exit and show error if unrecognized command line ↵Hennadii Stepanov
args are present 51e4dc49f5335b5bae6c14606d1cc653a08a96b1 gui: Show error if unrecognized command line args are present (John Moffett) Pull request description: Fixes https://github.com/bitcoin-core/gui/issues/741 Starting bitcoin-qt with non-hyphen ("-") arguments causes it to silently ignore any later valid options. For instance, invoking `bitcoin-qt -server=1 foo -regtest` on a fresh install will run `mainnet` instead of `regtest`. This change makes the client exit with an error message if any such "loose" arguments are encountered. This mirrors how `bitcoind` handles it: https://github.com/bitcoin/bitcoin/blob/c6287faae4c0e705a9258a340dfcf548906f12af/src/bitcoind.cpp#L127-L132 However, BIP-21 `bitcoin:` payment URIs are still allowed, but only if they're not followed by any additional options. ACKs for top commit: maflcko: lgtm ACK 51e4dc49f5335b5bae6c14606d1cc653a08a96b1 hernanmarino: tested ACK 51e4dc49f5335b5bae6c14606d1cc653a08a96b1 pablomartin4btc: tACK 51e4dc49f5335b5bae6c14606d1cc653a08a96b1 hebasto: ACK 51e4dc49f5335b5bae6c14606d1cc653a08a96b1, I have reviewed the code and it looks OK. Tree-SHA512: 3997a7a9a747314f13e118aee63e8679e00ed832d9c6f115559a4c39c9c4091572207c60e362cb4c19fc8da980d4b0b040050aa70c5ef84a855cb7e3568bbf13
2023-10-25iwyu: Export prevector.h from script.hMarcoFalke
This should cut some include bloat and seems fine to do, because prevector exists primarily to represent scripts. Also, add missing includes to script.h and addresstype.h
2023-10-24assumeutxo, blockstorage: prevent core dump on invalid hashpablomartin4btc
2023-10-24Merge bitcoin/bitcoin#28627: depends: zeromq 4.3.5fanquake
986d7fed057b995a720787cbbd21e1c41763fb83 depends: zeromq 4.3.5 (fanquake) Pull request description: First new point release of zeromq in two and a half years. Mostly bug fixes; the project also completed a relicense to the "Mozilla Public License". See https://github.com/zeromq/libzmq/releases/tag/v4.3.5. ACKs for top commit: hebasto: ACK 986d7fed057b995a720787cbbd21e1c41763fb83, I have reviewed the code and it looks OK. TheCharlatan: ACK 986d7fed057b995a720787cbbd21e1c41763fb83 Tree-SHA512: cdd6abfbbe10873c1ca267fed648c2e6ff17a4aff50c414924006e63fa39d501e803f8893a5cd966a2078b5c077f2578e482483e6723ea6f5760f16211d40998
2023-10-24Merge bitcoin/bitcoin#28210: build: Bump clang minimum supported version to 13fanquake
fae379b6b1c9d434821acc348f010d67c4fee927 build: Bump minimum supported Clang to clang-13 (MarcoFalke) fab1ef9512c364c2b906ebfacc76439816e216d1 Bump .python-version from 3.9.17 to 3.9.18 (MarcoFalke) Pull request description: All supported operating systems ship with clang-13 (or later), so bump the minimum to that and remove now unused workarounds for previous clang bugs. For reference: * https://packages.debian.org/bullseye/clang-13 * https://packages.ubuntu.com/jammy/clang (`clang-14`) and https://packages.ubuntu.com/jammy/clang-15 * CentOS-like 8/9 Stream: All Clang versions from 13 to 15 * FreeBSD 12/13: All Clang version from 13 to 16 * OpenSuse Tumbleweed ships with https://software.opensuse.org/package/clang (`clang16`); No idea about OpenSuse Leap This is for Bitcoin Core 27.0 in 2024 (next year), not the soon upcoming 26.0 next month. ACKs for top commit: fanquake: ACK fae379b6b1c9d434821acc348f010d67c4fee927 Tree-SHA512: 8ed2b227de39b60d3f004daa4a38ea66fe005988bd977046a40613fba847d88d272925732f24777c00264abb99e25874b05b4b9243868d304eba84b450835ccc
2023-10-24build: Bump minimum supported Clang to clang-13MarcoFalke
2023-10-24Bump .python-version from 3.9.17 to 3.9.18MarcoFalke
Requested in https://github.com/bitcoin/bitcoin/pull/28211/files#r1309945635
2023-10-24Merge bitcoin/bitcoin#28211: Bump python minimum supported version to 3.9fanquake
fa25e8b0a1610553014c786428f146ef9c694678 doc: Recommend lint image build on every call (MarcoFalke) faf70c1f330a92612cf381d32c791e9ba445d3f2 Bump python minimum version to 3.9 (MarcoFalke) fa8996b930886da712c09ffe4b58016b36c2ae5b ci: Bump i686_multiprocess.sh to latest Ubuntu LTS (MarcoFalke) Pull request description: All supported operating systems ship with python 3.9 (or later), so bumping the minimum should not cause any issues. A bump will allow new code to use new python 3.9 features. For reference: * https://packages.debian.org/bullseye/python3 * https://packages.ubuntu.com/focal/python3.9 * FreeBSD 12/13 also ships with 3.9 * CentOS-like 8/9 also ships with 3.9 (and 3.11) * OpenSuse Leap also ships with 3.9 (and 3.11) https://software.opensuse.org/package/python311-base This is for Bitcoin Core 27.0 in 2024 (next year), not the soon upcoming 26.0 next month. ACKs for top commit: Sjors: ACK fa25e8b0a1610553014c786428f146ef9c694678 jamesob: ACK fa25e8b0a1610553014c786428f146ef9c694678 ([`jamesob/ackr/28211.1.MarcoFalke.bump_python_minimum_supp`](https://github.com/jamesob/bitcoin/tree/ackr/28211.1.MarcoFalke.bump_python_minimum_supp)) Tree-SHA512: 86c9f6ac4b5ba94a62ee6a6062dd48a8295d8611a39cdb5829f4f0dbc77aaa1a51edccc7a99275bf699143ad3a6fe826de426d413e5a465e3b0e82b86d10c32e
2023-10-24depends: zeromq 4.3.5fanquake
https://github.com/zeromq/libzmq/releases/tag/v4.3.5
2023-10-24Merge bitcoin/bitcoin#28714: doc: remove release note fragments for 26.x branchfanquake
1ac5584f809630b4cc276437d6181ff9ce8a8592 doc: remove release note fragments for 26.x branch (fanquake) 799ce4d0507e4bda8a77ded1d4b09043ae38c68b build: bump version to 26.99 (fanquake) Pull request description: 26.x has been branched. Delete all release note fragments from this branch. Bump master version to 26.99. ACKs for top commit: stickies-v: ACK 1ac5584f809630b4cc276437d6181ff9ce8a8592 Tree-SHA512: d7d0c26333ed026460fb648ff5cb8f2f3abe150b47dcf011a563b8fcaad6efe59688f3aa2f23c246e003e37d9b612bb9b9f017ad17cf575455e3f73c6948cff8
2023-10-24doc: remove release note fragments for 26.x branchfanquake
2023-10-24build: bump version to 26.99fanquake
2023-10-24Merge bitcoin/bitcoin#28707: doc: add historical release notes for 24.2fanquake
3f482ac231c3a0077dd236c0ec8f5afc12b71859 doc: add historical release notes for 24.2 (fanquake) Pull request description: v24.2 has [been tagged](https://github.com/bitcoin/bitcoin/releases/tag/v24.2). ACKs for top commit: stickies-v: ACK 3f482ac231c3a0077dd236c0ec8f5afc12b71859 Tree-SHA512: 71773832910ecda7ed34c6545d184ecbc743d9a36aadd8e4bd367ff60ef5b8048d39335b2347878c4a1a076cc691f12f0e36e8db542757c883d7f83d1161625d
2023-10-24Merge bitcoin/bitcoin#28660: test: enable reindex readonly test on *BSDfanquake
5a0688a20d88a9641c02436abbd7b49e227f1a37 test: enable reindex readonly test on *BSD and macOS as root (Matthew Zipkin) Pull request description: see https://github.com/bitcoin/bitcoin/pull/27850#discussion_r1349505585 OpenBSD and FreeBSD don't have `chattr` but they do have `chflags`, use that method to make the block file immutable for the reindex_readonly test. Written and tested on a VPS running FreeBSD: ``` FreeBSD freebsd-13-1 13.2-RELEASE-p4 FreeBSD 13.2-RELEASE-p4 GENERIC amd64 ``` ACKs for top commit: maflcko: re-cr-lgtm-ACK 5a0688a20d88a9641c02436abbd7b49e227f1a37 jonatack: ACK 5a0688a20d88a9641c02436abbd7b49e227f1a37 tested on macOS only theStack: ACK 5a0688a20d88a9641c02436abbd7b49e227f1a37 Tree-SHA512: 8c88d282d09c00355d22c4c504b779f60e420327a5e07bcf80fa77b97fefcb04952af9ceaf439d9033a0a2448cb26a02663fe6bddcd4a74792857cfbaf1c5162
2023-10-24test: remove unused `find_output` helperSebastian Falbesoner
2023-10-24test: refactor: support sending funds with outpoint resultSebastian Falbesoner
This commit introduces a helper `create_outpoints` to execute the `send` RPC and immediately return the target address outpoints as UTXO dictionary in the common format, making the tests more readable and avoiding unnecessary duplication.
2023-10-23Merge bitcoin/bitcoin#28609: wallet: Reload watchonly and solvables wallets ↵Ryan Ofsky
after migration 4814e4063e674ad9b0a5c7e56059cd6a2bf9b764 test: Check tx metadata is migrated to watchonly (Andrew Chow) d616d30ea5fdfb897f8375ffd8b9f4536ae7835b wallet: Reload watchonly and solvables wallets after migration (Andrew Chow) 118f2d7d70b584eee7b89e58b5cd2d61c59a9bbf wallet: Copy all tx metadata to watchonly wallet (Andrew Chow) 9af87cf3485ce3fac553a284cde37a35d1085c25 test: Check that a failed wallet migration is cleaned up (Andrew Chow) Pull request description: Some incomplete/incorrect state as a result of migration can be mitigated/cleaned up by simply restarting the migrated wallets. We already do this for a wallet when it is migrated, but we do not for the new watchonly and solvables wallets that may be created. This PR introduces this behavior, in addition to creating those wallets initially without an attached chain. While implementing this, I noticed that not all `CWalletTx` metadata was being copied over to the watchonly wallet and so some data, such as time received, was being lost. This PR fixes this as a side effect of not having a chain attached to the watchonly wallet. A test has also been added. ACKs for top commit: ishaanam: light code review ACK 4814e4063e674ad9b0a5c7e56059cd6a2bf9b764 ryanofsky: Code review ACK 4814e4063e674ad9b0a5c7e56059cd6a2bf9b764. Just implemented the suggested orderpos, copyfrom, and path set comments since last review furszy: ACK 4814e406 Tree-SHA512: 0b992430df9f452cb252c2212df8e876613f43564fcd1dc00c6c31fa497adb84dfff6b5ef597590f9b288c5f64cb455f108fcc9b6c9d1fe9eb2c39e7f2c12a89
2023-10-23Merge bitcoin/bitcoin#28685: coinstats, assumeutxo: fix hash_serialized2 ↵Andrew Chow
calculation 4bfaad4eca01674a9c84a447a17594dc2b9a4c39 chainparams, assumeutxo: Fix signet txoutset hash (Fabian Jahr) a503cd0f0b55736743bcf8d2c46d271064772bef chainparams, assumeutxo: Fix testnet txoutset hash (Fabian Jahr) f6213929c519d0e615cacd3d6f479f1517be1662 assumeutxo: Check deserialized coins for out of range values (Fabian Jahr) 66865446a771327be9e972cdaf01154ea1bdff6d docs: Add release notes for #28685 (Fabian Jahr) cb0336817edc2b6aee2eca818f133841f613a767 scripted-diff: Rename hash_serialized_2 to hash_serialized_3 (Fabian Jahr) 351370a1d211615e3d5b158ccb0400cd79c5c085 coinstats: Fix hash_serialized2 calculation (Fabian Jahr) Pull request description: Closes #28675 The last commit demonstrates that theStack's analysis [here](https://github.com/bitcoin/bitcoin/issues/28675#issuecomment-1770389468) seems to be correct. There will be more changes needed for the rest of the test suite but the `feature_assumeutxo.py` with my additional tests pass. ACKs for top commit: achow101: ACK 4bfaad4eca01674a9c84a447a17594dc2b9a4c39 theStack: Code-review ACK 4bfaad4eca01674a9c84a447a17594dc2b9a4c39 ryanofsky: Code review ACK 4bfaad4eca01674a9c84a447a17594dc2b9a4c39 Tree-SHA512: 2f6abc92b282f7c5da46391803cf0804d13978d191d541f2509b532c538abccd0a081e46cda23d80d47206a05fa2b5d41b7ab246e6a263db7a7461d6292116ef
2023-10-23Merge bitcoin/bitcoin#28618: doc: assumeutxo prune and index notesRyan Ofsky
03f82087f6ce1c29327f34d12945200494e6956d doc: assumeutxo prune and index notes (Sjors Provoost) Pull request description: Based on recent comments on #27596. ACKs for top commit: pablomartin4btc: re ACK 03f82087f6ce1c29327f34d12945200494e6956d ryanofsky: ACK 03f82087f6ce1c29327f34d12945200494e6956d. Nice changes, these seem like very helpful notes Tree-SHA512: fe651b49f4d667400a3655899f27a96dd1eaf67cf9215fb35db5f44fb8c0313e7d541518be6791fec93392df24b909793f3886adb808e53228ed2a291165639d
2023-10-23test: enable reindex readonly test on *BSD and macOS as rootMatthew Zipkin
2023-10-23Merge bitcoin/bitcoin#28708: ci: Add missing --external to podman image prunefanquake
fa6588737714cf26571657fc216552a4291376da ci: Add missing --external to podman image prune (MarcoFalke) Pull request description: This should fix the out-of-space issues seen in CI. For example: https://cirrus-ci.com/task/6208410429947904?logs=ci#L8613 ``` Error: committing container for step {Env:[FILE_ENV=./ci/test/00_setup_env_native_msan.sh PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin] Command:run Args:[bash -c cd /ci_container_base/ && set -o errexit && source ./ci/test/00_setup_env.sh && ./ci/test/01_base_install.sh] Flags:[] Attrs:map[json:true] Message:RUN bash -c cd /ci_container_base/ && set -o errexit && source ./ci/test/00_setup_env.sh && ./ci/test/01_base_install.sh Original:RUN ["bash", "-c", "cd /ci_container_base/ && set -o errexit && source ./ci/test/00_setup_env.sh && ./ci/test/01_base_install.sh"]}: copying layers and metadata for container "8d882455cc157be6a85d9779b45cacf4dd92a37cfb16fad38213f758a830827d": writing blob: adding layer with blob "sha256:371f657e226fef20f4af6fb88a288dd6248c82c2088daca2d53aaacb51b4303a": processing tar file(write /usr/lib/x86_64-linux-gnu/perl/5.34.0/auto/Unicode/Collate/Collate.so: no space left on device): exit status 1 ACKs for top commit: stickies-v: utACK fa6588737714cf26571657fc216552a4291376da Tree-SHA512: a949b957654272b9c9450feebb64397542029019f031a18444b5b403aca899c972b3b163be716bf1bfbd5820430c70a6fec008771c6e13d3d8281ed100df575b