💬 furszy commented on pull request "test: Fixes and updates to wallet_backwards_compatibility.py for 25.0 and descriptor wallets":
(https://github.com/bitcoin/bitcoin/pull/28027#discussion_r1264468473)
s/sinced/since

💬 jonatack commented on pull request "I2P: also sleep after errors in Accept() & destroy the session if we get "Session was closed"":
(https://github.com/bitcoin/bitcoin/pull/28077#issuecomment-1636857565)
Initial testing with the Java I2P Router shows that this pull fixes the issue (nice work!)

On master without this PR, thousands of lines of error messages are logged (`Error accepting: Cannot decode Base64: "STREAM STATUS RESULT=I2P_ERROR"`) and bitcoind I2P session re-creation fails until the I2P router is manually stopped and restarted.

With this pull, the error is logged only once and there is no need to restart the I2P Router. A new persistent I2P session is created on the second try.
...

💬 ajtowns commented on pull request "p2p: Drop m_recently_announced_invs bloom filter":
(https://github.com/bitcoin/bitcoin/pull/27675#discussion_r1264577309)
I can see the point of adding `nodiscard` in that case for confusing APIs (eg, where someone not familiar with the API might write `foo.empty()` when they meant `foo.clear()`) but for a `foo.GetBar()` function, it just seems like busy work, that I don't think makes sense to do.

💬 ajtowns commented on pull request "p2p: Drop m_recently_announced_invs bloom filter":
(https://github.com/bitcoin/bitcoin/pull/27675#discussion_r1264577392)
Updated to not add txids to the known filter for wtxid peers, and to set the entry sequence to 0 when adding txs that were in a block that was reorged out.

💬 MarcoFalke commented on issue "fuzz: connman, `m_nodes` is always empty":
(https://github.com/bitcoin/bitcoin/issues/27980#issuecomment-1637013376)
Jup. Sgtm. If there are no downsides, I don't see a reason not to do it.

📝 MarcoFalke opened a pull request: "ci: Use DOCKER_BUILDKIT for lint image"
(https://github.com/bitcoin/bitcoin/pull/28083)
Currently the lint docker/podman image has many issues:

* It relies on an EOL debian version.
* It relies on a debian version different from the one used in the CI lint task.
* It relies on the legacy docker build command, which requires the user to make `cd ./ci/lint/` before the build step.
* It doesn't use the `.python-version` file, but a hardcoded version.

Fix all issues by using the recommended `DOCKER_BUILDKIT=1` to generate the image.

Also:
* Rename `/tmp/python` to `/python
...

📝 fanquake opened a pull request: "doc: update windows `-fstack-clash-protection` doc"
(https://github.com/bitcoin/bitcoin/pull/28084)
Now that changes have been made in GCC, to fix the build failures.
See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90458.

💬 pinheadmz commented on pull request "net: support unix domain sockets for -proxy and -onion":
(https://github.com/bitcoin/bitcoin/pull/27375#issuecomment-1637129953)
@vasild OK I got this branch passing all CI. ✅ I ran tests locally on Ubuntu and MacOS and also tested the feature in production on both platforms with `tor --SocksPort unix:/...` Everything is working! yay.

The real fix was here actually (diff below) Embarrassingly simple. This was a bug on Linux but NOT on macOS.

I did not need the `SendComplete` changes or `Socks5` changes which is great because that keeps review simple and the branch focused. I will be happy to review those changes if
...

👍 TheCharlatan approved a pull request: "doc: update windows `-fstack-clash-protection` doc"
(https://github.com/bitcoin/bitcoin/pull/28084#pullrequestreview-1531767457)
ACK 05ef059a333479e553382c2ae6ef6fde668ce3cb

💬 jonatack commented on pull request "I2P: also sleep after errors in Accept() & destroy the session if we get "Session was closed"":
(https://github.com/bitcoin/bitcoin/pull/28077#issuecomment-1637131364)
Verified that the issue doesn't occur with the i2pd router, only with the Java I2P router.

👍 hebasto approved a pull request: "doc: update windows `-fstack-clash-protection` doc"
(https://github.com/bitcoin/bitcoin/pull/28084#pullrequestreview-1531781254)
ACK 05ef059a333479e553382c2ae6ef6fde668ce3cb, I've verified that the fix commit is present in all branches starting from `gcc-11`.

✅ hebasto closed an issue: "Sign PSBT: Can't verify change output"
(https://github.com/bitcoin-core/gui/issues/732)

🚀 hebasto merged a pull request: "Show own outputs on PSBT signing window"
(https://github.com/bitcoin-core/gui/pull/740)

💬 GBKS commented on issue "Creating a Wallet Feature Guidelines Document":
(https://github.com/bitcoin/bitcoin/issues/28062#issuecomment-1637177521)
I think both lists are important. Maybe you can kick off that effort?

I can't speak to the core wallet, but with the related GUI project I am helping with, we are trying to create solid design documentation from the start ([see here](https://bitcoincore.app)), which may also end up including a good amount of feature specs. We're still early in this, but I do think it's worth the effort to document things well. It gets easier to make decisions, newcomers can more easily understand the goals an
...

💬 Crypt-iQ commented on pull request "http: add evhttp_connection_set_closecb to avoid g_requests hang":
(https://github.com/bitcoin/bitcoin/pull/27909#issuecomment-1637239099)
I am a complete libevent noob, but I noticed some odd behavior that makes me think this is a race condition. I used the following diff that logged both sides' port numbers and the above python script:

<details>
<summary>diff</summary>
<br>

```
diff --git a/src/httpserver.cpp b/src/httpserver.cpp
index 849e9b482..8bb9d407f 100644
--- a/src/httpserver.cpp
+++ b/src/httpserver.cpp
@@ -223,13 +223,21 @@ static void http_request_cb(struct evhttp_request* req, void* arg)
// comp
...

💬 doperiddle commented on issue "BIP324 tracking issue":
(https://github.com/bitcoin/bitcoin/issues/27634#issuecomment-1637262311)
> This issue will be updated to reflect the current state of [BIP324](https://github.com/bitcoin/bips/blob/master/bip-0324.mediawiki) integration.
>
> PRs ready for review:
>
> * [Make poly1305 support incremental computation + modernize #27993](https://github.com/bitcoin/bitcoin/pull/27993)
>
> Overall plan:
>
> * [x] ElligatorSwift integration in Bitcoin Core: [BIP324: ElligatorSwift integrations #27479](https://github.com/bitcoin/bitcoin/pull/27479)
>
> * [x] Dependency:
...

📝 theStack opened a pull request: "refactor: use Span for SipHash::Write"
(https://github.com/bitcoin/bitcoin/pull/28085)
This simple refactoring PR changes the interface for the `SipHash` arbitrary-data `Write` method to take a `Span<unsigned char>` instead of having to pass data and length. (`Span<std::byte>` seems to be more modern, but vectors of `unsigned char` are still used prety much everywhere where SipHash is called, and I didn't find it very appealing having to clutter the code with `Make(Writable)ByteSpan` helpers).

💬 russeree commented on pull request "rpc: doc: Added `longpollid` and `data` params to `template_request` #27998":
(https://github.com/bitcoin/bitcoin/pull/28056#issuecomment-1637417549)
Concept ACK

I do have a few questions about the implementation of the RPCHelpMan parameter text for 'longpollid' when used for a template instead of a propsal. My primary issue is the block proposal definition for ```longpollid``` at https://github.com/bitcoin/bitcoin/blob/57b8336dfed6312003cf34cd5ae7099f77115e73/src/rpc/mining.cpp#L613
varies from the current PRs RPCHelpMan when used as an optional parameter for a block template. But the use of the passed parameter for ```longpollid``` at
...

💬 MarcoFalke commented on issue "valgrind: Syscall param ppoll(ufds.events) points to uninitialised byte(s)":
(https://github.com/bitcoin/bitcoin/issues/28072#issuecomment-1637472437)
Ok, let's continue discussion there.

✅ MarcoFalke closed an issue: "valgrind: Syscall param ppoll(ufds.events) points to uninitialised byte(s)"
(https://github.com/bitcoin/bitcoin/issues/28072)