💬 Sjors commented on pull request "Add destroy to BlockTemplate schema":
(https://github.com/bitcoin/bitcoin/pull/31288#discussion_r1843367056)
I moved `destroy` to the top and renumbered.

💬 Sjors commented on pull request "rpc: add optional blockhash to waitfornewblock":
(https://github.com/bitcoin/bitcoin/pull/30635#issuecomment-2478215295)
@ryanofsky & @luke-jr thanks for reviewing, will address feedback soon(tm).

💬 Sjors commented on pull request "BlockAssembler: return selected packages virtual size and fee":
(https://github.com/bitcoin/bitcoin/pull/30391#issuecomment-2478222260)
@ismaelsadeeq are you referring to my statement at the end "so there's clients to break"?

Once we release the Mining interface, and multiprocess, people can build clients. If we then make changes in the next release, we could break those clients. But that's not an issue yet.

⚠️ vasild opened an issue: "Discover() will not run if listening on any address with an explicit bind=0.0.0.0"
(https://github.com/bitcoin/bitcoin/issues/31293)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

https://github.com/bitcoin/bitcoin/blob/85bcfeea23568053ea09013fb8263fa1511d7123/src/init.cpp#L1890-L1892

`Discover()` will run only if we are listening on all addresses (`bind_on_any` is `true`). However if `-bind=0.0.0.0:port` is explicitly given, then `bind_on_any` will end up being `false` and thus `Discover()` will not run when it should.

### Expected behaviour

Discover own addre
...

💬 vasild commented on issue "net: Tor service target port collides when running multiple nodes, making bitcoind error out":
(https://github.com/bitcoin/bitcoin/issues/31133#issuecomment-2478267049)
Opened https://github.com/bitcoin/bitcoin/issues/31293 just that we don't forget about it.

🤔 vasild reviewed a pull request: "net, init: derive default onion port if a user specified a -port"
(https://github.com/bitcoin/bitcoin/pull/31223#pullrequestreview-2438045624)
I reviewed the code and it looks safe and will achieve the intended purpose.

Since I introduced the current behavior which this PR is aiming to fix, I understand that I may have a bias towards downplaying the current issue (port collision) and a biased preference towards "It's fine, don't do anything". However, even with this realization, I can't help but think whether this will [introduce a bigger problem than the one it solves](https://github.com/bitcoin/bitcoin/pull/31223#issuecomment-2470
...

💬 vasild commented on pull request "net, init: derive default onion port if a user specified a -port":
(https://github.com/bitcoin/bitcoin/pull/31223#discussion_r1843426357)
Maybe further elaborate this with an example:

If you are using `-port=` with non-standard value, for example `-port=5555` and not using `-bind=...=onion`, previously Bitcoin Core would listen for incoming Tor connections on `127.0.0.1:8334`. Now it would listen on `127.0.0.1:5556` (`-port` plus one). If you configured the hidden service manually in `torrc` now you have to change it from `HiddenServicePort 8333 127.0.0.1:8334` to `HiddenServicePort 8333 127.0.0.1:5556`, or configure `bitcoind`
...

👍 TheCharlatan approved a pull request: "Add destroy to BlockTemplate schema"
(https://github.com/bitcoin/bitcoin/pull/31288#pullrequestreview-2438142576)
ACK b28972cd85e4472b386349d6cda8c233faeffd4f

👍 rkrux approved a pull request: "test: group executed tests within the same directory"
(https://github.com/bitcoin/bitcoin/pull/31291#pullrequestreview-2438169141)
Concept ACK b7f40e8094e00c13a6c847340645b16704fed63b

💬 rkrux commented on pull request "test: group executed tests within the same directory":
(https://github.com/bitcoin/bitcoin/pull/31291#discussion_r1843512387)
> Replicating the functional test framework behavior.

Should we use a prettier format here just like done in the functional tests? With this change the directory name is in plain nanoseconds like below.

```
drwx------ 3 rkrux staff 96B Nov 15 11:09 1731649167593349000
drwx------ 3 rkrux staff 96B Nov 15 11:11 1731649280506089000
```

Whereas for functional tests, the name is a little easier to read by using a date time format: https://github.com/bitcoin/bitcoin/blob/mast
...

💬 maflcko commented on pull request "test: group executed tests within the same directory":
(https://github.com/bitcoin/bitcoin/pull/31291#issuecomment-2478481544)
I think given that different processes (the normal case with `ctest -j$(nproc)`) still create different time dirs, I think it is also fine to leave this as-is. To me this is purely a style issue, so I don't mind either way.

💬 rkrux commented on pull request "test: group executed tests within the same directory":
(https://github.com/bitcoin/bitcoin/pull/31291#discussion_r1843545615)
> Think of each unit test as a separate child class inheriting from the testing setup class and implementing a run() method. Each one constructs a new instance of the setup class, thereby creating a new rand_str.

One thing that I noticed is that there are several such (`1731649167593349000`) time directories when the unit tests are run, each having around one test_name directory inside. Whereas for the functional tests, a dir such as `test_runner_?_🏃_20241115_150614` encapsulates all of them
...

💬 Sjors commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#discussion_r1843589729)
A `fee_threshold` of 0 combined with a `timeout` of 0 will immediately return a new template. This is useful for the Template Provider, because it needs to unconditionally send new templates to all connected clients if fees increased sufficiently for one its connected clients. (at least until cluster mempool makes the fee calculation trivially cheap)

📝 marcofleon converted_to_draft a pull request: "fuzz: Fix difficulty target generation in `p2p_headers_presync`"
(https://github.com/bitcoin/bitcoin/pull/31213)
In the `p2p_headers_presync` fuzz target, this assertion failed:
```
assert(total_work < chainman.MinimumChainWork());
```
Input that triggered the failure: [p2ppresync_crash.txt](https://github.com/user-attachments/files/17620203/p2ppresync_crash.txt)

The test previously used `ConsumeIntegralInRange` to generate header difficulty targets within a hardcoded range. The fuzzer found specific values in that range that correspond to very low thresholds due to how [`SetCompact`][setcompact-l
...

💬 Yygik commented on pull request "validation: fix m_best_header tracking and BLOCK_FAILED_CHILD assignment":
(https://github.com/bitcoin/bitcoin/pull/30666#discussion_r1843669771)
各方面看过嗯结果OK

💬 Yygik commented on pull request "validation: fix m_best_header tracking and BLOCK_FAILED_CHILD assignment":
(https://github.com/bitcoin/bitcoin/pull/30666#issuecomment-2478676296)
i额迷你发光你放哪更好

⚠️ Yygik opened an issue: "i额迷你发光你放哪更好"
(https://github.com/bitcoin/bitcoin/issues/31294)
i额迷你发光你放哪更好

_Originally posted by @Yygik in https://github.com/bitcoin/bitcoin/issues/30666#issuecomment-2478676296_

✅ Yygik closed an issue: "i额迷你发光你放哪更好"
(https://github.com/bitcoin/bitcoin/issues/31294)

:lock: fanquake locked an issue: "i额迷你发光你放哪更好"
(https://github.com/bitcoin/bitcoin/issues/31294)

💬 sipa commented on issue "Discover() will not run if listening on any address with an explicit bind=0.0.0.0":
(https://github.com/bitcoin/bitcoin/issues/31293#issuecomment-2478721606)
> Operating system and version
>
> Windows 3.11

Doubt.