aboutsummaryrefslogtreecommitdiff
path: root/src
AgeCommit message (Collapse)Author
2022-05-10validation: replace ::Params() calls with chainstate/chainman memberAnthony Towns
2022-05-10validation: remove redundant CChainParams params from ChainstateManager methodsAnthony Towns
2022-05-10validation: add CChainParams to ChainstateManagerAnthony Towns
2022-05-09Merge bitcoin/bitcoin#24946: Unroll the ChaCha20 inner loop for performanceMacroFake
81c09ee45caecf8d9daf6766b94cebf54f3f08cd Unroll the ChaCha20 inner loop for performance (Pieter Wuille) Pull request description: Unrolling the inner ChaCha20 loop gives a ~15% speedup for me in the CHACHA20_* benchmarks. It's a simple change, this performance helps with RNG generation, and will matter more for BIP324. ACKs for top commit: martinus: tested ACK 81c09ee with clang++ 13.0.1, test `CHACHA20_1MB`: MarcoFalke: ACK 81c09ee45caecf8d9daf6766b94cebf54f3f08cd 🍟 Tree-SHA512: 108bd0ba573bb08de92d611e7be7c09a2c2700f9655f44129b87f9b71f7e101dfc6bd345783e7b4b9b40f0b003913cf59187f422da8cdb5b20887f7855b2611a
2022-05-06Merge bitcoin/bitcoin#24804: Sanity assert GetAncestor() != nullptr where ↡MacroFake
appropriate 308dd2e93e92f4cac4e7d75478316af9bb2b77b8 Sanity assert GetAncestor() != nullptr where appropriate (Adam Jonas) Pull request description: Re-opening #17232. I have rebased the PR and addressed jonatack's nit suggestions. Add sanity asserts for return value of `CBlockIndex::GetAncestor()` where appropriate. In validation.cpp `CheckSequenceLocks`, check the return value of `tip->GetAncestor(maxInputHeight)` stored into `lp->maxInputBlock`. If it ever returns `nullptr` because the ancestor isn't found, it's going to be a bad bug to keep going, since a `LockPoints` object with the `maxInputBlock` member set to `nullptr` signifies no relative lock time. In the other places, the added asserts would prevent accidental dereferencing of a null pointer which is undefined behavior. Co-Authored-By: Adam Jonas <jonas@chaincode.com> Co-Authored-By: danra <danra@users.noreply.github.com> ACKs for top commit: jonatack: ACK 308dd2e93e92f4cac4e7d75478316af9bb2b77b8 Tree-SHA512: 5bfdaab1499607ae2c3cd3e2e9e8c37850bfd0e327e680f4e36c81f9c6d98a543af78ecfac1ab0e06325d264412615a04d52005875780c7db2a4d81bd2d2259a
2022-05-06Merge bitcoin/bitcoin#19426: refactor: Change * to & in ↡MacroFake
MutableTransactionSignatureCreator fac6cfc50f65c610f2df9af3ec2efff5eade6661 refactor: Change * to & in MutableTransactionSignatureCreator (MarcoFalke) Pull request description: The `MutableTransactionSignatureCreator` constructor takes in a pointer to a mutable transaction. This is problematic for several reasons: * It would be undefined behaviour to pass in a nullptr because for signature creation, the memory of the mutable transaction is accessed * No caller currently passes in a nullptr, so passing a reference as a pointer is confusing Fix all issues by replacing `*` with `&` in `MutableTransactionSignatureCreator` ACKs for top commit: theStack: Code-review ACK fac6cfc50f65c610f2df9af3ec2efff5eade6661 jonatack: ACK fac6cfc50f65c610f2df9af3ec2efff5eade6661 Tree-SHA512: d84296b030bd4fa2709e5adbfe43a5f8377d218957d844af69a819893252af671df7f00004f5ba601a0bd70f3c1c2e58c4f00e75684da663f28432bb5c89fb86
2022-05-06Merge bitcoin/bitcoin#24538: miner: bug fix? update for ancestor inclusion ↡MacroFake
using modified fees, not base e4303c337c8423f21c2c72ee1bcca3aaf46fa1cb [unit test] prioritisation in mining (glozow) 7a8d60676bc0eec289687b2dfd5d2b00b83c0eaa [miner] bug fix: update for parent inclusion using modified fee (glozow) 0f9a44461c294cf21a335e8a8c13e498baac110f MOVEONLY: group miner tests into MinerTestingSetup functions (glozow) Pull request description: Came up while reviewing #24364, where some of us incorrectly assumed that we use the same fee deduction in `CTxMemPoolModifiedEntry::nModFeesWithAncestors` when first constructing an entry and in `update_for_parent_inclusion`. Actually, the behavior is this: when a mempool entry's ancestor is included in the block template, we create a `CTxMemPoolModifiedEntry` for it, subtracting the ancestor's modified fees from `nModFeesWithAncestors`. If another ancestor is included, we update it again, but use the ancestor's _base_ fees instead. I can't explain why we use `GetFee` in one place and `GetModifiedFee` in the other, but I'm quite certain we should be using the same one for both. And should it be base or modified fees? Modified, otherwise the child inherits the prioritisation of the parent, but only until the parent gets mined. If we want prioritisation to cascade down to current in-mempool descendants, we should probably document that in the `prioritsetransaction` helpstring and implement it in `CTxMemPool::mapDeltas`, not as a quirk in the mining code? Wrote a test in which a mempool entry has 2 ancestors, both prioritised, and both included in a block template individually. This test should fail without the s/GetFee/GetModifiedFee commit. ACKs for top commit: ccdle12: tested ACK e4303c3 MarcoFalke: ACK e4303c337c8423f21c2c72ee1bcca3aaf46fa1cb πŸš— Tree-SHA512: 4cd94106fbc9353e9f9b6d5af268ecda5aec7539245298c940ca220606dd0737264505bfaae1f83d94765cc2d9e1a6e913a765048fe6c19292482241761a6762
2022-05-05Wrap boost::replace_allMacroFake
2022-05-05Sanity assert GetAncestor() != nullptr where appropriateAdam Jonas
Add sanity asserts for return value of `CBlockIndex::GetAncestor()` where appropriate. In validation.cpp `CheckSequenceLocks`, check the return value of `tip->GetAncestor(maxInputHeight)` stored into `lp->maxInputBlock`. If it ever returns `nullptr` because the ancestor isn't found, it's going to be a bad bug to keep going, since a `LockPoints` object with the `maxInputBlock` member set to `nullptr` signifies no relative lock time. In the other places, the added asserts would prevent accidental dereferencing of a null pointer which is undefined behavior. Co-Authored-By: Aurèle Oulès <aurele@oules.com> Co-Authored-By: danra <danra@users.noreply.github.com>
2022-05-05Merge bitcoin/bitcoin#24141: Rename message_command variables in src/net* ↡MacroFake
and src/rpc/net.cpp e71c51b27d420fbd6cc0a36f62e63e190e13473a refactor: rename command -> message type in comments in the src/net* files (Shashwat) 2b09593bddb0a93aebf84e5f43cdb4d5282c7984 scripted-diff: Rename message command to message type (Shashwat) Pull request description: This PR is a follow-up to #24078. > a message is not a command, but simply a message of some type The first commit covers the message_command variable name and comments not addressed in the original PR in `src/net*` files. The second commit goes beyond the original `src/net*` limit of #24078 and does similar changes in the `src/rpc/net.cpp` file. ACKs for top commit: MarcoFalke: review ACK e71c51b27d420fbd6cc0a36f62e63e190e13473a πŸ’₯ Tree-SHA512: 24015d132c00f15239e5d3dc7aedae904ae3103a90920bb09e984ff57723402763f697d886322f78e42a0cb46808cb6bc9d4905561dc6ddee9961168f8324b05
2022-05-04Merge bitcoin/bitcoin#22235: script: add script to generate example bitcoin.conflaanwj
b42643c2537ffbe99d6d94fb1fb3b7f9d5234f93 doc: update init.cpp -conf help text (josibake) 970b9987ad5bcb72e581c40a7cdd408d94a48c81 doc: update devtools, release-process readmes (josibake) 50635d27b45d125b6264ac2abfbd6a1129c7228f build: include bitcoin.conf in build outputs (josibake) 6aac946f49aea243de1dc50631bb72f0186bbf58 doc: update bitcoin-conf.md (Josiah Baker) 1c7e820ded0846ef6ab4be9616b0de452336ef64 script: add script to generate example bitcoin.conf (josibake) b483084d866c16d97a34251ae652bac94f85f61d doc: replace bitcoin.conf with placeholder file (josibake) Pull request description: create a script for parsing the output from `bitcoind --help` to create an example conf file for new users ## problem per #10746 , `bitcoin.conf` not being put into the data directory during installation causes some confusion for users when running bitcoin. in the discussion on the issue, one proposed solution was to have an example config file and instruct users to `cp` it into their data directory after startup. in addition to #10746 , there have been other requests for a "skeleton config file" (https://github.com/bitcoin/bitcoin/issues/19641) to help users get started with configuring bitcoind. the main issue with an example config file is that it creates a second source of truth regarding what options are available for configuring bitcoind. this means any changes to the options (including the addition or removal of options) would have to be updated for the command line and also updated in the example file. this PR addresses this issue by providing a script to generate an example file directly from the `bitcoind --help` on-demand by running `contrib/devtools/gen-bitcoin-conf.sh`. this solution was originally proposed on #10746 and would also solve #19641 . this guarantees any changes made to the command-line options or the command-line options help would also be reflected in the example file after compiling and running the script. the main purpose of this script is to generate a config file to be included with releases, same as `gen-manpages.sh`. this ensures every release also includes an up-to-date, full example config file for users to edit. the script is also available for users who compile from source for generating an example config for their compiled binary. ## special considerations this removes the `bitcoin.conf` example file from the repo as it is now generated by this script. the original example file did contain extra text related to how to use certain options but going forward all option help docs should be moved into `init.cpp` this also edits `init.cpp` to have the option help indicate that `-conf` is not usable from the config file. this is similar to how `-includeconf` 's help indicates it cannot be used from the command line ACKs for top commit: laanwj: Tested and code review ACK b42643c2537ffbe99d6d94fb1fb3b7f9d5234f93 Tree-SHA512: 4546e0cef92aa1398da553294ce4712d02e616dd72dcbe0b921af474e54f24750464ec813661f1283802472d1e8774e634dd1cc26fbf1f13286d3e0406c02c09
2022-05-04Merge bitcoin/bitcoin#24933: util: Replace non-threadsafe strerrorlaanwj
e3a06a3c6cbb288ac89a2725cf71ae8adaebf35c test: Add `strerror` to locale-dependence linter (laanwj) f00fb1265a8bc26e1612c771173325dbe49b3612 util: Increase buffer size to 1024 in SysErrorString (laanwj) 718da302c7b11b375042c3000d421fd93348c199 util: Refactor SysErrorString logic (laanwj) e7f2f77756d33c6be9c8998a575b263ff2d39270 util: Use strerror_s for SysErrorString on Windows (laanwj) 46971c6dbfbc39ebbc74ab1ed8c00edc12859373 util: Replace non-threadsafe strerror (laanwj) Pull request description: Some uses of non-threadsafe `strerror` have snuck into the code since they were removed in #4152. Add a wrapper `SysErrorString` for thread-safe strerror alternatives (with code from `NetworkErrorString`) and replace all uses of `strerror` with this. Edit: I've also added a commit that refactors the code so that buf[] is never read at all if the function fails, making some fragile-looking code unnecessary. Edit2: from the linux manpage: ``` ATTRIBUTES For an explanation of the terms used in this section, see attributes(7). β”Œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”¬β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”¬β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β” β”‚Interface β”‚ Attribute β”‚ Value β”‚ β”œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”Όβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”Όβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€ β”‚strerror() β”‚ Thread safety β”‚ MT-Unsafe race:strerror β”‚ β”œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”Όβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”Όβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€ … β”œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”Όβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”Όβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€ β”‚strerror_r(), β”‚ Thread safety β”‚ MT-Safe β”‚ β”‚strerror_l() β”‚ β”‚ β”‚ β””β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”΄β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”΄β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”˜ ``` As the function can be called from any thread at any time, using a non-thread-safe function is unacceptable. ACKs for top commit: jonatack: ACK e3a06a3c6cbb288ac89a2725cf71ae8adaebf35c Tree-SHA512: 20e71ebb9e979d4e1d8cafbb2e32e20c2a63f09115fe72cdde67c8f80ae98c531d286f935fd8a6e92a18b72607d7bd3e846b2d871d9691a6036b0676de8aaf25
2022-05-04Unroll the ChaCha20 inner loop for performancePieter Wuille
2022-05-04doc: update init.cpp -conf help textjosibake
update help to reflect this option cannot be used from the config file
2022-05-04Merge bitcoin/bitcoin#24852: util: optimize HexStrlaanwj
5e61532e72c1021fda9c7b213bd9cf397cb3a802 util: optimizes HexStr (Martin Leitner-Ankerl) 4e2b99f72a90b956f3050095abed4949aff9b516 bench: Adds a benchmark for HexStr (Martin Leitner-Ankerl) 67c8411c37b483caa2fe3f7f4f40b68ed2a9bcf7 test: Adds a test for HexStr that checks all 256 bytes (Martin Leitner-Ankerl) Pull request description: In my benchmark, this rewrite improves runtime 27% (g++) to 46% (clang++) for the benchmark `HexStrBench`: g++ 11.2.0 | ns/byte | byte/s | err% | ins/byte | cyc/byte | IPC | bra/byte | miss% | total | benchmark |--------------------:|--------------------:|--------:|----------------:|----------------:|-------:|---------------:|--------:|----------:|:---------- | 0.94 | 1,061,381,310.36 | 0.7% | 12.00 | 3.01 | 3.990 | 1.00 | 0.0% | 0.01 | `HexStrBench` master | 0.68 | 1,465,366,544.25 | 1.7% | 6.00 | 2.16 | 2.778 | 1.00 | 0.0% | 0.01 | `HexStrBench` branch clang++ 13.0.1 | ns/byte | byte/s | err% | ins/byte | cyc/byte | IPC | bra/byte | miss% | total | benchmark |--------------------:|--------------------:|--------:|----------------:|----------------:|-------:|---------------:|--------:|----------:|:---------- | 0.80 | 1,244,713,415.92 | 0.9% | 10.00 | 2.56 | 3.913 | 0.50 | 0.0% | 0.01 | `HexStrBench` master | 0.43 | 2,324,188,940.72 | 0.2% | 4.00 | 1.37 | 2.914 | 0.25 | 0.0% | 0.01 | `HexStrBench` branch Note that the idea for this change comes from denis2342 in #23364. This is a rewrite so no unaligned accesses occur. Also, the lookup table is now calculated at compile time, which hopefully makes the code a bit easier to review. ACKs for top commit: laanwj: Code review ACK 5e61532e72c1021fda9c7b213bd9cf397cb3a802 aureleoules: tACK 5e61532e72c1021fda9c7b213bd9cf397cb3a802. theStack: ACK 5e61532e72c1021fda9c7b213bd9cf397cb3a802 🚀 Tree-SHA512: 40b53d5908332473ef24918d3a80ad1292b60566c02585fa548eb4c3189754971be5a70325f4968fce6d714df898b52d9357aba14d4753a8c70e6ffd273a2319
2022-05-04Merge bitcoin/bitcoin#24976: netgroup: Follow-up for #22910fanquake
e5d183151709ab59d2fa6fe9e0243000e8d6abbe [netgroup] Use nStartByte as offset for the last byte of the group (dergoegge) Pull request description: This addresses my review [comments](https://github.com/bitcoin/bitcoin/pull/22910#discussion_r856095896) I left on #22910. This has no effect on the current logic as `nStartByte` is only used for internal addresses which only ever add 10 whole bytes to the returned group. However to avoid future bugs, I think we should use `nStartByte` as offset for the last byte as well, in case we ever add a new address type that makes makes use of `nStartByte` and adds fractional bytes to the group. ACKs for top commit: jnewbery: Code review ACK e5d183151709ab59d2fa6fe9e0243000e8d6abbe theStack: Concept and code-review ACK e5d183151709ab59d2fa6fe9e0243000e8d6abbe Tree-SHA512: 4c08c7d6cb38b553e998798b3e3b790177aaa2141a48e277dfd538e01a7fccadf644329e93c5b0fb5e7e4037494c8dfe061b94eb52c6b31dc21bdf99eb0e311a
2022-05-04Merge bitcoin/bitcoin#25057: refactor: replace remaining boost::split with ↡fanquake
SplitString f849e63bad963b8717d4bc45efdad9b08567a36e fuzz: SplitString with multiple separators (Martin Leitner-Ankerl) d1a9850102fe572b8a1e00b80c757dd82bf39f9d http: replace boost::split with SplitString (Martin Leitner-Ankerl) 0d7efcdf75607e19fac77bcd146773a03af14492 core_read: Replace boost::split with SplitString (Martin Leitner-Ankerl) b7ab9db545492927b774912e53aeb834a590621f Extend Split to work with multiple separators (Martin Leitner-Ankerl) Pull request description: As a followup of #22953, this removes the remaining occurrences of `boost::split` and replaces them with our own `SplitString`. To be able to do so, this extends the function `spanparsing::Split` to work with multiple separators. Finally this removes 3 more files from `lint-includes.py`. ACKs for top commit: theStack: Code-review ACK f849e63bad963b8717d4bc45efdad9b08567a36e Tree-SHA512: f37d4dbe11cab2046e646045b0f018a75f978d521443a2c5001512737a1370e22b09247d5db0e5c9e4153229a4e2d66731903c1bba3713711c4cae8cedcc775d
2022-05-04refactor: Change * to & in MutableTransactionSignatureCreatorMarcoFalke
2022-05-04Merge bitcoin/bitcoin#25060: blockstorage: add LIFETIMEBOUND to ↡MacroFake
GetFirstStoredBlock()::start_time 4cb9d214345550cb0299139b2badb73ba1e53532 blockstorage: add LIFETIMEBOUND to GetFirstStoredBlock()::start_time (Jon Atack) Pull request description: Suggested in https://github.com/bitcoin/bitcoin/pull/25016#discussion_r862330288, the lifetimebound attribute here indicates that a resource owned by the `start_block` param of `CBlockIndex* BlockManager::GetFirstStoredBlock()` can be retained by the method's return value, which enables detecting the use of out-of-scope stack memory (ASan `stack-use-after-scope`) at compile time. See https://releases.llvm.org/12.0.0/tools/clang/docs/AttributeReference.html#lifetimebound and #22278 for related discussion, and #25040 for a similar example. ACKs for top commit: MarcoFalke: review ACK 4cb9d214345550cb0299139b2badb73ba1e53532 Tree-SHA512: a3f5ef83ebb6f08555d7c89f2437a682071b4ad77a7aa3326b6d2282c909bf9fcf4dac6bf05ee1d9931f2102cad4a02df5468bde1cf377d7126e84e8541604dc
2022-05-04Merge bitcoin/bitcoin#25058: rpc: Move output script RPCs to separate file, ↡fanquake
rename misc.cpp fa758f9bc5e3a9a4653d7779a6a17adb8e51e92c scripted-diff: Rename rpc/misc.cpp to rpc/node.cpp (MacroFake) fa87eb8ce184a2f0d0ae2d19751b4f6b47af2349 rpc: Move output script RPCs to separate file (MacroFake) Pull request description: RPCs handling output scripts (addresses, scriptPubKeys, and output script descriptors) should not be placed in a file called `misc.cpp`, so move them out, then rename `misc.cpp`. ACKs for top commit: pk-b2: ACK https://github.com/bitcoin/bitcoin/pull/25058/commits/fa758f9bc5e3a9a4653d7779a6a17adb8e51e92c vincenzopalazzo: ACK https://github.com/bitcoin/bitcoin/pull/25058/commits/fa758f9bc5e3a9a4653d7779a6a17adb8e51e92c Tree-SHA512: 0cf8b5b8456361015513e93d3e604ea07d998dd578415b1d0e2918fb401fc44547fc1bb80b7c33c2086f6268e7b8f59837d2955f57434f646ea7921f0158b32d
2022-05-04Merge bitcoin/bitcoin#25040: refactor: Pass lifetimebound reference to ↡MacroFake
SingleThreadedSchedulerClient fa4652ce5995ace831b6a4d3125bfcac9563ff6f Pass lifetimebound reference to SingleThreadedSchedulerClient (MacroFake) Pull request description: Currently a pointer is passed, which is confusing and requires run-time asserts to avoid nullptr dereference. All call sites can pass a reference, so do that. Also mark it LIFETIMEBOUND to avoid call sites passing a temporary. Also, unrelated cleanup in touched lines. ACKs for top commit: pk-b2: ACK https://github.com/bitcoin/bitcoin/pull/25040/commits/fa4652ce5995ace831b6a4d3125bfcac9563ff6f jonatack: Code review ACK fa4652ce5995ace831b6a4d3125bfcac9563ff6f rebased to master, debug build, unit tests vincenzopalazzo: ACK https://github.com/bitcoin/bitcoin/pull/25040/commits/fa4652ce5995ace831b6a4d3125bfcac9563ff6f Tree-SHA512: cd7ec77347e195d659b8892d34c1e9644d4f88552a4d5fa310dc1756eb27050a99d3098b0b0d27f8474230f82c178fd9e22e7018d8248d5e47a7f4caad395e25
2022-05-04Merge bitcoin/bitcoin#25047: tidy: add readability-redundant-declarationMacroFake
c2b295881f852a9096c834334d6b84c988f579f5 tidy: add readability-redundant-declaration (fanquake) Pull request description: ACKs for top commit: vincenzopalazzo: ACK https://github.com/bitcoin/bitcoin/pull/25047/commits/c2b295881f852a9096c834334d6b84c988f579f5 jonatack: Review-only ACK c2b295881f852a9096c834334d6b84c988f579f5 Tree-SHA512: 992dd81f9d0c511efcd8d9d1a8c05fc1401b854272f28f7f31ca0922164ddd7d7c01bfcf5ca268472b5d68969137110f5c0844a52938d294750584e1a948a874
2022-05-04fuzz: SplitString with multiple separatorsMartin Leitner-Ankerl
Co-authored-by: MarcoFalke <falke.marco@gmail.com>
2022-05-04http: replace boost::split with SplitStringMartin Leitner-Ankerl
Also removes boost/algorithm/string.hpp from expected includes
2022-05-04core_read: Replace boost::split with SplitStringMartin Leitner-Ankerl
Note that `SplitString` doesn't support token compression, but in this case it does not matter as empty strings are already skipped anyways. Also removes split.hpp and classification.hpp from expected includes
2022-05-04Extend Split to work with multiple separatorsMartin Leitner-Ankerl
2022-05-03blockstorage: add LIFETIMEBOUND to GetFirstStoredBlock()::start_timeJon Atack
See PR 22278 for discussion. Co-authored-by: MarcoFalke <falke.marco@gmail.com>
2022-05-03Merge bitcoin/bitcoin#24470: Disallow more unsafe string->path conversions ↡MacroFake
allowed by path append operators f64aa9c411ad78259756a28756ec1eb8069b5ab4 Disallow more unsafe string->path conversions allowed by path append operators (Ryan Ofsky) Pull request description: Add more `fs::path` `operator/` and `operator+` overloads to prevent unsafe string->path conversions on Windows that would cause strings to be decoded according to the current Windows locale & code page instead of the correct string encoding. Update application code to deal with loss of implicit string->path conversions by calling `fs::u8path` or `fs::PathFromString` explicitly, or by just changing variable types from `std::string` to `fs::path` to avoid conversions altogether, or make them happen earlier. In all cases, there's no change in behavior either (1) because strings only contained ASCII characters and would be decoded the same regardless of what encoding was used, or (2) because of the 1:1 mapping between paths and strings using the `PathToString` and `PathFromString` functions. Motivation for this PR was just that I was experimenting with #24469 and noticed that operations like `fs::path / std::string` were allowed, and I thought it would be better not to allow them. ACKs for top commit: hebasto: ACK f64aa9c411ad78259756a28756ec1eb8069b5ab4 Tree-SHA512: 944cce49ed51537ee7a35ea4ea7f5feaf0c8fff2fa67ee81ec5adebfd3dcbaf41b73eb35e49973d5f852620367f13506fd12a7a9b5ae3a7a0007414d5c9df50f
2022-05-03Merge bitcoin/bitcoin#25053: Guard `#include <config/bitcoin-config.h>`fanquake
88044a14d9b2c6c70a3330ee1545a9eb39d14d89 Guard `#include <config/bitcoin-config.h>` (Hennadii Stepanov) Pull request description: A fix for builds when the `HAVE_CONFIG_H` macro is not defined. ACKs for top commit: Empact: Code Review ACK https://github.com/bitcoin/bitcoin/pull/25053/commits/88044a14d9b2c6c70a3330ee1545a9eb39d14d89 Tree-SHA512: f2bf1693c7671d7113dccaf66ae34a84719d86cb3271fa18b36611deab93a48d787b3ccfbd735d3b763017d709971cb1151d8d7f30390720009e6e2a6275b5b0
2022-05-03scripted-diff: Rename rpc/misc.cpp to rpc/node.cppMacroFake
-BEGIN VERIFY SCRIPT- git mv src/rpc/misc.cpp src/rpc/node.cpp sed -i 's@rpc/misc.cpp@rpc/node.cpp@g' $(git grep -l misc.cpp) sed -i 's,RegisterMiscRPCCommands,RegisterNodeRPCCommands,g' $(git grep -l RegisterMiscRPCCommands) -END VERIFY SCRIPT-
2022-05-03rpc: Move output script RPCs to separate fileMacroFake
Can be reviewed with --color-moved=dimmed-zebra --color-moved-ws=ignore-all-space
2022-05-03Merge bitcoin/bitcoin#25029: rpc: Move fee estimation RPCs to separate fileMacroFake
fa753abd7cffa05548ad5f21f2e8f9f6b06a7b04 rpc: Move fee estimation RPCs to separate file (MacroFake) Pull request description: Fee estimation is generally used by wallets when creating txs. It doesn't have anything to do with creating or submitting blocks. ACKs for top commit: pk-b2: ACK https://github.com/bitcoin/bitcoin/pull/25029/commits/fa753abd7cffa05548ad5f21f2e8f9f6b06a7b04 brunoerg: crACK fa753abd7cffa05548ad5f21f2e8f9f6b06a7b04 Tree-SHA512: 81e0edc936198a0baf0f5bfa8cfedc12db51759c7873bb0082dfc5f0040d7f275b35f639c6f5b86fa1ea03397b0d5e757c2ce1b6b16f1029880a39b9c3aaceda
2022-05-02Guard `#include <config/bitcoin-config.h>`Hennadii Stepanov
2022-05-02Merge bitcoin/bitcoin#25017: validation: make CScriptCheck and prevector ↡MacroFake
swap members noexcept e5485e8e4be7f2ee0671f58c3dcce35c68ba0ee0 test, bench: make prevector and checkqueue swap member functions noexcept (Jon Atack) abc1ee509025d92db5311c3f5df3b61c09cad24f validation: make CScriptCheck and prevector swap member functions noexcept (Jon Atack) Pull request description: along with those seen elsewhere in the codebase (prevector and checkqueue units/fuzz/bench). A swap must not fail; when a class has a swap member function, it should be declared noexcept. https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c84-a-swap-function-must-not-fail ACKs for top commit: pk-b2: ACK https://github.com/bitcoin/bitcoin/pull/25017/commits/e5485e8e4be7f2ee0671f58c3dcce35c68ba0ee0 w0xlt: ACK https://github.com/bitcoin/bitcoin/pull/25017/commits/e5485e8e4be7f2ee0671f58c3dcce35c68ba0ee0 Tree-SHA512: c82359d5e13f9262ce45efdae9baf71e41ed26568e0aff620e2bfb0ab37a62b6d56ae9340a28a0332c902cc1fa87da3fb72d6f6d6f53a8b7e695a5011f71f7f1
2022-05-01tidy: add readability-redundant-declarationfanquake
2022-04-30Reject invalid rpcauth formatsMacroFake
2022-04-30Merge bitcoin/bitcoin#24543: net processing: Move remaining globals into ↡MacroFake
PeerManagerImpl 778343a379026ef233dffea67f5226565f6d5720 scripted-diff: Rename PeerManagerImpl members (dergoegge) 91c339243e11ec42eeeaca8fe015fc1c3e6338e1 [net processing] Move nHighestFastAnnounce into PeerManagerImpl (dergoegge) 10b83e2aa3393ef2c942fde7ac86e8cf3ea224c1 [net processing] Move block cache state into PeerManagerImpl (dergoegge) a4c55a93ef9277e1043c286120e2417652ee8bbb [net processing] Inline and simplify UpdatePreferredDownload (dergoegge) 490c08f96a34ed436c3d2cf7b9a3ed72694b6147 [net processing] Move nPreferredDownload into PeerManagerImpl (dergoegge) a292df283a596efe7e1d40c33a6d614d70ed564d [net processing] Move mapNodeState into PeerManagerImpl (dergoegge) 37ecaf3e7a028486a0a1c9b717e8eb4214215805 [net processing] Move CNodeState declaration above PeerManagerImpl (dergoegge) Pull request description: This PR moves the remaining net processing globals into `PeerManagerImpl`. This will make testing the peer manager in isolation easier and also acts as a code clean up. ACKs for top commit: jnewbery: Code review ACK 778343a379026ef233dffea67f5226565f6d5720 MarcoFalke: ACK 778343a379026ef233dffea67f5226565f6d5720 πŸ—’ Tree-SHA512: 4f22105d1de37b94c3ef349f38784a30cf8d450d394a6a7849e5bd78940a71e3edbffa3d25e8efb35d7f698fd255f199de7bd4c33e23af5621a6e4e67ed43cb5
2022-04-30Pass lifetimebound reference to SingleThreadedSchedulerClientMacroFake
2022-04-29rpc: Move fee estimation RPCs to separate fileMacroFake
Can be reviewed with --color-moved=dimmed-zebra --color-moved-ws=ignore-all-space
2022-04-29test: Remove boost::split from getarg_tests.cppMacroFake
2022-04-29Merge bitcoin/bitcoin#25025: test: Remove boost::split from rpc_tests.cppMacroFake
fad35e9afdd0bb6e8d6bf7f34a31de11aeb2d39b test: Remove boost::split from rpc_tests.cpp (MacroFake) Pull request description: No need for boost, as there are no tabs. Can be tested with: ```diff diff --git a/src/test/rpc_tests.cpp b/src/test/rpc_tests.cpp index 50b5078110..ad6a888ad0 100644 --- a/src/test/rpc_tests.cpp +++ b/src/test/rpc_tests.cpp @@ -29,6 +29,7 @@ public: UniValue RPCTestingSetup::CallRPC(std::string args) { +Assert(args.find('\t')==std::string::npos); std::vector<std::string> vArgs; boost::split(vArgs, args, boost::is_any_of(" \t")); std::string strMethod = vArgs[0]; ACKs for top commit: fanquake: utACK fad35e9afdd0bb6e8d6bf7f34a31de11aeb2d39b Tree-SHA512: 3df789a222b407d61ad549adc4bbded00705d7c3db07472c31ce0e82216fe3ae27724b7f0ee3e85084bdf405cc28185e85487c9a7001620d6654fda77bab8eb3
2022-04-29Merge bitcoin/bitcoin#25016: refactor: GetFirstStoredBlock() and ↡fanquake
getblockchaininfo follow-ups e2b954e87f0c4cd5c8ac6e4d9c6b4d784844b4d2 rpc: use GetBlockTime() for getblockchaininfo#time (Jon Atack) 86ce844d3b287012f27c7b0bad6d11c9bdd3120e blockstorage, refactor: pass GetFirstStoredBlock() start_block by reference (Jon Atack) ed12c0a49d3c64d170aca9e66ef32a57d7933eeb blockstorage, refactor: make GetFirstStoredBlock() a member of BlockManager (Jon Atack) Pull request description: Picks up the remaining review feedback in #21726 and #24956. - make the global function `GetFirstStoredBlock()` a member of the `BlockManager` class - pass the `start_block` param of `GetFirstStoredBlock()` by reference instead of a pointer - use `GetBlockTime()` for RPC getblockchaininfo#time ACKs for top commit: MarcoFalke: ACK e2b954e87f0c4cd5c8ac6e4d9c6b4d784844b4d2 Tree-SHA512: 546e3c2e18245996b5b286829a605ae919eff3510963ec71b7c9ede521b1f501697e5b2f9d35d7a0606a74cbc8907201c58acf1e2cf7daaa86eefe2e3a8e296b
2022-04-29Merge bitcoin/bitcoin#25024: test: Split MempoolAncestryTests into twofanquake
fa2102e239a01fab648e60de915f9072c5544828 test: Split MempoolAncestryTests into two (MacroFake) Pull request description: The two tests don't share any state, so it seems clearer to put them in separate scopes. ACKs for top commit: jnewbery: Code review ACK fa2102e239 Tree-SHA512: 6669f50f8d5944fed55ecc88aa1bd139bddf6a40e3c2e8f88c3cc7e70cf6d4650c0dd652c7f304813893827c3930d626268655cd9b3f17ff9c9a1a02f0359714
2022-04-29Merge bitcoin/bitcoin#25013: Remove cs_main from verifymessage, move msg ↡fanquake
utils to new file fa60169811d6991a116bd37e1ff58049d2beee77 rpc: Move signmessage RPC util to new file (MacroFake) fa9425177e6be2c53fb5c1333636fa4d678ab401 Remove cs_main from verifymessage (MacroFake) Pull request description: The `verifymessage` RPC has several issues: * It takes `cs_main` for no reason, blocking progress on removing the `cs_main` global mutex. * It is located in a file called `misc`, which is not a very helpful name. Fix all issues. ACKs for top commit: vincenzopalazzo: ACK https://github.com/bitcoin/bitcoin/pull/25013/commits/fa60169811d6991a116bd37e1ff58049d2beee77 Tree-SHA512: c71a1f481b828e0a544405fecbbc7ca44e66ea46b498d7aed1f1c584d6a99724deb13e89d90b9d5cdeecbce293e6a41e9f7ae299543f6d761bf9e7a839b6c7f3
2022-04-29test: Remove boost::split from rpc_tests.cppMacroFake
2022-04-29test: Split MempoolAncestryTests into twoMacroFake
2022-04-29Merge bitcoin/bitcoin#25009: Crash debug builds on PCKG_MEMPOOL_ERRORMacroFake
fa10c9f5a1c9f8b37d51f43f98254feb9a8f9c53 Crash debug builds on PCKG_MEMPOOL_ERROR (MacroFake) Pull request description: Would be nice to allow fuzz targets to meaningfully cover this code ACKs for top commit: glozow: utACK fa10c9f5a1c9f8b37d51f43f98254feb9a8f9c53 vincenzopalazzo: ACK https://github.com/bitcoin/bitcoin/pull/25009/commits/fa10c9f5a1c9f8b37d51f43f98254feb9a8f9c53 Tree-SHA512: 68efacedbf72f67cf3dc0bb9927a698492cdc1b08df91ef6af863ad8828b78058a64e52d64d244a5b2966cb9e63797b2647d1bb222677bf83b26fca6e4b1dbf0
2022-04-28Merge bitcoin/bitcoin#18554: wallet: ensure wallet files are not reused ↡Andrew Chow
across chains 5f213213cb17429353ef7ec3e97b185af06d236f tests: add tests for cross-chain wallet use prevention (Seibart Nedor) 968765973b5bfde1ee4ad2fb5c19e24bce63ad0e wallet: ensure wallet files are not reused across chains (Seibart Nedor) Pull request description: This implements a proposal in #12805 and is a rebase of #14533. This seems to be a working approach, but I'm not sure why the `p2p_segwit.py` functional test needed a change, so I'll look into it more. ACKs for top commit: achow101: ACK 5f213213cb17429353ef7ec3e97b185af06d236f dongcarl: Code Review ACK 5f213213cb17429353ef7ec3e97b185af06d236f [deleted]: tACK https://github.com/bitcoin/bitcoin/pull/18554/commits/5f213213cb17429353ef7ec3e97b185af06d236f Tree-SHA512: 2c934300f113e772fc31c16ef5588526300bbc36e4dcef7d77bd0760c5c8f0ec77f766b1bed5503eb0157fa26dc900ed54d2ad1b41863c1f736ce5c1f3b67bec
2022-04-28Merge bitcoin/bitcoin#24984: wallet: ignore chainStateFlushed notifications ↡Andrew Chow
while attaching chain 2052e3aa9aa666bdc86dac370f1dd8fb978d3497 wallet: ignore chainStateFlushed notifications while attaching chain (Martin Zumsande) Pull request description: Fixes #24487 When a rescan is performed during `CWallet::AttachChain()` (e.g. when loading an old wallet) but this is interrupted by a shutdown signal, the wallet will currently stop the rescan, receive a `chainStateFlushed` signal, set the saved best block to the tip and shut down. At next startup, the rescan is not continued or repeated because of this. But some blocks have never been scanned by the wallet, which could lead to an incorrect balance. Fix this by ignoring `chainStateFlushed` notifications until the chain is attached. Since `CWallet::chainStateFlushed` is being manually called by `AttachChain()` anyway after finishing with the rescan, it is not a problem if intermediate notifications are ignored. Manual rescans started / aborted by the `rescanblockchain` / `abortrescan` RPCs are not affected by this. I didn't choose alternative ways of fixing this issue that would delay the validationinterface registration or change anything else about the handling of `blockConnected` signals for the reasons mentioned in [this existing comment](https://github.com/bitcoin/bitcoin/blob/master/src/wallet/wallet.cpp#L2937-L2944). ACKs for top commit: achow101: ACK 2052e3aa9aa666bdc86dac370f1dd8fb978d3497 ryanofsky: Code review ACK 2052e3aa9aa666bdc86dac370f1dd8fb978d3497. This is a straightforward fix for the bug described in #24487 where a wallet could skip scanning blocks if is shut down in the middle of a sync and a chainStateFlushed notification was received during the sync. It would be nice to write a test for this but probably would be tricky to write. w0xlt: Code Review ACK https://github.com/bitcoin/bitcoin/pull/24984/commits/2052e3aa9aa666bdc86dac370f1dd8fb978d3497 Tree-SHA512: a6186173d72b26bd4adbf2315e11af365004a723ea5565a0f7b868584dc47c321a6572eafaeb2420bd21eed1c7ad92b47e6218c5eb72313a3c6bee58364e2247
2022-04-28rpc: use GetBlockTime() for getblockchaininfo#timeJon Atack
generated by cgit v1.2.3 (git 2.26.2) at 2024-10-31 01:34:17 +0000