aboutsummaryrefslogtreecommitdiff
path: root/src
AgeCommit message (Collapse)Author
2021-12-16Merge bitcoin/bitcoin#23785: refactor: Move stuff to ChainstateManagerMarcoFalke
fab6d6b2d154893ab422dda87f3535d42c3e06f4 Move pindexBestInvalid to ChainstateManager (MarcoFalke) facd2137eceacb95e1f71c87ddc704d752b37272 Move m_failed_blocks to ChainstateManager (MarcoFalke) fa47b5c100f81c65c15b5a6afaf6c91bc0861264 Move AcceptBlockHeader to ChainstateManager (MarcoFalke) fa3d62cf7b3501a056b34c5458c14d2fe6a55bd7 Move FindForkInGlobalIndex from BlockManager to CChainState (MarcoFalke) Pull request description: Move globals or members of the wrong class to the right class. ACKs for top commit: naumenkogs: ACK fab6d6b2d154893ab422dda87f3535d42c3e06f4 Sjors: ACK fab6d6b2d154893ab422dda87f3535d42c3e06f4 shaavan: ACK fab6d6b2d154893ab422dda87f3535d42c3e06f4 Tree-SHA512: 926cbdfa22838517497bacb79ed5f521f64117c2aacf96a0176f62831b4713314a32abc0213df5ee067edf63e4a4300f752a26006d36e5aab415bb91209a271f
2021-12-16Merge bitcoin/bitcoin#23777: doc: follow-ups from #23365 (index backwards ↵MarcoFalke
search) e4a8d561edf3cfb326e86c87155fed41a61e7333 doc: add explanations for assert in index and magic numbers in test (Martin Zumsande) Pull request description: This adds two explanations suggested in the review of #23365, that I didn't manage to address before that PR was merged: https://github.com/bitcoin/bitcoin/pull/23365#discussion_r763981042 https://github.com/bitcoin/bitcoin/pull/23365#discussion_r763982639 ACKs for top commit: jnewbery: ACK e4a8d561edf3cfb326e86c87155fed41a61e7333 Tree-SHA512: 0500c8abb37bb3e3694463ad5e74b2e1483615ccf1d7529b0d5faa694652ada17d242dc7fda6d995733766c627d54178a2c8fa21a570cdf13292f64ff5425b56
2021-12-16Merge bitcoin/bitcoin#23750: rpcwallet: mention labels are disabled for ↵MarcoFalke
ranged descriptors 65efbba45d817261f590d043c69a9981e6b637bd rpcwallet: mention labels are deactivated for ranged descriptors (Antoine Poinsot) Pull request description: It was confusing when trying to use it as a blackbox. So mention it so next ones don't have to open the said box :) See #23749 for context ACKs for top commit: Sjors: utACK 65efbba45d817261f590d043c69a9981e6b637bd achow101: ACK 65efbba45d817261f590d043c69a9981e6b637bd Tree-SHA512: d8a3d1f81c16d95855ac2b01e8fd20e83d6dac1721b3da464a9a890e46102992a6882918be87b2a28b929349ee7f1beb1af6c88b22f065fbbb6948275a6d2b8f
2021-12-16Merge bitcoin/bitcoin#23721: wallet, refactor: Move restorewallet() logic to ↵MarcoFalke
the wallet section 62fa61fa4a33ff4d108a65d656ffe2cac4330824 refactor: remove the wallet folder if the restore fails (w0xlt) abbb7eccef3fc1c36f34756458d2792f6661e29f refactor: Move restorewallet() RPC logic to the wallet section (w0xlt) 4807f73f48f4ff1084fcf7aee94e5b14592bfda8 refactor: Implement restorewallet() logic in the wallet section (w0xlt) Pull request description: Currently `restorewallet()` logic is written in the RPC layer and it can´t be reused by GUI. So it moves this to the wallet section and then, GUI can access it. This is necessary to implement the "Restore Wallet" menu item in the GUI (which is already implemented in https://github.com/bitcoin-core/gui/pull/471 ). This commit also simplifies error handling and adds a new behavior: if the restore fails, the invalid wallet folder is removed. ACKs for top commit: achow101: ACK 62fa61fa4a33ff4d108a65d656ffe2cac4330824 shaavan: crACK 62fa61fa4a33ff4d108a65d656ffe2cac4330824 Tree-SHA512: 7ccfbad5943f38616ba0c2dd443c97a4b5bc1f6612dbf5a9e7a0263100aba36671fae929a2e7688442667be394645f44484af137a4802f204a33c4689eb27c39
2021-12-15refactor: remove the wallet folder if the restore failsw0xlt
2021-12-15refactor: Move restorewallet() RPC logic to the wallet sectionw0xlt
It also simplifies restorewallet() and loadwallet() RPC error handling.
2021-12-15Merge bitcoin/bitcoin#22674: validation: mempool validation and submission ↵W. J. van der Laan
for packages of 1 child + parents 046e8ff264be6b888c0f9a9d822e32aa74e19b78 [unit test] package submission (glozow) e12fafda2dfbbdf63f125e5af797ecfaa6488f66 [validation] de-duplicate package transactions already in mempool (glozow) 8310d942e046c5a9b6bd90afdcd3af68dd91e081 [packages] add sanity checks for package vs mempool limits (glozow) be3ff151a1f9665720cdf70d072b098a2f9726a9 [validation] full package accept + mempool submission (glozow) 144a29099a865ac1dc3e5291d9529fbcca9c83a4 [policy] require submitted packages to be child-with-unconfirmed-parents (glozow) d59ddc5c3d1c035474d7bc9fa9f8a0eeb1c8498c [packages/doc] define and document package rules (glozow) ba26169f6035c238378a3c9647213328a006fa23 [unit test] context-free package checks (glozow) 9b2fdca7f03911ac40fe0f8a0b5da534bee4554b [packages] add static IsChildWithParents function (glozow) Pull request description: This is 1 chunk of [Package Mempool Accept](https://gist.github.com/glozow/dc4e9d5c5b14ade7cdfac40f43adb18a); it restricts packages to 1 child with its parents, doesn't allow conflicts, and doesn't have CPFP (yet). Future PRs (see #22290) will add RBF and CPFP within packages. ACKs for top commit: laanwj: Code review ACK 046e8ff264be6b888c0f9a9d822e32aa74e19b78 Tree-SHA512: 37dbba37d527712f8efef71ee05c90a8308992615af35f5e0cfeafc60d859cc792737d125aac526e37742fe7683ac8c155ac24af562426213904333c01260c95
2021-12-15Move pindexBestInvalid to ChainstateManagerMarcoFalke
A private member is better than a global.
2021-12-15Move m_failed_blocks to ChainstateManagerMarcoFalke
The member is unrelated to block storage (BlockManager). It is related to validation. Fix the confusion by moving it. Can be reviewed with --color-moved=dimmed-zebra --color-moved-ws=ignore-all-space
2021-12-15Move AcceptBlockHeader to ChainstateManagerMarcoFalke
This is needed for the next commit.
2021-12-15Move FindForkInGlobalIndex from BlockManager to CChainStateMarcoFalke
The helper was moved in commit b026e318c39f59a06e29f1b25c7f577e01b25ccb, which also mentioned that it could be moved to CChainState. So do that, as the functionality is not block-storage related. This also allows to drop one function argument.
2021-12-15Merge bitcoin/bitcoin#23769: Disallow copies of CChainMarcoFalke
faf2614f60efe972b47b6fa00cfbc22d04ea7239 style: Use 4 spaces for indendation, not 5 (MarcoFalke) fada66fc2c716fc148104b6180da12efc5d05e41 Disallow copies of CChain (MarcoFalke) Pull request description: Creating a copy of the chain is not a valid use case in normal operation. Also, it massively degrades performance. However, it seems to be a mistake that no one looks out for during review: * https://github.com/bitcoin/bitcoin/pull/22677#discussion_r760400537 Fix this by disallowing it. ACKs for top commit: jamesob: ACK faf2614f60efe972b47b6fa00cfbc22d04ea7239 ([`jamesob/ackr/23769.1.MarcoFalke.disallow_copies_of_cchai`](https://github.com/jamesob/bitcoin/tree/ackr/23769.1.MarcoFalke.disallow_copies_of_cchai)) glozow: utACK faf2614f60efe972b47b6fa00cfbc22d04ea7239, nice. prusnak: utACK faf2614 Tree-SHA512: 27b908c78842e4700e118adb876c09c3d1ec04662310e983309e2cd6fa8ad38c9359ff45f36a804359b9f117e351c4739e651b3e6754c14e6c6fcd7ae5e68342
2021-12-15Merge bitcoin-core/gui#508: Prevent negative values of progressPerHourHennadii Stepanov
71d33380ed6858b4a65b396332bfb22d984642a6 qt: prevent negative values of progressPerHour (HiLivin) Pull request description: Added a similar guard to _progressPerHour_ as is placed at _remainingMSecs_. It prevents the display of negative values like "-0.00%" in some cases. ACKs for top commit: hebasto: ACK 71d33380ed6858b4a65b396332bfb22d984642a6 jarolrod: ACK 71d3338 shaavan: reACK 71d33380ed6858b4a65b396332bfb22d984642a6 Tree-SHA512: 5427cdf4441b542196008034355ea00a075adf8b9aeeb383bacdb4e5fbda23d665448a50035aac93cbf401d5d6211d39a2c7c294568d9f5548a5c7579e201c44
2021-12-15Merge bitcoin/bitcoin#23758: net: Use type-safe mockable time for peer ↵MarcoFalke
connection time fad943821e35d0eb2143061e718f0193e12a4c71 scripted-diff: Rename touched member variables (MarcoFalke) fa663a4c0d20487ed3f7a93e1c2ca9932b05f5a8 Use mockable time for peer connection time (MarcoFalke) fad7ead146a152f46b25ce6623e05cbb1dbc8cca refactor: Use type-safe std::chrono in net (MarcoFalke) Pull request description: Benefits: * Type-safe * Mockable * Allows to revert a temporary test workaround ACKs for top commit: naumenkogs: ACK fad943821e35d0eb2143061e718f0193e12a4c71 shaavan: ACK fad943821e35d0eb2143061e718f0193e12a4c71 Tree-SHA512: af9bdfc695ab727b100c6810a7289d29b02b0ea9fa4fee9cc1f3eeefb52c8c465ea2734bae0c1c63b3b0d6264ba2c493268bc970ef6916570eb166de77829d82
2021-12-15Merge bitcoin/bitcoin#22362: Drop only invalid entries when reading banlist.jsonW. J. van der Laan
faa6c3d44c861c0486c1369e1d098b7645ab07cd net: Drop only invalid entries when reading banlist.json (MarcoFalke) Pull request description: All entries will be dropped when there is at least one invalid one in `banlist.json`. Fix this by only dropping invalid ones. Also suggested in https://github.com/bitcoin/bitcoin/pull/20966#issuecomment-861150204 ACKs for top commit: laanwj: Re-ACK faa6c3d44c861c0486c1369e1d098b7645ab07cd Tree-SHA512: 5a58e7f1dcabf78d0c65d8c6d5d997063af1efeaa50ca7730fc00056fda7e0061b6f7a38907ea045fe667c9f61d392e01e556b425a95e6b126e3c41cd33deb83
2021-12-15Merge bitcoin/bitcoin#23757: [build] Android: fix GUI not loading on Qt 5.15fanquake
27f353d8efca19bc2f7dc79b09d4737c9401a768 build, android: Fix Android GUI not loading on Qt 5.15 (Igor Cota) 6fc5c772d47cca01968c451f070f997c9f7641e0 build, qt: use static QAndroidPlatformIntegrationPlugin (Igor Cota) Pull request description: PR moved from https://github.com/bitcoin-core/gui/pull/504 as it escaped the confines of `src/qt`. ACKs for top commit: hebasto: re-ACK 27f353d8efca19bc2f7dc79b09d4737c9401a768 promag: utACK 27f353d8efca19bc2f7dc79b09d4737c9401a768 Tree-SHA512: 4b6e6b2fb1923b89934f11caa8c05c6f340881689273f0c08916144e623f03fd5b781f1a53af83f6e87dce211fe02a1cb87e5943d13811c791cc8aa458184d9f
2021-12-15Merge bitcoin/bitcoin#23174: validation: have LoadBlockIndex account for ↵MarcoFalke
snapshot use 2283b9cd1ee0fbd1e8ebc61673b1fe7596199a24 test: add tests for LoadBlockIndex when using multiple chainstates (James O'Beirne) 0fd599a51a700c028d6f7ed8067d2d9f6e6a04a5 validation: have LoadBlockIndex account for snapshot use (James O'Beirne) d0c6e61f5dd3b6af818459d9d03b7ba316c5a3f7 validation: don't modify genesis during snapshot load (James O'Beirne) Pull request description: This is part of the [assumeutxo project](https://github.com/bitcoin/bitcoin/projects/11) (parent PR: #15606) --- Currently, `BlockManager::LoadBlockIndex` adds all blocks that have downloaded transactions to the active chain state's `setBlockIndexCandidates` set, ignoring the background chain state. This PR changes ChainstateManager::LoadBlockIndex to update `setBlockIndexCandidates` in the background chain, not just the active chain. In the active chain, the same blocks are added as before. In the background chain, only blocks that have actually been validated, not blocks marked assumed-valid are added so the background chain will continue to download and validate assumed-valid blocks. ACKs for top commit: MarcoFalke: Concept ACK 2283b9cd1ee0fbd1e8ebc61673b1fe7596199a24 🤽 Sjors: utACK 2283b9cd1ee0fbd1e8ebc61673b1fe7596199a24 Tree-SHA512: 7c9a80802df4722d85d12b78d2e7f628ac5f11cb8be66913d5c3230339bd1220c6723805509d4460826a17d1dc04b0ae172eb7d09ac0ea5dc5e41d77975cbd5e
2021-12-15doc: add explanations for assert in index and magic numbers in testMartin Zumsande
These were suggested in review of #23365 Co-authored-by: John Newbery <john@johnnewbery.com>
2021-12-15Merge bitcoin/bitcoin#23756: refactor: Fix implicit integer sign changes in ↵MarcoFalke
strencodings fa5865a9e339e7fe4068fbcce2ac055ed49eddfd Reduce size of strencodings decode tables (MarcoFalke) fad6761cf7875c7e91fd76398a71a9f9c6b3ea82 Fix implicit integer sign changes in strencodings (MarcoFalke) Pull request description: This removes casts where they are nonsensical (ToUpper/ToLower) and adds them where needed. Then, remove the suppressions. Also, reduce static memory usage. ACKs for top commit: shaavan: crACK fa5865a9e339e7fe4068fbcce2ac055ed49eddfd Tree-SHA512: 29850fb616789befad17f71731f322b2238e94ec431cda293629de540f7ab02060a9fc5b7666168ca2592048df5a39a2975177f47771d4d8211d90321052549d
2021-12-15Merge bitcoin/bitcoin#23713: refactor, test: refactor ↵MarcoFalke
addrman_tried_collisions test to directly check for collisions caac999ff0fc5c98fa438b7e96fe1232f6573fd5 refactor: remove dependence on AddrManTest (josibake) f961c477b56737c546c275e4d86cecfa3f75d48c refactor: check Good() in tried_collisions test (josibake) 207f1c825c632c54af009516d376d392ea9106fa refactor: make AddrMan::Good return bool (josibake) Pull request description: Previously, the `addrman_tried_collisions` test behaved in the following way: 1. add an address to addrman 2. attempt to move the new address to the tried table (using `AddrMan.Good()`) 3. verify that `num_addrs` matched `size()` to check for collisions in the new table `AddrMan.size()`, however, returns the number of unique address in addrman, regardless of whether they are in new or tried. This means the test would still pass for addresses where a collision did occur in the tried table. After 3 collisions in the tried table, there would eventually be a collision in the new table when trying to add a new address, which was then detected by checking `num_addrs - collisions == size()`. While the collision in the new table was caused by a collision in the tried table, the test is misleading as it's not directly testing for collisions in the tried table and misses 3 collisions before identifying a collision in the new table. ### solution To more directly test the tried table, I refactored `AddrMan::Good()` to return a boolean after successfully adding an address to the tried table. This makes the test much cleaner by first adding an address to new, calling `Good` to move it to the tried table, and checking if it was successful or not. It is worth noting there are other reasons, aside from collisions, which will cause `Good` to return false. That being said, this is an improvement over the previous testing methodology. Additionally, having `Good()` return a boolean is useful outside of testing as it allows the caller to handle the case where `Good` is unable to move the entry to the tried table (e.g https://github.com/bitcoin/bitcoin/blob/a06364741358feae04813050e4225eb43fc386e3/src/rpc/net.cpp#L945). ### followup As a follow up to this PR, I plan to look at the following places `Good()` is called and see if it makes sense to handle the case where it is unable to add an entry to tried: * https://github.com/bitcoin/bitcoin/blob/a06364741358feae04813050e4225eb43fc386e3/src/rpc/net.cpp#L945 * https://github.com/bitcoin/bitcoin/blob/a06364741358feae04813050e4225eb43fc386e3/src/net.cpp#L2067 * https://github.com/bitcoin/bitcoin/blob/a06364741358feae04813050e4225eb43fc386e3/src/net_processing.cpp#L2708 ACKs for top commit: jnewbery: utACK caac999ff0 mzumsande: Code review ACK caac999ff0fc5c98fa438b7e96fe1232f6573fd5 Tree-SHA512: f328896b1f095e8d2581fcdbddce46fc0491731a0440c6fff01081fa5696cfb896dbbe1d183eda2c100f19aa111e1f8b096ef93582197edc6b791de563a58f99
2021-12-14refactor: Implement restorewallet() logic in the wallet sectionw0xlt
Currently restorewallet() logic is written in the RPC layer and it can´t be reused by GUI. So it reimplements this in the wallet and interface sections and then, GUI can access it.
2021-12-14qt: prevent negative values of progressPerHourHiLivin
Added a similar guard to progressPerHour as is placed at remainingMSecs. It prevents the display of negative values like "-0.00%" in some cases.
2021-12-14net: Drop only invalid entries when reading banlist.jsonMarcoFalke
Currently all entries in the file are dropped. Fix that by only dropping the invalid ones
2021-12-14Merge bitcoin/bitcoin#22777: net processing: don't request tx relay on ↵MarcoFalke
feeler connections eaf6be0114a6d7763767da9496907fe8a670ff9e [net processing] Do not request transaction relay from feeler connections (John Newbery) 0220b834b175dc8c45a2c60213474a72c0ef8193 [test] Add testing for outbound feeler connections (John Newbery) Pull request description: Feelers are short-lived connections used to test the viability of peers. The bitcoind node will periodically open feeler connections to addresses in its addrman, wait for a `version` message from the peer, and then close the connection. Currently, we set `fRelay` to `1` in the `version` message for feeler connections, indicating that we want the peer to relay transactions to us. However, we close the connection immediately on receipt of the `version` message, and so never process any incoming transaction announcements. This PR changes that behaviour to instead set `fRelay` to `0` indicating that we do not wish to receive transaction announcements from the peer. This PR also extends the `addconnection` RPC to allow creating outbound feeler connections from the node to the test framework, and a test to verify that the node sets `fRelay` to `0` in the `version` message to feeler connections. ACKs for top commit: naumenkogs: ACK eaf6be0114a6d7763767da9496907fe8a670ff9e MarcoFalke: review ACK eaf6be0114a6d7763767da9496907fe8a670ff9e 🏃 Tree-SHA512: 1c56837dbd0a396fe404a5e39f7459864d15f666664d6b35ad109628b13158e077e417e586bf48946a23bd5cbe63716cb4bf22cdf8781b74dfce6047b87b465a
2021-12-14refactor: remove dependence on AddrManTestjosibake
2021-12-14refactor: check Good() in tried_collisions testjosibake
Rather than try to infer a collision by checking `AddrMan::size`, check whether or not moving to the tried table was successful by checking the output from `AddrMan::Good`
2021-12-14refactor: make AddrMan::Good return booljosibake
If AddrMan::Good is unable to add an entry to tried (for a number of reasons), return false. This makes it much easier and cleaner to directly test for tried collisions. It also allows anyone calling Good() to handle the case where adding an address to tried is unsuccessful. Update docs to doxygen style.
2021-12-14Merge bitcoin/bitcoin#23575: fuzz: Rework FillNodefanquake
fa19bab90a3ccc2f76c20aa805292d6a9c5d8071 fuzz: Rework FillNode (MarcoFalke) fae6e31df7c6df04f41fc8401e2a9781a4d75be7 refactor: Set fSuccessfullyConnected in FillNode (MarcoFalke) fa3583f856e34b6c6134745da14f5206cf71fa3e fuzz: Avoid negative NodeId in ConsumeNode (MarcoFalke) Pull request description: Currently `FillNode` is a bit clumsy because it directly modifies memory of `CNode`. This gets in the way of moving that memory to `Peer`. Also, it isn't particularly consistent. See for example https://github.com/bitcoin/bitcoin/pull/21160#discussion_r739206139 . Fix all issues by sending a `version`/`verack` in `FillNode` and let net_processing figure out the internal details. ACKs for top commit: jnewbery: Strong concept ACK and light code review ACK fa19bab90a3ccc2f76c20aa805292d6a9c5d8071 Tree-SHA512: 33261d857c3fa6d5d39d742624009a29178ad5a15eb3fd062da741affa5a4854fd45ed20d59a6bba2fb068cf7b39cad6f95b2910be7cb6afdc27cd7917955b67
2021-12-14style: Use 4 spaces for indendation, not 5MarcoFalke
Also, other whitespace fixes in the touched file. Can be trivially reviewed with "--ignore-all-space --word-diff-regex=. -U0".
2021-12-14Disallow copies of CChainMarcoFalke
2021-12-14Merge bitcoin/bitcoin#23766: refactor: remove unneeded calls to strprintf()MarcoFalke
eca159c305c1ff3d9b806755026a23bf2aad9a48 refactor: remove unneeded calls to strprintf() (fanquake) Pull request description: ACKs for top commit: laanwj: Code review ACK eca159c305c1ff3d9b806755026a23bf2aad9a48 Tree-SHA512: 6f0b25dd6506f7223e3603f89c9d6e33feb9a7b524986146f70d10973b7a346830889656b37b2ab5cf78ddea458ca6979dd848403a900cb79f62a1494b968f99
2021-12-14refactor: remove unneeded calls to strprintf()fanquake
2021-12-14Merge bitcoin/bitcoin#23762: wallet: Replace Assume with Assert where needed ↵fanquake
in coinselection fa26c5564429d3201f43850afaf1b4df2ed11ad9 wallet: Replace Assume with Assert where needed in coinselection (MarcoFalke) Pull request description: `Assume` should only be used when a failed check is recoverable. The checks here don't recover and would run into UB, so use `Assert` instead. ACKs for top commit: theStack: Code-review ACK fa26c5564429d3201f43850afaf1b4df2ed11ad9 Tree-SHA512: 0cf9435f9ec44794022ce0274cba602aec95102ab73f4c8a93dae54ef4c0a594f6a81640477039719ddfb6f23b05f8ece3e4886ef7f8a725efff45685ac49d92
2021-12-13test: add tests for LoadBlockIndex when using multiple chainstatesJames O'Beirne
Incorporates feedback from Russ Yanofsky.
2021-12-13validation: have LoadBlockIndex account for snapshot useJames O'Beirne
Ensure that blocks past the snapshot base block (i.e. the end of the assumed-valid region of the chain) are not included in setBlockIndexCandidates for the background validation chainstate. These blocks, while fully validated and lacking the BLOCK_ASSUMED_VALID flag, *rely* on blocks which are assumed-valid, and so shouldn't be added to the IBD chainstate. Co-authored-by: Russ Yanofsky <russ@yanofsky.org>
2021-12-13validation: don't modify genesis during snapshot loadJames O'Beirne
Avoid modifying the genesis block index entry during snapshot load. This is because, in a future change that fixes LoadBlockIndex for UTXO snapshots, we detect block index entries that are reliant on assumed-valid ancestors and treat them specially. Since the genesis block doesn't have BLOCK_VALID_SCRIPTS, it would be erroneously marked BLOCK_ASSUMED_VALID during snapshot load if we didn't skip it here. This would cause a "setBlockIndexCandidates() empty" assertion to be tripped since all block index entries would be marked assume-valid due to genesis, which is never re-validated. There's probably no good reason to modify the genesis block index entry during snapshot load anyway...
2021-12-13build, android: Fix Android GUI not loading on Qt 5.15Igor Cota
Qt 5.14 introduced certain breaking changes to the way it parses AndroidManifest.xml metadata https://lists.qt-project.org/pipermail/interest/2020-January/034372.html
2021-12-13build, qt: use static QAndroidPlatformIntegrationPluginIgor Cota
2021-12-13Merge bitcoin/bitcoin#23755: rpc: Quote user supplied strings in error messagesMarcoFalke
fa24a3df8796cbf4eeb35d950a4c848d605e5b22 rpc: Quote user supplied strings in error messages (MarcoFalke) Pull request description: I can't see a downside doing this and this fixes a fuzzing crash Background: This is a follow-up to commit 926fc2a0d4ff64cf2ff8e1dfa64eca2ebd24e090, which introduced the "starts_with-hack". Maybe an alternative to the hack would be to assign a unique error code to internal bugs? However, I think this can be done in an separate pull request and the changes here make sense even on their own. ACKs for top commit: fanquake: ACK fa24a3df8796cbf4eeb35d950a4c848d605e5b22 - to fix the fuzzers. Tree-SHA512: d998626406a64396a037a6d1fce22fce3dadb7567c2f9638e450ebe8fb8ae77d134e15dd02555326732208f698d77b0028bc62be9ceee9c43282b61fe95fccbd
2021-12-13Merge bitcoin/bitcoin#23761: build: use __SIZEOF_INT128__ for checking ↵MarcoFalke
__int128 availability e9440aeb5cad98fea9971f5126461e0a2b30ab54 build: use __SIZEOF_INT128__ for checking __int128 availability (fanquake) Pull request description: We already use this in the blockfilter code, https://github.com/bitcoin/bitcoin/blob/bf66e258a84e18935fde3ebb9a4b0392bf883222/src/blockfilter.cpp#L34-L36 so not sure we need to maintain two different ways of testing for the same functionality. Consolidate on testing for `__SIZEOF_INT128__`, which we already use, is supported by the compilers we care about, and is also used by libsecp256k1. ACKs for top commit: sipa: utACK e9440aeb5cad98fea9971f5126461e0a2b30ab54 Zero-1729: crACK e9440aeb5cad98fea9971f5126461e0a2b30ab54 Tree-SHA512: 8aeef1734486a863b5091123bb5f9ba8868b1e2b4b35114586e3eb5862a38d4a1518ed069f37f41cb5e5ce2f6c87d95671996366d5ee990e0c90f268a8978ba3
2021-12-13rpc: Quote user supplied strings in error messagesMarcoFalke
2021-12-13Merge bitcoin/bitcoin#23738: validation, log: improve logging of ↵MarcoFalke
ChainstateManager snapshot persistance 50209a42ad05e92ad0945fa104c65d2b1f2a2c1c validation, doc: remove TODO comment (Jon Atack) 8e37fa83934ad47a4a73c326213e2073dfc35e29 validation, log: improve logging in FlushSnapshotToDisk() (Jon Atack) 271252c0bda5779a255f23a73ceb6f722559f0d1 validation, log: extract FlushSnapshotToDisk() function (Jon Atack) Pull request description: Use the `LOG_TIME_MILLIS_WITH_CATEGORY_MSG_ONCE` macro to improve the logging of ChainstateManager snapshot persistance, log task start and completion separately and no longer manually track the duration, as suggested by Marco Falke in https://github.com/bitcoin/bitcoin/pull/22872#discussion_r715571280. Extract the flushing into one function, which clarifies the logic, extends the improved logging to both flushing call sites, and allows logging the prefix `FlushSnapshotToDisk`, which is similar to `FlushStateToDisk`. before ``` [snapshot] flushing coins cache (0 MB)... done (0.00ms) [snapshot] flushing snapshot chainstate to disk ``` after ``` FlushSnapshotToDisk: flushing coins cache (0 MB) started ... FlushSnapshotToDisk: completed (0.00ms) FlushSnapshotToDisk: saving snapshot chainstate (0 MB) started ... FlushSnapshotToDisk: completed (0.00ms) ``` The logging can be observed in the output of ``` ./src/test/test_bitcoin -t validation_chainstate_tests -- DEBUG_LOG_OUT ``` Top commit has no ACKs. Tree-SHA512: 5d954cd8c7455f8625152a43663a237f04717bb834aed62925a56e17c711fca6ccfc03783970b6b0bde44f64617d804b423a7048287c06ee816db36247acf272
2021-12-13wallet: Replace Assume with Assert where needed in coinselectionMarcoFalke
2021-12-13build: use __SIZEOF_INT128__ for checking __int128 availabilityfanquake
We already use this in the blockfilter code, so not sure we need to maintain two different ways of testing for the same functionality. Consolidate on testing for __SIZEOF_INT128__, which we already use, is supported by the compilers we care about, and is also used by libsecp256k1.
2021-12-13Merge bitcoin/bitcoin#23365: index: Fix backwards search for bestblockMarcoFalke
9600ea01450b0d39be90eb2971c1ac5c9b69a66e test: Add edge case of pruning up to index height (Martin Zumsande) 698c524698c33595a4d555eaa9e21bc19b4d3e93 index: Fix backwards search for bestblock (Martin Zumsande) Pull request description: This PR attempts to fix an intermittent Init issue encountered during the stress testing of #23289, which relates to the pruning-compatible filter reconstruction logic introduced in #15946. The problem would occur when the node starts with `-txindex=1` but `ThreadSync` is interrupted after it sets `m_best_block_index` to Genesis, and before it gets do any further work. In that case, during the next restart of the node, an Init error would be thrown because `BaseIndex::Init()` tries to backtrack from the tip to the last block which has been successfully indexed (here: Genesis), but the backtracking logic didn't work properly in this case: The loop `while (block_to_test && block->pprev && (block->pprev->nStatus & BLOCK_HAVE_DATA))` checks if a predecessor exists **before** performing the check `block_to_test == block` and then possbily setting `prune_violation = false` If `block_to_test` and `block` are the Genesis block this check will not be reached because `block->pprev` does not exist. To reproduce this bug on regtest: 1) start a node with a fresh datadir using `-txindex=1` (or any other index) 2) stop and restart without any index 3) mine a block 3) stop and restart again with the index enabled ->InitError `Error: txindex best block of the index goes beyond pruned data. (...)` Fix this by requiring that we have the data for the block of the current iteration `block` (instead of requiring it for the predecessor `block->pprev`) That way, the check for `block_to_test == block` is also reached when `block_to_test` is the Genesis block. No longer requiring the data of `block->pprev` also means that we can now prune up to `m_best_block_index` height without requiring a reindex (one block more than before). I added this edge case to `feature_blockfilterindex_prune.py`, the new version should fail on master. ACKs for top commit: ryanofsky: Partial code review ACK 9600ea01450b0d39be90eb2971c1ac5c9b69a66e for the code change, not the test changes. (Test changes are indirect and little over my head.) It seems obvious that previous code `prune_violation = true, while (block->pprev)` would incorrectly detect a prune violation at the genesis block, and the fix here make sense and looks correct. Tree-SHA512: c717f372cee8fd49718b1b18bfe237aa6ba3ff4468588c10e1272d7a2ef3981d10af4e57de51dec295e2ca72d441bc6c2812f7990011a94d7f818775e3ff1a38
2021-12-13scripted-diff: Rename touched member variablesMarcoFalke
-BEGIN VERIFY SCRIPT- ren() { sed -i "s/\<$1\>/$2/g" $( git grep -l "$1" ./src/ ) ; } ren nLastBlockTime m_last_block_time ren nLastTXTime m_last_tx_time ren nTimeConnected m_connected -END VERIFY SCRIPT-
2021-12-13Use mockable time for peer connection timeMarcoFalke
This allows to revert the temporary commit 0bfb9208df556f77cddfedfd73e5811a0e031d34 (test: fix test failures in test/functional/p2p_timeouts.py).
2021-12-13refactor: Use type-safe std::chrono in netMarcoFalke
2021-12-13Reduce size of strencodings decode tablesMarcoFalke
2021-12-13Fix implicit integer sign changes in strencodingsMarcoFalke
generated by cgit v1.2.3 (git 2.26.2) at 2024-11-08 11:40:06 +0000