💬 l0rinc commented on pull request "optimization: pack util::Xor into 64 bit chunks instead of doing it byte-by-byte":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r1814564883)
Yes, just saw it, my mistake

🚀 fanquake merged a pull request: "fees: Remove CLIENT_VERSION serialization"
(https://github.com/bitcoin/bitcoin/pull/29702)

💬 l0rinc commented on pull request "optimization: pack util::Xor into 64 bit chunks instead of doing it byte-by-byte":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r1814606913)
Replaced it with `memcpy` and it looks like the compiler successfully uses with [vector instructions](https://www.felixcloutier.com/x86/pxor): https://godbolt.org/z/Koscjconz

💬 maflcko commented on issue "CI failure: `Error: Directory '/tmp/ccache_dir' must be created in advance.`":
(https://github.com/bitcoin/bitcoin/issues/31108#issuecomment-2434777782)
I think asking everyone (probably 100 pull requests overall) to push (normal push, force-push, rebase, ...), or to ignore the failure is a bit too much.

I'll try a temporary workaround.

💬 stickies-v commented on pull request "rest: Support transaction broadcast in REST interface":
(https://github.com/bitcoin/bitcoin/pull/31065#issuecomment-2434788469)
> What is the use of REST interface that doesn't have enough endpoints?

It allows for a binary response format, which is not possible with the (JSON-)RPC interface. For large queries, not having the JSON de/serialization round-trip can make a very meaningful performance difference. As such, I believe adding non-performance-critical REST endpoints is always going to be a tough sell, because they wouldn't be meaningfully different from their RPC alternative, thus adding less benefit for the ma
...

💬 stickies-v commented on issue "Support transaction broadcast in REST interface":
(https://github.com/bitcoin/bitcoin/issues/31017#issuecomment-2434794745)
Note: #31065 was closed because of lack of conceptual support to implement this feature.

💬 laanwj commented on issue "V2 Only Option":
(https://github.com/bitcoin/bitcoin/issues/29618#issuecomment-2434815458)
> CJDNS is still a routed ove rlay network, where packets do not always go the shortest (internet) route to the destination. That is a latency hit vs native IP.

There are some edge cases where super low latency matters, such as mining (or spy nodes :sweat_smile: ), but in general, propagation of anything over the P2P network can be slow and that's fine. There's no tight UI loop. It's not worth to compromise privacy over latency-like it might the case of web browsing, or even the lightning ne
...

👍 laanwj approved a pull request: "test: fix intermittent failure in p2p_seednode.py, don't connect to random IPs"
(https://github.com/bitcoin/bitcoin/pull/31142#pullrequestreview-2391962749)
Code review ACK 6c9fe7b73ea1572b8b56c716ab13d9866f91c6e9

💬 dergoegge commented on pull request "Introduce `g_fuzzing` global for fuzzing checks":
(https://github.com/bitcoin/bitcoin/pull/31093#discussion_r1814676869)
Done!

💬 l0rinc commented on pull request "optimization: pack util::Xor into 64 bit chunks instead of doing it byte-by-byte":
(https://github.com/bitcoin/bitcoin/pull/31144#issuecomment-2434840321)
> where many (most?) of the writes are single bytes (or 4 bytes)

Thanks, I've extended your previous benchmarks with both Autofile serialization and very small vectors.
I will also run a reindex of 400k blocks before and after to see if the effect is measurable or not.

💬 naumenkogs commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1814685980)
Okay, see the last two commits [here](https://github.com/naumenkogs/bitcoin/commits/2024-10-30116-fix-underflow/).

I also dropped the `<0.001` check, it's kinda pointless anyway...

💬 sdaftuar commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1814688729)
Fixed.

💬 fanquake commented on pull request "build: replace custom `MAC_OSX` macro with existing `__APPLE__`":
(https://github.com/bitcoin/bitcoin/pull/29450#issuecomment-2434855072)
@l0rinc can you rebase this?

💬 sdaftuar commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1814692827)
@0xB10C Any thoughts on changing the way replacement logging works via tracepoints, in the package RBF case? One issue I can think of with what I'm proposing here is that the "package hash" we'd be sending is not one that would come through any other tracing messages -- would that be a problem?

If that is a problem, would it make sense to add a tracepoint that lines up with the existing `txpackages` debug log message, which (as of this PR) would now report the package hash along with the tran
...

💬 sdaftuar commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1814694147)
Tracking this issue above (https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1814692827), so marking this conversation as resolved.

💬 sdaftuar commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1814698298)
I think this is done?

💬 laanwj commented on pull request "net: option to disallow v1 connection on ipv4 and ipv6 peers":
(https://github.com/bitcoin/bitcoin/pull/30951#issuecomment-2434870262)
> Well, to gain the benefits of BIP324. For example if you are concerned about someone inspecting the traffic to your node it really doesn't matter how many BIP324 peers you have if you have just one unencrypted connection.

Right. Would it maybe make sense to have an option to restrict transaction broadcast an relay to v2-only, for IPv4 and IPv6 connections? Instead of not connect at all? Eg consider v1 connections blocks-only (except those over Tor/I2P). This alleviates the risk of network
...

💬 maflcko commented on pull request "Introduce `g_fuzzing` global for fuzzing checks":
(https://github.com/bitcoin/bitcoin/pull/31093#discussion_r1814701175)
I don't see it. I still see `else if (g_fuzzing && !val) abort();`. Thus, I don't see any `||g_fuzzing`. Also, the `std::abort` wasn't removed nor the "duplicate" val check.

💬 maflcko commented on pull request "Introduce `g_fuzzing` global for fuzzing checks":
(https://github.com/bitcoin/bitcoin/pull/31093#discussion_r1814702785)
If you want to keep them for some reason, it would be good to explain, and then also add the missing includes.

💬 sdaftuar commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1814706398)
Agreed, I have restored this argument where `LimitMempoolSize()` is invoked in `AcceptSingleTransaction()`, and I pulled this change out into its own commit so that it's easier to review and less tangled with the other changes.

More thoughts on this [here](https://github.com/bitcoin/bitcoin/pull/28251#issuecomment-2433501732). It would be nice to have a test that packages which involve a parent transaction that would be just at the bottom of the mempool without their children are able to mak
...

📝 maflcko opened a pull request: "ci: Temporary workaround for old CCACHE_DIR cirrus env"
(https://github.com/bitcoin/bitcoin/pull/31146)
On a CI re-run, the historic env vars and CI config is used from Cirrus. However, the most recent CI config and CI scripts from this repo are used. This may lead to issues.

For example, `CCACHE_DIR` in the old location may be missing on new CI workers and lead to errors.

Fix it, by falling back to the old logic when the old `CCACHE_DIR` was detected.