💬 juanitoddd commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2876596959)
Concept NACK

There should be freedom and sovereignty on what users relay on their nodes mempool. To marked them as deprecated sets the precedent that in the future node runners wont be able to configure this.

💬 vasild commented on pull request "rpc: add cpu_load to getpeerinfo":
(https://github.com/bitcoin/bitcoin/pull/31672#discussion_r2086877583)
Took some, thanks!

💬 maflcko commented on pull request "lint: Check for missing trailing newline":
(https://github.com/bitcoin/bitcoin/pull/32477#discussion_r2086882108)
Happy to add any, if you see one and a reason to add it and no reason against adding it.

👍 pinheadmz approved a pull request: "config: allow setting -proxy per network"
(https://github.com/bitcoin/bitcoin/pull/32425#pullrequestreview-2836872116)
ACK e98c51fcce9ae3f441a416cab32a5c85756c6c64

Trivial changes since last ACK. Built and tested again on mainnet, this time added i2p as well, connected to peers on every network we support. Tried various proxy settings.

The code change is small and mostly just contained in `init.cpp`. We used to call `SetProxy()` on every network type with the same proxy endpoint, this new code parses more fine-grained config options and sets separate proxies based on the options.

<details><summary>Show
...

💬 maflcko commented on pull request "lint: Check for missing trailing newline":
(https://github.com/bitcoin/bitcoin/pull/32477#discussion_r2086887015)
Not sure. This will make compile time and runtime slower. I don't see the point?

💬 maflcko commented on pull request "lint: Check for missing trailing newline":
(https://github.com/bitcoin/bitcoin/pull/32477#discussion_r2086887675)
thx, done

👋 fanquake's pull request is ready for review: "[29.x] Backports"
(https://github.com/bitcoin/bitcoin/pull/32292)

💬 instagibbs commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2086911846)
that's not the same behavior IIUC

```
diff --git a/test/functional/mempool_datacarrier.py b/test/functional/mempool_datacarrier.py
index 6347215b86..ce1d65a9ed 100755
--- a/test/functional/mempool_datacarrier.py
+++ b/test/functional/mempool_datacarrier.py
@@ -20,16 +20,17 @@ from random import randbytes
# The historical maximum, now used to test coverage
CUSTOM_DATACARRIER_ARG = 83

class DataCarrierTest(BitcoinTestFramework):
def set_test_params(self):
- self.num_
...

💬 edilmedeiros commented on pull request "signet: omit commitment for some trivial challenges":
(https://github.com/bitcoin/bitcoin/pull/29032#issuecomment-2876659639)
> > I was wondering whether OP_2...OP_16 should also be treated as trivial,
>
> It may be useful to have multiple public signets out there and avoid them accidentally reorging each other.

It would be useful indeed.

[BIP325](https://github.com/bitcoin/bips/blob/master/bip-0325.mediawiki) specifies a behavior that's impossible to detect in the general case, though:

> There is one other acceptable special case: if a block's challenge is e.g. `OP_TRUE` (`0x51`), where an empty solution w
...

💬 fanquake commented on pull request "[27.x] Backports":
(https://github.com/bitcoin/bitcoin/pull/32479#issuecomment-2876661608)
This is failing in the macOS CI. An issue with Boost Process:
```bash
configure:34786: checking whether Boost.Process can be used
configure:34817: g++ -std=gnu++11 -std=c++20 -o conftest -Wno-error=narrowing -isystem /usr/local/include -DBOOST_MULTI_INDEX_DISABLE_SERIALIZATION -pthread conftest.cpp >&5
conftest.cpp:64:9: error: no type named 'opstream' in namespace 'boost::process'
bp::opstream stdin_stream;
~~~~^
conftest.cpp:65:18: error: use of undeclared identifier 'stdou
...

💬 theStack commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2086920895)
are these prepended paths (here and also for the `gui`, `bench` and `test` commands above) still relevant? They seem to reflect a folder structure that doesn't exist anymore since https://github.com/bitcoin/bitcoin/pull/31161

💬 theStack commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2086894554)
```suggestion
//! strace -e trace=execve -s 10000 build/bin/bitcoin ...
```

💬 instagibbs commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2086933393)
No, these are different concepts entirely. @Sjors asked for a comment and thought it wouldn't hurt. Leaving as is or removing this if it adds to confusion. Thoughts?

💬 instagibbs commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2086933574)
style difference I guess? we assert the invariant we expect and I like subtests cleaning up their state in general for mempool stuff. Keeping as is.

💬 instagibbs commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2086933716)
taken

💬 instagibbs commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2086933886)
taken with minor modifications

💬 instagibbs commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2086936293)
combined commits, still leaning towards a long deprecation cycle to not mislead longer term

💬 instagibbs commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2086937998)
The options is ostensibly to stop people from data packing, not vbytes usage per se. The smaller the payload, the more overhead they are incuring.

💬 laanwj commented on pull request "net, pcp: handle multi-part responses and filter for default route while querying default gateway":
(https://github.com/bitcoin/bitcoin/pull/32159#issuecomment-2876721714)
> you might be interested in circling back here to review?

Yes, will look into it.

> This is not a regression from migration from libnatpmp as I first thought.

Removed the backport label as this was confirmed not to be a regression (thanks!).

🤔 darosior reviewed a pull request: "Introduce per-txin sighash midstate cache for legacy/p2sh/segwitv0 scripts"
(https://github.com/bitcoin/bitcoin/pull/32473#pullrequestreview-2836993312)
Neat. Concept ACK.

> I'm open to discussing alternatives, however.

I think enabling it by consensus is unnecessarily risky. I would rather take some slightly more complicate logic, for instance by having `CScriptCheck` own the sighash cache:
<details>

<summary>Click to see diff</summary>

```diff
diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp
index ffac2acb948..fe24dae6c54 100644
--- a/src/script/interpreter.cpp
+++ b/src/script/interpreter.cpp
@@ -1700,7 +
...

💬 instagibbs commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2876729479)
Thanks to everyone who gave thoughtful review, I hopefully addressed all code-related questions, if not let me know.