🤔 sipa reviewed a pull request: "refactor: C++20 operators"
(https://github.com/bitcoin/bitcoin/pull/33771#pullrequestreview-3552981158)
ACK 48840bfc2d7beeac0ddf56a3c26b243156ec8936

🤔 sipa reviewed a pull request: "netbase: Remove "tor" as a network specification"
(https://github.com/bitcoin/bitcoin/pull/34031#pullrequestreview-3553002765)
ACK 7efb18c38150344ca6f7efbcd8441792a2ea921f

14 major releases ought to be a long enough deprecation period.

🚀 fanquake merged a pull request: "refactor: C++20 operators"
(https://github.com/bitcoin/bitcoin/pull/33771)

💬 fanquake commented on pull request "net: Waste less time in socket handling":
(https://github.com/bitcoin/bitcoin/pull/34025#issuecomment-3628023796)
cc @theuni @vasild

💬 fanquake commented on pull request "netbase: Remove "tor" as a network specification":
(https://github.com/bitcoin/bitcoin/pull/34031#issuecomment-3628025038)
cc @laanwj

💬 pablomartin4btc commented on pull request "argsman, cli: GNU-style command-line option parsing (allows options after non-option arguments)":
(https://github.com/bitcoin/bitcoin/pull/33540#issuecomment-3628033533)
-<ins>_**Updates**_</ins>:
- Added @w0xlt's [fix](https://github.com/bitcoin/bitcoin/pull/33540#pullrequestreview-3490620415) on long (double dash --) options that [worked](https://github.com/bitcoin/bitcoin/pull/33540#pullrequestreview-3490254189) on `master`, adding a new test for it (the test would pass in `master` but fail without @w0xlt's changes).
- Refactored both `ArgsManager::ProcessOptionKey` (new function added in this PR) and `ArgsManager::ParseParameters` in separated commits maki
...

💬 maflcko commented on pull request "Add util::Expected (std::expected)":
(https://github.com/bitcoin/bitcoin/pull/34006#discussion_r2599397406)
> I think hodlinator's solution adding requires clauses might avoid the need to specialize [#34006 (comment)](https://github.com/bitcoin/bitcoin/pull/34006#discussion_r2592454740)

No, I don't think this will generally be more correct. `std::expected<void, B>::value()` exists, so hiding it will make it differ from the std version.

I think the only way to replicate all behavior of `std::expected` 100%, would be to implement it exactly the same way. I'd prefer not to write the void-specializa
...

🤔 sipa reviewed a pull request: "miniscript refactor: Remove unique_ptr-indirection"
(https://github.com/bitcoin/bitcoin/pull/31713#pullrequestreview-3553094316)
reACK d782bffc5461f7361ae410963466ae1577ef63df

💬 maflcko commented on pull request "Add util::Expected (std::expected)":
(https://github.com/bitcoin/bitcoin/pull/34006#discussion_r2599407230)
It'll be used in the kernel, soon. So I'd rather avoid the churn. In any case, it will likely remain empty, and behavior should all be the same. So I think it doesn't matter and my preference would be to just keep this as-is for now.

💬 maflcko commented on pull request "Add util::Expected (std::expected)":
(https://github.com/bitcoin/bitcoin/pull/34006#discussion_r2599410546)
ok, i'll try to fixup the second commit tomorrow, or in a follow-up, whichever happens earlier.

🤔 hebasto reviewed a pull request: "guix: reduce allowed exported symbols"
(https://github.com/bitcoin/bitcoin/pull/33950#pullrequestreview-3553159236)
> FWIW i cherry-picked commit [7b90b4f](https://github.com/bitcoin/bitcoin/commit/7b90b4f5bb10e2156709b07e3996f867e2421232) on top of [b354d1c](https://github.com/bitcoin/bitcoin/commit/b354d1ce5c47997aa2f93910e05c0f8daa8736eb) (the commit before #33181). A full guix build finished succesfully. So it must have been something earlier even!

The `_fini` and `_init` symbols have no longer been exported since commit b5fc6d46a3854c18f6e8dfc89540d24ef778caa2 in https://github.com/bitcoin/bitcoin/pull/
...

💬 ryanofsky commented on issue "Memory leak when using IPC mining interface":
(https://github.com/bitcoin/bitcoin/issues/33940#issuecomment-3628241016)
re: https://github.com/bitcoin/bitcoin/issues/33940#issuecomment-3611006053

>* `git clone https://github.com/stratum-mining/sv2-apps -b v0.1.0`

Thanks for the instructions. I've been experimenting with the v0.1.0 pool_sv2 app on regtest with a python script to generate transactions & connect blocks quickly to make a memory leak happen without needing to wait a long time.

So far, behavior has been mostly as expected, where generating lots of transactions without connecting new blocks causes bl
...

💬 Sjors commented on issue "Memory leak when using IPC mining interface":
(https://github.com/bitcoin/bitcoin/issues/33940#issuecomment-3628281213)
> if the python client connected blocks too quickly, the mining client seemed to fall behind and only release templates after a delay

There's a hardcoded 10(?) second grace period where the Template Provider (both implementations) hold on to templates after a new tip is connected. It's there for two reasons:

1. Trying to relay the block anyway (requires further node changes, since we currently honour the first-seen rule even when that means "our" block loses)
2. Giving pools a chance to verify
...

💬 theuni commented on issue "Net split meta issue":
(https://github.com/bitcoin/bitcoin/issues/33958#issuecomment-3628282838)

> In particular, I think good goals to have here are (not in priority order):
>
> 1. simplify the codebase so it's easier to understand and update
>
> 2. improve robustness of the codebase, so that if there are bugs, they have less impact
>
> 3. make the code higher performance, so that we can have more peers, and/or run on cheaper hardware
>

Goal #1 is a logical separation of concerns. Today, the many of interactions between `CConnman` and `PeerManager` are subtle and (I'd gue
...

👍 ryanofsky approved a pull request: "doc: document capnproto and libmultiprocess deps in 29.x"
(https://github.com/bitcoin/bitcoin/pull/33623#pullrequestreview-3553439261)
Code review ACK 2cf352fd8e6a77003e38d954b6c879b20d4b960a. Thanks for making all these changes and for opening the fix originally.

👍 ryanofsky approved a pull request: "mining: getCoinbase() returns struct instead of raw tx"
(https://github.com/bitcoin/bitcoin/pull/33819#pullrequestreview-3553451082)
Code review ACK 1bd6cae51fedeff62755e03ddcb9346b868f6d83. Just rebased with no changes, to avoid trivial merge conflict.

💬 ryanofsky commented on pull request "test: interface_ipc.py minor fixes and cleanup":
(https://github.com/bitcoin/bitcoin/pull/34003#discussion_r2599729002)
re: https://github.com/bitcoin/bitcoin/pull/34003#discussion_r2598858386

> In [fac12f7](https://github.com/bitcoin/bitcoin/commit/fac12f7cf9ebc0b906bbb20ae126cf7e7552fdbb) _test: fix interface_ipc.py template destruction_: similar to my comment above, this only shows that `waitNext()` returns immediately if fees have gone up before the call, but the original intent was to show that a fee increase causes the wait to be interrupted.

Makes sense. Again there should be no change of behavior in
...

💬 ryanofsky commented on pull request "test: interface_ipc.py minor fixes and cleanup":
(https://github.com/bitcoin/bitcoin/pull/34003#discussion_r2599716362)
re: https://github.com/bitcoin/bitcoin/pull/34003#discussion_r2598844209

> I think the original intention here was to demonstrate that `generate` interrupts an ongoing `waitNext()`.
>
> Now it only shows that `waitNext()` returns immediately if the tip has changes before calling. Though perhaps it always did that by accident.

Yes behavior should be unchanged. Original code was:

```python
waitnext = template.result.waitNext(ctx, waitoptions)
self.generate(self.nodes[0], 1)
template
...

💬 Sjors commented on pull request "validation: Remove min_pow_checked arg in ProcessNewBlockHeaders":
(https://github.com/bitcoin/bitcoin/pull/34022#issuecomment-3628534056)
This was intentional, as explained in the commit message of ed6cddd98e32263fc116a4380af6d66da20da990 (part of #25717):

> This commit adds a new argument to `AcceptBlockHeader()` so that we can ensure that all call-sites which might cause a new header to be accepted into memory have to grapple with the question of whether the header is safe to accept, or needs further validation.

Not sure if we need to keep it forever.

cc @sdaftuar

💬 Sjors commented on pull request "validation: Remove min_pow_checked arg in ProcessNewBlockHeaders":
(https://github.com/bitcoin/bitcoin/pull/34022#discussion_r2599761636)
Maybe something loosely based on the original commit message:

> all call-sites which might cause a new header to be accepted into memory have to grapple with the question of whether the header is safe to accept, or needs further validation

🤔 mzumsande reviewed a pull request: "Broadcast own transactions only via short-lived Tor or I2P connections"
(https://github.com/bitcoin/bitcoin/pull/29415#pullrequestreview-3553664703)
re-ACK https://github.com/bitcoin/bitcoin/commit/f391296b17a755153960e0afc736df37d1d5fb1e