💬 ryanofsky commented on issue "Mining Interface doesn't allow for Bitcoin Core to create blocks when it wants":
(https://github.com/bitcoin/bitcoin/issues/31109#issuecomment-2591648579)
> After the notification fires, the miner calling back in to request a CreateNewBlock run will now have to wait behind `cs_main` for random P2P stuff

I don't see why this would be true. There's no reason Mining.createNewBlock or BlockTemplate.waitNext methods need to use cs_main. cs_main is an implementation detail and not part of the interface. For example we could have:

```c++
std::shared_ptr<CBlockTemplate> g_block_template;
std::mutex g_block_template_mutex;
std::condition_variable g_block
...

🤔 mzumsande reviewed a pull request: "test: p2p: fix sending of manual INVs in tx download test"
(https://github.com/bitcoin/bitcoin/pull/31658#pullrequestreview-2551617694)
Concept ACK

Good find! I think that `p2p_orphan_handling.py` has a similar [spot](https://github.com/bitcoin/bitcoin/blob/e7c479495509c068215b73f6df070af2d406ae15/test/functional/p2p_orphan_handling.py#L431).

💬 i-am-yuvi commented on pull request "test: p2p: fix sending of manual INVs in tx download test":
(https://github.com/bitcoin/bitcoin/pull/31658#issuecomment-2591741584)
It could be good followup! Can you also change that as well @theStack

👍 i-am-yuvi approved a pull request: "test: p2p: fix sending of manual INVs in tx download test"
(https://github.com/bitcoin/bitcoin/pull/31658#pullrequestreview-2551649291)
Tested ACK e0b333682222927d64217b07bb8cfd7ff3139660

✅ fanquake closed a pull request: "1"
(https://github.com/bitcoin/bitcoin/pull/31659)

📝 fanquake locked a pull request: "1"
(https://github.com/bitcoin/bitcoin/pull/31659)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...

💬 maflcko commented on pull request "test: p2p: fix sending of manual INVs in tx download test":
(https://github.com/bitcoin/bitcoin/pull/31658#issuecomment-2591895415)
> It might be good to avoid issues like this in the future, happy to add test framework improvements if someone has a concrete idea.

Would it not be possible to use mocktime and sync_with_ping?

💬 TheCharlatan commented on pull request "kernel: Move kernel-related cache constants to kernel cache":
(https://github.com/bitcoin/bitcoin/pull/31483#discussion_r1916121562)
Yeah, that should work. I'll fuzz with this for some time.

💬 vasild commented on pull request "ci: detect outbound internet traffic generated while running tests":
(https://github.com/bitcoin/bitcoin/pull/31349#issuecomment-2591928909)
`bbfc58a0af...69e076664d`: drop the tests changes from this PR because they were already merged via https://github.com/bitcoin/bitcoin/pull/31646. So here remains a CI change to detect future regressions.

If people want to run this manually, outside of the CI in a way that causes false positive (discussion above), what about something like this:

```diff
--- i/ci/test/03_test_script.sh
+++ w/ci/test/03_test_script.sh
@@ -184,13 +184,15 @@ function traffic_monitor_end()
# "permissio
...

💬 Sjors commented on issue "Mining Interface doesn't allow for Bitcoin Core to create blocks when it wants":
(https://github.com/bitcoin/bitcoin/issues/31109#issuecomment-2591934116)
> Having the Mining interface should only make it easier to implement support for other protocols because they can use it without needing to interact with validation code.

Which can be seen in action in RPC code that calls these methods.

>> An interface that pushes the new work itself, rather than a notification, would not have this issue.

> `waitNext` is providing an interface that pushes the new work itself. A reference to the new block is in the return value.

@TheBlueMatt when you opened
...

💬 Sjors commented on pull request "rpc: add gettarget , target getmininginfo field and show next block info":
(https://github.com/bitcoin/bitcoin/pull/31583#discussion_r1916158555)
In testnet3 and testnet4 the difficulty depends on the timestamp. After 20 minutes it drops to 1. These methods derive difficulty using the current timestamp.

💬 stratospher commented on pull request "validation: stricter internal handling of invalid blocks":
(https://github.com/bitcoin/bitcoin/pull/31405#discussion_r1914320847)
528b8cc: this would be more accurate for descendants.
```suggestion
} else if (pindexFirstInvalid != pindex) {
```

💬 stratospher commented on pull request "validation: stricter internal handling of invalid blocks":
(https://github.com/bitcoin/bitcoin/pull/31405#discussion_r1914366243)
7f88af8b: when `invalidateblock` is called multiple times, we would need to clear the `BLOCK_FAILED_VALID` from previous iteration so that both `BLOCK_FAILED_VALID` and `BLOCK_FAILED_CHILD` don't happen together on the same block. ex: [this situation](https://github.com/bitcoin/bitcoin/blob/35bf426e02210c1bbb04926f4ca2e0285fbfcd11/test/functional/rpc_invalidateblock.py#L91)
```
if (it->second->nStatus & BLOCK_FAILED_VALID) {
it->second->nStatus = (it->second->nStatus ^ BLOCK_FAILED_VALID)
...

💬 maflcko commented on pull request "rpc: add gettarget , target getmininginfo field and show next block info":
(https://github.com/bitcoin/bitcoin/pull/31583#discussion_r1916173716)
I'd say that test-only docs shouldn't be put in the real RPC docs for mainnet. If someone is running on t3/t4 they should be aware of the rules already. And if they are not aware, "if found now" doesn't help to explain the rules to them.

⚠️ fxcoudert opened an issue: "The 28.1 release is not tagged in github"
(https://github.com/bitcoin/bitcoin/issues/31660)
### Is there an existing issue for this?

- [x] I have searched the existing issues

### Current behaviour

In GitHub releases, 28.0 is still marked as latest. We are holding from updating in Homebrew until this is resolved: https://github.com/Homebrew/homebrew-core/pull/204066

### Expected behaviour

28.1 should be marked as latest release, to confirm it is authentic.

### Steps to reproduce

1. Go to https://github.com/bitcoin/bitcoin
2. Observe that latest release is 28.0

### Relevant log o
...

💬 Sjors commented on pull request "rpc: add gettarget , target getmininginfo field and show next block info":
(https://github.com/bitcoin/bitcoin/pull/31583#discussion_r1916184067)
Ok, I dropped "if found now".

💬 TheCharlatan commented on pull request "kernel: Move kernel-related cache constants to kernel cache":
(https://github.com/bitcoin/bitcoin/pull/31483#issuecomment-2592035361)
Updated 922aa7a25650bdfd8428198f171f88915af1ffa0 -> 430d5feee9b8be3a85e85696c449753783301c2b ([kernel_cache_sizes_15](https://github.com/TheCharlatan/bitcoin/tree/kernel_cache_sizes_15) -> [kernel_cache_sizes_16](https://github.com/TheCharlatan/bitcoin/tree/kernel_cache_sizes_16), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernel_cache_sizes_15..kernel_cache_sizes_16))

* Addressed @ryanofsky's [comment_1](https://github.com/bitcoin/bitcoin/pull/31483#discussion_r1915387311), [c
...

👍 laanwj approved a pull request: "Cleanups to port mapping module post UPnP drop"
(https://github.com/bitcoin/bitcoin/pull/31157#pullrequestreview-2551977080)
Code review ACK 70398ae05bc36a2788e87f67ae06962f43fe35a6

📝 hebasto opened a pull request: "depends: Always set `CMAKE_BUILD_TYPE=None` globally"
(https://github.com/bitcoin/bitcoin/pull/31661)
This PR fixes a regression for the `libevent` package introduced in https://github.com/bitcoin/bitcoin/pull/29835.

Instead of configuring `CMAKE_BUILD_TYPE=None` per package, set it globally to ensure consistent behaviour. This prevents packages with a default build type (usually "Release") from overriding our build type-specific flags, such as `-O2` or `-O1 -g`.

For packages that do not set a default build type, this change does not affect their behaviour.

💬 Sjors commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#issuecomment-2592154041)
The first commit a7d43645ea353021b2143407ab06336105c82434 changes `coinbaseMaxAdditionalWeight` in the Mining interface to `blockReservedWeight`. This decouples it from the Stratum v2 concept of `CoinbaseOutputDataSize` which is strictly limited to the outputs added:

> The maximum additional serialized bytes which the pool will add in coinbase transaction outputs

https://github.com/stratum-mining/sv2-spec/blob/main/07-Template-Distribution-Protocol.md#71-coinbaseoutputdatasize-client---se
...