| Age | Commit message (Collapse) | Author |
|---|
|
feeler connections
eaf6be0114a6d7763767da9496907fe8a670ff9e [net processing] Do not request transaction relay from feeler connections (John Newbery)
0220b834b175dc8c45a2c60213474a72c0ef8193 [test] Add testing for outbound feeler connections (John Newbery)
Pull request description:
Feelers are short-lived connections used to test the viability of peers. The bitcoind node will periodically open feeler connections to addresses in its addrman, wait for a `version` message from the peer, and then close the connection.
Currently, we set `fRelay` to `1` in the `version` message for feeler connections, indicating that we want the peer to relay transactions to us. However, we close the connection immediately on receipt of the `version` message, and so never process any incoming transaction announcements. This PR changes that behaviour to instead set `fRelay` to `0` indicating that we do not wish to receive transaction announcements from the peer.
This PR also extends the `addconnection` RPC to allow creating outbound feeler connections from the node to the test framework, and a test to verify that the node sets `fRelay` to `0` in the `version` message to feeler connections.
ACKs for top commit:
naumenkogs:
ACK eaf6be0114a6d7763767da9496907fe8a670ff9e
MarcoFalke:
review ACK eaf6be0114a6d7763767da9496907fe8a670ff9e 🏃
Tree-SHA512: 1c56837dbd0a396fe404a5e39f7459864d15f666664d6b35ad109628b13158e077e417e586bf48946a23bd5cbe63716cb4bf22cdf8781b74dfce6047b87b465a
|
|
-BEGIN VERIFY SCRIPT-
ren() { sed -i "s/\<$1\>/$2/g" $( git grep -l "$1" ./src/ ) ; }
ren nLastBlockTime m_last_block_time
ren nLastTXTime m_last_tx_time
ren nTimeConnected m_connected
-END VERIFY SCRIPT-
|
|
This allows to revert the temporary commit
0bfb9208df556f77cddfedfd73e5811a0e031d34 (test: fix test failures in
test/functional/p2p_timeouts.py).
|
|
|
|
|
|
up test
fadc0c80ae4e526fb2b503f7cc02f6122aaf1de5 p2p: Make timeout mockable and type safe, speed up test (MarcoFalke)
fa6d5a238d2c94440105ddd4f1554f85659d6c5b scripted-diff: Rename m_last_send and m_last_recv (MarcoFalke)
Pull request description:
Use type-safe time for better code readability/maintainability and mockable time for better testability. This speeds up the p2p_timeout test.
This is also a bugfix for intermittent test issues like: https://cirrus-ci.com/task/4769904156999680?command=ci#L2836
Fixes #20654
ACKs for top commit:
laanwj:
Code review ACK fadc0c80ae4e526fb2b503f7cc02f6122aaf1de5
naumenkogs:
ACK fadc0c80ae4e526fb2b503f7cc02f6122aaf1de5
Tree-SHA512: 28c6544c97f188c8a0fbc80411c74ab74ffd055885322c325aa3d1c404b29c3fd70a737e86083eecae58ef394db1cb56bc122d06cff63742aa89a8e868730c64
|
|
|
|
11daf6ceb1d9ea1f8d638b123eecfe39d162a7c3 More Span simplifications (Pieter Wuille)
568dd2f83900a11a4dbba1250722791a135bf0a9 Replace MakeSpan helper with Span deduction guide (Pieter Wuille)
Pull request description:
C++17 supports [user-defined deduction guides](https://en.cppreference.com/w/cpp/language/class_template_argument_deduction), allowing class constructors to be invoked without specifying class template arguments. Instead, the code can contain rules to infer the template arguments from the constructor argument types.
This alleviates the need for the `MakeSpan` helper. Convert the existing MakeSpan rules into deduction rules for `Span` itself, and replace all invocations of `MakeSpan` with just `Span` ones.
ACKs for top commit:
MarcoFalke:
re-ACK 11daf6ceb1d9ea1f8d638b123eecfe39d162a7c3 Only change is removing a hunk in the tests 🌕
Tree-SHA512: 10f3e82e4338f39d9b7b407cd11aac7ebe1e9191b58e3d7f4e5e338a4636c0e126b4a1d912127c7446f57ba356c8d6544482e47f97901efea6a54fffbfd7895f
|
|
fa551b3bdd380bcaa8fa929b378b3b6c81a6f65c Remove GetAdjustedTime from init.cpp (MarcoFalke)
fa815f8473c56df66302340c5961d18226a60e6f Replace addrman.h include with forward decl in net.h (MarcoFalke)
Pull request description:
It seems confusing to call `GetAdjustedTime` there, because no offset could have been retrieved from the network at this point. Even if connman was started, `timedata` needs at least 5 peer connections to calculate an offset.
Fix the confusion by replacing `GetAdjustedTime` with `GetTime`, which does not change behavior.
Also:
* Replace magic number with `MAX_FUTURE_BLOCK_TIME` to clarify the context
* Add test, which passes both on current master and this pull request
* An unrelated refactoring commit, happy to drop
ACKs for top commit:
dongcarl:
Code Review ACK fa551b3bdd380bcaa8fa929b378b3b6c81a6f65c, noticed the exact same thing here: https://github.com/bitcoin/bitcoin/pull/23280/commits/e073634c37f3a1e140920c6e5e3f2c1ae47cd293
mzumsande:
Code Review ACK fa551b3bdd380bcaa8fa929b378b3b6c81a6f65c
jnewbery:
Code review ACK fa551b3bdd380bcaa8fa929b378b3b6c81a6f65c
shaavan:
ACK fa551b3bdd380bcaa8fa929b378b3b6c81a6f65c
theStack:
Code-review ACK fa551b3bdd380bcaa8fa929b378b3b6c81a6f65c
Tree-SHA512: 15807a0e943e3e8d8c5250c8f6d7b56afb26002b1e290bf93636a2c747f27e78f01f1de04ce1a83d6339e27284c69c43e077a8467545c4078746f4c1ecb1164d
|
|
Change `CConnman::CreateNodeFromAcceptedSocket()` to take a `Sock`
argument instead of `SOCKET`.
This makes the method mockable and also a little bit shorter as some
`CloseSocket()` calls are removed (the socket will be closed
automatically by the `Sock` destructor on early return).
|
|
Change `CConnman::ListenSocket` to use a pointer to `Sock` instead of a
bare `SOCKET` and use `Sock::Accept()` instead of bare `accept()`. This
will help mocking / testing / fuzzing more code.
|
|
Also, add missing addrman.h includes
|
|
Based on suggestions by MarcoFalke <falke.marco@gmail.com>
|
|
Listening sockets in `CConnman::vhListenSocket` are always valid
(underlying file descriptor is not `INVALID_SOCKET`).
|
|
-BEGIN VERIFY SCRIPT-
ren() { sed -i "s/$1/$2/g" $3 $4 $5; }
ren cs_vAddedNodes m_added_nodes_mutex src/net.h src/net.cpp
ren vAddedNodes m_added_nodes src/net.h src/net.cpp
ren cs_vNodes m_nodes_mutex src/net.h src/net.cpp src/test/util/net.h
ren vNodesDisconnectedCopy nodes_disconnected_copy src/net.cpp
ren vNodesDisconnected m_nodes_disconnected src/net.h src/net.cpp
ren vNodesCopy nodes_copy src/net.cpp
ren vNodesSize nodes_size src/net.cpp
ren vNodes m_nodes src/net.h src/net.cpp src/test/util/net.h
-END VERIFY SCRIPT-
|
|
The RecursiveMutex cs_totalBytesRecv is only used at two places: in
CConnman::RecordBytesRecv() to increment the nTotalBytesRecv member, and in
CConnman::GetTotalBytesRecv() to read it. For this simple use-case, we can
make the member std::atomic instead to achieve the same result.
|
|
CConnman::vNodes
f52b6b2d9f482353821da0ef4c485c402a396c8d net: split CConnman::SocketHandler() (Vasil Dimov)
c7eb19ec8302e6a5abd89c0566540c2c862e9121 style: remove unnecessary braces (Vasil Dimov)
664ac22c5379db65757fc3aab91fff8765683e7f net: keep reference to each node during socket wait (Vasil Dimov)
75e8bf55f5a014faada7712a9640dc35e8c86f15 net: dedup and RAII-fy the creation of a copy of CConnman::vNodes (Vasil Dimov)
Pull request description:
_This is a piece of https://github.com/bitcoin/bitcoin/pull/21878, chopped off to ease review._
The following pattern was duplicated in CConnman:
```cpp
lock
create a copy of vNodes, add a reference to each one
unlock
... use the copy ...
lock
release each node from the copy
unlock
```
Put that code in a RAII helper that reduces it to:
```cpp
create snapshot "snap"
... use the copy ...
// release happens when "snap" goes out of scope
ACKs for top commit:
jonatack:
ACK f52b6b2d9f482353821da0ef4c485c402a396c8d changes since last review are reordered commits, removing an unneeded local variable, and code formatting and documentation improvements
LarryRuane:
code review ACK f52b6b2d9f482353821da0ef4c485c402a396c8d
promag:
Code review ACK f52b6b2d9f482353821da0ef4c485c402a396c8d, only format changes and comment tweaks since last review.
Tree-SHA512: 5ead7b4c641ebe5b215e7baeb7bc0cdab2a588b2871d9a343a1d518535c55c0353d4e46de663f41513cdcc79262938ccea3232f6d5166570fc2230286c985f68
|
|
-BEGIN VERIFY SCRIPT-
sed -i 's/\<proxyType\>/Proxy/g' $(git grep -l proxyType)
-END VERIFY SCRIPT-
|
|
`CConnman::SocketHandler()` does 3 things:
1. Check sockets for readiness
2. Process ready listening sockets
3. Process ready connected sockets
Split the processing (2. and 3.) into separate methods to make the code
easier to grasp.
Also, move the processing of listening sockets after the processing of
connected sockets to make it obvious that there is no dependency and
also explicitly release the snapshot before dealing with listening
sockets - it is only necessary for the connected sockets part.
|
|
They were needed to define the scope of `LOCK(cs_vNodes)` which was
removed in the previous commit. Re-indent in a separate commit to ease
review (use `--ignore-space-change`).
|
|
Create the snapshot of `CConnman::vNodes` to operate on earlier in
`CConnman::SocketHandler()`, before calling `CConnman::SocketEvents()`
and pass the `vNodes` copy from the snapshot to `SocketEvents()`.
This will keep the refcount of each node incremented during
`SocketEvents()` so that the `CNode` object is not destroyed before
`SocketEvents()` has finished.
Currently in `SocketEvents()` we only remember file descriptor numbers
(when not holding `CConnman::cs_vNodes`) which is safe, but we will
change this to remember pointers to `CNode::m_sock`.
|
|
The following pattern was duplicated in CConnman:
```cpp
lock
create a copy of vNodes, add a reference to each one
unlock
... use the copy ...
lock
release each node from the copy
unlock
```
Put that code in a RAII helper that reduces it to:
```cpp
create snapshot "snap"
... use the copy ...
// release happens when "snap" goes out of scope
```
|
|
|
|
-BEGIN VERIFY SCRIPT-
ren() { sed -i "s/\<$1\>/$2/g" $(git grep -l "$1" ./src) ; }
ren nLastSend m_last_send
ren nLastRecv m_last_recv
-END VERIFY SCRIPT-
|
|
In some cases addresses come from an external source as a string or as a
`struct sockaddr_in6`, without a tag to tell whether it is a private
IPv6 or a CJDNS address. In those cases interpret the address as a CJDNS
address instead of an IPv6 address if `-cjdnsreachable` is set and the
seemingly-IPv6-address belongs to `fc00::/8`. Those external sources are:
* `-externalip=`
* `-bind=`
* UPnP
* `getifaddrs(3)` (called through `-discover`)
* `addnode`
* `connect`
* incoming connections (returned by `accept(2)`)
|
|
TransportDeserializer::GetMessage()
f3e451bebfe2e2d8de901d8ac29c064a51d3b746 [net] Replace GetID() with id in TransportDeserializer constructor (Troy Giorshev)
8c96008ab18075abca03bff6b3675643825a21ca [net] Don't return an optional from TransportDeserializer::GetMessage() (Troy Giorshev)
Pull request description:
Also, access mapRecvBytesPerMsgCmd with `at()` not `find()`. This
throws an error if COMMAND_OTHER doesn't exist, which should never
happen. `find()` instead just accessed the last element, which could make
debugging more difficult.
Resolves review comments from PR19107:
- https://github.com/bitcoin/bitcoin/pull/19107#discussion_r478718436
- https://github.com/bitcoin/bitcoin/pull/19107#discussion_r478714497
ACKs for top commit:
theStack:
Code-review ACK f3e451bebfe2e2d8de901d8ac29c064a51d3b746
ryanofsky:
Code review ACK f3e451bebfe2e2d8de901d8ac29c064a51d3b746. Changes since last review in https://github.com/bitcoin/bitcoin/pull/20364#pullrequestreview-534369904 were simplifying by dropping the third commit, rebasing, and cleaning up some style & comments in the first commit.
Tree-SHA512: 37de4b25646116e45eba50206e82ed215b0d9942d4847a172c104da4ed76ea4cee29a6fb119f3c34106a9b384263c576cb8671d452965a468f358d4a3fa3c003
|
|
|
|
details
021f86953e8a1dff8ecc768186368d345c865cc2 [style] Run changed files through clang formatter. (Amiti Uttarwar)
375750387e35ed751d1f5ab48860bdec93977f64 scripted-diff: Rename CAddrInfo to AddrInfo (Amiti Uttarwar)
dd8f7f250095e58bbf4cd4effb481b52143bd1ed scripted-diff: Rename CAddrMan to AddrMan (Amiti Uttarwar)
3c263d3f63c3598954ee2b65a0e721e3c22e52f8 [includes] Fix up included files (Amiti Uttarwar)
29727c2aa1233f7c5b91a17884c405e0aef10c6e [doc] Update comments (Amiti Uttarwar)
14f9e000d05f82b364d5a142cafc70b10406b660 [refactor] Update GetAddr_() function signature (Amiti Uttarwar)
40acd6fc9a8098fed85abf4fb727a5f0dff8a2ff [move-only] Move constants to test-only header (Amiti Uttarwar)
7cf41bbb38db5008f9b69037b88138076d6a6cc5 [addrman] Change CAddrInfo access (Amiti Uttarwar)
e3f1ea659c9eb1e8be4579923d6acaaab148c2ef [move-only] Move CAddrInfo to test-only header file (Amiti Uttarwar)
7cba9d56185b9325ce41d79364e448462fff0f6a [net, addrman] Remove external dependencies on CAddrInfo objects (Amiti Uttarwar)
8af5b54f973e11c847345418d8631bc301b96130 [addrman] Introduce CAddrMan::Impl to encapsulate addrman implementation. (Amiti Uttarwar)
f2e5f38f09ee40933f752680fe7d75ee8e529fae [move-only] Match ordering of CAddrMan declarations and definitions (Amiti Uttarwar)
5faa7dd6d871eac1a0ec5c4a93f2ad7577781a56 [move-only] Move CAddrMan function definitions to cpp (Amiti Uttarwar)
Pull request description:
Introduce the pimpl pattern for AddrMan to separate the implementation details from the externally used object representation. This reduces compile-time dependencies and conceptually clarifies AddrMan's interface from the implementation specifics.
Since the unit & fuzz tests currently rely on accessing AddrMan internals, this PR introduces addrman_impl.h, which is exclusively imported by addrman.cpp and test files.
ACKs for top commit:
jnewbery:
ACK 021f86953e8a1dff8ecc768186368d345c865cc2
GeneFerneau:
utACK [021f869](https://github.com/bitcoin/bitcoin/pull/22950/commits/021f86953e8a1dff8ecc768186368d345c865cc2)
mzumsande:
ACK 021f86953e8a1dff8ecc768186368d345c865cc2
rajarshimaitra:
Concept + Code Review ACK https://github.com/bitcoin/bitcoin/pull/22950/commits/021f86953e8a1dff8ecc768186368d345c865cc2
theuni:
ACK 021f86953e8a1dff8ecc768186368d345c865cc2
Tree-SHA512: aa70cb77927a35c85230163c0cf6d3872382d79048b0fb79341493caa46f8e91498cb787d8b06aba4da17b2f921f2230e73f3d66385519794fff86a831b3a71d
|
|
secure computing mode)
4747da3a5b639b5a336b737e7e3cbf060cf2efcf Add syscall sandboxing (seccomp-bpf) (practicalswift)
Pull request description:
Add experimental syscall sandboxing using seccomp-bpf (Linux secure computing mode).
Enable filtering of system calls using seccomp-bpf: allow only explicitly allowlisted (expected) syscalls to be called.
The syscall sandboxing implemented in this PR is an experimental feature currently available only under Linux x86-64.
To enable the experimental syscall sandbox the `-sandbox=<mode>` option must be passed to `bitcoind`:
```
-sandbox=<mode>
Use the experimental syscall sandbox in the specified mode
(-sandbox=log-and-abort or -sandbox=abort). Allow only expected
syscalls to be used by bitcoind. Note that this is an
experimental new feature that may cause bitcoind to exit or crash
unexpectedly: use with caution. In the "log-and-abort" mode the
invocation of an unexpected syscall results in a debug handler
being invoked which will log the incident and terminate the
program (without executing the unexpected syscall). In the
"abort" mode the invocation of an unexpected syscall results in
the entire process being killed immediately by the kernel without
executing the unexpected syscall.
```
The allowed syscalls are defined on a per thread basis.
I've used this feature since summer 2020 and I find it to be a helpful testing/debugging addition which makes it much easier to reason about the actual capabilities required of each type of thread in Bitcoin Core.
---
Quick start guide:
```
$ ./configure
$ src/bitcoind -regtest -debug=util -sandbox=log-and-abort
…
2021-06-09T12:34:56Z Experimental syscall sandbox enabled (-sandbox=log-and-abort): bitcoind will terminate if an unexpected (not allowlisted) syscall is invoked.
…
2021-06-09T12:34:56Z Syscall filter installed for thread "addcon"
2021-06-09T12:34:56Z Syscall filter installed for thread "dnsseed"
2021-06-09T12:34:56Z Syscall filter installed for thread "net"
2021-06-09T12:34:56Z Syscall filter installed for thread "msghand"
2021-06-09T12:34:56Z Syscall filter installed for thread "opencon"
2021-06-09T12:34:56Z Syscall filter installed for thread "init"
…
# A simulated execve call to show the sandbox in action:
2021-06-09T12:34:56Z ERROR: The syscall "execve" (syscall number 59) is not allowed by the syscall sandbox in thread "msghand". Please report.
…
Aborted (core dumped)
$
```
---
[About seccomp and seccomp-bpf](https://en.wikipedia.org/wiki/Seccomp):
> In computer security, seccomp (short for secure computing mode) is a facility in the Linux kernel. seccomp allows a process to make a one-way transition into a "secure" state where it cannot make any system calls except exit(), sigreturn(), and read() and write() to already-open file descriptors. Should it attempt any other system calls, the kernel will terminate the process with SIGKILL or SIGSYS. In this sense, it does not virtualize the system's resources but isolates the process from them entirely.
>
> […]
>
> seccomp-bpf is an extension to seccomp that allows filtering of system calls using a configurable policy implemented using Berkeley Packet Filter rules. It is used by OpenSSH and vsftpd as well as the Google Chrome/Chromium web browsers on Chrome OS and Linux. (In this regard seccomp-bpf achieves similar functionality, but with more flexibility and higher performance, to the older systrace—which seems to be no longer supported for Linux.)
ACKs for top commit:
laanwj:
Code review and lightly tested ACK 4747da3a5b639b5a336b737e7e3cbf060cf2efcf
Tree-SHA512: e1c28e323eb4409a46157b7cc0fc29a057ba58d1ee2de268962e2ade28ebd4421b5c2536c64a3af6e9bd3f54016600fec88d016adb49864b63edea51ad838e17
|
|
|
|
-BEGIN VERIFY SCRIPT-
git grep -l CAddrMan src/ test/ | xargs sed -i 's/CAddrMan/AddrMan/g'
-END VERIFY SCRIPT-
|
|
CAddrInfo objects are an implementation detail of how AddrMan manages and adds
metadata to different records. Encapsulate this logic by updating Select &
SelectTriedCollision to return the additional info that the callers need.
|
|
Extend the addconnection RPC method to allow creating outbound
feeler connections. Extend the test framework to accept those
feeler connections.
|
|
`mapLocalHost`
330d3aa1a2c740dfa31bed3a6ed6b5f88e5426ad refactor: net: avoid duplicate map lookups to `mapLocalHost` (Sebastian Falbesoner)
Pull request description:
This simple refactoring PR aims to avoid duplicate lookups to `mapLocalHost`: instead of calling `count()` (to first find out whether a key is in the map) and then `operator[]` (to get the value to the passed key, or default-construct one if not found), use either
* `find()` and dereference the returned iterator (for simple lookups), see https://www.cplusplus.com/reference/map/map/find/
* `emplace()` and use the returned <iterator, inserted> pair (for lookups where a new element should be inserted if the key isn't found), see https://www.cplusplus.com/reference/map/map/emplace/
ACKs for top commit:
naumenkogs:
ACK 330d3aa1a2c740dfa31bed3a6ed6b5f88e5426ad
jonatack:
Code review ACK 330d3aa1a2c740dfa31bed3a6ed6b5f88e5426ad plus rebase to master + debug build
Tree-SHA512: d13da6a927ff561eee8ac6b093bf3586dfe31d6c94173a5a6d8f3698e0ee224fb394d3635155d5141c165da59d2c2c37260122eb4f2e8bcda3e8a29b901d213e
|
|
|
|
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
|
|
This saves passing around a reference to the asmap std::vector<bool>.
|
|
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
|
|
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
|
|
Improve readability of code, simplify future scripted diff cleanup PRs, and be
more consistent with naming for GetBoolArg.
This will also be useful for replacing runtime settings type checking
with compile time checking.
-BEGIN VERIFY SCRIPT-
git grep -l GetArg | xargs sed -i 's/GetArg(\([^)]*\( [0-9]\+\|-1\|port\|BaseParams().RPCPort()\|Params().GetDefaultPort()\|_TIMEOUT\|Height\|_WORKQUEUE\|_THREADS\|_CONNECTIONS\|LIMIT\|SigOp\|Bytes\|_VERSION\|_AGE\|_CHECKS\|Checks() ? 1 : 0\|_BANTIME\|Cache\|BLOCKS\|LEVEL\|Weight\|Version\|BUFFER\|TARGET\|WEIGHT\|TXN\|TRANSACTIONS\|ADJUSTMENT\|i64\|Size\|nDefault\|_EXPIRY\|HEIGHT\|SIZE\|SNDHWM\|_TIME_MS\)\))/GetIntArg(\1)/g'
-END VERIFY SCRIPT-
Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
|
|
Add a GetAsmap() getter function that returns a reference to const.
|
|
The m_ prefix indicates that a variable is a data member. Using it as
a parameter name is misleading.
Also update the name of the function from copyStats to CopyStats to
comply with our style guide.
|
|
https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines.html#c131-avoid-trivial-getters-and-setters
|
|
|
|
This logic is a no-op since it was introduced in commit
f9f5cfc50637f2cd1540923caf337e2651ec1625.
m_addr_name is never initialized to the empty string, because
ToStringIPPort never returns an empty string.
|
|
|
|
Also, access mapRecvBytesPerMsgCmd with `at()` not `find()`. This
throws an error if COMMAND_OTHER doesn't exist, which should never
happen. `find()` instead just accessed the last element, which could make
debugging more difficult.
Resolves review comments from PR19107:
- https://github.com/bitcoin/bitcoin/pull/19107#discussion_r478718436
- https://github.com/bitcoin/bitcoin/pull/19107#discussion_r478714497
|
|
|
