✅ 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.

💬 theStack commented on pull request "test: Remove struct.pack from almost all places":
(https://github.com/bitcoin/bitcoin/pull/29401#issuecomment-2143824209)
Concept ACK

💬 theStack commented on pull request "doc, rpc: Release notes and follow-ups for #29612":
(https://github.com/bitcoin/bitcoin/pull/30167#discussion_r1623464945)
nit: not a must, but it could be interesting for users to know what exactly has improved with the new format (i.e. enhanced metadata, enabling more detailed error messages etc.)

👍 theStack approved a pull request: "doc, rpc: Release notes and follow-ups for #29612"
(https://github.com/bitcoin/bitcoin/pull/30167#pullrequestreview-2092374868)
utACK efc1b5be8a4696c0db19ba18316b2d4ed09e10f2

💬 davidgumberg commented on pull request "Lint: support running individual rust linters and improve subtree exclusion":
(https://github.com/bitcoin/bitcoin/pull/29965#issuecomment-2143889113)
Rebased to address merge conflict

💬 hebasto commented on issue "test: SegFault in `ismine_tests` on SunOS / illumos":
(https://github.com/bitcoin/bitcoin/issues/29908#issuecomment-2143979930)
I've tested the recent master branch @ 457e1846d2bf6ef9d54b9ba1a330ba8bbff13091 on the updated system:
```
$ uname -a
SunOS openindiana 5.11 illumos-82079dec87 i86pc i386 i86pc
```

The issue is no longer reproducible.

Closing.

✅ hebasto closed an issue: "test: SegFault in `ismine_tests` on SunOS / illumos"
(https://github.com/bitcoin/bitcoin/issues/29908)

✅ fanquake closed an issue: "build, android: `make apk` fails if depends source cache is in a non-default location"
(https://github.com/bitcoin/bitcoin/issues/22522)