Age | Commit message (Collapse) | Author |
|
and SetSocketNonBlocking() to Sock methods
b527b549504672704a61f70d2565b9489aaaba91 net: convert standalone SetSocketNonBlocking() to Sock::SetNonBlocking() (Vasil Dimov)
29f66f76826056f53d971ac734b7ed49b39848d3 moveonly: move SetSocketNonBlocking() from netbase to util/sock (Vasil Dimov)
b4bac556791b5bb8aa118d4c1fed42c3fe45550c net: convert standalone IsSelectableSocket() to Sock::IsSelectable() (Vasil Dimov)
5db7d2ca0aa51ff25f97bf21ce0cbc9e6b741cbd moveonly: move IsSelectableSocket() from compat.h to sock.{h,cpp} (Vasil Dimov)
Pull request description:
_This is a piece of #21878, chopped off to ease review._
* convert standalone `IsSelectableSocket()` to `Sock::IsSelectable()`
* convert standalone `SetSocketNonBlocking()` to `Sock::SetNonBlocking()`
This further encapsulates syscalls inside the `Sock` class and makes the callers mockable.
ACKs for top commit:
jonatack:
ACK b527b549504672704a61f70d2565b9489aaaba91 review/debug build/unit tests at each commit, cross-referenced the changes with `man select` and `man errno`, ran a signet node on the last commit with ip4/ip6//tor/i2p/cjdns and network connections were nominal
dergoegge:
Code review ACK b527b549504672704a61f70d2565b9489aaaba91
Tree-SHA512: af783ce558c7a89e173f7ab323fb3517103d765c19b5d14de29f64706b4e1fea3653492e8ea73ae972699986aaddf2ae72c7cfaa7dad7614254283083b7d2632
|
|
This avoids initializing variables with the copy-constructor of a
non-trivially copyable type.
|
|
This further encapsulates syscalls inside the `Sock` class.
Co-authored-by: practicalswift <practicalswift@users.noreply.github.com>
|
|
To be converted to a method of the `Sock` class.
|
|
This makes the callers mockable.
|
|
|
|
-BEGIN VERIFY SCRIPT-
sed -i -E -e '/^([a-z]+ )?Mutex [a-z]/ s/Mutex/GlobalMutex/' $(git grep -lE '^([a-z]+ )?Mutex [a-z]')
-END VERIFY SCRIPT-
|
|
-BEGIN VERIFY SCRIPT-
sed -i 's,ValidAsCString,ContainsNoNUL,g' $(git grep -l ValidAsCString)
-END VERIFY SCRIPT-
|
|
|
|
Since the former is mockable, this makes it easier to test higher level
code that sets the TCP_NODELAY flag.
|
|
|
|
c71117fcb04fc2e45b5e76fe96b077a07b0c0f82 net: remove non-blocking bool from interface (Bushstar)
Pull request description:
SetSocketNonBlocking was added in 0.11 in the PR below with a second argument to toggle non-blocking mode for the socket. That argument has always been set to true in all subsequent releases and I'm not sure why it is present.
https://github.com/bitcoin/bitcoin/pull/4491
ACKs for top commit:
promag:
Code review ACK c71117fcb04fc2e45b5e76fe96b077a07b0c0f82.
lsilva01:
Code review ACK https://github.com/bitcoin/bitcoin/pull/22052/commits/c71117fcb04fc2e45b5e76fe96b077a07b0c0f82
vasild:
ACK c71117fcb04fc2e45b5e76fe96b077a07b0c0f82
Tree-SHA512: feebfcfa75d997460a0ba42ffe1e0c25a7e0bfcad12510ad73ea4942cc1c653f9ad429adbbb00b9288fe319009552906fcb635a14dfd7dcbde3853baab6be065
|
|
on non-default ports
36ee76d1afbb278500fc8aa01606ec933b52c17d net: remove unused CNetAddr::GetHash() (Vasil Dimov)
d0abce9a50dd4f507e3a30348eabffb7552471d5 net: include the port when deciding a relay destination (Vasil Dimov)
2e38a0e6865187d1f0d0f016d3df7cce414a7c4f net: add CServiceHash constructor so the caller can provide the salts (Vasil Dimov)
97208634b96f2d9a55f2ead7b0ef407da729d7bd net: open p2p connections to nodes that listen on non-default ports (Vasil Dimov)
Pull request description:
By default, for mainnet, the p2p listening port is 8333. Bitcoin Core
has a strong preference for only connecting to nodes that listen on that
port.
Remove that preference because connections over clearnet that involve
port 8333 make it easy to detect, analyze, block or divert Bitcoin p2p
traffic before the connection is even established (at TCP SYN time).
For further justification see the OP of:
https://github.com/bitcoin/bitcoin/pull/23306
ACKs for top commit:
laanwj:
Concept and light code review ACK 36ee76d1afbb278500fc8aa01606ec933b52c17d
prayank23:
ACK https://github.com/bitcoin/bitcoin/pull/23542/commits/36ee76d1afbb278500fc8aa01606ec933b52c17d
stickies-v:
tACK 36ee76d1a
jonatack:
ACK 36ee76d1afbb278500fc8aa01606ec933b52c17d
glozow:
utACK 36ee76d1afbb278500fc8aa01606ec933b52c17d
Tree-SHA512: 7f45ab7567c51c19fc50fabbaf84f0cc8883a8eef84272b76435c014c31d89144271d70dd387212cc1114213165d76b4d20a5ddb8dbc958fe7e74e6ddbd56d11
|
|
connections
0eea83a85ec6b215d44facc2b16ee1b035275a6b scripted-diff: rename `proxyType` to `Proxy` (Vasil Dimov)
e53a8505dbb6f9deaae8ac82793a4fb760a1e0a6 net: respect -onlynet= when making outbound connections (Vasil Dimov)
Pull request description:
Do not make outbound connections to hosts which belong to a network
which is restricted by `-onlynet`.
This applies to hosts that are automatically chosen to connect to and to
anchors.
This does not apply to hosts given to `-connect`, `-addnode`,
`addnode` RPC, dns seeds, `-seednode`.
Fixes https://github.com/bitcoin/bitcoin/issues/13378
Fixes https://github.com/bitcoin/bitcoin/issues/22647
Supersedes https://github.com/bitcoin/bitcoin/pull/22651
ACKs for top commit:
naumenkogs:
utACK 0eea83a85ec6b215d44facc2b16ee1b035275a6b
prayank23:
reACK https://github.com/bitcoin/bitcoin/pull/22834/commits/0eea83a85ec6b215d44facc2b16ee1b035275a6b
jonatack:
ACK 0eea83a85ec6b215d44facc2b16ee1b035275a6b code review, rebased to master, debug built, and did some manual testing with various config options on signet
Tree-SHA512: 37d68b449dd6d2715843fc84d85f48fa2508be40ea105a7f4a28443b318d0b6bd39e3b2ca2a6186f2913836adf08d91038a8b142928e1282130f39ac81aa741b
|
|
By default, for mainnet, the p2p listening port is 8333. Bitcoin Core
has a strong preference for only connecting to nodes that listen on that
port.
Remove that preference because connections over clearnet that involve
port 8333 make it easy to detect, analyze, block or divert Bitcoin p2p
traffic before the connection is even established (at TCP SYN time).
For further justification see the OP of:
https://github.com/bitcoin/bitcoin/pull/23306
|
|
-BEGIN VERIFY SCRIPT-
./contrib/devtools/copyright_header.py update ./
-END VERIFY SCRIPT-
Commits of previous years:
* 2020: fa0074e2d82928016a43ca408717154a1c70a4db
* 2019: aaaaad6ac95b402fe18d019d67897ced6b316ee0
|
|
|
|
- consistent param naming between function declaration and definition
- brackets, param naming and localvar naming per current standards
in doc/developer-notes.md
- update/improve doxygen documentation in the declaration
- improve comments and other localvar names
- constness
- named args
|
|
LookupHost()
a989f98d240a84b5c798252acaa4a316ac711189 refactor: net: subnet lookup: use single-result LookupHost() (Sebastian Falbesoner)
Pull request description:
plus describe single IP subnet case for more clarity
ACKs for top commit:
jonatack:
utACK a989f98d240a84b5c798252acaa4a316ac711189 the patch rebases cleanly to master, the debug build is green, and it is essentially the same patch as c8991f0251dd2a modulo local variable naming, braced initialization, and a comment
vasild:
ACK a989f98d240a84b5c798252acaa4a316ac711189
Tree-SHA512: 082d3481b1fa5e5f3267b7c4a812954b67b36d1f94c5296fe20110699f053e5042dfa13f728ae20249e9b8d71e930c3b119410125d0faeccdfbdc259223ee3a6
|
|
-BEGIN VERIFY SCRIPT-
sed -i 's/\<proxyType\>/Proxy/g' $(git grep -l proxyType)
-END VERIFY SCRIPT-
|
|
|
|
This allows to use "cjdns" as an argument to the `getnodeaddresses` RPC
and to the `-onlynet=` parameter.
|
|
|
|
|
|
40316a37cb02cf8a9a8b2cbd4d7153ffa57e7ec5 test: add I2P test for a runaway SAM proxy (Vasil Dimov)
2d8ac779708322e1235e823edfc9c8f6e2dd65e4 fuzz: add tests for the I2P Session public interface (Vasil Dimov)
9947e44de0cbd79e99d883443a9ac441d8c69713 i2p: use pointers to Sock to accommodate mocking (Vasil Dimov)
82d360b5a88d9057b6c09b61cd69e426c7a2412d net: change ConnectSocketDirectly() to take a Sock argument (Vasil Dimov)
b5861100f85fef77b00f55dcdf01ffb4a2a112d8 net: add connect() and getsockopt() wrappers to Sock (Vasil Dimov)
5a887d49b2b39e59d7cce8e9d5b89c21ad694f8b fuzz: avoid FuzzedSock::Recv() repeated errors with EAGAIN (Vasil Dimov)
3088f83d016e7ebb6e6aa559e6326fa0ef0d6282 fuzz: extend FuzzedSock::Recv() to support MSG_PEEK (Vasil Dimov)
9b05c49ade729311a0f4388a109530ff8d0ed1f9 fuzz: implement unimplemented FuzzedSock methods (Vasil Dimov)
Pull request description:
Change the networking code and the I2P code to be fully mockable and use `FuzzedSocket` to fuzz the I2P methods `Listen()`, `Accept()` and `Connect()`.
Add a mocked `Sock` implementation that returns a predefined data on reads and use it for a regression unit test for the bug fixed in https://github.com/bitcoin/bitcoin/pull/21407.
ACKs for top commit:
practicalswift:
Tested ACK 40316a37cb02cf8a9a8b2cbd4d7153ffa57e7ec5
MarcoFalke:
Concept ACK 40316a37cb
jonatack:
re-ACK 40316a37cb02cf8a9a8b2cbd4d7153ffa57e7ec5 reviewed `git range-diff 01bb3afb 23c861d 40316a3` and the new unit test commit, debug built, ran unit tests, ran bitcoind with an I2P service and network operation with seven I2P peers (2 in, 5 out) is looking nominal
laanwj:
Code review ACK 40316a37cb02cf8a9a8b2cbd4d7153ffa57e7ec5
Tree-SHA512: 7fc4f129849e16e0c7e16662d9f4d35dfcc369bb31450ee369a2b97bdca95285533bee7787983e881e5a3d248f912afb42b4a2299d5860ace7129b0b19623cc8
|
|
|
|
Change `ConnectSocketDirectly()` to take a `Sock` argument instead of a
bare `SOCKET`. With this, use the `Sock`'s (possibly mocked) methods
`Connect()`, `Wait()` and `GetSockOpt()` instead of calling the OS
functions directly.
|
|
plus describe single IP subnet case for more clarity
|
|
|
|
|
|
|
|
timeout logic
|
|
|
|
So that help texts include "i2p" in:
* `./bitcoind -help` (in `-onlynet` description)
* `getpeerinfo` RPC
* `getnetworkinfo` RPC
Co-authored-by: Jon Atack <jon@atack.com>
|
|
|
|
Deduplicate `MSG_NOSIGNAL` and `MSG_DONTWAIT` definitions from `net.cpp`
and `netbase.cpp` to `compat.h` where they can also be reused by other
code.
|
|
Move `maxWait` out of `InterruptibleRecv()` and rename it to
`MAX_WAIT_FOR_IO` so that it can be reused by other code.
|
|
automate helps
96635e61777add29b6a34d47767a63f43b2919af init: use GetNetworkNames() in -onlynet help (Jon Atack)
0dbde700a6e9894a8ead20f2eebd0ff6554ef2d9 rpc: use GetNetworkNames() in getnetworkinfo and getpeerinfo helps (Jon Atack)
1c3af37881adbbc37fb9924bcf69c403fcae1ae7 net: create GetNetworkNames() (Jon Atack)
b45eae4d5332f1da71ba9ec983fe7818fa4d32e9 net: update NET_UNROUTABLE to not_publicly_routable in GetNetworkName() (Jon Atack)
Pull request description:
per the IRC discussion today at http://www.erisian.com.au/bitcoin-core-dev/log-2021-01-19.html#l-87
- return a more helpful string name for `Network::NET_UNROUTABLE`: "not_publicly_routable" instead of "unroutable"
- update the RPC getpeerinfo "network" help, and automate it and the getnetworkinfo "network#name" and the -onlynet help doc generation
ACKs for top commit:
theStack:
re-ACK 96635e61777add29b6a34d47767a63f43b2919af 🌳
MarcoFalke:
review ACK 96635e61777add29b6a34d47767a63f43b2919af 🐗
Tree-SHA512: 511a7d987126b48a7a090739aa7c4964b6186a3ff8f5f7eec9233dd816c6b7a6dc91b3ea6b824aa68f218a8a3ebdc6ffd214e9a88af38f2bf23f3257c4284c3a
|
|
In the arguments of `InterruptibleRecv()`, `Socks5()` and
`ConnectThroughProxy()` the variable `hSocket` was previously of type
`SOCKET`, but has been changed to `Sock`. Thus rename it to `sock` to
imply its type, to distinguish from other `SOCKET` variables and to
abide to the coding style wrt variables' names.
|
|
Use the `Sock` class instead of `SOCKET` for `InterruptibleRecv()` and
`Socks5()`.
This way the `Socks5()` function can be tested by giving it a mocked
instance of a socket.
Co-authored-by: practicalswift <practicalswift@users.noreply.github.com>
|
|
Introduce a class to manage the lifetime of a socket - when the object
that contains the socket goes out of scope, the underlying socket will
be closed.
In addition, the new `Sock` class has a `Send()`, `Recv()` and `Wait()`
methods that can be overridden by unit tests to mock the socket
operations.
The `Wait()` method also hides the
`#ifdef USE_POLL poll() #else select() #endif` technique from higher
level code.
|
|
Move `CloseSocket()` (and `NetworkErrorString()` which it uses) from
`netbase.{h,cpp}` to newly added `src/util/sock.{h,cpp}`.
This is necessary in order to use `CloseSocket()` from a newly
introduced Sock class (which will live in `src/util/sock.{h,cpp}`).
`sock.{h,cpp}` cannot depend on netbase because netbase will depend
on it.
|
|
Move `MillisToTimeval()` from `netbase.{h,cpp}` to
`src/util/system.{h,cpp}`.
This is necessary in order to use `MillisToTimeval()` from a newly
introduced `src/util/sock.{h,cpp}` which cannot depend on netbase
because netbase will depend on it.
|
|
|
|
|
|
|
|
Before this change `CNetAddr::ip` was a fixed-size array of 16 bytes,
not being able to store larger addresses (e.g. TORv3) and encoded
smaller ones as 16-byte IPv6 addresses.
Change its type to `prevector`, so that it can hold larger addresses and
do not disguise non-IPv6 addresses as IPv6. So the IPv4 address
`1.2.3.4` is now encoded as `01020304` instead of
`00000000000000000000FFFF01020304`.
Rename `CNetAddr::ip` to `CNetAddr::m_addr` because it is not an "IP" or
"IP address" (TOR addresses are not IP addresses).
In order to preserve backward compatibility with serialization (where
e.g. `1.2.3.4` is serialized as `00000000000000000000FFFF01020304`)
introduce `CNetAddr` dedicated legacy serialize/unserialize methods.
Adjust `CSubNet` accordingly. Still use `CSubNet::netmask[]` of fixed 16
bytes, but use the first 4 for IPv4 (not the last 4). Only allow
subnetting for IPv4 and IPv6.
Co-authored-by: Carl Dong <contact@carldong.me>
|
|
removed trailing whitespace to make linter happy
|
|
|
|
characters. Add tests.
7a046cdc1423963bdcbcf9bb98560af61fa90b37 tests: Avoid using C-style NUL-terminated strings as arguments (practicalswift)
fefb9165f23fe9d10ad092ec31715f906e0d2ee7 tests: Add tests to make sure lookup methods fail on std::string parameters with embedded NUL characters (practicalswift)
9574de86ad703ad942cdd0eca79f48c0d42b102b net: Avoid using C-style NUL-terminated strings as arguments in the netbase interface (practicalswift)
Pull request description:
Don't allow resolving of `std::string`:s with embedded `NUL` characters.
Avoid using C-style `NUL`-terminated strings as arguments in the `netbase` interface
Add tests.
The only place in where C-style `NUL`-terminated strings are actually needed is here:
```diff
+ if (!ValidAsCString(name)) {
+ return false;
+ }
...
- int nErr = getaddrinfo(pszName, nullptr, &aiHint, &aiRes);
+ int nErr = getaddrinfo(name.c_str(), nullptr, &aiHint, &aiRes);
if (nErr)
return false;
```
Interface changes:
```diff
-bool LookupHost(const char *pszName, std::vector<CNetAddr>& vIP, unsigned int nMaxSolutions, bool fAllowLookup);
+bool LookupHost(const std::string& name, std::vector<CNetAddr>& vIP, unsigned int nMaxSolutions, bool fAllowLookup);
-bool LookupHost(const char *pszName, CNetAddr& addr, bool fAllowLookup);
+bool LookupHost(const std::string& name, CNetAddr& addr, bool fAllowLookup);
-bool Lookup(const char *pszName, CService& addr, int portDefault, bool fAllowLookup);
+bool Lookup(const std::string& name, CService& addr, int portDefault, bool fAllowLookup);
-bool Lookup(const char *pszName, std::vector<CService>& vAddr, int portDefault, bool fAllowLookup, unsigned int nMaxSolutions);
+bool Lookup(const std::string& name, std::vector<CService>& vAddr, int portDefault, bool fAllowLookup, unsigned int nMaxSolutions);
-bool LookupSubNet(const char *pszName, CSubNet& subnet);
+bool LookupSubNet(const std::string& strSubnet, CSubNet& subnet);
-CService LookupNumeric(const char *pszName, int portDefault = 0);
+CService LookupNumeric(const std::string& name, int portDefault = 0);
-bool ConnectThroughProxy(const proxyType &proxy, const std::string& strDest, int port, const SOCKET& hSocketRet, int nTimeout, bool *outProxyConnectionFailed);
+bool ConnectThroughProxy(const proxyType &proxy, const std::string& strDest, int port, const SOCKET& hSocketRet, int nTimeout, bool& outProxyConnectionFailed);
```
It should be noted that the `ConnectThroughProxy` change (from `bool *outProxyConnectionFailed` to `bool& outProxyConnectionFailed`) has nothing to do with `NUL` handling but I thought it was worth doing when touching this file :)
ACKs for top commit:
EthanHeilman:
ACK 7a046cdc1423963bdcbcf9bb98560af61fa90b37
laanwj:
ACK 7a046cdc1423963bdcbcf9bb98560af61fa90b37
Tree-SHA512: 66556e290db996917b54091acd591df221f72230f6b9f6b167b9195ee870ebef6e26f4cda2f6f54d00e1c362e1743bf56785d0de7cae854e6bf7d26f6caccaba
|