aboutsummaryrefslogtreecommitdiff
path: root/src/test/util
AgeCommit message (Collapse)Author
2023-02-01Merge bitcoin/bitcoin#25974: test, build: Separate `read_json` function into ↵fanquake
its own module 7a820cee0e6408f5848799011d82dd29ee7fa8c5 test, build: Separate `read_json` function into its own module (Hennadii Stepanov) Pull request description: Currently, 4 source files rely on the definition of the `read_json` function provided in `src/test/script_tests.cpp`. This PR breaks this entanglement, improves code structure and maintainability. ACKs for top commit: fanquake: ACK 7a820cee0e6408f5848799011d82dd29ee7fa8c5 Tree-SHA512: f1567989f76cb54ab86cc48927851a8c424b08a9483d02d4918b629e0c792108bad4ccf7fa341d57b0921d91e84bf8fa3b9c07e5fdf12c64d9d5da83e4e464fb
2023-02-01Merge bitcoin/bitcoin#26705: clang-tidy: Fix ↵MarcoFalke
`modernize-use-default-member-init` in headers and force to check all headers b0e916913cedb8154419ec818bb9094a72fc8379 clang-tidy: Force to check all headers (Hennadii Stepanov) 96ee992ac3535848e2dc717bf284339badd40dcb clang-tidy: Fix `modernize-use-default-member-init` in headers (Hennadii Stepanov) Pull request description: This PR: - fixes the only [remained](https://github.com/bitcoin/bitcoin/pull/26705#issuecomment-1353742082) check in headers, i.e., `modernize-use-default-member-init` - forces `clang-tidy` check all headers Closes bitcoin/bitcoin#26703. ACKs for top commit: MarcoFalke: review ACK b0e916913cedb8154419ec818bb9094a72fc8379 🍹 Tree-SHA512: 4d33fe873094914541ae81968cdb4e7a7a01b3fdd4f25bc6daa8a53f45dab80565a5b3607ddc338f122369ca5a0a2d0d09c8e78cabe1beb6bd50c115bc5c5210
2023-01-31clang-tidy: Fix `modernize-use-default-member-init` in headersHennadii Stepanov
See https://clang.llvm.org/extra/clang-tidy/checks/modernize/use-default-member-init.html
2023-01-30Add xoroshiro128++ PRNGMartin Leitner-Ankerl
Xoroshiro128++ is a fast non-cryptographic random generator. Reference implementation is available at https://prng.di.unimi.it/ Co-Authored-By: Pieter Wuille <pieter@wuille.net>
2023-01-30net: simplify the call to vProcessMsg.splice()Vasil Dimov
At the time when ```cpp pnode->vProcessMsg.splice(pnode->vProcessMsg.end(), pnode->vRecvMsg, pnode->vRecvMsg.begin(), it); ``` is called, `it` is certainly `pnode->vRecvMsg.end()` which makes the call equivalent to: ```cpp pnode->vProcessMsg.splice(pnode->vProcessMsg.end(), pnode->vRecvMsg, pnode->vRecvMsg.begin(), pnode->vRecvMsg.end()); ``` which is equivalent to: ```cpp pnode->vProcessMsg.splice(pnode->vProcessMsg.end(), pnode->vRecvMsg); ``` Thus, use the latter. Further, maybe irrelevant, but the latter has constant complexity while the original code is `O(length of vRecvMsg)`.
2023-01-27test, build: Separate `read_json` function into its own moduleHennadii Stepanov
2023-01-16Add BlockManager::IsPruneMode()MarcoFalke
2023-01-11Merge bitcoin/bitcoin#26695: bench: BlockAssembler on a mempool with packagesAndrew Chow
04528054fcde61aa00e009dbbe1ac350ca1cf748 [bench] BlockAssembler with mempool packages (glozow) 6ce265acf4ff6ee5057b46bcb8b55abc4422e6f8 [test util] lock cs_main before pool.cs in PopulateMempool (glozow) 8791410662ce3ab7ba6bbe9813c55369edd6e4c9 [test util] randomize fee in PopulateMempool (glozow) cba5934eb697aedbe1966ebc2817ab87232a1b59 [miner] allow bypassing TestBlockValidity (glozow) c0588523083c9c78770b8b19a52a919db56250d9 [refactor] parameterize BlockAssembler::Options in PrepareBlock (glozow) a2de971ba1c588488dde653a76853666429d4911 [refactor] add helper to apply ArgsManager to BlockAssembler::Options (glozow) Pull request description: Performance of block template building matters as miners likely want to be able to start mining on a block with transactions asap after a block is found. We would want to know if a mempool PR accidentally caused, for example, a 100x slowdown. An `AssembleBlock()` bench exists, but it operates on a mempool with 101 transactions, each with 0 ancestors or descendants and with the same fee. Adding a bench with a more complex mempool is useful because (1) it's more realistic (2) updating packages can potentially cause the algorithm to take a long time. ACKs for top commit: kevkevinpal: Tested ACK [0452805](https://github.com/bitcoin/bitcoin/pull/26695/commits/04528054fcde61aa00e009dbbe1ac350ca1cf748) achow101: ACK 04528054fcde61aa00e009dbbe1ac350ca1cf748 stickies-v: ACK 04528054f Tree-SHA512: 38c138d6a75616651f9b1faf4e3a1cd833437a486f4e84308fbee958e8462bb570582c88f7ba7ab99d80191e97855ac2cf27c43cc21585d3e4b0e227effe2fb5
2022-12-27clang-tidy: Add `performance-no-automatic-move` checkHennadii Stepanov
https://clang.llvm.org/extra/clang-tidy/checks/performance/no-automatic-move.html
2022-12-24scripted-diff: Bump copyright headersHennadii Stepanov
-BEGIN VERIFY SCRIPT- ./contrib/devtools/copyright_header.py update ./ -END VERIFY SCRIPT- Commits of previous years: - 2021: f47dda2c58b5d8d623e0e7ff4e74bc352dfa83d7 - 2020: fa0074e2d82928016a43ca408717154a1c70a4db - 2019: aaaaad6ac95b402fe18d019d67897ced6b316ee0
2022-12-22[test util] lock cs_main before pool.cs in PopulateMempoolglozow
2022-12-22[test util] randomize fee in PopulateMempoolglozow
This makes the contents of the mempool more realistic and iterating by ancestor feerate order more meaningful. If transactions have varying feerates, it's also more likely that packages will need to be updated during block template assembly.
2022-12-22[refactor] parameterize BlockAssembler::Options in PrepareBlockglozow
2022-11-30refactor: Move `txmempool_entry.h` --> `kernel/mempool_entry.h`Hennadii Stepanov
2022-11-29Merge bitcoin/bitcoin#26532: wallet: bugfix, invalid crypted key ↵Andrew Chow
"checksum_valid" set 13d97608297bd56ed033d0e754d2e50447b02af0 test: load wallet, coverage for crypted keys (furszy) 373c99633ec7f20557db2734c49116ee4ad15423 refactor: move DuplicateMockDatabase to wallet/test/util.h (furszy) ee7a984f85015b610be4929b7c35cb501c1fbf7c refactor: unify test/util/wallet.h with wallet/test/util.h (furszy) cc5a5e81217506ec6f9fff34056290f8f40a7396 wallet: bugfix, invalid crypted key "checksum_valid" set (furszy) Pull request description: At wallet load time, the crypted key "checksum_valid" variable is always set to false. Which, on every wallet decryption call, forces the process to re-write all the ckeys to db when it's not needed. Note: The first commit fixes the issue, the two commits in the middle are cleanups so `DuplicateMockDatabase` can be used without duplicating code. And, the last one is pure test coverage for the crypted keys loading process. Includes test coverage for the following scenarios: 1) "All ckeys checksums valid" test: Loads an encrypted wallet with all the crypted keys with a valid checksum and verifies that 'CWallet::Unlock' doesn't force an entire crypted keys re-write. (we force a complete ckeys re-write if we find any missing crypted key checksum during the wallet loading process) 2) "Missing checksum in one ckey" test: Verifies that loading up a wallet with, at least one, 'ckey' with no checksum triggers a complete re-write of the crypted keys. 3) "Invalid ckey checksum error" test: Verifies that loading up a ckey with an invalid checksum stops the wallet loading process with a corruption error. 4) "Invalid ckey pubkey error" test: Verifies that loading up a ckey with an invalid pubkey stops the wallet loading process with a corruption error. ACKs for top commit: achow101: ACK 13d97608297bd56ed033d0e754d2e50447b02af0 aureleoules: ACK 13d97608297bd56ed033d0e754d2e50447b02af0 Tree-SHA512: 9ea630ee4a355282fbeee61ca04737294382577bb4b2631f50e732568fdab8f72491930807fbda58206446c4f26200cdc34d8afa14dbe1241aec713887d06a0b
2022-11-22Merge bitcoin/bitcoin#26376: test: Use type-safe NodeSeconds for ↵fanquake
TestMemPoolEntryHelper fa2d01470a9f1a91f35ed8013635ac47dabd868b test: Use type-safe NodeSeconds for TestMemPoolEntryHelper (MacroFake) Pull request description: test-only refactor to drop the deprecated `GetTime` in favour of the type-safe alternative ACKs for top commit: aureleoules: ACK fa2d01470a9f1a91f35ed8013635ac47dabd868b - verified that there is no behavior change Tree-SHA512: 5b64dae19c7bba9e8d90377c85891bc86f60ffbe67ea28d5ed3bd38f6dc30d3fbfba00bf49a16792922bddf83a52c632b6e5e5d8ffe1619fd9bf63effc60d59a
2022-11-21refactor: unify test/util/wallet.h with wallet/test/util.hfurszy
files share the same purpose, and we shouldn't have wallet code inside the test directory. This later is needed to use wallet util functions in the bench and test binaries without be forced to duplicate them.
2022-11-18Merge bitcoin/bitcoin#17786: refactor: Nuke policy/fees->mempool circular ↵glozow
dependencies c8dc0e3eaa9e0f956c5177bcb69632beb0d51770 refactor: Inline `CTxMemPoolEntry` class's functions (Hennadii Stepanov) 75bbe594e54402ed248ecf2bdfe54e8283d20fff refactor: Move `CTxMemPoolEntry` class to its own module (Hennadii Stepanov) Pull request description: This PR: - gets rid of the `policy/fees` -> `txmempool` -> `policy/fees` circular dependency - is an alternative to #13949, which nukes only one circular dependency ACKs for top commit: ryanofsky: Code review ACK c8dc0e3eaa9e0f956c5177bcb69632beb0d51770. Just include and whitespace changes since last review, and there's a moveonly commit now so it's very easy to review theStack: Code-review ACK c8dc0e3eaa9e0f956c5177bcb69632beb0d51770 glozow: utACK c8dc0e3eaa9e0f956c5177bcb69632beb0d51770, agree these changes are an improvement. Tree-SHA512: 36ece824e6ed3ab1a1e198b30a906c8ac12de24545f840eb046958a17315ac9260c7de26e11e2fbab7208adc3d74918db7a7e389444130f8810548ca2e81af41
2022-11-16refactor: Move `CTxMemPoolEntry` class to its own moduleHennadii Stepanov
This change nukes the policy/fees->mempool circular dependency. Easy to review using `diff --color-moved=dimmed-zebra`.
2022-10-31refactor: move url.h/cpp from lib util to lib commonfanquake
2022-10-27Merge bitcoin/bitcoin#25685: wallet: Faster transaction creation by removing ↵Andrew Chow
pre-set-inputs fetching responsibility from Coin Selection 3fcb545ab26be3e785b5e5654be0bdc77099d827 bench: benchmark transaction creation process (furszy) a8a75346d7e7247596c8a580d65ceaad49c97b97 wallet: SelectCoins, return early if target is covered by preset-inputs (furszy) f41712a734dc119f8a5e053a9cfa1f0411b5e8f1 wallet: simplify preset inputs selection target check (furszy) 5baedc33519661af9d19efcefd23dca8998d2547 wallet: remove fetch pre-selected-inputs responsibility from SelectCoins (furszy) 295852f61998a025b0b28a0671e6e1cf0dc08d0d wallet: encapsulate pre-selected-inputs lookup into its own function (furszy) 37e7887cb4bfd7db6eb462ed0741c45aea22a990 wallet: skip manually selected coins from 'AvailableCoins' result (furszy) 94c0766b0cd1990c1399a745c88c2ba4c685d8d1 wallet: skip available coins fetch if "other inputs" are disallowed (furszy) Pull request description: #### # Context (Current Flow on Master) In the transaction creation process, in order to select which coins the new transaction will spend, we first obtain all the available coins known by the wallet, which means walking-through the wallet txes map, gathering the ones that fulfill certain spendability requirements in a vector. This coins vector is then provided to the Coin Selection process, which first checks if the user has manually selected any input (which could be internal, aka known by the wallet, or external), and if it does, it fetches them by searching each of them inside the wallet and/or inside the Coin Control external tx data. Then, after finding the pre-selected-inputs and gathering them in a vector, the Coin Selection process walks-through the entire available coins vector once more just to erase coins that are in both vectors. So the Coin Selection process doesn’t pick them twice (duplicate inputs inside the same transaction). #### # Process Workflow Changes Now, a new method, `FetchCoins` will be responsible for: 1) Lookup the user pre-selected-inputs (which can be internal or external). 2) And, fetch the available coins in the wallet (excluding the already fetched ones). Which will occur prior to the Coin Selection process. Which allows us to never include the pre-selected-inputs inside the available coins vector in the first place, as well as doing other nice improvements (written below). So, Coin Selection can perform its main responsibility without mixing it with having to fetch internal/external coins nor any slow and unneeded duplicate coins verification. #### # Summarizing the Improvements: 1) If any pre-selected-input lookup fail, the process will return the error right away. (before, the wallet was fetching all the wallet available coins, walking through the entire txes map, and then failing for an invalid pre-selected-input inside SelectCoins) 2) The pre-selected-inputs lookup failure causes are properly described on the return error. (before, we were returning an "Insufficient Funds" error for everything, even if the failure was due a not solvable external input) 3) **Faster Coin Selection**: no longer need to "remove the pre-set inputs from the available coins vector so that Coin Selection doesn't pick them" (which meant to loop-over the entire available coins vector at Coin Selection time, erasing duplicate coins that were pre-selected). Now, the available coins vector, which is built after the pre-selected-inputs fetching, doesn’t include the already selected inputs in the first place. 4) **Faster transaction creation** for transactions that only use manually selected inputs. We now will return early, as soon as we finish fetching the pre-selected-inputs and not perform the resources expensive calculation of walking-through the entire wallet txes map to obtain the available coins (coins that we will not use). --------------------------- Added a new bench (f6d0bb2) measuring the transaction creation process, for a wallet with ~250k UTXO, only using the pre-selected-inputs inside coin control. Setting `m_allow_other_inputs=false` to disallow the wallet to include coins automatically. #### Result on this PR (tip f6d0bb2d): | ns/op | op/s | err% | total | benchmark |--------------------:|--------------------:|--------:|----------:|:---------- | 1,048,675.00 | 953.58 | 0.3% | 0.06 | `WalletCreateTransaction` vs #### Result on master (tip 4a4289e2): | ns/op | op/s | err% | total | benchmark |--------------------:|--------------------:|--------:|----------:|:---------- | 96,373,458.20 | 10.38 | 0.2% | 5.30 | `WalletCreateTransaction` The benchmark took to run in master: **96.37 milliseconds**, while in this PR: **1 millisecond** 🚀 . ACKs for top commit: S3RK: Code Review ACK 3fcb545ab26be3e785b5e5654be0bdc77099d827 achow101: ACK 3fcb545ab26be3e785b5e5654be0bdc77099d827 aureleoules: reACK 3fcb545ab26be3e785b5e5654be0bdc77099d827 Tree-SHA512: 42f833e92f40c348007ca565a4c98039e6f1ff25d8322bc2b27115824744779baf0b0a38452e4e2cdcba45076473f1028079bbd0f670020481ec5d3db42e4731
2022-10-26bench: benchmark transaction creation processfurszy
Goal 1: Benchmark the transaction creation process for pre-selected-inputs only. Setting `m_allow_other_inputs=false` to disallow the wallet to include coins automatically. Goal 2: Benchmark the transaction creation process for pre-selected-inputs and coin selection. ----------------------- Benchmark Setup: 1) Generates a 5k blockchain, loading the wallet with 5k transactions with two outputs each. 2) Fetch 4 random UTXO from the wallet's available coins and pre-select them as inputs inside CoinControl. Benchmark (Goal 1): Call `CreateTransaction` providing the coin control, who has set `m_allow_other_inputs=false` and the manually selected coins. Benchmark (Goal 2): Call `CreateTransaction` providing the coin control, who has set `m_allow_other_inputs=true` and the manually selected coins.
2022-10-26Merge bitcoin/bitcoin#25704: refactor: Remove almost all validation option ↵MacroFake
globals aaaa7bd0ba60aa7114810d4794940882d987c0aa iwyu: Add missing includes (MacroFake) fa9ebec096ae185576a54aa80bd2a9e57f867ed4 Remove g_parallel_script_checks (MacroFake) fa7c834b9f988fa7f2ace2d67b1628211b7819df Move ::fCheckBlockIndex into ChainstateManager (MacroFake) fa43188d86288fa6666307a77c106c8f069ebdbe Move ::fCheckpointsEnabled into ChainstateManager (MacroFake) cccca83099453bf0882bce4f897f77eee5836e8b Move ::nMinimumChainWork into ChainstateManager (MacroFake) fa29d0b57cdeb91c8798d5c90ba9cc18085e99fb Move ::hashAssumeValid into ChainstateManager (MacroFake) faf44876db555f7488c8df96db9fa88b793f897c Move ::nMaxTipAge into ChainstateManager (MacroFake) Pull request description: It seems preferable to assign globals to a class (in this case `ChainstateManager`), than to leave them dangling. This should clarify scope for code-readers, as well as clarifying unit test behaviour. ACKs for top commit: dergoegge: Code review ACK aaaa7bd0ba60aa7114810d4794940882d987c0aa ryanofsky: Code review ACK aaaa7bd0ba60aa7114810d4794940882d987c0aa. No changes since last review, other than rebase aureleoules: reACK aaaa7bd0ba60aa7114810d4794940882d987c0aa Tree-SHA512: 83ec3ba0fb4f1dad95810d4bd4e578454e0718dc1bdd3a794cc4e48aa819b6f5dad4ac4edab3719bdfd5f89cbe23c2740a50fd56c1ff81c99e521c5f6d4e898d
2022-10-24test: Use type-safe NodeSeconds for TestMemPoolEntryHelperMacroFake
2022-10-18test: Use C++11 member initializers for TestMemPoolEntryHelperMacroFake
Co-authored-by: Aurèle Oulès <aurele@oules.com>
2022-10-18Remove g_parallel_script_checksMacroFake
2022-10-18Move ::fCheckBlockIndex into ChainstateManagerMacroFake
This changes the flag for the bitcoin-chainstate executable. Previously it was false, now it is the chain's default value (still false for the main chain).
2022-10-18test: Remove unused txmempool include from testsMacroFake
2022-10-13Merge bitcoin/bitcoin#25667: assumeutxo: snapshot initializationAndrew Chow
bf9597606166323158bbf631137b82d41f39334f doc: add note about snapshot chainstate init (James O'Beirne) e4d799528696c5ede38c257afaffd367917e0de8 test: add testcases for snapshot initialization (James O'Beirne) cced4e7336d93a2dc88e4a61c49941887766bd72 test: move-only-ish: factor out LoadVerifyActivateChainstate() (James O'Beirne) 51fc9241c08a00f1f407f1534853a5cddbbc0a23 test: allow on-disk coins and block tree dbs in tests (James O'Beirne) 3c361391b8f5971eb3c7b620aa7ad9b437cc515e test: add reset_chainstate parameter for snapshot unittests (James O'Beirne) 00b357c215ed900145bd770525a341ba0ed9c027 validation: add ResetChainstates() (James O'Beirne) 3a29dfbfb2c16a50d854f6f81428a68aa9180509 move-only: test: make snapshot chainstate setup reusable (James O'Beirne) 8153bd9247dad3982d54488bcdb3960470315290 blockmanager: avoid undefined behavior during FlushBlockFile (James O'Beirne) ad67ff377c2b271cb4683da2fb25fd295557f731 validation: remove snapshot datadirs upon validation failure (James O'Beirne) 34d159033106cc595cfa852695610bfe419c989c add utilities for deleting on-disk leveldb data (James O'Beirne) 252abd1e8bc5cdf4368ad55e827a873240535b28 init: add utxo snapshot detection (James O'Beirne) f9f1735f139b6a1f1c7fea50717ff90dc4ba2bce validation: rename snapshot chainstate dir (James O'Beirne) d14bebf100aaaa25c7558eeed8b5c536da99885f db: add StoragePath to CDBWrapper/CCoinsViewDB (James O'Beirne) Pull request description: This is part of the [assumeutxo project](https://github.com/bitcoin/bitcoin/projects/11) (parent PR: https://github.com/bitcoin/bitcoin/pull/15606) --- Half of the replacement for #24232. The original PR grew larger than expected throughout the review process. This change adds the ability to initialize a snapshot-based chainstate during init if one is detected on disk. This is of course unused as of now (aside from in unittests) given that we haven't yet enabled actually loading snapshots. Don't be scared! There are some big move-only commits in here. Accompanying changes include: - moving the snapshot coinsdb directory from being called `chainstate_[base blockhash]` to `chainstate_snapshot`, since we only support one snapshot in use at a time. This simplifies some logic, but it necessitates writing that base blockhash out to a file within the coinsdb dir. See [discussion here](https://github.com/bitcoin/bitcoin/pull/24232#discussion_r832762880). - adding a simple fix in `FlushBlockFile()` that avoids a crash when attemping to flush to disk before `LoadBlockIndexDB()` is called, which happens when calling `MaybeRebalanceCaches()` during multiple chainstate init. - improving the unittest to allow testing with on-disk chainstates - necessary to test a simulated restart and re-initialization. ACKs for top commit: naumenkogs: utACK bf9597606166323158bbf631137b82d41f39334f ariard: Code Review ACK bf9597606 ryanofsky: Code review ACK bf9597606166323158bbf631137b82d41f39334f. Changes since last review: rebasing, switching from CAutoFile to AutoFile, adding comments, switching from BOOST_CHECK to Assert in test util, using chainman.GetMutex() in tests, destroying one ChainstateManager before creating a new one in tests fjahr: utACK bf9597606166323158bbf631137b82d41f39334f aureleoules: ACK bf9597606166323158bbf631137b82d41f39334f Tree-SHA512: 15ae75caf19f8d12a12d2647c52897904d27b265a7af6b4ae7b858592eeadb8f9da6c2394b6baebec90adc28742c053e3eb506119577dae7c1e722ebb3b7bcc0
2022-10-12Merge bitcoin/bitcoin#25421: net: convert standalone IsSelectableSocket() ↵glozow
and SetSocketNonBlocking() to Sock methods b527b549504672704a61f70d2565b9489aaaba91 net: convert standalone SetSocketNonBlocking() to Sock::SetNonBlocking() (Vasil Dimov) 29f66f76826056f53d971ac734b7ed49b39848d3 moveonly: move SetSocketNonBlocking() from netbase to util/sock (Vasil Dimov) b4bac556791b5bb8aa118d4c1fed42c3fe45550c net: convert standalone IsSelectableSocket() to Sock::IsSelectable() (Vasil Dimov) 5db7d2ca0aa51ff25f97bf21ce0cbc9e6b741cbd moveonly: move IsSelectableSocket() from compat.h to sock.{h,cpp} (Vasil Dimov) Pull request description: _This is a piece of #21878, chopped off to ease review._ * convert standalone `IsSelectableSocket()` to `Sock::IsSelectable()` * convert standalone `SetSocketNonBlocking()` to `Sock::SetNonBlocking()` This further encapsulates syscalls inside the `Sock` class and makes the callers mockable. ACKs for top commit: jonatack: ACK b527b549504672704a61f70d2565b9489aaaba91 review/debug build/unit tests at each commit, cross-referenced the changes with `man select` and `man errno`, ran a signet node on the last commit with ip4/ip6//tor/i2p/cjdns and network connections were nominal dergoegge: Code review ACK b527b549504672704a61f70d2565b9489aaaba91 Tree-SHA512: af783ce558c7a89e173f7ab323fb3517103d765c19b5d14de29f64706b4e1fea3653492e8ea73ae972699986aaddf2ae72c7cfaa7dad7614254283083b7d2632
2022-09-20Merge bitcoin/bitcoin#26036: net: add NetEventsInterface::g_msgproc_mutexfanquake
d575a675cc884b1bebdb6197f2ef45c51788d4a3 net_processing: add thread safety annotation for m_highest_fast_announce (Anthony Towns) 0ae7987f68211729d87c9255889f4d9d1aa6a37f net_processing: add thread safety annotations for PeerManagerImpl members accessed only via the msgproc thread (Anthony Towns) a66a7ccb822f0f1f554d27d04735b7fb585d3b71 net_processing: add thread safety annotations for Peer members accessed only via the msgproc thread (Anthony Towns) bf12abe4542f2cf658516ea7e7fbbff6864c2208 net: drop cs_sendProcessing (Anthony Towns) 1e78f566d575a047a6f0b762bc79601e0208d103 net: add NetEventsInterface::g_msgproc_mutex (Anthony Towns) Pull request description: There are many cases where we assume message processing is single-threaded in order for how we access node-related memory to be safe. Add an explicit mutex that we can use to document this, which allows the compiler to catch any cases where we try to access that memory from other threads and break that assumption. ACKs for top commit: MarcoFalke: review ACK d575a675cc884b1bebdb6197f2ef45c51788d4a3 📽 dergoegge: Code review ACK d575a675cc884b1bebdb6197f2ef45c51788d4a3 w0xlt: ACK https://github.com/bitcoin/bitcoin/pull/26036/commits/d575a675cc884b1bebdb6197f2ef45c51788d4a3 vasild: ACK d575a675cc884b1bebdb6197f2ef45c51788d4a3 modulo the missing runtime checks Tree-SHA512: b886d1aa4adf318ae64e32ccaf3d508dbb79d6eed3f1fa9d8b2ed96f3c72a3d38cd0f12e05826c9832a2a1302988adfd2b43ea9691aa844f37d8f5c37ff20e05
2022-09-15net: drop cs_sendProcessingAnthony Towns
SendMessages() is now protected g_msgproc_mutex; so this additional per-node mutex is redundant.
2022-09-15net: add NetEventsInterface::g_msgproc_mutexAnthony Towns
There are many cases where we assume message processing is single-threaded in order for how we access node-related memory to be safe. Add an explicit mutex that we can use to document this, which allows the compiler to catch any cases where we try to access that memory from other threads and break that assumption.
2022-09-13test: move-only-ish: factor out LoadVerifyActivateChainstate()James O'Beirne
in TestingSetup(). This is used in the following commit to test reinitializing chainstates after snapshot validation and cleanup. Best reviewed with `git diff --color-moved=dimmed-zebra`.
2022-09-13test: allow on-disk coins and block tree dbs in testsJames O'Beirne
Used when testing cleanup of on-disk chainstate data for snapshot testcases. Also necessary for simulating node restart in .cpp tests.
2022-09-13test: add reset_chainstate parameter for snapshot unittestsJames O'Beirne
This CreateAndActivateUTXOSnapshot parameter is necessary once we perform snapshot completion within ABC, since the existing UpdateTip test will fail because the IBD chain that has generated the snapshot will exceed the base of the snapshot. Being able to test snapshots being loaded into a mostly-uninitialized datadir allows for more realistic unittest scenarios.
2022-09-13Merge bitcoin/bitcoin#24513: CChainState -> Chainstateglozow
00eeb31c7660e2c28f189f77a6905dee946ef408 scripted-diff: rename CChainState -> Chainstate (James O'Beirne) Pull request description: Alright alright alright, I know: we hate refactors. We especially hate cosmetic refactors. Nobody knows better than I that changing broad swaths of code out from under our already-abused collaborators, only to send a cascade of rebase bankruptcies, is annoying at best and sadistic at worst. And for a rename! The indignation! But just for a second, imagine yourself. Programming `bitcoin/bitcoin`, on a sandy beach beneath a lapis lazuli sky. You go to type the name of what is probably the most commonly used data structure in the codebase, and you *only hit shift once*. What could you do in such a world? You could do anything. [The only limit is yourself.](https://zombo.com/) --- So maybe you like the idea of this patch but really don't want to deal with rebasing. You're in luck! Here're the commands that will bail you out of rebase bankruptcy: ```sh git rebase -i $(git merge-base HEAD master) \ -x 'sed -i "s/CChainState/Chainstate/g" $(git ls-files | grep -E ".*\.(py|cpp|h)$") && git commit --amend --no-edit' # <commit changed?> git add -u && git rebase --continue ``` --- ~~Anyway I'm not sure how serious I am about this, but I figured it was worth proposing.~~ I have decided I am very serious about this. Maybe we can have nice things every once in a while? ACKs for top commit: MarcoFalke: cr ACK 00eeb31c7660e2c28f189f77a6905dee946ef408 hebasto: ACK 00eeb31c7660e2c28f189f77a6905dee946ef408 glozow: ACK 00eeb31c7660e2c28f189f77a6905dee946ef408, thanks for being the one to propose this w0xlt: ACK https://github.com/bitcoin/bitcoin/pull/24513/commits/00eeb31c7660e2c28f189f77a6905dee946ef408 Tree-SHA512: b828a99780614a9b74f7a9c347ce0687de6f8d75232840f5ffc26e02bbb25a3b1f5f9deabbe44f82ada01459586ee8452a3ee2da05d1b3c48558c8df6f49e1b1
2022-09-09scripted-diff: rename CChainState -> ChainstateJames O'Beirne
-BEGIN VERIFY SCRIPT- sed -i 's/CChainState/Chainstate/g' $(git grep -l CChainState ':(exclude)doc/release-notes*') -END VERIFY SCRIPT- Co-authored-by: MacroFake <falke.marco@gmail.com>
2022-09-05test: remove Boost Test from libtest utilfanquake
Context is the discussion here: https://github.com/bitcoin/bitcoin/pull/25974/files#r961541457.
2022-09-02Merge bitcoin/bitcoin#25962: net: Add CNodeOptions and increase constnessMacroFake
377e9ccda469731d535829f184b70c73ed46b6ef scripted-diff: net: rename permissionFlags to permission_flags (Anthony Towns) 0a7fc428978c4db416fdcf9bf0b79de17d0558d7 net: make CNode::m_prefer_evict const (Anthony Towns) d394156b99d6b9a99aedee78658310d169ca188d net: make CNode::m_permissionFlags const (Anthony Towns) 9dccc3328eeaf9cd66518d812c878def5d014e36 net: add CNodeOptions for optional CNode constructor params (Anthony Towns) Pull request description: Adds CNodeOptions to make it easier to add optional parameters to the CNode constructor, and makes prefer_evict and m_permissionFlags actually const. ACKs for top commit: naumenkogs: ACK 377e9ccda469731d535829f184b70c73ed46b6ef jonatack: ACK 377e9ccda469731d535829f184b70c73ed46b6ef per `git range-diff 52dcb1d 2f3602b 377e9cc` vasild: ACK 377e9ccda469731d535829f184b70c73ed46b6ef ryanofsky: Code review ACK 377e9ccda469731d535829f184b70c73ed46b6ef. Looks good and feel free to ignore suggestions! Tree-SHA512: 06fd6748770bad75ec8c966fdb73b7534c10bd61838f6f1b36b3f3d6a438e58f6a7d0edb011977e5c118ed7ea85325fac35e10dde520fef249f7a780cf500a85
2022-09-01Merge bitcoin/bitcoin#25614: Severity-based logging, step 2Andrew Chow
958048057087e6562b474f9028316c00ec03c2e4 Update debug logging section in the developer notes (Jon Atack) 1abaa31aa3d833caf2290d6c90f57f7f79d146c0 Update -debug and -debugexclude help docs for severity level logging (Jon Atack) 45f92821621a60891044f57c7a7bc4ab4c7d8a01 Create BCLog::Level::Trace log severity level (Jon Atack) 2a8712db4fb5d06f1a525a79bb0f793cb733aaa6 Unit test coverage for -loglevel configuration option (klementtan) eb7bee5f84d41e35cb4296e01bea2aa5ac80a856 Create -loglevel configuration option (klementtan) 98a1f9c68744074f29fa5fa67514218b5ee9edc4 Unit test coverage for log severity levels (klementtan) 9c7507bf76e79da99766a69df939520ea0a125d1 Create BCLog::Logger::LogLevelsString() helper function (klementtan) 8fe3457dbb4146952b92fb9509bbe4e97dc1f05b Update LogAcceptCategory() and unit tests with log severity levels (klementtan) c2797cfc602c5cdd899a7c11b37bb5711cebff38 Add BCLog::Logger::SetLogLevel()/SetCategoryLogLevel() for string inputs (klementtan) f6c0cc03509255ffa4dfd6e2822fce840dd0b181 Add BCLog::Logger::m_category_log_levels data member and getter/setter (Jon Atack) 2978b387bffc226fb1eaca4d30f24a0deedb2a36 Add BCLog::Logger::m_log_level data member and getter/setter (Jon Atack) f1379aeca9d3a8c4d3528de4d0af8298cb42fee4 Simplify BCLog::Level enum class and LogLevelToStr() function (Jon Atack) Pull request description: This is an updated version of https://github.com/bitcoin/bitcoin/pull/25287 and the next steps in parent PR #25203 implementing, with Klement Tan, user-configurable, per-category severity log levels based on an idea by John Newbery and refined in GitHub discussions by Wladimir Van der Laan and Marco Falke. - simplify the `BCLog::Level` enum class and the `LogLevelToStr()` function and add documentation - update the logging logic to filter logs by log level both globally and per-category - add a hidden `-loglevel` help-debug config option to allow testing setting the global or per-category severity level on startup for logging categories enabled with the `-debug` configuration option or the logging RPC (Klement Tan) - add a `trace` log severity level selectable by the user; the plan is for the current debug messages to become trace, LogPrint ones to become debug, and LogPrintf ones to become info, warning, or error ``` $ ./src/bitcoind -help-debug | grep -A10 loglevel -loglevel=<level>|<category>:<level> Set the global or per-category severity level for logging categories enabled with the -debug configuration option or the logging RPC: info, debug, trace (default=info); warning and error levels are always logged. If <category>:<level> is supplied, the setting will override the global one and may be specified multiple times to set multiple category-specific levels. <category> can be: addrman, bench, blockstorage, cmpctblock, coindb, estimatefee, http, i2p, ipc, leveldb, libevent, lock, mempool, mempoolrej, net, proxy, prune, qt, rand, reindex, rpc, selectcoins, tor, util, validation, walletdb, zmq. ``` See the individual commit messages for details. ACKs for top commit: jonatack: One final push per `git range-diff a5d5569 ce3c4c9 9580480` (should be trivial to re-ACK) to ensure this pull changes no default behavior in any way for users or the tests/CI in order to be completely v24 compatible, to update the unit test setup in general, and to update the debug logging section in the developer notes. klementtan: reACK https://github.com/bitcoin/bitcoin/commit/958048057087e6562b474f9028316c00ec03c2e4 1440000bytes: reACK https://github.com/bitcoin/bitcoin/pull/25614/commits/958048057087e6562b474f9028316c00ec03c2e4 vasild: ACK 958048057087e6562b474f9028316c00ec03c2e4 dunxen: reACK 9580480 brunoerg: reACK 958048057087e6562b474f9028316c00ec03c2e4 Tree-SHA512: 476a638e0581f40b5d058a9992691722e8b546471ec85e07cbc990798d1197fbffbd02e1b3d081b4978404e07a428378cdc8e159c0004b81f58be7fb01b7cba0
2022-09-01net: make CNode::m_permissionFlags constAnthony Towns
2022-08-29Require callers of AcceptBlockHeader() to perform anti-dos checksSuhas Daftuar
In order to prevent memory DoS, we must ensure that we don't accept a new header into memory until we've performed anti-DoS checks, such as verifying that the header is part of a sufficiently high work chain. This commit adds a new argument to AcceptBlockHeader() so that we can ensure that all call-sites which might cause a new header to be accepted into memory have to grapple with the question of whether the header is safe to accept, or needs further validation. This patch also fixes two places where low-difficulty-headers could have been processed without such validation (processing an unrequested block from the network, and processing a compact block). Credit to Niklas Gögge for noticing this issue, and thanks to Sjors Provoost for test code.
2022-08-20Create BCLog::Level::Trace log severity levelJon Atack
for verbose log messages for development or debugging only, as bitcoind may run more slowly, that are more granular/frequent than the Debug log level, i.e. for very high-frequency, low-level messages to be logged distinctly from higher-level, less-frequent debug logging that could still be usable in production. An example would be to log higher-level peer events (connection, disconnection, misbehavior, eviction) as Debug, versus Trace for low-level, high-volume p2p messages in the BCLog::NET category. This will enable the user to log only the former without the latter, in order to focus on high-level peer management events. With respect to the name, "trace" is suggested as the most granular level in resources like the following: - https://sematext.com/blog/logging-levels - https://howtodoinjava.com/log4j2/logging-levels Update the test framework and add test coverage.
2022-08-20Create -loglevel configuration optionklementtan
- add a -loglevel=<level>|<category:level> config option to allow users to set a global -loglevel and category-specific log levels. LogPrintLevel messages with a higher severity level than -loglevel will not be printed in the debug log. - for now, this config option is debug-only during the migration to severity-based logging - update unit and functional tests Co-authored-by: "Jon Atack <jon@atack.com>"
2022-08-05Merge bitcoin/bitcoin#25721: refactor: Replace BResult with util::ResultMacroFake
a23cca56c0a7f4a267915b4beba3af3454c51603 refactor: Replace BResult with util::Result (Ryan Ofsky) Pull request description: Rename `BResult` class to `util::Result` and update the class interface to be more compatible with `std::optional` and with a full-featured result class implemented in https://github.com/bitcoin/bitcoin/pull/25665. Motivation for this change is to update existing `BResult` usages now so they don't have to change later when more features are added in https://github.com/bitcoin/bitcoin/pull/25665. This change makes the following improvements originally implemented in https://github.com/bitcoin/bitcoin/pull/25665: - More explicit API. Drops potentially misleading `BResult` constructor that treats any bilingual string argument as an error. Adds `util::Error` constructor so it is never ambiguous when a result is being assigned an error or non-error value. - Better type compatibility. Supports `util::Result<bilingual_str>` return values to hold translated messages which are not errors. - More standard and consistent API. `util::Result` supports most of the same operators and methods as `std::optional`. `BResult` had a less familiar interface with `HasRes`/`GetObj`/`ReleaseObj` methods. The Result/Res/Obj naming was also not internally consistent. - Better code organization. Puts `src/util/` code in the `util::` namespace so naming reflects code organization and it is obvious where the class is coming from. Drops "B" from name because it is undocumented what it stands for (bilingual?) - Has unit tests. ACKs for top commit: MarcoFalke: ACK a23cca56c0a7f4a267915b4beba3af3454c51603 🏵 jonatack: ACK a23cca56c0a7f4a267915b4beba3af3454c51603 Tree-SHA512: 2769791e08cd62f21d850aa13fa7afce4fb6875a9cedc39ad5025150dbc611c2ecfd7b3aba8b980a79fde7fbda13babdfa37340633c69b501b6e89727bad5b31
2022-08-03validationcaches: Add and use ValidationCacheSizesCarl Dong
Also: - Make DEFAULT_MAX_SIG_CACHE_SIZE into constexpr DEFAULT_MAX_SIG_CACHE_BYTES to utilize the compile-time integer arithmetic overflow checking available to constexpr. - Fix comment (MiB instead of MB) for DEFAULT_MAX_SIG_CACHE_BYTES. - Pass in max_size_bytes parameter to InitS*Cache(), modify log line to no longer allude to maxsigcachesize being split evenly between the two validation caches. - Fix possible integer truncation and add a comment. [META] I've kept the integer types as int64_t in order to not introduce unintended behaviour changes, in the next commit we will make them size_t.
2022-08-03cuckoocache: Return approximate memory sizeCarl Dong
Returning the approximate total size eliminates the need for InitS*Cache() to do nElems*sizeof(uint256). The cuckoocache has a better idea of this information.
2022-08-03refactor: Replace BResult with util::ResultRyan Ofsky
Rename `BResult` class to `util::Result` and update the class interface to be more compatible with `std::optional` and with a full-featured result class implemented in https://github.com/bitcoin/bitcoin/pull/25665. Motivation for this change is to update existing `BResult` usages now so they don't have to change later when more features are added in #25665. This change makes the following improvements originally implemented in #25665: - More explicit API. Drops potentially misleading `BResult` constructor that treats any bilingual string argument as an error. Adds `util::Error` constructor so it is never ambiguous when a result is being assigned an error or non-error value. - Better type compatibility. Supports `util::Result<bilingual_str>` return values to hold translated messages which are not errors. - More standard and consistent API. `util::Result` supports most of the same operators and methods as `std::optional`. `BResult` had a less familiar interface with `HasRes`/`GetObj`/`ReleaseObj` methods. The Result/Res/Obj naming was also not internally consistent. - Better code organization. Puts `src/util/` code in the `util::` namespace so naming reflects code organization and it is obvious where the class is coming from. Drops "B" from name because it is undocumented what it stands for (bilingual?) - Has unit tests.
2022-08-02sort after scripted-diffMacroFake
generated by cgit v1.2.3 (git 2.26.2) at 2025-03-18 08:27:45 +0000