💬 vasild commented on pull request "Make it possible to disable Tor binds and abort startup on bind failure":
(https://github.com/bitcoin/bitcoin/pull/22729#discussion_r1623233322)
Changed to use F-strings.

💬 vasild commented on pull request "Make it possible to disable Tor binds and abort startup on bind failure":
(https://github.com/bitcoin/bitcoin/pull/22729#discussion_r1623235276)
Done, thanks!

💬 vasild commented on pull request "Make it possible to disable Tor binds and abort startup on bind failure":
(https://github.com/bitcoin/bitcoin/pull/22729#issuecomment-2143477308)
`2e5e69a2d6...8c3087150c`: forgot to address one suggestion, sorry for the noise

👍 vasild approved a pull request: "init: fixes file descriptor accounting"
(https://github.com/bitcoin/bitcoin/pull/30065#pullrequestreview-2092138396)
ACK a9e211a5302630635b825655cd984683794aa42c

Note: this is the last but one commit. The last commit 23f843d816 `init: fix min required fds check and account for outbounds` now contains a single dummy change of removing an empty line. Can/should be dropped altogether. You can `push -f` a9e211a5302630635b825655cd984683794aa42c into this branch.

Thanks!

💬 vasild commented on pull request "init: fixes file descriptor accounting":
(https://github.com/bitcoin/bitcoin/pull/30065#discussion_r1623248432)
Excellent! Some simplifications leading to further simplifications. :heart:

✅ kosuodhmwa closed an issue: "VM disk for OS (Debian 12.x) gets smaller and smaller - NOT the same disk i used for .bitdoin data directory which is mounted on another disk"
(https://github.com/bitcoin/bitcoin/issues/30191)

💬 kosuodhmwa commented on issue "VM disk for OS (Debian 12.x) gets smaller and smaller - NOT the same disk i used for .bitdoin data directory which is mounted on another disk":
(https://github.com/bitcoin/bitcoin/issues/30191#issuecomment-2143529011)
solved

✅ kosuodhmwa closed an issue: "Tons of Socks5() connect to x.x.x.x:8333 failed: connection refused-messages when i use TOR - why?"
(https://github.com/bitcoin/bitcoin/issues/29759)

💬 kosuodhmwa commented on issue "Tons of Socks5() connect to x.x.x.x:8333 failed: connection refused-messages when i use TOR - why?":
(https://github.com/bitcoin/bitcoin/issues/29759#issuecomment-2143529197)
closed

🤔 vasild reviewed a pull request: "fuzz: Make FuzzedSock fuzz friendlier"
(https://github.com/bitcoin/bitcoin/pull/30211#pullrequestreview-2092217352)
Approach ACK 617d43e656816a532ca275707dffeb6e2a2e1fc7

Just one blocker below (about the overflow).

Thank you for the improvements!

💬 vasild commented on pull request "fuzz: Make FuzzedSock fuzz friendlier":
(https://github.com/bitcoin/bitcoin/pull/30211#discussion_r1623330207)
from the commit message:

> FuzzedSock only supports peeking at one byte at a time, which is not
> fuzzer friendly when trying to receive long data.

I am not sure how likely is this to happen, but at least it is not ruled out by the `recv(2)` documentation: a peek read of 10 bytes returns just 1 byte and repeating the peek read of 10 bytes returns the same 1 byte over and over again. The app can never peek past the 1 byte without actually consuming/reading that byte. I guess it must be rea
...

💬 vasild commented on pull request "fuzz: Make FuzzedSock fuzz friendlier":
(https://github.com/bitcoin/bitcoin/pull/30211#discussion_r1623329527)
This will overflow the buffer in `buf` in the memcopy below if peek read is called with N bytes and then non-peek read is called with M bytes and M < N.

💬 vasild commented on pull request "fuzz: Make FuzzedSock fuzz friendlier":
(https://github.com/bitcoin/bitcoin/pull/30211#discussion_r1623328097)
I guess using `ConsumeRandomLengthByteVector()` is slower than `ConsumeBytes()` because it uses an intermediate `std::string`, but is probably insignificant.

The comment says:

```cpp
// Returns a std::string of length from 0 to |max_length|. When it runs out of
// input data, returns what remains of the input. Designed to be more stable
// with respect to a fuzzer inserting characters than just picking a random
// length and then consuming that many bytes with |ConsumeBytes|.
inline s
...

💬 vasild commented on pull request "fuzz: Make FuzzedSock fuzz friendlier":
(https://github.com/bitcoin/bitcoin/pull/30211#discussion_r1623330824)
Wow! Good catch! Maybe this deserves a comment because there is deeper reason for the way it is written than one may see at a first glance. (aka to prevent somebody restoring it back to `? requested : 0` in a future refactor, code reorganization).

🤔 glozow reviewed a pull request: "fuzz: increase `txorphan` harness stability"
(https://github.com/bitcoin/bitcoin/pull/30186#pullrequestreview-2092227439)
utACK 8defc182a31d828ad0f53ebf7e3be9e9cfc42d09

💬 glozow commented on pull request "policy: bump TX_MAX_STANDARD_VERSION to 3":
(https://github.com/bitcoin/bitcoin/pull/29496#discussion_r1623341181)
thanks! fixed these now

💬 TheCharlatan commented on pull request "RFC: Remove boost usage from kernel api / headers":
(https://github.com/bitcoin/bitcoin/pull/28335#issuecomment-2143743732)
Closing this. There is some work being done towards removing boost from the mempool wholesale. Since that would be preferable, pruning just the headers can be revisited in case that effort proves unsuccessful.

✅ TheCharlatan closed a pull request: "RFC: Remove boost usage from kernel api / headers"
(https://github.com/bitcoin/bitcoin/pull/28335)

💬 hebasto commented on issue "build, android: `make apk` fails if depends source cache is in a non-default location":
(https://github.com/bitcoin/bitcoin/issues/22522#issuecomment-2143768165)
This can be closed as builds for Android are [not possible](https://github.com/bitcoin/bitcoin/issues/29360) in the master branch.

💬 theStack commented on pull request "test: improve BDB parser (handle internal/overflow pages, support all page sizes)":
(https://github.com/bitcoin/bitcoin/pull/30125#discussion_r1623397692)
The main reason for using `struct.unpack` in this PR is to be consistent with the already existing code in the bdb module (i.e. avoiding to mix up different serialization methods), and I didn't want to introduce large refactoring changes in this PR. I strongly agree though that `int.{to,from}_bytes` is preferred and support #29401.