… header is split across two buffers

80d4423 Test buffered valid message (Troy Giorshev)

Pull request description:

  This PR is a tweak of bitcoin#19302.  This sends a valid message.

  Additionally, this test includes logging in the same vein as bitcoin#19272.

ACKs for top commit:
  MarcoFalke:
    tested ACK 80d4423 (added an assert(false) to observe deterministic coverage) 🌦
  gzhao408:
    ACK bitcoin@80d4423 👊

Tree-SHA512: 3b1aa5ec480a1661917354788923d64595e2886448c9697ec0606a81293e8b4a4642b2b3cc9afb2206ce6f74e5c6d687308c5ad19cb73c5b354d3071ad8496f8

…e (naming)

…lock in an INV

7467366 [net processing] Only send a getheaders for one block in an INV (John Newbery)

Pull request description:

  Headers-first is the primary method of announcement on the network. If a node fell back sending blocks by inv, it's probably for a re-org. The final block hash provided should be the highest, so send a getheaders and then fetch the blocks we need to catch up.

  Sending many GETHEADERS messages to the peer would cause them to send a large number of potentially large HEADERS messages with redundant data, which is a waste of bandwidth.

ACKs for top commit:
  sipa:
    utACK 7467366
  mzumsande:
    utACK 7467366 as per ajtowns' reasoning.
  naumenkogs:
    utACK 7467366
  ajtowns:
    ACK 7467366
  jonatack:
    ACK 7467366

Tree-SHA512: 59e243b80d3f0873709dfacb2e4ffba34689aad7de31ec7f69a64e0e3a0756235a0150e4082ff5de823949ba4411ee1aed2344b4749b62e0eb1ea906e41f5ea9

…g include

83da576 net: use CMessageHeader::HEADER_SIZE, add missing include (Jon Atack)

Pull request description:

  as suggested 16 months ago by Gleb Naumenko in bitcoin#15197 (comment).

  `static constexpr CMessageHeader::HEADER_SIZE` is already used in this file, `src/net.cpp`, in 2 instances. This commit replaces the remaining 2 integer values in the file with it and adds the explicit include header.

  Co-authored by: Gleb Naumenko <[email protected]>

ACKs for top commit:
  naumenkogs:
    utACK 83da576
  practicalswift:
    ACK 83da576 -- patch looks correct
  theStack:
    ACK 83da576 -- verified that its just magic number elimination refactoring and additionally checked that all tests pass 👍

Tree-SHA512: 5b915483bca4ea162c259865a1b615d73b88a1b1db3f82db05f770d10b8a42494d948f5b21badbcce2d9efa5915b8cbb6af83073867c23d2f152c0d35ac37b96

2896c41 Do not answer GETDATA for to-be-announced tx (Pieter Wuille)
f2f32a3 Push down use of cs_main into FindTxForGetData (Pieter Wuille)
c6131bf Abstract logic to determine whether to answer tx GETDATA (Pieter Wuille)

Pull request description:

  This PR intends to improve transaction-origin privacy.

  In general, we should try to not leak information about what transactions we have (recently) learned about before deciding to announce them to our peers. There is a controlled transaction dissemination process that reveals our transactions to peers that has various safeguards for privacy (it's rate-limited, delayed & batched, deterministically sorted, ...), and ideally there is no way to test which transactions we have before that controlled process reveals them. The handling of the `mempool` BIP35 message has protections in this regard as well, as it would be an obvious way to bypass these protections (handled asynchronously after a delay, also deterministically sorted).

  However, currently, if we receive a GETDATA for a transaction that we have not yet announced to the requester, we will still respond to it if it was announced to *some* other peer already (because it needs to be in `mapRelay`, which only happens on the first announcement). This is a slight privacy leak.

  Thankfully, this seems easy to solve: `setInventontoryTxToSend` keeps track of the txids we have yet to announce to a peer - which almost(*) exactly corresponds to the transactions we know of that we haven't revealed to that peer. By checking whether a txid is in that set before responding to a GETDATA, we can filter these out.

  (*) Locally resubmitted or rebroadcasted transactions may end up in setInventoryTxToSend while the peer already knows we have them, which could result in us incorrectly claiming we don't have such transactions if coincidentally requested right after we schedule reannouncing them, but before they're actually INVed. This is made even harder by the fact that filterInventoryKnown will generally keep known reannouncements out of setInventoryTxToSend unless it overflows (which needs 50000 INVs in either direction before it happens).

  The condition for responding now becomes:

  ```
    (not in setInventoryTxToSend) AND
    (
      (in relay map) OR
      (
        (in mempool) AND
        (old enough that it could have expired from relay map) AND
        (older than our last getmempool response)
      )
    )
  ```

ACKs for top commit:
  naumenkogs:
    utACK 2896c41
  ajtowns:
    ACK 2896c41
  amitiuttarwar:
    code review ACK 2896c41
  jonatack:
    ACK 2896c41 per `git diff 2b3f101 2896c41` only change since previous review is moving the recency check up to be verified first in `FindTxForGetData`, as it was originally in 353a391 (good catch), before looking up the transaction in the relay pool.
  jnewbery:
    code review ACK 2896c41

Tree-SHA512: e7d5bc006e626f60a2c108a9334f3bbb67205ace04a7450a1e4d4db1d85922a7589e0524500b7b4953762cf70554c4a08eec62c7b38b486cbca3d86321600868

…tions from configure

d51f0fa doc: add release notes for 26896 (fanquake)
2b24879 build: remove --enable-upnp-default from configure (fanquake)
02f5a5e build: remove --enable-natpmp-default from configure (fanquake)
25a0e8b Remove configure-time setting of DEFAULT_UPNP (fanquake)
06562e5 Remove configure-time setting of DEFAULT_NATPMP (fanquake)

Pull request description:

  This PR removes the `--enable-upnp-default` and `--enable-natpmp-default` options from configure.

  It's odd to me that we maintain configure-time options for setting the default port-forwarding runtime state (but no other similar options), and I'm not sure what use-case it satisfies, that can't be achieved by multiple other means. I also doubt that we'll ever restart using these in release builds, or turning on any of this by default.

  I think the only scenario these options would be used is when you want to compile your own binaries (we don't use them in Guix), with port-forwarding on by default, but otherwise can't or don't want to use a `.conf` file, can't or don't want to pass command line options at runtime, and also don't want to modify the source code?

ACKs for top commit:
  hebasto:
    ACK d51f0fa, rebased and comments have been addressed since my recent [review](bitcoin#26896 (review)).
  TheCharlatan:
    ACK d51f0fa

Tree-SHA512: 481decd8bddd8b03b7319591e3acf189f7b6b96c9a9a8c5bc1a3f8ec00d0b8f9b52d2f5c28a298a2ec947cfe9611cfd184e393ccb2e4e21bfce86ca7d4de60d3

fabe44e bench: Start nodes with -nodebuglogfile (MarcoFalke)

Pull request description:

  For benchmarking we don't want to depend on the speed of the disk or the amount of debug logging

ACKs for top commit:
  fanquake:
    ACK fabe44e - This makes some of these benchmarks significantly faster to run. MempoolEviction total runtime is down from ~46s to 11s on my machine:

Tree-SHA512: d99700901650325896b9115d20b84a27042152f46266f595bf7ea1414528c0b346f4e707a12ee8b8ba99c35cf155e645e67971c1b2a679c4e609c400ff8b08ae

…num opcode serialize

2748e87 script: prevent UB when computing abs value for num opcode serialize (pierrenn)

Pull request description:

  This was reported by practicalswift here bitcoin#18046

  It seems that the original author of the line used a reference to glibc `abs`: https://github.com/lattera/glibc/blob/master/stdlib/abs.c

  However depending on some implementation details this can be undefined behavior for unusual values.

  A detailed explanation of the UB is provided here : https://stackoverflow.com/questions/17313579/is-there-a-safe-way-to-get-the-unsigned-absolute-value-of-a-signed-integer-with (by [Billy O'Neal](https://twitter.com/malwareminigun))

  Simple relevant godbolt example :  https://godbolt.org/z/yRwtCG

  Thanks!

ACKs for top commit:
  sipa:
    ACK 2748e87
  MarcoFalke:
    ACK 2748e87, only checked that the bitcoind binary does not change with clang -O2 🎓
  practicalswift:
    ACK 2748e87

Tree-SHA512: 539a34c636c2674c66cb6e707d9d0dfdce63f59b5525610ed88da10c9a8d59d81466b111ad63b850660cef3750d732fc7755530c81a2d61f396be0707cd86dec

…owngrade after upgrade

489ebfd tests: feature_backwards_compatibility.py test downgrade after upgrade (Andrew Chow)

Pull request description:

  After upgrading the node, try to go back to the original version to make sure that using a newer node version does not prevent the wallet file from being downgraded again.

ACKs for top commit:
  MarcoFalke:
    ACK 489ebfd

Tree-SHA512: 86231de6514b3657912fd9d6621212166fd2b29b591fc97120092c548babcf1d6f50b5bd103b28cecde395a26809134f01c1a198725596c3626420de3fd1f017