⚠️ ryanofsky reopened an issue: "Assertion hit on shutdown of bitcoin-node with connected mining interface client"
(https://github.com/bitcoin/bitcoin/issues/33387)
### Is there an existing issue for this?

- [x] I have searched the existing issues

### Current behaviour

Running c0894a0a2be032cd9a5d5945643689230ab10255 `bitcoin-node` with Sjor's [sv2-tp] connected resulted in an unclean shutdown:
```
2025-09-14T10:24:34Z CreateNewBlock(): block weight: 460455 txs: 371 fees: 230840 sigops 1336
2025-09-14T10:25:05Z CreateNewBlock(): block weight: 513153 txs: 441 fees: 290027 sigops 1489
2025-09-14T10:25:36Z CreateNewBlock(): block weight: 635048 txs: 541 f
...

💬 ryanofsky commented on issue "Assertion hit on shutdown of bitcoin-node with connected mining interface client":
(https://github.com/bitcoin/bitcoin/issues/33387#issuecomment-3291934337)
Will reopen since I think this is probably worth following up on, even if it may not be an urgent priority.

If I'm understanding the problem correctly, it might not be to hard to reproduce with a functional test calling IPC methods during shutdown, though the test would be inherently racy. It might be easier to reproduce reliably with a unit test.

👍 ryanofsky approved a pull request: "mining: add applySolution() to interface"
(https://github.com/bitcoin/bitcoin/pull/33374#pullrequestreview-3224409033)
Concept ACK, this does seem like a more general solution than 33372.

💬 ryanofsky commented on pull request "mining: add applySolution() to interface":
(https://github.com/bitcoin/bitcoin/pull/33374#discussion_r2348861453)
> re: [#33374 (comment)](https://github.com/bitcoin/bitcoin/pull/33374#discussion_r2348144204)

Looking at the PR more, since this is just adding a new method and should be backwards compatible, probably nothing in my comment above applies here, other than that I think it's reasonable for applySolution to follow submitSolution in the .capnp file like it does in the .h file, and for the methods to not be renumbered or sorted by number.

💬 ryanofsky commented on pull request "mining: add applySolution() to interface":
(https://github.com/bitcoin/bitcoin/pull/33374#discussion_r2348937808)
> Another thing that's not great here is that `m_block_template->block` is mutated.

It does seem nicer to not to mutate the block template. And as long as entire block is being serialized and sent over the socket anyway, probably the cost of copying it is not significant.

But one thought is that maybe serializing whole block is not necessary, and this could just return CBlockHeader?

Another idea is that this could be changed to use output parameters, and the client could decide what inf
...

📝 purpleKarrot opened a pull request: "cmake: exclude secp256k1 from all"
(https://github.com/bitcoin/bitcoin/pull/33390)
Instead of setting the EXCLUDE_FROM_ALL target property, pass EXCLUDE_FROM_ALL to `add_subdirectory()`.

This has the following advanteges:

* It is shorter (obviously).
* Target properties are set only in the `CMakeLists.txt` file that defines the target.
* Install rules defined in the subdirectory are excluded as well. This is what we want, because secp256k1 is linked statically.

💬 stickies-v commented on pull request "coins: warn on oversized `-dbcache`":
(https://github.com/bitcoin/bitcoin/pull/33333#discussion_r2348947467)
> Maybe worth pulling the below snippet into a helper so it can be reused here.

Agreed, a carved out function that just calculates and clamps `db_cache_bytes` makes most sense, and I think we should get rid of `ShouldWarnOversizedDbCache`.

💬 stickies-v commented on pull request "coins: warn on oversized `-dbcache`":
(https://github.com/bitcoin/bitcoin/pull/33333#discussion_r2348956416)
> Are you anticipating an error here or is this just a way to signal that we don't really support systems with less than 1 GiB? I left this out

In my diff, I'm left shifting `total_ram` later so I wanted to ensure no UB happened. `1<<20` is 1 MiB so I don't think we need to handle that with anything more complex than an assert.

💬 hebasto commented on pull request "cmake: exclude secp256k1 from all":
(https://github.com/bitcoin/bitcoin/pull/33390#issuecomment-3292109747)
CI fails:
```
The following tests FAILED:
4 - secp256k1_noverify_tests (Not Run)
5 - secp256k1_tests (Not Run)
6 - secp256k1_exhaustive_tests (Not Run)
Errors while running CTest
```

💬 djkazic commented on issue "v30.0 Testing":
(https://github.com/bitcoin/bitcoin/issues/33368#issuecomment-3292126185)
Upgraded from v29.1 to v30.0rc1 on two nodes. Everything has been stable for the last 2 days.

👍 TheCharlatan approved a pull request: "Add a "tx output spender" index"
(https://github.com/bitcoin/bitcoin/pull/24539#pullrequestreview-3224343678)
ACK 845b54defebc1987b9d654ea7b611a4ddebe345a

I previously wrote software that had to add a bunch of extra logic to accurately react when a certain output was spent. This would have made my life considerably easier.

💬 TheCharlatan commented on pull request "Add a "tx output spender" index":
(https://github.com/bitcoin/bitcoin/pull/24539#discussion_r2348820869)
Nit: Mark this and `CreateKeyPrefix` as `static`.

💬 TheCharlatan commented on pull request "Add a "tx output spender" index":
(https://github.com/bitcoin/bitcoin/pull/24539#discussion_r2348904680)
Similar to the blockfilterindex, I would suggest a brief comment here on the schema. Something like (though this reads a bit clunky):
The database stores a siphash of a transaction out point in pairs with the flat file position on disk the transaction is stored in. It is key-only and supports retrieving a transaction by one of its input's out point.

💬 TheCharlatan commented on pull request "Add a "tx output spender" index":
(https://github.com/bitcoin/bitcoin/pull/24539#discussion_r2348828075)
Nit (format): Extra whitespace on this block.

💬 TheCharlatan commented on pull request "test: Add submitblock test in interface_ipc":
(https://github.com/bitcoin/bitcoin/pull/33380#issuecomment-3292195172)
Updated 557644ee9499583b6d00efda289fb65e8359e084 -> 0a26731c4cc182e887ce959cdd301227cdc752d7 ([interface_ipc_submitblock_0](https://github.com/TheCharlatan/bitcoin/tree/interface_ipc_submitblock_0) -> [interface_ipc_submitblock_1](https://github.com/TheCharlatan/bitcoin/tree/interface_ipc_submitblock_1), [compare](https://github.com/TheCharlatan/bitcoin/compare/interface_ipc_submitblock_0..interface_ipc_submitblock_1))

* Added @Sjors's [suggested patch](https://github.com/bitcoin/bitcoin/pull
...

💬 HowHsu commented on issue "bench: unrealistic ConnectBlock benchmarks":
(https://github.com/bitcoin/bitcoin/issues/33375#issuecomment-3292198687)
I can confirm that this issue is true, during my testing for my PR： [https://github.com/bitcoin/bitcoin/pull/32791](url) .
To test ConnectBlock() realistically, I think we can leverage `CVerifyDB::VerifyDB()`, it is called on node init. It mainly replays
the latest `-check_blocks` blocks in the best chain when you set `-check_level` to 4. Considering cache state, you have to manually
set it at the begining (flush it for no-cache case, pre-load entries for cache-hit case, .etc).Surely you have to
...

💬 pinheadmz commented on issue "v30.0 Testing":
(https://github.com/bitcoin/bitcoin/issues/33368#issuecomment-3292272617)
- Running 30.0rc1 on 32-bit ARM RPi with LND
- Running 30.0rc1 on x86 Ubuntu 20 `bitcoin-node` with Sv2 template provider and my own [idiotic block-art web stuff](https://thebitcoinblockclock.com/btc/)
- Checked codesigned GUI on macos/ARM, will go through the testing guide
- Will run IBD on Debian 12 VM as well with `-coinstatsindex=1`

👋 pinheadmz's pull request is ready for review: "Remove unnecessary casts when calling socket operations"
(https://github.com/bitcoin/bitcoin/pull/33378)

📝 ryanofsky opened a pull request: "test: Prevent disk space warning during node_init_tests"
(https://github.com/bitcoin/bitcoin/pull/33391)
mzumsande pointed out https://github.com/bitcoin/bitcoin/pull/32345#issuecomment-3286964369 that this test was print a warning:

```
Warning: Disk space for "/tmp/test_common bitcoin/node_init_tests/init_test/bf78678cb7723a3e84b5/blocks" may not accommodate the block files. Approximately 810 GB of data will be stored in this directory.
```

Fix by setting regtest instead of mainnet network before running the test.

💬 ryanofsky commented on pull request "ipc: Handle unclean shutdowns better":
(https://github.com/bitcoin/bitcoin/pull/32345#issuecomment-3292311207)
re: https://github.com/bitcoin/bitcoin/pull/32345#issuecomment-3286964369

> Maybe covered by the previous comments, but running `test_bitcoin` on master now prints out a warning, caused by `node_init_tests`:
>
> ```
> Running 671 test cases...
> Warning: Disk space for "/tmp/test_common bitcoin/node_init_tests/init_test/bf78678cb7723a3e84b5/blocks" may not accommodate the block files. Approximately 810 GB of data will be stored in this directory.
> ```

Thanks! Should be fixed in #333
...