💬 theuni commented on pull request "refactor: use recommended type hiding on multi_index types":
(https://github.com/bitcoin/bitcoin/pull/30194#issuecomment-2143082508)
Accidentally pushed a temp branch here, ignore the noise. Still ready for review.

💬 theStack commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#discussion_r1623063336)
As already [mentioned in the secp256k1 PR](https://github.com/bitcoin-core/secp256k1/pull/1519#issuecomment-2143167510), this assert makes the indexer crash on signet on block 198023 for tx https://mempool.space/signet/tx/d73f4a19f3973e90af6df62e735bb7b31f3d5ab8e7e26e7950651b436d093313, which contains input pubkeys that add up to zero (point of infinity):

```
.....
2024-05-31T22:31:57Z Syncing bip352 index with block chain from height 113222
2024-05-31T22:32:27Z Syncing bip352 index with b
...

💬 laanwj commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#issuecomment-2143396412)
> Maybe too late or has been looked at. A PCP/NAT-PMP library https://github.com/libpcp/pcp

Yes, some revievers have been looking at it as reference/comparison.

This has been said before but: implementing a self-contained version of the RFCs here is intentional, we don't want to introduce a dependency on a third-party library. It shoudn't be needed for a straightforward fixed packet-based protocol. The intent is to have code that integrates with bitcoin core's frameworks for logging, netwo
...

👍 tdb3 approved a pull request: "doc: JSON-RPC request Content-Type is application/json"
(https://github.com/bitcoin/bitcoin/pull/30215#pullrequestreview-2092102442)
ACK for 3c08e11c3ea4499e8d20609e2417cac859b3e98e

This is a reasonable change, which over time can help nudge implementations to usage of the more appropriate content-type `application/json`.

#### Spec tracing
I didn't see mention of content-type or linked RFC in the JSON-RPC v1 spec (https://www.jsonrpc.org/specification_v1), but the JSON-RPC 2.0 spec does reference RFC 4627 (https://www.ietf.org/rfc/rfc4627.txt), which specifies (in section 6) the type/subtype of `application/json`. RFC
...

💬 brunoerg commented on pull request "fuzz: Make FuzzedSock fuzz friendlier":
(https://github.com/bitcoin/bitcoin/pull/30211#issuecomment-2143444520)
Concept ACK

🤔 Amygo777 reviewed a pull request: "JSON-RPC request Content-Type is application/json"
(https://github.com/bitcoin/bitcoin/pull/29946#pullrequestreview-2092111082)
Nice

💬 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-2143471790)
`d0c8109dd0...2e5e69a2d6`: rebase, address suggestions and most importantly - @achow101, wow! thank you for catching this! The problem was that when a functional test requests `bind_to_localhost_only=False` then the framework would not use `bind=127.0.0.1` and it will start `bitcoind` without any `bind=...` and thus would still try to bind on `127.0.0.1:18445` causing collisions. The solution is to use `bind=0.0.0.0` in that case.

💬 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_r1623233243)
Dropped this line altogether.

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