aboutsummaryrefslogtreecommitdiff
path: root/src/test
AgeCommit message (Collapse)Author
2022-02-01Merge bitcoin/bitcoin#24190: test: Fix sanitizer suppresions in streams_testsMarcoFalke
faa630aa15bbda0f3b0cf3b6f31cf8fdaeb66975 test: Fix sanitizer suppresions in streams_tests (MarcoFalke) Pull request description: Two changes (that also make sense on their own) to remove the file-wide sanitizer suppression: * `FindByte` no longer takes a `char`, but an `uint8_t`, after commit 196b4599201dbce3e0317e9b98753fa6a244b82d. * The `key` vector of unsigned chars can be removed and inlined as initializer-list. This avoids a bunch of verbose code like `clear()` and `push_back` of `char`s. ACKs for top commit: PastaPastaPasta: utACK faa630aa15bbda0f3b0cf3b6f31cf8fdaeb66975, I have reviewed the changes and agree it makes sense to merge Tree-SHA512: 747b9d4676fad6d07f3955668639c93333625e69199ff4c499f01167de3875990d93db85e775a7f5b1b684575dceaec8aa000b4db15525fc47b699bac1c85e3d
2022-01-31Merge bitcoin/bitcoin#24136: Extract CTxIn::MAX_SEQUENCE_NONFINAL constant, ↵MarcoFalke
rework BIP 65/68/112 docs fa4339e4c1bb60e0d9263d4f0fe65d03aad52f88 Extract CTxIn::MAX_SEQUENCE_NONFINAL constant (MarcoFalke) Pull request description: Extracting the constant makes it possible to attach documentation to it. Also, rework the docs for the other "sequence constants". ACKs for top commit: w0xlt: reACK fa4339e for specifying the transaction version. darosior: re-ACK fa4339e4c1bb60e0d9263d4f0fe65d03aad52f88 luke-jr: crACK fa4339e4c1bb60e0d9263d4f0fe65d03aad52f88 Tree-SHA512: 8d8f3dd5afb33eb5b72aa558e1e03de874c5ed02aa1084888e92ed86f3aaa5c725db45ded02e14cdfa67a92ac6774e97185b697f20a8ab63abbfcaa2fcd1fc6a
2022-01-31Merge bitcoin/bitcoin#24179: fuzz: Speed up script fuzz targetMarcoFalke
fa6842978d01f6707564a841303033d7bfbabb3b fuzz: Speed up script fuzz target (MarcoFalke) Pull request description: Currently the script fuzz target takes the longest time (5000 seconds, aka 80 minutes, see https://cirrus-ci.com/task/5651378755338240?logs=ci#L4501). Fix this by making it twice as fast. Instead of running all possible combinations for all fuzz inputs, consume a bool and decide at runtime which path to take. I moved the new calls to the end to not invalidate existing fuzz inputs. ACKs for top commit: prusnak: ACK fa6842978d01f6707564a841303033d7bfbabb3b Tree-SHA512: 5e408255f96f9e92e472f4e8a8a0f8d8814bad444ac0ff7d5db5ed84a59a861135ffe5e04d81f479b0695cb17e4d7af005734959dd4aa9328bdc5acc98f36665
2022-01-28test: Fix sanitizer suppresions in streams_testsMarcoFalke
2022-01-28Merge bitcoin/bitcoin#23508: Add getdeploymentinfo RPCMarcoFalke
a3809228917b8f750090c8bfec8e283391dbb524 Release notes for getdeploymentinfo rpc (Anthony Towns) 240cad09baefcf363cce36a4b2795122adfce27f rpc: getdeploymentinfo: include signalling info (Anthony Towns) 376c0c6dae2bebbb3e1352377e71fb1996d09f64 rpc: getdeploymentinfo: include block hash/height (Anthony Towns) a7469bcd35692d56f57e91b3f21d30855bdf6531 rpc: getdeploymentinfo: change stats to always refer to current period (Anthony Towns) 7f15c1841b98de6931a7ac68e16635a05d3e96cf rpc: getdeploymentinfo: allow specifying a blockhash other than tip (Anthony Towns) fd826130a0a4e67fdc26f8064f4ecb4ff79b3333 rpc: move softfork info from getblockchaininfo to getdeploymentinfo (Anthony Towns) Pull request description: The aim of this PR is to improve the ability to monitor soft fork status. It first moves the softfork section from getblockchaininfo into a new RPC named getdeploymentinfo, which is then also able to query the status of forks at an arbitrary block rather than only at the tip. In addition, bip9 status is changed to indicate the status of the given block, rather than just for the next block, and an additional field is included to indicate whether each block in the signalling period signaled. ACKs for top commit: laanwj: Code review and lightly tested ACK a3809228917b8f750090c8bfec8e283391dbb524 Sjors: tACK a3809228917b8f750090c8bfec8e283391dbb524 fjahr: tACK a3809228917b8f750090c8bfec8e283391dbb524 Tree-SHA512: 7417d733b47629f229c5128586569909250481a3e94356c52fe67a03fd42cd81745246e384b98c4115fb61587714c879e4bc3e5f5c74407d9f8f6773472a33cb
2022-01-27Merge bitcoin/bitcoin#23438: refactor: Use spans of std::byte in serializelaanwj
fa5d2e678c809c26bd40d7e7c171529d3ffb5903 Remove unused char serialize (MarcoFalke) fa24493d6394b3a477535f480664c9596f18e3c5 Use spans of std::byte in serialize (MarcoFalke) fa65bbf217b725ada35107b4ad646d250228355c span: Add BytePtr helper (MarcoFalke) Pull request description: This changes the serialize code (`.read()` and `.write()` functions) to take a `Span` instead of a pointer and size. This is a breaking change for the serialize interface, so at no additional cost we can also switch to `std::byte` (instead of using `char`). The benefits of using `Span`: * Less verbose and less fragile code when passing an already existing `Span`(-like) object to or from serialization The benefits of using `std::byte`: * `std::byte` can't accidentally be mistaken for an integer The goal here is to only change serialize to use spans of `std::byte`. If needed, `AsBytes`, `MakeUCharSpan`, ... can be used (temporarily) to pass spans of the right type. Other changes that are included here: * [#22167](https://github.com/bitcoin/bitcoin/pull/22167) (refactor: Remove char serialize by MarcoFalke) * [#21906](https://github.com/bitcoin/bitcoin/pull/21906) (Preserve const in cast on CTransactionSignatureSerializer by promag) ACKs for top commit: laanwj: Concept and code review ACK fa5d2e678c809c26bd40d7e7c171529d3ffb5903 sipa: re-utACK fa5d2e678c809c26bd40d7e7c171529d3ffb5903 Tree-SHA512: 08ee9eced5fb777cedae593b11e33660bed9a3e1711a7451a87b835089a96c99ce0632918bb4666a4e859c4d020f88fb50f2dd734216b0c3d1a9a704967ece6f
2022-01-27fuzz: Speed up script fuzz targetMarcoFalke
2022-01-27Merge bitcoin/bitcoin#22932: Add CBlockIndex lock annotations, guard ↵laanwj
nStatus/nFile/nDataPos/nUndoPos by cs_main 6ea56827842b9b2bd730edc38f3a7b1f46f6247b Guard CBlockIndex::nStatus/nFile/nDataPos/nUndoPos by cs_main (Jon Atack) 5d59ae0ba88849b1eb0d7350871bc19fcd5ef601 Remove/inline ReadRawBlockFromDisk(block_data, pindex, message_start) (Hennadii Stepanov) eaeeb88768db529b5241ccd42f1e87579908b4df Require IsBlockPruned() to hold mutex cs_main (Jon Atack) ca47b005770f71aa229ecc1f7b8146a96ff02151 Require CBlockIndex::IsValid() to hold cs_main (Vasil Dimov) e9f3aa5f6a7b39e8d5f2069617e5e382798d8d60 Require CBlockIndex::RaiseValidity() to hold cs_main (Vasil Dimov) 8ef457cb83fac796f8b6a56977b1016193fc1185 Require CBlockIndex::IsAssumedValid() to hold cs_main (Vasil Dimov) 572393448b4d32f91b92edc84b4200ab52d62422 Require CBlockIndex::GetUndoPos() to hold mutex cs_main (Jon Atack) 2e557ced2830fc54476e598d52225f1679205e7d Require WriteUndoDataForBlock() to hold mutex cs_main (Jon Atack) 6fd4341c10b319399c58d71c4ddeae4417e337d7 Require CBlockIndex::GetBlockPos() to hold mutex cs_main (Jon Atack) Pull request description: Issues: - `CBlockIndex` member functions `GetBlockPos()`, `GetUndoPos()`, `IsAssumedValid()`, `RaiseValidity()`, and `IsValid()` and block storage functions `WriteUndoDataForBlock()` and `IsBlockPruned()` are missing thread safety lock annotations to help ensure that they are called with mutex cs_main to avoid bugs like #22895. Doing this also enables the next step: - `CBlockIndex::nStatus` may be racy, i.e. potentially accessed by multiple threads, see #17161. A solution is to guard it by cs_main, along with fellow data members `nFile`, `nDataPos` and `nUndoPos`. This pull: - adds thread safety lock annotations for the functions listed above - guards `CBlockIndex::nStatus`, `nFile`, `nDataPos` and `nUndoPos` by cs_main How to review and test: - debug build with clang and verify there are no `-Wthread-safety-analysis` warnings - review the code to verify each annotation or lock is necessary and sensible, or if any are missing - look for whether taking a lock can be replaced by a lock annotation instead - for more information about Clang thread safety analysis, see - https://clang.llvm.org/docs/ThreadSafetyAnalysis.html - https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#lockingmutex-usage-notes - https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#threads-and-synchronization Mitigates/potentially closes #17161. ACKs for top commit: laanwj: Code review ACK 6ea56827842b9b2bd730edc38f3a7b1f46f6247b Tree-SHA512: 3ebf429c8623c51f944a7245a2e48d2aa088dec4c4914b40aa6049e89856c1ee8586f6e2e3b65195190566637a33004468b51a781e61a082248748015167569b
2022-01-26Extract CTxIn::MAX_SEQUENCE_NONFINAL constantMarcoFalke
2022-01-26fs: consistently use fsbridge for fopen()fanquake
2022-01-26fs: consistently use fsbridge for {i,o}fstreamfanquake
Part of #20744, but this can be done now, and will simplify the diff.
2022-01-25Guard CBlockIndex::nStatus/nFile/nDataPos/nUndoPos by cs_mainJon Atack
Co-authored-by: Vasil Dimov <vd@FreeBSD.org>
2022-01-25Require CBlockIndex::RaiseValidity() to hold cs_mainVasil Dimov
2022-01-25Require CBlockIndex::IsAssumedValid() to hold cs_mainVasil Dimov
2022-01-25Require CBlockIndex::GetBlockPos() to hold mutex cs_mainJon Atack
2022-01-25Merge bitcoin/bitcoin#24105: Optimize CHECKSIGADD Script ValidationMarcoFalke
cfa575266bc0198574a82e8e386040e969b05dea Optimize CHECKSIGADD Script Validation (Jeremy Rubin) Pull request description: This is a mild validation improvement that improves performance by caching some signature data when you have a Taproot script fragment that uses CHECKSIGADD Multisignatures with sighash single. In some basic testing I showed this to have about a 0.6% speedup during block validation for a block with a lot of CHECKSIGADDs, but that was with the entirety of block validation so the specific impact on the script interpreter performance should be a bit more once you subtract things like coin fetching. If desired I can produce a more specific/sharable bench for this, the code I used to test was just monkey patching the existing taproot tests since generating valid spends is kinda tricky. But it's sort of an obvious win so I'm not sure it needs a rigorous bench, but I will tinker on one of those while the code is being reviewed for correctness. The overhead of this approach is that: 1. ScriptExecutionData is no longer const 2. around 32 bytes of extra stack space 3. zero extra hashing since we only cache on first use ACKs for top commit: sipa: utACK cfa575266bc0198574a82e8e386040e969b05dea MarcoFalke: review ACK cfa575266bc0198574a82e8e386040e969b05dea jonatack: ACK cfa575266bc0198574a82e8e386040e969b05dea theStack: Code-review ACK cfa575266bc0198574a82e8e386040e969b05dea Tree-SHA512: d5938773724bb9c97b6fd623ef7efdf7f522af52dc0903ecb88c38a518b628d7915b7eae6a774f7be653dc6bcd92e9abc4dd5e8b11f3a995e01e0102d2113d09
2022-01-25Merge bitcoin/bitcoin#23804: validation: followups for de-duplication of ↵fanquake
packages 3cd7f693d3ed1bb7cf9ba3e0c482174df3684972 [unit test] package parents are a mix (glozow) de075a98eaf0b3f7676c5c78b50b66902202b34c [validation] better handle errors in SubmitPackage (glozow) 9d88853e0c85f765f7d982b15e8122ede50110ed AcceptPackage fixups (glozow) 2db77cd3b835d052de678755bcdde5a645ce2d65 [unit test] different witness in package submission (glozow) 9ad211c5753dbd148ba6f0ed56854f6364362ca8 [doc] more detailed explanation for deduplication (glozow) 83d4fb71260f268abd41d083fb3458476aed83ce [packages] return DIFFERENT_WITNESS for same-txid-different-witness tx (glozow) Pull request description: This addresses some comments from review on e12fafda2dfbbdf63f125e5af797ecfaa6488f66 from #22674. - Improve documentation about de-duplication: [comment](https://github.com/bitcoin/bitcoin/pull/22674/files#r770156708) - Fix code looking up same-txid-different-witness transaction in mempool: [comment](https://github.com/bitcoin/bitcoin/pull/22674/files#r770804029) - Improve the interface for when a same-txid-different-witness transaction is swapped: [comment](https://github.com/bitcoin/bitcoin/pull/22674/files#r770782822) - Add a test for witness swapping: [comment](https://github.com/bitcoin/bitcoin/pull/22674/files#r770804029) - Add a test for packages with a mix of duplicate/different witness/new parents: [comment](https://github.com/bitcoin/bitcoin/pull/22674#discussion_r773037608) - Fix issue with not notifying `CValidationInterface` when there's a partial submission due to fail-fast: [comment](https://github.com/bitcoin/bitcoin/pull/22674#discussion_r773013162) ACKs for top commit: achow101: ACK 3cd7f693d3ed1bb7cf9ba3e0c482174df3684972 t-bast: LGTM, ACK https://github.com/bitcoin/bitcoin/pull/23804/commits/3cd7f693d3ed1bb7cf9ba3e0c482174df3684972 instagibbs: ACK 3cd7f693d3ed1bb7cf9ba3e0c482174df3684972 ariard: ACK 3cd7f69 Tree-SHA512: a5d86ca86edab80a5a05fcbb828901c058b3f2fa2552912ea52f2871e29c3cf4cc34020e7aac2217959c9c3a01856f4bd3d631d844635b98144f212f76c2f3ef
2022-01-24Merge bitcoin/bitcoin#24078: net, refactor: Rename CNetMessage::m_command ↵MarcoFalke
with CNetMessage::m_type 224d87855ec38cc15866d9673e1b19942a82c1cd net, refactor: Drop tautological local variables (Hennadii Stepanov) 3073a9917b31d15ba958ea8148585633ba905f8b scripted-diff: Rename CNetMessage::m_command with CNetMessage::m_type (Hennadii Stepanov) Pull request description: https://github.com/bitcoin/bitcoin/pull/18533#issue-594592488: > a message is not a command, but simply a message of some type Continuation of bitcoin/bitcoin#18533 and bitcoin/bitcoin#18937. ACKs for top commit: theStack: Concept and code-review ACK 224d87855ec38cc15866d9673e1b19942a82c1cd shaavan: Code Review ACK 224d87855ec38cc15866d9673e1b19942a82c1cd w0xlt: crACK 224d878 Tree-SHA512: 898cafb44708dae1413fcc1533d809d75878891354f1b5edaaec1287f4921c31adc9330f4d42d82544a39689886bc17fee71ea587f9199fd5cc849d376f82176
2022-01-23Merge bitcoin/bitcoin#24021: Rename and move PoissonNextSend functionsfanquake
9b8dcb25b57ad31b77c9f37d9a1f5b07dc6378b4 [net processing] Rename PoissonNextSendInbound to NextInvToInbounds (John Newbery) ea99f5d01e56ab0192d211da1034ffb299876937 [net processing] Move PoissonNextSendInbound to PeerManager (John Newbery) bb060746df22c956b8f44e5b8cd1ae4ed73faddc scripted-diff: replace PoissonNextSend with GetExponentialRand (John Newbery) 03cfa1b6035dbcf6a414f9bc432bd9e612801ebb [refactor] Use uint64_t and std namespace in PoissonNextSend (John Newbery) 9e64d69bf74c8a381fb59841519cc3736bce14d4 [move] Move PoissonNextSend to src/random and update comment (John Newbery) Pull request description: `PoissonNextSend` and `PoissonNextSendInbound` are used in the p2p code to obfuscate various regularly occurring processes, in order to make it harder for others to get timing-based information deterministically. The naming of these functions has been confusing to several people (including myself, see also #23347) because the resulting random timestamps don't follow a Poisson distribution but an exponential distribution (related to events in a Poisson process, hence the name). This PR - moves `PoissonNextSend()` out of `net` to `random` and renames it to `GetExponentialRand()` - moves `PoissonNextSendInbound()` out of `CConnman` to `PeerManager` and renames it to `NextInvToInbounds()` - adds documentation for these functions This is work by jnewbery - due to him being less active currently, I opened the PR and will address feedback. ACKs for top commit: jnewbery: ACK 9b8dcb25b5 hebasto: ACK 9b8dcb25b57ad31b77c9f37d9a1f5b07dc6378b4, I have reviewed the code and it looks OK, I agree it can be merged. theStack: ACK 9b8dcb25b57ad31b77c9f37d9a1f5b07dc6378b4 📊 Tree-SHA512: 85c366c994e7147f9981fe863fb9838502643fa61ffd32d55a43feef96a38b79a5daa2c4d38ce01074897cc95fa40c76779816edad53f5265b81b05c3a1f4f50
2022-01-20Merge bitcoin/bitcoin#23171: qa: test descriptors with mixed xpubs and const ↵Andrew Chow
pubkeys 36012ef143917f97179d3ba6599ef36a26a9a014 qa: test descriptors with mixed xpubs and const pubkeys (Antoine Poinsot) Pull request description: Writing unit tests for Miniscript descriptors i noticed that `test/descriptor_tests`'s `DoCheck()` assumes that a descriptor would either contain only extended keys or only const pubkeys: if it detects an xpub in the descriptor it would assert the number of cached keys is equal to the number of keys in the descriptor, which does not hold if the descriptor also contains const (raw?) public keys since we only cache parent xpubs. ACKs for top commit: achow101: ACK 36012ef143917f97179d3ba6599ef36a26a9a014 Tree-SHA512: 2ede67a6dff726bcad3e260f3deb25c9b77542ed1880eb4ad136730b741014ce950396c69c7027225de1ef27108d609bafd055188b88538ace0beb13c7e34b0b
2022-01-20refactor tests to fix ubsan suppressionsMarcoFalke
2022-01-19Optimize CHECKSIGADD Script ValidationJeremy Rubin
2022-01-19scripted-diff: rename cs_mapLocalHost -> g_maplocalhost_mutexw0xlt
-BEGIN VERIFY SCRIPT- s() { sed -i 's/cs_mapLocalHost/g_maplocalhost_mutex/g' $1; } s src/net.cpp s src/net.h s src/rpc/net.cpp s src/test/net_tests.cpp -END VERIFY SCRIPT-
2022-01-17[unit test] package parents are a mixglozow
2022-01-17[unit test] different witness in package submissionglozow
If/when witness replacement is implemented in the future, this test case can be easily replaced with witness replacement tests.
2022-01-17Merge bitcoin/bitcoin#23373: test: Parse command line arguments from unit ↵MarcoFalke
and fuzz tests, make addrman consistency check ratio easier to change 7f122a4188af7130be9251611e41136a17c814f1 fuzz: non-addrman fuzz tests: override-able check ratio (Vasil Dimov) 3bd83e273d104e9474af8f1bdf4f969163e33ade fuzz: addrman fuzz tests: override-able check ratio (Vasil Dimov) 46b0fe78298c8f416a91dec9d4e0f3f4cb1e68b0 test: non-addrman unit tests: override-able check ratio (Vasil Dimov) 81e4d54d3a95f7bffeb353217a6c32eb2aca8b5c test: addrman unit tests: override-able check ratio (Vasil Dimov) 6dff6214be768a3fab6d5201daf5ef6071764746 bench: put addrman check ratio in a variable (Vasil Dimov) 6f7c7567c578b5a41f8e90ce4491e40f7faeaa56 fuzz: parse the command line arguments in fuzz tests (Vasil Dimov) 92a0f7e58d4b6323d21f1c45d4c20266c35df030 test: parse the command line arguments in unit tests (Vasil Dimov) Pull request description: Previously command line arguments passed to unit and fuzz tests would be ignored by the tests themselves. They would be used by the boost test framework (e.g. `--run_test="addrman_tests/*"`) or by the fuzzer (e.g. `-runs=1`). However both provide ways to pass down the extra arguments to the test itself. Use that, parse the arguments and make them available to the tests via `gArgs`. This makes the tests more flexible as they can be run with any bitcoind config option specified on the command line. When creating `AddrMan` objects in tests, use `-checkaddrman=` (if provided) instead of hardcoding the check ratio in many different places. See https://github.com/bitcoin/bitcoin/pull/20233#issuecomment-889813074 for further motivation for this. ACKs for top commit: mzumsande: re-ACK 7f122a4188af7130be9251611e41136a17c814f1 josibake: reACK https://github.com/bitcoin/bitcoin/pull/23373/commits/7f122a4188af7130be9251611e41136a17c814f1 Tree-SHA512: 3a05e61e4d70a0569bb67594bcce3aad8fdef63cdcc54e2823a3bc9f18679571985004412b6c332a210f67849bab32d8467b4115fbff8f5fac9834982e60dcf3
2022-01-17Merge bitcoin/bitcoin#23992: fuzz: Limit fuzzed time to years 2000-2100MarcoFalke
fa7238300c18938cdf627cacfc58d4b81602417f fuzz: Limit fuzzed time to years 2000-2100 (MarcoFalke) Pull request description: It doesn't make sense to fuzz times in the past, as Bitcoin Core will refuse to start in the past. Fix that and also remove a sanitizer suppression, which would be hit in net_processing in `ProcessMessage`: ```cpp if (addr.nTime <= 100000000 || addr.nTime > nNow + 10 * 60) addr.nTime = nNow - 5 * 24 * 60 * 60; // <-- Here ``` This changes the format of fuzz inputs. Previously a time value was (de)serialized as 40 bytes, now it is 32 bytes. ACKs for top commit: mzumsande: Code Review ACK fa7238300c18938cdf627cacfc58d4b81602417f Tree-SHA512: ca6e7233beec2d9ef9fd481d8f1331942a4d2c8fe518b857629bebcc53a4f42ae123b994cf5d359384a0a8022098ff5a9c146600bc2593c6d88734e25bc240ad
2022-01-15scripted-diff: Rename CNetMessage::m_command with CNetMessage::m_typeHennadii Stepanov
-BEGIN VERIFY SCRIPT- sed -i 's/std::string m_command;/std::string m_type;/g' ./src/net.h sed -i 's/* command and size./* type and size./g' ./src/net.h sed -i 's/msg.m_command/msg.m_type/g' ./src/net.cpp ./src/net_processing.cpp ./src/test/fuzz/p2p_transport_serialization.cpp -END VERIFY SCRIPT-
2022-01-15rpc: getdeploymentinfo: include signalling infoAnthony Towns
2022-01-15rpc: getdeploymentinfo: change stats to always refer to current periodAnthony Towns
On a period boundary, getdeploymentinfo (and previously getblockchaininfo) would report the status and statistics for the next block rather than the current block. Change this to always report the status/statistics of the current block, but add status-next to report the status for the next block.
2022-01-15rpc: move softfork info from getblockchaininfo to getdeploymentinfoAnthony Towns
2022-01-13[net processing] Move PoissonNextSendInbound to PeerManagerJohn Newbery
2022-01-12Merge bitcoin/bitcoin#24041: util: Restore GetIntArg saturating behaviorMarcoFalke
b5c9bb5cb9f4a8db57b33ef7399310c7d6de5822 util: Restore GetIntArg saturating behavior (James O'Beirne) Pull request description: The new locale-independent atoi64 method introduced in #20452 parses large integer values higher than maximum representable value as 0 instead of the maximum value, which breaks backwards compatibility. This commit restores compatibility and adds test coverage for this case in terms of the related GetIntArg and strtoll functions. Specifically, command line or bitcoin.conf integer values greater than `9223372036854775807` (`2**63-1`) used to be parsed as `9223372036854775807` before #20452. Then #20452 caused them to be parsed as `0`. And after this PR they will be parsed as `9223372036854775807` again. This change is a stripped-down alternative version of #23841 by jamesob ACKs for top commit: jamesob: Github ACK https://github.com/bitcoin/bitcoin/pull/24041/commits/b5c9bb5cb9f4a8db57b33ef7399310c7d6de5822 vincenzopalazzo: ACK https://github.com/bitcoin/bitcoin/pull/24041/commits/b5c9bb5cb9f4a8db57b33ef7399310c7d6de5822 MarcoFalke: review ACK b5c9bb5cb9f4a8db57b33ef7399310c7d6de5822 🌘 Tree-SHA512: 4e8abdbabf3cf4713cf5a7c5169539159f359ab4109a4e7e644cc2e9b2b0c3c532fad9f6b772daf015e1c5340ce59280cd9a41f2730afda6099cbf636b7d23ae
2022-01-11util: Restore GetIntArg saturating behaviorJames O'Beirne
The new locale-independent atoi64 method introduced in #20452 parses large integer values higher than maximum representable value as 0 instead of the maximum value, which breaks backwards compatibility. This commit restores compatibility and adds test coverage for this case in terms of the related GetIntArg and strtoll functions. Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
2022-01-11fuzz: non-addrman fuzz tests: override-able check ratioVasil Dimov
Make it possible to override from the command line (without recompiling) the addrman check ratio in non-addrman fuzz tests (connman and deserialize) instead of hardcoding it to 0: ``` FUZZ=connman ./src/test/fuzz/fuzz --checkaddrman=5 ```
2022-01-11fuzz: addrman fuzz tests: override-able check ratioVasil Dimov
Make it possible to override from the command line (without recompiling) the addrman check ratio in addrman fuzz tests instead of hardcoding it to 0: ``` FUZZ=addrman ./src/test/fuzz/fuzz --checkaddrman=5 ```
2022-01-11test: non-addrman unit tests: override-able check ratioVasil Dimov
Make it possible to override from the command line (without recompiling) the addrman check ratio in the common `TestingSetup::m_node::addrman` (used by all unit tests) instead of hardcoding it to 0: ``` test_bitcoin --run_test="transaction_tests/tx_valid" -- -checkaddrman=1 ```
2022-01-11test: addrman unit tests: override-able check ratioVasil Dimov
In addrman unit tests, make it possible to override the check ratio from the command line, without recompiling: ``` test_bitcoin --run_test="addrman_tests/*" -- -checkaddrman=1 ``` Also, make the arguments of the constructor of `AddrManTest` the same as the arguments of `AddrMan`.
2022-01-11fuzz: parse the command line arguments in fuzz testsVasil Dimov
Retrieve the command line arguments from the fuzzer and save them for later retrieval by `BasicTestingSetup` so that we gain extra flexibility of passing any config options on the test command line, e.g.: ``` FUZZ=addrman ./src/test/fuzz/fuzz --checkaddrman=5 ``` A fuzz test should call `MakeNoLogFileContext<>()` in its initialize function in order to invoke the constructor of `BasicTestingSetup`, which sets `gArgs`.
2022-01-11test: parse the command line arguments in unit testsVasil Dimov
Retrieve the command line arguments from boost and pass them to `BasicTestingSetup` so that we gain extra flexibility of passing any config options on the test command line, e.g.: ``` test_bitcoin -- -printtoconsole=1 -checkaddrman=5 ```
2022-01-11Remove cs_main lock annotation from ChainstateManager.m_blockmanRyan Ofsky
BlockManager is a large data structure, and cs_main is not required to take its address or access every part of it. Individual BlockManager fields and methods which do require cs_main like m_block_index and LookupBlockIndex are already annotated separately, and these other annotations describe locking requirements more accurately and do a better job enforcing thread safety. Since cs_main is not needed to access the address of the m_block object, this commit drops cs_main LOCK calls which were added pointlessly to satisfy this annotation in the past. Co-authored-by: Carl Dong <contact@carldong.me>
2022-01-11Merge bitcoin/bitcoin#23497: Add `src/node/` and `src/wallet/` code to ↵MarcoFalke
`node::` and `wallet::` namespaces e5b6aef61221b621ad77b5f075a16897e08835bf Move CBlockFileInfo::ToString method where class is declared (Russell Yanofsky) f7086fd8ff084ab0dd656d75b7485e59263bdfd8 Add src/wallet/* code to wallet:: namespace (Russell Yanofsky) 90fc8b089d591cabff60ee829a33f96c37fd27ba Add src/node/* code to node:: namespace (Russell Yanofsky) Pull request description: There are no code changes, this is just adding `namespace` and `using` declarations and `node::` or `wallet::` qualifiers in some places. Motivations for this change are: - To make it easier to see when node and wallet code is being accessed places where it shouldn't be. For example if GUI code is accessing node and wallet internals or if wallet and node code are referencing each other. - To make source code organization clearer ([#15732](https://github.com/bitcoin/bitcoin/issues/15732)), being able to know that `wallet::` code is in `src/wallet/`, `node::` code is in `src/node/`, `init::` code is in `src/init/`, `util::` code is in `src/util/`, etc. Reviewing with `git log -p -n1 -U0 --word-diff-regex=.` can be helpful to verify this is only updating declarations, not changing code. ACKs for top commit: achow101: ACK e5b6aef61221b621ad77b5f075a16897e08835bf MarcoFalke: Concept ACK e5b6aef61221b621ad77b5f075a16897e08835bf 🍨 Tree-SHA512: 3797745c90246794e2d55a2ee6e8b0ad5c811e4e03a242d3fdfeb68032f8787f0d48ed4097f6b7730f540220c0af99ef423cd9dbe7f76b2ec12e769a757a2c8d
2022-01-10fuzz: Limit fuzzed time to years 2000-2100MarcoFalke
2022-01-10Merge bitcoin/bitcoin#23994: Consolidate all uses of the fast range mapping ↵MarcoFalke
technique in util/fastrange.h efab28b06bfaa50c41337e84136cb58437e7ba00 Add FastRange32 function and use it throughout the codebase (Pieter Wuille) 96ecd6fa3e0f53c3a25eb7c328220b819f8dde03 scripted-diff: rename MapIntoRange to FastRange64 (Pieter Wuille) c6d15c45d971fb25551b7a66a2615e3f0bee999b [moveonly] Move MapIntoRange() to separate util/fastrange.h (Pieter Wuille) Pull request description: Several places in the codebase use the fast range mapping technique described in https://lemire.me/blog/2016/06/27/a-fast-alternative-to-the-modulo-reduction/, some for 32-bit ranges, some for 64-bit ones. Move all of these to `util/fastrange.h`, and give them a consistent name. ACKs for top commit: Sjors: ACK efab28b06bfaa50c41337e84136cb58437e7ba00 shaavan: reACK efab28b06bfaa50c41337e84136cb58437e7ba00 MarcoFalke: review ACK efab28b06bfaa50c41337e84136cb58437e7ba00 🍸 Tree-SHA512: 3190a25ef21d17f0ab2afcd9b8d5a1813fdfac0d93996878017e84ff62eee412c823d6149ae8e92cfc3214458641e83ace4b022b4a0fe0679f78dbaee21c6227
2022-01-07Merge bitcoin/bitcoin#23974: Make blockstorage globals private members of ↵fanquake
BlockManager fa68a6c2fc6754c160e0f98007785602201b3c47 scripted-diff: Rename touched member variables (MarcoFalke) facd3df21f344dd84e5f28862056700c1fded17c Make blockstorage globals private members of BlockManager (MarcoFalke) faa8c2d7d75f8d9782709e73e00e35851e233392 doc: Clarify nPruneAfterHeight for signet (MarcoFalke) fad381b2f8e1beb18f748fbeb820e63545b9b0fd test: Load genesis block to allow flush (MarcoFalke) fab262174b96854d2df5bee7da578990c9e9cb1e Move blockstorage-related unload to BlockManager::Unload (MarcoFalke) fa467f3913918701c765f9bc754203b4591b894f move-only: Create WriteBlockIndexDB helper (MarcoFalke) fa88cfd3f9896d5b56ea6c111a23f90a79253c18 Move functions to BlockManager (MarcoFalke) Pull request description: Globals aren't too nice because they hide dependencies, also they make testing harder. Fix that by removing some. ACKs for top commit: Sjors: ACK fa68a6c2fc6754c160e0f98007785602201b3c47 ryanofsky: Code review ACK fa68a6c2fc6754c160e0f98007785602201b3c47. Nice changes! Tree-SHA512: 6abc5929a5e43a05e238276721d46a64a44f23dca18c2caa9775437a32351d6815d88b88757254686421531d0df13861bbd3a202e13a3192798d87a96abef65d
2022-01-06Add src/wallet/* code to wallet:: namespaceRussell Yanofsky
2022-01-06Add src/node/* code to node:: namespaceRussell Yanofsky
2022-01-06scripted-diff: rename MapIntoRange to FastRange64Pieter Wuille
-BEGIN VERIFY SCRIPT- sed -i -e 's/MapIntoRange/FastRange64/' src/blockfilter.cpp src/test/fuzz/golomb_rice.cpp src/util/fastrange.h -END VERIFY SCRIPT-
2022-01-06Merge bitcoin/bitcoin#23760: util: move MapIntoRange() for reuse in fuzz testsMarcoFalke
df2307cdc3d08233d17beb9a50c144baaef1f44e util: move MapIntoRange() for reuse in fuzz tests (fanquake) Pull request description: ACKs for top commit: shaavan: ACK df2307cdc3d08233d17beb9a50c144baaef1f44e Tree-SHA512: 31bf18f50a82e442ff025d6be0db5666b463a1fc16ec6b2112c77bb815515d27f8a537a0c9934c7daa3f4d526b47e8d6333f75a13b271e6efa550f8e71504b0a
2022-01-06Merge bitcoin/bitcoin#23855: refactor: Post-"Chainstate loading sequence ↵MarcoFalke
coalescence" fixups e3544c864e3e56867de25b8db7b012d58b378050 init: Use clang-tidy named args syntax (Carl Dong) 3401630417d994b53ff3a89db2ea759ab1ec6f0f style-only: Rename *Chainstate return values (Carl Dong) 1dd582782d3c182aa952f23ec577f6a0a8672e7b docs: Make LoadChainstate comment more accurate (Carl Dong) 6b83576388e205116a0ebc67b9949f309eea1207 node/chainstate: Use MAX_FUTURE_BLOCK_TIME (Carl Dong) Pull request description: There are 2 proposed fixups in discussions in #23280 which I have not implemented: 1. An overhaul to return types and an option type for the two `*Chainstate` functions: https://github.com/bitcoin/bitcoin/pull/23280#issuecomment-984149564 - The change reintroduces stringy return types and is quite involved. It could be discussed in a separate PR. 2. Passing in the unix time to `VerifyChainstate` instead of a callback to get the time: https://github.com/bitcoin/bitcoin/pull/23280#discussion_r765051533 - I'm not sure it matters much whether it's a callback or just the actual unix time. Also, I think `VerifyDB` can take quite a while, and I don't want to impose that the function have to "run quickly" in order to have it be correct. If reviewers feel strongly about either of the two fixups listed above, please feel free to open a PR based on mine and I'll close this one! ACKs for top commit: ryanofsky: Code review ACK e3544c864e3e56867de25b8db7b012d58b378050 MarcoFalke: ACK e3544c864e3e56867de25b8db7b012d58b378050 🐸 Tree-SHA512: dd1de0265b6785eef306e724b678ce03d7c54ea9f4b5ea0ccd7af59cce2ea3aba73fd4af0c15e2dca9265807dc4075f9afa2ec103672677b6638b1a4fc090904