🤔 dergoegge reviewed a pull request: "ci: Add test coverage job"
(https://github.com/bitcoin/bitcoin/pull/27547#pullrequestreview-1432356263)
Concept ACK

Thank you for working on this, I think this would be valuable to have.

It seems like it would be easy to replace codecov with something we could host on our own infra (should that become necessary). For now codecov seems fine to me though.

> Read and write access to ... pull requests

Does this mean codecov can push changes to pull request or just that it can comment?

💬 dergoegge commented on pull request "ci: Add test coverage job":
(https://github.com/bitcoin/bitcoin/pull/27547#discussion_r1197600768)
Maybe add an option to skip this step? then devs could run this job locally, without it failing to upload the reports.

📝 MarcoFalke opened a pull request: " test: Add test to check tx in the last block can be downloaded "
(https://github.com/bitcoin/bitcoin/pull/27695)
If a peer received an `inv` about a transaction, which was included in a block before receiving the corresponding `getdata`, it can be beneficial to send this transaction to the peer to aid compact block relay.

Add a test for this to avoid breaking it in the future.

📝 hebasto opened a pull request: "build: Do not define `ENABLE_ZMQ` when ZMQ is not available"
(https://github.com/bitcoin/bitcoin/pull/27696)
A new behavior is consistent with the other optional dependencies.

The source code contains `#if ENABLE_ZMQ` lines only:
```
$ git grep ENABLE_ZMQ -- src/*.cpp
src/init.cpp:#if ENABLE_ZMQ
src/init.cpp:#if ENABLE_ZMQ
src/init.cpp:#if ENABLE_ZMQ
src/init.cpp:#if ENABLE_ZMQ
src/init.cpp:#if ENABLE_ZMQ
```

Change in description line -- "Define to 1..." --> "Define this symbol.." -- is motivated by the fact that the actual value of the defined `ENABLE_ZMQ` macro does not matter at all.
...

🚀 fanquake merged a pull request: "build: Bump minimum supported Clang to clang-10"
(https://github.com/bitcoin/bitcoin/pull/27682)

💬 fanquake commented on pull request "build: Bump minimum supported GCC to g++-9":
(https://github.com/bitcoin/bitcoin/pull/27662#issuecomment-1552830486)
Ok. Lets kill `l_filesystem.m4`.

💬 MarcoFalke commented on pull request "p2p: Stop relaying non-mempool txs":
(https://github.com/bitcoin/bitcoin/pull/27625#issuecomment-1552831521)
> I think I agree with https://github.com/bitcoin/bitcoin/pull/27625#issuecomment-1544047433 that we should ensure we serve any txs in the most recent block

Added a test for this in https://github.com/bitcoin/bitcoin/pull/27695, which should fail on this pull as is. I think the test only checks for `sendcmpct 0` (low bandwidth) behavior. And only for the case where the `tx`-`getdata` was already sent on the wire, because otherwise Bitcoin Core on current `master` would queue it behind the `cm
...

💬 brunoerg commented on pull request "fuzz: net, add `recv_flood_size`, `prefer_evict` in `ConsumeNode`":
(https://github.com/bitcoin/bitcoin/pull/27678#issuecomment-1552835319)
I noticed they were missing while working on net fuzzing improvement, so would be better to add them just when adding coverage for functions that use them?

💬 dergoegge commented on pull request "fuzz: net, add `recv_flood_size`, `prefer_evict` in `ConsumeNode`":
(https://github.com/bitcoin/bitcoin/pull/27678#issuecomment-1552837477)
> so would be better to add them just when adding coverage for functions that use them?

Yes I think that would be better, otherwise there is no need to add them.

💬 brunoerg commented on pull request "fuzz: net, add `recv_flood_size`, `prefer_evict` in `ConsumeNode`":
(https://github.com/bitcoin/bitcoin/pull/27678#issuecomment-1552838477)
> Yes I think that would be better, otherwise there is no need to add them.

Ok, cool. I will change this PR to draft.

📝 brunoerg converted_to_draft a pull request: "fuzz: net, add `recv_flood_size`, `prefer_evict` in `ConsumeNode`"
(https://github.com/bitcoin/bitcoin/pull/27678)
This PR adds `recv_flood_size` and `prefer_evict` in `CNodeOptions` when creating a new CNode in `ConsumeNode`. I noticed they were missing while working on an improvement for net fuzzing.

Checked that #27324 added `recv_flood_size` into `CNodeOptions` and #25962 added`prefer_evict`.

💬 fanquake commented on pull request "fuzz: net, add `recv_flood_size`, `prefer_evict` in `ConsumeNode`":
(https://github.com/bitcoin/bitcoin/pull/27678#issuecomment-1552839469)
I think it can just be closed for now? Can be re-opened when there are relevant changes.

👍 fanquake approved a pull request: "ci: Use credits for ARM task"
(https://github.com/bitcoin/bitcoin/pull/27690#pullrequestreview-1432431581)
ACK fa3761d19d716c2a5b25c76d092b1afbf65ec1d0

🚀 fanquake merged a pull request: "ci: Use credits for ARM task"
(https://github.com/bitcoin/bitcoin/pull/27690)

💬 MarcoFalke commented on pull request "fuzz: Print error message when FUZZ is missing":
(https://github.com/bitcoin/bitcoin/pull/27672#discussion_r1197651079)
Thx, done

👍 TheCharlatan approved a pull request: "doc: remove mention of glibc 2.10+"
(https://github.com/bitcoin/bitcoin/pull/27689#pullrequestreview-1432438295)
ACK 7014e080154f9c82e4fbe043af292a1e761cdb55

👍 TheCharlatan approved a pull request: "doc: Rework build-unix.md"
(https://github.com/bitcoin/bitcoin/pull/27685#pullrequestreview-1432442207)
ACK fa29651c3ff3e13a42d6505b14971265562f088a

💬 stickies-v commented on pull request "net: Allow inbound whitebind connections to more aggressively evict peers when slots are full":
(https://github.com/bitcoin/bitcoin/pull/27600#discussion_r1197660996)
I'm not sure that's a strict improvement. Documenting the interface (i.e. pretty much just the first line of what's in `net.cpp`) seems more appropriate to be in the header indeed, but all the rest sounds more like implementation details that may best be kept close to the source code?

Also, my main issue is with (having just) the word "find", which to me sounds like there are no side effects.

🚀 fanquake merged a pull request: "doc: remove mention of glibc 2.10+"
(https://github.com/bitcoin/bitcoin/pull/27689)

🚀 fanquake merged a pull request: "msvc: Rename `libbitcoinconsensus` to `libbitcoin_consensus` and other adjustments"
(https://github.com/bitcoin/bitcoin/pull/27615)