aboutsummaryrefslogtreecommitdiff
path: root/src/util
AgeCommit message (Collapse)Author
2022-09-14scripted-diff: rename pszThread to thread_namestickies-v
Since it is now a string_view instead of a const char*, update the name to reflect that the variable is no longer a "Pointer to String, Zero-terminated" (psz). -BEGIN VERIFY SCRIPT- sed -i s/pszThread/thread_name/ $(git grep -l pszThread src) -END VERIFY SCRIPT-
2022-09-13refactor: use std::string for thread namesstickies-v
2022-08-31Merge bitcoin/bitcoin#25872: Fix issues when calling std::move(const&)fanquake
fa875349e22f2f0f9c2c98ee991372d08ff90318 Fix iwyu (MacroFake) faad673716cfbad1e715f1bdf8ac00938a055aea Fix issues when calling std::move(const&) (MacroFake) Pull request description: Passing a symbol to `std::move` that is marked `const` is a no-op, which can be fixed in two ways: * Remove the `const`, or * Remove the `std::move` ACKs for top commit: ryanofsky: Code review ACK fa875349e22f2f0f9c2c98ee991372d08ff90318. Looks good. Good for univalue to support c++11 move optimizations Tree-SHA512: 3dc5cad55b93cfa311abedfb811f35fc1b7f30a1c68561f15942438916c7de25e179c364be11881e01f844f9c2ccd71a3be55967ad5abd2f35b10bb7a882edea
2022-08-30Merge bitcoin/bitcoin#25717: p2p: Implement anti-DoS headers syncfanquake
3add23454624c4c79c9eebc060b6fbed4e3131a7 ui: show header pre-synchronization progress (Pieter Wuille) 738421c50f2dbd7395b50a5dbdf6168b07435e62 Emit NotifyHeaderTip signals for pre-synchronization progress (Pieter Wuille) 376086fc5a187f5b2ab3a0d1202ed4e6c22bdb50 Make validation interface capable of signalling header presync (Pieter Wuille) 93eae27031a65b4156df49015ae45b2b541b4e5a Test large reorgs with headerssync logic (Suhas Daftuar) 355547334f7d08640ee1fa291227356d61145d1a Track headers presync progress and log it (Pieter Wuille) 03712dddfbb9fe0dc7a2ead53c65106189f5c803 Expose HeadersSyncState::m_current_height in getpeerinfo() (Suhas Daftuar) 150a5486db50ff77c91765392149000029c8a309 Test headers sync using minchainwork threshold (Suhas Daftuar) 0b6aa826b53470c9cc8ef4a153fa710dce80882f Add unit test for HeadersSyncState (Suhas Daftuar) 83c6a0c5249c4ecbd11f7828c84a50fb473faba3 Reduce spurious messages during headers sync (Suhas Daftuar) ed6cddd98e32263fc116a4380af6d66da20da990 Require callers of AcceptBlockHeader() to perform anti-dos checks (Suhas Daftuar) 551a8d957c4c44afbd0d608fcdf7c6a4352babce Utilize anti-DoS headers download strategy (Suhas Daftuar) ed470940cddbeb40425960d51cefeec4948febe4 Add functions to construct locators without CChain (Pieter Wuille) 84852bb6bb3579e475ce78fe729fd125ddbc715f Add bitdeque, an std::deque<bool> analogue that does bit packing. (Pieter Wuille) 1d4cfa4272cf2c8b980cc8762c1ff2220d3e8d51 Add function to validate difficulty changes (Suhas Daftuar) Pull request description: New nodes starting up for the first time lack protection against DoS from low-difficulty headers. While checkpoints serve as our protection against headers that fork from the main chain below the known checkpointed values, this protection only applies to nodes that have been able to download the honest chain to the checkpointed heights. We can protect all nodes from DoS from low-difficulty headers by adopting a different strategy: before we commit to storing a header in permanent storage, first verify that the header is part of a chain that has sufficiently high work (either `nMinimumChainWork`, or something comparable to our tip). This means that we will download headers from a given peer twice: once to verify the work on the chain, and a second time when permanently storing the headers. The p2p protocol doesn't provide an easy way for us to ensure that we receive the same headers during the second download of peer's headers chain. To ensure that a peer doesn't (say) give us the main chain in phase 1 to trick us into permanently storing an alternate, low-work chain in phase 2, we store commitments to the headers during our first download, which we validate in the second download. Some parameters must be chosen for commitment size/frequency in phase 1, and validation of commitments in phase 2. In this PR, those parameters are chosen to both (a) minimize the per-peer memory usage that an attacker could utilize, and (b) bound the expected amount of permanent memory that an attacker could get us to use to be well-below the memory growth that we'd get from the honest chain (where we expect 1 new block header every 10 minutes). After this PR, we should be able to remove checkpoints from our code, which is a nice philosophical change for us to make as well, as there has been confusion over the years about the role checkpoints play in Bitcoin's consensus algorithm. Thanks to Pieter Wuille for collaborating on this design. ACKs for top commit: Sjors: re-tACK 3add23454624c4c79c9eebc060b6fbed4e3131a7 mzumsande: re-ACK 3add23454624c4c79c9eebc060b6fbed4e3131a7 sipa: re-ACK 3add23454624c4c79c9eebc060b6fbed4e3131a7 glozow: ACK 3add234546 Tree-SHA512: e7789d65f62f72141b8899eb4a2fb3d0621278394d2d7adaa004675250118f89a4e4cb42777fe56649d744ec445ad95141e10f6def65f0a58b7b35b2e654a875
2022-08-24Make Join() util work with any container typeMacroFake
Also, remove helper that is only used in tests.
2022-08-23Add bitdeque, an std::deque<bool> analogue that does bit packing.Pieter Wuille
2022-08-20Fix iwyuMacroFake
2022-08-20Fix issues when calling std::move(const&)MacroFake
2022-08-19Remove Join() helper only used in testsMacroFake
Also remove redundant return type that can be deduced by the compiler.
2022-08-08refactor: Drop `boost/algorithm/string/replace.hpp` dependencyHennadii Stepanov
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-04Merge bitcoin/bitcoin#24675: util: Use ArgsManager::GetPathArg more widelyfanquake
b01f336708019f8c8274ea701d3446e4123e7af2 util, refactor: Drop explicit conversion to fs::path (Hennadii Stepanov) 138c668e2b4d64279ddefbe07c1d9b7c3d3c537c util, refactor: Use GetPathArg to read "-rpccookiefile" value (Hennadii Stepanov) 1276090705060fcc97072481c2383bbaaa556194 util, refactor: Use GetPathArg to read "-conf" value (Hennadii Stepanov) Pull request description: This PR is a continuation of bitcoin/bitcoin#24265 and bitcoin/bitcoin#24306. Now the following command-line arguments / configure options been read with the `GetPathArg` method: - `-conf`, also `includeconf` values been normalized - `-rpccookiefile` ACKs for top commit: jarolrod: Code Review ACK b01f336708019f8c8274ea701d3446e4123e7af2 ryanofsky: Code review ACK b01f336708019f8c8274ea701d3446e4123e7af. Changes since last review: just dropping first commit (NormalizedPathFromString) as suggested Tree-SHA512: 2d26d50b73542acdbcc63a32068977b2a49a017d31ca337471a0446f964eb0a6e3e4e3bb1ebe6771566a260f2cae3bc2ebe93b4b523183cea0d51768daab85c9
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-07-27Merge bitcoin/bitcoin#24697: refactor address relay timefanquake
fa64dd6673767992eb4e0e775fb0afdfd298610d refactor: Use type-safe std::chrono for addrman time (MarcoFalke) fa2ae373f33fa76dc4e435e7cb4778055aa6afd5 Add type-safe AdjustedTime() getter to timedata (MarcoFalke) fa5103a9f5f8559ab005c0b012d3d3a8057d81fb Add ChronoFormatter to serialize (MarcoFalke) fa253d385f9201ea10beacecf3e0e80ff69f3138 util: Add HoursDouble (MarcoFalke) fa21fc60c292ab947b2200e54201440f16230566 scripted-diff: Rename addrman time symbols (MarcoFalke) fa9284c3e9acec4b44b2560256f27b3d78c753e2 refactor: Remove not needed std::max (MacroFake) Pull request description: Those refactors are overlapping with, but otherwise largely unrelated to #24662. ACKs for top commit: naumenkogs: utACK fa64dd6673767992eb4e0e775fb0afdfd298610d dergoegge: Code review ACK fa64dd6673767992eb4e0e775fb0afdfd298610d Tree-SHA512: a50625e78036e7220a11997e6d9b6c6b317cb38ce02b1835fb41cbee2d8bfb1faf29b29d8990be78d6b5e15e9a9d8dec33bf25fa439b47610ef708950969724b
2022-07-26refactor: remove unnecessary string initializationsfanquake
2022-07-26util: Add HoursDoubleMarcoFalke
2022-07-22Merge bitcoin/bitcoin#25331: Add HashWriter without ser-type and ser-version ↵fanquake
and use it where possible faf9accd662974a69390213fee1b5c6237846b42 Use HashWriter where possible (MacroFake) faa5425629d35708326b255570c51139aef0c8c4 Add HashWriter without ser-type and ser-version (MacroFake) Pull request description: This was done in the context of https://github.com/bitcoin/bitcoin/pull/25284 , but I think it also makes sense standalone. The basic idea is that serialization type should not be initialized when it is not needed. Same for the serialization version. So do this here for `HashWriter`. `CHashWriter` remains in places where it is not yet possible. ACKs for top commit: sipa: utACK faf9accd662974a69390213fee1b5c6237846b42 Empact: utACK https://github.com/bitcoin/bitcoin/pull/25331/commits/faf9accd662974a69390213fee1b5c6237846b42 Tree-SHA512: 544cc712436e49f6e608120bcd3ddc5ea72dd236554ce30fb6cfff34a92d7e67b6e6527336ad0f5b6365e2b2884f4c6508aef775953ccd9312f17752729703f2
2022-07-20Use HashWriter where possibleMacroFake
2022-07-20refactor: move compat.h into compat/fanquake
2022-07-20Merge bitcoin/bitcoin#25285: Add AutoFile without ser-type and ser-version ↵fanquake
and use it where possible facc2fa7b8a218a0df6a19772e1641ea68dda2e3 Use AutoFile where possible (MacroFake) 6666803c897e4ad27b45cb74e3a9aa74a335f1bf streams: Add AutoFile without ser-type and ser-version (MacroFake) Pull request description: This was done in the context of https://github.com/bitcoin/bitcoin/pull/25284 , but I think it also makes sense standalone. The basic idea is that serialization type should not be initialized when it is not needed. Same for the serialization version. So do this here for `AutoFile`. `CAutoFile` remains in places where it is not yet possible. ACKs for top commit: laanwj: Code review ACK facc2fa7b8a218a0df6a19772e1641ea68dda2e3 fanquake: ACK facc2fa7b8a218a0df6a19772e1641ea68dda2e3 Tree-SHA512: d82d024d55af57565ac53d9d1517afafc12b46964effba0332de62a6c77869356fa77f89e6d4834438fff44c45b64fccdf5a1358bfea03e28dfe55013b3c099d
2022-07-18Merge bitcoin/bitcoin#25487: [kernel 3b/n] Decouple `{Dump,Load}Mempool` ↵glozow
from `ArgsManager` cb3e9a1e3f8d72daaa361fc45dd853775e754b9d Move {Load,Dump}Mempool to kernel namespace (Carl Dong) aa306765419f7dbea12b12e15553039835ba0e4d Move DEFAULT_PERSIST_MEMPOOL out of libbitcoinkernel (Carl Dong) 06b88ffb8ae7f2b2a93a32908cd80e77fafd270c LoadMempool: Pass in load_path, stop using gArgs (Carl Dong) b857ac60d9a0433036519c26675378bbf56a1de1 test/fuzz: Invoke LoadMempool via CChainState (Carl Dong) b3267258b052557fc136b9a4dcb754afb9219470 Move FopenFn to fsbridge namespace (Carl Dong) ae1e8e37567fa603a5977d7d05105c682dd3f7db mempool: Use NodeClock+friends for LoadMempool (Carl Dong) f9e8e5719f28d84f68f7d75e26c8e7fccac8e7d3 mempool: Improve comments for [GS]etLoadTried (Carl Dong) 813962da0b17b918941c6849996845e35d84a451 scripted-diff: Rename m_is_loaded -> m_load_tried (Carl Dong) 413f4bb52b72e082ad8716664ede48352b8e7e5a DumpMempool: Pass in dump_path, stop using gArgs (Carl Dong) bd4407817e523e3c5b347bc6be25ed007cb27034 DumpMempool: Use std::chrono instead of weird int64_t arthmetics (Carl Dong) c84390b741ab7b61c9f702d8b447c8cadc1257c8 test/mempool_persist: Test manual savemempool when -persistmempool=0 (Carl Dong) Pull request description: This is part of the `libbitcoinkernel` project: #24303, https://github.com/bitcoin/bitcoin/projects/18 ----- This PR moves `{Dump,Load}Mempool` into its own `kernel/mempool_persist` module and introduces `ArgsManager` `node::` helpers in `node/mempool_persist_args`to remove the scattered calls to `GetBoolArg("-persistmempool", DEFAULT_PERSIST_MEMPOOL)`. More context can be gleaned from the commit messages. ----- One thing I was reflecting on as I wrote this was that in the long run, I think we should probably invert the validation <-> mempool relationship. Instead of mempool not depending on validation, it might make more sense to have validation not depend on mempool. Not super urgent since `libbitcoinkernel` will include both validation and mempool, but perhaps something for the future. ACKs for top commit: glozow: re ACK cb3e9a1e3f8d72daaa361fc45dd853775e754b9d via `git range-diff 7ae032e...cb3e9a1` MarcoFalke: ACK cb3e9a1e3f 🔒 ryanofsky: Code review ACK cb3e9a1e3f8d72daaa361fc45dd853775e754b9d Tree-SHA512: 979d7237c3abb5a1dd9b5ad3dbf3b954f906a6d8320ed7b923557f41a4472deccae3e8a6bca0018c8e7a3c4a93afecc502acd1e26756f2054f157f1c0edd939d
2022-07-15DumpMempool: Use std::chrono instead of weird int64_t arthmeticsCarl Dong
This makes it so that DumpMempool doesn't depend on MICRO anymore
2022-07-14Merge bitcoin/bitcoin#25594: refactor: Return BResult from restoreWalletMacroFake
fa475e9c7977a952617738f2ee8cf600c07d4df8 refactor: Return BResult from restoreWallet (MacroFake) fa8de09edc9ec4e6d171df80f746174a0ec58afb Prepare BResult for non-copyable types (MacroFake) Pull request description: This avoids the `error` in-out param (and if `warnings` is added to `BResult`, it will avoid passing that in-out param as well). Also, as it is needed for this change, prepare `BResult` for non-copyable types. ACKs for top commit: w0xlt: reACK https://github.com/bitcoin/bitcoin/pull/25594/commits/fa475e9c7977a952617738f2ee8cf600c07d4df8 ryanofsky: Code review ACK fa475e9c7977a952617738f2ee8cf600c07d4df8. Changes since last review were replacing auto with explicit type and splitting commits Tree-SHA512: 46350883572f13721ddd198f5dfb88d2fa58ebcbda416f74da3563ea15c920fb1e6ff30558526a4ac91c36c21e6afe27751a4e51b7b8bcbcbe805209f4e9014b
2022-07-13Merge bitcoin/bitcoin#25472: build: Increase MS Visual Studio minimum versionfanquake
630c1711b47ce50805f4dd2883777a100f7e5339 refactor: Drop no longer needed `util/designator.h` (Hennadii Stepanov) 88ec5d40dcf5d9f95217b123b48203b2f334c0a1 build: Increase MS Visual Studio minimum version (Hennadii Stepanov) 555f9dd5d39b316bf404017401b5aedc23ec6226 rpc, refactor: Add `decodepsbt_outputs` (Hennadii Stepanov) 0c432cbbfa9e83a68e061b388745326e278369fb rpc, refactor: Add `decodepsbt_inputs` (Hennadii Stepanov) 01d95a3964267d243df00d9bcc93b28adcfe16d7 rpc, refactor: Add `getblock_prevout` (Hennadii Stepanov) Pull request description: Visual Studio 2022 with `/std:c++20` supports [designated initializers](https://github.com/bitcoin/bitcoin/pull/24531). ACKs for top commit: sipsorcery: reACK 630c1711b47ce50805f4dd2883777a100f7e5339. Tree-SHA512: 5b8933940dd69061c6b077512168bebb6fea05d429b63ffbab191950798b4c825e8484b1a651db0ae13f97eae481097d3c16395659c0f3b9f847af2aaf44b65d
2022-07-12Prepare BResult for non-copyable typesMacroFake
2022-07-12Merge bitcoin/bitcoin#25324: refactor: add most of src/util to iwyuMacroFake
07f2c25d04c39a0074e1d9ee1b24b3e359c8153f refactor: add most of src/util to iwyu (fanquake) Pull request description: These files change infrequently, and not much header shuffling is required. We don't add everything in src/util/ yet, because IWYU makes some dubious suggestions, which I'm going to follow up with upstream. Soon we'll swap `src/util/xyz.cpp` for just `src/util/`. ACKs for top commit: hebasto: ACK 07f2c25d04c39a0074e1d9ee1b24b3e359c8153f, I have reviewed the code and it looks OK, I agree it can be merged. Tree-SHA512: 07d76435c2bff1a62c4967eb0efaafe619cc3bbaf4166741d8520927b24336c01aee59822f8082ee2a01e15046a0f5d506b4b23a6e40ceb750f3226ed8167847
2022-07-08Introduce generic 'Result' classfurszy
Useful to encapsulate the function result object (in case of having it) or, in case of failure, the failure reason. This let us clean lot of boilerplate code, as now instead of returning a boolean and having to add a ref arg for the return object and another ref for the error string. We can simply return a 'BResult<Obj>'. Example of what we currently have: ``` bool doSomething(arg1, arg2, arg3, arg4, &result, &error_string) { do something... if (error) { error_string = "something bad happened"; return false; } result = goodResult; return true; } ``` Example of what we will get with this commit: ``` BResult<Obj> doSomething(arg1, arg2, arg3, arg4) { do something... if (error) return {"something happened"}; // good return {goodResult}; } ``` This allows a similar boilerplate cleanup on the function callers side as well. They don't have to add the extra pre-function-call error string and result object declarations to pass the references to the function.
2022-07-08refactor: add most of src/util to iwyufanquake
These files change infrequently, and not much header shuffling is required. We don't add everything in src/util/ yet, because IWYU makes some dubious suggestions, which I'm going to follow up with upstream.
2022-07-07refactor: Drop no longer needed `util/designator.h`Hennadii Stepanov
2022-06-30Merge bitcoin/bitcoin#24836: add RPC (-regtest only) for testing package policyfanquake
e866f0d0666664885d4c15c79bf59cc59975887a [functional test] submitrawpackage RPC (glozow) fa076515b07ac4b10b2134e323bf4f56be5996a8 [rpc] add new submitpackage RPC (glozow) Pull request description: It would be nice for LN/wallet/app devs to test out package policy, package RBF, etc., but the only interface to do so right now is through unit tests. This PR adds a `-regtest` only RPC interface so people can test by submitting raw transaction data. It is regtest-only, as it would be unsafe/confusing to create an actual mainnet interface while package relay doesn't exist. Note that the functional tests are there to ensure the RPC interface is working properly; they aren't for testing policy itself. See src/test/txpackage_tests.cpp. ACKs for top commit: t-bast: Tested ACK against eclair https://github.com/bitcoin/bitcoin/pull/24836/commits/e866f0d0666664885d4c15c79bf59cc59975887a ariard: Code Review ACK e866f0d0 instagibbs: code review ACK e866f0d0666664885d4c15c79bf59cc59975887a Tree-SHA512: 824a26b10d2240e0fd85e5dd25bf499ee3dd9ba8ef4f522533998fcf767ddded9f001f7a005fe3ab07ec95e696448484e26599803e6034ed2733125c8c376c84
2022-06-29util: remove MSVC warning pragmasfanquake
4786 - I don't think this exists any more? 4805 - Is already defined (globally) in the MSVC project. Dropped 4717 and 4804, as it seems they are no-longer supressing anything. See: https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warnings-c4000-c5999
2022-06-29Use AutoFile where possibleMacroFake
2022-06-29Merge bitcoin/bitcoin#25290: [kernel 3a/n] Decouple `CTxMemPool` from ↵MacroFake
`ArgsManager` d1684beabe5b738c2cc83de83e1aaef11a761b69 fees: Pass in a filepath instead of referencing gArgs (Carl Dong) 9a3d825c30e8e6118d74a4e568744cb9d03f7f5d init: Remove redundant -*mempool*, -limit* queries (Carl Dong) 6c5c60c4124293d948735756f84efc85262ea66f mempool: Use m_limit for UpdateTransactionsFromBlock (Carl Dong) 9e93b1030182eff92ef91181e17c7dd498c7e164 node/ifaces: Use existing MemPoolLimits (Carl Dong) 38af2bcf358a72b9457d370282e57f4be1c5c849 mempoolaccept: Use limits from mempool in constructor (Carl Dong) 9333427014695ac235c96d48791098168dfdc9db mempool: Introduce (still-unused) MemPoolLimits (Carl Dong) 716bb5fbd31077bbe99d11a54d6c2c250afc8085 scripted-diff: Rename anc/desc size limit vars to indicate SI unit (Carl Dong) 1ecc77321deb61b9f6888e4e10752b9d972fd26e scripted-diff: Rename DEFAULT_MEMPOOL_EXPIRY to indicate time unit (Carl Dong) aa9141cd8185cb7ad532bc16feb9d302b05d9697 mempool: Pass in -mempoolexpiry instead of referencing gArgs (Carl Dong) 51c7a41a5eb6fcb60333812c770d80227cf7b64d init: Only determine maxmempool once (Carl Dong) 386c9472c8764738282e6d163b42e15a8feda7ea mempool: Make GetMinFee() with custom size protected (Carl Dong) 82f00de7a6a60cbc9ad0c6e1d0ffb1bc70c49af5 mempool: Pass in -maxmempool instead of referencing gArgs (Carl Dong) f1941e8bfd2eecc478c7660434b1ebf6a64095a0 pool: Add and use MemPoolOptions, ApplyArgsManOptions (Carl Dong) 0199bd35bb44e32ee0db9b51c9d1bd7518c26f19 fuzz/rbf: Add missing TestingSetup (Carl Dong) ccbaf546a68d6cda8ed3efd0598c0e4121b366bb scripted-diff: Rename DEFAULT_MAX_MEMPOOL_SIZE to indicate SI unit (Carl Dong) fc02f77ca604f0221171bfde3059b34f5d0fb1cd ArgsMan: Add Get*Arg functions returning optional (Carl Dong) Pull request description: This is part of the `libbitcoinkernel` project: #24303, https://github.com/bitcoin/bitcoin/projects/18 ----- As mentioned in the Stage 1 Step 2 description of [the `libbitcoinkernel` project](https://github.com/bitcoin/bitcoin/issues/24303), `ArgsManager` will not be part of `libbitcoinkernel`. Therefore, it is important that we remove any dependence on `ArgsManager` by code that will be part of `libbitcoinkernel`. This is the first in a series of PRs aiming to achieve this. This PR removes `CTxMemPool+MempoolAccept`'s dependency on `ArgsManager` by introducing a `CTxMemPool::Options` struct, which is used to specify `CTxMemPool`'s various options at construction time. These options are: - `-maxmempool` -> `CTxMemPool::Options::max_size` - `-mempoolexpiry` -> `CTxMemPool::Options::expiry` - `-limitancestorcount` -> `CTxMemPool::Options::limits::ancestor_count` - `-limitancestorsize` -> `CTxMemPool::Options::limits::ancestor_size` - `-limitdescendantcount` -> `CTxMemPool::Options::limits::descendant_count` - `-limitdescendantsize` -> `CTxMemPool::Options::limits::descendant_size` More context can be gleaned from the commit messages. The important commits are: - 56eb479ded8bfb2ef635bb6f3b484f9d5952c70d "pool: Add and use MemPoolOptions, ApplyArgsManOptions" - a1e08b70f3068f4e8def1c630d8f50cd54da7832 "mempool: Pass in -maxmempool instead of referencing gArgs" - 6f4bf3ede5812b374828f08fc728ceded2f10024 "mempool: Pass in -mempoolexpiry instead of referencing gArgs" - 5958a7fe4806599fc620ee8c1a881ca10fa2dd16 "mempool: Introduce (still-unused) MemPoolLimits" Reviewers: Help needed in the following commits (see commit messages): - a1e08b70f3068f4e8def1c630d8f50cd54da7832 "mempool: Pass in -maxmempool instead of referencing gArgs" - 0695081a797e9a5d7787b78b0f8289dafcc6bff7 "node/ifaces: Use existing MemPoolLimits" Note to Reviewers: There are perhaps an infinite number of ways to architect `CTxMemPool::Options`, the current one tries to keep it simple, usable, and flexible. I hope we don't spend too much time arguing over the design here since that's not the point. In the case that you're 100% certain that a different design is strictly better than this one in every regard, please show us a fully-implemented branch. ----- TODO: - [x] Use the more ergonomic `CTxMemPool::Options` where appropriate - [x] Doxygen comments for `ApplyArgsManOptions`, `MemPoolOptions` ----- Questions for Reviewers: 1. Should we use `std::chrono::seconds` for `CTxMemPool::Options::expiry` and `CTxMemPool::m_expiry` instead of an `int64_t`? Something else? (`std::chrono::hours`?) 2. Should I merge `CTxMemPool::Limits` inside `CTxMemPool::Options`? ACKs for top commit: MarcoFalke: ACK d1684beabe5b738c2cc83de83e1aaef11a761b69 🍜 ryanofsky: Code review ACK d1684beabe5b738c2cc83de83e1aaef11a761b69. Just minor cleanups since last review, mostly switching to brace initialization Tree-SHA512: 2c138e52d69f61c263f1c3648f01c801338a8f576762c815f478ef5148b8b2f51e91ded5c1be915e678c0b14f6cfba894b82afec58d999d39a7bb7c914736e0b
2022-06-28Merge bitcoin/bitcoin#25480: Replace CountSecondsDouble with ↵fanquake
Ticks<SecondsDouble> fa956e7508986991008e2f6126ab307924b3f353 Replace CountSecondsDouble with Ticks<SecondsDouble> (MacroFake) Pull request description: Seems odd to have two ways to say exactly the same thing when one is sufficient. ACKs for top commit: fanquake: ACK fa956e7508986991008e2f6126ab307924b3f353 shaavan: ACK fa956e7508986991008e2f6126ab307924b3f353 w0xlt: ACK https://github.com/bitcoin/bitcoin/pull/25480/commits/fa956e7508986991008e2f6126ab307924b3f353 Tree-SHA512: b599470e19b693da1ed1102d1e86b08cb03adaddf2048752b6d050fdf86055be117ff0ae10b6953d03e00eaaf7b0cfa350137968b67d6c5b3ca68c5aa50ca6aa
2022-06-28Merge bitcoin/bitcoin#24378: refactor: make bind() and listen() ↵laanwj
mockable/testable b2733ab6a85b234a88b83bdc77a0d043e18385b3 net: add new method Sock::Listen() that wraps listen() (Vasil Dimov) 3ad7de225efce3e76530f56bee8a8f7a75ea0f3c net: add new method Sock::Bind() that wraps bind() (Vasil Dimov) Pull request description: _This is a piece of #21878, chopped off to ease review._ Add new methods `Sock::Bind()` and `Sock::Listen()` that wrap `bind()` and `listen()`. This will help to increase `Sock` usage and make more code mockable. ACKs for top commit: pk-b2: ACK b2733ab6a85b234a88b83bdc77a0d043e18385b3 laanwj: Code review ACK b2733ab6a85b234a88b83bdc77a0d043e18385b3 Tree-SHA512: c6e737606703e2106fe60cc000cfbbae3a7f43deadb25f70531e2cac0457e0b0581440279d14c76c492eb85c12af4adde52c30baf74542c41597e419817488e8
2022-06-28Merge bitcoin/bitcoin#25426: net: add new method Sock::GetSockName() that ↵laanwj
wraps getsockname() and use it in GetBindAddress() a8d6abba5ec4ae2a3375e9be0b739f298899eca2 net: change GetBindAddress() to take Sock argument (Vasil Dimov) 748dbcd9f29dbe4110da8a06f08e3eefa95f5321 net: add new method Sock::GetSockName() that wraps getsockname() (Vasil Dimov) Pull request description: _This is a piece of #21878, chopped off to ease review._ Wrap the syscall `getsockname()` in `Sock::GetSockName()` and change `GetBindAddress()` to take a `Sock` argument so that it can use the wrapper. This further encapsulates syscalls inside the `Sock` class and makes the callers mockable. ACKs for top commit: laanwj: Code review ACK a8d6abba5ec4ae2a3375e9be0b739f298899eca2 Tree-SHA512: 3a73463258c0057487fb3fd67215816b03a1c5160f45e45930eaeef86bb3611ec385794cdb08339aa074feba8ad67cd2bfd3836f6cbd40834e15d933214a05dc
2022-06-27Replace CountSecondsDouble with Ticks<SecondsDouble>MacroFake
2022-06-23[rpc] add new submitpackage RPCglozow
It could be unsafe/confusing to create an actual mainnet interface while package relay doesn't exist. However, a regtest-only interface allows wallet/application devs to test current package policies.
2022-06-23rpc: Use steady_clock for getrpcinfo durationsMacroFake
2022-06-22ArgsMan: Add Get*Arg functions returning optionalCarl Dong
This allows the caller to not provide a default at all and just check inside the optional to see if the arg was set or not.
2022-06-22Merge bitcoin/bitcoin#25428: Remove Sock::Release() and CloseSocket()laanwj
a724c39606273dfe4c6f9887ef8b77d0a98f1b34 net: rename Sock::Reset() to Sock::Close() and make it private (Vasil Dimov) e8ff3f0c52e7512a580bc907dc72e5bb141b4217 net: remove CloseSocket() (Vasil Dimov) 175fb2670a2a24220afb3eea99b7b65b0aa89c76 net: remove now unused Sock::Release() (Vasil Dimov) Pull request description: _This is a piece of #21878, chopped off to ease review._ * `Sock::Release()` is unused, thus remove it * `CloseSocket()` is only called from `Sock::Reset()`, so move the body of `CloseSocket()` inside `Sock::Reset()` and remove `CloseSocket()` - this helps to hide low level file descriptor sockets inside the `Sock` class. * Rename `Sock::Reset()` to `Sock::Close()` and make it `private` - to be used only in the destructor and in the `Sock` assignment operator. This simplifies the public API by removing one method from it. ACKs for top commit: laanwj: Code review ACK a724c39606273dfe4c6f9887ef8b77d0a98f1b34 Tree-SHA512: 4b12586642b3d049092fadcb1877132e285ec66a80af92563a7703c6970e278e0f2064fba45c7eaa78eb65db94b3641fd5e5264f7b4f61116d1a6f3333868639
2022-06-22net: rename Sock::Reset() to Sock::Close() and make it privateVasil Dimov
Outside of `Sock`, `Sock::Reset()` was used in just one place (in `i2p.cpp`) which can use the assignment operator instead. This simplifies the public `Sock` API by having one method less.
2022-06-20net: remove CloseSocket()Vasil Dimov
Do the closing in `Sock::Reset()` and remove the standalone `CloseSocket()`. This reduces the exposure of low-level sockets (i.e. integer file descriptors) outside of the `Sock` class.
2022-06-20net: remove now unused Sock::Release()Vasil Dimov
2022-06-20net: add new method Sock::GetSockName() that wraps getsockname()Vasil Dimov
This will help to increase `Sock` usage and make more code mockable.
2022-06-20build: globally define NOMINMAXfanquake
Define (and document) `NOMINMAX` once, rather than across multiple source files. Defining this prevents the definition of min/max macros when using mingw-w64, which may conflict with unprefixed std::min/max usage. While that might not be the case for us, we'd always prefer to use the standard library in any case. For example: https://github.com/mingw-w64/mingw-w64/blob/73cadc06c62c6af5faf76f64ef08e684b48de48c/mingw-w64-headers/include/ntdef.h#L289-L300
2022-06-16Merge bitcoin/bitcoin#24356: refactor: replace CConnman::SocketEvents() with ↵laanwj
mockable Sock::WaitMany() 6e68ccbefea6509c61fc4405a391a517c6057bb0 net: use Sock::WaitMany() instead of CConnman::SocketEvents() (Vasil Dimov) ae263460bab9e6aa112dc99790c8ef06a56ec838 net: introduce Sock::WaitMany() (Vasil Dimov) cc74459768063a923fb6220a4f420eaf211aee7b net: also wait for exceptional events in Sock::Wait() (Vasil Dimov) Pull request description: _This is a piece of #21878, chopped off to ease review._ `Sock::Wait()` waits for IO events on one socket. Introduce a similar `virtual` method `WaitMany()` that waits simultaneously for IO events on more than one socket. Use `WaitMany()` instead of `CConnman::SocketEvents()` (and ditch the latter). Given that the former is a `virtual` method, it can be mocked by unit and fuzz tests. This will help to make bigger parts of `CConnman` testable (unit and fuzz). ACKs for top commit: laanwj: Code review ACK 6e68ccbefea6509c61fc4405a391a517c6057bb0 jonatack: re-ACK 6e68ccbefea6509c61fc4405a391a517c6057bb0 per `git range-diff e18fd47 6747729 6e68ccb`, and verified rebase to master and debug build Tree-SHA512: 917fb6ad880d64d3af1ebb301c06fbd01afd8ff043f49e4055a088ebed6affb7ffe1dcf59292d822f10de5f323b6d52d557cb081dd7434634995f9148efcf08f
2022-06-10Merge bitcoin/bitcoin#24931: Strengthen thread safety assertionsMacroFake
ce893c0497fc9b8ab9752153dfcc77c9f427545e doc: Update developer notes (Anthony Towns) d2852917eecad6ab422a7b2c9892d351a7f0cc96 sync.h: Imply negative assertions when calling LOCK (Anthony Towns) bba87c0553780eacf0317fbfec7330ea27aa02f8 scripted-diff: Convert global Mutexes to GlobalMutexes (Anthony Towns) a559509a0b8cade27199740212d7b589f71a0e3b sync.h: Add GlobalMutex type (Anthony Towns) be6aa72f9f8d50b6b5b19b319a74abe7ab4099ff qt/clientmodel: thread safety annotation for m_cached_tip_mutex (Anthony Towns) f24bd45b37e1b2d19e5a053dbfefa30306c1d41a net_processing: thread safety annotation for m_tx_relay_mutex (Anthony Towns) Pull request description: This changes `LOCK(mutex)` for non-global, non-recursive mutexes to be annotated with the negative capability for the mutex it refers to, to prevent . clang applies negative capabilities recursively, so this helps avoid forgetting to annotate functions. This can't reasonably be used for globals, because clang would require every function to be annotated with `EXCLUSIVE_LOCKS_REQUIRED(!g_mutex)` for each global mutex; so this introduces a trivial `GlobalMutex` subclass of `Mutex`, and reduces the annotations for both `GlobalMutex` to `LOCKS_EXCLUDED` which only catches trivial errors (eg (`LOCK(x); LOCK(x);`). ACKs for top commit: MarcoFalke: review ACK ce893c0497fc9b8ab9752153dfcc77c9f427545e 🐦 hebasto: ACK ce893c0497fc9b8ab9752153dfcc77c9f427545e Tree-SHA512: 5c35e8c7677ce3d994a7e3774f4344adad496223a51b3a1d1d3b5f20684b2e1d5cff688eb3fbc8d33e1b9940dfa76e515f9434e21de6f3ce3c935e29a319f529
2022-06-09net: introduce Sock::WaitMany()Vasil Dimov
It allows waiting concurrently on more than one socket. Being a `virtual` `Sock` method it can be overriden by tests. Will be used to replace `CConnman::SocketEvents()`.
2022-06-09net: also wait for exceptional events in Sock::Wait()Vasil Dimov
This mimics closely `CConnman::SocketEvents()` and the underlying `poll(2)`.
generated by cgit v1.2.3 (git 2.26.2) at 2024-11-22 08:04:21 +0000