Age | Commit message (Collapse) | Author |
|
5cc783f5f32ffd550d1b298fc2e9cf6c0439f9fe qt: ensure translator comments end in full stop (Jarol Rodriguez)
Pull request description:
This is a follow-up to #318 which addresses this [nit](https://github.com/bitcoin-core/gui/pull/318#discussion_r706856893) by addressing it globally.
This ensures that all GUI translator comments end in a full stop. If a comment does not end in a full stop, a translator may think that the rest of the comment is being cut off.
While here, add a colon to the word "see" for any comments touched which point to look at a link.
ACKs for top commit:
hebasto:
ACK 5cc783f5f32ffd550d1b298fc2e9cf6c0439f9fe, I have reviewed the code and it looks OK, I agree it can be merged.
shaavan:
Code Review ACK 5cc783f5f32ffd550d1b298fc2e9cf6c0439f9fe
Tree-SHA512: 67a1d56175c974e0af9b460fa44163f7ce139a7b81cfaf8ed2c0e7fb6d5120957c3135d96010aeb6229689468e36673fe9571b5a8c3e1c07e047aba1bd563444
|
|
9bd168bf5457c6fd9770769547d8757bf14813b0 qt: add missing tooltips to options menu settings (Jarol Rodriguez)
Pull request description:
This adds missing tooltips to the text of the `Size of database cache` and the `Number of script verification threads` settings.
All settings in the Options window will now have appropriate tooltip texts.
ACKs for top commit:
jonatack:
ACK 9bd168bf5457c6fd9770769547d8757bf14813b0 tested on Debian 5.10.46-4 (2021-08-03)
hebasto:
ACK 9bd168bf5457c6fd9770769547d8757bf14813b0, tested on Linux Mint 20.2 (Qt 5.12.8).
Tree-SHA512: d71946bfee33c624a8b79eafe514d2c902090a40bc25097be4c7da4a80270f53305002af1b27d5fd082a0f45f838e22036632f9445918c4b8898073b33c09c08
|
|
|
|
This ensures that all gui translator comments end in a full stop.
If a comment does not end in a full stop, a translator may think
that the rest of the comment is being cut off.
While here, add a colon to the word "see" for any comments
touched which point to look at a link.
|
|
0b869df1c913839855148d728514c76ba7664092 qt: Add cancel button to configuration options popup (Shashwat)
Pull request description:
This PR renames the **OK** button to **Continue** and adds a **Cancel** button to the configuration options pop-up.
This feature will give the user an option to abort opening the configuration file if they want to. This is an essential helpful feature that was missing in the master branch.
In some windows managers such as Windows I3. The exit button at the top right corner is missing. So this feature becomes crucial there. And even when the exit button is there, it doesn't prevent the opening of the configuration file even when pressed.
Additionally, it will always be possible to close using Keyboard Shortcut. This PR helps accessibility for those who need to use a mouse.
<table>
<tr>
<td>Master
</td>
<td>PR
</td>
</tr>
<tr>
<td>
![Cancel-conf master(1)](https://user-images.githubusercontent.com/85434418/127555137-7a16dffd-109d-4024-917b-6b85f4df4f4a.png)
</td>
<td>
![Screenshot from 2021-09-07 20-15-28](https://user-images.githubusercontent.com/85434418/132365729-14f71f92-220b-4bb6-bed4-8315bd5697e6.png)
</td>
</tr>
</table>
ACKs for top commit:
hebasto:
ACK 0b869df1c913839855148d728514c76ba7664092, tested on Linux Mint 20.2 (Qt 5.12.8):
prayank23:
tACK https://github.com/bitcoin-core/gui/pull/391/commits/0b869df1c913839855148d728514c76ba7664092
Tree-SHA512: c314e8b84064134f028f66f5015eb0f6ba33d5d4174c9ff49dcb5d2b577dce6019f59f9c7913393a415a323ea98c26febf5ca26e3e2102e7a1d31171e01937f1
|
|
3ec061d9da0c8742bd9dec94ffeb82a11d000aba qt: Add "Copy address" item to the context menu in the Peers table (Hennadii Stepanov)
Pull request description:
Picking up #264
This adds a `Copy Address` context menu action to the `Peers Tab`.
Based on the first commit of PR #317 so that we can use `Qt::DisplayRole` in the `copyEntryData` function.
| Master | PR |
| ----------- | ----------- |
| ![Screen Shot 2021-05-05 at 4 51 11 AM](https://user-images.githubusercontent.com/23396902/117117822-fb067400-ad5d-11eb-9466-228456108e52.png) | ![Screen Shot 2021-05-05 at 4 49 15 AM](https://user-images.githubusercontent.com/23396902/117117835-fe99fb00-ad5d-11eb-8de0-f6a9acdbf40e.png) |
ACKs for top commit:
shaavan:
tACK 3ec061d9da0c8742bd9dec94ffeb82a11d000aba
luke-jr:
utACK 3ec061d9da0c8742bd9dec94ffeb82a11d000aba
hebasto:
ACK 3ec061d9da0c8742bd9dec94ffeb82a11d000aba, tested on Linux Mint 20.2 (Qt 5.12.8):
Tree-SHA512: be0d7324592aae3928fa3cc522294f17226419fe8cbe3587df12a36bd4fa9c81bead377b13051e950b9a3fcd290b273861e70d6c76b75cdf76eaf58224b834cd
|
|
This adds a cancel buttion to the configuration options window
|
|
corrupted
fa55c3dc1b4bbdc6a53bd11fa6c0b2ec6bbb64ae Raise InitError when peers.dat is invalid or corrupted (MarcoFalke)
fa4e2ccfd8ae96c381947285bef47cb39474ac89 Inline ReadPeerAddresses (MarcoFalke)
fa5aeec80c6cdca9ca027d80dff3b397911ff2c2 Move LoadAddrman from init to addrdb (MarcoFalke)
Pull request description:
peers.dat is silently erased when it can not be parsed or when it appears corrupted. Fix that by notifying the user. This might help in the following examples:
* The user provided the database, but picked the wrong one.
* A future version of Bitcoin Core wrote the file and it can't be read.
* The file was corrupted by a logic bug in Bitcoin Core.
* The file was corrupted by a disk failure.
ACKs for top commit:
jonatack:
Code review re-ACK fa55c3dc1b4bbdc6a53bd11fa6c0b2ec6bbb64ae per `git range-diff eb1f570 fa59c6d fa55c3` and verified the new tests fail on master, except "Check mocked addrman is valid", as expected
prayank23:
tACK https://github.com/bitcoin/bitcoin/commit/fa55c3dc1b4bbdc6a53bd11fa6c0b2ec6bbb64ae
vasild:
ACK fa55c3dc1b4bbdc6a53bd11fa6c0b2ec6bbb64ae
Tree-SHA512: 78264a78ee570a3c3262cf9c8542b5ffaffa5f52da1eef66c8c381f346989272967cfe1769c573502d9d7d3f7ad68c3ac3b2ec734185d2e4e7595b7122b14196
|
|
32748da0f47f7aa9fba78dfb29aa426b14f15624 whitespace fixups after move and scripted-diff (glozow)
fa47622e8dc66bec9ea690aec3f0999108d76dc9 scripted-diff: rename variables in policy/rbf (glozow)
ac761f0a23c9c469fa00885edf3d5c9ae7c6a2b3 MOVEONLY: fee checks (Rules 3 and 4) to policy/rbf (glozow)
9c2f9f89846264b503d5573341bb78cf609cbc5e MOVEONLY: check that fees > direct conflicts to policy/rbf (glozow)
3f033f01a6b0f7772ae1b21044903b8f4249ad08 MOVEONLY: check for disjoint conflicts and ancestors to policy/rbf (glozow)
7b60c02b7d5e2ab12288393d2258873ebb26d811 MOVEONLY: BIP125 Rule 2 to policy/rbf (glozow)
f8ad2a57c61d1e817e2445226688e03080fc8688 Make GetEntriesForConflicts return std::optional (glozow)
Pull request description:
This PR does not change behavior. It extracts the BIP125 logic into helper functions (and puts them in the policy/rbf* files). This enables three things - I think each one individually is pretty good:
- Implementation of package RBF (see #22290). I want it to be as close to BIP125 as possible so that it doesn't become a distinct fee-bumping mechanism. Doing these move-only commits first means the diff is mostly mechanical to review, and I just need to create a function that mirrors the single transaction validation.
- We will be able to isolate and test our RBF logic alone. Recently, there have been some discussions on discrepancies between our code and BIP125, as well as proposals for improving it. Generally, I think making this code more modular and de-bloating validation.cpp is probably a good idea.
- Witness Replacement (replacing same-txid-different-wtxid when the witness is significantly smaller and therefore higher feerate) in a BIP125-similar way. Hopefully it can just be implemented with calls to the rbf functions (i.e. `PaysForRBF`) and an edit to the relevant mempool entries.
ACKs for top commit:
mjdietzx:
ACK 32748da0f47f7aa9fba78dfb29aa426b14f15624
theStack:
Code-review ACK 32748da0f47f7aa9fba78dfb29aa426b14f15624 📐
MarcoFalke:
review ACK 32748da0f47f7aa9fba78dfb29aa426b14f15624 🦇
Tree-SHA512: d89985c8b4b42b54861018deb89468e04968c85a3fb1113bbcb2eb2609577bc4fd9bf254593b5bd0e7ab059a0fa8192d1a903b00f77e6f120c7a80488ffcbfc0
|
|
853c4edb70f897a6a7165abaea4a303d7d448721 [net] Remove asmap argument from CNode::CopyStats() (John Newbery)
9fd5618610e91e3949536c5122cf31eb58c9aa6b [asmap] Make DecodeAsmap() a utility function (John Newbery)
bfdf4ef334a16ef6108a658bf4f8514754128c18 [asmap] Remove SanityCheckASMap() from netaddress (John Newbery)
07a9eccb60485e71494664cc2b1964ae06a3dcf0 [net] Remove CConnman::Options.m_asmap (John Newbery)
Pull request description:
These small cleanups to the asmap code are the first 4 commits from #22910. They're minor improvements that are independently useful whether or not 22910 is merged.
ACKs for top commit:
naumenkogs:
ACK 853c4edb70f897a6a7165abaea4a303d7d448721
theStack:
Concept and code-review ACK 853c4edb70f897a6a7165abaea4a303d7d448721 🗺️
fanquake:
ACK 853c4edb70f897a6a7165abaea4a303d7d448721
Tree-SHA512: 64783743182592ac165df6ff8d18870b63861e9204ed722c207fca6938687aac43232a5ac4d8228cf8b92130ab0349de1b410a2467bb5a9d60dd9a7221b3b85b
|
|
to fix duration, improve BCLog::LogMsg()
f530202353a4f8bb444966559aa15681ab3cebc6 Make unexpected time type in BCLog::LogMsg() a compile-time error (Martin Ankerl)
bddae7e7ff7bb5931ed807acaef7336f2ee98476 Add util/types.h with ALWAYS_FALSE template (MarcoFalke)
498b323425d960274c40472a6a847afc1982201d log, timer: improve BCLog::LogMsg() (Jon Atack)
8d2f847ed913f15677ae978a412015ac844ffceb sync: inline lock contention logging macro to fix time duration (Jon Atack)
Pull request description:
Follow-up to #22736.
The first commit addresses the issue identified and reported by Martin Ankerl in https://github.com/bitcoin/bitcoin/pull/22736#discussion_r703019629 to fix the lock contention duration reporting.
The next three commits make improvements to the timer code in `BCLog::LogMsg()` and add `util/types.h` with an `ALWAYS_FALSE` template, that springboard from https://github.com/bitcoin/bitcoin/pull/22736#discussion_r702747920 by Marco Falke.
ACKs for top commit:
martinus:
re-ACK f530202353a4f8bb444966559aa15681ab3cebc6. I ran a fully synced node for about a day. My node was mostly idle though so not much was going on. I [wrote a little script](https://github.com/martinus/bitcoin-stuff/blob/main/scripts/parse-debuglog-contention-single.rb) to parse the `debug.log` and summarize the output to see if anything interesting was going on, here is the result:
theStack:
ACK f530202353a4f8bb444966559aa15681ab3cebc6
Tree-SHA512: 37d093eac5590e1b5846ab5994d0950d71e131177d1afe4a5f7fcd614270f977e0ea117e7af788e9a74ddcccab35b42ec8fa4db3a3378940d4988df7d21cdaaa
|
|
6045a1464252075f4135bd4a69d202d55d124eb2 util: remove libevent <= 2.0.18 back-compat code (fanquake)
Pull request description:
Now that we require libevent >=2.0.21, remove backwards compatibility code for older versions.
ACKs for top commit:
kristapsk:
ACK 6045a1464252075f4135bd4a69d202d55d124eb2
Tree-SHA512: 49a237ee3cef78b105f8ea91dc3e541fe700fe3a3d02a88f85ec91772068ffbe508dbe196a4d693399b2bcf903251b9bc2573f04cb8f2e21a2ea481f35bfde32
|
|
fa309ee61c09726a8780acaea94502712f817921 bench: Fix 32-bit compilation failure in addrman bench (MarcoFalke)
fae0295a799499268caca9c385ac4d7061543980 ci: Switch multiprocess to i686 build (MarcoFalke)
Pull request description:
Building for i686 with clang helps to catch bugs early for:
* The OSS-Fuzz i686 clang libFuzzer build
* The arm 32-bit native clang build
Fixes #22889
ACKs for top commit:
hebasto:
ACK fa309ee61c09726a8780acaea94502712f817921
Tree-SHA512: 581820d319aae2fcd4dd44979ee3d4164a575f0438476890aa2a7447f1392a5da26766cd6ab954530499b54f66eec2417bdeefdd7efb19bc27dd679cd2b9d0ce
|
|
Now that we require libevent >=2.0.21, remove backwards compatibility
code for older versions.
|
|
e6998838e5548991274ad2bf1697d862905b8837 doc: Add IPv6 address to zmq example (nthumann)
8abe5703a9bb76bc92204a6f69775790e96208fa test: Add IPv6 test to zmq (nthumann)
ded449b726e47f35798ef1c4b1e59123a0dc2b61 zmq: Enable IPv6 on listening socket (nthumann)
Pull request description:
This PR adds support for listening on IPv6 addresses with bitcoinds ZMQ interface, just like the RPC server.
Currently, it is not possible to specify an IPv6 address, as the `ZMQ_IPV6` [socket option](http://api.zeromq.org/master:zmq-setsockopt#toc27) is not set and therefore the ZMQ initialization fails, if one does so. The absence of this option has also been noted [here](https://github.com/bitcoin/bitcoin/issues/15198#issuecomment-617378512).
With this PR one can e.g. set `-zmqpubhashblock=tcp://[::1]:28333` to listen on the IPv6 loopback address.
ACKs for top commit:
laanwj:
Code review ACK e6998838e5548991274ad2bf1697d862905b8837
theStack:
Tested ACK e6998838e5548991274ad2bf1697d862905b8837 🌱
Tree-SHA512: 43c3043d8d5c79794d475926259c1be975b694db4fcc1f7750a9a28e242f0fa1b531735a63ea5777498003aa5834f6243f39742d0f3941f2f37593d0c7890700
|
|
|
|
No need to have a function that is only called in one place
|
|
Init should only concern itself with the initialization order, not the
detailed initialization logic of every module.
Also, inlining logic into a method that is ~800 lines of code, makes it
impossible to unit test on its own.
|
|
|
|
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
|
|
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
|
|
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
|
|
This was missed in #21052.
|
|
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
|
|
|
|
|
|
This saves passing around a reference to the asmap std::vector<bool>.
|
|
DecopeAsmap is a pure utility function and doesn't have any
dependencies on addrman, so move it to util/asmap.
Reviewer hint: use:
`git diff --color-moved=dimmed-zebra --color-moved-ws=ignore-all-space`
|
|
SanityCheckASMap(asmap, bits) simply calls through to SanityCheckASMap(asmap)
in util/asmap. Update all callers to simply call that function.
|
|
This data member was introduced in ec45646de9e but never used.
|
|
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.
|
|
|
|
This is a follow-up to the code move in commit a820e79512b67b1bfda20bdc32b47086d2b0910d
|
|
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
|
|
|
|
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
|
|
- make timer code more homogeneous
- replace division with multiplication
- log if the time type is unexpected
|
|
Co-authored-by: Martin Ankerl <martin.ankerl@gmail.com>
|
|
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
|
|
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
|
|
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
|
|
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
|
|
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
|
|
Co-authored-by: Jarol Rodriguez <jarolrod@tutanota.com>
|
|
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
|
|
|
|
Also add a regression test.
|
|
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
|
|
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
|
|
No change in behavior, the lock is already held at call sites.
|