aboutsummaryrefslogtreecommitdiff
path: root/src
AgeCommit message (Collapse)Author
2023-02-06Merge bitcoin/bitcoin#26345: refactor: modernize the implementation of uint256.*Andrew Chow
935acdcc79d1dc5ac04a83b92e5919ddbfa29329 refactor: modernize the implementation of uint256.* (pasta) Pull request description: - Constructors of uint256 to utilize Span instead of requiring a std::vector - converts m_data into a std::array - Prefers using `WIDTH` instead of `sizeof(m_data)` - make all the things constexpr - replace C style functions with c++ equivalents - memset -> std::fill This may also be replaced by std::memset, but I think that std::fill is more idiomatic of modern c++ and readable. - memcpy -> std::copy Note: In practice, implementations of std::copy avoid multiple assignments and use bulk copy functions such as std::memmove if the value type is TriviallyCopyable and the iterator types satisfy LegacyContiguousIterator. (https://en.cppreference.com/w/cpp/algorithm/copy) This could also likely be replaced by std::memcpy, but as said above, I believe the using std::copy is the more c++ way to do anything and is almost guaranteed to compile to the same asm - memcmp -> std::memcmp ACKs for top commit: achow101: ACK 935acdcc79d1dc5ac04a83b92e5919ddbfa29329 hebasto: Approach ACK 935acdcc79d1dc5ac04a83b92e5919ddbfa29329. aureleoules: reACK 935acdcc79d1dc5ac04a83b92e5919ddbfa29329 john-moffett: ACK 935acdcc79d1dc5ac04a83b92e5919ddbfa29329 stickies-v: Approach ACK 935acdcc7 Tree-SHA512: 4f1ba54ff2198eea0e505d41e73d552c84c60f6878d5c85a94a8ab57f39afc94ef8d79258e7afd01fa84ec2a99f4404bb877eecd671f65e1ee9273f3129fc650
2023-02-06Merge bitcoin/bitcoin#27036: test: Remove last uses of snprintf and simplifyMarcoFalke
b8032293e67a3b61ecad531412be5330f7cb39e3 Remove use of snprintf and simplify (John Moffett) Pull request description: These are the only remaining uses of `snprintf` in our project, and they can cause unexpected issues -- for example, see https://github.com/bitcoin/bitcoin/issues/27014. Change them to use our `ToString` (which uses a locale-independent version of `std::to_string`) to convert an `int` to `std::string`. Also remove resulting unused parts of `StringContentsSerializer`. Closes https://github.com/bitcoin/bitcoin/issues/27014 ACKs for top commit: Sjors: tACK b8032293e67a3b61ecad531412be5330f7cb39e3, fixes #27014. Tree-SHA512: c903977e654711929decafe8887d0de13b38a340d7082875acc5d41950d834dcfde074e9cabecaf5f9a760f62c34322297b4b156af29761650ef5803b1a54b59
2023-02-05Merge bitcoin/bitcoin#27030: Update nanobench to version v4.3.10fanquake
82f895d7b540ae421f80305a4f7cbb42905fb2c6 Update nanobench to version v4.3.10 (Martin Leitner-Ankerl) Pull request description: Nothing has changed that would affect Bitcoin's usage of nanobench. Here is a detailed list of the changes * Plenty of clang-tidy updates * documentation updates * faster Rng::shuffle * Enable perf counters on older kernels * Raise default minimum epoch time to 1ms (doesn't effect bitcoin's usage) * Add support for custom information per benchmark ACKs for top commit: hebasto: ACK 82f895d7b540ae421f80305a4f7cbb42905fb2c6, I've reviewed the code, all related changes from #26642 have been implemented. Tree-SHA512: 942518398809a2794617a347ab8182b784a8e822e84de5af078b2531eabb438412d687cac22a21936585e60e07138a89b41c28c9750744c05a3d1053f55cad01
2023-02-05Merge bitcoin/bitcoin#27009: validation: Skip VerifyDB checks of level >=3 ↵fanquake
if dbcache is too small fe683f352480245add0b27fe7efef5fef4c1e8c3 log: Log VerifyDB Progress over multiple lines (Martin Zumsande) 61431e3a57b5613d8715c93c6eae0058e0217eaa validation: Skip VerifyDB checks of level >=3 if dbcache is too small (Martin Zumsande) Pull request description: This is the first two commits from #25574, leaving out all changes to `-verifychain` error-handling : - The Problem of [25563](https://github.com/bitcoin/bitcoin/issues/25563) is that when we skip blocks at level 3 due to an insufficient dbcache (skipping some `DisconnectBlock()` calls), we would still attempt the level 4 checks, attempting to reconnect a block that was never disconnected, leading to an assert in `ConnectBlock()`. Fix this by not attempting level 4 checks in this case. - Logging of verification progress is now split over multiple lines. This is more verbose, but now each update has its own timestamp, and other threads logging concurrently will no longer lead to mangled output. This can be tested with a small `dbcache` value, for example: `bitcoind -signet -dbcache=10` `bitcoin-cli -signet verifychain 4 1000` Fixes #25563 ACKs for top commit: MarcoFalke: review ACK fe683f352480245add0b27fe7efef5fef4c1e8c3 🗄 john-moffett: ACK fe683f352480245add0b27fe7efef5fef4c1e8c3 Tree-SHA512: 3e2e0f8b73cbc518a0fa17912c1956da437787aab95001c110b01048472e0dfe4783c44df22bd903d198069dd2f6b02bfdf74e0b934c7a776f144c2e86cb818a
2023-02-03Merge bitcoin/bitcoin#25966: test: Remove redundant testAndrew Chow
fb1c6c14c11ccd4833c1a24f77c507f098d369ad test: Remove redundant test (yancy) Pull request description: I can't think of any reason to keep this test case around labeled [fix me](https://github.com/bitcoin/bitcoin/blob/master/src/wallet/test/coinselector_tests.cpp#L242). The test was originally added [here](https://github.com/bitcoin/bitcoin/commit/4566ab75f277612425337bf7786c1d3a410d894a) however there was never an assertion about the coins that should be selected, only that a solution is found (which is a redundant solution to the test [above](https://github.com/bitcoin/bitcoin/blob/master/src/wallet/test/coinselector_tests.cpp#L222)). The comment was later added here to [fix](https://github.com/bitcoin/bitcoin/commit/384273260a6ccbcf79dade0830011f528e5a1581) it, however it's unclear what exactly it's testing. A test was later added [here](https://github.com/bitcoin/bitcoin/blob/master/src/wallet/test/coinselector_tests.cpp#L366) where if the [long term fee](https://github.com/bitcoin/bitcoin/blob/master/src/wallet/test/coinselector_tests.cpp#L357) is less than the current [fee](https://github.com/bitcoin/bitcoin/blob/master/src/wallet/test/coinselector_tests.cpp#L356), then select fewer UTXOs, which may have been the original intent. ACKs for top commit: S3RK: ACK fb1c6c14c11ccd4833c1a24f77c507f098d369ad Zero-1729: Concept ACK fb1c6c14c11ccd4833c1a24f77c507f098d369ad achow101: ACK fb1c6c14c11ccd4833c1a24f77c507f098d369ad Tree-SHA512: bce2cdae669c144ffaa130237a1643e3b6728e13d603cebf5d9493c4c7c68b3635868e4d93d210783c2ded2a871f185ca09a2053168c05b26a1e056ff6edf68f
2023-02-03Merge bitcoin/bitcoin#27037: rpc: decode Miniscript descriptor when possible ↵Andrew Chow
in decodescript 6699d850e466a65ddd0802be747188ef40cf9de0 doc: release notes for #27037 (Antoine Poinsot) dfc9acbf0170bde6f2abb879b5584dabd1266531 rpc: decode Miniscript descriptor when possible in decodescript (Antoine Poinsot) Pull request description: The descriptor inference logic would previously always use a dummy signing provider and would never analyze the witness script of a P2WSH scriptPubKey. It's often not possible to infer a Miniscript only from the onchain Script, but it was such a low hanging fruit that it's probably worth having it? Fixes https://github.com/bitcoin/bitcoin/issues/27007. I think it also closes https://github.com/bitcoin/bitcoin/issues/25606. ACKs for top commit: instagibbs: ACK https://github.com/bitcoin/bitcoin/pull/27037/commits/6699d850e466a65ddd0802be747188ef40cf9de0 achow101: ACK 6699d850e466a65ddd0802be747188ef40cf9de0 sipa: utACK 6699d850e466a65ddd0802be747188ef40cf9de0 Tree-SHA512: e592bf1ad45497e7bd58c26b33cd9d05bb3007f1e987bee773d26013c3824e1b394fe4903809d80997d5ba66616cc79d77850cd7e7f847a0efb2211c59466982
2023-02-03Merge bitcoin-core/gui#653: Show watchonly balance only for Legacy walletsHennadii Stepanov
fdb8dc8a5a10c39927dc4b8ddc5e0038a633c50e gui: Show watchonly balance only for Legacy wallets (Andrew Chow) Pull request description: Descriptor wallets do not have a watchonly balance as wallets are designated watchonly or not. Thus we should not be displaying the empty watchonly balance for descriptor wallets. The result is that instead of the send page showing "Watch-only balance: 0.00000000 BTC" for watchonly descriptor wallets, we see the actual balance as "Balance: 10.00000000 BTC" ACKs for top commit: johnny9: tACK fdb8dc8a5a10c39927dc4b8ddc5e0038a633c50e furszy: ACK fdb8dc8a hebasto: ACK fdb8dc8a5a10c39927dc4b8ddc5e0038a633c50e Tree-SHA512: e5c0703a62d25c881c8dadfb9cffd482791f3d437a4ec5ae0088ce1a2069c2455ad6d3ec6c95a4404a3b55fbd727f92694529c35052236951553ca90c4ed31b5
2023-02-03Merge bitcoin-core/gui#705: doc: Fix comment about how wallet txs are sortedHennadii Stepanov
c497a198db6f417d2612078a9fbc101e259fab33 Fix comment about how wallet txs are sorted (John Moffett) Pull request description: The wallet transactions in the node are not sorted by txid (or any hash) since https://github.com/bitcoin/bitcoin/pull/24699. This is how they're stored in memory now: https://github.com/bitcoin-core/gui/blob/835212cd1d8f8fc7f19775f5ff8cc21c099122b2/src/wallet/wallet.h#L397-L399 ACKs for top commit: achow101: ACK c497a198db6f417d2612078a9fbc101e259fab33 jarolrod: ACK c497a198db6f417d2612078a9fbc101e259fab33 Tree-SHA512: e72559991688452ef254474d4235dc75fac655bce04909c3a0eece907360f4c6f57707db9b4373a4bd2271b23c57e863684c33e0728adf48e477c5499cdfdad7
2023-02-03Remove use of snprintf and simplifyJohn Moffett
One test case uses snprintf to convert an int to a string. Change it to use ToString (which uses a locale-independent version of std::to_string). Also remove unnecessary parts of StringContentsSerializer.
2023-02-03rpc: decode Miniscript descriptor when possible in decodescriptAntoine Poinsot
The descriptor inference logic would previously always use a dummy signing provider and would never analyze the witness script of a P2WSH scriptPubKey. Note even a valid Miniscript might not always be decodable from Script without more contextual information (for instance the key preimage for a pk_h).
2023-02-03Merge bitcoin/bitcoin#26875: Tests: Fill out dust limit unit test for known ↵MarcoFalke
types except bare multisig b093f5619f8f9b7d63ee60ff04de00b907b13d64 Fill out dust limit unit test for known types except bare multisig (Greg Sanders) Pull request description: Having the constants checked explicitly in a single spot helps with possible regressions and also useful for documentation. In addition, add a check for undefined v1 witness programs. ACKs for top commit: theStack: Code-review ACK b093f5619f8f9b7d63ee60ff04de00b907b13d64 MarcoFalke: review ACK b093f5619f8f9b7d63ee60ff04de00b907b13d64 🥉 Tree-SHA512: 1421f75471739d29b9ef59b0a925b6b07e4e9af92822dbe56eedfb590be9a00fb0c34312146c7c1b5211906461ed00bfa2eb53c88595c6e5a27694b2dc21df38
2023-02-03Update nanobench to version v4.3.10Martin Leitner-Ankerl
Nothing has changed that would affect Bitcoin's usage of nanobench. Here is a detailed list of the changes * Plenty of clang-tidy updates * documentation updates * faster Rng::shuffle * Enable perf counters on older kernels * Raise default minimum epoch time to 1ms (doesn't effect bitcoin's usage) * Add support for custom information per benchmark
2023-02-02Merge bitcoin/bitcoin#27004: test: Use std::unique_ptr over manual delete in ↵fanquake
coins_tests fab9f7d1bd48198d3e0d3c3a08e404ea73a2bc8d test: Use std::unique_ptr over manual delete in coins_tests (MarcoFalke) Pull request description: Makes the code smaller and easier to read ACKs for top commit: stickies-v: ACK fab9f7d1bd48198d3e0d3c3a08e404ea73a2bc8d john-moffett: ACK fab9f7d1bd48198d3e0d3c3a08e404ea73a2bc8d Tree-SHA512: 30d2d2097906e61fdef47a52fc6a0c5ce2417bc41c3c82dafc1b216c655f31dabf9c1c13759575a696f61bbdfdba3f442be032d5e5145b7a54fae2a927824621
2023-02-02Merge bitcoin-core/gui#695: Fix misleading RPC console wallet messageHennadii Stepanov
576f7b86147447215566f0b15ef0b56cd1282929 Fix misleading RPC console wallet message (John Moffett) Pull request description: ## Misleading message from RPCConsole window ## In certain circumstances, the GUI console will display the message 'Executing command without any wallet' when it is, in fact, using the currently loaded wallet. For instance: ![scr3](https://user-images.githubusercontent.com/116917595/211404066-d49a6cbf-d3c3-4e89-8720-3583c6acf521.gif) In RPC calls, if no wallet is explicitly selected and there is exactly one wallet loaded, the [default](https://github.com/bitcoin-core/gui/blob/39363a4b945114f5e4718f75098f3036e8fe6a1d/src/wallet/rpc/util.cpp#L71-L93) is to act on that loaded wallet. The GUI console acts that way in reality, but sometimes erroneously reports that it's not acting on any particular wallet. The root issue is due to the logic that prevents changing the selected wallet if the RPCConsole is visible: https://github.com/bitcoin-core/gui/blob/39363a4b945114f5e4718f75098f3036e8fe6a1d/src/qt/rpcconsole.cpp#L783-L786 This PR removes that unnecessary logic. This does have some ramifications. Prior to this PR, if a user opened the console window without any wallets loaded, then opened two or more wallets, the RPC console would select "None" of the wallets and any wallet-specific RPCs would fail. However, the behavior was different if the user hadn't had the console window open. In that case, if they opened the RPC Console window _after_ loading at least the first wallet, it would select the first-loaded wallet. This context-dependent behavior is (IMO) undesirable, and this PR changes it to be consistent. ACKs for top commit: hebasto: ACK 576f7b86147447215566f0b15ef0b56cd1282929, tested on Ubuntu 22.04 (Qt 5.15.3). Tree-SHA512: 627da186025ba4f4e8df7fdd1b10363f923c4ecc50f023bbf2aece6e2593da65c45147c933effaca9040f813a6e46f034fc2d1ee2fb0f401000a3a6221a0e36e
2023-02-02Merge bitcoin-core/gui#704: Correctly limit overview transaction listHennadii Stepanov
08209c039ff4ca5be4982da7a2ab7a624117ce1a Correctly limit overview transaction list (John Moffett) Pull request description: Fixes #703 The way the main overview page limits the number of transactions displayed (currently 5) is not an appropriate use of Qt. Our subclassed transaction sort/filter proxy model returns a maximum of `5` in `rowCount()`. However, the model itself actually may hold significantly more. While this has _worked_, it breaks the contract of `rowCount()`. If `bitcoin-qt` is run with a DEBUG build of Qt, it'll result in an assert-crash in certain relatively common situations (see #703 for details). Instead of artificially limiting the `rowCount()` in the subclassed filter, we can hide/unhide the rows in the displaying `QListView` upon any changes in the sorted proxy filter. I loaded a wallet with 20,000 transactions and did not notice any performance differences between master and this branch. For reference, this is the list I'm referring to: <img width="934" alt="image" src="https://user-images.githubusercontent.com/116917595/214947304-3f289380-3510-487b-80e8-d19428cf2f0f.png"> ACKs for top commit: Sjors: tACK 08209c039ff4ca5be4982da7a2ab7a624117ce1a hebasto: ACK 08209c039ff4ca5be4982da7a2ab7a624117ce1a, tested on Ubuntu 22.04. Tree-SHA512: c2a7b1a2a6e6ff30694830d7c722274c4c47494a81ce9ef25f8e5587c24871b02343969f4437507693d4fd40ba7a212702b159cf54b3357d8d76c02bc8245113
2023-02-02Merge bitcoin/bitcoin#26992: refactor: Remove unused CDataStream ↵fanquake
SerializeMany constructor fa47b28dfc2a6577519e10da68ebd8da93568434 refactor: Remove unused CDataStream SerializeMany constructor (MarcoFalke) Pull request description: Seems odd to have an unused method. Moreover, the function is fragile and dangerous, because one could have a `std::vector vec_a` and type `CDataStream{vec_a, 0, 0}.size()` and `CDataStream{0, 0, vec_a}.size()`, assuming they are the same thing, when in fact they are not. (The first takes over the memory as is, the second serializes the vector). So my suggestion would be to remove the unused method and introduce a new method when this functionality is needed. For example: `static DataStream FromMany(Args&&... args)`. ACKs for top commit: stickies-v: ACK fa47b28dfc2a6577519e10da68ebd8da93568434 Tree-SHA512: 9593a034b997e33a0794f779f76f02425b1097b218cf8cb1facb7f874fa69da328ce567a79138015baeebe004ae7d103dda4f64f83e8ad375b6dae6b66d3d950
2023-02-02Merge bitcoin/bitcoin#27005: util: Use steady clock for logging timerfanquake
fad7af700e3f57d16631e27fbe2fd7aaa6c9a950 Use steady clock for logging timer (MarcoFalke) Pull request description: The logging timer has many issues: * The underlying clock is mockable, meaning that benchmarks are useless when mocktime was set at the beginning or end of the benchmark. * The underlying clock is not monotonic, meaning that benchmarks are useless when the system time was changed during the benchmark. Fix all issues in this patch. ACKs for top commit: stickies-v: Approach ACK fad7af700e3f57d16631e27fbe2fd7aaa6c9a950 john-moffett: ACK fad7af700e3f57d16631e27fbe2fd7aaa6c9a950 Tree-SHA512: bec8da0f338ed4611e1807937575e1b2afda25139d88015b1c29fa7d13946fbfbc4ee589b576c0508d505df5e5fafafcbc07d63ce4bab4b01475260d9d5d2107
2023-02-01Merge bitcoin/bitcoin#26910: wallet: migrate wallet, exit early if no legacy ↵Andrew Chow
data exist 6d31900e52efa2c7c7a220d8c8ad6353c412a2aa wallet: migrate wallet, exit early if no legacy data exist (furszy) Pull request description: The process first creates a backup file then return an error, without removing the recently created file, when notices that the db is already running sqlite. ACKs for top commit: john-moffett: ACK 6d31900e52efa2c7c7a220d8c8ad6353c412a2aa achow101: ACK 6d31900e52efa2c7c7a220d8c8ad6353c412a2aa ishaanam: crACK 6d31900e52efa2c7c7a220d8c8ad6353c412a2aa Tree-SHA512: 9fb52e80de96e129487ab91bef13647bc4570a782003b1e37940e2a00ca26283fd24ad39bdb63a984ae0a56140b518fd0d74aa2fc59ab04405b2c179b7d3c54a
2023-02-01Merge bitcoin/bitcoin#27015: p2p: 26847 fixups (AddrMan totals)fanquake
dc70c1eb08ba8f0e77ac0810312a67468ade9419 addrman: Use std::nullopt instead of {} (Martin Zumsande) 59cc66abb945c11f30fa571899127275528c5fce test: Remove AddrMan unit test that fails consistency checks (Martin Zumsande) Pull request description: Two fixups for #26847: * Now that `AddrMan::Size()` performs internal consistency tests (it didn't before), we can't call it in the `load_addrman_corrupted` unit tests, where we deal with an artificially corrupted `AddrMan`. This would fail the test when using `-checkaddrman=1` (leading to spurious CI fails). Therefore remove the tests assertion, which is not particularly helpful anyway (in production we abort init when peers.dat is corrupted instead of querying AddrMan in its corrupted state). (See https://github.com/bitcoin/bitcoin/pull/26847#issuecomment-1411458339) * Use `std::nullopt` instead of `{}` for default args (suggested in https://github.com/bitcoin/bitcoin/pull/26847#discussion_r1090643603) ACKs for top commit: MarcoFalke: lgtm ACK dc70c1eb08ba8f0e77ac0810312a67468ade9419 Tree-SHA512: dd8a988e23d71a66d3dd30560bb653c9ad17db6915abfa5f722818b0ab18921051ec9223bfbc75d967df8bcd204dfe473d680bf68e8a8e4e4998fbb91dc973c5
2023-02-01Merge bitcoin/bitcoin#26935: refactor: Fix clang-tidy ↵fanquake
readability-const-return-type violations fa451d4b60ee0538b3ea6b946740a64734b35b6d Fix clang-tidy readability-const-return-type violations (MarcoFalke) Pull request description: This comes up during review, so instead of wasting review cycles on this, just enforce it via CI ACKs for top commit: stickies-v: utACK fa451d4b6 hebasto: ACK fa451d4b60ee0538b3ea6b946740a64734b35b6d. Tree-SHA512: 144a85612f00ec43f7ea1fdaa11901ca981a9f0465a8849745712d741b201b9c3307118172ee0b8efd12bebf25bc6f32a6e2c908495e371f9ada0a917994f44e
2023-02-01addrman: Use std::nullopt instead of {}Martin Zumsande
2023-02-01test: Remove AddrMan unit test that fails consistency checksMartin Zumsande
Now that Size() performs internal consistency checks, it will rightfully fail (and assert) when dealing with a corrupted AddrMan. Therefore remove this check.
2023-02-01Merge bitcoin/bitcoin#27010: refactor: use `Hash` helpers for double-SHA256 ↵MarcoFalke
calculations 87f11ef47fea31d51bcc3f5df68f78fb28e3d8dd refactor: use `Hash` helper for double-SHA256 calculations (Sebastian Falbesoner) Pull request description: We have two helper templates `Hash(const T& in1)` and `Hash(const T& in1, const T& in2)` available for calculating the double-SHA256 hash of one object or two concatenated objects, respectively: https://github.com/bitcoin/bitcoin/blob/b5868f4b1f884e8d6612f34ca4005fe3a992053d/src/hash.h#L74-L89 This PR uses them in order to increase readability and simplify the code. As in #15294 (which inspired this PR, doing the same for RIPEMD160), the helper is not utilized in validation.cpp and script/interpreter.cpp to avoid touching consensus-relevant code. ACKs for top commit: john-moffett: ACK 87f11ef47fea31d51bcc3f5df68f78fb28e3d8dd stickies-v: ACK 87f11ef47fea31d51bcc3f5df68f78fb28e3d8dd MarcoFalke: review ACK 87f11ef47fea31d51bcc3f5df68f78fb28e3d8dd 😬 Tree-SHA512: 11d7e3d00c89685107784010fbffb33ccafb4d1b6a76c4dceb937b29bb234ef4d54581b16bd0737c8d2994a90cf4fe10a9738c7cc5b6d085c6a819f06176dab9
2023-02-01Merge bitcoin/bitcoin#26991: doc: followups to #26471glozow
47c174d8ce4c5f36c41203aedde86c5f0da90217 doc: NetPermissionFlags for tx relay in blocksonly (willcl-ark) e325e0fccba4981d28053b79473ddaa44355e6e8 doc: Fix comment syntax error (willcl-ark) Pull request description: Fix syntax error and specify `NetPermissionFlags` for whitelisted tx relay ACKs for top commit: w0xlt: ACK https://github.com/bitcoin/bitcoin/pull/26991/commits/47c174d8ce4c5f36c41203aedde86c5f0da90217 Tree-SHA512: eb579dc599a96a3ea79c01ac3e76160ec59cf71c2486c9401da8fbbd96ae756ba647aa9ba874835946bc76ba02782729da788617f982ae5a852139e10e7dfd75
2023-02-01Merge bitcoin/bitcoin#25974: test, build: Separate `read_json` function into ↵fanquake
its own module 7a820cee0e6408f5848799011d82dd29ee7fa8c5 test, build: Separate `read_json` function into its own module (Hennadii Stepanov) Pull request description: Currently, 4 source files rely on the definition of the `read_json` function provided in `src/test/script_tests.cpp`. This PR breaks this entanglement, improves code structure and maintainability. ACKs for top commit: fanquake: ACK 7a820cee0e6408f5848799011d82dd29ee7fa8c5 Tree-SHA512: f1567989f76cb54ab86cc48927851a8c424b08a9483d02d4918b629e0c792108bad4ccf7fa341d57b0921d91e84bf8fa3b9c07e5fdf12c64d9d5da83e4e464fb
2023-02-01Fix clang-tidy readability-const-return-type violationsMarcoFalke
2023-02-01Merge bitcoin/bitcoin#26705: clang-tidy: Fix ↵MarcoFalke
`modernize-use-default-member-init` in headers and force to check all headers b0e916913cedb8154419ec818bb9094a72fc8379 clang-tidy: Force to check all headers (Hennadii Stepanov) 96ee992ac3535848e2dc717bf284339badd40dcb clang-tidy: Fix `modernize-use-default-member-init` in headers (Hennadii Stepanov) Pull request description: This PR: - fixes the only [remained](https://github.com/bitcoin/bitcoin/pull/26705#issuecomment-1353742082) check in headers, i.e., `modernize-use-default-member-init` - forces `clang-tidy` check all headers Closes bitcoin/bitcoin#26703. ACKs for top commit: MarcoFalke: review ACK b0e916913cedb8154419ec818bb9094a72fc8379 🍹 Tree-SHA512: 4d33fe873094914541ae81968cdb4e7a7a01b3fdd4f25bc6daa8a53f45dab80565a5b3607ddc338f122369ca5a0a2d0d09c8e78cabe1beb6bd50c115bc5c5210
2023-02-01Merge bitcoin/bitcoin#26888: net: simplify the call to vProcessMsg.splice()MarcoFalke
dfc01ccd73e1f12698278d467c241f398da9fc7d net: simplify the call to vProcessMsg.splice() (Vasil Dimov) Pull request description: At the time when ```cpp pnode->vProcessMsg.splice(pnode->vProcessMsg.end(), pnode->vRecvMsg, pnode->vRecvMsg.begin(), it); ``` is called, `it` is certainly `pnode->vRecvMsg.end()` which makes the call equivalent to: ```cpp pnode->vProcessMsg.splice(pnode->vProcessMsg.end(), pnode->vRecvMsg, pnode->vRecvMsg.begin(), pnode->vRecvMsg.end()); ``` which is equivalent to: ```cpp pnode->vProcessMsg.splice(pnode->vProcessMsg.end(), pnode->vRecvMsg); ``` Thus, use the latter. Further, maybe irrelevant, but the latter has constant complexity while the original code is `O(length of vRecvMsg)`. ACKs for top commit: theStack: Code-review ACK dfc01ccd73e1f12698278d467c241f398da9fc7d MarcoFalke: review ACK dfc01ccd73e1f12698278d467c241f398da9fc7d 🐑 jonatack: Light review ACK dfc01ccd73e1f12698278d467c241f398da9fc7d Tree-SHA512: 9f4eb61d1caf4af9a61ba2f54b915fcfe406db62c58ab1ec42f736505b6792e9379a83d0458d6cc04f289edcec070b7c962f94a920ab51701c3cab103152866f
2023-01-31Merge bitcoin/bitcoin#26847: p2p: track AddrMan totals by network and table, ↵Andrew Chow
improve precision of adding fixed seeds 80f39c99ef2d30e3e2d8dbc068d25cf92aa32344 addrman, refactor: combine two size functions (Amiti Uttarwar) 4885d6f197736cb89fdfac250b280ec10829d903 addrman, refactor: move count increment into Create() (Martin Zumsande) c77c877a8e916878e09f64b2faa12eeca7528cc8 net: Load fixed seeds from reachable networks for which we don't have addresses (Martin Zumsande) d35595a78a4a6cae72d3204c1ec3f82f77a10d56 addrman: add function to return size by network and table (Martin Zumsande) Pull request description: AddrMan currently doesn't track the number of its entries by network, it only knows the total number of addresses. This PR makes AddrMan keep track of these numbers, which would be helpful for multiple things: 1. Allow to specifically add fixed seeds to AddrMan of networks where we don't have any addresses yet - even if AddrMan as a whole is not empty (partly fixing #26035). This is in particular helpful if the user abruptly changes `-onlynet` settings (such that addrs that used to be reachable are no longer and vice versa), in which case they currently could get stuck and not find any outbound peers. The second commit of this PR implements this. 1. (Future work): Add logic for automatic connection management with respect to networks - such as making attempts to have at least one connection to each reachable network as suggested [here](https://github.com/bitcoin/bitcoin/issues/26035#issuecomment-1249420209). This would involve requesting an address from a particular network from AddrMan, and expanding its corresponding function `AddrMan::Select()` to do this requires internal knowledge of the current number of addresses for each network and table to avoid getting stuck in endless loops. 1. (Future work): Perhaps display the totals to users. At least I would find this helpful to debug, the existing option (`./bitcoin-cli -addrinfo`) is rather indirect by doing the aggregation itself in each call, doesn't distinguish between new and tried, and being based on `AddrMan::GetAddr()` it's also subject to a quality filter which we probably don't want in this spot. ACKs for top commit: naumenkogs: utACK 80f39c9 stratospher: ACK 80f39c9 achow101: ACK 80f39c99ef2d30e3e2d8dbc068d25cf92aa32344 vasild: ACK 80f39c99ef2d30e3e2d8dbc068d25cf92aa32344 Tree-SHA512: 6359f2e3f4db7c120c0789d92d74cb7d87a2ceedb7d6a34b5eff20c7f55c5c81092d10ed94efe29afc1c66947820a0d9c14876ee0c8d1f8e068a6df4e1131927
2023-01-31refactor: use `Hash` helper for double-SHA256 calculationsSebastian Falbesoner
2023-01-31Merge bitcoin/bitcoin#23670: build: Build minisketch test in `make check`, ↵fanquake
not in `make` 6d58117a31a88eec3f0a103f9d1fc26cf0b48348 build: Build minisketch test in `make check`, not in `make` (Hennadii Stepanov) Pull request description: On master (d1e42659bbdd8da170542d8c638242cd94f71a7d): ``` $ ./autogen.sh && ./configure --without-gui --disable-wallet && make clean $ make 2>&1 | grep LD | grep -v .la CXXLD bitcoind CXXLD bitcoin-cli CXXLD bitcoin-tx CXXLD bitcoin-util CXXLD test/test_bitcoin CXXLD bench/bench_bitcoin CXXLD minisketch/test CXXLD test/fuzz/fuzz CXXLD univalue/test/object CXXLD univalue/test/unitester $ make check 2>&1 | grep LD CCLD exhaustive_tests CCLD tests ``` With this PR: ``` $ ./autogen.sh && ./configure --without-gui --disable-wallet && make clean $ make 2>&1 | grep LD | grep -v .la CXXLD bitcoind CXXLD bitcoin-cli CXXLD bitcoin-tx CXXLD bitcoin-util CXXLD test/test_bitcoin CXXLD bench/bench_bitcoin CXXLD test/fuzz/fuzz CXXLD univalue/test/object CXXLD univalue/test/unitester $ make check 2>&1 | grep LD CXXLD minisketch/test CCLD exhaustive_tests CCLD tests ``` In fact, this PR restores behavior that was before bitcoin/bitcoin#22646, and that behavior looks more optimal. As an outcome, the `contrib/guix/libexec/build.sh` does not spend resources to build binaries which are not a part of the release package. ACKs for top commit: TheCharlatan: ACK 6d58117a31a88eec3f0a103f9d1fc26cf0b48348 Tree-SHA512: 4957c8f88a01aca005813bf4c1c26f433756bf68ea0c958481c638ead229fa8e23ecae3a8ac31ea555876ba6f2cc10ecd91caf2e2f664de5cb529ec05fb38fa7
2023-01-31Use steady clock for logging timerMarcoFalke
2023-01-31Merge bitcoin/bitcoin#26974: refactor: rpc: set TxToJSON default verbosity ↵MarcoFalke
to SHOW_DETAILS a24e633339c45eaca28fc7af0488956332ac300c refactor: rpc: set TxToJSON default verbosity to SHOW_DETAILS (stickies-v) Pull request description: `TxToJSON()` and `TxToUniv()` are only to be called when we want to decode the transaction (i.e. its details) into JSON. If `TxVerbosity` is `SHOW_TXID`, the function should not have been (and currently is not) called in the first place. There is no behaviour change, current logic simply assumes anything less than `TxVerbosity::SHOW_DETAILS_AND_PREVOUT` equals `TxVerbosity::SHOW_DETAILS`. With this change, the assumptions and intent become more explicit. ACKs for top commit: w0xlt: ACK https://github.com/bitcoin/bitcoin/pull/26974/commits/a24e633339c45eaca28fc7af0488956332ac300c Tree-SHA512: b97235adae49b972bdbe10aca1438643fb35ec66a4e57166b1975b3015bc5a06a711feebe4453a8fefe71781e484b21ef80847d8e8a33694a3abcc863accd4d7
2023-01-31log: Log VerifyDB Progress over multiple linesMartin Zumsande
This allows to log a timestamp for each entry, and avoids potential interference with other threads that could log concurrently.
2023-01-31validation: Skip VerifyDB checks of level >=3 if dbcache is too smallMartin Zumsande
The previous behavior, skipping some L3 DisconnectBlock calls, but still attempting to reconnect these blocks at L4, makes ConnectBlock assert. The variable skipped_l3_checks is introduced because even with an insufficient cache for the L3 checks, the L1/L2 checks in the same loop should still be completed. Fixes #25563.
2023-01-31clang-tidy: Force to check all headersHennadii Stepanov
2023-01-31clang-tidy: Fix `modernize-use-default-member-init` in headersHennadii Stepanov
See https://clang.llvm.org/extra/clang-tidy/checks/modernize/use-default-member-init.html
2023-01-31test: Use std::unique_ptr over manual delete in coins_testsMarcoFalke
2023-01-30Add assertions that BatchWrite(erase=true) erasesPieter Wuille
2023-01-30Avoid unclear {it = ++it;}Pieter Wuille
2023-01-30Follow coding style for named argumentsPieter Wuille
2023-01-30Make test/fuzz/coins_view exercise CCoinsViewCache::Sync()Pieter Wuille
2023-01-30Merge bitcoin/bitcoin#26965: refactor: Remove stray cs_main redundant ↵fanquake
declaration faba08b5b4dd5dedb457db18696de6e15839c696 refactor: Remove stray cs_main redundant declaration (MarcoFalke) fa02591edff0255c5120b5acb2366542a61c251f doc: Export threadsafety.h from sync.h (MarcoFalke) Pull request description: Looks like this was forgotten when introducing kernel/cs_main ? Also, there is a commit to export threadsafety.h from sync.h. ACKs for top commit: hebasto: ACK faba08b5b4dd5dedb457db18696de6e15839c696 Tree-SHA512: 0aa58e7693b6fcd504f9da7339f8baa463a6407f67b27f68002db705f4642321ac3765f16c3d906c925ee24085591b79160a62fa5f4aaf6f2e5dcc788411800d
2023-01-30Merge bitcoin/bitcoin#17487: coins: allow write to disk without cache dropfanquake
1d7935b45ac61791399989effc18aece8b368fbb test: add test for coins view flush behavior using Sync() (James O'Beirne) 2c3cbd6c007a588e667751024027462268626fdb test: add use of Sync() to coins tests (James O'Beirne) 6d8affca96c7a34f5f104c5a3122e7420ffc083c test: refactor: clarify the coins simulation (James O'Beirne) 79cedc36afe2e72e42839d861734d73d545d21b8 coins: add Sync() method to allow flush without cacheCoins drop (James O'Beirne) Pull request description: This is part of the [assumeutxo project](https://github.com/bitcoin/bitcoin/projects/11): Parent PR: #15606 Issue: #15605 Specification: https://github.com/jamesob/assumeutxo-docs/tree/master/proposal --- In certain circumstances, we may want to flush chainstate data to disk without emptying `cacheCoins`, which affects performance. UTXO snapshot activation is one such case, as we populate `cacheCoins` with the snapshot contents and want to persist immediately afterwards but also enter IBD. See also #15265, which makes the case that under normal operation a flush-without-erase doesn't necessarily add much benefit. I open this PR even in light of the previous discussion because (i) flush-without-erase almost certainly provides benefit in the case of snapshot activation (especially on spinning disk hardware) and (ii) this diff is fairly small and gives us convenient options for more granular cache management without changing existing policy. See also #15218. ACKs for top commit: sipa: ACK 1d7935b45ac61791399989effc18aece8b368fbb achow101: ACK 1d7935b45ac61791399989effc18aece8b368fbb Sjors: tACK 1d7935b45ac61791399989effc18aece8b368fbb Tree-SHA512: 897583963e98661767d2d09c9a22f6019da24125558cd88770bfe2d017d924f23a9075b729e4b1febdec5b0709a38e8fa1ef94d62aa88650556b06cb4826c845
2023-01-30Merge bitcoin/bitcoin#26649: refactor: Use AutoFile and HashVerifier ↵fanquake
(without ser-type and ser-version) where possible eeee61065fe165dcce9625f7cc4cfce9e432aafa Use AutoFile and HashVerifier where possible (MarcoFalke) fa961141f7fc515fbb6fcb9d8417108034b256ce Add HashVerifier (MarcoFalke) Pull request description: This was done in the context of https://github.com/bitcoin/bitcoin/pull/25284 , but I think it also makes sense standalone. The basic idea is that serialization type should not be initialized when it is not needed. Same for the serialization version. So do this here for `AutoFile` and `HashVerifier`. `CAutoFile` and `CHashVerifier` remain in places where it is not yet possible. ACKs for top commit: stickies-v: ACK eeee61065fe165dcce9625f7cc4cfce9e432aafa Tree-SHA512: 93786778c309ecfdc1ed43552d24ff9d966954d69a47f66faaa6de24daacd25c651f3f62bde5abbb362700298fb3c04ffbd3207a0dd13d0bd8bff7fd6d07dcf8
2023-01-30refactor: Remove unused CDataStream SerializeMany constructorMarcoFalke
2023-01-30Merge bitcoin/bitcoin#26896: build: Remove port-forwarding runtime setting ↵fanquake
options from configure d51f0fa4b7b19281efe65aacf414845c661d0a13 doc: add release notes for 26896 (fanquake) 2b248798d96f794db08b7725730b5fb4e00b9b10 build: remove --enable-upnp-default from configure (fanquake) 02f5a5e7b5fd7ba35e407d4409202a0e0fed003c build: remove --enable-natpmp-default from configure (fanquake) 25a0e8ba0b31d8bd265df0589fe49241a60d0fc2 Remove configure-time setting of DEFAULT_UPNP (fanquake) 06562e5fa771dab275a9cab4914cd64d961a52bc Remove configure-time setting of DEFAULT_NATPMP (fanquake) Pull request description: This PR removes the `--enable-upnp-default` and `--enable-natpmp-default` options from configure. It's odd to me that we maintain configure-time options for setting the default port-forwarding runtime state (but no other similar options), and I'm not sure what use-case it satisfies, that can't be achieved by multiple other means. I also doubt that we'll ever restart using these in release builds, or turning on any of this by default. I think the only scenario these options would be used is when you want to compile your own binaries (we don't use them in Guix), with port-forwarding on by default, but otherwise can't or don't want to use a `.conf` file, can't or don't want to pass command line options at runtime, and also don't want to modify the source code? ACKs for top commit: hebasto: ACK d51f0fa4b7b19281efe65aacf414845c661d0a13, rebased and comments have been addressed since my recent [review](https://github.com/bitcoin/bitcoin/pull/26896#pullrequestreview-1273910740). TheCharlatan: ACK d51f0fa4b7b19281efe65aacf414845c661d0a13 Tree-SHA512: 481decd8bddd8b03b7319591e3acf189f7b6b96c9a9a8c5bc1a3f8ec00d0b8f9b52d2f5c28a298a2ec947cfe9611cfd184e393ccb2e4e21bfce86ca7d4de60d3
2023-01-30doc: Fix comment syntax errorwillcl-ark
2023-01-30net: simplify the call to vProcessMsg.splice()Vasil Dimov
At the time when ```cpp pnode->vProcessMsg.splice(pnode->vProcessMsg.end(), pnode->vRecvMsg, pnode->vRecvMsg.begin(), it); ``` is called, `it` is certainly `pnode->vRecvMsg.end()` which makes the call equivalent to: ```cpp pnode->vProcessMsg.splice(pnode->vProcessMsg.end(), pnode->vRecvMsg, pnode->vRecvMsg.begin(), pnode->vRecvMsg.end()); ``` which is equivalent to: ```cpp pnode->vProcessMsg.splice(pnode->vProcessMsg.end(), pnode->vRecvMsg); ``` Thus, use the latter. Further, maybe irrelevant, but the latter has constant complexity while the original code is `O(length of vRecvMsg)`.
2023-01-30Merge bitcoin/bitcoin#26499: wallet: Abandon descendants of orphaned coinbasesglozow
b0fa5989e1b77a343349bd36f8bc407f9366a589 test: Check that orphaned coinbase unconf spend is still abandoned (Andrew Chow) 9addbd78901124a48fd41a82a9557fcf3490191d wallet: Automatically abandon orphaned coinbases and their children (Andrew Chow) Pull request description: When a block is reorged out of the main chain, any descendants of the coinbase will no longer be valid. Currently they are only marked as inactive, which means that our balance calculations will still include them. In order to be excluded from the balance calculation, they need to either be abandoned or conflicted. This PR goes with the abandoned method. Note that even when they are included in balance calculations, coin selection will not select outputs belonging to these transactions because they are not in the mempool. Fixes #14148 ACKs for top commit: furszy: ACK b0fa5989 with a not-blocking nit. aureleoules: reACK b0fa5989e1b77a343349bd36f8bc407f9366a589 ishaanam: ACK b0fa5989e1b77a343349bd36f8bc407f9366a589 Tree-SHA512: 68f12e7aa8df392d8817dc6ac0becce8dbe83837bfa538f47027e6730e5b2e1b1a090cfcea2dc598398fdb66090e02d321d799f087020d7e1badcf96e598c3ac
generated by cgit v1.2.3 (git 2.26.2) at 2024-11-13 01:24:15 +0000