aboutsummaryrefslogtreecommitdiff
AgeCommit message (Collapse)Author
2024-05-14crypto: add NUMS_H constjosibake
2024-05-10Merge bitcoin/bitcoin#29252: kernel: Remove key module from kernel libraryAva Chow
96378fe734e5fb6167eb20036d7170572a647edb Refactor: Remove ECC_Start and ECC_Stop from key header (TheCharlatan) 41eba5bd716bea47c8731d156d053afee92a7f12 kernel: Remove key module from kernel library (TheCharlatan) a08d2b3cb971c68e9a50b991b2953fa4541cf48a tools: Use ECC_Context helper in bitcoin-tx and bitcoin-wallet tools (Ryan Ofsky) 28905c1a64a87a56f16aea8a4d23dea7eec9ca59 test: Use ECC_Context helper in bench and fuzz tests (Ryan Ofsky) 538fedde1d9c96a2bbe06cacc0cd6903135fbc83 common: Add ECC_Context RAII wrapper for ECC_Start/ECC_Stop (Ryan Ofsky) Pull request description: The key module's functionality is not used by the kernel library, but currently kernel users are still required to initialize the key module's `secp256k1_context_sign` global as part of the `kernel::Context` through `ECC_Start`. So move the `ECC_Start` call to the `NodeContext` ctor instead to completely remove the key module from the kernel library. The gui tests currently keep multiple `NodeContext` objects in memory, so call `ECC_Stop` manually to avoid triggering an assertion on `ECC_Start`. --- This PR is part of the [libbitcoinkernel project](https://github.com/bitcoin/bitcoin/issues/27587). It removes a module from the kernel library. ACKs for top commit: achow101: ACK 96378fe734e5fb6167eb20036d7170572a647edb ryanofsky: Code review ACK 96378fe734e5fb6167eb20036d7170572a647edb. Just suggested comment changes since last review. theuni: utACK 96378fe734e5fb6167eb20036d7170572a647edb Tree-SHA512: 40be427e8e2c920c0e3ce64a9bdd90551be27a89af11440bfb6ab0dd3a1d1ccb7cf1f82383cd782818cd1bb44d5ae5d2161cf4d78d3127ce4987342007090bab
2024-05-10Merge bitcoin/bitcoin#29948: test: add missing comparison of node1's mempool ↵Ava Chow
in MempoolPackagesTest e912717ff63f111d8f1cd7ed1fcf054e28f36409 test: add missing comparison of node1's mempool in MempoolPackagesTest (umiumi) Pull request description: #29941 Recreated a pull request because there was a conflict. Trying to resolve the conflict but the old one automatically closed. Add missing comparison for TODO comments in `mempool_packages.py` Also, notice that the ancestor size limits and descendant size limits actually implemented in #21800 , so I removed the todo for those two size limits. ACKs for top commit: kevkevinpal: ACK [e912717](https://github.com/bitcoin/bitcoin/pull/29948/commits/e912717ff63f111d8f1cd7ed1fcf054e28f36409) achow101: ACK e912717ff63f111d8f1cd7ed1fcf054e28f36409 alfonsoromanz: Tested ACK e912717ff63f111d8f1cd7ed1fcf054e28f36409. The code looks good to me and the test execution is successful. rkrux: tACK [e912717](https://github.com/bitcoin/bitcoin/pull/29948/commits/e912717ff63f111d8f1cd7ed1fcf054e28f36409) Tree-SHA512: 8cb51746b0547369344c9ceef59599bfe9c91d424687af5e24dc6641f9e99fb433515d79c724e71fd3d5e02994f0cef623d3674367b8296b05c3c6fcdde282ef
2024-05-09Merge bitcoin/bitcoin#30006: test: use sleepy wait-for-log in reindex readonlyAva Chow
fd6a7d3a13d89d74e161095b0e9bd3570210a40c test: use sleepy wait-for-log in reindex readonly (Matthew Zipkin) Pull request description: Also rename the busy wait-for-log method to prevent recurrence. See https://github.com/bitcoin/bitcoin/pull/27039#discussion_r1532578152 ACKs for top commit: maflcko: utACK fd6a7d3a13d89d74e161095b0e9bd3570210a40c achow101: ACK fd6a7d3a13d89d74e161095b0e9bd3570210a40c tdb3: ACK for fd6a7d3a13d89d74e161095b0e9bd3570210a40c rkrux: ACK [fd6a7d3](https://github.com/bitcoin/bitcoin/pull/30006/commits/fd6a7d3a13d89d74e161095b0e9bd3570210a40c) Tree-SHA512: 7ff0574833df1ec843159b35ee88b8bb345a513ac13ed0b72abd1bf330c454a3f9df4d927871b9e3d37bfcc07542b06ef63acef8e822cd18499adae8cbb0cda8
2024-05-09Merge bitcoin/bitcoin#29939: test: add MiniWallet tagging support to avoid ↵Ava Chow
UTXO mixing, use in `fill_mempool` dd8fa861939d5b8bdd844ad7cab015d08533a91a test: use tagged ephemeral MiniWallet instance in fill_mempool (Sebastian Falbesoner) b2037ad4aeb4e16c7eb1e5756d0d1ee20172344b test: add MiniWallet tagging support to avoid UTXO mixing (Sebastian Falbesoner) c8e6d08236ff225db445009bf513d6d25def8eb2 test: refactor: eliminate COINBASE_MATURITY magic number in fill_mempool (Sebastian Falbesoner) 4f347140b1a31237597dd1821adcde8bd5761edc test: refactor: move fill_mempool to new module mempool_util (Sebastian Falbesoner) Pull request description: Different MiniWallet instances using the same mode (either ADDRESS_OP_TRUE, RAW_OP_TRUE or RAW_P2PK) currently always create and spend UTXOs with identical output scripts, which can cause unintentional tx dependencies (see e.g. the discussion in https://github.com/bitcoin/bitcoin/pull/29827#discussion_r1565443465). In order to avoid mixing of UTXOs between instances, this PR introduces the possibility to provide a MiniWallet tag name, that is used to derive a different internal key for the taproot construction, leading to a different P2TR output script. Note that since we use script-path spending and only the key-path is changed here, no changes in the MiniWallet spending logic are needed. The new tagging option is then used in the `fill_mempool` helper to create an ephemeral wallet for the filling txs, as suggested in https://github.com/bitcoin/bitcoin/pull/29827#discussion_r1565964264. To avoid circular dependencies, `fill_mempool` is moved to a new module `mempool_util.py` first. I'm still not sure if a generic word like "tag" is the right term for what this tries to achieve, happy to pick up better suggestions. Also, maybe passing a tag name is overkill and a boolean flag like "random_output_script" is sufficient? ACKs for top commit: glozow: ACK dd8fa861939 achow101: ACK dd8fa861939d5b8bdd844ad7cab015d08533a91a rkrux: tACK [dd8fa86](https://github.com/bitcoin/bitcoin/pull/29939/commits/dd8fa861939d5b8bdd844ad7cab015d08533a91a) brunoerg: utACK dd8fa861939d5b8bdd844ad7cab015d08533a91a Tree-SHA512: 5ef3558c3ef5ac32cfa79c8f751972ca6bceaa332cd7daac7e93412a88e30dec472cb041c0845b04abf8a317036d31ebddfc3234e609ed442417894c2bdeeac9
2024-05-09Merge bitcoin/bitcoin#29122: test: adds outbound eviction functional tests, ↵Ava Chow
updates comment in ConsiderEviction d53d84834747c37f4060a9ef379e0a6b50155246 test: adds outbound eviction tests for non outbound-full-relay peers (Sergi Delgado Segura) a8d9a0edc7cef2c31a557ef53eb45520976b0d65 test: adds outbound eviction functional tests, updates comment in ConsiderEviction (Sergi Delgado Segura) Pull request description: ## Motivation While checking the outbound eviction code I realized a case was not considered within the comments, which in turn made me realize we had no functional tests for the outbound eviction case (when I went to check/add the test case). This PR updates the aforementioned comment and adds functional tests to cover the outbound eviction logic, in addition to the existing unit tests found at `src/test/denialofservice_tests.cpp`. ACKs for top commit: davidgumberg: reACK https://github.com/bitcoin/bitcoin/commit/d53d84834747c37f4060a9ef379e0a6b50155246 tdb3: Re ACK for d53d84834747c37f4060a9ef379e0a6b50155246 achow101: ACK d53d84834747c37f4060a9ef379e0a6b50155246 cbergqvist: ACK d53d84834747c37f4060a9ef379e0a6b50155246 Tree-SHA512: 633b84bb1229fe21e2f650c1beada33ca7f190b64eafd64df2266516d21175e5d652e019ff7114f00cb8bd19f5817dc19e65adf75767a88e24dc0842ce40c63e
2024-05-09Merge bitcoin/bitcoin#28793: contrib: Add asmap-toolAva Chow
6abe772a17e09fe96e68cd3311280d5a30f6378b contrib: Add asmap-tool (Fabian Jahr) Pull request description: This adds `asmap.py` and `asmap-tool.py` from sipa's `nextgen` branch: https://github.com/sipa/asmap/tree/nextgen The motivation is that we should maintain the tooling for de- and encoding asmap files within the bitcoin core repository because it is not possible to use an asmap file that is not encoded. We already had an earlier version of `asmap.py` within the seeds contrib tools. The newer version only had a small amount of changes and is still compatible, so the old version is removed from contrib/seeds and the new version is made available to `makeseeds.py`. ACKs for top commit: virtu: ACK [6abe772](https://github.com/bitcoin/bitcoin/commit/6abe772a17e09fe96e68cd3311280d5a30f6378b) 0xB10C: ACK 6abe772a17e09fe96e68cd3311280d5a30f6378b achow101: ACK 6abe772a17e09fe96e68cd3311280d5a30f6378b brunoerg: ACK 6abe772a17e09fe96e68cd3311280d5a30f6378b Tree-SHA512: cc2a82ffa4eb46fa0ce4ca769dd82f8d0d2f37fc3652aa748eeb060e1142f9da4035008fe89433e2fd524a4dc153b7b9c085748944b49137b37009b0c0be8afb
2024-05-09Merge bitcoin/bitcoin#29973: test: Assumeutxo: ensure failure when importing ↵Ava Chow
a snapshot twice b259b0e8d360726b062c4b0453d1cf5a68e1933f [Test] Assumeutxo: ensure failure when importing a snapshot twice (Alfonso Roman Zubeldia) Pull request description: I am getting familiar with the `assume_utxo` tests and I found that the scenario of trying to activate a snapshot twice is not covered. This test is to ensure failure when loading a snapshot if there is already a snapshot-based chainstate. ACKs for top commit: fjahr: Code review ACK b259b0e8d360726b062c4b0453d1cf5a68e1933f kevkevinpal: tACK [b259b0e](https://github.com/bitcoin/bitcoin/pull/29973/commits/b259b0e8d360726b062c4b0453d1cf5a68e1933f) achow101: ACK b259b0e8d360726b062c4b0453d1cf5a68e1933f rkrux: tACK [b259b0e](https://github.com/bitcoin/bitcoin/pull/29973/commits/b259b0e8d360726b062c4b0453d1cf5a68e1933f) Tree-SHA512: 3510861390d0e40cdad6861b728df04827a1b63e642f3d956aee66ed2770b1cb7e3aa3eb00c62eb9da0544703c943cc5296936c9ebfcac18c719741c354421bb
2024-05-09Refactor: Remove ECC_Start and ECC_Stop from key headerTheCharlatan
They are unused outside of the key module now.
2024-05-09kernel: Remove key module from kernel libraryTheCharlatan
The key module's functionality is not used by the kernel library, but currently kernel users are still required to initialize the key module's `secp256k1_context_sign` global as part of the `kernel::Context` through `ECC_Start`.
2024-05-09tools: Use ECC_Context helper in bitcoin-tx and bitcoin-wallet toolsRyan Ofsky
2024-05-09test: Use ECC_Context helper in bench and fuzz testsRyan Ofsky
2024-05-09common: Add ECC_Context RAII wrapper for ECC_Start/ECC_StopRyan Ofsky
2024-05-09Merge bitcoin/bitcoin#30063: build, test: Remove unused `TIMEOUT` ↵merge-script
environment variable 189d0da3f6f561c808fdd9fbd4dfd34ccfa23fe1 build, test: Remove unused `TIMEOUT` environment variable (Hennadii Stepanov) Pull request description: Setting the `TIMEOUT` environment variable has been a noop in both cases since its introduction. It seems to have been inadvertently copy-pasted from existed code. For example, in commit d80e3cbece857b293a4903ef49c4d543bb2cfb7f, it was needlessly copied from a valid case a few lines above for the `qa/pull-tester/run-bitcoind-for-test.sh` script. ACKs for top commit: maflcko: utACK 189d0da3f6f561c808fdd9fbd4dfd34ccfa23fe1 edilmedeiros: ACK 189d0da3f6f561c808fdd9fbd4dfd34ccfa23fe1 Tree-SHA512: 61111eba30e0c82a0220bea48eba451cd9caa68785b48ec8a91059ca5aadfaff2f6d2ccdc5aa737c5cefa33579cb735431bb9e94bda8fa047825d7bd28d542fb
2024-05-08Merge bitcoin/bitcoin#29292: rpc: improve submitpackage documentation and ↵Ava Chow
other improvements 78e52f663f3e3ac86260b913dad777fd7218f210 doc: rpc: fix submitpackage examples (stickies-v) 1a875d4049574730d4a53a1b68bd29b80ad96d38 rpc: update min package size error message in submitpackage (stickies-v) f9ece258aa868d0776caa86b94e71ba05a9b287e doc: rpc: submitpackage takes sorted array (stickies-v) 17f74512f0d19cb452ed79a4bff5a222fcdb53c4 test: add bounds checking for submitpackage RPC (stickies-v) Pull request description: `submitpackage` requires the package to be topologically sorted with the child being the last element in the array, but this is not documented in the RPC method or the error messages. Also sneaking in some other minor improvements that I found while going through the code: - Informing the user that `package` needs to be an array of length between `1` and `MAX_PACKAGE_COUNT` is confusing when `IsChildWithPackage()` requires that the package size >= 2. Remove this check to avoid code duplication and sending a confusing error message. - fixups to the `submitpackage` examples ACKs for top commit: fjahr: re-ACK 78e52f663f3e3ac86260b913dad777fd7218f210 instagibbs: ACK https://github.com/bitcoin/bitcoin/pull/29292/commits/78e52f663f3e3ac86260b913dad777fd7218f210 achow101: ACK 78e52f663f3e3ac86260b913dad777fd7218f210 glozow: utACK 78e52f663f3e3ac86260b913dad777fd7218f210 Tree-SHA512: a8845621bb1cbf784167fc7c82cb8ceb105868b65b26d3465f072d1c04ef3699e85a21a524ade805d423bcecbc34f7d5bff12f2c21cbd902ae1fb154193ebdc9
2024-05-08Merge bitcoin/bitcoin#29335: test: Handle functional test disk-full errorAva Chow
357ad110548d726021547d85b5b2bfcf3191d7e3 test: Handle functional test disk-full error (Brandon Odiwuor) Pull request description: Fixes: https://github.com/bitcoin/bitcoin/issues/23099 Handle disk-full more gracefully in functional tests ACKs for top commit: itornaza: re-ACK 357ad110548d726021547d85b5b2bfcf3191d7e3 achow101: reACK 357ad110548d726021547d85b5b2bfcf3191d7e3 cbergqvist: reACK 357ad110548d726021547d85b5b2bfcf3191d7e3. Looks good! tdb3: re ACK for 357ad110548d726021547d85b5b2bfcf3191d7e3 Tree-SHA512: 9bb0d3fbe84600c88873b9f55d4b5d1443f79ec303467680c301be2b4879201387f203d9d1984169461f321037189b5e10a6a4b9d61750de638f072d2f95d77e
2024-05-08Merge bitcoin/bitcoin#26326: net: don't lock cs_main while reading blocks in ↵Ava Chow
net processing 75d27fefc7a04ebdda7be5724a014b6a896e7325 net: reduce LOCK(cs_main) scope in ProcessGetBlockData (Andrew Toth) 613a45cd4b5482aedbdc7c61c839ea05996935c6 net: reduce LOCK(cs_main) scope in GETBLOCKTXN (Andrew Toth) Pull request description: Inspired by https://github.com/bitcoin/bitcoin/pull/11913 and https://github.com/bitcoin/bitcoin/pull/26308. `cs_main` doesn't need to be locked while reading blocks. This removes the locks in `net_processing`. ACKs for top commit: sr-gi: ACK [75d27fe](https://github.com/bitcoin/bitcoin/pull/26326/commits/75d27fefc7a04ebdda7be5724a014b6a896e7325) achow101: ACK 75d27fefc7a04ebdda7be5724a014b6a896e7325 furszy: ACK 75d27fefc with a non-blocking nit. mzumsande: Code Review ACK 75d27fefc7a04ebdda7be5724a014b6a896e7325 TheCharlatan: ACK 75d27fefc7a04ebdda7be5724a014b6a896e7325 Tree-SHA512: 79b85f748f68ecfb2f2afd3267857dd41b8e76dd482c9c922037399dcbce7b1e5d4c708a4f5fd17c3fb6699b0d88f26a17cc1d92db115dd43c8d4392ae27cec4
2024-05-08Merge bitcoin/bitcoin#28336: rpc: parse legacy pubkeys consistently with ↵Ava Chow
specific error messages 98570fe29bb08d7edc48011aa6b9731c6ab4ed2e test: add coverage for parsing cryptographically invalid pubkeys (Sebastian Falbesoner) c740b154d193b91ca42f18759098d3fef6eaab05 rpc: use `HexToPubKey` helper for all legacy pubkey-parsing RPCs (Sebastian Falbesoner) 100e8a75bf5d8196c005331bd8f2ed42ada6d8d0 rpc: check and throw specific pubkey parsing errors in `HexToPubKey` (Sebastian Falbesoner) Pull request description: Parsing legacy public keys can fail for three reasons (in this order): - pubkey is not in hex - pubkey has an invalid length (not 33 or 65 bytes for compressed/uncompressed, respectively) - pubkey is crytographically invalid, i.e. is not on curve (`CPubKey.IsFullyValid()` check) Many RPCs currently perform these checks manually with different error messages, even though we already have a `HexToPubKey` helper. This PR puts all three checks in this helper (the length check was done on the call-sites before), adds specific error messages for each case, and consequently uses it for all RPCs that parse legacy pubkeys. This leads to deduplicated code and also to more consistent and detailed error messages for the user. Affected RPC calls are `createmultisig`, `addmultisigaddress`, `importpubkey`, `importmulti`, `fundrawtransaction`, `walletcreatefundedpsbt`, `send` and `sendall`. Note that the error code (-5 a.k.a. `RPC_INVALID_ADDRESS_OR_KEY`) doesn't change in any of the causes, so the changes are not breaking RPC API compatibility. Only the messages are more specific. The last commits adds test coverage for the cryptographically invalid (not-on-curve) pubkey case which wasn't exercised before. ACKs for top commit: stratospher: tested ACK 98570fe. davidgumberg: ACK https://github.com/bitcoin/bitcoin/pull/28336/commits/98570fe29bb08d7edc48011aa6b9731c6ab4ed2e Eunovo: Tested ACK https://github.com/bitcoin/bitcoin/pull/28336/commits/98570fe29bb08d7edc48011aa6b9731c6ab4ed2e achow101: ACK 98570fe29bb08d7edc48011aa6b9731c6ab4ed2e Tree-SHA512: cfa474176e95b5b18f3a9da28fdd9e87195cd58994c1331198f2840925fff322fd323a6371feab74a1b32e4b9ea58a6dc732fa751b4cdd45402c1029af609ece
2024-05-08build, test: Remove unused `TIMEOUT` environment variableHennadii Stepanov
Setting the `TIMEOUT` environment variable has been a noop in both cases since its introduction. It seems to have been inadvertently copy-pasted from existing code. For example, in commit d80e3cbece857b293a4903ef49c4d543bb2cfb7f, it was needlessly copied from a valid case a few line above for the `qa/pull-tester/run-bitcoind-for-test.sh` script.
2024-05-08Merge bitcoin/bitcoin#30053: test: added test coverage to loadtxoutset could ↵merge-script
not open file ee67bba76cca2355541f99bb731f58479981b29e test: added test coverage to loadtxoutset (kevkevin) Pull request description: The functional test coverage did not cover the rpc error of "Couldn't open file..." for loadtxoutset and this test adds coverage for it This adds coverage to this line https://github.com/bitcoin/bitcoin/blob/master/src/rpc/blockchain.cpp#L2777 ACKs for top commit: maflcko: ACK ee67bba76cca2355541f99bb731f58479981b29e davidgumberg: LGTM ACK https://github.com/bitcoin/bitcoin/pull/30053/commits/ee67bba76cca2355541f99bb731f58479981b29e rkrux: ACK [ee67bba](https://github.com/bitcoin/bitcoin/pull/30053/commits/ee67bba76cca2355541f99bb731f58479981b29e) alfonsoromanz: ACK ee67bba76cca2355541f99bb731f58479981b29e. Code looks good to me. I also ran `test/functional/feature_assumeutxo.py` to make sure all tests passes, including this one. tdb3: ACK for ee67bba76cca2355541f99bb731f58479981b29e Tree-SHA512: 210a7eb928f625d2a8d9acb63ee83cb4aaec9c267e5a0c52ad219c2935466e2cdc68667e30ad29566e6060981587e5bec42805d296f6e60f9b3b13f3330575f2
2024-05-08Merge bitcoin/bitcoin#30054: ci: Exclude feature_init for now in valgrind taskmerge-script
fab179d10243e85cdb172a9f08bcb7ec19ddf74d ci: Exclude feature_init for now in valgrind task (MarcoFalke) Pull request description: Fixes https://github.com/bitcoin/bitcoin/issues/30011 ACKs for top commit: fanquake: ACK fab179d10243e85cdb172a9f08bcb7ec19ddf74d Tree-SHA512: 5943a2abcec59253af8775e8ac7a120011a92cb66711b01a7e377a9302175d56c7de39ce028edc875b1584bf65458f92face2b0ee2028e84f4d3978d2cbafd0a
2024-05-08Merge bitcoin/bitcoin#30025: doc: fix broken relative md linksmerge-script
4b9f49da2b120e81516ddc3dc577d7a2e58e02d3 doc: fix broken relative md links (willcl-ark) Pull request description: These relative links in our documentation are broken, fix them. ACKs for top commit: maflcko: ACK 4b9f49da2b120e81516ddc3dc577d7a2e58e02d3 ryanofsky: Code review ACK 4b9f49da2b120e81516ddc3dc577d7a2e58e02d3. Thanks for the updates! ismaelsadeeq: Re ACK 4b9f49da2b120e81516ddc3dc577d7a2e58e02d3 Tree-SHA512: df4ef5ddece6c21125ce719ed6a4f69aba4f884c353ff7a8445ecb6438ed6bf0ff8268a1ae19cdd910adaadc189c6861c445b4d469f92ee81874d810dcbd0846
2024-05-07Merge bitcoin-core/gui#819: Fix misleading signmessage error with segwitmerge-script
fb9f150759b22772dd48983a2be1ea397245e289 gui: fix misleading signmessage error with segwit (willcl-ark) Pull request description: As described in https://github.com/bitcoin/bitcoin/issues/10542 (and numerous other places), message signing in Bitcoin Core does not support "signing with a segwit address" and likely will not in the foreseeable future, or at least until a new message-signing standard is agreed upon. Therefore update the possibly misleading error message presented to the user in the GUI to detail more specifically the reason their message cannot be signed, in the case that a non P2PKH address is entered. This change takes the [suggested wording](https://github.com/bitcoin/bitcoin/issues/10542#issuecomment-1960313569) from @adiabat. Perhaps with this we can close https://github.com/bitcoin/bitcoin/issues/10542 ? ACKs for top commit: hebasto: ACK fb9f150759b22772dd48983a2be1ea397245e289. Tree-SHA512: 5ba8d722ad3632dad2e0a2aa94b0f466b904e7885a247a5d26ebdfce54e3611090b103029d8dfce92adc49e50fe5f4830f687d867b4c56c3ea997e519b4e188d
2024-05-07Merge bitcoin/bitcoin#29494: build: Assume HAVE_CONFIG_H, Add IWYU pragma ↵Ava Chow
keep to bitcoin-config.h includes fa09451f8e6799682d7e7c863f25334fd1c7dce3 Add lint check for bitcoin-config.h include IWYU pragma (MarcoFalke) dddd40ba8267dea11a3eb03d5cf8b51dbb99be5d scripted-diff: Add IWYU pragma keep to bitcoin-config.h includes (MarcoFalke) Pull request description: The `bitcoin-config.h` includes have issues: * The header is incompatible with iwyu, because symbols may be defined or not defined. So the `IWYU pragma: keep` is needed to keep the include when a symbol is not defined on a platform. Compare the previous discussion in https://github.com/bitcoin/bitcoin/pull/29408#issuecomment-1948959711 * Guarding the includes by `HAVE_CONFIG_H` is verbose and brittle. Now that all build config dependencies have been removed from low level headers, the benefits are questionable, and the guard can be removed. The linter could also be tricked by guarding the include by `#if defined(HAVE_C0NFIG_H)` (`O` replaced by `0`). Compare the previous discussion in https://github.com/bitcoin/bitcoin/pull/29404#discussion_r1483189853 . ACKs for top commit: achow101: ACK fa09451f8e6799682d7e7c863f25334fd1c7dce3 TheCharlatan: ACK fa09451f8e6799682d7e7c863f25334fd1c7dce3 hebasto: re-ACK fa09451f8e6799682d7e7c863f25334fd1c7dce3, only rebased since my recent [review](https://github.com/bitcoin/bitcoin/pull/29494#pullrequestreview-2028864535) (`timedata.cpp` removed in https://github.com/bitcoin/bitcoin/pull/29623). Tree-SHA512: 47cb973f7f24bc625acc4e78683371863675d186780236d55d886cf4130e05a78bb04f1d731aae7088313b8e963a9677cc77cf518187dbd99d776f6421ca9b52
2024-05-07ci: Exclude feature_init for now in valgrind taskMarcoFalke
2024-05-07Merge bitcoin/bitcoin#29984: net: Replace ifname check with IFF_LOOPBACK in ↵merge-script
Discover a68fed111be393ddbbcd7451f78bc63601253ee0 net: Fix misleading comment for Discover (laanwj) 7766dd280d9a4a7ffdfcec58224d0985cfd4169b net: Replace ifname check with IFF_LOOPBACK in Discover (laanwj) Pull request description: Checking the interface name is kind of brittle. In the age of network namespaces and containers, there is no reason a loopback interface can't be called differently. Check for the `IFF_LOOPBACK` flag to detect loopback interface instead. Also remove a misleading comment in Discover's doc comment. ACKs for top commit: sipa: utACK a68fed111be393ddbbcd7451f78bc63601253ee0 willcl-ark: utACK a68fed111be393ddbbcd7451f78bc63601253ee0 theuni: utACK a68fed111be393ddbbcd7451f78bc63601253ee0. Satoshi-era brittleness :) Tree-SHA512: e2d7fc541f40f6a6af08286e7bcb0873ff55debdcd8b38b03f274897b673a6fb51d84d6c7241a02a9567ddf2645f50231d91bb1f55307ba7c6e68196c29b0edf
2024-05-07doc: rpc: fix submitpackage examplesstickies-v
2024-05-07rpc: update min package size error message in submitpackagestickies-v
Currently, the only allowed package topology has a min size of 2. Update the error message to reflect that.
2024-05-07doc: rpc: submitpackage takes sorted arraystickies-v
2024-05-07test: add bounds checking for submitpackage RPCstickies-v
2024-05-06test: added test coverage to loadtxoutsetkevkevin
The functional test coverage did not cover the rpc error of Couldn't open file for loadtxoutset and this test adds coverage for it
2024-05-06Merge bitcoin/bitcoin#29845: rpc: return warnings as an array instead of ↵Ava Chow
just a single one 42fb5311b19582361409d65c6fddeadbee14bb97 rpc: return warnings as an array instead of just a single one (stickies-v) Pull request description: The RPC documentation for `getblockchaininfo`, `getmininginfo` and `getnetworkinfo` states that "warnings" returns "any network and blockchain warnings". In practice, only a single warning (i.e. the latest one that is set) is returned, the other ones are ignored. Fix that by returning all warnings as an array. As a side benefit, clean up the GetWarnings() logic. Since this PR changes the RPC result schema, I've added release notes. Users can temporarily revert to the old results by using `-deprecatedrpc=warnings`, until it's removed in a future version. --- Some historical context from git log: - when `GetWarnings` was introduced in 401926283a200994ecd7df8eae8ced8e0b067c46, it was used in the `getinfo` RPC, where only a [single error/warning was returned](https://github.com/bitcoin/bitcoin/commit/401926283a200994ecd7df8eae8ced8e0b067c46#diff-7442c48d42cd5455a79915a0f00cce5e13359db46437a32b812876edb0a5ccddR250) (similar to how it is now). - later on, "warnings" RPC response fields were introduced, e.g. in ef2a3de25c882396e1776b554878d2784b6b7391, with the description [stating](https://github.com/bitcoin/bitcoin/commit/ef2a3de25c882396e1776b554878d2784b6b7391#diff-1021bd3c74415ad9719bd764ad6ca35af5dfb33b1cd863c0be49bdf52518af54R411) that it returned "any network warnings" but in practice still only a single warning was returned ACKs for top commit: achow101: re-ACK 42fb5311b19582361409d65c6fddeadbee14bb97 tdb3: Re ACK for 42fb5311b19582361409d65c6fddeadbee14bb97 TheCharlatan: ACK 42fb5311b19582361409d65c6fddeadbee14bb97 maflcko: ACK 42fb5311b19582361409d65c6fddeadbee14bb97 🔺 Tree-SHA512: 4225ed8979cd5f030dec785a80e7452a041ad5703445da79d2906ada983ed0bbe7b15889d663d75aae4a77d92e302c93e93eca185c7bd47c9cce29e12f752bd3
2024-05-06Merge bitcoin/bitcoin#29773: build, bench, msvc: Add missing benchmarksmerge-script
31a15f0aff79d2b34a9640909b9e6fb39a647b60 bench: Disable WalletCreate* benchmarks when building with MSVC (Hennadii Stepanov) 23dc0c19acd54cad1bed2f14df024b6b533f2330 msvc, bench: Add missing source files to bench_bitcoin project (Hennadii Stepanov) Pull request description: On the master branch, the `bench_bitcoin.vcxproj` MSVC project misses wallet-specific source files. This PR fixes this issue. Benchmark run on Windows: ``` > src\bench_bitcoin.exe -filter="CoinSelection|BnBExhaustion|Wallet.*" | ns/op | op/s | err% | total | benchmark |--------------------:|--------------------:|--------:|----------:|:---------- | 398,800.00 | 2,507.52 | 1.5% | 0.01 | `BnBExhaustion` | 584,450.00 | 1,711.01 | 1.5% | 0.01 | `CoinSelection` | 86,603,650.00 | 11.55 | 0.4% | 1.91 | `WalletAvailableCoins` | 7,604.00 | 131,509.73 | 0.9% | 0.01 | `WalletBalanceClean` | 124,028.57 | 8,062.66 | 2.6% | 0.01 | `WalletBalanceDirty` | 7,587.12 | 131,802.30 | 1.9% | 0.01 | `WalletBalanceMine` | 48.58 | 20,583,872.99 | 0.9% | 0.01 | `WalletBalanceWatch` | 2,371,060.00 | 421.75 | 1.3% | 0.13 | `WalletCreateTxUseOnlyPresetInputs` | 96,861,760.00 | 10.32 | 0.9% | 5.31 | `WalletCreateTxUsePresetInputsAndCoinSelection` | 280.71 | 3,562,424.13 | 1.5% | 0.01 | `WalletIsMineDescriptors` | 1,033.47 | 967,618.32 | 0.3% | 0.01 | `WalletIsMineLegacy` | 282.36 | 3,541,599.91 | 0.5% | 0.01 | `WalletIsMineMigratedDescriptors` | 484,547,300.00 | 2.06 | 1.0% | 2.43 | `WalletLoadingDescriptors` | 29,924,300.00 | 33.42 | 0.4% | 0.15 | `WalletLoadingLegacy` ``` ACKs for top commit: maflcko: lgtm ACK 31a15f0aff79d2b34a9640909b9e6fb39a647b60 Tree-SHA512: 0241af06126edf612489322cdce66ba43792066b5400b1719a8b9d1ec62030e8a9d497e2f01e38290e94c387db59ccf2a458f4b35d3dc8030a1a1413d89eb792
2024-05-06Merge bitcoin/bitcoin#29960: depends: pass verbose through to cmake based ↵merge-script
makefiles 7c69baf227252511455bc06e315f6a3c7fc5a398 depends: pass verbose through to cmake based make (Max Edwards) Pull request description: While testing https://github.com/bitcoin/bitcoin/pull/29708 I was not able to enable verbose output to check which flags were being given to the compiler. With this PR, running depends with V=1 will enable verbose output from makefiles generated by cmake. How to test: ```shell make -C depends libnatpmp V=1 ``` ACKs for top commit: hebasto: ACK 7c69baf227252511455bc06e315f6a3c7fc5a398. Tested using the folowing command: fanquake: ACK 7c69baf227252511455bc06e315f6a3c7fc5a398 Tree-SHA512: 81cd1326e940c5f14cbde96735fd02b03c1150881ed88d1e8dfa9385dfa12284bfa2cdfe097ce5f43a726c1718afb76ae16f71552ab68c207d74fdc1f7bb46ae
2024-05-06Merge bitcoin/bitcoin#30031: msvc: Compile `test\fuzz\miniscript.cpp`merge-script
9155b733e153e799f09cc7f7e9199ad776b2cbb1 build, msvc: Compile test\fuzz\miniscript.cpp (Hennadii Stepanov) Pull request description: This PR resolves the remained point from the https://github.com/bitcoin/bitcoin/pull/29774#issuecomment-2028808614: > What is the issue with the ... miniscript fuzz tests? From the CI [log](https://github.com/bitcoin/bitcoin/actions/runs/8941546183/job/24562123707?pr=30031#step:29:234): ``` miniscript_script: succeeded against 721 files in 1s. Run miniscript_script with args ['D:\\a\\bitcoin\\bitcoin\\src\\fuzz.exe', WindowsPath('D:/a/_temp/qa-assets/fuzz_seed_corpus/miniscript_script')] miniscript_smart: succeeded against 1429 files in 2s. Run miniscript_smart with args ['D:\\a\\bitcoin\\bitcoin\\src\\fuzz.exe', WindowsPath('D:/a/_temp/qa-assets/fuzz_seed_corpus/miniscript_smart')] miniscript_stable: succeeded against 1871 files in 2s. Run miniscript_stable with args ['D:\\a\\bitcoin\\bitcoin\\src\\fuzz.exe', WindowsPath('D:/a/_temp/qa-assets/fuzz_seed_corpus/miniscript_stable')] miniscript_string: succeeded against 918 files in 3s. Run miniscript_string with args ['D:\\a\\bitcoin\\bitcoin\\src\\fuzz.exe', WindowsPath('D:/a/_temp/qa-assets/fuzz_seed_corpus/miniscript_string')] ``` ACKs for top commit: maflcko: ACK 9155b733e153e799f09cc7f7e9199ad776b2cbb1 TheCharlatan: ACK 9155b733e153e799f09cc7f7e9199ad776b2cbb1 Tree-SHA512: 967f199aac41733265532518ff7b1d881ba5a7bbde9f827d6a0b6d984c94a65b20d5854ce0ea247158eaa17b21d4c9f7d25c79bac17960788bacd2586112630b
2024-05-05test: use tagged ephemeral MiniWallet instance in fill_mempoolSebastian Falbesoner
2024-05-05test: add MiniWallet tagging support to avoid UTXO mixingSebastian Falbesoner
Note that this commit doesn't change behaviour yet, as tagging isn't used in any MiniWallet instance.
2024-05-05test: refactor: eliminate COINBASE_MATURITY magic number in fill_mempoolSebastian Falbesoner
2024-05-05test: refactor: move fill_mempool to new module mempool_utilSebastian Falbesoner
This is needed to avoid circular dependencies in later commits. Can be reviewed via `--color-moved=dimmed-zebra`.
2024-05-04net: reduce LOCK(cs_main) scope in ProcessGetBlockDataAndrew Toth
This also changes behavior if ReadBlockFromDisk or ReadRawBlockFromDisk fails. Previously, the node would crash due to an assert. This has been replaced with logging the error, disconnecting the peer, and returning early.
2024-05-04net: reduce LOCK(cs_main) scope in GETBLOCKTXNAndrew Toth
Also adds a static assertion that MAX_BLOCKTXN_DEPTH <= MIN_BLOCKS_TO_KEEP
2024-05-04build, msvc: Compile test\fuzz\miniscript.cppHennadii Stepanov
2024-05-04Merge bitcoin/bitcoin#28657: miniscript: make operator""_mst constevalmerge-script
63317103c9f2b0635558da814567bb79c17ae851 miniscript: make operator_mst consteval (Pieter Wuille) Pull request description: It seems modern compilers don't realize that all invocations of operator""_mst can be evaluated at compile time, despite the `constexpr` keyword. Since C++20, we can force them to evaluate at compile time using `consteval`, turning all the miniscript type constants into actual compile-time constants. This should give a nice but not very important speedup for miniscript logic, but it's also a way to start testing C++20 features. ACKs for top commit: hebasto: re-ACK 63317103c9f2b0635558da814567bb79c17ae851. theuni: utACK 63317103c9f2b0635558da814567bb79c17ae851 Tree-SHA512: bdc9f1a6499b8bb3ca04f1a158c31e6876ba97206f95ee5718f50efd58b5b4e6b8867c07f791848430bfaa130b9676d8a68320b763cda9a340c75527acbfcc9e
2024-05-04Merge bitcoin/bitcoin#29907: test: Fix `test/streams_tests.cpp` compilation ↵merge-script
on SunOS / illumos 976e5d8f7b2bc77cb1443b8bf0f38cb07db70e9b test: Fix `test/streams_tests.cpp` compilation on SunOS / illumos (Hennadii Stepanov) Pull request description: On systems where `int8_t` is defined as `char`, the `{S,Uns}erialize(Stream&, signed char)` functions become undefined. This PR resolves the issue by testing `{S,Uns}erialize(Stream&, int8_t)` instead. No behavior change on systems where `int8_t` is defined as `signed char`, which is the case for most other systems. Fixes https://github.com/bitcoin/bitcoin/issues/29884. An alternative approach is mentioned in https://github.com/bitcoin/bitcoin/issues/29884#issuecomment-2058434577 as well. ACKs for top commit: maflcko: lgtm ACK 976e5d8f7b2bc77cb1443b8bf0f38cb07db70e9b theuni: ACK 976e5d8f7b2bc77cb1443b8bf0f38cb07db70e9b. Nice to have the serialization concept actually tested :) Tree-SHA512: 1033863e584fa8e99a281b236fa01fc919f610a024bcec792116762e28c1c16ee481bd01325c3a0ca9dd9d753176aa63bd9ac7e08a9bbce772db2949d06f6e61
2024-05-04Merge bitcoin/bitcoin#25972: build: no-longer disable WARN_CXXFLAGS when ↵merge-script
CXXFLAGS is set f0e22be69a15248c42964d57f44ce8c37a36081d build: no-longer disable WARN_CXXFLAGS when CXXFLAGS is set (fanquake) b088062e687d95deff28b0715fd4859449b56584 ci: remove -Wdocumentation from -Werror in multiprocess CI (fanquake) bec6a88fbcff0bd974e820cefd2be2d00b6f6c56 ci: remove -Warray-bounds from -Werror for win64 (fanquake) 7469ac7032e26fe805c5d15ecded2d44253bc9c1 ci: disable -Werror=maybe-uninitialized for Windows builds (fanquake) Pull request description: Now that `CXXFLAGS` are [back in user control](https://github.com/bitcoin/bitcoin/pull/24391), I don't think there's a reason to no-longer use our warning flags when `CXXFLAGS` has been overriden (this includes, by default, when building from depends). Anyone can suppress warnings from third-party code by passing the relevant `-Wno-` options in `CXXFLAGS`. Closes: #18092. ACKs for top commit: maflcko: utACK f0e22be69a15248c42964d57f44ce8c37a36081d 🍡 hebasto: ACK f0e22be69a15248c42964d57f44ce8c37a36081d. theuni: ACK f0e22be69a15248c42964d57f44ce8c37a36081d. It'll be nice to have this fixed. TheCharlatan: ACK f0e22be69a15248c42964d57f44ce8c37a36081d Tree-SHA512: dcef4bd4a57bab6f586ca015fad725e7a38bf24b7a08808a74d8c8ca47cf68c5fca7b04ed38649a047c6929fb708e2c97f2000fc46d5a8d25da49951c5bb0f66
2024-05-03Merge bitcoin/bitcoin#30024: doc: replace remaining "520" magic nums with ↵Ava Chow
MAX_SCRIPT_ELEMENT_SIZE ffc674595cb19b2fdc5705b355bdd3e7f724b860 Replace remaining "520" magic numbers with MAX_SCRIPT_ELEMENT_SIZE (Jon Atack) Pull request description: Noticed these while reviewing BIPs yesterday. It would be clearer and more future-proof to refer to their constant name. ACKs for top commit: instagibbs: ACK ffc674595cb19b2fdc5705b355bdd3e7f724b860 sipa: ACK ffc674595cb19b2fdc5705b355bdd3e7f724b860 achow101: ACK ffc674595cb19b2fdc5705b355bdd3e7f724b860 glozow: ACK ffc674595cb19b2fdc5705b355bdd3e7f724b860, agree it's clearer for these comments to refer to the greppable name of the limit rather than the number Tree-SHA512: 462afc1c64543877ac58cb3acdb01d42c6d08abfb362802f29f3482d75401a2a8adadbc2facd222a9a9fefcaab6854865ea400f50ad60bec17831d29f7798afe
2024-05-03Merge bitcoin/bitcoin#30029: test: remove duplicate `WITNESS_SCALE_FACTOR` ↵Ava Chow
constant definition af3c18169a075222fe0795ab24b8b20ad5e30ae4 [test]: remove duplicate WITNESS_SCALE_FACTOR (ismaelsadeeq) Pull request description: Notice this while working on #29523 - `blocktools.py` and `messages.py` both define `WITNESS_SCALE_FACTOR` constant https://github.com/bitcoin/bitcoin/blob/99d7538cdb2a0ab7a7a2116cd5f33b95fc52b00e/test/functional/test_framework/blocktools.py#L48 https://github.com/bitcoin/bitcoin/blob/99d7538cdb2a0ab7a7a2116cd5f33b95fc52b00e/test/functional/test_framework/messages.py#L68 - This PR deletes the one in `blocktools.py` and update the tests to only use `WITNESS_SCALE_FACTOR` from `messages.py` ACKs for top commit: maflcko: ACK af3c18169a075222fe0795ab24b8b20ad5e30ae4 sipa: ACK af3c18169a075222fe0795ab24b8b20ad5e30ae4 achow101: ACK af3c18169a075222fe0795ab24b8b20ad5e30ae4 glozow: lgtm ACK af3c18169a075222fe0795ab24b8b20ad5e30ae4 brunoerg: ACK af3c18169a075222fe0795ab24b8b20ad5e30ae4 willcl-ark: ACK af3c18169a075222fe0795ab24b8b20ad5e30ae4 Tree-SHA512: 6bd8060c9eea10e03940acee2aa4cd08e4e0afb6d26be3e6300ad405fd0af5b373a00e994eb39515a2dcafa1625562bcd57945049a84b9c9dcc7ea60c24f0911
2024-05-03miniscript: make operator_mst constevalPieter Wuille
It seems modern compilers don't realize that all invocations of operator""_mst can be evaluated at compile time, despite the constexpr keyword. Since C++20, we can force them to evaluate at compile time, turning all the miniscript type constants into actual compile-time constants. It appears that MSVC does not support consteval operator"" when used inside certain expressions. For the few places where this happens, define a constant outside the operator call. Co-Authored-By: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
2024-05-03doc: fix broken relative md linkswillcl-ark
These relative links in our documentation are broken, fix them.
2024-05-03Merge bitcoin/bitcoin#30026: refactor, test: Always initialize pointerRyan Ofsky
bd2de7ac591d7704b79304089ad1fb57e085da8b refactor, test: Always initialize pointer (Hennadii Stepanov) Pull request description: This change fixes MSVC warning [C4703](https://learn.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-4-c4703). All `DisableSpecificWarnings` dropped from `test_bitcoin.vcxproj` as all remained are inherited from `common.init.vcxproj`. Required to simplify warning suppression porting to the CMake-based build system. ACKs for top commit: maflcko: utACK bd2de7ac591d7704b79304089ad1fb57e085da8b sipsorcery: utACK bd2de7ac591d7704b79304089ad1fb57e085da8b. ryanofsky: Code review ACK bd2de7ac591d7704b79304089ad1fb57e085da8b Tree-SHA512: 006db041d3c3697a77d9df14de86cf7c8a10804b45789df01268b2236cf6452e77dc57e89f5d5a6bc26d4b5cd483f0722d6035649c8a523b57954bb1fc810d0c
generated by cgit v1.2.3 (git 2.26.2) at 2024-10-30 19:25:17 +0000