aboutsummaryrefslogtreecommitdiff
path: root/src
AgeCommit message (Collapse)Author
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-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-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-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-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-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
2024-05-03gui: fix misleading signmessage error with segwitwillcl-ark
As described in #10542 (and numerous other places), message signing in Bitcoin Core only supports message signing using P2PKH addresses, 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.
2024-05-03Merge bitcoin/bitcoin#30012: opportunistic 1p1c followupsmerge-script
7f6fb73c82fdff2afe5edefcc335ba6707d5627d [refactor] use reference in for loop through iters (glozow) 6119f76ef72c1adc55c1a384c20f8ba9e1a01206 Process every MempoolAcceptResult regardless of PackageValidationResult (glozow) 2b482dc1f3fb248ccbe7eeb8c9c07d4bf1295a70 [refactor] have ProcessPackageResult take a PackageToValidate (glozow) c2ada0530719e044bb498e0d78907a208214a71e [doc] remove redundant PackageToValidate comment (glozow) 9a762efc7a118c44556fa9ad404f6b54103bad41 [txpackages] use std::lexicographical_compare instead of sorting hex strings (glozow) 8496f69e1c2d1961db2604829cb6a289eb8dd3d6 [refactor] make MempoolAcceptResult::m_replaced_transactions non-optional (glozow) Pull request description: Followups from #28970: - https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1568781077 - https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1585554972 - https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1581019326 - https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1581036209 - https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1586258730 ACKs for top commit: instagibbs: reACK https://github.com/bitcoin/bitcoin/pull/30012/commits/7f6fb73c82fdff2afe5edefcc335ba6707d5627d Tree-SHA512: 9d8393d5f2fedbc6ebce582ff2a8ed074a02dd8e7dbf562c14d48b439fdc1ee6c3203b3609366d3c883d44655cc1a5c83a75ca56e490d25c1a34d95a0622d458
2024-05-03Merge bitcoin/bitcoin#30017: refactor, fuzz: Make 64-bit shift explicitmerge-script
b50d127a7710d790c2ba4a08f01b832c2a0b1203 refactor: Make 64-bit shift explicit (Hennadii Stepanov) Pull request description: This PR fixes MSVC warning [C4334](https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-3-c4334) in the fuzzing code. Similar to https://github.com/bitcoin/bitcoin/pull/26252. All `DisableSpecificWarnings` dropped from `fuzz.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 b50d127a7710d790c2ba4a08f01b832c2a0b1203 sipsorcery: utACK b50d127a7710d790c2ba4a08f01b832c2a0b1203 Tree-SHA512: 18f6082b4234506ad2f9df54e577031b97cdf9f7ef64cad4162f275660716ab73587a97d3af0f778dfd48d2751d8676b5d3381d0aa837fcc60a09704473a9209
2024-05-02refactor, test: Always initialize pointerHennadii Stepanov
This change fixes MSVC warning C4703. See: 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`.
2024-05-02Merge bitcoin/bitcoin#29961: refactor: remove remaining unused code from ↵Ava Chow
cpp-subprocess 8b52e7f628304e83b0e36fd97e617de0f71c5a62 update comments in cpp-subprocess (check_output references) (Sebastian Falbesoner) 97f159776ec06f767df1d4990aa7d0859140f52f remove unused method `Popen::kill` from cpp-subprocess (Sebastian Falbesoner) 908c51fe4afeba0af500c6275027b1afa1b3bd19 remove commented out code in cpp-subprocess (Sebastian Falbesoner) ff79adbe056220202f7a56d67f788c38fc49ef9f remove unused templates from cpp-subprocess (Sebastian Falbesoner) Pull request description: This PR removes remaining code that is unused within the cpp-subprocess module (templates and commented out code). Happy to add more removals if anyone finds more unused parts. Note that there are some API functions of the `Popen` class that we don't use, e.g. `wait()`, `pid()`, `poll()`, `kill()`, but they sound IMHO common enough to be useful in the future, so not sure how deep we should go there. ACKs for top commit: fjahr: Code review ACK 8b52e7f628304e83b0e36fd97e617de0f71c5a62 achow101: ACK 8b52e7f628304e83b0e36fd97e617de0f71c5a62 hebasto: ACK 8b52e7f628304e83b0e36fd97e617de0f71c5a62. Tree-SHA512: 14c1cd2216185d941923f06fdc7acbeed66cd87e2691d9a352f7309b3e07fe4877b580f598a2e4106f9c48395ed6de00a0bfb5d3c3af9c4624d1956a0f543e99
2024-05-02Replace remaining "520" magic numbers with MAX_SCRIPT_ELEMENT_SIZEJon Atack
2024-05-02refactor: Avoid unused-variable warning in init.cppMarcoFalke
2024-05-02[refactor] use reference in for loop through itersglozow
2024-05-02refactor: Make 64-bit shift explicitHennadii Stepanov
This change fixes MSVC level-3 warning C4334. See: https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-3-c4334 All `DisableSpecificWarnings` dropped from `fuzz.vcxproj` as all remained are inherited from `common.init.vcxproj`.
2024-05-01Merge bitcoin/bitcoin#29120: test: Add test case for spending bare multisigmerge-script
e504b1fa1fa4d014b329dea81bfdf1aa55db238f test: Add test case for spending bare multisig (Brandon Odiwuor) Pull request description: Fixes https://github.com/bitcoin/bitcoin/issues/29113 ACKs for top commit: ajtowns: ACK e504b1fa1fa4d014b329dea81bfdf1aa55db238f ; LGTM and just checking the 1-of-3 case seems fine maflcko: utACK e504b1fa1fa4d014b329dea81bfdf1aa55db238f achow101: ACK e504b1fa1fa4d014b329dea81bfdf1aa55db238f willcl-ark: reACK e504b1fa1fa4d014b329dea81bfdf1aa55db238f Tree-SHA512: 641a12599efa34e1a3eb65b125318df326628fef3e6886410ea9e63a044664fad7bcad46d1d6f41ddc59630746b9963cedb569c2682b5940b32b9225883da8f2
2024-05-01fuzz: txorphan tests fixupsSergi Delgado Segura
Adds the following fixups in txorphan fuzz tests: - Don't bond the output count of the created orphans based on the number of available coins - Allow duplicate inputs, when applicable, but don't store duplicate outpoints Rationale --------- The way the test is currently written, duplicate inputs are allowed based on a random flag (`duplicate_input`). If the flag is unset, upon selecting an outpoint as input for a new transaction, the input is popped to prevent re-selection, and later re-added to the collection (once all inputs have been picked). However, the re-addition to the collection is performed independently of whether the flag was set or not. This means that, if the flag is set, the selected inputs are duplicated which in turn makes these inputs more likely to be re-picked in the following iteration of the loop. Additionally, both the input and output count of the transaction and bonded to the number of available outpoints. This makes sense for the former, but the latter shouldn't be.
2024-05-01rpc: return warnings as an array instead of just a single onestickies-v
The RPC documentation for `getblockchaininfo`, `getmininginfo` and `getnetworkinfo` states that "warnings" returns "any network and blockchain warnings". In practice, only a single warning is returned. Fix that by returning all warnings as an array. As a side benefit, cleans up the GetWarnings() logic.
2024-05-01Process every MempoolAcceptResult regardless of PackageValidationResultglozow
2024-05-01[refactor] have ProcessPackageResult take a PackageToValidateglozow
2024-05-01[doc] remove redundant PackageToValidate commentglozow
2024-05-01[txpackages] use std::lexicographical_compare instead of sorting hex stringsglozow
No behavior change, but getting the hex string is more expensive than necessary.
2024-05-01[refactor] make MempoolAcceptResult::m_replaced_transactions non-optionalglozow
2024-05-01scripted-diff: Add IWYU pragma keep to bitcoin-config.h includesMarcoFalke
-BEGIN VERIFY SCRIPT- perl -0777 -pi -e 's/#if defined\(HAVE_CONFIG_H\)\n#include <config\/bitcoin-config.h>.*\n#endif.*\n/#include <config\/bitcoin-config.h> \/\/ IWYU pragma: keep\n/g' $( git grep -l '#include <config/bitcoin-config.h>' ) -END VERIFY SCRIPT-
2024-04-30Merge bitcoin/bitcoin#29983: msvc: Compile `test\fuzz\bitdeque.cpp`Ava Chow
774359b4a96d2724dc70f900cb71e084a77164da build, msvc: Compile `test\fuzz\bitdeque.cpp` (Hennadii Stepanov) 85f50a46c50e7e56b5ee2d7258021939cd80c550 refactor: Fix "error C2248: cannot access private member" on MSVC (Hennadii Stepanov) Pull request description: This PR resolves one point from the https://github.com/bitcoin/bitcoin/pull/29774#issuecomment-2028808614: > What is the issue with the bitdeque... ? ACKs for top commit: maflcko: lgtm ACK 774359b4a96d2724dc70f900cb71e084a77164da sipa: utACK 774359b4a96d2724dc70f900cb71e084a77164da achow101: ACK 774359b4a96d2724dc70f900cb71e084a77164da dergoegge: utACK 774359b4a96d2724dc70f900cb71e084a77164da Tree-SHA512: dba5c0217b915468af08475795437a10d8e8dedfadeb319f36d9b1bf54a91a8b2c61470a6047565855276c2bc8589c7776dc19237610b65b57cc841a303de8b3
2024-04-30Merge bitcoin/bitcoin#28016: p2p: gives seednode priority over dnsseed if ↵Ava Chow
both are provided 82f41d76f1c6ad38290917dad5499ffbe6b3974d Added seednode prioritization message to help output (tdb3) 3120a4678ab2a71a381e847688f44068749cfa97 Gives seednode priority over dnsseed if both are provided (Sergi Delgado Segura) Pull request description: This is a follow-up of #27577 If both `seednode` and `dnsseed` are provided, the node will start a race between them in order to fetch data to feed the `addrman`. This PR gives priority to `seednode` over `dnsseed` so if some nodes are provided as seeds, they can be tried before defaulting to the `dnsseeds` ACKs for top commit: davidgumberg: untested reACK https://github.com/bitcoin/bitcoin/commit/82f41d76f1c6ad38290917dad5499ffbe6b3974d itornaza: tested re-ACK 82f41d76f1c6ad38290917dad5499ffbe6b3974d achow101: ACK 82f41d76f1c6ad38290917dad5499ffbe6b3974d cbergqvist: ACK 82f41d76f1c6ad38290917dad5499ffbe6b3974d Tree-SHA512: 4e39e10a7449af6cd9b8f9f6878f846b94bca11baf89ff2d4fbcd4f28293978a6ed71a3a86cea36d49eca891314c834e32af93f37a09c2cc698a878f84d31c62
2024-04-30Merge bitcoin/bitcoin#29623: Simplify network-adjusted time warning logicAva Chow
c6be144c4b774a03a8bcab5a165768cf81e9b06b Remove timedata (stickies-v) 92e72b5d0d49aa395e626c238bc28aba8e4c3d44 [net processing] Move IgnoresIncomingTxs to PeerManagerInfo (dergoegge) 7d9c3ec622d73a98d07ab3cee78751718982a5bc [net processing] Introduce PeerManagerInfo (dergoegge) ee178dfcc1175e0af8163216c9c024f4bfc97965 Add TimeOffsets helper class (stickies-v) 55361a15d1aa6984051441bce88112000688fb43 [net processing] Use std::chrono for type-safe time offsets (stickies-v) 038fd979effb54ee76ce1b7cf078e920c652326a [net processing] Move nTimeOffset to net_processing (dergoegge) Pull request description: [An earlier approach](https://github.com/bitcoin/bitcoin/commits/1d226ae1f984c5c808f5c24c431b959cdefa692e/) in #28956 involved simplifying and refactoring the network-adjusted time calculation logic, but this was eventually [left out](https://github.com/bitcoin/bitcoin/pull/28956#issuecomment-1904214370) of the PR to make it easier for reviewers to focus on consensus logic changes. Since network-adjusted time is now only used for warning/informational purposes, cleaning up the logic (building on @dergoegge's approach in #28956) should be quite straightforward and uncontroversial. The main changes are: - Previously, we would only calculate the time offset from the first 199 outbound peers that we connected to. This limitation is now removed, and we have a proper rolling calculation. I've reduced the set to 50 outbound peers, which seems plenty. - Previously, we would automatically use the network-adjusted time if the difference was < 70 mins, and warn the user if the difference was larger than that. Since there is no longer any automated time adjustment, I've changed the warning threshold to ~~20~~ 10 minutes (which is an arbitrary number). - Previously, a warning would only be raised once, and then never again until node restart. This behaviour is now updated to 1) warn to log for every new outbound peer for as long as we appear out of sync, 2) have the RPC warning toggled on/off whenever we go in/out of sync, and 3) have the GUI warn whenever we are out of sync (again), but limited to 1 messagebox per 60 minutes - no more globals - remove the `-maxtimeadjustment` startup arg Closes #4521 ACKs for top commit: sr-gi: Re-ACK [c6be144](https://github.com/bitcoin/bitcoin/pull/29623/commits/c6be144c4b774a03a8bcab5a165768cf81e9b06b) achow101: reACK c6be144c4b774a03a8bcab5a165768cf81e9b06b dergoegge: utACK c6be144c4b774a03a8bcab5a165768cf81e9b06b Tree-SHA512: 1063d639542e882186cdcea67d225ad1f97847f44253621a8c4b36c4d777e8f5cb0efe86bc279f01e819d33056ae4364c3300cc7400c087fb16c3f39b3e16b96
2024-04-30Merge bitcoin/bitcoin#28970: p2p: opportunistically accept 1-parent-1-child ↵Ava Chow
packages e518a8bf8abf3d7b83c9013f56d0dca18ae04d6f [functional test] opportunistic 1p1c package submission (glozow) 87c5c524d63c833cf490c7f2f73d72695ad480df [p2p] opportunistically accept 1-parent-1-child packages (glozow) 6c51e1d7d021ed6523107a6db87a865aaa8fc4c9 [p2p] add separate rejections cache for reconsiderable txns (glozow) 410ebd6efaf20fe4715c9b825103b74db69f35ac [fuzz] break out parent functions and add GetChildrenFrom* coverage (glozow) d095316c1c23e9460dfbd9fdbaf292063adcd080 [unit test] TxOrphanage::GetChildrenFrom* (glozow) 2f51cd680fb4323f1c792dae37d4c4e0e0e35804 [txorphanage] add method to get all orphans spending a tx (glozow) 092c978a42e8f4a02291b994713505ba8aac8b28 [txpackages] add canonical way to get hash of package (glozow) c3c1e15831c463df7968b028a77e787da7e6256d [doc] restore comment about why we check if ptx HasWitness before caching rejected txid (glozow) 6f4da19cc3b1b7cd23cb4be95a6bb9acb79eb3bf guard against MempoolAcceptResult::m_replaced_transactions (glozow) Pull request description: This enables 1p1c packages to propagate in the "happy case" (i.e. not reliable if there are adversaries) and contains a lot of package relay-related code. See https://github.com/bitcoin/bitcoin/issues/27463 for overall package relay tracking. Rationale: This is "non-robust 1-parent-1-child package relay" which is immediately useful. - Relaying 1-parent-1-child CPFP when mempool min feerate is high would be a subset of all package relay use cases, but a pretty significant improvement over what we have today, where such transactions don't propagate at all. [1] - Today, a miner can run this with a normal/small maxmempool to get revenue from 1p1c CPFP'd transactions without losing out on the ones with parents below mempool minimum feerate. - The majority of this code is useful for building more featureful/robust package relay e.g. see the code in #27742. The first 2 commits are followups from #29619: - https://github.com/bitcoin/bitcoin/pull/29619#discussion_r1523094034 - https://github.com/bitcoin/bitcoin/pull/29619#discussion_r1519819257 Q: What makes this short of a more full package relay feature? (1) it only supports packages in which 1 of the parents needs to be CPFP'd by the child. That includes 1-parent-1-child packages and situations in which the other parents already pay for themselves (and are thus in mempool already when the package is submitted). More general package relay is a future improvement that requires more engineering in mempool and validation - see #27463. (2) We rely on having kept the child in orphanage, and don't make any attempt to protect it while we wait to receive the parent. If we are experiencing a lot of orphanage churn (e.g. an adversary is purposefully sending us a lot of transactions with missing inputs), we will fail to submit packages. This limitation has been around for 12+ years, see #27742 which adds a token bucket scheme for protecting package-related orphans at a limited rate per peer. (3) Our orphan-handling logic is somewhat opportunistic; we don't make much effort to resolve an orphan beyond asking the child's sender for the parents. This means we may miss packages if the first sender fails to give us the parent (intentionally or unintentionally). To make this more robust, we need receiver-side logic to retry orphan resolution with multiple peers. This is also an existing problem which has a proposed solution in #28031. [1]: see this writeup and its links https://github.com/bitcoin/bips/blob/02ec218c7857ef60914e9a3d383b68caf987f70b/bip-0331.mediawiki#propagate-high-feerate-transactions ACKs for top commit: sr-gi: tACK e518a8bf8abf3d7b83c9013f56d0dca18ae04d6f instagibbs: reACK e518a8bf8abf3d7b83c9013f56d0dca18ae04d6f theStack: Code-review ACK e518a8bf8abf3d7b83c9013f56d0dca18ae04d6f :package: dergoegge: light Code review ACK e518a8bf8abf3d7b83c9013f56d0dca18ae04d6f achow101: ACK e518a8bf8abf3d7b83c9013f56d0dca18ae04d6f Tree-SHA512: 632579fbe7160cb763bbec6d82ca0dab484d5dbbc7aea90c187c0b9833b8d7c1e5d13b8587379edd3a3b4a02a5a1809020369e9cd09a4ebaf729921f65c15943
2024-04-30Merge bitcoin/bitcoin#29906: Disable util::Result copying and assignmentAva Chow
6a8b2befeab25e4e92d8e947a23e78014695e06c refactor: Avoid copying util::Result values (Ryan Ofsky) 834f65e82405bbed336f98996bc8cef366bbed0f refactor: Drop util::Result operator= (Ryan Ofsky) Pull request description: This PR just contains the first two commits of #25665. It disables copying of `util::Result` objects because unnecessary copies are inefficient and not possible after #25665, which makes `util::Result` object move-only. It disables the assignment operator and replaces it with an `Update()` method, because #25665 adds more information to `util::Result` objects (warning and error messages and failure values) and having an assignment operator that overwrites data instead of merging it would make it easy to accidentally erase existing information while trying to assign new information. ACKs for top commit: stickies-v: re-ACK 6a8b2befeab25e4e92d8e947a23e78014695e06c achow101: ACK 6a8b2befeab25e4e92d8e947a23e78014695e06c furszy: re-ACK https://github.com/bitcoin/bitcoin/commit/6a8b2befeab25e4e92d8e947a23e78014695e06c Tree-SHA512: 3f21af9031d50d6c68cca69133de03080f69b1ddcf8b140bdeb762069f14645209b2586037236d15b6ebd8973af0fbefd7e83144aeb7b84078a4cb4df812f984
2024-04-30Merge bitcoin/bitcoin#29990: fuzz: don't allow adding duplicate transactions ↵glozow
to the mempool cc15c5bfd1eb3903de246c124702a7c66c687008 fuzz: don't allow adding duplicate transactions to the mempool (Suhas Daftuar) Pull request description: Filter duplicate transaction ids from being added to the mempool in the `partially_downloaded_block` fuzz target. I think a prerequisite for calling `CTxMemPool::addUnchecked` should be that the underlying txid doesn't already exist in the mempool (otherwise `addUnchecked` would need a way to return failure, which we don't currently have). ACKs for top commit: glozow: utACK cc15c5bfd1eb3903de246c124702a7c66c687008 makes sense to me maflcko: lgtm ACK cc15c5bfd1eb3903de246c124702a7c66c687008 brunoerg: ACK cc15c5bfd1eb3903de246c124702a7c66c687008 dergoegge: utACK cc15c5bfd1eb3903de246c124702a7c66c687008 Tree-SHA512: 85f84ce405aba584e6d00391515f0a86c5648ce8b2da69036e50a6c1f6833d050d09b1972cc5ffbe7c4edb3e5f7f965ef34bd839deeddac27a889cc8d2e53b8f
2024-04-29doc: removed help text saying that peers may not connect automaticallykevkevin
This help text was introduced about two years ago and now a significant portion of users are using version 23.0 and higher
2024-04-29Merge bitcoin/bitcoin#29277: RPC: access RPC arguments by nameRyan Ofsky
30a6c999351041d6a1e8712a9659be1296a1b46a rpc: access some args by name (stickies-v) bbb31269bfa449e82d3b6a20c2c3481fb3dcc316 rpc: add named arg helper (stickies-v) 13525e0c248eab9b199583cde76430c6da2426e2 rpc: add arg helper unit test (stickies-v) Pull request description: Adds string overloads for the `RPCHelpMan::Arg` and `RPCHelpMan::MaybeArg` helpers to be able to access RPC arguments by name instead of index number. Especially in RPCs with a large number of parameters, this can be quite helpful. Example usage: ```cpp const auto action{self.Arg<std::string>("action")}; ``` Most of the LoC is adding test coverage and documentation updates. No behaviour change. An alternative approach to #27788 with significantly less overhaul. ACKs for top commit: fjahr: Code review ACK 30a6c999351041d6a1e8712a9659be1296a1b46a maflcko: ACK 30a6c999351041d6a1e8712a9659be1296a1b46a 🥑 ryanofsky: Code review ACK 30a6c999351041d6a1e8712a9659be1296a1b46a. Nice change! Implementation is surprisingly simple and additional unit test coverage is welcome, too. Tree-SHA512: 4904f5f914fe1d421d32f60edb7c5a028c8ea0f140a2f207a106b4752d441164e073066a6bf2e17693f859fe847815a96609d3cf521e0ac4178d8cd09362ea3d
2024-04-29Merge bitcoin/bitcoin#29872: test: Add missing Assert(mock_time_in >= 0s) to ↵merge-script
SetMockTime fae0db555c12dca75fb09e5fa7bbabdf39b8c1df refactor: Use chrono type for g_mock_time (MarcoFalke) fa382d3dd0592f3cbd6e1de791449f49e06dae86 test: Add missing Assert(mock_time_in >= 0s) to SetMockTime (MarcoFalke) Pull request description: Seems odd to have the assert in the *deprecated* function, but not in the other. Fix this by adding it to the other, and by inlining the deprecated one. Also, use chrono type for the global mocktime variable. ACKs for top commit: davidgumberg: crACK https://github.com/bitcoin/bitcoin/pull/29872/commits/fae0db555c12dca75fb09e5fa7bbabdf39b8c1df stickies-v: ACK fae0db555c12dca75fb09e5fa7bbabdf39b8c1df Tree-SHA512: 630c2917422ff2a7fa307114f95f22ad3c205429ffe36e67f0b2650733e40c876289c1aecebe882a9123d3106db7606bd6eff067ed6e2ecb95765984d3fe8612
2024-04-28fuzz: don't allow adding duplicate transactions to the mempoolSuhas Daftuar
2024-04-28update comments in cpp-subprocess (check_output references)Sebastian Falbesoner
Remove obsolete `check_output` references in the comments and remove the numbering of the Popen API methods, as they don't seem to provide a value and just make diffs larger for future changes.
2024-04-28remove unused method `Popen::kill` from cpp-subprocessSebastian Falbesoner
2024-04-28net: Fix misleading comment for Discoverlaanwj
All network addresses are being iterated over and added, not just the first one per interface.
2024-04-28net: Replace ifname check with IFF_LOOPBACK in Discoverlaanwj
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.
2024-04-28refactor: Fix "error C2248: cannot access private member" on MSVCHennadii Stepanov
2024-04-28Merge bitcoin/bitcoin#29774: build: Enable fuzz binary in MSVCmerge-script
18fd522ca9a74cf8690a6c9b9b729e78c6ed41fd ci, msvc: Add "Run fuzz binaries" step (Hennadii Stepanov) 52933d7283736fe3ae15e7ac44c02ca3bd95fe6d fuzz: Pass `SystemRoot` environment variable to subprocess (Hennadii Stepanov) 23cb8207cdd6c674480840b76626039cdfe7cb13 ci, msvc: Add "Clone fuzz corpus" step (Hennadii Stepanov) 19dceddf4bcdb74e84cf27229039a239b873d41b build, msvc: Build `fuzz.exe` binary (Hennadii Stepanov) 4c078d7bd278fa8b4db6e1da7b9b747f49a8ac4c build, msvc: Enable preprocessor conformance mode (Hennadii Stepanov) 09f5a74198c328c80539c17d951a70558e6b361e fuzz: Re-implement `read_stdin` in portable way (Hennadii Stepanov) Pull request description: Closes https://github.com/bitcoin/bitcoin/issues/29760. Suggested in https://github.com/bitcoin/bitcoin/pull/29758#issuecomment-2025593572. ACKs for top commit: maflcko: lgtm ACK 18fd522ca9a74cf8690a6c9b9b729e78c6ed41fd 🔍 sipsorcery: tACK 18fd522ca9a74cf8690a6c9b9b729e78c6ed41fd sipa: utACK 18fd522ca9a74cf8690a6c9b9b729e78c6ed41fd Tree-SHA512: 672ed6926ee9091f68f13780e77b60fc1d48731f16e847d849374f8426ffe1dafd9bcab06a27af62e8052ba345bb57f20f40579d6be8540c12ef85c23a6eec8b
2024-04-26test: adds outbound eviction functional tests, updates comment in ↵Sergi Delgado Segura
ConsiderEviction
2024-04-26[p2p] opportunistically accept 1-parent-1-child packagesglozow
2024-04-26[p2p] add separate rejections cache for reconsiderable txnsglozow
2024-04-26[fuzz] break out parent functions and add GetChildrenFrom* coverageglozow
It's very hard to randomly construct a transaction that would be the parent of an existing orphanage tx. For functions like AddChildrenToWorkSet and GetChildren that take orphan parents, use a tx that was previously constructed.
2024-04-26[unit test] TxOrphanage::GetChildrenFrom*glozow
2024-04-26[txorphanage] add method to get all orphans spending a txglozow
2024-04-26[txpackages] add canonical way to get hash of packageglozow
2024-04-26test: Add two more urlDecode testsMarcoFalke