Commit graph

299 commits

Author SHA1 Message Date
fanquake
6af9b31bfc
Merge #19107: p2p: Move all header verification into the network layer, extend logging
deb52711a1 Remove header checks out of net_processing (Troy Giorshev)
52d4ae46ab Give V1TransportDeserializer CChainParams& member (Troy Giorshev)
5bceef6b12 Change CMessageHeader Constructor (Troy Giorshev)
1ca20c1af8 Add doxygen comment for ReceiveMsgBytes (Troy Giorshev)
890b1d7c2b Move checksum check from net_processing to net (Troy Giorshev)
2716647ebf Give V1TransportDeserializer an m_node_id member (Troy Giorshev)

Pull request description:

  Inspired by #15206 and #15197, this PR moves all message header verification from the message processing layer and into the network/transport layer.

  In the previous PRs there is a change in behavior, where we would disconnect from peers upon a single failed checksum check.  In various discussions there was concern over whether this was the right choice, and some expressed a desire to see how this would look if it was made to be a pure refactor.

  For more context, see https://bitcoincore.reviews/15206.html#l-81.

  This PR improves the separation between the p2p layers, helping improvements like [BIP324](https://github.com/bitcoin/bitcoin/pull/18242) and #18989.

ACKs for top commit:
  ryanofsky:
    Code review ACK deb52711a1 just rebase due to conflict on adjacent line
  jnewbery:
    Code review ACK deb52711a1.

Tree-SHA512: 1a3b7ae883b020cfee1bef968813e04df651ffdad9dd961a826bd80654f2c98676ce7f4721038a1b78d8790e4cebe8060419e3d8affc97ce2b9b4e4b72e6fa9f
2020-09-29 16:14:40 +08:00
Troy Giorshev
deb52711a1 Remove header checks out of net_processing
This moves header size and netmagic checking out of net_processing and
into net.  This check now runs in ReadHeader, so that net can exit early
out of receiving bytes from the peer.  IsValid is now slimmed down, so
it no longer needs a MessageStartChars& parameter.

Additionally this removes the rest of the m_valid_* members from
CNetMessage.
2020-09-22 22:05:18 -04:00
Troy Giorshev
52d4ae46ab Give V1TransportDeserializer CChainParams& member
This adds a CChainParams& member to V1TransportDeserializer member, and
use it in place of many Params() calls.  In addition to reducing the
number of calls to a global, this removes a parameter from GetMessage
(and will later allow us to remove one from CMessageHeader::IsValid())
2020-09-22 22:01:14 -04:00
Troy Giorshev
5bceef6b12 Change CMessageHeader Constructor
This commit removes the single-parameter contructor of CMessageHeader
and replaces it with a default constructor.

The single parameter contructor isn't used anywhere except for tests.
There is no reason to initialize a CMessageHeader with a particular
messagestart.  This messagestart should always be replaced when
deserializing an actual message header so that we can run checks on it.

The default constructor initializes it to zero, just like the command
and checksum.

This also removes a parameter of a V1TransportDeserializer constructor,
as it was only used for this purpose.
2020-09-22 22:01:14 -04:00
Troy Giorshev
890b1d7c2b Move checksum check from net_processing to net
This removes the m_valid_checksum member from CNetMessage.  Instead,
GetMessage() returns an Optional.

Additionally, GetMessage() has been given an out parameter to be used to
hold error information.  For now it is specifically a uint32_t used to
hold the raw size of the corrupt message.

The checksum check is now done in GetMessage.
2020-09-22 22:01:14 -04:00
Troy Giorshev
2716647ebf Give V1TransportDeserializer an m_node_id member
This is intended to only be used for logging.

This will allow log messages in the following commits to keep recording
the peer's ID, even when logging is moved into V1TransportDeserializer.
2020-09-22 22:01:14 -04:00
MarcoFalke
fae0548686
fuzz: Remove needless guard 2020-09-22 22:32:18 +02:00
MarcoFalke
77771a03df
refactor: Remove SignetTxs::m_valid and use optional instead
m_valid implies the block solution has been checked, which is not the
case. It only means the txs could be parsed. C++17 comes with
std::optional, so just use that instead.
2020-09-22 22:31:31 +02:00
Wladimir J. van der Laan
77376034d4
Merge #17785: p2p: Unify Send and Receive protocol versions
ddefb5c0b7 p2p: Use the greatest common version in peer logic (Hennadii Stepanov)
e084d45562 p2p: Remove SetCommonVersion() from VERACK handler (Hennadii Stepanov)
8d2026796a refactor: Rename local variable nSendVersion (Hennadii Stepanov)
e9a6d8b13b p2p: Unify Send and Receive protocol versions (Hennadii Stepanov)

Pull request description:

  On master (6fef85bfa3) `CNode` has two members to keep protocol version:
  - `nRecvVersion` for received messages
  - `nSendVersion` for messages to send

  After exchanging with `VERSION` and `VERACK` messages via protocol version `INIT_PROTO_VERSION`, both nodes set `nRecvVersion` _and_ `nSendVersion` to _the same_ value which is the greatest common protocol version.

  This PR:
  - replaces two `CNode` members, `nRecvVersion` `nSendVersion`, with `m_greatest_common_version`
  - removes duplicated getter and setter

  There is no change in behavior on the P2P network.

ACKs for top commit:
  jnewbery:
    ACK ddefb5c0b7
  naumenkogs:
    ACK ddefb5c0b7
  fjahr:
    Code review ACK ddefb5c0b7
  amitiuttarwar:
    code review but untested ACK ddefb5c0b7
  benthecarman:
    utACK `ddefb5c`

Tree-SHA512: 5305538dbaa5426b923b0afd20bdef4f248d310855d1d78427210c00716c67b7cb691515c421716b6157913e453076e293b10ff5fd2cd26a8e5375d42da7809d
2020-09-22 00:14:32 +02:00
Wladimir J. van der Laan
8c5f68118c
Merge #18267: BIP-325: Signet [consensus]
8258c4c007 test: some sanity checks for consensus logic (Anthony Towns)
e47ad375bf test: basic signet tests (Karl-Johan Alm)
4c189abdc4 test: add small signet fuzzer (practicalswift)
ec9b25d046 test: signet network selection tests (Karl-Johan Alm)
3efe298dcc signet: hard-coded parameters for Signet Global Network VI (2020-09-07) (Karl-Johan Alm)
c7898bca4e qt: update QT to support signet network (Karl-Johan Alm)
a8de47a1c9 consensus: add signet validation (Karl-Johan Alm)
e8990f1214 add signet chain and accompanying parameters (Karl-Johan Alm)
404682b7cd add signet basic support (signet.cpp) (Karl-Johan Alm)
a2147d7dad validation: move GetWitnessCommitmentIndex to consensus/validation (Karl-Johan Alm)

Pull request description:

  This PR is a part of BIP-325 (https://github.com/bitcoin/bips/blob/master/bip-0325.mediawiki), and is a sub-PR of #16411.

  * Signet consensus (this)
  * Signet RPC tools (pending)
  * Signet utility scripts (contrib/signet) (pending)

ACKs for top commit:
  jonatack:
    re-ACK 8258c4c007 per `git diff dbeea65 8258c4c`, only change since last review is updated `-signet*` config option naming.
  fjahr:
    re-ACK 8258c4c
  laanwj:
    ACK 8258c4c007
  MarcoFalke:
    Approach ACK 8258c4c007 🌵

Tree-SHA512: 5d158add96755910837feafa8214e13695b769a6aec3a2da753cf672618bef377fac43b0f4b772a87b25dd9f0c1c9b29f2789785d7a7d47a155cdcf48f7c975d
2020-09-21 22:33:00 +02:00
practicalswift
4c189abdc4
test: add small signet fuzzer 2020-09-18 10:19:42 +09:00
practicalswift
fc7f84a9ca tests: Add fuzzing harness for Keccak and SHA3_256 2020-09-10 14:54:30 +00:00
Hennadii Stepanov
e9a6d8b13b
p2p: Unify Send and Receive protocol versions
There is no change in behavior on the P2P network.
2020-09-07 21:03:44 +03:00
John Newbery
58bd369b0d scripted-diff: [net processing] Rename PeerLogicValidation to PeerManager
-BEGIN VERIFY SCRIPT-
sed -i 's/PeerLogicValidation/PeerManager/g' $(git grep -l PeerLogicValidation ./src ./test)
sed -i 's/peer_logic/peerman/g' $(git grep -l peer_logic ./src ./test)
-END VERIFY SCRIPT-

PeerLogicValidation was originally net_processing's implementation to
the validation interface. It has since grown to contain much of
net_processing's logic. Therefore rename it to reflect its
responsibilities.

Suggested in
https://github.com/bitcoin/bitcoin/pull/10756#pullrequestreview-53892618.
2020-09-07 11:15:48 +01:00
John Newbery
2297b26b3c [net_processing] Pass chainparams to PeerLogicValidation constructor
Keep a references to chainparams, rather than calling the global
Params() function every time it's needed. This is fine, since
globalChainParams does not get updated once it's been set, and it's
available at the point of constructing the PeerLogicValidation object.
2020-09-07 11:13:58 +01:00
Amiti Uttarwar
dff16b184b [refactor] Restructure logic to check for addr relay.
We previously identified if we relay addresses to the connection by checking
for the existence of the m_addr_known data structure. With this commit, we
answer this question based on the connection type.

IsAddrRelayPeer() checked for the existence of the m_addr_known
2020-09-02 17:18:21 -07:00
Amiti Uttarwar
8d6ff46f55 scripted-diff: Rename OUTBOUND ConnectionType to OUTBOUND_FULL_RELAY
-BEGIN VERIFY SCRIPT-
sed -i 's/OUTBOUND, /OUTBOUND_FULL_RELAY, /g' src/net.h
sed -i 's/ConnectionType::OUTBOUND/ConnectionType::OUTBOUND_FULL_RELAY/g' src/test/net_tests.cpp src/test/fuzz/process_message.cpp src/test/fuzz/process_messages.cpp src/net.cpp src/test/denialofservice_tests.cpp src/net.h src/test/fuzz/net.cpp
-END VERIFY SCRIPT-
2020-09-02 13:34:58 -07:00
MarcoFalke
61b8c04d78
Merge #19379: tests: Add fuzzing harness for SigHasLowR(...) and ecdsa_signature_parse_der_lax(...)
46fcac1e4b tests: Add fuzzing harness for ec_seckey_import_der(...) and ec_seckey_export_der(...) (practicalswift)
b667a90389 tests: Add fuzzing harness for SigHasLowR(...) and ecdsa_signature_parse_der_lax(...) (practicalswift)

Pull request description:

  Add fuzzing harness for `SigHasLowR(...)` and `ecdsa_signature_parse_der_lax(...)`.

  See [`doc/fuzzing.md`](https://github.com/bitcoin/bitcoin/blob/master/doc/fuzzing.md) for information on how to fuzz Bitcoin Core. Don't forget to contribute any coverage increasing inputs you find to the [Bitcoin Core fuzzing corpus repo](https://github.com/bitcoin-core/qa-assets).

  Happy fuzzing :)

ACKs for top commit:
  Crypt-iQ:
    ACK 46fcac1e4b

Tree-SHA512: 11a4856a1efd9a04030a8c8aee2413fd5be1ea248147e649a48a55bacdf732bb48a19ee1ce2761d47d4dd61c9598aec53061b961b319ad824d539dda11a8ccf4
2020-08-31 10:56:34 +02:00
practicalswift
cc26fab48d tests: Add fuzzing harness for CNode 2020-08-27 17:50:39 +00:00
MarcoFalke
8d6224fefe
Merge #19628: net: change CNetAddr::ip to have flexible size
102867c587 net: change CNetAddr::ip to have flexible size (Vasil Dimov)
1ea57ad674 net: don't accept non-left-contiguous netmasks (Vasil Dimov)

Pull request description:

  (chopped off from #19031 to ease review)

  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). Do not accept
  invalid netmasks that have 0-bits followed by 1-bits and only allow
  subnetting for IPv4 and IPv6.

  Co-authored-by: Carl Dong <contact@carldong.me>

ACKs for top commit:
  sipa:
    utACK 102867c587
  MarcoFalke:
    Concept ACK 102867c587
  ryanofsky:
    Code review ACK 102867c587. Just many suggested updates since last review. Thanks for following up on everything!
  jonatack:
    re-ACK 102867c587 diff review, code review, build/tests/running bitcoind with ipv4/ipv6/onion peers
  kallewoof:
    ACK 102867c587

Tree-SHA512: d60bf716cecf8d3e8146d2f90f897ebe956befb16f711a24cfe680024c5afc758fb9e4a0a22066b42f7630d52cf916318bedbcbc069ae07092d5250a11e8f762
2020-08-25 18:10:25 +02:00
Vasil Dimov
102867c587
net: change CNetAddr::ip to have flexible size
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>
2020-08-24 21:50:59 +02:00
fanquake
4fefd80f08
Merge #19704: Net processing: move ProcessMessage() to PeerLogicValidation
daed542a12 [net_processing] Move ProcessMessage to PeerLogicValidation (John Newbery)
c556770b5e [net_processing] Change PeerLogicValidation to hold a connman reference (John Newbery)

Pull request description:

  Rather than ProcessMessage() being a static function in net_processing.cpp, make it a private member function of PeerLogicValidation. This is the start of moving static functions and global variables into PeerLogicValidation to make it better encapsulated.

ACKs for top commit:
  jonatack:
    ACK daed542a12 code review and debug tested
  promag:
    Code review ACK daed542a12.
  MarcoFalke:
    re-ACK daed542a12, only change is removing second commit 🎴
  theStack:
    Code Review ACK daed542a12

Tree-SHA512: ddebf410d114d9ad5a9e536950018ff333a347c035d74fcc101fb4a3f20a281782c7eac2b7d1bd1c8f6bc7e59f5b5630fb52c2e1b4c32df454fa584673bd021e
2020-08-24 21:50:37 +08:00
John Newbery
daed542a12 [net_processing] Move ProcessMessage to PeerLogicValidation 2020-08-21 13:10:41 +01:00
practicalswift
46fcac1e4b tests: Add fuzzing harness for ec_seckey_import_der(...) and ec_seckey_export_der(...) 2020-08-18 18:03:57 +00:00
practicalswift
b667a90389 tests: Add fuzzing harness for SigHasLowR(...) and ecdsa_signature_parse_der_lax(...) 2020-08-18 18:03:56 +00:00
Sebastian Falbesoner
71e0f07e9c util: remove unused c-string variant of atoi64() 2020-08-17 17:56:59 +02:00
MeshCollider
b887060d06 Replace usage of GetScriptForWitness with GetScriptForDestination 2020-08-14 08:44:42 +12:00
fanquake
ce3bdd0ed1
Merge #19316: [net] Cleanup logic around connection types
01e283068b [net] Remove unnecessary default args on CNode constructor (Amiti Uttarwar)
bc5d65b3ca [refactor] Remove IsOutboundDisconnectionCandidate (Amiti Uttarwar)
2f2e13b6c2 [net/refactor] Simplify multiple-connection checks (Amiti Uttarwar)
7f7b83deb2 [net/refactor] Rework ThreadOpenConnections logic (Amiti Uttarwar)
35839e963b [net] Fix bug where AddrFetch connections would be counted as outbound full relay (Amiti Uttarwar)
4972c21b67 [net/refactor] Clarify logic for selecting connections in ThreadOpenConnections (Amiti Uttarwar)
60156f5fc4 [net/refactor] Remove fInbound flag from CNode (Amiti Uttarwar)
7b322df629 [net/refactor] Remove m_addr_fetch member var from CNode (Amiti Uttarwar)
14923422b0 [net/refactor] Remove fFeeler flag from CNode (Amiti Uttarwar)
49efac5cae [net/refactor] Remove m_manual_connection flag from CNode (Amiti Uttarwar)
d3698b5ee3 [net/refactor] Add connection type as a member var to CNode (Amiti Uttarwar)
46578c03e9 [doc] Describe different connection types (Amiti Uttarwar)
442abae2ba [net/refactor] Add AddrFetch connections to ConnectionType enum (Amiti Uttarwar)
af59feb052 [net/refactor] Extract m_addr_known logic from initializer list (Amiti Uttarwar)
e1bc29812d [net/refactor] Add block relay only connections to ConnectionType enum (Amiti Uttarwar)
0e52a659a2 [net/refactor] Add feeler connections to ConnectionType enum (Amiti Uttarwar)
1521c47438 [net/refactor] Add manual connections to ConnectionType enum (Amiti Uttarwar)
26304b4100 [net/refactor] Introduce an enum to distinguish type of connection (Amiti Uttarwar)
3f1b7140e9 scripted-diff: Rename OneShot to AddrFetch (Amiti Uttarwar)

Pull request description:

  **This is part 1 of #19315, which enables the ability to test `outbound` and `block-relay-only` connections from the functional tests.** Please see that PR for more information of overall functionality.

  **This PR simplifies how we manage different connection types.** It introduces an enum with the various types of connections so we can explicitly define the connection type. The existing system relies on a series of independent flags, then has asserts scattered around to ensure that conflicting flags are not enabled at the same time. I find this approach to be both brittle and confusing. While making these changes, I found a small bug due to the silent assumptions.

  This PR also proposes a rename from `OneShot` to `AddrFetch`. I find the name `OneShot` to be very confusing, especially when we also have `onetry` manual connections. Everyone I've talked to offline has agreed that the name is confusing, so I propose a potential alternative. I think this is a good opportunity for a rename since I'm creating an enum to explicitly define the connection types.
  (some context for the unfamiliar: `oneshot` or `addrfetch` connections are short-lived connections created on startup. They connect to the seed peers, send a `getaddr` to solicit addresses, then close the connection.)

  Overview of this PR:
  * rename `oneshot` to `addrfetch`
  * introduce `ConnectionType` enum
  * one by one, add different connection types to the enum
  * expose the `conn_type` on CNode, and use this to reduce reliance on flags (& asserts)
  * fix the bug in counting different type of connections
  * some additional cleanup to simplify logic and make expectations explicit/inclusive rather than implicit/exclusive.

ACKs for top commit:
  jnewbery:
    utACK 01e283068b
  laanwj:
    Code review ACK 01e283068b, the commits are pretty straightforward to follow, and I think this is a move in the right direction overall
  vasild:
    ACK 01e283068
  sdaftuar:
    ACK 01e283068b.
  fanquake:
    ACK 01e283068b - I don't have as much experience with the networking code but these changes look fairly straight forward, the new code seems more robust/understandable and the additional documentation is great. I'm glad that a followup branch is already underway. There might be some more review comments here later today, so keep an eye on the discussion, however I'm going to merge this now.
  jb55:
    wow this code was messy before... ACK 01e283068b

Tree-SHA512: 7bb644a6ed5849913d777ebc2ff89133ca0fbef680355a9a344e07496a979e6f9ff21a958e8eea93dcd7d5c343682b0c7174b1a3de380a4247eaae73da436e15
2020-08-12 10:01:44 +08:00
Amiti Uttarwar
442abae2ba [net/refactor] Add AddrFetch connections to ConnectionType enum
- AddrFetch connections are short lived connections used to getaddr from a peer
- previously called "one shot" connections
2020-08-07 17:18:16 -07:00
Amiti Uttarwar
e1bc29812d [net/refactor] Add block relay only connections to ConnectionType enum 2020-08-07 17:18:16 -07:00
Amiti Uttarwar
0e52a659a2 [net/refactor] Add feeler connections to ConnectionType enum 2020-08-07 17:18:16 -07:00
Amiti Uttarwar
1521c47438 [net/refactor] Add manual connections to ConnectionType enum 2020-08-07 17:18:16 -07:00
Amiti Uttarwar
26304b4100 [net/refactor] Introduce an enum to distinguish type of connection
- extract inbound & outbound types
2020-08-07 17:18:16 -07:00
Wladimir J. van der Laan
34eb236258
Merge #19326: Simplify hash.h interface using Spans
77c507358b Make Hash[160] consume range-like objects (Pieter Wuille)
02c4cc5c5d Make CHash256/CHash160 output to Span (Pieter Wuille)
0ef97b1b10 Make MurmurHash3 consume Spans (Pieter Wuille)
e549bf8a9a Make CHash256 and CHash160 consume Spans (Pieter Wuille)
2a2182c387 Make script/standard's BaseHash Span-convertible (Pieter Wuille)
e63dcc3a67 Add MakeUCharSpan, to help constructing Span<[const] unsigned char> (Pieter Wuille)
567825049f Make uint256 Span-convertible by adding ::data() (Pieter Wuille)
131a2f0337 scripted-diff: rename base_blob::data to m_data (Pieter Wuille)

Pull request description:

  This makes use of the implicit constructions and conversions to Span introduced in #18468 to simplify the hash.h interface:

  * All functions that take a pointer and a length are changed to take a Span instead.
  * The Hash() and Hash160() functions are changed to take in "range" objects instead of begin/end iterators.

ACKs for top commit:
  laanwj:
    re-ACK 77c507358b
  jonatack:
    Code review re-ACK 77c5073 per `git range-diff 14ceddd 49fc016 77c5073`

Tree-SHA512: 9ec929891b1ddcf30eb14b946ee1bf142eca1442b9de0067ad6a3c181e0c7ea0c99c0e291e7f6e7a18bd7bdf78fe94ee3d5de66e167401674caf91e026269771
2020-08-03 17:27:49 +02:00
Wladimir J. van der Laan
14ceddd290
Merge #18991: Cache responses to GETADDR to prevent topology leaks
3bd67ba5a4 Test addr response caching (Gleb Naumenko)
cf1569e074 Add addr permission flag enabling non-cached addr sharing (Gleb Naumenko)
acd6135b43 Cache responses to addr requests (Gleb Naumenko)
7cc0e8101f Remove useless 2500 limit on AddrMan queries (Gleb Naumenko)
ded742bc5b Move filtering banned addrs inside GetAddresses() (Gleb Naumenko)

Pull request description:

  This is a very simple code change with a big p2p privacy benefit.

  It’s currently trivial to scrape any reachable node’s AddrMan (a database of all nodes known to them along with the timestamps).
  We do have a limit of one GETADDR per connection, but a spy can disconnect and reconnect even from the same IP, and send GETADDR again and again.

  Since we respond with 1,000 random records at most, depending on the AddrMan size it takes probably up to 100 requests for an spy to make sure they scraped (almost) everything.
  I even have a script for that. It is totally doable within couple minutes.

  Then, with some extra protocol knowledge a spy can infer the direct peers of the victim, and other topological stuff.

  I suggest to cache responses to GETADDR on a daily basis, so that an attacker gets at most 1,000 records per day, and can’t track the changes in real time. I will be following up with more improvements to addr relay privacy, but this one alone is a very effective. And simple!

  I doubt any of the real software does *reconnect to get new addrs from a given peer*, so we shouldn’t be cutting anyone.
  I also believe it doesn’t have any negative implications on the overall topology quality. And the records being “outdated” for at most a day doesn’t break any honest assumptions either.

ACKs for top commit:
  jnewbery:
    reACK 3bd67ba5a4
  promag:
    Code review ACK 3bd67ba5a4.
  ariard:
    Code Review ACK 3bd67ba

Tree-SHA512: dfa5d03205c2424e40a3f8a41af9306227e1ca18beead3b3dda44aa2a082175bb1c6d929dbc7ea8e48e01aed0d50f0d54491caa1147471a2b72a46c3ca06b66f
2020-08-03 14:48:52 +02:00
Pieter Wuille
77c507358b Make Hash[160] consume range-like objects 2020-07-30 13:57:54 -07:00
Pieter Wuille
02c4cc5c5d Make CHash256/CHash160 output to Span 2020-07-30 13:57:54 -07:00
Pieter Wuille
e549bf8a9a Make CHash256 and CHash160 consume Spans 2020-07-30 13:57:53 -07:00
Gleb Naumenko
cf1569e074 Add addr permission flag enabling non-cached addr sharing 2020-07-30 14:38:50 +03:00
Hennadii Stepanov
7b3851e947
refactor: Drop unused CBufferedFile::Seek() 2020-07-26 22:46:28 +03:00
fanquake
2031aa92a3
Merge #19562: test: Fix fuzzer compilation on macOS
c8992e8959 test: Fix fuzzer compilation on macOS fixes #19557 (freenancial)

Pull request description:

  fixes #19557

  Before the fix:
  ```
  ➜  bitcoin git:(fix-fuzzer-macos) make
  Making all in src
    CXX      test/fuzz/addition_overflow-addition_overflow.o
  In file included from test/fuzz/addition_overflow.cpp:7:
  ./test/fuzz/util.h:335:13: error: no matching function for call to 'AdditionOverflow'
          if (AdditionOverflow((uint64_t)fuzzed_file->m_offset, random_bytes.size())) {
              ^~~~~~~~~~~~~~~~
  ./test/fuzz/util.h:201:16: note: candidate template ignored: deduced conflicting types for parameter 'T' ('unsigned long long' vs. 'unsigned long')
  NODISCARD bool AdditionOverflow(const T i, const T j) noexcept
                 ^
  ./test/fuzz/util.h:346:13: error: no matching function for call to 'AdditionOverflow'
          if (AdditionOverflow(fuzzed_file->m_offset, n)) {
              ^~~~~~~~~~~~~~~~
  ./test/fuzz/util.h:201:16: note: candidate template ignored: deduced conflicting types for parameter 'T' ('long long' vs. 'long')
  NODISCARD bool AdditionOverflow(const T i, const T j) noexcept
                 ^
  ```

  After the fix:
  ```
  ➜  bitcoin git:(fix-fuzzer-macos) ./configure --enable-fuzz --with-sanitizers=fuzzer,address,undefined CC=/usr/local/opt/llvm/bin/clang CXX=/usr/local/opt/llvm/bin/clang++ --disable-asm && make clean && make -j5
  ...
  ...
    CXXLD    test/fuzz/uint256_deserialize
  Making all in doc/man
  make[1]: Nothing to be done for `all'.
  make[1]: Nothing to be done for `all-am'.
  ```

ACKs for top commit:
  fanquake:
    ACK c8992e8959 - tested that compiling works on macOS.
  MarcoFalke:
    review ACK c8992e8959

Tree-SHA512: 965cdc61b30db0e2209c91b29f0d42de927a9a5b85e1e70f22d1452e0955f876726c7a8c1d1a5f448f12bf24eec3000802071cd4ae28d8605343fd43d174ca84
2020-07-22 18:03:41 +08:00
fanquake
597d2f905e
Merge #19548: fuzz: add missing overrides to signature_checker
c0f09c2c9d fuzz: add missing overrides to signature_checker (Jon Atack)

Pull request description:

  These functions in `fuzz/signature_checker.cpp` override virtual member functions and should be marked `override` instead of `virtual`, which is for introducing a new virtual function. The overridden virtual functions are in `script/interpreter.h:151/156/161`.

  Also, per MarcoFalke suggestion, add missing parentheses in `fuzz/scriptnum_ops.cpp` and remove useless `unsigned int >= 0` conditional in `fuzz/script.cpp`.

  These changes fix 5 compile warnings in gcc 10 and 3 in clang 11/12.

ACKs for top commit:
  vasild:
    ACK c0f09c2
  MarcoFalke:
    review ACK c0f09c2c9d

Tree-SHA512: 76ce73ec577c1f23cf8646c31d44dcd6c6303732c47187d041a8921d0d24a50163989a375352ebc221abf2ac337bc0902149be46b6f9eebc071d2f364c407f71
2020-07-22 17:32:36 +08:00
Jon Atack
c0f09c2c9d
fuzz: add missing overrides to signature_checker
and also

- add missing parentheses in fuzz/scriptnum_ops.cpp

- remove useless unsigned int conditional in fuzz/script.cpp

These changes fix 5 compile warnings in gcc 10.
2020-07-22 05:27:13 +02:00
freenancial
c8992e8959 test: Fix fuzzer compilation on macOS
fixes #19557
2020-07-21 15:23:49 -07:00
Jon Atack
1cdc2a642b
fuzz: fix unused variable addrdb compiler warning 2020-07-19 08:31:34 +02:00
MarcoFalke
090d877160
Merge #19143: tests: Add fuzzing harnesses for CAutoFile, CBufferedFile, LoadExternalBlockFile and other FILE* consumers
ad6c34881d tests: Add fuzzing harness for CBlockPolicyEstimator::{Read,Write} (policy/fees.h) (practicalswift)
614e0807a8 tests: Add fuzzing harness for CBufferedFile::{SetPos,GetPos,GetType,GetVersion} (stream.h) (practicalswift)
7bcc71e5f8 tests: Add fuzzing harness for LoadExternalBlockFile(...) (validation.h) (practicalswift)
9823376030 tests: Add fuzzing harness for CBufferedFile (streams.h) (practicalswift)
f3aa659be6 tests: Add fuzzing harness for CAutoFile (streams.h) (practicalswift)
e507c0799d tests: Add serialization/deserialization fuzzing helpers WriteToStream(…)/ReadFromStream(…) (practicalswift)
e48094a506 tests: Add FuzzedAutoFileProvider which provides a CAutoFile interface to FuzzedDataProvider (practicalswift)
9dbcd6854c tests: Add FuzzedFileProvider which provides a FILE* interface to FuzzedDataProvider using fopencookie (practicalswift)

Pull request description:

  Add fuzzing harnesses for `CAutoFile`, `CBufferedFile`, `LoadExternalBlockFile` and other `FILE*` consumers:
  * Add `FuzzedFileProvider` which provides a `FILE*` interface to `FuzzedDataProvider` using `fopencookie`
  * Add `FuzzedAutoFileProvider` which provides a `CAutoFile` interface to `FuzzedDataProvider`
  * Add serialization/deserialization fuzzing helpers `WriteToStream(…)`/`ReadFromStream(…)`
  * Add fuzzing harness for `CAutoFile` (`streams.h`)
  * Add fuzzing harness for `CBufferedFile` (`streams.h`)
  * Add fuzzing harness for `LoadExternalBlockFile(...)` (`validation.h`)
  * Add fuzzing harness for `CBlockPolicyEstimator::Read` and `CBlockPolicyEstimator::Write` (`policy/fees.h`)

  See [`doc/fuzzing.md`](https://github.com/bitcoin/bitcoin/blob/master/doc/fuzzing.md) for information on how to fuzz Bitcoin Core. Don't forget to contribute any coverage increasing inputs you find to the [Bitcoin Core fuzzing corpus repo](https://github.com/bitcoin-core/qa-assets).

  Happy fuzzing :)

ACKs for top commit:
  Crypt-iQ:
    Tested ACK ad6c348

Tree-SHA512: a38e142608218496796a527d7e59b74e30279a2815450408b7c27a76ed600cebc6b88491e831665a0639671e2d212453fcdca558500bbadbeb32b267751f8f72
2020-07-18 09:24:58 +02:00
MarcoFalke
affed844ba
Merge #19174: refactor: replace CConnman pointers by references in net_processing.cpp
0c8461a88e refactor: replace CConnman pointers by references in net_processing.cpp (Sebastian Falbesoner)

Pull request description:

  This is a follow-up to the recently merged PR https://github.com/bitcoin/bitcoin/pull/19053, replacing ~~two more types of~~ one more type of pointer (CConnman) by references to increase the code quality -- pointers should either check for `nullptr` or be replaced by references, and the latter strategy seems to be more reasonable.

  Again, to keep the review burden managable, the changes are kept simple,
  * only tackling `CConnman*` ~~and `BanMan*`~~ pointers
  * only within the net_processing module, i.e. no changes that would need adaption in other modules
  * keeping the names of the variables as they are

ACKs for top commit:
  jnewbery:
    utACK 0c8461a88e
  MarcoFalke:
    ACK 0c8461a88e 🕧

Tree-SHA512: 79dc05144bcfb5e0bbc62180285aadcc6199f044fa3016c0f54f7b7f45037415260970037bd63b18fafefb8aef448549dae14b780bafb540fa2373f493a17f71
2020-07-16 08:07:25 +02:00
Wladimir J. van der Laan
43125596ce
Merge #19296: tests: Add fuzzing harness for AES{CBC,}256{Encrypt,Decrypt}, poly1305_auth, CHKDF_HMAC_SHA256_L32, ChaCha20 and ChaCha20Poly1305AEAD
cca7c577d5 tests: Add fuzzing harness for ChaCha20Poly1305AEAD (practicalswift)
2fc4e5916c tests: Add fuzzing harness for ChaCha20 (practicalswift)
e9e8aac029 tests: Add fuzzing harness for CHKDF_HMAC_SHA256_L32 (practicalswift)
ec86ca1aaa tests: Add fuzzing harness for poly1305_auth(...) (practicalswift)
4cee53bba7 tests: Add fuzzing harness for AES256CBCEncrypt/AES256CBCDecrypt (practicalswift)
9352c32325 tests: Add fuzzing harness for AES256Encrypt/AES256Decrypt (practicalswift)

Pull request description:

  Add fuzzing harness for `AES{CBC,}256{Encrypt,Decrypt}`, `poly1305_auth`, `CHKDF_HMAC_SHA256_L32`, `ChaCha20` and `ChaCha20Poly1305AEAD`.

  See [`doc/fuzzing.md`](https://github.com/bitcoin/bitcoin/blob/master/doc/fuzzing.md) for information on how to fuzz Bitcoin Core. Don't forget to contribute any coverage increasing inputs you find to the [Bitcoin Core fuzzing corpus repo](https://github.com/bitcoin-core/qa-assets).

  Happy fuzzing :)

ACKs for top commit:
  laanwj:
    ACK cca7c577d5

Tree-SHA512: cff9acefe370c12a3663aa55145371df835479c6ab8f6d81bbf84e0f81a9d6b0d94e45ec545f9dd5e1702744eaa7947a1f4ffed0171f446fc080369161afd740
2020-07-15 14:45:12 +02:00
practicalswift
ad6c34881d tests: Add fuzzing harness for CBlockPolicyEstimator::{Read,Write} (policy/fees.h) 2020-07-15 11:41:21 +00:00
practicalswift
614e0807a8 tests: Add fuzzing harness for CBufferedFile::{SetPos,GetPos,GetType,GetVersion} (stream.h) 2020-07-15 11:41:21 +00:00