aboutsummaryrefslogtreecommitdiff
AgeCommit message (Collapse)Author
2021-09-09Merge bitcoin/bitcoin#22925: test: Add missing suppression ↵MarcoFalke
signed-integer-overflow:addrman.cpp facb534c37725ca446fd56d781b70ba26508bd2a test: Add missing suppression signed-integer-overflow:addrman.cpp (MarcoFalke) Pull request description: Steps to reproduce: [crash-d5f88bd8d0d460ffbab217b856b8582600c00503.log](https://github.com/bitcoin/bitcoin/files/7130854/crash-d5f88bd8d0d460ffbab217b856b8582600c00503.log) ``` $ FUZZ=addrman ./src/test/fuzz/fuzz ./crash-d5f88bd8d0d460ffbab217b856b8582600c00503.log INFO: Running with entropic power schedule (0xFF, 100). INFO: Seed: 1257085025 INFO: Loaded 1 modules (379531 inline 8-bit counters): 379531 [0x562577b768a8, 0x562577bd3333), INFO: Loaded 1 PC tables (379531 PCs): 379531 [0x562577bd3338,0x56257819dbe8), ./src/test/fuzz/fuzz: Running 1 inputs 1 time(s) each. Running: ./crash-d5f88bd8d0d460ffbab217b856b8582600c00503.log addrman.cpp:80:14: runtime error: signed integer overflow: 2105390 - -9223372036854775808 cannot be represented in type 'long' #0 0x5625752f0179 in CAddrInfo::IsTerrible(long) const addrman.cpp:80:14 #1 0x56257531917d in CAddrMan::GetAddr_(std::vector<CAddress, std::allocator<CAddress> >&, unsigned long, unsigned long, std::optional<Network>) const addrman.cpp:874:16 #2 0x562574f0251b in CAddrMan::GetAddr(unsigned long, unsigned long, std::optional<Network>) const ./addrman.h:259:9 #3 0x562574eff7ad in addrman_fuzz_target(Span<unsigned char const>) test/fuzz/addrman.cpp:295:26 SUMMARY: UndefinedBehaviorSanitizer: signed-integer-overflow addrman.cpp:80:14 in ACKs for top commit: practicalswift: cr ACK facb534c37725ca446fd56d781b70ba26508bd2a Tree-SHA512: 6368c48be8762c793f760d86caaf37a10caffa08f6903f3667dd08f7f67fade10f385fbffc451ddcbeeecc9fd02526ed97ab9de13398a75fffa55976a99af6b9
2021-09-09Merge bitcoin/bitcoin#22912: ci: Fix merge_script in MSVC taskfanquake
f78cc90524dc15c8981da2a480621c2e47e3c7dd ci: Fix merge_script in MSVC task (Hennadii Stepanov) Pull request description: The new `merge_script` in the MSVC build task does not really exit early when the task is triggered by a non-pr. In the current code https://github.com/bitcoin/bitcoin/blob/e4aa9b15b9f80a08076ad329b473fe9107d9e65e/.cirrus.yml#L104 the `exit 0` command exits from the PowerShell call, not the recent `merge_script`. This cause the next lines https://github.com/bitcoin/bitcoin/blob/e4aa9b15b9f80a08076ad329b473fe9107d9e65e/.cirrus.yml#L105-L107 are executed unconditionally. Here is an excerpt from [CI task log](https://api.cirrus-ci.com/v1/task/4578647416766464/logs/merge.log) for the ["Merge #22915: Remove confusing CAddrDB " commit](https://github.com/bitcoin/bitcoin/commit/896649996bdaa80300fa20027a9789558233268d): ``` ... C:\Users\ContainerAdministrator\AppData\Local\Temp\cirrus-ci-build>call powershell.exe -NoLogo -EncodedCommand ZwBpAHQAIAByAGUAcwBlAHQAIAAtAC0AaABhAHIAZAA= HEAD is now at 896649996 Merge bitcoin/bitcoin#22915: Remove confusing CAddrDB C:\Users\ContainerAdministrator\AppData\Local\Temp\cirrus-ci-build>if 0 NEQ 0 exit /b 0 C:\Users\ContainerAdministrator\AppData\Local\Temp\cirrus-ci-build>call powershell.exe -NoLogo -EncodedCommand aQBmACAAKAAkAGUAbgB2ADoAQwBJAFIAUgBVAFMAXwBQAFIAIAAtAGUAcQAgACQAbgB1AGwAbAApACAAewAgAGUAeABpAHQAIAAwADsAIAB9AA== C:\Users\ContainerAdministrator\AppData\Local\Temp\cirrus-ci-build>if 0 NEQ 0 exit /b 0 C:\Users\ContainerAdministrator\AppData\Local\Temp\cirrus-ci-build>call powershell.exe -NoLogo -EncodedCommand ZwBpAHQAIABmAGUAdABjAGgAIAAkAGUAbgB2ADoAQwBJAFIAUgBVAFMAXwBSAEUAUABPAF8AQwBMAE8ATgBFAF8AVQBSAEwAIAAkAGUAbgB2ADoAQwBJAFIAUgBVAFMAXwBCAEEAUwBFAF8AQgBSAEEATgBDAEgA From https://github.com/bitcoin/bitcoin * branch HEAD -> FETCH_HEAD C:\Users\ContainerAdministrator\AppData\Local\Temp\cirrus-ci-build>if 0 NEQ 0 exit /b 0 C:\Users\ContainerAdministrator\AppData\Local\Temp\cirrus-ci-build>call powershell.exe -NoLogo -EncodedCommand ZwBpAHQAIABtAGUAcgBnAGUAIABGAEUAVABDAEgAXwBIAEUAQQBEAA== Already up to date. C:\Users\ContainerAdministrator\AppData\Local\Temp\cirrus-ci-build>if 0 NEQ 0 exit /b 0 ``` This PR fixes this issue, and makes `merge_script` log more readable. ACKs for top commit: MarcoFalke: Concept ACK f78cc90524dc15c8981da2a480621c2e47e3c7dd Tree-SHA512: c88b115f99f9019a4100a10df051e32c05487612c13105d10873b9cf38965eeca731604d36610ae750cb1f93ba77ce97dca7599fe4984181210d0753be4eb9a0
2021-09-09Merge bitcoin/bitcoin#22922: ci: Fix and enable tests on WindowsMarcoFalke
fa0c194db34e776ddc68d8f585b7d66162c2617c cirrus: Enable tests on windows (MarcoFalke) fadecbd9a4d47957f42672911675c400caeaac24 test: Fix tests on Windows (MarcoFalke) Pull request description: Only a cherry-picked list. `--extended` can be enabled in a follow-up. ACKs for top commit: hebasto: ACK fa0c194db34e776ddc68d8f585b7d66162c2617c, tested locally on Windows 10 Pro 20H2 (build 19042.1165): Tree-SHA512: 47cfbcef7ce5fe0c62b77a1e45ace513c4f0109b1fcfaec94faf9e488fe9430d6ba0e852230021d432847eb1389a4e4b7cf39bf17f7b09bae36af3079e0d7399
2021-09-09Merge bitcoin/bitcoin#22915: Remove confusing CAddrDBMarcoFalke
fade9a1a4db71241ccad03fdacfb626453952963 Remove confusing CAddrDB (MarcoFalke) fa7f77b7d1709bf35808fced0d67b6e97b784d63 Fix addrdb includes (MarcoFalke) fa3f5d0dae2381038439b91ef2af85ec277c8294 Move addrman includes from .h to .cpp (MarcoFalke) Pull request description: Split out from #22762 to avoid having to carry it around in (an)other rebase(s) ACKs for top commit: ryanofsky: Code review ACK fade9a1a4db71241ccad03fdacfb626453952963 lsilva01: Code Review ACK https://github.com/bitcoin/bitcoin/pull/22915/commits/fade9a1a4db71241ccad03fdacfb626453952963 Tree-SHA512: 7615fb0b6235d0c1e6f8cd6263dd18c4d95890567c2b797fe6fce6cb12cc85ce6eacbe07dbb6d81b05d179ef03b42edfd61c940e35a1044ce6d363b54c2dae5c
2021-09-09Merge bitcoin/bitcoin#22390: system: skip trying to set the locale on NetBSDfanquake
fdd71448e78f442ffd93a3a3398a5062eaba9f1b system: skip trying to set the locale on NetBSD (fanquake) Pull request description: Just treat it the same as the other BSDs. Fixes #17379. ACKs for top commit: laanwj: Code review ACK fdd71448e78f442ffd93a3a3398a5062eaba9f1b practicalswift: cr ACK fdd71448e78f442ffd93a3a3398a5062eaba9f1b Tree-SHA512: 5fe0a66f014279ad2683b548692a36af493377fb92d1f28b15dc4feef871190fe08ef40dcc4f5ba21a525fe365c42fb429fe4be0673a1e96db163af587c23204
2021-09-09Merge bitcoin/bitcoin#22903: Enable clang-tidy bugprone-argument-comment and ↵fanquake
fix violations fa57fa1a2e764792b873998ddf38db2ac061dcb6 Enable clang-tidy bugprone-argument-comment and fix violations (MarcoFalke) Pull request description: Named arguments can be dangerous when they are wrong, because they are not enforced by the compiler. Currently there are only minor typos, no actual bugs. Fix the typos and add the `.clang-tidy` file to make it easier to find them in the future. ACKs for top commit: practicalswift: cr ACK fa57fa1a2e764792b873998ddf38db2ac061dcb6 fanquake: ACK fa57fa1a2e764792b873998ddf38db2ac061dcb6 Tree-SHA512: b66f01e0a1e77e56ed8454002176df660cc2cc0947a90785aa33cc5b8003a1f99fd8b2f8f89f2a0bf180ff2c42c031d69e669d127bb557b879c17975275a220b
2021-09-08cirrus: Enable tests on windowsMarcoFalke
2021-09-08test: Add missing suppression signed-integer-overflow:addrman.cppMarcoFalke
2021-09-08test: Fix tests on WindowsMarcoFalke
2021-09-08Merge bitcoin/bitcoin#22916: doc: add missing copyright header to ↵MarcoFalke
getuniquepath.cpp 69a439b8807cb150068d196a0e2fd57fd80057df doc: add missing copyright header to getuniquepath.cpp (fanquake) Pull request description: This was missed in #21052. ACKs for top commit: hebasto: ACK 69a439b8807cb150068d196a0e2fd57fd80057df, but a scripted-diff using `copyright_header.py insert` looks preferable. Tree-SHA512: 1a75ffd93fa8e5e83821e1fe984353422740ebf9e203dc873debf4dffec0e23f55a1e31f806dd2d66087d3620f6dc447af69f9124f0bcc6676ef91ee35374832
2021-09-08doc: add missing copyright header to getuniquepath.cppfanquake
This was missed in #21052.
2021-09-08Merge bitcoin/bitcoin#22879: addrman: Fix format string in deserialize errorfanquake
fab0b55cf060c2b14fae5cee13f0a2dcaebde892 addrman: Fix format string in deserialize error (MarcoFalke) facce4ca44bc206b7656e297a7fa5dfb83a01012 test: Remove useless overwrite (MarcoFalke) Pull request description: The format string is evaluated differently on modern compilers (clang 10 and later, as well as gcc 10 and later). Work around the behaviour change in compilers by pinning the underlying type of the format arguments. Can be tested by observing a failing test when running against master compiled with clang 10 or gcc 10 (or later). ACKs for top commit: jonatack: ACK fab0b55cf060c2b14fae5cee13f0a2dcaebde892 verified the test fails on master as expected only at line 61 (assertion fixed by the code change); the last two test additions pass as expected mzumsande: ACK fab0b55cf060c2b14fae5cee13f0a2dcaebde892 Tree-SHA512: 07462901435107f3bc79098fd7d06446bfe8fe065fffdd35adfcba8f1dd3c499575006557afe7bc74b79d690c5ef7b58e3e031e908161be5529cf237e3b30609
2021-09-07ci: Fix merge_script in MSVC taskHennadii Stepanov
2021-09-07Merge bitcoin/bitcoin#22899: ci: Build and cache static Qt instead of ↵MarcoFalke
downloading a pre-built one 3a68546fd0981e032005d3a5d2ca7b2ab9acc70b ci: Build and cache static Qt instead of downloading a pre-built one (Hennadii Stepanov) Pull request description: This PR makes the MSVC build CI task free of [pre-built static Qt binaries](https://github.com/sipsorcery/qt_win_binary/releases). It uses the approach which is documented in #22890. It takes about 13 minutes to build a static Qt dependency (for 8 CPUs): ![Screenshot from 2021-09-06 08-59-08](https://user-images.githubusercontent.com/32963518/132167857-bce49c74-f258-4468-b45b-75d0cf3c670c.png) with the maximum total time: ![Screenshot from 2021-09-06 08-59-26](https://user-images.githubusercontent.com/32963518/132167881-b84bd4de-38cc-4cb1-b9f7-35642cbea8cc.png) There is an additional benefit of this PR. It is no longer required to build a new static Qt package when a CI Windows image upgrades its building tools, and breaks the compatibility with the recent Qt package. ACKs for top commit: sipsorcery: utACK 3a68546fd0981e032005d3a5d2ca7b2ab9acc70b. Tree-SHA512: 2cf358ccecb26293b52c04158d6d3366ae6257cc3c04262e02234f7d7a03086885c67f0aad5702fcaa6f035fe4a09967a81245c561614875ecd2e90e2e00bbaa
2021-09-07ci: Build and cache static Qt instead of downloading a pre-built oneHennadii Stepanov
2021-09-07Merge bitcoin/bitcoin#22909: ci: Add missed comments and test_bitcoin.exe ↵MarcoFalke
command line option 64015eb01495689d8e02fb82c06eaa3f442ff6e5 ci: Add missed comments and test_bitcoin.exe command line option (Hennadii Stepanov) Pull request description: This PR is a #21551 follow up, and it: - adds missed comments, see https://github.com/bitcoin/bitcoin/pull/21551#discussion_r703342550 - restores missed `-l test_suite` command line option for `test_bitcoin.exe`, see https://github.com/bitcoin/bitcoin/pull/21551#discussion_r703348955 ACKs for top commit: MarcoFalke: cr ACK 64015eb01495689d8e02fb82c06eaa3f442ff6e5 Tree-SHA512: ad1c91544da39a94f033bc55ae5fdaf5774475702edd026635389e68d20e2608cb599dd51f3c1412e0287beef073352e48d9ec005c94df38cfe4fe2d21a94fe3
2021-09-07ci: Add missed comments and test_bitcoin.exe command line optionHennadii Stepanov
2021-09-07Merge bitcoin/bitcoin#21551: ci: Move Windows MSVC build from AppVeyor to CirrusMarcoFalke
97292b19140db32c6d85d63b70382e7bf60a55c4 ci: Drop AppVeyor CI integration (Hennadii Stepanov) 1fb70793b237b9a3a00ff744739e512dd7755937 ci: Add Windows task to Cirrus CI (Hennadii Stepanov) Pull request description: This PR: - gets rid off unreliable AppVeyor CI - places all CI configs in one place - allows to enable functional tests in the future (using a persistent worker) | no populated `vcpkg` cache | populated `vcpkg` cache | |---|---| | ![Screenshot from 2021-09-02 15-47-04](https://user-images.githubusercontent.com/32963518/131846156-9367bffc-9093-40ca-98c3-15db74e24113.png) | ![Screenshot from 2021-09-02 14-06-26](https://user-images.githubusercontent.com/32963518/131833053-a501454d-eecf-463c-a3a4-b89d2a494058.png) | Currently, AppVeyor builds take about 44..48 minutes. ACKs for top commit: sipsorcery: re-ACK 97292b19140db32c6d85d63b70382e7bf60a55c4. Tree-SHA512: 3af50d9fd68eb12f39724810dacf948e4068573b5dfd0dbaeb05d19d4bd6f10bd9a432656dcc32b742684b438d31305eace85c602296d7a1bf84b2f1fcc06f02
2021-09-07Remove confusing CAddrDBMarcoFalke
The class only stores the file path, reading it from a global. Globals are confusing and make testing harder. The method reading from a stream does not even use any class members, so putting it in a class is also confusing.
2021-09-07Fix addrdb includesMarcoFalke
2021-09-07Move addrman includes from .h to .cppMarcoFalke
This is a follow-up to the code move in commit a820e79512b67b1bfda20bdc32b47086d2b0910d
2021-09-07Merge bitcoin/bitcoin#19833: wallet: Avoid locking cs_wallet recursivelyMarcoFalke
5fabde6fadd1b07e981c97f5087d67c4179340ba wallet: AddWalletDescriptor requires cs_wallet lock (João Barbosa) 32d036e8dab5f5b24096d9765236441e7b6a3b34 wallet: GetLabelAddresses requires cs_wallet lock (João Barbosa) Pull request description: This is another small change towards non recursive wallet lock. ACKs for top commit: hebasto: ACK 5fabde6fadd1b07e981c97f5087d67c4179340ba, I have reviewed the code and it looks OK, I agree it can be merged. Tree-SHA512: 00506f0159c56854a171e58a451db8dd9b9f735039697b1cf2ca7f54de61fb51cc1e5eff42265233e041b4b1bfd29c2247496dc4456578e1a23c323bdec2901b
2021-09-07Enable clang-tidy bugprone-argument-comment and fix violationsMarcoFalke
2021-09-07Merge bitcoin-core/gui#398: refactor: Pass WalletModel object to the ↵Hennadii Stepanov
WalletView constructor d319c4dae9ed7d59d71b926e677707fce4194d0c qt, refactor: Replace WalletFrame::addWallet with WalletFrame::addView (Hennadii Stepanov) 92ddc02a16a74e10f24190929f05e2dcf2b55871 qt, refactor: Declare getWalletModel with const and noexcept qualifiers (Hennadii Stepanov) ca0e680bdcaa816c53355777d788a4c8478bb117 qt, refactor: Drop redundant checks of walletModel (Hennadii Stepanov) 404373bc6ac0589e9e28690c9d09114d626a3dc3 qt, refactor: Pass WalletModel object to WalletView constructor (Hennadii Stepanov) Pull request description: An instance of the `WalletView` class without the `walletModel` data member being set is invalid. So, it is better to set it in the constructor. Establishing one more `WalletView` class's invariant in constructor: - allows to drop all of checks of the`walletModel` in member functions - makes reasoning about the code that uses instances of the `WalletView` class easier Possible follow ups could extend this approach to other classes, e.g., `OverviewPage`, `TransactionView`, `ReceiveCoinsDialog`, `SendCoinsDialog`, `AddressBookPage`. ACKs for top commit: ShaMan239: Code review ACK d319c4dae9ed7d59d71b926e677707fce4194d0c promag: Code review ACK d319c4dae9ed7d59d71b926e677707fce4194d0c. jarolrod: ACK d319c4dae9ed7d59d71b926e677707fce4194d0c Tree-SHA512: b0c61f82811bb5aba2738067b53dc9ea4439230d547ce5c8fd85c480d8d70ea15f9942dbf13842383acbce467fba1ab4e132e37c56b654b46ba897301a41066e
2021-09-07ci: Drop AppVeyor CI integrationHennadii Stepanov
2021-09-07ci: Add Windows task to Cirrus CIHennadii Stepanov
2021-09-06Merge bitcoin/bitcoin#22397: build: Fix macOS Apple Silicon build with ↵W. J. van der Laan
miniupnpc and libnatpmp 2445df4eb36ba0d90e1283f36e629e1cf69eeef7 build: Fix macOS Apple Silicon build with miniupnpc and libnatpmp (Hennadii Stepanov) Pull request description: On master (7a49fdc58115845ece3a9890bf9498bee6b559de) the `configure` script does not pick up Homebrew's `miniupnpc` and `libnatpmp` packages on macOS Apple Silicon: ``` % ./configure --with-miniupnpc ... checking for miniupnpc/miniupnpc.h... no checking for miniupnpc/upnpcommands.h... no checking for miniupnpc/upnperrors.h... no ... checking whether to build with support for UPnP... configure: error: "UPnP requested but cannot be built. Use --without-miniupnpc." ``` ``` % ./configure --with-natpmp ... checking for natpmp.h... no ... checking whether to build with support for NAT-PMP... configure: error: NAT-PMP requested but cannot be built. Use --without-natpmp ``` The preferred Homebrew [prefix for Apple Silicon](https://docs.brew.sh/Installation) is `/opt/homebrew`. Therefore, if we do not use `pkg-config` to detect packages, we should set the `CPPFLAGS` and `LDFLAGS` variables for them explicitly. ACKs for top commit: Zero-1729: re-tACK 2445df4eb36ba0d90e1283f36e629e1cf69eeef7 (re-tested on an M1 Machine running macOS 11.4). jarolrod: re-ACK 2445df4eb36ba0d90e1283f36e629e1cf69eeef7 Tree-SHA512: d623d26492f463812bf66ca519847ff4b23d517466b6c51c3caf3642a582d02e5f03ce57915742b29f01bf9bceb731a3978ef9a5fdc82e568bcb62548eda758a
2021-09-06Merge bitcoin/bitcoin#21930: doc: update enumerator naming in developer notesMarcoFalke
77f37f58ad2f349cecb2eda28b415267d3d7d76e doc: update enum naming in developer notes (Jon Atack) Pull request description: Per our current doc, the general rule is we follow the C++ Core Guidelines, which for enumerator naming stipulate: https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Renum-caps ``` Don’t use ALL_CAPS for enumerators Reason: Avoid clashes with macros. ``` but our examples (and often, codebase) are contradictory to it (and perhaps to common usage), which can be confusing when a contributor needs to choose a style to use. This patch: - updates the enumerator examples to snake_case per CPP Core Guidelines - clarifies for contributors that in this project enumerators may be snake_case, PascalCase or ALL_CAPS, and to use what seems appropriate. ACKs for top commit: practicalswift: cr ACK 77f37f58ad2f349cecb2eda28b415267d3d7d76e ryanofsky: ACK 77f37f58ad2f349cecb2eda28b415267d3d7d76e. I think this is good because it modernizes the example and clarifies current conventions. promag: ACK 77f37f58ad2f349cecb2eda28b415267d3d7d76e. Tree-SHA512: 7facc607fe5e1abab0f635864340143f13c2e4bb074eb17eac7d829dcd0cf244c5c617fc49d35e8774e8af1fa1205eeebe0cca81f538a2a61f6a7ba200878bc6
2021-09-06Merge bitcoin/bitcoin#22791: init: Fix asmap/addrman initialization order bugMarcoFalke
724c4975622bc22cedc3f3814dfc8e66cf8371f7 [fuzz] Add ConsumeAsmap() function (John Newbery) 5840476714ffebb2599999c85a23b52ebcff6090 [addrman] Make m_asmap private (John Newbery) f9002cb5dbd573cd9ca200de21319fa296e26055 [net] Rename the copyStats arg from m_asmap to asmap (John Newbery) f572f2b2048994b3b50f4cfd5de19e40b1acfb22 [addrman] Set m_asmap in CAddrMan initializer list (John Newbery) 593247872decd6d483a76e96d79433247226ad14 [net] Remove CConnMan::SetAsmap() (John Newbery) 50fd77045e2f858a53486b5e02e1798c92ab946c [init] Read/decode asmap before constructing addrman (John Newbery) Pull request description: Commit 181a1207 introduced an initialization order bug: CAddrMan's m_asmap must be set before deserializing peers.dat. The first commit restores the correct initialization order. The remaining commits make `CAddrMan::m_asmap` usage safer: - don't reach into `CAddrMan`'s internal data from `CConnMan` - set `m_asmap` in the initializer list and make it const - make `m_asmap` private, and access it (as a reference to const) from a getter. This ensures that peers.dat deserialization must happen after setting m_asmap, since m_asmap is set during CAddrMan construction. ACKs for top commit: mzumsande: Tested ACK 724c4975622bc22cedc3f3814dfc8e66cf8371f7 amitiuttarwar: code review but utACK 724c497562 naumenkogs: utACK 724c4975622bc22cedc3f3814dfc8e66cf8371f7 vasild: ACK 724c4975622bc22cedc3f3814dfc8e66cf8371f7 MarcoFalke: review ACK 724c4975622bc22cedc3f3814dfc8e66cf8371f7 👫 Tree-SHA512: 684a4cf9e3d4496c9997fb2bc4ec874809987055c157ec3fad1d2143b8223df52b5a0af787d028930b27388c8efeba0aeb2446cb35c337a5552ae76112ade726
2021-09-06Merge bitcoin/bitcoin#22848: MOVEONLY: Expose BanMapToJson / BanMapFromJsonMarcoFalke
6919c823cbce92248647880fb1d912828449ae57 MOVEONLY: Expose BanMapToJson / BanMapFromJson (Russell Yanofsky) Pull request description: This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/projects/10). --- CSubNet serialization code that was removed in #22570 fa4e6afdae7b82df638b60edf37ac36d57a8cb4f was needed by multiprocess code to share ban map between gui and node processes. Rather than adding it back, use suggestion from MarcoFalke https://github.com/bitcoin/bitcoin/pull/10102#discussion_r690922929 to use JSON serialization. This requires making BanMapToJson / BanMapFromJson functions public. ACKs for top commit: promag: reACK 6919c823cbce92248647880fb1d912828449ae57. Tree-SHA512: ce909a61b7869d16cf2e9f91b643dd9d2604efc5777703d3b77a4c40cb0ccdd20396ba87b1ec85aade142e12ff9ea4c95c7155840354873579565471779f5a33
2021-09-06Merge bitcoin/bitcoin#22736: log, sync: change lock contention from ↵MarcoFalke
preprocessor directive to log category 7e698732836121912f179b7c743a72dd6fdffa72 sync: remove DEBUG_LOCKCONTENTION preprocessor directives (Jon Atack) 9b08006bc502e67956d6ab518388fad6397cac8d log, sync: improve lock contention logging and add time duration (Jon Atack) 3f4c6b87f1098436693c4990f2082515ec0ece26 log, timer: add timing macro in usec LOG_TIME_MICROS_WITH_CATEGORY (Jon Atack) b7a17444e0746c562ae97b26eba431577947b06a log, sync: add LOCK logging category, apply it to lock contention (Jon Atack) Pull request description: To enable lock contention logging, `DEBUG_LOCKCONTENTION` has to be defined at compilation. Once built, the logging is not limited to a category and is high frequency, verbose and in all-caps. With these factors combined, it seems likely to be rarely used. This patch: - adds a `lock` logging category - adds a timing macro in microseconds, `LOG_TIME_MICROS_WITH_CATEGORY` - updates `BCLog::LogMsg()` to omit irrelevant decimals for microseconds and skip unneeded code and math - improves the lock contention logging, drops the all-caps, and displays the duration in microseconds - removes the conditional compilation directives - allows lock contentions to be logged on startup with `-debug=lock` or at run time with `bitcoin-cli logging '["lock"]'` ``` $ bitcoind -signet -debug=lock 2021-09-01T12:40:01Z LockContention: cs_vNodes, net.cpp:1920 started 2021-09-01T12:40:01Z LockContention: cs_vNodes, net.cpp:1920 completed (4μs) 2021-09-01T12:40:01Z LockContention: cs_vNodes, net.cpp:1302 started 2021-09-01T12:40:01Z LockContention: cs_vNodes, net.cpp:1302 completed (4μs) 2021-09-01T12:40:02Z LockContention: cs_vNodes, net.cpp:2242 started 2021-09-01T12:40:02Z LockContention: cs_vNodes, net.cpp:2242 completed (20μs) 2021-09-01T12:43:04Z LockContention: ::cs_main, validation.cpp:4980 started 2021-09-01T12:43:04Z LockContention: ::cs_main, validation.cpp:4980 completed (3μs) $ bitcoin-cli -signet logging "lock": true, $ bitcoin-cli -signet logging [] '["lock"]' "lock": false, $ bitcoin-cli -signet logging '["lock"]' "lock": true, ``` I've tested this with Clang 13 and GCC 10.2.1, on Debian, with and without `--enable-debug`. ACKs for top commit: hebasto: re-ACK 7e698732836121912f179b7c743a72dd6fdffa72, added a contention duration to the log message since my [previous](https://github.com/bitcoin/bitcoin/pull/22736#pullrequestreview-743764606) review. theStack: re-ACK 7e698732836121912f179b7c743a72dd6fdffa72 🔏 ⏲️ Tree-SHA512: c4b5eb88d3a2c051acaa842b3055ce30efde1f114f61da6e55fcaa27476c1c33a60bc419f7f5ccda532e1bdbe70815222ec2b2b6d9226f29c8e94e598aacfee7
2021-09-06Merge bitcoin/bitcoin#22841: ci: Fuzz with -ftrivial-auto-var-init=patternMarcoFalke
fa0a5fa744108d81bee9600c80bfda6ca11e5255 ci: Fuzz with -ftrivial-auto-var-init=pattern (MarcoFalke) Pull request description: This makes memory bugs deterministic. `-ftrivial-auto-var-init=pattern` is incompatible with other memory sanitizers (like valgrind and msan), but that is irrelevant here, because the address sanitizer in this fuzz CI config is already incompatible with them. `-ftrivial-auto-var-init=pattern` goes well with `-fsanitize=bool` and `-fsanitize=enum`, but those are already enabled via `-fsanitize=undefined`. See https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html#available-checks ACKs for top commit: practicalswift: cr ACK fa0a5fa744108d81bee9600c80bfda6ca11e5255 Tree-SHA512: ed6be953cd99eadb1ba245ba30170747eff66be54d2773c8d26a3a6aee0fdcd6967c596f4f4ab1d238de6a6526623dac5211f0ba77f1986639395d7921bdc19f
2021-09-06Merge bitcoin/bitcoin#22894: netinfo: clarify client and server versions in ↵MarcoFalke
header e952d7557eaf2610e302e9d70381ef057d07f6bf netinfo: clarify client and server versions in header (Jon Atack) Pull request description: Clarify in -netinfo output that both the client and the server versions are provided. before ``` Bitcoin Core v22.0.0rc3 - 70016/Satoshi:22.99.0/ ``` after ``` Bitcoin Core client v22.0.0rc3 - server 70016/Satoshi:22.99.0/ ``` Closes #22873. ACKs for top commit: benthecarman: utACK e952d7557eaf2610e302e9d70381ef057d07f6bf prayank23: ACK https://github.com/bitcoin/bitcoin/pull/22894/commits/e952d7557eaf2610e302e9d70381ef057d07f6bf Zero-1729: tACK e952d7557eaf2610e302e9d70381ef057d07f6bf Tree-SHA512: 3e817892d398aabacb1401fd5b1816c4d4f563b4f8cf1096bdb8b53f7c4ef82d4caee09f5c7724f1fe292f837434a332acefba735152ed24a238bb6f006df909
2021-09-06Merge bitcoin/bitcoin#22861: test: Update test README and lint scriptfanquake
fa0937de35176fdcf637e1af16be4469725e60cc test: Rename bitcoin-util-test.py to util/test_runner.py (MarcoFalke) fa050bbc0ad479063735b0325daa717ded404c8f test: Update test README and lint script (MarcoFalke) Pull request description: * Remove unused `yq` * Update fuzzing docs ACKs for top commit: Saviour1001: ACK <code>[fa0937d](https://github.com/bitcoin/bitcoin/pull/22861/commits/fa0937de35176fdcf637e1af16be4469725e60cc)</code> practicalswift: cr ACK fa0937de35176fdcf637e1af16be4469725e60cc fanquake: ACK fa0937de35176fdcf637e1af16be4469725e60cc Tree-SHA512: 6b148d838e1fcf219ab92e579948e34ea7ce8b4692a3d28bb2a51aaa34cbc7cdbd79e72ce787b485fdf524e5b3521b033692583602d4e379bd160e0e41d66e28
2021-09-05Merge bitcoin/bitcoin#22888: build: Update default platform toolset in ↵MarcoFalke
msvc-autogen.py d1267fdbb01120827785451d6468137a0bed46c5 build: Update default platform toolset in msvc-autogen.py (Hennadii Stepanov) Pull request description: The platform toolset was updated from v141 to v142 in #17364. ACKs for top commit: sipsorcery: ACK d1267fdbb01120827785451d6468137a0bed46c5. Tree-SHA512: 390dc4876948af7f6b9f26eb1e64262f6386c2541a4647a6cb7abd8f1283c63ffbf5efba5e67a8075f0085a0557fc3f1b41e70f3ede0e8cef60d0fe2d6bf3be8
2021-09-05Merge bitcoin/bitcoin#22591: Util: error if settings json exists, but is ↵MarcoFalke
unreadable 2b071265c37da22f15769945fd159b50a14792a3 error if settings.json exists, but is unreadable (Tyler Chambers) Pull request description: If settings.json exists, but is unreadable, we should error instead of overwriting. Fixes #22571 ACKs for top commit: Zero-1729: tACK 2b071265c37da22f15769945fd159b50a14792a3 ShaMan239: tACK 2b071265c37da22f15769945fd159b50a14792a3 prayank23: tACK https://github.com/bitcoin/bitcoin/pull/22591/commits/2b071265c37da22f15769945fd159b50a14792a3 ryanofsky: Code review ACK 2b071265c37da22f15769945fd159b50a14792a3. Thanks for the fix! Note that PR https://github.com/bitcoin-core/gui/pull/379 will change the appearance of dialogs shown in screenshots above. So it could be interesting to test the two PRs together (but current testing seems more than sufficient) theStack: ACK 2b071265c37da22f15769945fd159b50a14792a3 📁 Tree-SHA512: 6f7f96ce8a13213d0335198a2245d127264495c877105058d1503252435915b332a6e55068ac21088f4c0c017d564689f4956213328d5bdee81d73711efc5511
2021-09-05netinfo: clarify client and server versions in headerJon Atack
2021-09-05addrman: Fix format string in deserialize errorMarcoFalke
Also add a regression test.
2021-09-05Merge bitcoin/bitcoin#22859: Replace uses of boost::trim* with ↵fanquake
locale-independent alternatives (#18130 rebased) 696c76d6604c9c4faddfc4b6684e2788bb577ba6 tests: Add TrimString(...) tests (practicalswift) 4bf18b089e1bb1f3ab513cbdf6674bd1074f4621 Replace use of boost::trim_right with locale-independent TrimString (Ben Woosley) 93551862a18965bcee0c883c54807e8726e2f50f Replace use of boost::trim use with locale-independent TrimString (Ben Woosley) Pull request description: This is [#18130 rebased](https://github.com/bitcoin/bitcoin/pull/18130#issuecomment-900158759). > `TrimString` is an existing alternative. > Note `TrimString` uses `" \f\n\r\t\v"` as the pattern, which is consistent with the default behavior of `std::isspace`. See: https://en.cppreference.com/w/cpp/string/byte/isspace ACKs for top commit: jb55: utACK 696c76d6604c9c4faddfc4b6684e2788bb577ba6 practicalswift: ACK 696c76d6604c9c4faddfc4b6684e2788bb577ba6 jonatack: ACK 696c76d6604c9c4faddfc4b6684e2788bb577ba6 theStack: Code-review ACK 696c76d6604c9c4faddfc4b6684e2788bb577ba6 Tree-SHA512: 6a70e3777602dfa65a60353e5c6874eb951e4a806844cd4bdaa4237cad980a4f61ec205defc05a29f9707776835975838f6cc635259c42adfe37ceb02ba9358d
2021-09-04build: Update default platform toolset in msvc-autogen.pyHennadii Stepanov
The platform toolset was updated from v141 to v142 in #17364.
2021-09-04Merge bitcoin/bitcoin#22278: Add LIFETIMEBOUND to CScript where neededfanquake
fa7e6c56f58678b310898a158053ee9ff8b27fe7 Add LIFETIMEBOUND to InitializeChainstate (MarcoFalke) fa5c896724bb359b4b9a3f89580272bfe5980c1b Add LIFETIMEBOUND to CScript where needed (MarcoFalke) Pull request description: Without this, stack-use-after-scope can only be detected at runtime with ASan or code review, both of which are expensive. Use `LIFETIMEBOUND` to turn this error into a compile warning. See https://releases.llvm.org/12.0.0/tools/clang/docs/AttributeReference.html#lifetimebound Example: ```cpp const CScript a{WITH_LOCK(::cs_main, return CScript{} << OP_0 << OP_1)}; ``` Before: (no warning) After: ``` warning: returning reference to local temporary object [-Wreturn-stack-address] const CScript a{WITH_LOCK(::cs_main, return CScript{} << OP_0 << OP_1)}; ^~~~~~~~~ ./sync.h:276:65: note: expanded from macro 'WITH_LOCK' #define WITH_LOCK(cs, code) [&]() -> decltype(auto) { LOCK(cs); code; }() ^~~~ ACKs for top commit: theuni: utACK fa7e6c56f58678b310898a158053ee9ff8b27fe7. jonatack: Light ACK fa7e6c56f58678b310898a158053ee9ff8b27fe7 debug build with clang 13, reproduced the example compiler warning in the pull description, and briefly looked at `clang::lifetimebound` support in earlier versions of clang; it is in clang 7 (https://releases.llvm.org/7.0.0/tools/clang/docs/AttributeReference.html#lifetimebound-clang-lifetimebound), did not see references to it in earlier docs Tree-SHA512: e915acdc4532445205b7703fab61a5d682231ace78ecfb274cb8523ca2bddefd85828f50ac047cfb1afaff92a331f5f7b5a1472539f999e30f7cf8ac8c3222f3
2021-09-03wallet: AddWalletDescriptor requires cs_wallet lockJoão Barbosa
No change in behavior, the lock is already held at call sites.
2021-09-03wallet: GetLabelAddresses requires cs_wallet lockJoão Barbosa
No change in behavior, the lock is already held at call sites.
2021-09-03Merge bitcoin/bitcoin#22860: test: Always clear reject reason in IsStandard ↵MarcoFalke
tx test fa1b08eb1413d547b5e322f20e6907b2f827a162 test: Always clear reject reason in IsStandard tx test (MarcoFalke) Pull request description: For some tests the reject reason wasn't cleared between runs and thus subsequent tests might (theoretically) fail to verify the correct reject reason. ACKs for top commit: benthecarman: ACK fa1b08eb1413d547b5e322f20e6907b2f827a162 theStack: Code-review ACK fa1b08eb1413d547b5e322f20e6907b2f827a162 Tree-SHA512: fcb727a690f92a4cf06127c302ba464f1e8cb997498e4f7fd9e210d193559b07e6efdb9d5c8a0bef3fe643bdfd5fedd431aaace20978dd49e56b8e770cb9f930
2021-09-03test: Remove useless overwriteMarcoFalke
2021-09-03Add LIFETIMEBOUND to InitializeChainstateMarcoFalke
2021-09-03Add LIFETIMEBOUND to CScript where neededMarcoFalke
2021-09-03MOVEONLY: Expose BanMapToJson / BanMapFromJsonRussell Yanofsky
CSubNet serialization code that was removed in fa4e6afdae7b82df638b60edf37ac36d57a8cb4f was needed by multiprocess code to share ban map between gui and node processes. Rather than adding it back, use suggestion from MarcoFalke <falke.marco@gmail.com> https://github.com/bitcoin/bitcoin/pull/10102#discussion_r690922929 to use JSON serialization. This requires making BanMapToJson / BanMapFromJson functions public.
2021-09-03Merge bitcoin/bitcoin#22100: refactor: Clean up new wallet spend, receive ↵Samuel Dobson
files added #21207 b11a195ef450bd138aa03204a5e74fdd3ddced26 refactor: Detach wallet transaction methods (followup for move-only) (Russell Yanofsky) Pull request description: This makes `CWallet` and `CWalletTx` methods in `spend.cpp` and `receive.cpp` files into standalone functions. It's a followup to [#21207 MOVEONLY: CWallet transaction code out of wallet.cpp/.h](https://github.com/bitcoin/bitcoin/pull/21207), which moved code from `wallet.cpp` to new `spend.cpp` and `receive.cpp` files. There are no changes in behavior. This is just making methods into functions and removing circular dependencies created by #21207. There are no comment or documentation changes, either. Removed comments from `transaction.h` are just migrated to `spend.h`, `receive.h`, and `wallet.h`. --- This commit was split off from #21206 so there are a few earlier review comments there ACKs for top commit: achow101: ACK b11a195ef450bd138aa03204a5e74fdd3ddced26 Sjors: utACK b11a195ef450bd138aa03204a5e74fdd3ddced26 meshcollider: light ACK b11a195ef450bd138aa03204a5e74fdd3ddced26 Tree-SHA512: 75ce818d3f03b728b14b12e2d21bd20b7be73978601989cb37ff98254393300d1bb7823281449cd3d9e40756d67d42bd9a46bbdafd2e8baa95aaf2cb1c84549f
2021-09-03qt, refactor: Replace WalletFrame::addWallet with WalletFrame::addViewHennadii Stepanov
No need to pass an instance of the WalletModel class to this method. Co-authored-by: João Barbosa <joao.paulo.barbosa@gmail.com>
generated by cgit v1.2.3 (git 2.26.2) at 2025-01-05 03:53:24 +0000