💬 hebasto commented on pull request "ZMQ: Support UNIX domain sockets":
(https://github.com/bitcoin/bitcoin/pull/27679#discussion_r1580774357)
> > No warnings for me locally using
>
> Did you turn on the relevant warning flag

My bad! :facepalm:

💬 laanwj commented on pull request "guix: remove bzip2 from deps":
(https://github.com/bitcoin/bitcoin/pull/29895#issuecomment-2079034463)
Always some surprises...
```python
#!/usr/bin/env python3
from os import path
import os
from subprocess import run
import bz2
import gzip

GIT='git'
TMPDIR='tmp_checkout'
BRANCH='remove_xz_guix'
BASE_COMMIT='c05c214f2e9cfd6070a3c7680bfa09358fd9d97a'

def compare_streams(f, g, bsize=1024*1024):
while True:
b1 = f.read(bsize)
b2 = g.read(bsize)
if b1 != b2:
return False
if len(b1) == 0 and len(b2) == 0: # Both at EOF

...

💬 Sjors commented on pull request "Reintroduce external signer support for Windows":
(https://github.com/bitcoin/bitcoin/pull/29868#issuecomment-2079035706)
Tested the Guix build on Windows 11.

💬 fanquake commented on pull request "depends: Do not consider `CC` environment variable for detecting system":
(https://github.com/bitcoin/bitcoin/pull/29963#issuecomment-2079037412)
> Surely this isn't the only place env vars like this would cause issues? Feels like a cat-and-mouse game.

Yea, as-is this doesn't seem like a great fix, and may just break other things?

A better diff might be something like:
```diff
diff --git a/depends/Makefile b/depends/Makefile
index 005d9696fb..091511758d 100644
--- a/depends/Makefile
+++ b/depends/Makefile
@@ -51,7 +51,7 @@ FALLBACK_DOWNLOAD_PATH ?= https://bitcoincore.org/depends-sources
C_STANDARD ?= c11
CXX_STANDARD ?= c
...

👍 hebasto approved a pull request: "refactor: Avoid unused-variable warning in init.cpp"
(https://github.com/bitcoin/bitcoin/pull/29968#pullrequestreview-2024633825)
ACK fa5ba429a71eb81848560b4a547d1717efd33593, tested on Ubuntu 24.04 with `-Wall`.

💬 Sjors commented on pull request "guix: remove bzip2 from deps":
(https://github.com/bitcoin/bitcoin/pull/29895#issuecomment-2079052784)
@laanwj did you mean `'-b', BRANCH` instead of `'-b', branch`?

I also get a difference on qrencode when running this on macOS 13.6.6 with Python 3.12.1.

💬 laanwj commented on pull request "guix: remove bzip2 from deps":
(https://github.com/bitcoin/bitcoin/pull/29895#issuecomment-2079056842)
Contents are the same, there is a slight difference in the metadata:
```
--- qrencode-4.1.1.tar_gz.hex 2024-04-26 11:58:29.335356741 +0200
+++ qrencode-4.1.1.tar_bz2.hex 2024-04-26 11:58:16.387499575 +0200
@@ -4,7 +4,7 @@
00000060 00 00 00 00 30 30 30 30 37 35 35 00 30 30 30 31 |....0000755.0001|
00000070 37 35 30 00 30 30 30 31 37 35 30 00 30 30 30 30 |750.0001750.0000|
00000080 30 30 30 30 30 30 30 00 31 33 37 33 34 33 30 33 |0000000.13734303|
-00000090 36 36 36 00 30 3
...

💬 hebasto commented on pull request "depends: Do not consider `CC` environment variable for detecting system":
(https://github.com/bitcoin/bitcoin/pull/29963#issuecomment-2079059322)
> A better diff might be something like:
> ...
> which would at least be using the option that is meant to be used for this.

I agree. Implemented.

Thanks!

💬 glozow commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1580742501)
Thanks, I've moved it to the outer loop.
Kept bool, seems harmless and I imagine we can find some more interesting code paths by sometimes not setting it immediately.

💬 glozow commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1580739332)
As discussed offline, I agree the fuzz test has a few quirks that could be fixed up, but I think they are out of scope for this PR so I'm going to mark this resolved.

💬 glozow commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1580743469)
added

💬 glozow commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1580756146)
haha oops, deleted

💬 glozow commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1580755767)
Ok I've done a bit of refactoring to split this into 2 functions for finding the package and validating/processing the package, and made a `PackageToValidate` struct that's difficult to misuse. Got rid of these "set me if you find something" variables.

💬 glozow commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1580737130)
fixed

💬 glozow commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1580750294)
Packages aren't ever reconsiderable since we never submit anything beyond 1p1c. I've clarified this in the docs now. I definitely think we should cache a rejection when the error is `PCKG_POLICY` or `PCKG_MEMPOOL_ERROR`.

💬 glozow commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1580736833)
added

💬 glozow commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1580698811)
done

💬 glozow commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1580693314)
removed

💬 josibake commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#issuecomment-2079095313)
@Sjors I've updated #28122 to sort outpoints lexicographically and added @setavenger 's test case. This should fix the discrepancy between your two indexes. Thanks again for testing this! Really glad we caught this one.

💬 Sjors commented on pull request "Silent Payments: Implement BIP352":
(https://github.com/bitcoin/bitcoin/pull/28122#discussion_r1580842378)
7a27533332d14cf98f7e165f464142132e9f9d85: if you're going to change this here instead of adding a custom sort for BIP352, then you should add a note that it's critical for BIP352.

However I'm not sure if we should sort this way by default. If I wanted to list transaction outputs, I'd want to sorted by their integer version.

That said, I don't know if we currently expose any RPC where this sort order is exposed.