Commit graph

587 commits

Author SHA1 Message Date
Ross Nicoll bad91d661e
Merge pull request #2522 from rnicoll/1.21-merge-1.21.2-rc2
Merge Bitcoin Core 0.21.2-rc2
2021-09-24 22:13:35 +01:00
Ross Nicoll 1e72191e81
Introduce ReadBlockOrHeaderFromDisk 2021-09-09 19:24:50 +01:00
Malta Micael 7b4867ea2b
CPureBlockHeader 2021-09-07 21:40:10 +01:00
Ross Nicoll b2554af0df Bitcoin Core 0.21.2 release candidate 2
Tree-SHA512: 7bcdf6c42ac75fb24df8d6b60bddcac5f14363a3f7dd89a239f798bb14b5c911c2d7535a0372c2998719d33a561d0d28b0b6764aaf1f2ec330d4035ce965997b
 -----BEGIN PGP SIGNATURE-----
 
 iQIzBAABCAAdFiEEz7FuIclQ9n+pXlWPLuufXMCVJsEFAmEp9EoACgkQLuufXMCV
 JsGvcQ//ZSQOCEFH3e+gBuXHthXG2UZsHagC715ruQquhRt4aOKiFLu/eJGwiKBt
 Frc1wLF21dgtl/2JUtVBpVb54whni1PKgDowtkk4Ge7gWM0awW2OnvdgA21E+syD
 501duaW6ORvo5icmf7uPQ7u/g9Ks1s1chBr1bLnYqqqg2e3/aqC4+drTjd8wziy3
 J+8Bzc+KLAklgv8cIpH1EDuFGrumlMYxW0KHqJeN7Vk8wGX8PINrkEGCyrBDQ7DY
 BEd6txwcivPxqMfj61OP60DU0oG6IVECGMPrANtyK+ba0FHl+GkNQuTaX8zv4Ik9
 dKscJ1OiRX2ER/pEJTwg8PJfOET5D/WRc1xSmrYBOK7cFLtYVqH2yvnHiLlWvH1P
 7TQOAjfzQwa+yUnGRf2dWW+Rngv876JjBUX5vbm4UC5Geo8Bdl3z9U9glaq03fVd
 3Q8uAgljLnXWFiylK9oDYhsFfWSUYgjyhM11LCb5K3y3t7ageclu6RTl574jy5sb
 8qBwQxpJt5UtwIK2SNqq1iDmMW3J65sb3CmTND9ppDA79GfZzwlvGVwO1bN2TC6m
 0dGTPvOPWS9x/PSq8+8o1uCHDzHrW9B5lUikf8kx+zNI7Rd3QLJ0pbdqwrJ3CcOk
 ttV5QXAYD2trfkCKEi69eiIpH21rasqjyI11ZoH4u+0BCpw8hQg=
 =GXjC
 -----END PGP SIGNATURE-----

Merge tag 'v0.21.2rc2' into 1.21-dev

Bitcoin Core 0.21.2 release candidate 2

Tree-SHA512: 7bcdf6c42ac75fb24df8d6b60bddcac5f14363a3f7dd89a239f798bb14b5c911c2d7535a0372c2998719d33a561d0d28b0b6764aaf1f2ec330d4035ce965997b
2021-09-03 13:40:49 +01:00
Pieter Wuille a653aacbd6 Add logging and addr rate limiting statistics
Includes logging improvements by Vasil Dimov and John Newbery.

Github-Pull: #22387
Rebased-From: f424d601e1b6870e20bc60f5ccba36d2e210377b
2021-08-05 09:40:04 -07:00
Pieter Wuille 8df3e5bd84 Randomize the order of addr processing
Github-Pull: #22387
Rebased-From: 5648138f5949013331c017c740646e2f4013bc24
2021-08-05 09:40:04 -07:00
Pieter Wuille 83dfe6c65e Rate limit the processing of incoming addr messages
While limitations on the influence of attackers on addrman already
exist (affected buckets are restricted to a subset based on incoming
IP / network group), there is no reason to permit them to let them
feed us addresses at more than a multiple of the normal network
rate.

This commit introduces a "token bucket" rate limiter for the
processing of addresses in incoming ADDR and ADDRV2 messages.
Every connection gets an associated token bucket. Processing an
address in an ADDR or ADDRV2 message from non-whitelisted peers
consumes a token from the bucket. If the bucket is empty, the
address is ignored (it is not forwarded or processed). The token
counter increases at a rate of 0.1 tokens per second, and will
accrue up to a maximum of 1000 tokens (the maximum we accept in a
single ADDR or ADDRV2). When a GETADDR is sent to a peer, it
immediately gets 1000 additional tokens, as we actively desire many
addresses from such peers (this may temporarily cause the token
count to exceed 1000).

The rate limit of 0.1 addr/s was chosen based on observation of
honest nodes on the network. Activity in general from most nodes
is either 0, or up to a maximum around 0.025 addr/s for recent
Bitcoin Core nodes. A few (self-identified, through subver) crawler
nodes occasionally exceed 0.1 addr/s.

Github-Pull: #22387
Rebased-From: 0d64b8f709b4655d8702f810d4876cd8d96ded82
2021-08-05 09:40:00 -07:00
Ross Nicoll 72bc180795 Cap maximum fee
Cap maximum fee at 21 million Dogecoins to ensure there is no overflow in the fee logic.
2021-07-09 22:06:26 +01:00
Ross Nicoll 4288f6cee7
Update src/net_processing.cpp 2021-06-03 16:55:08 +01:00
Ross Nicoll 5640cc921d Stop sending a Bitcoin alert to Dogecoin clients 2021-06-03 10:06:21 +01:00
Ross Nicoll 3afaf906f1 Replace consensus values with Dogecoin equivalents
* Replace chain parameters with Dogecoin values
* Update maximum coins to match Dogecoin
* Disable version 2 block requirement
* Update coinbase maturity to match Dogecoin
2021-06-02 10:37:16 +01:00
Ross Nicoll e29e13aae8
Adjust download timeouts to handle shorter block interval (#1866) 2021-05-17 13:15:46 +01:00
practicalswift 95218ee95c net: Avoid UBSan warning in ProcessMessage(...)
Github-Pull: #21043
Rebased-From: f5f2f9716885e7548809e77f46b493c896a019bf
2021-02-11 12:42:31 +01:00
Pieter Wuille bead935470 Send and require SENDADDRV2 before VERACK
See the corresponding BIP change: https://github.com/bitcoin/bips/pull/1043

Github-Pull: #20564
Rebased-From: 1583498fb6781c01ca2f33c09319ed793964c574
2020-12-10 11:37:22 +01:00
Pieter Wuille 9e806887a8 Don't send 'sendaddrv2' to pre-70016 software
Github-Pull: #20564
Rebased-From: c5a89196602e43ebb1cdc9cd4f08d153419c13e1
2020-12-10 11:37:18 +01:00
MarcoFalke 1209b6c692
Merge #20212: net: fix output of peer address in version message
af3b0dfc54 net: fix output of peer address in version message (Vasil Dimov)

Pull request description:

  If `-logips -debug=net` is specified then we print the contents of the
  version message we send to the peer, including his address. Because the
  addresses in the version message use pre-BIP155 encoding they cannot
  represent a Tor v3 address and we would actually send 16 `0`s instead (a
  dummy IPv6 address). However we would print the full address in the log
  message. Before this fix:

  ```
  2020-10-21T12:24:17Z send version message: version 70016, blocks=653500, us=[::]:0, them=xwjtp3mj427zdp4tljiiivg2l5ijfvmt5lcsfaygtpp6cw254kykvpyd.onion:8333, peer=0
  ```

  This is confusing because we pretend to send one thing while we actually
  send another. Adjust the printout to reflect what we are sending. After
  this fix:

  ```
  2020-10-21T12:26:54Z send version message: version 70016, blocks=653500, us=[::]:0, them=[::]:0, peer=0
  ```

ACKs for top commit:
  MarcoFalke:
    review ACK af3b0dfc54
  jnewbery:
    utACK af3b0dfc54

Tree-SHA512: f169d7b4f07c219e541f7c37ea23b82c77e50085fc72ec62f1dd46970389916e177268d07d45c7be94dd209d1903f8f23eaff62b7fa782f6057dd36bb96bba82
2020-11-04 13:41:52 +01:00
fanquake 8387f832d6
Merge #20187: Addrman: test-before-evict bugfix and improvements for block-relay-only peers
16d9bfc417 Avoid test-before-evict evictions of current peers (Suhas Daftuar)
e8b215a086 Refactor test for existing peer connection into own function (Suhas Daftuar)
4fe338ab3e Call CAddrMan::Good() on block-relay-only peer addresses (Suhas Daftuar)
daf5553126 Avoid calling CAddrMan::Connected() on block-relay-only peer addresses (Suhas Daftuar)

Pull request description:

  This PR does two things:

  * Block-relay-only interaction with addrman.
    * Calling `CAddrMan::Connected()` on an address that was a block-relay-only peer causes the time we report in `addr` messages containing that peer to be updated; particularly now that we use anchor connections with a our block-relay-only peers, this risks leaking information about those peers.  So, stop this.
    * Avoiding calling `CAddrMan::Good()` on block-relay-only peer addresses causes the addrman logic around maintaining the new and tried table to be less good, and in particular makes it so that block-relay-only peer addresses are more likely to be evicted from the addrman (for no good reason I can think of).  So, mark those addresses as good when we connect.

  * Fix test-before-evict bug. There's a bug where if we get a collision in the tried table with an existing address that is one of our current peers, and the connection is long-lived enough, then `SelectTriedCollisions()` might return that existing peer address to us as a test-before-evict connection candidate. However, our logic for new outbound connections would later prevent us from actually making a connection; the result would be that when we get a collision with a long-lived current peer, that peer's address is likely to get evicted from the tried table.  Fix this by checking to see if a test-before-evict candidate is a peer we're currently connected to, and if so, mark it as `Good()`.

ACKs for top commit:
  sipa:
    utACK 16d9bfc417
  amitiuttarwar:
    code review ACK 16d9bfc417
  mzumsande:
    Code-Review ACK 16d9bfc417.
  jnewbery:
    utACK 16d9bfc417
  ariard:
    Code Review ACK 16d9bfc.
  jonatack:
    Tested ACK 16d9bfc417

Tree-SHA512: 188ccb814e436937cbb91d29d73c316ce83f4b9c22f1cda56747f0949a093e10161ae724e87e4a2d85ac40f85f5f6b4e87e97d350a1ac44f80c57783f4423324
2020-11-03 09:38:35 +08:00
Suhas Daftuar 4fe338ab3e Call CAddrMan::Good() on block-relay-only peer addresses
Being able to invoke Good() is important for address management (new vs tried
table, tried table eviction via test-before-evict). We mitigate potential
information leaks by not calling Connected() on these peer addresses.
2020-10-27 11:15:21 -04:00
Suhas Daftuar daf5553126 Avoid calling CAddrMan::Connected() on block-relay-only peer addresses
Connected() updates the time we serve in addr messages, so avoid leaking
block-relay-only peer connections by avoiding these calls.
2020-10-27 11:14:58 -04:00
Troy Giorshev d419fdedbe [net processing] Don't add AlreadyHave txs to recentRejects
Now, we only add a transaction to our recentRejects filter if we didn't
already have it, meaning that it is added at most once, as intended.
2020-10-27 06:07:41 -04:00
Vasil Dimov af3b0dfc54
net: fix output of peer address in version message
If `-logips -debug=net` is specified then we print the contents of the
version message we send to the peer, including his address. Because the
addresses in the version message use pre-BIP155 encoding they cannot
represent a Tor v3 address and we would actually send 16 `0`s instead (a
dummy IPv6 address). However we would print the full address in the log
message. Before this fix:

```
2020-10-21T12:24:17Z send version message: version 70016, blocks=653500, us=[::]:0, them=xwjtp3mj427zdp4tljiiivg2l5ijfvmt5lcsfaygtpp6cw254kykvpyd.onion:8333, peer=0
```

This is confusing because we pretend to send one thing while we actually
send another. Adjust the printout to reflect what we are sending. After
this fix:

```
2020-10-21T12:26:54Z send version message: version 70016, blocks=653500, us=[::]:0, them=[::]:0, peer=0
```
2020-10-21 14:40:42 +02:00
fanquake c92aa8357c
Merge #19911: net: guard vRecvGetData with cs_vRecv and orphan_work_set with g_cs_orphans
da0988daf1 scripted-diff: rename vRecvGetData (Neha Narula)
ba951812ec Guard vRecvGetData (now in net processing) with its own mutex (Neha Narula)
2d9f2fca43 Move vRecvGetData to net processing (Neha Narula)
673247b58c Lock before checking if orphan_work_set is empty; indicate it is guarded (Neha Narula)
8803aee668 Move m_orphan_work_set to net_processing (Neha Narula)
9c47cb29f9 [Rename only] Rename orphan_work_set to m_orphan_work_set. (Neha Narula)

Pull request description:

  Add annotations to guard `vRecvGetData` and `orphan_work_set` and fix up places where they were accessed without a lock. There is no current data race because they happen to be accessed by only one thread, but this might not always be the case.

  Original discussion: https://github.com/bitcoin/bitcoin/pull/18861#discussion_r451778445

ACKs for top commit:
  MarcoFalke:
    review ACK da0988daf1 🐬
  jnewbery:
    Code review ACK da0988daf1
  hebasto:
    ACK da0988daf1, I have reviewed the code and it looks correct, I agree it can be merged.

Tree-SHA512: 31cadd319ddc9273a87e77afc4db7339fd636e816b5e742eba5cb32927ac5cc07a672b2268d2d38a75a0f1b17d93836adab9acf7e52f26ea9a43f54efa57257e
2020-10-19 10:25:38 +08:00
fanquake 661fe5d65c
Merge #20146: net: Send post-verack handshake messages at most once
fa1f6f237d net: Send post-verack handshake messages at most once (MarcoFalke)

Pull request description:

  There is no need to send `SENDHEADERS` and `SENDCMPCT` messages as a reply to each `VERACK` that is received. For alive checks, a `PING`/`PONG` can be used.

ACKs for top commit:
  jonatack:
    Concept ACK fa1f6f237d this is the only code section that sets `fCurrentlyConnected` and `fSuccessfullyConnected` to true. Could add a test. I did not verify if this code is actually being called repeatedly post initial verack; was it?
  hebasto:
    ACK fa1f6f237d, I have reviewed the code and it looks OK, I agree it can be merged.
  naumenkogs:
    ACK fa1f6f237d
  laanwj:
    Code review ACK fa1f6f237d

Tree-SHA512: c841d5d3807254a49463bbcfac3b32881b34a9d3206899544c86322c20988e17ad2ae243cba227fd3825a914f0cb2584451edda2414aecee6d5e3f5a0636f08a
2020-10-15 07:59:19 +08:00
Wladimir J. van der Laan c2c4dbaebd
Merge #19988: Overhaul transaction request logic
fd9a0060f0 Report and verify expirations (Pieter Wuille)
86f50ed10f Delete limitedmap as it is unused now (Pieter Wuille)
cc16fff3e4 Make txid delay penalty also apply to fetches of orphan's parents (Pieter Wuille)
173a1d2d3f Expedite removal of tx requests that are no longer needed (Pieter Wuille)
de11b0a4ef Reduce MAX_PEER_TX_ANNOUNCEMENTS for non-PF_RELAY peers (Pieter Wuille)
242d16477d Change transaction request logic to use txrequest (Pieter Wuille)
5b03121d60 Add txrequest fuzz tests (Pieter Wuille)
3c7fe0e5a0 Add txrequest unit tests (Pieter Wuille)
da3b8fde03 Add txrequest module (Pieter Wuille)

Pull request description:

  This replaces the transaction request logic with an encapsulated class that maintains all the state surrounding it. By keeping it stand alone, it can be easily tested (using included unit tests and fuzz tests).

  The major changes are:

  * Announcements from outbound (and whitelisted) peers are now always preferred over those from inbound peers. This used to be the case for the first request (by delaying the first request from inbound peers), and a bias afters. The 2s delay for requests from inbound peers still exists, but after that, if viable outbound peers remain for any given transaction, they will always be tried first.
  * No more hard cap of 100 in flight transactions per peer, as there is less need for it (memory usage is linear in the number of announcements, but independent from the number in flight, and CPU usage isn't affected by it). Furthermore, if only one peer announces a transaction, and it has over 100 in flight already, we still want to request it from them. The cap is replaced with a rule that announcements from such overloaded peers get an additional 2s delay (possibly combined with the existing 2s delays for inbound connections, and for txid peers when wtxid peers are available).
  * The limit of 100000 tracked announcements is reduced to 5000; this was excessive. This can be bypassed using the PF_RELAY permission (to accommodate locally dumping a batch of many transactions).

  This replaces #19184, rebased on #18044 and with many small changes.

ACKs for top commit:
  ariard:
    Code Review ACK fd9a006. I've reviewed the new TxRequestTracker, its integration in net_processing, unit/functional/fuzzing test coverage. I looked more for soundness of new specification rather than functional consistency with old transaction request logic.
  MarcoFalke:
    Approach ACK fd9a0060f0 🏹
  naumenkogs:
    Code Review ACK fd9a006. I've reviewed everything, mostly to see how this stuff works at the lower level (less documentation-wise, more implementation-wise), and to try breaking it with unexpected sequences of events.
  jnewbery:
    utACK fd9a0060f0
  jonatack:
    WIP light ACK fd9a0060f0 have read the code, verified that each commit is hygienic, e.g. debug build clean and tests green, and have been running a node on and off with this branch and grepping the net debug log. Am still unpacking the discussion hidden by GitHub by fetching it via the API and connecting the dots, storing notes and suggestions in a local branch; at this point none are blockers.
  ryanofsky:
    Light code review ACK fd9a0060f0, looking at txrequest implementation, unit test implementation, and net_processing integration, just trying to understand how it works and looking for anything potentially confusing in the implementation. Didn't look at functional tests or catch up on review discussion. Just a sanity check review focused on:

Tree-SHA512: ea7b52710371498b59d9c9cfb5230dd544fe9c6cb699e69178dea641646104f38a0b5ec7f5f0dbf1eb579b7ec25a31ea420593eff3b7556433daf92d4b0f0dd7
2020-10-14 18:36:59 +02:00
Neha Narula da0988daf1 scripted-diff: rename vRecvGetData
-BEGIN VERIFY SCRIPT-
sed -i 's/vRecvGetData/m_getdata_requests/g' src/net_processing.cpp
-END VERIFY SCRIPT-
2020-10-14 10:08:44 -04:00
Neha Narula ba951812ec Guard vRecvGetData (now in net processing) with its own mutex
This requires slightly reorganizing the logic in GETBLOCKTXN to
maintain locking order.
2020-10-14 10:08:44 -04:00
Neha Narula 2d9f2fca43 Move vRecvGetData to net processing 2020-10-14 10:08:44 -04:00
Neha Narula 673247b58c Lock before checking if orphan_work_set is empty; indicate it is guarded 2020-10-14 10:08:44 -04:00
Neha Narula 8803aee668 Move m_orphan_work_set to net_processing 2020-10-14 10:08:37 -04:00
MarcoFalke fa1f6f237d
net: Send post-verack handshake messages at most once 2020-10-14 10:09:50 +02:00
Neha Narula 9c47cb29f9 [Rename only] Rename orphan_work_set to m_orphan_work_set.
This helps distinguish the member from any local variables.
2020-10-13 17:38:38 -04:00
Pieter Wuille fd9a0060f0 Report and verify expirations 2020-10-12 12:14:53 -07:00
Pieter Wuille cc16fff3e4 Make txid delay penalty also apply to fetches of orphan's parents 2020-10-12 12:14:53 -07:00
Pieter Wuille 173a1d2d3f Expedite removal of tx requests that are no longer needed
Whenever a transaction is added to the mempool or orphan pool, both
its txid and wtxid are considered AlreadyHave, and thus will eventually
be removed from m_txrequest.

The same is true for hashes added to the reject filter, but note that sometimes
only the wtxid is added (in which case only the wtxid can be removed from
m_txrequest).
2020-10-12 12:14:53 -07:00
Pieter Wuille de11b0a4ef Reduce MAX_PEER_TX_ANNOUNCEMENTS for non-PF_RELAY peers
Maintaining up to 100000 INVs per peer is excessive, as that is far more
than fits in a typical mempool.

Also disable the "overload" penalty for PF_RELAY peers.
2020-10-12 12:14:53 -07:00
Pieter Wuille 242d16477d Change transaction request logic to use txrequest
This removes most transaction request logic from net_processing, and
replaces it with calls to a global TxRequestTracker object.

The major changes are:

* Announcements from outbound (and whitelisted) peers are now always
  preferred over those from inbound peers. This used to be the case for the
  first request (by delaying the first request from inbound peers), and
  a bias afters. The 2s delay for requests from inbound peers still exists,
  but after that, if viable outbound peers remain for any given transaction,
  they will always be tried first.

* No more hard cap of 100 in flight transactions per peer, as there is less
  need for it (memory usage is linear in the number of announcements, but
  independent from the number in flight, and CPU usage isn't affected by it).
  Furthermore, if only one peer announces a transaction, and it has over 100
  in flight and requestable already, we still want to request it from them.
  The cap is replaced with an additional 2s delay (possibly combined with the
  existing 2s delays for inbound connections, and for txid peers when wtxid
  peers are available).

Includes functional tests written by Marco Falke and Antoine Riard.
2020-10-12 12:14:11 -07:00
Pieter Wuille 56f9dba015 Only relay IPv4, IPv6, Tor addresses 2020-10-11 11:29:11 -07:00
fanquake 0b2abaa666
Merge #19954: Complete the BIP155 implementation and upgrade to TORv3
dcf0cb4776 tor: make a TORv3 hidden service instead of TORv2 (Vasil Dimov)
353a3fdaad net: advertise support for ADDRv2 via new message (Vasil Dimov)
201a4596d9 net: CAddress & CAddrMan: (un)serialize as ADDRv2 (Vasil Dimov)
1d3ec2a1fd Support bypassing range check in ReadCompactSize (Pieter Wuille)

Pull request description:

  This PR contains the two remaining commits from #19031 to complete the [BIP155](https://github.com/bitcoin/bips/blob/master/bip-0155.mediawiki) implementation:

  `net: CAddress & CAddrMan: (un)serialize as ADDRv2`
  `net: advertise support for ADDRv2 via new message`

  plus one more commit:

  `tor: make a TORv3 hidden service instead of TORv2`

ACKs for top commit:
  jonatack:
    re-ACK dcf0cb4776 per `git diff 9b56a68 dcf0cb4` only change since last review is an update to the release notes which partially picked up the suggested text. Running a node on this branch and addnode-ing to 6 other Tor v3 nodes, I see "addrv2" and "sendaddrv2" messages in getpeerinfo in both the "bytesrecv_per_msg" and "bytessent_per_msg" JSON objects.
  sipa:
    ACK dcf0cb4776
  hebasto:
    re-ACK dcf0cb4776, the node works flawlessly in all of the modes: Tor-only, clearnet-only, mixed.
  laanwj:
    Edit: I have to retract this ACK for now, I'm having some problems with this PR on a FreeBSD node. It drops all outgoing connections with this dcf0cb4776 merged on master (12a1c3ad1a).
  ariard:
    Code Review ACK dcf0cb4

Tree-SHA512: 28d4d0d817b8664d2f4b18c0e0f31579b2f0f2d23310ed213f1f436a4242afea14dfbf99e07e15889bc5c5c71ad50056797e9307ff8a90e96704f588a6171308
2020-10-11 08:51:57 +08:00
Vasil Dimov 353a3fdaad
net: advertise support for ADDRv2 via new message
Introduce a new message `sendaddrv2` to signal support for ADDRv2.
Send the new message immediately after sending the `VERACK` message.

Add support for receiving and parsing ADDRv2 messages.

Send ADDRv2 messages (instead of ADDR) to a peer if he has
advertised support for it.

Co-authored-by: Carl Dong <contact@carldong.me>
2020-10-09 16:42:50 +02:00
Pieter Wuille b6834e312a Avoid 'timing mishap' warnings when mocking 2020-10-08 01:04:29 -07:00
Pieter Wuille ec3916f40a Use mockable time everywhere in net_processing 2020-10-08 01:04:29 -07:00
fanquake db88db4727
Merge #19339: validation: re-delegate absurd fee checking from mempool to clients
b048b275d9 [validation] Remove absurdfee from accepttomempool (John Newbery)
932564b9cf scripted-diff: update max-fee-exceeded error message to include RPC (gzhao408)
8f1290c601 [rpc/node] check for high fee before ATMP in clients (gzhao408)

Pull request description:

  Picked up from #15810. Add separate fee-checking logic for clients that need to enforce max fee rates, then remove the `absurdFee` logic from ATMP.

  ATMP's `nAbsurdFee` argument is used to enforce user-specific behavior (it is not policy since it isn't applied consistently: it is only ever used in RPC and wallet, and set to 0 everywhere else internally). It should be removed from `AcceptToMemoryPool` because (1) validation results/mempool behavior should not be user-specific and (2) enforcing a max fee rate should be the responsibility of the client instead of the mempool.

  Note: this PR does not intend to _remove_ protection from high fees, just re-delegate the responsibility to clients.

ACKs for top commit:
  jnewbery:
    utACK b048b275d9
  LarryRuane:
    re-ACK b048b275d9
  MarcoFalke:
    re-ACK b048b275d9 , only change is squashing one commit 🏦
  instagibbs:
    utACK b048b275d9

Tree-SHA512: 57c17ba16d230a4cae2896dd6a64c924f307757824e35784bf96da7b10aff2d8ea910710edf35e981035623a155f8766209a92a0fdb856549fde78bc3eaae4d2
2020-10-07 10:58:30 +08:00
John Newbery b048b275d9 [validation] Remove absurdfee from accepttomempool
Mempool behavior should not be user-specific.
Checking that txfee is acceptable should be
the responsibility of the wallet or client, not
the mempool.
2020-10-05 04:55:01 -07:00
MarcoFalke cce1513179
Merge #19723: Ignore unknown messages before VERACK
675e55e013 Ignore unknown messages before VERACK (Suhas Daftuar)

Pull request description:

  This allows for feature negotiation to take place with messages between VERSION and VERACK in the future, without requiring additional software changes to specifically ignore messages for features that are unimplemented by our software.

ACKs for top commit:
  sipa:
    utACK 675e55e013
  practicalswift:
    ACK 675e55e01392971aa56bda56cb09498b466d0902: patch looks correct
  MarcoFalke:
    ACK 675e55e013
  hebasto:
    ACK 675e55e013, the offender peer will be eventually disconnected due to the timeout.

Tree-SHA512: 8d2b1d8b9843f2ee26b2c30f7c5ff0bfcfbe3f46b32cd0369c48ece26624151091237e83ce3f18c6da004099026602cfab1642ac916db777f047d170b365c007
2020-10-04 15:39:26 +02:00
Wladimir J. van der Laan 597488d37c
Merge #19871: doc: Clarify scope of eviction protection of outbound block-relay peers
d76925478e [doc] Clarify semantic of peer's m_protect w.r.t to outbound eviction logics (Antoine Riard)
ac71fe936d [doc] Clarify scope of eviction protection of outbound block-relay peers (Antoine Riard)

Pull request description:

  Block-relay-only peers were introduced by #15759. According to its
  author, it was intented to make them only immune to outbound peer
  rotation-based eviction and not from all eviction as modified comment
  leans to think of.

  Clearly indicate that outbound block-relay peers aren't protected
  from eviction by the bad/lagging chain logic.

  Fix #19863

ACKs for top commit:
  naumenkogs:
    ACK d76925478e
  jonatack:
    ACK d76925478e

Tree-SHA512: 597fbd62838a6e39276024165b11514cad20a2e9d33cf9202d261cbadcb62b2df427c858e0cb57e585840d4c1d4600104aa53916bb868541f2580e4eed9b4b52
2020-10-02 16:42:38 +02:00
MarcoFalke 7b7cb70f4c
Merge #19498: Tidy up ProcessOrphanTx
001343f4bc ProcessOrphanTx: Move AddToCompactExtraTransactions call into ProcessOrphanTx (John Newbery)
4fce726bd1 ProcessOrphanTx: Remove aliases (John Newbery)
e07c5d9423 ProcessOrphanTx: Remove outdated commented (John Newbery)
4763b51bca ProcessOrphanTx: remove useless setMisbehaving set (John Newbery)
55c79a9cef ProcessOrphanTx: remove useless done variable (John Newbery)
6e8dd99ef1 [net processing] Add doxygen comments for orphan data and function (John Newbery)

Pull request description:

  Originally a follow-up to #19364, this simplifies the logic in ProcessOrphanTx() and removes unused variables.

ACKs for top commit:
  troygiorshev:
    ACK 001343f4bc
  sipa:
    utACK 001343f4bc
  MarcoFalke:
    ACK 001343f4bc 🌮

Tree-SHA512: be558457f2e08ebb6bddcd49bdd75bd410c3650da44a76c688fc9f07822f94d5a1af93fa1342678052b2c8163cdb9745c352c7884325ab0a41fa593c3eb89116
2020-09-30 15:53:25 +02:00
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
MarcoFalke 4f45ea1f73
Merge #19725: [RPC] Add connection type to getpeerinfo, improve logs
a512925e19 [doc] Release notes (Amiti Uttarwar)
50f94b34a3 [rpc] Deprecate getpeerinfo addnode field (Amiti Uttarwar)
df091b9b50 [refactor] Rename test file to allow any getpeerinfo deprecations. (Amiti Uttarwar)
395acfa83a [rpc] Add connection type to getpeerinfo RPC, update tests (Amiti Uttarwar)
49c10a9ca4 [log] Add connection type to log statement (Amiti Uttarwar)

Pull request description:

  After #19316, we can more directly expose information about the connection type on the `getpeerinfo` RPC. Doing so also makes the existing addnode field redundant, so this PR begins the process of deprecating this field.

  This PR also includes one commit that improves a log message, as both use a shared function to return the connection type as a string.

  Suggested by sdaftuar- https://github.com/bitcoin/bitcoin/pull/19316#discussion_r468001604 & https://github.com/bitcoin/bitcoin/pull/19316#discussion_r468018093

ACKs for top commit:
  jnewbery:
    Code review ACK a512925e19.
  sipa:
    utACK a512925e19
  guggero:
    Tested and code review ACK a512925e.
  MarcoFalke:
    cr ACK a512925e19 🌇
  promag:
    Code review ACK a512925e19.

Tree-SHA512: 601a7a38aee235ee59aca690784f886dc2ae4e418b2e6422c4b58cd597376c00f74910f66920b08a08a0bec28bf8022e71a1435785ff6ba8a188954261aba78e
2020-09-26 17:24:54 +02:00
MarcoFalke 8235dca621
Merge #19979: Replace LockAssertion with AssertLockHeld, remove LockAssertion
0bd1184adf Remove unused LockAssertion struct (Hennadii Stepanov)
ab2a44297f Replace LockAssertion with a proper thread safety annotations (Hennadii Stepanov)
73f71e1996 refactor: Use explicit function type instead of template (Hennadii Stepanov)

Pull request description:

  This PR replaces `LockAssertion` with `AssertLockHeld`, and removes `LockAssertion`.

  This PR is compared with alternatives in https://github.com/bitcoin-core/bitcoin-devwiki/wiki/AssertLockHeld-PRs

ACKs for top commit:
  MarcoFalke:
    ACK 0bd1184adf
  ajtowns:
    ACK 0bd1184adf
  vasild:
    ACK 0bd1184ad

Tree-SHA512: ef7780dd689faf0bb479fdb97c49bc652e2dd10c148234bb95502dfbb676442d8565ee37864d923ca21a25f9dc2a335bf46ee82c095e387b59a664ab05c0ae41
2020-09-23 16:37:07 +02: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