faecb74562d012a336837d3b39572c235ad2eb9d Expose integral m_conn_type in CNodeStats, remove m_conn_type_string (Jon Atack)
Pull request description:
Currently, strings are stored for what are actually integral (strong) enum types. This is fine, because the strings are only used as-is for the debug log and RPC. However, it complicates using them in the GUI. User facing strings in the GUI should be translated and only string literals can be picked up for translation, not runtime `std::string`s.
Fix that by removing the `std::string` members and replace them by strong enum integral types.
ACKs for top commit:
jonatack:
Code review ACK faecb74562d012a336837d3b39572c235ad2eb9d
theStack:
Code review ACK faecb74562d012a336837d3b39572c235ad2eb9d 🌲
Tree-SHA512: 24df2bd0645432060e393eb44b8abaf20fe296457d07a867b0e735c3e2e75af7b03fc6bfeca734ec33ab816a7c8e1f8591a5ec342f3afe3098a4e41f5c2cfebb
fa44417fcb41fc228d9337e9eba4977a495de692 fuzz: Add missing muhash registration (MarcoFalke)
Pull request description:
otherwise it is not possible to run the target
ACKs for top commit:
laanwj:
Code review ACK fa44417fcb41fc228d9337e9eba4977a495de692
Tree-SHA512: b6495b84890a778d3569deddfc5703d1bed95d7d5c1eb0766fd5bd9afe86a3a0ccf4726aa16cba4a5fd182d23bfb7e815f3af4eb635915397c90805585b5699e
a7599c80ebb9579df45e2d6ccf3168302cf42f03 test: run mempool_compatibility.py even with wallet disabled (Michael Dietz)
Pull request description:
Another functional test rewritten as proposed in https://github.com/bitcoin/bitcoin/issues/20078
ACKs for top commit:
MarcoFalke:
review ACK a7599c80ebb9579df45e2d6ccf3168302cf42f03 didn't test
Tree-SHA512: cc7a617e5489ed27bbdbdee85a82fa08525375061f7f4524577a6b8ecb340396adac88419b51f513be22ca53edd0a3bd5d572d9f43ffc2c18550b0ef9069d238
196b7276495c5d125e3799aee6cfc54be6720ec7 depends: Add comment about cache invalidation (Carl Dong)
949c480e527532be58d6184deb313d91339efdbf depends: Fully determine path for darwin cctools (Carl Dong)
880660acfa547558f6ef5adff6768de95e53af6e depends: Fully determine path for darwin_{CC,CXX} (Carl Dong)
80331107416b8a6cb487ee1c89a39c6a8bced27b depends: Quote to prevent word splitting in config.site (Carl Dong)
77b1ef89a07bf7a493ce4abaccbbff793cbde9be depends: Remove -fuse-ld line (Carl Dong)
300733921863c176535806c40afdc813b99e7459 depends: Pin clang search paths for darwin host (Carl Dong)
107f33d434ebbe6f93fa187e2af1f6f850e82d3b depends: Delay expansion of per-package vars (Carl Dong)
Pull request description:
> Hello clang/lib/frontend,
> I search your headers once again.
> Because it's time for some housekeeping,
> Within the code I was tweaking,
> And the targets I was making with my build,
> Are unfulfilled,
> It's just language compliance.
>
> In reference works I scroll alone
> Pages cribbed from holy tomes
> In the details of a template
> My code's behaviour has now found its fate
> When my hopes were dashed as a note left it as described:
> As undefined
> It's not in compliance
>
> And from the standard text I saw
> Ten thousand errors, maybe more
> Threading used without locking
> Pointers referenced after freeing
> Linters writing warnings that coders will never fix
> But still they tick
> The box that claims compliance
>
> "Fools," said I, "you do not know"
> Errors, like a cancer, grow
> Hear my words that I might reach you
> Use -Wall and it might teach you
> But my words and compiler errors fade.
> Schedules forbade compliance.
>
> And the people bowed and prayed
> With static checking torn and frayed
> The markets flashed out their warning
> In the words that they were forming
> As recruiters said "The search for more profits leads to writing stuff in CSS,
> And node.js.
> Without a need for compliance"
Many thanks to ajtowns for the above contribution!
-----
This PR is ready for review!
When cross-compiling for macOS, the SDK gives us the entire context/sysroot on which we should base the build. This means that we can be extremely specific w/re our search path ordering in order to avoid build problems that arise out of a user's specific environment/system setup and improve the robustness of our macOS toolchain. This PR does 2 things to this end:
1. Unset environment variables which are known to alter search paths.
1. Makes us (in the case of macOS builds) explicitly specify the list of system include search paths and its ordering, rather than rely on `clang`'s unreliable autodetection routine. Here is the [rabbit-hole gist](https://gist.github.com/dongcarl/5cdc6990b7599e8a5bf6d2a9c70e82f9).
See the added comments in `depends/hosts/darwin.mk` for more details:
8b8296dc70/depends/hosts/darwin.mk (L37-L60)
We can be this specific _only_ because macOS builds are neatly contained in an SDK, **and** we are cross-compiling. Native toolchains should rely on the environment/distro/user to know how best to build for the running system.
Note: Although the `-u` flag of `env` is not a POSIX standard flag, it seems like it is useful enough to be implemented in [coreutils](https://www.gnu.org/software/coreutils/manual/html_node/env-invocation.html), [busybox](https://busybox.net/downloads/BusyBox.html#env), [FreeBSD](https://www.freebsd.org/cgi/man.cgi?env).
ACKs for top commit:
laanwj:
code review ACK 196b7276495c5d125e3799aee6cfc54be6720ec7
Tree-SHA512: 406442df16d9aa0aef62f9fa94f72d7e48374301f3d826bf32f183e1610942aa44a4adfac7bead1f14aded0044fac400e1328fcd933b2337e55a024f034b5013
ef712298c3f8bc2afdad783f05080443b72b3f77 util: Check for file being NULL in DirectoryCommit (Luke Dashjr)
457490403853321d308c6ca6aaa90d6f8f29b4cf Fix possible data race when committing block files (Evan Klitzke)
220bb16cbee5b91d0bc0fcc6c71560d631295fa5 util: Introduce DirectoryCommit commit function to sync a directory (Evan Klitzke)
ce5cbaea63ad4ea78e533bdb14f47f414061ae7f util.h: Document FileCommit function (Evan Klitzke)
844d650eea3bd809884cc5dd996a388bdc58314e util: Prefer Mac-specific F_FULLSYNC over fdatasync in FileCommit (Evan Klitzke)
f6cec0bcaf560fa310853ad3fe17022602b63d5f util: Refactor FileCommit from an #if sequence nested in #else, to a sequence of #elif (Evan Klitzke)
Pull request description:
Reviving #12696
ACKs for top commit:
laanwj:
Code review ACK ef712298c3f8bc2afdad783f05080443b72b3f77
Tree-SHA512: 07d650990ef4c18d645dee3f9a199a940683ad17557d79d93979a76c4e710d8d70e6eae01d1a5991494a24a7654eb7db868be0c34a31e70b2509945d95bc9cce
Instead of doing the awkward /bin path prepending at config.site
creation time, set darwin_{CC,CXX} in a way that fully determines the
program's path (clang/clang++) similar to how AC_PATH_{TOOL,PROG} would
do.
Also see the added comment block in depends/Makefile for more context on
determining $PATH for our config.site.
clang warns when a command line option is unused, and some of our tests
use Werror, so unfortunately we cannot use this flag to pin our linker
for now. Leaving this commit in for future reference, as it would be
great if there's more granularity to Werror and we can be explicit about
what linker we want to use.
Prior to this commit, when int_vars was called for packages, it would
immediately expand the "single-dollar variables", which may be defined
in terms of variables which are not yet determined (e.g. variables
defined in package/*.mk, which are included after int_vars is called).
This is required for the next commit as after that commit, for darwin
cross-builds:
0. int_vars is defined in terms of $(1)_cc
1. $(1)_cc is defined in terms of darwin_CC
2. ... which is defined in terms of clang_resource_dir
3. ... which is defined in terms of native_cctools_clang_version
4. which is undetermined at the time when int_vars is being expanded and evaluated
a191e23b8e7f0e19fc0359825eb7ca0d47966fa9 doc: Add release notes (Hennadii Stepanov)
ae749d12ddbaf592fbdb65d98ca35a0ff5566992 doc: Add libnatpmp stuff (Hennadii Stepanov)
e28f9be87a0f3c59a9184d602fe7947526df6a97 ci: Add libnatpmp-dev package to some builds (Hennadii Stepanov)
5a0185b6c9c838290103314916190a0330ed9a82 gui: Add NAT-PMP network option (Hennadii Stepanov)
a39f7336a3b493d46a4486c4c94fdca1b3151370 net: Add -natpmp command line option (Hennadii Stepanov)
28acffd9d53ec437e908abb8c84497a4f41b91ed net: Add NAT-PMP to port mapping loop (Hennadii Stepanov)
a8d9f275d0ca64797cc89627f8003b48b3efef63 net: Add libnatpmp support (Hennadii Stepanov)
58e8364dcdc4e57b0caac09f8402e6535301de9b gui: Apply port mapping changes on dialog exit (Hennadii Stepanov)
cf151cc68c95a8943e43e3fa4061e176262779e7 scripted-diff: Rename UPnP stuff (Hennadii Stepanov)
4e91b1e24d96e0cdccdd2a3ed034413f3ba6bae6 net: Add flags for port mapping protocols (Hennadii Stepanov)
8b50d1b5bb29b7d1ea0245ba75a8df3144e312dc net: Keep trying to use UPnP when -upnp=1 (Hennadii Stepanov)
28e2961fd6a2a9101fc08fb748430989291aaf7e refactor: Replace magic number with named constant (Hennadii Stepanov)
02ccf69dd6b772423acb343d16ef2bdbb3e3da03 refactor: Move port mapping code to its own module (Hennadii Stepanov)
Pull request description:
Close#11902
This PR is an alternative to:
- #12288
- #15717
To compile with NAT-PMP support on Ubuntu [`libnatpmp-dev`](https://packages.ubuntu.com/source/bionic/libnatpmp) should be available.
Log excerpt:
```
2020-02-05T20:12:28Z [mapport] NAT-PMP: public address = 95.164.65.194
2020-02-05T20:12:28Z [mapport] AddLocal(95.164.65.194:18333,3)
2020-02-05T20:12:28Z [mapport] NAT-PMP: port mapping successful.
```
See: [`libnatpmp`](https://miniupnp.tuxfamily.org/libnatpmp.html)
---
Some follow-ups are out of this PR's scope:
- mention NAT-PMP library in the version message
- ~integrate NAT-PMP into the GUI~ (already [added](https://github.com/bitcoin/bitcoin/pull/18077#issuecomment-589405068))
ACKs for top commit:
laanwj:
Tested and code review ACK a191e23b8e7f0e19fc0359825eb7ca0d47966fa9
Tree-SHA512: 10e19267c21bf30f20ff1abfc882d526049f0e790b95e12f109dc2bed7c0aef45de03eaf967f4e667e7509be04f1873a5c508087393d947205f3aab2ad6d7cf1
9815332d5158d69a94abeaf465a2c07bd8e43359 test: Change MuHash Python implementation to match cpp version again (Fabian Jahr)
01297fb3ca57e4b8cbc5a89fc7c6367de33b0bc6 fuzz: Add MuHash consistency fuzz test (Fabian Jahr)
b111410914041b72961536c3e4037eba103a8085 test: Add MuHash3072 fuzz test (Fabian Jahr)
c1225273857f9fa2e2276396e3f8b3ea48306df3 bench: Add Muhash benchmarks (Fabian Jahr)
7b1242229d1fcc9277238a3aefb3431061c82bfa test: Add MuHash3072 unit tests (Fabian Jahr)
adc708c98dbf03b1735edc91f813a36580781a95 crypto: Add MuHash3072 implementation (Fabian Jahr)
0b4d290bf5b0a4d156c523431bf89aaa9ffe92e5 crypto: Add Num3072 implementation (Fabian Jahr)
589f958662a2dcaacdb9a66f1088c74828a39577 build: Check for 128 bit integer support (Fabian Jahr)
Pull request description:
This is the first split of #18000 which implements the Muhash algorithm and uses it to calculate the UTXO set hash in `gettxoutsetinfo`.
ACKs for top commit:
laanwj:
Code review ACK 9815332d5158d69a94abeaf465a2c07bd8e43359
Tree-SHA512: 4bc090738f0e3d80b74bdd8122e24a8ce80121120fd37c7e4335a73e7ba4fcd7643f2a2d559e2eebf54b8e3a3bd5f12cfb27ba61ded135fda210a07a233eae45
eeee43bc48ea7fbacd3c5e3f076f01f04744adb8 fuzz: Use ConsumeWeakEnum for ServiceFlags (MarcoFalke)
fa9949b91414ee0da376a322cee32ba4e3989d8c fuzz: Add ConsumeWeakEnum helper, Extract ALL_NET_PERMISSION_FLAGS (MarcoFalke)
faaef9434c19e3643322ee442c240c166af5adbd fuzz: [refactor] Extract ALL_CONNECTION_TYPES constant (MarcoFalke)
fa42da2d5424c0aeccfae4b49fde2bea330b63dc fuzz: Use ConsumeNode in process_message target (MarcoFalke)
fa121f058fdc5f09dd11678480f551246cb3c5e2 fuzz: Use ConsumeNode in process_messages target (MarcoFalke)
Pull request description:
The fuzz tests have several problems:
* The array passed to the fuzz engine to pick `net_permission_flags` is outdated
* The process_message* targets has the service flags as well as connection type hardcoded, limiting potential coverage
* The service flags deserialization from the fuzz engine doesn't allow for easy "exact matches". The fuzz engine has to explore a 64-bit space to hit an "exact match" (only one bit set)
Fix all issues in the commits in this pull
ACKs for top commit:
mzumsande:
ACK eeee43bc48ea7fbacd3c5e3f076f01f04744adb8 after rebase.
Tree-SHA512: 1ad9520c7e708b7f4994ae8f77886ffca33d7c542756e2a3e07dbbbe59e360f9fcaccf2e2fb57d9bc731d4aeb4938fb1c5c546e9d2744b007af5626f5cb377fe
195fcb53a09e9b852544778a077727f81d31303e qt: Follow Qt docs when implementing rowCount and columnCount (Hennadii Stepanov)
Pull request description:
[`QAbstractItemModel::rowCount`](https://doc.qt.io/qt-5/qabstractitemmodel.html#rowCount):
> **Note:** When implementing a table based model, `rowCount()` should return 0 when the parent is valid.
[`QAbstractItemModel::columnCount`](https://doc.qt.io/qt-5/qabstractitemmodel.html#columnCount):
> **Note:** When implementing a table based model, `columnCount()` should return 0 when the parent is valid.
ACKs for top commit:
jarolrod:
Tested ACK 195fcb53a09e9b852544778a077727f81d31303e. Compiled and ran on macOS (Big Sur 11.1 and Catalina 10.15.7), Arch Linux, and FreeBSD. visually verified no weird effects with the `Address`, `Ban`, `Peer`, and `Transaction` tables. As already stated, the code change brings us inline with what the QT Docs recommend.
Tree-SHA512: 179a3430e68e77b22cdf642964cd96c023a2286ee256bbeb25b43df3d2eef6f59978c8d92173c6be5071d127fdcd6aa338142f6eaf003ff08e4abd65172d20ca
fa210689e27b0d78b2fe894e51e364179db7a3ce net: Move SocketSendData lock annotation to header (MarcoFalke)
fa0a71781a964d944db9ecc002675ef32249f62e net: Move CConnman/NetEventsInterface after CNode in header file (MarcoFalke)
Pull request description:
Lock annotations must be in the header, otherwise the will have limited or no effect
ACKs for top commit:
jnewbery:
utACK fa210689e2
Tree-SHA512: c25aac793662227f2bb163e2f98cd38e89b43a03bad925d192d6166dee76a456110eaf55be5e4b4f2ddcf20bde7e3fc82a7a4338670a1f2d027d01b4a8908303
90f9fc274bf69b33adea593146ff5d6793123781 qt: Save QSplitter state in QSettings (Hennadii Stepanov)
Pull request description:
This PR adds the ability to save the `QSplitter` widget state in `QSettings` during shutdown, and restore it on startup.
A user no longer needs to adjust the splitter every time :)
ACKs for top commit:
jonasschnelli:
utACK 90f9fc274bf69b33adea593146ff5d6793123781
jonatack:
ACK 90f9fc274bf69b33adea593146ff5d6793123781 this sets the "PeersTabSplitterSizes" value in the RPCConsole dtor and restores it in the RPCConsole ctor; tested in Debian with various split settings, tab open/close sequences, and shutdown methods, and the Peers window split state was faithfully maintained.
Tree-SHA512: efbd6a4cee512982944955d36775e75a8a217b1dc49e62d42c6e402d2710dd44324b2c3c1edeb5fe38d9229e0e4a39734d1f4e63405ade8694762e1bbf72020b
31b136e5802e1b1e5f9a9589736afe0652f34da2 Don't declare de facto const reference variables as non-const (practicalswift)
1c65c075ee4c7f98d9c1fac5ed7576b96374d4e9 Don't declare de facto const member functions as non-const (practicalswift)
Pull request description:
_Meta: This is the second and final part of the `const` refactoring series (part one: #20581). **I promise: no more refactoring PRs from me in a while! :)** I'll now go back to focusing on fuzzing/hardening!_
Changes in this PR:
* Don't declare de facto const member functions as non-const
* Don't declare de facto const reference variables as non-const
Awards for finding candidates for the above changes go to:
* `clang-tidy`'s [`readability-make-member-function-const`](https://clang.llvm.org/extra/clang-tidy/checks/readability-make-member-function-const.html) check ([list of `clang-tidy` checks](https://clang.llvm.org/extra/clang-tidy/checks/list.html))
* `cppcheck`'s `constVariable` check ([list of `cppcheck` checks](https://sourceforge.net/p/cppcheck/wiki/ListOfChecks/))
See #18920 for instructions on how to analyse Bitcoin Core using Clang Static Analysis, `clang-tidy` and `cppcheck`.
ACKs for top commit:
ajtowns:
ACK 31b136e5802e1b1e5f9a9589736afe0652f34da2
jonatack:
ACK 31b136e5802e1b1e5f9a9589736afe0652f34da2
theStack:
ACK 31b136e5802e1b1e5f9a9589736afe0652f34da2 ❄️
Tree-SHA512: f58f8f00744219426874379e9f3e9331132b9b48e954d24f3a85cbb858fdcc98009ed42ef7e7b4619ae8af9fc240a6d8bfc1c438db2e97b0ecd722a80dcfeffe
6f2c4fd0775a9c45eacc4bab8f138528852fdf44 netinfo: add user help documentation (Jon Atack)
Pull request description:
This is the help doc commit of #20764 without the rest of the PR or anything new since the 0.21.0 branch-off in order to target giving users a -netinfo help doc for 0.21.
- to test the new help
```
$ ./src/bitcoin-cli -netinfo help
```
- to see the updated short help
```
$ ./src/bitcoin-cli -help | grep -A4 netinfo
```
<details><summary><code>-netinfo</code> help doc</summary><p>
```
$ ./src/bitcoin-cli -netinfo help
-netinfo level "help"
Returns a network peer connections dashboard with information from the remote server.
Under the hood, -netinfo fetches the data by calling getpeerinfo and getnetworkinfo.
An optional integer argument from 0 to 4 can be passed for different peers listings.
Pass "help" to see this detailed help documentation.
If more than one argument is passed, only the first one is read and parsed.
Suggestion: use with the Linux watch(1) command for a live dashboard; see example below.
Arguments:
1. level (integer 0-4, optional) Specify the info level of the peers dashboard (default 0):
0 - Connection counts and local addresses
1 - Like 0 but with a peers listing (without address or version columns)
2 - Like 1 but with an address column
3 - Like 1 but with a version column
4 - Like 1 but with both address and version columns
2. help (string "help", optional) Print this help documentation instead of the dashboard.
Result:
* The peers listing in levels 1-4 displays all of the peers sorted by direction and minimum ping time:
Column Description
------ -----------
<-> Direction
"in" - inbound connections are those initiated by the peer
"out" - outbound connections are those initiated by us
type Type of peer connection
"full" - full relay, the default
"block" - block relay; like full relay but does not relay transactions or addresses
net Network the peer connected through ("ipv4", "ipv6", "onion", "i2p", or "cjdns")
mping Minimum observed ping time, in milliseconds (ms)
ping Last observed ping time, in milliseconds (ms)
send Time since last message sent to the peer, in seconds
recv Time since last message received from the peer, in seconds
txn Time since last novel transaction received from the peer and accepted into our mempool, in minutes
blk Time since last novel block passing initial validity checks received from the peer, in minutes
age Duration of connection to the peer, in minutes
asmap Mapped AS (Autonomous System) number in the BGP route to the peer, used for diversifying
peer selection (only displayed if the -asmap config option is set)
id Peer index, in increasing order of peer connections since node startup
address IP address and port of the peer
version Peer version and subversion concatenated, e.g. "70016/Satoshi:21.0.0/"
* The connection counts table displays the number of peers by direction, network, and the totals
for each, as well as a column for block relay peers.
* The local addresses table lists each local address broadcast by the node, the port, and the score.
Examples:
Connection counts and local addresses only
> bitcoin-cli -netinfo
Compact peers listing
> bitcoin-cli -netinfo 1
Full dashboard
> bitcoin-cli -netinfo 4
Full live dashboard, adjust --interval or --no-title as needed (Linux)
> watch --interval 1 --no-title ./src/bitcoin-cli -netinfo 4
See this help
> bitcoin-cli -netinfo help
```
</p></details>
ACKs for top commit:
laanwj:
ACK 6f2c4fd0775a9c45eacc4bab8f138528852fdf44
Tree-SHA512: dd49b1ce65546dacfb8ba9f9d57de0eae55560fd05533cf26c0b5d6ec65bf1de789c3287e90a0e2f47707532fab2fe62919a4192a7ffd58ac8eec18293e9aaeb
fa6c114ae604571435e8c4d25906a8b6d5b9984c test: Add sanitizer suppressions for AMD EPYC CPUs (MarcoFalke)
Pull request description:
Currently the ci system only runs on intel cpus (and some arm devices), but it won't run on CPUs `Using the 'shani(1way,2way)' SHA256 implementation` (excerpt from debug log).
For reference, google cloud CPUs (which is what Cirrus CI uses) print `Using the 'sse4(1way),sse41(4way),avx2(8way)' SHA256 implementation`
The traceback I got:
```
crypto/sha256_shani.cpp:87:18: runtime error: unsigned integer overflow: 0 - 1 cannot be represented in type 'size_t' (aka 'unsigned long')
#0 0x55c0000e95ec in sha256_shani::Transform(unsigned int*, unsigned char const*, unsigned long) /root/bitcoin/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/crypto/sha256_shani.cpp:87:18
#1 0x55bfffb926f8 in (anonymous namespace)::SelfTest() /root/bitcoin/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/crypto/sha256.cpp:517:9
#2 0x55bfffb906ed in SHA256AutoDetect[abi:cxx11]() /root/bitcoin/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/crypto/sha256.cpp:626:5
#3 0x55bfff87ab97 in BasicTestingSetup::BasicTestingSetup(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::vector<char const*, std::allocator<char const*> > const&) /root/bitcoin/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/test/util/setup_common.cpp:104:5
#4 0x55bffe885877 in main /root/bitcoin/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/qt/test/test_main.cpp:52:27
#5 0x7f20c3bf60b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2)
#6 0x55bffe7a5f6d in _start (/root/bitcoin/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/qt/test/test_bitcoin-qt+0x1d00f6d)
SUMMARY: UndefinedBehaviorSanitizer: unsigned-integer-overflow crypto/sha256_shani.cpp:87:18 in
ACKs for top commit:
laanwj:
Anyhow ACK fa6c114ae604571435e8c4d25906a8b6d5b9984c
Tree-SHA512: 968a1d28eedec58c337b1323862f583cb1bcd78c5f03396940b9ab53ded12f8c6652877909aba05ee5586532137418fd817ff979bd7bef6e07856094f9d7f9b1
378aedc45248cea82d9a3e6dc1038d6828008a76 [net] Add cs_vSend lock annotations (John Newbery)
673254515a2f97e53dd8c7335c836b083ba7e31a [net] Move RecordBytesSent() call out of cs_vSend lock (John Newbery)
Pull request description:
RecordBytesSent() does not require cs_vSend to be locked, so reduce the scope of cs_vSend.
Also correctly annotate the CNode data members that are guarded by cs_vSend.
This is a simpler alternative to #19673.
ACKs for top commit:
jnewbery:
ok, reverting to commit 378aedc which has two ACKs already. Any style issues can be fixed up in future PRs.
troygiorshev:
ACK 378aedc45248cea82d9a3e6dc1038d6828008a76
theStack:
re-ACK 378aedc45248cea82d9a3e6dc1038d6828008a76
MarcoFalke:
review ACK 378aedc45248cea82d9a3e6dc1038d6828008a76 🔌
Tree-SHA512: e9cd6c472b7e1479120c1bf2d1c640cf6d18c7d589a5f9b7dfc4875e5790adaab403a7a1b945a47e79e7249a614b8583270e4549f89b22e8a9edb2e4818b0d07
0e51a355128a825d428fe2b9017c25085731fc04 refactor: Use Mutex type for some mutexes in CNode class (Hennadii Stepanov)
Pull request description:
No need the `RecursiveMutex` type for the `CNode::cs_vSend`, `CNode::cs_hSocket` and `CNode::cs_vRecv`.
Related to #19303.
ACKs for top commit:
jnewbery:
utACK 0e51a35512
MarcoFalke:
review ACK 0e51a355128a825d428fe2b9017c25085731fc04 🔊
Tree-SHA512: 678ee5e3c15ad21a41cb86ec7179741bd505a138638fdc07f41d6d677c38fbf2208219bfc0509e3675e721fc8d8816e858070db7b87c5d72ad93aae81f7e1636
819d03b932134ee91df3b0fe98a481a331ce57bf refactor: took out unused member functions (Zero)
ed69213c2b2a99023bdee5168614cb8b71990f5f build: enable unused member function diagnostic (Zero)
Pull request description:
This PR enables the `-Wunused-member-function` compiler diagnostic, as discussed in #19702.
> **Notice**: The `unused-member-function` diagnostic is only available on clang. Therefore, clang should be used to test this PR.
- [x] Include the `-Wunused-member-function`diagnostic in `./configure.ac`. (ed69213c2b2a99023bdee5168614cb8b71990f5f)
- [x] Resolve the reported warnings. (819d03b932134ee91df3b0fe98a481a331ce57bf)
Currently, enabling this flag no longer reports the following warnings:
> **Note**: output from `make 2>&1 | grep "warning: unused member function" | sort | uniq -c`
```
1 index/blockfilterindex.cpp:54:5: warning: unused member function 'DBHeightKey' [-Wunused-member-function]
2 script/bitcoinconsensus.cpp:50:9: warning: unused member function 'GetType' [-Wunused-member-function]
1 test/util_tests.cpp:1975:14: warning: unused member function 'operator=' [-Wunused-member-function]
```
All tests have passed locally (from `make check` & `src/test/test_bitcoin`).
This PR closes#19702.
ACKs for top commit:
practicalswift:
ACK 819d03b932134ee91df3b0fe98a481a331ce57bf - patch still looks correct :)
MarcoFalke:
ACK 819d03b932134ee91df3b0fe98a481a331ce57bf
pox:
Tested ACK 819d03b932134ee91df3b0fe98a481a331ce57bf with clang after `make clean`. No unused member function warnings.
theStack:
tested ACK 819d03b932134ee91df3b0fe98a481a331ce57bf
Tree-SHA512: 5fdfbbb02b3dc618a90a874a5caa5e01e596fc1d14a209e75a6981f01b253f9bca0cfac8fdd758dd7151986609fb76571c3745124a29cfd4f8cbb8d82a07272e
1112035d32ffe73a4522226c8cb2f6a5878d3ada doc: fix various typos (Ikko Ashimine)
e8640849c775efcf202dbd34736fed8d61379c49 doc: Use https URLs where possible (Sawyer Billings)
Pull request description:
Consolidates / fixes the changes from #20762, #20836, #20810. There is no output when `test/lint/lint-all.sh` is run.
Closes#20807.
ACKs for top commit:
MarcoFalke:
ACK 1112035d32ffe73a4522226c8cb2f6a5878d3ada
Tree-SHA512: 22ca824688758281a74e5ebc6a84a358142351434e34c88c6b36045d2d241ab95fd0958565fd2060f98317e62e683323b5320cc7ec13592bf340e6922294ed78
