aboutsummaryrefslogtreecommitdiff
AgeCommit message (Collapse)Author
2022-05-28Merge bitcoin/bitcoin#25224: Get time less often in ↵fanquake
AddrManImpl::ResolveCollisions_() fa27ee88edbf696b7eef2efbfcf1446ec522fd85 Get time less often in AddrManImpl::ResolveCollisions_() (MarcoFalke) Pull request description: First commit split out from https://github.com/bitcoin/bitcoin/pull/24697 ACKs for top commit: sipa: utACK fa27ee88edbf696b7eef2efbfcf1446ec522fd85 fanquake: ACK fa27ee88edbf696b7eef2efbfcf1446ec522fd85 Tree-SHA512: 40c8594d2a5ce02a392ac5f9f120c24c6bcd495b0bcc901fd6064dde9f6123cd109504cee7b612a9555b70cfd7759cbd6cd496d007bb374c27610d01b464191c
2022-05-27Merge bitcoin/bitcoin#25214: multiprocess build fix: ipc/capnp/init.capnp.h: ↵fanquake
No such file or directory 44904aa632cbe0722a52f1907ae015f1c6338ddf multiprocess build cleanup: comment on manual dependencies (Ryan Ofsky) 6e1c16c1442ab30694a3668b45af5bfd532f1e12 multiprocess build fix: ipc/capnp/init.capnp.h: No such file or directory (Ryan Ofsky) Pull request description: Error was reported by SatoriHoshiAiko in https://github.com/bitcoin/bitcoin/issues/25207 and happens unpredictably because make doesn't always build dependencies in the same order. The source file `src/ipc/capnp/protocol.cpp` includes some generated headers so needs to have an explicit dependency specified in the makefile so the headers will be generated before the file is compiled. #19160 added the explicit dependency, but it was incorrect because it referred to an old file path from before the source file was renamed (`ipc.cpp` -> `protocol.cpp`) ACKs for top commit: hebasto: re-ACK 44904aa632cbe0722a52f1907ae015f1c6338ddf Tree-SHA512: 578ef4ef35a97e9d8d4e6d4edf39e57a32674234bf145e8159268324cb6ba15b420517afc07f6f42bb11a562954ea74af686c3fb92ce178c1fc378421b352531
2022-05-27Merge bitcoin/bitcoin#24408: rpc: add rpc to get mempool txs spending ↵MacroFake
specific prevouts 418557034055f740951294e7677ae9fd5149ea9b Add RPC to get mempool txs spending outputs (t-bast) Pull request description: We add an RPC to fetch mempool transactions spending any of the given outpoints. Without this RPC, application developers need to first call `getrawmempool` which returns a long list of `txid`, then fetch each of these transactions individually (`getrawtransaction`) to check whether they spend the given outpoints, which wastes a lot of bandwidth (in the worst case we need to transfer the whole mempool). For example in lightning, when we discover that one of our channel funding transactions has been spent, we need to find the spending transaction to claim our outputs from it. We are currently forced to fetch the whole mempool to do the analysis ourselves, which is quite costly. I believe that this RPC is also generally useful when doing some introspection on your mempool after one of your transactions failed to broadcast, for example when you implement RBF at the application level. Fetching and analyzing the conflicting transaction gives you more information to successfully replace it. ACKs for top commit: darosior: re-utACK 418557034055f740951294e7677ae9fd5149ea9b vincenzopalazzo: re-ACK https://github.com/bitcoin/bitcoin/pull/24408/commits/418557034055f740951294e7677ae9fd5149ea9b danielabrozzoni: re-tACK 418557034055f740951294e7677ae9fd5149ea9b w0xlt: reACK https://github.com/bitcoin/bitcoin/pull/24408/commits/418557034055f740951294e7677ae9fd5149ea9b Tree-SHA512: 206687efb720308b7e0b6cf16dd0a994006c0b5a290c8eb386917a80130973a6356d0d5cae1c63a01bb29e066dd721594969db106cba7249214fcac90d2c3dbc
2022-05-27Merge bitcoin/bitcoin#24934: refactor, miner: Delete call to ↵MacroFake
UpdatePackagesForAdded at beginning of addPackageTxs 7036cf52aa080d2a63993c2298555252d507dd2f Delete UpdatePackagesForAdded at beginning of addPackageTxs. (KevinMusgrave) Pull request description: In `CreateNewBlock` (in miner.cpp), `inBlock` is cleared before `addPackageTxs`, so `inBlock` will be empty in the first call to `UpdatePackagesForAdded`. I saw this brought up in these [PR review club logs](https://bitcoincore.reviews/24538) and there didn't seem to be a definitive answer for why the call is necessary. There's also an [old PR](https://github.com/bitcoin/bitcoin/pull/10200) where this change was going to be applied, but it got closed. If `addPackageTxs` can be called when `inBlock` is not empty, then maybe a test should be added for that case. All the tests seem to pass with this deletion. ACKs for top commit: glozow: utACK 7036cf52aa080d2a63993c2298555252d507dd2f Tree-SHA512: 9e757b71b9035f68a0c6fef229b8cd83f1bdbe23f05bb02cc1bab8c3c177805b388bceb2bb1f0bce354791ccb29f351a6c51979b96ffe4d9fc6c978f83e36afc
2022-05-27Merge bitcoin/bitcoin#25201: windeploy: Renewed windows code signing certificatefanquake
7e9fe6d800ee8f3381e8f6ad2371f7775c68fad9 windeploy: Renewed windows code signing certificate (Andrew Chow) Pull request description: The current windows code signing certificate expires on May 26 23:59:59 2022 GMT. I have purchased a new code signing certificate which will expire on May 29 23:59:59 2024 GMT. ACKs for top commit: laanwj: ACK 7e9fe6d800ee8f3381e8f6ad2371f7775c68fad9 fanquake: ACK 7e9fe6d800ee8f3381e8f6ad2371f7775c68fad9 - tested above with OpenSSL 3 & faketime. Tree-SHA512: 283eb863d4db0573c7e78fe9d8f1b855533fc45b0995cd2d66e40b5242eb9bc9317b01e1b151fe49d512cd4aa6c48e2390017070f79db46493813fdd0a0f568a
2022-05-27Merge bitcoin/bitcoin#25217: scripts and tools: update lint-logs.py to ↵MacroFake
detect LogPrintLevel, mention WalletLogPrintf 75848ec2daa43b648cf497c59924d389e8243320 scripts and tools: update lint-logs.py to detect LogPrintLevel() (Jon Atack) Pull request description: Follow-up to #24464 that added the `LogPrintLevel()` macro. - update the `lint-logs.py` script to detect `LogPrintLevel()` - add `WalletLogPrintf()` (already detected but not mentioned) to the linter suggestion Example output: ``` $ test/lint/lint-logs.py All calls to LogPrintf(), LogPrint(), LogPrintLevel(), and WalletLogPrintf() should be terminated with "\n". src/addrdb.cpp:147: LogPrintf("banlist.dat ignored because it can only be read by " PACKAGE_NAME " version 22.x. Remove %s to silence this warning.", fs::quoted(fs::PathToString(m_banlist_dat))); src/addrman.cpp:388: LogPrint(BCLog::ADDRMAN, "addrman lost %i new and %i tried addresses due to collisions or invalid addresses", nLostUnk, nLost); src/banman.cpp:41: LogPrintf("Recreating the banlist database"); src/banman.cpp:66: LogPrint(BCLog::NET, "Flushed %d banned node addresses/subnets to disk %dms", banmap.size(), src/banman.cpp:194: LogPrint(BCLog::NET, "Removed banned node address/subnet: %s", sub_net.ToString()); src/net.cpp:2092: LogPrint(BCLog::NET, "Trying to make an anchor connection to %s", addrConnect.ToString()); src/net.cpp:2408: LogPrintLevel(BCLog::Level::Error, BCLog::NET, "%s", strError.original); src/net.cpp:2416: LogPrintf("%s", strError.original); src/net.cpp:2432: LogPrintf("%s", strError.original); src/net.cpp:2453: LogPrintLevel(BCLog::Level::Error, BCLog::NET, "%s", strError.original); src/netbase.cpp:573: LogPrintf("wait for connect to %s failed: %s", src/netbase.cpp:578: LogPrint(BCLog::NET, "connection attempt to %s timed out", addrConnect.ToString()); src/netbase.cpp:590: LogPrintf("getsockopt() for %s failed: %s", addrConnect.ToString(), NetworkErrorString(WSAGetLastError())); src/wallet/wallet.cpp:186: wallet->WalletLogPrintf("Releasing wallet"); src/wallet/wallet.cpp:1809: WalletLogPrintf("Rescan completed in %15dms", duration_milliseconds.count()); ``` ACKs for top commit: laanwj: ACK 75848ec2daa43b648cf497c59924d389e8243320 Tree-SHA512: 78b3bc4428aaec2d198e1ff1c44b614fa2f2a228feb6303d97765136e4796171928d341c6e55a1d4e9e5a82ab099b14fee562b878dcf7bd2276f87bd9a8d323e
2022-05-26Merge bitcoin/bitcoin#24032: Add defaults to vDeployments to avoid ↵laanwj
uninitialized variables c4c5b9ca6e98cf44309af73edf5559940a04d00f consensus/params: set default values for BIP9Deployment (Anthony Towns) Pull request description: Adds default values for `vDeployments` in `consensus/params.h` so that undefined behaviour is avoided if a deployment is not initialized. Also adds a check in the unit tests to alert if this is happening, since even if it doesn't invoke undefined behaviour it's probably a mistake. ACKs for top commit: laanwj: Code review ACK c4c5b9ca6e98cf44309af73edf5559940a04d00f Tree-SHA512: 22d7ff86a817d9e9e67c47301fc3b7e9d5821c13565d7706711f113dea220eea29b413a7c8d029691009159cebc85a108d77cb52418734091c196bafb2b12423
2022-05-26Merge bitcoin/bitcoin#25003: tracing: fix ↵Andrew Chow
`coin_selection:aps_create_tx_internal` calling logic 6b636730f4befee39d57fcfd51298f3015dbf563 tracing: fix `coin_selection:aps_create_tx_internal` calling logic (Sebastian Falbesoner) Pull request description: According to the documentation, the tracepoint `coin_selection:aps_create_tx_internal` "Is called when the second `CreateTransactionInternal` with Avoid Partial Spends enabled completes." Currently it is only called if the second call to `CreateTransactionInternal` succeeds, i.e. the third parameter is always `true` and we don't get notified in the case that it fails. This PR fixes this by moving the tracepoint call and the `use_aps` boolean variable outside the if body. ACKs for top commit: achow101: ACK 6b636730f4befee39d57fcfd51298f3015dbf563 furszy: re-ACK 6b636730 Tree-SHA512: 453825123aa10748642c7dd94324ced2d07df0f4fac478b0947a34820b515ae300f75721679a90a164f3127029739df55c4de035c4567e663893c3c6dbdef216
2022-05-26Merge bitcoin/bitcoin#15936: interfaces: Expose settings.json methods to GUIMacroFake
f9fdcec7e932843a91ddf7f377e00bd2a6efb82a settings: Add resetSettings() method (Ryan Ofsky) 77fabffef4ea840ee15c97061048fe8443d74658 init: Remove Shutdown() node.args reset (Ryan Ofsky) 0e55bc6e7fe439404dc56093a0949395dae51e6b settings: Add update/getPersistent/isIgnored methods (Ryan Ofsky) Pull request description: Add `interfaces::Node` `updateSetting`, `forceSetting`, `resetSettings`, `isSettingIgnored`, and `getPersistentSetting` methods so GUI is able to manipulate `settings.json` file and use and modify node settings. (Originally this PR also contained GUI changes to unify bitcoin-qt and bitcoind persistent settings and call these methods, but the GUI commits have been dropped from this PR and moved to bitcoin-core/gui/pull/602) ACKs for top commit: vasild: ACK f9fdcec7e932843a91ddf7f377e00bd2a6efb82a hebasto: re-ACK f9fdcec7e932843a91ddf7f377e00bd2a6efb82a, only a function renamed since my recent [review](https://github.com/bitcoin/bitcoin/pull/15936#pullrequestreview-979324357). Tree-SHA512: 4cac853ee29be96d2ff38404165b9dfb7c622b2a9c99a15979596f3484ffde0da3d9c9c372677dff5119ca7cffa6383d81037fd9889a29cc9285882a8dc0c268
2022-05-26Merge bitcoin/bitcoin#25216: Doc: Fix parameter in hwm example blocklaanwj
43ff37f60eac1f867c798c1e3f18cb402f7ee795 Update zmq.md: Fix parameter in hwm example block (mutatrum) Pull request description: Looks like a copy/paste error when `zmqpubsequence` was introduced. ACKs for top commit: theStack: ACK 43ff37f60eac1f867c798c1e3f18cb402f7ee795 Tree-SHA512: 4d0abbba4e9fd0adc2824d0804891d677d56216d245ed0d3bffbaf76042c650edf68975d1ec4728328e69421c024fe6f8800c7e0a497934082206ea4f15cc517
2022-05-26Merge bitcoin/bitcoin#25210: doc: remove misleading AreInputsStandard() commentlaanwj
be6d4315c150646cf672778e9232f086403e95df doc: remove misleading AreInputsStandard() comment (James O'Beirne) Pull request description: This check isn't any longer just about bad pay-to-script-hash inputs; it also excludes any kind of nonstandard input, unknown witness versions, coinbases, etc. ACKs for top commit: laanwj: ACK be6d4315c150646cf672778e9232f086403e95df dunxen: ACK be6d431 jonatack: ACK be6d4315c150646cf672778e9232f086403e95df Tree-SHA512: 1c4befadff6a7b5789901ca2a2cc39adc35c688f7e3c093ab5292123f9193ce078731016b773b3d05f7004ff01ee62f23f8362ae8d05134d41dc097ba094a42b
2022-05-26Merge bitcoin/bitcoin#25202: log: Use severity-based logging for ↵laanwj
leveldb/libevent messages, reverse LogPrintLevel order c4e77177276ea2b79c4675cb2678ee2cc757b743 refactor: Change LogPrintLevel order to category, severity (laanwj) ce920713bf0810614c2c0c994511b50d4f660bce leveldb: Log messages from leveldb with category and debug level (laanwj) 18ec120bb9e1fc9d27d2419da4c580bd3cde7e86 http: Use severity-based logging for messages from libevent (laanwj) bd971bffb02c7b06aac9a479f7e5ed8f71de2bec logging: Unconditionally log levels >= WARN (laanwj) Pull request description: Log messages from leveldb and libevent libraries in the severity+level based log format introduced in bitcoin/bitcoin#24464. Example of messages before: ``` 2022-05-24T18:11:57Z [libevent] libevent: event_add: event: 0x55da963fcc10 (fd 10), EV_READ call 0x7f1c7a254620 2022-05-24T18:11:57Z [libevent] libevent: Epoll ADD(1) on fd 10 okay. Old events were 0; read change was 1 (add); write change was 0 (none); close change was 0 (none) 2022-05-24T18:12:08Z leveldb: Generated table #609127@1: 6445 keys, 312916 bytes 2022-05-24T18:12:08Z leveldb: Generated table #609128@1: 5607 keys, 268548 bytes 2022-05-24T18:12:08Z leveldb: Generated table #609129@1: 189 keys, 9384 bytes 2022-05-24T18:12:08Z leveldb: Generated table #609130@1: 293 keys, 13818 bytes ``` Example of messages after: ``` 2022-05-24T17:59:52Z [libevent:debug] event_add: event: 0x5652f44dac10 (fd 10), EV_READ call 0x7f210f2e6620 2022-05-24T17:59:52Z [libevent:debug] Epoll ADD(1) on fd 10 okay. Old events were 0; read change was 1 (add); write change was 0 (none); close change was 0 (none) 2022-05-24T17:59:53Z [leveldb:debug] Recovering log #1072 2022-05-24T17:59:53Z [leveldb:debug] Level-0 table #1082: started 2022-05-24T17:59:53Z [leveldb:debug] Level-0 table #1082: 193 bytes OK 2022-05-24T17:59:53Z [leveldb:debug] Delete type=3 #1070 2022-05-24T17:59:53Z [leveldb:debug] Delete type=0 #1072 ``` The first commit changes it so that messages with level Warning and Error are always logged independent of the `-debug` setting. I think this is useful to make sure warnings and errors, which tend to be important, are not lost. In the future this should be made more configurable. Last commit changes LogPrintLevel argument order to category, severity: This is more consistent with the other functions, as well as with the logging output itself. If we want to make this change, we should do it before it's all over the place. ACKs for top commit: jonatack: Tested ACK c4e77177276ea2b79c4675cb2678ee2cc757b743 Tree-SHA512: ea48a1517f8c1b23760e59933bbd64ebf09c32eb947e31b8c2696b4630ac1f4303b126720183e2de052bcede3a17e475bbf3fbb6378a12b0fa8f3582d610930d
2022-05-26scripts and tools: update lint-logs.py to detect LogPrintLevel()Jon Atack
and add WalletLogPrintf() (already detected) to the lint-logs.py suggestion Co-authored-by: laanwj <126646+laanwj@users.noreply.github.com>
2022-05-26Merge bitcoin/bitcoin#25132: consensus: Add BIP-341 specified constraints in ↵laanwj
`ComputeTaprootMerkleRoot` bd7c5e2f0a1b572decdf3a4bafe4ee990e1f4953 Add BIP-341 specified constraints to `ComputeTaprootMerkleRoot` (David Bakin) Pull request description: [**N.B.:** This PR **_does not change the consensus_**. It only adds `assert` statements according to the current consensus in consensus-sensitive code (`interpreter.cpp`). So that's why the bot added the "consensus" tag and I prefixed the PR title with "consensus".] BIP 341 specifies [constraints on the size of the control block _c_ used to compute the taproot merkle root](https://github.com/bitcoin/bips/blob/master/bip-0341.mediawiki#script-validation-rules). > The last stack element is called the control block _c_, and must have length _33 + 32m_, for a value of _m_ that is an integer between 0 and 128, inclusive. Fail if it does not have such a length. The actual merkle root is computed in `ComputeTaprootMerkleRoot` ([interpreter.cpp@1833](https://github.com/bitcoin/bitcoin/blob/master/src/script/interpreter.cpp#L1833)) - this code does _not_ check these constraints. All the callers do check the constraints before calling `ComputeTaprootMerkleRoot`. But in the future there may be more callers, and these checks may be inadvertently omitted at those future calls. Also, code at/near the current call sites may also change and skip these checks. Therefore _this PR adds those checks as `asserts` directly in `ComputeTaprootMerkleRoot`_ to help prevent that error. No unit tests provided: they'd have to be death tests as these are `assert` statements which raise `SIGABRT` and kill the program. Boost Test has a way to implement death tests (see the in-progress draft PR #25097 at [this code (you may have to click to expand the diff)](https://github.com/bitcoin/bitcoin/pull/25097/files#diff-21483d0e032747850208f21325b29cde89e9c1f55f83a7a166a388cc5c27115aR1089) and could be added here if desired by reviewers. Current callers of `ComputeTaprootMerkleRoot`: - `InferTaprootTree` ([standard.cpp@1552](https://github.com/bitcoin/bitcoin/blob/master/src/script/standard.cpp#L546)) - `VerifyTaprootCommittment` ([interpreter.cpp@1859](https://github.com/bitcoin/bitcoin/blob/master/src/script/interpreter.cpp#L1859)) does a partial check, but it is called from `VerifyWitnessProgram` ([interpreter.cpp@1922](https://github.com/bitcoin/bitcoin/blob/master/src/script/interpreter.cpp#L1918)) where a full check is done ACKs for top commit: sipa: ACK bd7c5e2f0a1b572decdf3a4bafe4ee990e1f4953 theStack: ACK bd7c5e2f0a1b572decdf3a4bafe4ee990e1f4953 Tree-SHA512: cec5aebfdf9fc9d13011abdf8427c934edf1df1ffc32b3e7cc5580f12809f24cf83e64ab0c843fabfce3591873bfcfa248277b30820fd786684319a196e4e550
2022-05-25Merge bitcoin/bitcoin#25213: fuzz: fix crash at coinselection, add missing ↵Andrew Chow
fee rate. c97e961d4652acfc55fd50273cf0a20beb4857e0 fuzz: coinselection, add missing fee rate. (furszy) Pull request description: Fixing https://github.com/bitcoin/bitcoin/pull/25083#issuecomment-1136774756. Without the fee rate, 'GroupOutputs' will crash at group insertion time `OutputGroup::Insert` because now `output.GetEffectiveValue()` asserts that the value exists. ACKs for top commit: achow101: ACK c97e961d4652acfc55fd50273cf0a20beb4857e0 ishaanam: ACK c97e961d4652acfc55fd50273cf0a20beb4857e0 Xekyo: ACK c97e961d4652acfc55fd50273cf0a20beb4857e0 brunoerg: ACK c97e961d4652acfc55fd50273cf0a20beb4857e0 Tree-SHA512: f495886a5078842103e0f05a9018d7cb491967d8362f893dd7b21132b98e94321df860c50acb69e84d7232779f5915322ca6b5f99908e05e0480012580ee9d5d
2022-05-25multiprocess build cleanup: comment on manual dependenciesRyan Ofsky
Also move manual dependency closer to actual build target
2022-05-25Update zmq.md: Fix parameter in hwm example blockmutatrum
2022-05-25Add BIP-341 specified constraints to `ComputeTaprootMerkleRoot`David Bakin
BIP 341 specifies constraints on the size of the control block _c_ used to compute the taproot merkle root. > The last stack element is called the control block _c_, and must have > length _33 + 32m_, for a value of m that is an integer between 0 and > 128, inclusive. Fail if it does not have such a length. (See BIP-341 "Script Validation Rules" here: https://github.com/bitcoin/bips/blob/master/bip-0341.mediawiki#script-validation-rules)
2022-05-25fuzz: coinselection, add missing fee rate.furszy
Otherwise, 'GroupOutputs' will crash at group insertion time (output.GetEffectiveValue() asserts that the value exists).
2022-05-25multiprocess build fix: ipc/capnp/init.capnp.h: No such file or directoryRyan Ofsky
Error was reported by SatoriHoshiAiko in https://github.com/bitcoin/bitcoin/issues/25207 and happens unpredictably because make doesn't always build dependencies in the same order. The source file src/ipc/capnp/protocol.cpp includes some generated headers so needs to have an explicit dependency specified in the makefile so the headers will be generated before the file is compiled. #19160 added the explicit dependency, but it was incorrect because it referred to an old file path from before the source file was renamed (ipc.cpp -> protocol.cpp)
2022-05-25doc: remove misleading AreInputsStandard() commentJames O'Beirne
This check isn't any longer just about bad pay-to-script-hash inputs; it also excludes any kind of nonstandard input, unknown witness versions, coinbases, etc.
2022-05-25Merge bitcoin/bitcoin#25197: contrib: Remove keys that are no longer used ↵laanwj
for merging d4b3dc5b0a726cc4cc7a8467be43126e78f841cf contrib: Remove keys that are no longer used for merging (Hennadii Stepanov) Pull request description: See: - https://bitcoin-irc.chaincode.com/bitcoin-core-dev/2021-10-21#726591 - https://bitcoin-irc.chaincode.com/bitcoin-core-dev/2021-12-09#750000 Also updated `trusted-git-root` to be right after **meshcollider**'s last merge. The latest similar change was bitcoin/bitcoin#7713. A related discussion on [IRC](https://bitcoin-irc.chaincode.com/bitcoin-core-dev/2021-10-22#727090): > [12:28](https://bitcoin-irc.chaincode.com/bitcoin-core-dev/2021-10-22#727090) \<MarcoFalke> jonasschnelli: I was about to ask you whether you planned to remove your fingerprint from the "trusted-keys" for merging, but it looks like this will break verify-commits ... > [12:31](https://bitcoin-irc.chaincode.com/bitcoin-core-dev/2021-10-22#727091) \<laanwj> you would also have a add all his merge commits to exceptions, i guess > [12:32](https://bitcoin-irc.chaincode.com/bitcoin-core-dev/2021-10-22#727092) \<laanwj> or patch the script to allow different key for different ranges of commits > [13:15](https://bitcoin-irc.chaincode.com/bitcoin-core-dev/2021-10-22#727118) \<jonasschnelli> MarcoFalke: I had no plan to remove my keyid,… would that make sense and how would you fix verify commits? > [13:16](https://bitcoin-irc.chaincode.com/bitcoin-core-dev/2021-10-22#727119) \<jonasschnelli> Ideally, we should set en expiration date next to those keyid ACKs for top commit: laanwj: ACK d4b3dc5b0a726cc4cc7a8467be43126e78f841cf Tree-SHA512: 6c23c932288b56b546a9ba45288205fae063e3f98ff308393acffd5d79eb5097417de1c3d8e865a3f66734740ca2388b2452c3c810e45cdf3b15ccfa215f574e
2022-05-25refactor: Change LogPrintLevel order to category, severitylaanwj
This is more consistent with the other functions, as well as with the logging output itself. If we want to make this change, we should do it before it's all over the place.
2022-05-25leveldb: Log messages from leveldb with category and debug levellaanwj
2022-05-25http: Use severity-based logging for messages from libeventlaanwj
Map libevent's severity to our own severity level for logging.
2022-05-25logging: Unconditionally log levels >= WARNlaanwj
Messages with level `WARN` or higher should be logged even when the category is not provided with `-debug=`, to make sure important warnings are not lost.
2022-05-25Get time less often in AddrManImpl::ResolveCollisions_()MarcoFalke
This makes the code less verbose. Also, future changes that change how to get the time are less verbose. Moreover, GetAdjustedTime() might arbitrarily change the value during the execution of this function. For example, the system time advances over a second boundary, or the network adjusts the time arbitrarily. Most of the time however the value will not change, so it seems better to always lock the value in this scope for clarity.
2022-05-25Merge bitcoin/bitcoin#25192: test: add coverage for unknown value to ↵MacroFake
-blockfilterindex 295ff61934f8adea04dabb47695070e2cfd0548e test: add coverage for unknown -blockfilterindex (brunoerg) Pull request description: This PR adds test coverage for the following init error: https://github.com/bitcoin/bitcoin/blob/44037a29129a830fd9c9580f0818387756cfd7d3/src/init.cpp#L844 Passing an unknown value to -blockfilterindex should throw an error. ACKs for top commit: dunxen: cr-ACK 295ff61 Tree-SHA512: 1444903cf0696406c485ce0575f951d527fe7d699094d5845622c0b57c954d6d7dcf1e78ef0c4e8b9b26f53b79583f07fec0e8d8e7f04aa744d2a8cd98329db9
2022-05-25Merge bitcoin/bitcoin#24757: build, ci: add `DEBUG_LOCKCONTENTION` to ↵fanquake
--enable-debug and CI bd5dbc30dbef10f0fb4508b461e77787213aa0de doc: update developer notes wrt --enable-debug and DEBUG_LOCKCONTENTION (Jon Atack) 345647c4da12b5a0f9c6fd7e519df743264f5611 ci: add DEBUG_LOCKCONTENTION to CI task containing DEBUG_LOCKORDER (Jon Atack) 247d17033f2727e9f7d5dddecaea37c46f1230ee build: add DEBUG_LOCKCONTENTION to --enable-debug configuration (Jon Atack) Pull request description: - Add `DEBUG_LOCKCONTENTION` flag to the `--enable-debug` configuration - Add `DEBUG_LOCKCONTENTION` to the native tsan CI task that contains `DEBUG_LOCKORDER` (verified that the CI has all logging categories enabled by default, except libevent and leveldb) - Update the developer notes that `--enable-debug` configures `DEBUG_LOCKCONTENTION` Related to https://github.com/bitcoin/bitcoin/issues/24709. Top commit has no ACKs. Tree-SHA512: 8e9c068d9a4841ad1ab08a2bf4ce96d6fee195e458f6802852cba0d71deb9a485059d355ac8bd1fc15410437f19503b77fc425bf53a1d48dc82a43a979daad17
2022-05-24Merge bitcoin/bitcoin#24464: logging: Add severity level to logslaanwj
e11cdc930375eaec8d737e116138b2f2018c099f logging: Add log severity level to net.cpp (klementtan) a8290649a6578df120a71c9493acdf071e010d96 logging: Add severity level to logs. (klementtan) Pull request description: **Overview**: This PR introduces a new macro, `LogPrintLevel`, that allows developers to add logs with the severity level. Additionally, it will also print the log category if it is specified. Sample log: ``` 2022-03-04T16:41:15Z [opencon] [net:debug] trying connection XX.XX.XXX.XXX:YYYYY lastseen=2.7hrs ``` **Motivation**: This feature was suggested in #20576 and I believe that it will bring the following benefits: * Allow for easier filtering of logs in `debug.log` * Can be extended to allow users to select the minimum level of logs they would like to view (not in the scope of this PR) **Details**: * New log format. `... [category:level]...`. ie: * Do not print category if `category == NONE` * Do not print level if `level == NONE` * If `category == NONE` and `level == NONE`, do not print any fields (current behaviour) * Previous logging functions: * `LogPrintf`: no changes in log as it calls `LogPrintf_` with `category = NONE` and `level = NONE` * `LogPrint`: prints additional `[category]` field as it calls `LogPrintf_` with `category = category` and `level = NONE` * `net.cpp`: As a proof of concept, updated logs with obvious severity (ie prefixed with `Warning/Error:..`) to use the new logging with severity. **Testing**: * Compiling and running `bitcoind` with this PR should instantly display logs with the category name (ie `net/tor/...`) * Grepping for `net:debug` in `debug.log` should display the updated logs with severity level: <details> <summary>Code</summary> ``` $ grep "net:debug" debug.log 2022-03-04T16:41:15Z [opencon] [net:debug] trying connection XXX:YYY lastseen=2.7hrs 2022-03-04T16:41:16Z [opencon] [net:debug] trying connection XXX:YYY lastseen=16.9hrs 2022-03-04T16:41:17Z [opencon] [net:debug] trying connection XXX:YYY lastseen=93.2hrs 2022-03-04T16:41:18Z [opencon] [net:debug] trying connection XXX:YYY lastseen=2.7hrs ``` </details> ACKs for top commit: laanwj: Code review and lightly tested ACK e11cdc930375eaec8d737e116138b2f2018c099f Tree-SHA512: 89a8c86667ccc0688e5acfdbd399aac1f5bec9f978a160e40b0210b0d9b8fdc338479583fc5bd2e2bc785821363f174f578d52136d228e8f638a20abbf0a568f
2022-05-24windeploy: Renewed windows code signing certificateAndrew Chow
2022-05-24Merge bitcoin/bitcoin#24410: [kernel 2a/n] Split hashing/index ↵laanwj
`GetUTXOStats` codepaths, decouple from `coinstatsindex` 664a14ba7ccb40aa82d35a59831acd35db1897a6 coinstats: Move GetUTXOStats to rpc/blockchain (Carl Dong) f1006875665ffe8ff5da8185effe25b860743b4e kernel: Use ComputeUTXOStats in validation (Carl Dong) faa52387e8e4856445b1cfc9b5e072ce8f690f36 style-only: Rearrange using decls after scripted-diff (Carl Dong) f329a9298c06ffe74b9e9fbc07bfe6d282fef9cb scripted-diff: Move src/kernel/coinstats to kernel:: (Carl Dong) 0e54456f0498e52131f8ae0c76b4dfe25f45b076 Use only kernel/coinstats.h in index/coinstatsindex.h (Carl Dong) 80970985c965f79b8c376c8a922497e385445dd8 coinstats: Split node/coinstats.h to kernel/coinstats.h (Carl Dong) 35f73ce4b2efd7341fe55f77b334f27ad8aad090 coinstats: Move hasher codepath to kernel/coinstats (Carl Dong) b7634fe02b6b030f5d62502c73db84ba9a276640 Move logic from LookupUTXOStatsWithIndex to CoinStatsIndex::LookUpStats (Carl Dong) 1352e410a5b84070279ff28399083cb3ab278593 coinstats: Separate hasher/index lookup codepaths (Carl Dong) 524463daf6a10b20a4e20116a68101a684929eda coinstats: Return purely out-param CCoinsStats (Carl Dong) 46eb9fc56a296a2acea10ec7e5bf7b1827f73c45 coinstats: Extract index_requested in-member to in-param (Carl Dong) a789f3f2b878e1236f8e043a8bb1ffb1afc1b673 coinstats: Extract hash_type in-member to in-param (Carl Dong) 102294898d708b7adc0150aba8e500a4aa19bc1c includes: Remove rpc/util.h -> node/coinstats.h (Carl Dong) 0848db9c35d9eae4d68cbdbef68c337656f3c906 fuzz: Remove useless GetUTXOStats fuzz case (Carl Dong) 52b1939993771d0a8a718ca1667241872de8241a kernel: Remove unnecessary blockfilter{index,}.cpp (Carl Dong) Pull request description: Part of: #24303 Depends on: #24322 The `GetUTXOStats` function has 2 codepaths: - One which queries the `CoinStatsIndex` for the UTXO hash - One which actually performs the hashing For `libbitcoinkernel`, the only place where we call `GetUTXOStats` is in `PopulateAndValidateSnapshots`, which uses the `SHA256D` hash, and is therefore unable to use the `CoinStatsIndex` since that only provides `MuHash` hashes. Not that I think indices necessarily belong in `libbitcoinkernel` anyway. This PR separates these 2 aforementioned codepaths of `GetUTXOStats`, uses the hashing codepath in `PopulateAndValidateSnapshots`, and removes the need to link in `index/coinstatsindex.cpp` and `node/coinstats.cpp`. ----- Logistically, this PR: - Extracts out the `index_requested` and `hash_type` members of `CoinStats`, which served as "in-params" to `GetUTXOStats` embedded within the `CoinStats` struct. This allows `CoinStats` to only consist of "out-param" members, and be returned by `GetUTXOStats` without needing to be an "in-out" param - Introduce the purely virtual `UTXOHashers` class, with 3 implementations: `SHA256DHasher`, `MuHashHasher`, and `NullHasher`. These replace the existing template-based polymorphism. - Split `GetUTXOStats` into: - `CalculateUTXOStatsWithHasher(UTXOHasher&, ...)`, and - `LookupUTXOStatsWithIndex(CoinStatsIndex&, ...)` - Use `CalculateUTXOStatsWithHasher` directly where appropriate (`src/validation.cpp` and `src/fuzz`) - Move `GetUTXOStats` to `rpc/blockchain`, which is the only place that depends on `GetUTXOStats`'s weird fallback behaviour - Move `LookupUTXOStatsWithIndex` to `index/coinstatsindex` Code organization: - `src/` - `kernel/` → only contains the hashing codepath - `coinstats.cpp` → hashing codepath implementations - `coinstats.h` → header for `kernel/coinstats.cpp` - `index/` → only contains the index codepath - `coinstatsindex.cpp` → index codepath implementations - `coinstatsindex.h` - `validation.cpp` → only uses the hashing codepath - `rpc/blockchain.cpp` → uses both the hashing and index codepath, old `GetUTXOStats` fallback logic moved here as static - `test/fuzz/coins_view.cpp` → only uses the hashing codepath TODOs: - [x] Commit messages could be fleshed out more Would love any feedback! ACKs for top commit: laanwj: Code review ACK 664a14ba7ccb40aa82d35a59831acd35db1897a6 Tree-SHA512: 18722c7bd279174d2d1881fec33ea04a9b261aae1c12e998cf434ef297d8ded47de69c526c8033a2ba7abc93ba3d2ff5faf4ce05e8888c725c31cf885ce3ef73
2022-05-24contrib: Remove keys that are no longer used for mergingHennadii Stepanov
See: https://bitcoin-irc.chaincode.com/bitcoin-core-dev/2021-10-21#726591 https://bitcoin-irc.chaincode.com/bitcoin-core-dev/2021-12-09#750000 Also updated trusted-git-root to be right after meshcollider's last merge.
2022-05-24Merge bitcoin-core/gui#593: Getting ready to Qt 6 (8/n). Use ↵Hennadii Stepanov
`QRegularExpression` in `AddressBookSortFilterProxyModel` class e280087946184b37c8a1eb345fae30ebb07f3384 qt: Use `QRegularExpression` in `AddressBookSortFilterProxyModel` class (Hennadii Stepanov) 5c5d8f2465be70cbc256ec59913ac0a3c7c958be qt, test: Add tests for searching in `AddressBookPage` dialog (Hennadii Stepanov) Pull request description: This is a step in [migration](https://github.com/bitcoin/bitcoin/pull/24798) to Qt 6. Related: - bitcoin-core/gui#578 - bitcoin-core/gui#585 No behavior change. To ensure this, tests have been added. ACKs for top commit: hebasto: > tACK [e280087](https://github.com/bitcoin-core/gui/commit/e280087946184b37c8a1eb345fae30ebb07f3384) on Ubuntu 21.10 Qt 5.15.2 promag: Tested ACK e280087946184b37c8a1eb345fae30ebb07f3384 with Qt6 on macOS 12 M1. w0xlt: tACK https://github.com/bitcoin-core/gui/pull/593/commits/e280087946184b37c8a1eb345fae30ebb07f3384 on Ubuntu 21.10 Qt 5.15.2 jarolrod: Tested ACK https://github.com/bitcoin-core/gui/commit/e280087946184b37c8a1eb345fae30ebb07f3384 on M1 mac, x86 mac, x86 Linux with Qt5 and separately with Qt6 Tree-SHA512: 664baacc1504deb2f7fa651ea4a44f3942f5c9058befe4d2ce292beed032d4b1697710cfd10c0909602d8a4a6eeb680414e4a1f56d2038478c1ae2f34965d74f
2022-05-24Merge bitcoin-core/gui#601: refactor: Pass interfaces::Node references to ↵Hennadii Stepanov
OptionsModel constructor 31122aa979c4c9a40e276cfc44243420c367ba4f refactor: Pass interfaces::Node references to OptionsModel constructor (Ryan Ofsky) Pull request description: Giving OptionsModel access to the node interface is needed as part of #602 to get bitcoind and bitcoin-qt to use the same settings instead of different settings. It has been split off from #602 to simplify that PR. Previously these commits were part of bitcoin/bitcoin#15936 and also had some review discussion there. ACKs for top commit: promag: Code review ACK 31122aa979c4c9a40e276cfc44243420c367ba4f. furszy: Code ACK 31122aa9 jarolrod: ACK 31122aa979c4c9a40e276cfc44243420c367ba4f Tree-SHA512: b8529322fd7ba97e19864129e6cf5f9acc58c124f2e5a7c50aca15772c8549de3c24e8b0c27e8cf2c06fd26529e9cdb898b0788a1de3cbfdfbdd3f85c9f0fe69
2022-05-24Merge bitcoin/bitcoin#25149: refactor: Add thread safety annotation to ↵MacroFake
`BanMan::SweepBanned()` ab7538832043a6c15e45a178fb6bb6298a00108f refactor: Remove redundant scope in `BanMan::SweepBanned()` (Hennadii Stepanov) 52c0b3e859089c0ac98e7261ded6391b4f8eeeaf refactor: Add thread safety annotation to `BanMan::SweepBanned()` (Hennadii Stepanov) 3919059deb60d6ead9defc9d213a3f0c2ab72e90 refactor: Move code from ctor into private `BanMan::LoadBanlist()` (Hennadii Stepanov) Pull request description: This PR adds a proper thread safety annotation to `BanMan::SweepBanned()`. Also a simple refactoring applied. ACKs for top commit: ajtowns: ACK ab7538832043a6c15e45a178fb6bb6298a00108f w0xlt: ACK https://github.com/bitcoin/bitcoin/pull/25149/commits/ab7538832043a6c15e45a178fb6bb6298a00108f theStack: Code-review ACK ab7538832043a6c15e45a178fb6bb6298a00108f Tree-SHA512: 8699079c308454f3ffe72be2e77f0935214156bd509f9338b1104f8d128bfdd02ee06ee8c1c99b2eefdf317a00edd555d52990caaeb1ed4540eedc1e3c9d1faf
2022-05-23test: add coverage for unknown -blockfilterindexbrunoerg
2022-05-23coinstats: Move GetUTXOStats to rpc/blockchainCarl Dong
rpc/blockchain.cpp is now the only user of the vestigial GetUTXOStats(...). And since GetUTXOStats(...)'s special fallback logic was only really relevant/meant for rpc/blockchain.cpp, we can just move it there.
2022-05-23kernel: Use ComputeUTXOStats in validationCarl Dong
This is the "fruit of our labor" for this patchset. ChainstateManager::PopulateAndValidateSnapshot can now directly call ComputeUTXOStats(...). Our consensus engine is now fully decoupled from all indices. See the src/Makefile.am for some satisfying removals.
2022-05-23style-only: Rearrange using decls after scripted-diffCarl Dong
2022-05-23scripted-diff: Move src/kernel/coinstats to kernel::Carl Dong
Introduces a new kernel:: namespace and move all of src/kernel/coinstats under it. In the verify script, lines like: line="$(grep -n 'namespace node {' -- src/kernel/coinstats.h | tail -n1 | cut -d: -f1)" sed -i -e "${line}s@namespace node {@namespace kernel {@" -- src/kernel/coinstats.h Are intended to replace only the last instance of "namespace node" with "namespace kernel", this is to avoid replacing forward declarations of things inside the node:: namespace. -BEGIN VERIFY SCRIPT- sed -E -i 's@namespace node@namespace kernel@g' -- src/kernel/coinstats.cpp line="$(grep -n 'namespace node {' -- src/kernel/coinstats.h | tail -n1 | cut -d: -f1)" sed -i -e "${line}s@namespace node {@namespace kernel {@" -- src/kernel/coinstats.h line="$(grep -n '// namespace node' -- src/kernel/coinstats.h | tail -n1 | cut -d: -f1)" sed -i -e "${line}s@// namespace node@// namespace kernel@" -- src/kernel/coinstats.h things='(CCoinsStats|CoinStatsHashType|GetBogoSize|TxOutSer|ComputeUTXOStats)' git grep -lE 'node::'"$things" | xargs sed -E -i 's@node::'"$things"'@kernel::\1@g' sed -E -i 's@'"$things"'@kernel::\1@g' -- src/node/coinstats.cpp src/node/coinstats.h sed -E -i 's@BlockManager@node::\0@g' -- src/kernel/coinstats.cpp -END VERIFY SCRIPT-
2022-05-23Use only kernel/coinstats.h in index/coinstatsindex.hCarl Dong
Removes a circular dependency, horray!
2022-05-23coinstats: Split node/coinstats.h to kernel/coinstats.hCarl Dong
Most of this commit is pure-move. After this change: - kernel/coinstats.h -> Contains declarations for: - enum class CoinStatsHashType - struct CCoinsStats - GetBogoSize(...) - TxOutSer(...) - ComputeUTXOStats(...) - node/coinstats.h -> Just GetUTXOStats, which will be removed as we change callers to directly use the hashing/indexing codepaths in future commits.
2022-05-23coinstats: Move hasher codepath to kernel/coinstatsCarl Dong
As mentioned in a previous commit, the hashing codepath can now be moved to a separate file. This decouples callers that only rely on the hashing codepath from the indexing one. This is key for libbitcoinkernel, which needs to have the CoinsStats hashing codepath for AssumeUTXO, but does not wish to be coupled with indexes. Note that only the .cpp file is split in this commit, the header files will be split in a subsequent commit and the #includes to node/coinstats.h will be adjusted to only #include the necessary headers.
2022-05-23Move logic from LookupUTXOStatsWithIndex to CoinStatsIndex::LookUpStatsCarl Dong
The indexing codepath logic in node/coinstats.cpp is simple enough to be moved into CoinStatsIndex::LookUpStats, avoiding an additional layer of function calls. Callers are modified accordingly. Also, add 2 missed BOOST_CHECKs to the coinstatsindex_initial_sync unit test.
2022-05-23coinstats: Separate hasher/index lookup codepathsCarl Dong
Split out ComputeUTXOStats and LookupUTXOStatsWithIndex from GetUTXOStats, since the hashing and indexing codepaths are quite disparate in practice. Also allow add a constructor to CCoinsStats for it to be constructed from a a block height and hash. This is used in both codepaths. Also add a note in GetUTXOStats documenting a behaviour quirk that predates this patchset. [META] This allows the hashing codepath to be moved to a separate file in a future commit, decoupling callers that only rely on the hashing codepath from the indexing one. This is key for libbitcoinkernel, which needs to have the hashing codepath for AssumeUTXO, but does not wish to be coupled with indexes.
2022-05-23coinstats: Return purely out-param CCoinsStatsCarl Dong
In previous commits in this patchset, we removed all in-param members of CCoinsStats. Now that that's done, we can modify GetUTXOStats to return an optional CCoinsStats instead of a status bool. Callers are modified accordingly. In rpc/blockchain.cpp, we discover that GetUTXOStats' status bool when getting UTXO stats for pprev was not checked for error. We fix this as well.
2022-05-23Merge bitcoin/bitcoin#25176: Fix frequent -netinfo JSON errors from missing ↵MacroFake
getpeerinfo#relaytxes a17c5e96b602fed65166037b78d98605e915206b Rename NetinfoRequestHandler::is_block_relay data member to is_tx_relay (Jon Atack) f0bb7db34ce0c8e62dad7d33c1c6b1f49ca81c5e Fix frequent -netinfo JSON errors from null getpeerinfo#relaytxes (Jon Atack) Pull request description: CLI -netinfo frequently returns "error: JSON value is not a boolean as expected" since the merge of #21160, which moved fRelayTxes (renamed to m_relay_txs in that pull) from CNodeStats to CNodeStateStats. This change made getpeerinfo "relaytxes" an optional field that can return UniValue IsNull(). It is the only optional field consumed by -netinfo where the latter didn't already handle that case. See also https://github.com/bitcoin/bitcoin/pull/24691. Also rename the NetinfoRequestHandler::is_block_relay data member to is_tx_relay and inverse its boolean logic. The naming is out of date and incorrect, as lack of request of tx relay does not imply block relay, and a preference for tx relay doesn't imply that block relay isn't happening. Thanks to Marco Falke and Martin Zumsande for their feedback on this. (I may look at reducing the number of optional node stats fields via refactoring at the net processing level, but ongoing refactoring there may make that slow or complicated and this is a one-line fix that works now.) ACKs for top commit: mzumsande: Code review ACK a17c5e96b602fed65166037b78d98605e915206b Tree-SHA512: dc54ce80b78122874a6794555f99e5b328a1574b52bb3e7f974c699c2b759a60ea0807a6483c5bc0414a950d853c0eeeb13dcc1b790d3917c6ee4c9c99fe159f
2022-05-23Merge bitcoin/bitcoin#25015: test: Use permissions from git in lint-files.pyMacroFake
908fb7e2ec37fe68675d38dbfee4df9f861bb2b5 test: Use permissions from git in `lint-files.py` (laanwj) 48d2e80a7479a44b0ab09e87542c8cb7a8f72223 test: Don't use shell=True in `lint-files.py` (laanwj) Pull request description: Improvements to the `lint-files.py` script: - Avoid use of `shell=True`. - Check the permissions in git's metadata instead of in the filesystem. This stops the umask or filesystem from interfering. It's also more efficient as it only needs a single call to `git ls-files`. (what triggered this change was `File "..." contains a shebang line, but has the file permission 775 instead of the expected executable permission 755.` errors running the script locally). ACKs for top commit: vincenzopalazzo: re-tACK https://github.com/bitcoin/bitcoin/commit/908fb7e2ec37fe68675d38dbfee4df9f861bb2b5 Tree-SHA512: 2eaf868c55a9c3508b12658a5b3ac429963fd0551e645332d0ac54be56fefccee95115e4667386df24b46b545593cb0d0bf8c6cecab73f9cb19d37ddf704c614
2022-05-23Merge bitcoin/bitcoin#25083: Set effective_value when initializing a COutputAndrew Chow
6fbb0edac22c63f1b723f731c2601b1d46879a58 Set effective_value when initializing a COutput (ishaanam) Pull request description: Previously in COutput, effective_value was initialized as the absolute value of the txout and the fee as 0. effective_value along with the fee was calculated outside of the COutput constructor and set after the object had been initialized. These changes will allow either the fee or the feerate to be passed in a COutput constructor and the fee and effective_value are calculated and set in the constructor. As a result, AvailableCoins also needs to be passed the feerate when utxos are being spent. When balance is calculated or the coins are being listed and feerate is neither available nor required, AvailableCoinsListUnspent is used instead, which runs AvailableCoins while providing the default value for `feerate`. Unit tests for the calculation of effective value have also been added. ACKs for top commit: achow101: re-ACK 6fbb0edac22c63f1b723f731c2601b1d46879a58 Xekyo: re-ACK 6fbb0edac22c63f1b723f731c2601b1d46879a58 w0xlt: Code Review ACK https://github.com/bitcoin/bitcoin/pull/25083/commits/6fbb0edac22c63f1b723f731c2601b1d46879a58 furszy: Looks good, have been touching this area lately, code review ACK 6fbb0eda. Tree-SHA512: 5943ee4f4b0c1dcfe146f2fc22853e607259d6d53156b80a8a8f4baa70760a8b25ab822777b7f5d21ecb02dac08bdee704a9a918d5660276d6994b19a284b256
generated by cgit v1.2.3 (git 2.26.2) at 2024-11-09 10:16:55 +0000