aboutsummaryrefslogtreecommitdiff
path: root/src/init.cpp
AgeCommit message (Collapse)Author
2022-09-21Merge bitcoin/bitcoin#26145: [24.x] init: abort if i2p/cjdns are chosen via ↵fanquake
-onlynet but are unreachable 68209a7b5c0326e14508d9cf749771605bd6ffe7 rpc: make addpeeraddress work with cjdns addresses (Martin Zumsande) a8a9ed67cc447d204304ccfd844c45fd76486c6a init: Abort if i2p/cjdns are chosen via -onlynet but unreachable (Martin Zumsande) Pull request description: Identical commit from https://github.com/bitcoin/bitcoin/pull/25989 ACKs for top commit: fanquake: ACK 68209a7b5c0326e14508d9cf749771605bd6ffe7 Tree-SHA512: eec335df06b4c209cfe3473cb623828effd00c45a5dd605bb920edd265de1c789627482b005a51e89b8fc79cc4c5d26ff1fc306f2e4573897c5c7f083aa22861
2022-09-19init: Abort if i2p/cjdns are chosen via -onlynet but unreachableMartin Zumsande
...because -i2psam or -cjdnsreachable are not provided. This mimics existing behavior for -onlynet=onion and non-specified proxy.
2022-09-19doc: Move -permitbaremultisig to the relay help categoryMacroFake
2022-09-16Merge bitcoin/bitcoin#25499: Use steady clock for all millis bench loggingfanquake
fa521c960337a65d4ce12cd1ef009c652ffe57e6 Use steady clock for all millis bench logging (MacroFake) Pull request description: Currently `GetTimeMillis` is used for bench logging in milliseconds integral precision. Replace it to use a steady clock that is type-safe and steady. Microsecond or float precision can be done in a follow-up. ACKs for top commit: fanquake: ACK fa521c960337a65d4ce12cd1ef009c652ffe57e6 - started making the same change. Tree-SHA512: 86a810e496fc663f815acb8771a6c770331593715cde85370226685bc50c13e8e987e3c5efd0b4e48b36ebd2372255357b709204bac750d41e94a9f7d9897fa6
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-13Merge bitcoin/bitcoin#24991: init: allow startup with -onlynet=onion ↵fanquake
-listenonion=1 2d0b4e4ff66e60c85f86c526a53f8fb242ebb7d0 init: allow startup with -onlynet=onion -listenonion=1 (Vasil Dimov) Pull request description: It does not make sense to specify `-onlynet=onion` without providing a Tor proxy (even if other `-onlynet=...` are given). This is checked during startup. However, it was forgotten that a Tor proxy can also be retrieved from "Tor control" to which we connect if `-listenonion=1`. So, the full Tor proxy retrieval logic is: 1. get it from `-onion` 2. get it from `-proxy` 3. if `-listenonion=1`, then connect to "Tor control" and get the proxy from there (was forgotten before this change) Fixes https://github.com/bitcoin/bitcoin/issues/24980 ACKs for top commit: mzumsande: Tested ACK 2d0b4e4ff66e60c85f86c526a53f8fb242ebb7d0 MarcoFalke: ACK 2d0b4e4ff66e60c85f86c526a53f8fb242ebb7d0 🕸 Tree-SHA512: d1d18e07a8a40a47b7f00c31cb291a3d3a9b24eeb28c5e4720d5df4997f488583a3a010d46902b4b600d2ed1136a368e1051c133847ae165e0748b8167603dc3
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-07Merge bitcoin/bitcoin#25678: p2p: skip querying dns seeds if `-onlynet` ↵fanquake
disables IPv4 and IPv6 385f5a4c3feb716fcf3f2b4823535df6da6bb67b p2p: Don't query DNS seeds when both IPv4 and IPv6 are unreachable (Martin Zumsande) 91f0a7fbb79fe81a75370a4b60dcdd2e55edfa81 p2p: add only reachable addresses to addrman (Martin Zumsande) Pull request description: Currently, `-onlynet` does not work well in connection with initial peer discovery, because DNS seeds only resolve to IPv6 and IPv4 adresses: With `-onlynet=i2p`, we would load clearnet addresses from DNS seeds into addrman, be content our addrman isn't empty so we don't try to query hardcoded seeds (although these exist for i2p!), and never attempt to make an automatic outbound connection. With `-onlynet=onion` and `-proxy` set, we wouldn't load addresses via DNS, but will make AddrFetch connections (through a tor exit node) to a random clearnet peer the DNS seed resolves to (see https://github.com/bitcoin/bitcoin/issues/6808#issuecomment-147652505), thus breaching the `-onlynet` preference of the user - this has been reported in the two issues listed below. This PR proposes two changes: 1.) Don't load addresses that are unreachable (so that we wouldn't connect to them) into addrman. This is already the case for addresses received via p2p addr messages, this PR implements the same for addresses received from DNS seeds and fixed seeds. This means that in the case of `-onlynet=onion`, we wouldn't load fixed seed IPv4 addresses into addrman, only the onion ones. 2.) Skip trying the DNS seeds if neither IPv4 nor IPv6 are reachable and move directly to adding the hardcoded seeds from networks we can connect to. This is done by soft-setting `-dnsseed` to 0 in this case, unless `-dnsseed=1` was explicitly specified, in which case we abort with an `InitError`. Fixes #6808 Fixes #12344 ACKs for top commit: naumenkogs: utACK 385f5a4c3feb716fcf3f2b4823535df6da6bb67b vasild: ACK 385f5a4c3feb716fcf3f2b4823535df6da6bb67b Tree-SHA512: 33a8c29faccb2d9b937b017dba4ef72c10e05e458ccf258f1aed3893bcc37c2e984ec8de998d2ecfa54282abbf44a132e97d98bbcc24a0dcf1871566016a9b91
2022-09-06p2p: Don't query DNS seeds when both IPv4 and IPv6 are unreachableMartin Zumsande
This happens, for example, if the user specified -onlynet=onion or -onlynet=i2p. DNS seeds only resolve to IPv4 / IPv6 addresses, making their answers useless to us, since we don't want to make connections to these. If, within the DNS seed thread, we'd instead do fallback AddrFetch connections to one of the clearnet addresses the DNS seed resolves to, we might get usable addresses from other networks if lucky, but would be violating our -onlynet user preference in doing so. Therefore, in this case it is better to rely on fixed seeds for networks we want to connect to. Co-authored-by: Vasil Dimov <vd@FreeBSD.org>
2022-09-05init: allow startup with -onlynet=onion -listenonion=1Vasil Dimov
It does not make sense to specify `-onlynet=onion` without providing a Tor proxy (even if other `-onlynet=...` are given). This is checked during startup. However, it was forgotten that a Tor proxy can also be retrieved from "Tor control" to which we connect if `-listenonion=1`. So, the full Tor proxy retrieval logic is: 1. get it from `-onion` 2. get it from `-proxy` 3. if `-listenonion=1`, then connect to "Tor control" and get the proxy from there (was forgotten before this change) Fixes https://github.com/bitcoin/bitcoin/issues/24980
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-01Merge bitcoin/bitcoin#25951: log: Move validation option logging to ↵MacroFake
LoadChainstate() fa4c59d65bdf1c64056b18a58f8aaa2800995aa6 Move blockstorage option logging to LoadChainstate() (MacroFake) fa3358b668d8415adc1b5586caa382fe4140ad42 Move validation option logging to LoadChainstate() (MacroFake) Pull request description: This would allow libbitcoinkernel users to see the options logged as well. Currently they would only be logged for bitcoind. Behavior change suggested in the refactoring pull https://github.com/bitcoin/bitcoin/pull/25704#discussion_r956166460 ACKs for top commit: ryanofsky: Code review ACK fa4c59d65bdf1c64056b18a58f8aaa2800995aa6. Only change since last review is moving pruning logprints out of `AppInitParameterInteraction` as suggested jonatack: Review ACK fa4c59d65bdf1c64056b18a58f8aaa2800995aa6 Tree-SHA512: f27508ca06a78ef162f002d556cf830df374fe95fd4f10bf22c24b6b48276ce49f52f82ffedc43596c872ddcf08321ca03651495fd3abde16254cb8afab39d33
2022-09-01Move blockstorage option logging to LoadChainstate()MacroFake
2022-09-01Merge bitcoin/bitcoin#25887: init: avoid unsetting service bits from ↵MacroFake
`nLocalServices` 1b5bec78e942473b6ffac4004b1cf6d535fd2892 init: avoid unsetting service bits from `nLocalServices` (Sebastian Falbesoner) Pull request description: This PR is a late follow-up to the [review club session about the PR "Default to NODE_WITNESS in nLocalServices" ](https://bitcoincore.reviews/21090#l-90) (#21090): ``` 17:32 <lightlike> hmm, if we are in pruned mode, we first set NODE_NETWORK and then unset it later in init.cpp. that seems a bit strange. ... 17:33 <jnewbery> lightlike: ah yes, you're right. That does seem a bit messy. ``` Rather than setting the service bit `NODE_NETWORK` first and then unset it (if in `fPruneMode`), start with the bare minimum flags that we always serve and only add `NODE_NETWORK` if we are running as a non-pruned node. This seems to be a more logical approach than currently on master. ACKs for top commit: naumenkogs: ACK 1b5bec78e942473b6ffac4004b1cf6d535fd2892 stickies-v: ACK 1b5bec78e942473b6ffac4004b1cf6d535fd2892 LarryRuane: ACK 1b5bec78e942473b6ffac4004b1cf6d535fd2892 Tree-SHA512: 2e82d66c4298ffacff41d9e0458b74b83bc156a1fa49e3f3471e942878e5dd2b253b5597ee5ec1d9c8726b432751d05e40f0c580f3976a9e00a7d1f417921ab0
2022-08-29Move validation option logging to LoadChainstate()MacroFake
2022-08-22Merge bitcoin/bitcoin#25775: docs: remove non-signaling mentions of BIP125fanquake
1dc03dda05e9dce128e57f05bb7b1bb02b3cfb9e [doc] remove non-signaling mentions of BIP125 (glozow) 32024d40f03fbf47c64d814fa5f2c2a73ec14cb7 scripted-diff: remove mention of BIP125 from non-signaling var names (glozow) Pull request description: We have pretty thorough documentation of our RBF policy in doc/policy/mempool-replacements.md. It enumerates each rule with several sentences of rationale. Also, each rule pretty much has its own function (3 and 4 share one), with extensive comments. The doc states explicitly that our rules are similar but differ from BIP125, and contains a record of historical changes to RBF policy. We should not use "BIP125" as synonymous with our RBF policy because: - Our RBF policy is different from what is specified in BIP125, for example: - the BIP does not mention our rule about the replacement feerate being higher (our Rule 6) - the BIP uses minimum relay feerate for Rule 4, while we have used incremental relay feerate since #9380 - the "inherited signaling" question (CVE-2021-31876). Call it discrepancy, ambiguous wording, doc misinterpretation, or implementation details, I would recommend users refer to doc/policy/mempool-replacements.md - the signaling policy is configurable, see #25353 - Our RBF policy may change further - We have already marked BIP125 as only "partially implemented" in docs/bips.md since 1fd49eb498c75a1d14193bb736d195a3dc75ae12 - See comments from people who are not me recently: - https://github.com/bitcoin/bitcoin/pull/25038#discussion_r909507429 - https://github.com/bitcoin/bitcoin/pull/25575#issuecomment-1179519204 This PR removes all non-signaling mentions of BIP125 (if people feel strongly, we can remove all mentions of BIP125 period). It may be useful to refer to the concept of "tx opts in to RBF if it has at least one nSequence less than (0xffffffff - 1)" as "BIP125 signaling" because: - It is succint. - It has already been widely marketed as BIP125 opt-in signaling. - Our API uses it when referring to signaling (e.g. getmempoolentry["bip125-replaceable"] and wallet error message "not BIP 125 replaceable"). Changing those is more invasive. - If/when we have other ways to signal in the future, we can disambiguate them this way. See #25038 which proposes another way of signaling, and where I pulled these commits from. Alternatives: - Changing our policy to match BIP125. This doesn't make sense as, for example, we would have to remove the requirement that a replacement tx has a higher feerate (Rule 6). - Changing BIP125 to match what we have. This doesn't make sense as it would be a significant change to a BIP years after it was finalized and already used as a spec to implement RBF in other places. - Document our policy as a new BIP and give it a number. This might make sense if we don't expect things to change a lot, and can be done as a next step. ACKs for top commit: darosior: ACK 1dc03dda05e9dce128e57f05bb7b1bb02b3cfb9e ariard: ACK 1dc03dda t-bast: ACK https://github.com/bitcoin/bitcoin/commit/1dc03dda05e9dce128e57f05bb7b1bb02b3cfb9e Tree-SHA512: a3adc2039ec5785892d230ec442e50f47f7062717392728152bbbe27ce1c564141f85253143f53cb44e1331cf47476d74f5d2f4b3cd873fc3433d7a0aa783e02
2022-08-20init: avoid unsetting service bits from `nLocalServices`Sebastian Falbesoner
Rather than setting the service bit `NODE_NETWORK` first and then unset it, start out the bare minimum flags that every node serves and only add `NODE_NETWORK` if we are running as a non-pruned node.
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-19Merge bitcoin/bitcoin#25707: refactor: Make const references to avoid ↵MacroFake
unnecessarily copying objects and enable two clang-tidy checks ae7ae36d311a869b3bda41d29dc0e47fade77d28 tidy: Enable two clang-tidy checks (Aurèle Oulès) 081b0e53e3adca7ea57d23e5fcd9db4b86415a72 refactor: Make const refs vars where applicable (Aurèle Oulès) Pull request description: I added const references to some variables to avoid unnecessarily copying objects. Also added two clang-tidy checks : [performance-for-range-copy](https://releases.llvm.org/11.1.0/tools/clang/tools/extra/docs/clang-tidy/checks/performance-for-range-copy.html) and [performance-unnecessary-copy-initialization](https://releases.llvm.org/12.0.0/tools/clang/tools/extra/docs/clang-tidy/checks/performance-unnecessary-copy-initialization.html). ACKs for top commit: vasild: ACK ae7ae36d311a869b3bda41d29dc0e47fade77d28 MarcoFalke: review ACK ae7ae36d311a869b3bda41d29dc0e47fade77d28 Tree-SHA512: f6ac6b0cd0eee1e0c34d2f186484bc0f7ec6071451cccb33fa88a67d93d92b304e2fac378b88f087e94657745bca4e966dbc443759587400eb01b1f3061fde8c
2022-08-16refactor: Add lock annotations to Active* methodsMacroFake
This is a refactor, putting the burden to think about thread safety to the caller. Otherwise, there is a risk that the caller will assume thread safety where none exists, as is evident in the previous two commits.
2022-08-16Fix UB/data-race in RPCNotifyBlockChangeMacroFake
ActiveTip() is *not* thread-safe, as the required ::cs_main lock will be released as ActiveChainstate() returns. ActiveTip() is an alias for ActiveChainstate().m_chain.Tip(), so m_chain may be involved in a data-race (UB).
2022-08-04[doc] remove non-signaling mentions of BIP125glozow
Our RBF policy is different from the rules specified in BIP125. For example, the BIP does not mention Rule 6, and our Rule 4 uses the (configurable) incremental relay feerate (distinct from the minimum relay feerate). Those interested in our policy should refer to doc/policy/mempool-replacements.md instead. These rules may also continue to diverge with package RBF and other RBF improvements. Keep references to the BIP125 signaling wrt sequence numbers, since that is still correct and widely used. It is helpful to refer to this as "BIP125 signaling" since it is unambiguous and succint, especially if we have multiple ways to signal replaceability in the future. The rule numbers in doc/policy/mempool-replacements.md correspond largely to those of BIP 125, so we can still refer to them like "Rule 5."
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-02sort after scripted-diffMacroFake
2022-08-02scripted-diff: Move mempool_args to src/nodeMacroFake
It is part of the node library. Also, it won't be moved to the kernel lib, as it will be pruned of ArgsManager. -BEGIN VERIFY SCRIPT- # Move module git mv src/mempool_args.cpp src/node/ git mv src/mempool_args.h src/node/ # Replacements sed -i 's:mempool_args\.h:node/mempool_args.h:g' $(git grep -l mempool_args) sed -i 's:mempool_args\.cpp:node/mempool_args.cpp:g' $(git grep -l mempool_args) sed -i 's:MEMPOOL_ARGS_H:NODE_MEMPOOL_ARGS_H:g' $(git grep -l MEMPOOL_ARGS_H) -END VERIFY SCRIPT-
2022-08-02Remove ::g_max_datacarrier_bytes globalMacroFake
2022-08-02Combine datacarrier globals into oneMacroFake
2022-08-02Remove ::fIsBareMultisigStd globalMacroFake
2022-08-02Remove ::dustRelayFeeMacroFake
2022-08-02Remove ::incrementalRelayFee and ::minRelayTxFee globalsMacroFake
2022-08-02Remove ::fRequireStandard globalMacroFake
2022-08-02Return optional error from ApplyArgsManOptionsMacroFake
Also pass in a (for now unused) reference to the params. Both changes are needed for the next commit.
2022-07-30Use steady clock for all millis bench loggingMacroFake
2022-07-27refactor: Make const refs vars where applicableAurèle Oulès
This avoids initializing variables with the copy-constructor of a non-trivially copyable type.
2022-07-19refactor: Reduce number of SanityChecks return valuesRyan Ofsky
2022-07-19refactor: Reduce number of LoadChainstate return valuesRussell Yanofsky
2022-07-19refactor: Reduce number of LoadChainstate parametersRussell Yanofsky
2022-07-19Merge bitcoin/bitcoin#25494: indexes: Stop using node internal typesfanquake
7878f97bf15b6e7c9b47d1c0d96419b97e1bdcbd indexes, refactor: Remove CChainState use in index CommitInternal method (Ryan Ofsky) ee3a079fab2c33b4186b62ab822753954a4e545f indexes, refactor: Remove CBlockIndex* uses in index Rewind methods (Ryan Ofsky) dc971be0831959e7ee6a6df9e1aa46091351a8fb indexes, refactor: Remove CBlockIndex* uses in index WriteBlock methods (Ryan Ofsky) bef4e405f3de2718dfee279a9abff4daf016da26 indexes, refactor: Remove CBlockIndex* uses in index Init methods (Ryan Ofsky) addb4f2af183a25ce4a6b6485b5b49575a2ba31b indexes, refactor: Remove CBlockIndex* uses in coinstatsindex LookUpOne function (Ryan Ofsky) 33b4d48cfcdf145f49cb2283ac3e2936a4e23fff indexes, refactor: Pass Chain interface instead of CChainState class to indexes (Ryan Ofsky) a0b5b4ae5a24536d333cbce2ea584f2d935c651f interfaces, refactor: Add more block information to block connected notifications (Ryan Ofsky) Pull request description: Start transitioning index code away from using internal node types like `CBlockIndex` and `CChain` so index code is less coupled to node code and index code will later be able to stop locking cs_main and sync without having to deal with validationinterface race conditions, and so new indexes are easier to write and can run as plugins or separate processes. This PR contains the first 7 commits from https://github.com/bitcoin/bitcoin/pull/24230#issuecomment-1165625977 which have been split off for easier review. Previous review comments can be found in #24230 ACKs for top commit: MarcoFalke: ACK 7878f97bf15b6e7c9b47d1c0d96419b97e1bdcbd though did not review the last commit 🤼 mzumsande: Code Review ACK 7878f97bf15b6e7c9b47d1c0d96419b97e1bdcbd Tree-SHA512: f84ac2eb6dca2c305566ddeb35ea14d0b71c00860c0fd752bbcf1a0188be833d8c2a6ac9d3ef6ab5b46fbd02d7a24cbb8f60cf12464cb8ba208e22287f709989
2022-07-18indexes, refactor: Pass Chain interface instead of CChainState class to indexesRyan Ofsky
Passing abstract Chain interface will let indexes run in separate processes. This commit does not change behavior in any way.
2022-07-18refactor: remove unused using directivesfanquake
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-15Move {Load,Dump}Mempool to kernel namespaceCarl Dong
Also: 1. Add the newly introduced kernel/mempool_persist.cpp to IWYU CI script 2. Add chrono mapping for iwyu
2022-07-15Move DEFAULT_PERSIST_MEMPOOL out of libbitcoinkernelCarl Dong
It is no longer used by anything inside libbitcoinkernel, move it to node/mempool_persist_args.h where it belongs.
2022-07-15LoadMempool: Pass in load_path, stop using gArgsCarl Dong
Also: 1. Have CChainState::LoadMempool and ::ThreadImport take in paths and pass it through untouched to LoadMempool. 2. Make LoadMempool exit early if the load_path is empty. 3. Adjust the call to ::ThreadImport in ::AppInitMain to correctly pass in an empty path if mempool persistence is disabled.
2022-07-15scripted-diff: Rename m_is_loaded -> m_load_triedCarl Dong
m_is_loaded/IsLoaded() doesn't actually indicate whether or not the mempool was successfully, loaded, but rather if a load has been attempted and did not result in a catastrophic ShutdownRequested. -BEGIN VERIFY SCRIPT- find_regex="\bm_is_loaded\b" \ && git grep -l -E "$find_regex" \ | xargs sed -i -E "s@$find_regex@m_load_tried@g" find_regex="\bIsLoaded\b" \ && git grep -l -E "$find_regex" \ | xargs sed -i -E "s@$find_regex@GetLoadTried@g" find_regex="\bSetIsLoaded\b" \ && git grep -l -E "$find_regex" \ | xargs sed -i -E "s@$find_regex@SetLoadTried@g" -END VERIFY SCRIPT-
2022-07-15DumpMempool: Pass in dump_path, stop using gArgsCarl Dong
Also introduce node::{ShouldPersistMempool,MempoolPath} helper functions in node/mempool_persist_args.{h,cpp} which are used by non-kernel DumpMempool callers to determine whether or not to automatically dump the mempool and where to dump it to.
2022-07-14Use designated initializers for ChainstateManager::OptionsCarl Dong
This wasn't available at the time when ChainstateManager::Options was introduced but is helpful to be explicit and ensure correctness.
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-08Merge bitcoin/bitcoin#25353: Add a `-mempoolfullrbf` node settingMacroFake
4c9666bd73645b94ae81be1faf7a0b8237dd6e99 Mention `mempoolfullrbf` in policy/mempool-replacements.md (Antoine Riard) aae66ab43d794bdfaa2dade91760cc55861c9693 Update getmempoolinfo RPC with `mempoolfullrbf` (Antoine Riard) 3e27e317270fdc2dd02794fea9da016387699636 Introduce `mempoolfullrbf` node setting. (Antoine Riard) Pull request description: This is ready for review. Recent discussions among LN devs have brought back on the surface concerns about the security of multi-party funded transactions against pinnings attacks and other mempool-based nuisances. The lack of full-rbf transaction-relay topology connected to miners open the way to cheap and naive DoS against multi-party funded transactions (e.g coinjoins, dual-funded channels, on-chain DLCs, ...) without solutions introducing an overhead cost or centralization vectors afaik . For more details, see [0]. This PR implements a simple `fullrbf` setting, where the node always allows transaction replacement, ignoring BIP125 opt-in flag. The default value of the setting stays **false**, therefore opt-in replacement is still the default Bitcoin Core replacement policy. Contrary to a previous proposal of mine and listening to feedbacks collected since then [1], I think this new setting simply offers more flexibility in a node transaction-relay policy suiting one's application requirements, without arguing a change of the default behavior. I [posted](https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2022-June/020557.html) on the ML to invite operators with a bitcoin application sensitive to full-rbf (e.g dual-funded LN channels service providers) or mempool researchers to join a bootstrapped full-rbf activated peers network for experimentation and learning. If people have strong opinions against the existence of such full-rbf transaction-relay network, I'm proposing to express them on the future thread. [0] https://lists.linuxfoundation.org/pipermail/lightning-dev/2021-May/003033.html [1] https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2021-June/019074.html Follow-up suggestions : - soft-enable opt-in RBF in the wallet : https://github.com/bitcoin/bitcoin/pull/25353#issuecomment-1154918789 - p2p discovery and additional outbound connection to full-rbf peers : https://github.com/bitcoin/bitcoin/pull/25353#issuecomment-1156044401 - match the code between RPC, wallet and mempool about disregard of inherited signaling : #22698 ACKs for top commit: instagibbs: reACK https://github.com/bitcoin/bitcoin/pull/25353/commits/4c9666bd73645b94ae81be1faf7a0b8237dd6e99 glozow: ACK 4c9666bd73645b94ae81be1faf7a0b8237dd6e99, a few nits which are non-blocking. w0xlt: ACK https://github.com/bitcoin/bitcoin/pull/25353/commits/4c9666bd73645b94ae81be1faf7a0b8237dd6e99 Tree-SHA512: 9e288bf22e06a9808804e58178444ef1830c3fdd42fd8a7cd7ffb101f8f586e08b000679be407d63ca76a56f7216227b368ff630c81f3fac3243db1a1202ab1c