💬 ajtowns commented on pull request "RFC: blocks: add `-reobfuscate-blocks` arg to xor existing blk/rev on startup":
(https://github.com/bitcoin/bitcoin/pull/33324#issuecomment-3290838371)
tidy wants emplace_back over push_back

💬 Sjors commented on pull request "mining: add applySolution() to interface":
(https://github.com/bitcoin/bitcoin/pull/33374#discussion_r2348144204)
We could order them alphabetically, thematically or based on when they were introduced. I think the first two are better, but no strong preference.

@ryanofsky what do you think is better?

💬 Sjors commented on pull request "mining: add applySolution() to interface":
(https://github.com/bitcoin/bitcoin/pull/33374#discussion_r2348155638)
That could be a problem indeed.

Another thing that's not great here is that `m_block_template->block` is mutated. Will look into it. My assumption is that in general a client will call `submitSolution` first, since it's time sensitive, and then this method. So it should be fine if it copies to block rather than mutates the original even if that's marginally slower.

💬 Sjors commented on pull request "mining: add applySolution() to interface":
(https://github.com/bitcoin/bitcoin/pull/33374#discussion_r2348160819)
> is being run against mainnet params for ~100 blocks

It uses mainnet in order to have difficulty adjustment, but we should probably _also_ run against regtest for segwit coverage.

💬 Sjors commented on pull request "wallet: warn against accidental unsafe older() import":
(https://github.com/bitcoin/bitcoin/pull/33135#discussion_r2348170725)
Yes, it's only printed when you set the log level to debug and it's useful for debugging if the test breaks.

💬 Sjors commented on pull request "wallet: warn against accidental unsafe older() import":
(https://github.com/bitcoin/bitcoin/pull/33135#discussion_r2348172173)
There's some sort of linter command that can check this, but I forgot which.

💬 pythcoiner commented on pull request "wallet: warn against accidental unsafe older() import":
(https://github.com/bitcoin/bitcoin/pull/33135#issuecomment-3290932833)
tACK 5e9737ff49 code review + test importing few "Liana" descriptor with timelocks > 65535

💬 Sjors commented on pull request "test: Add submitblock test in interface_ipc":
(https://github.com/bitcoin/bitcoin/pull/33380#issuecomment-3291011638)
ACK 557644ee9499583b6d00efda289fb65e8359e084

Could expand it a bit:

```diff
diff --git a/test/functional/interface_ipc.py b/test/functional/interface_ipc.py
index 1651b53bda..89c1fd7d31 100755
--- a/test/functional/interface_ipc.py
+++ b/test/functional/interface_ipc.py
@@ -186,20 +186,33 @@ class IPCInterfaceTest(BitcoinTestFramework):
balance = miniwallet.get_balance()
coinbase.vout[0].scriptPubKey = miniwallet.get_output_script()
coinbase.v
...

✅ fanquake closed an issue: "cmake: Errors not actually errors?"
(https://github.com/bitcoin/bitcoin/issues/33153)

🚀 fanquake merged a pull request: "cmake: Fix regression in `secp256k1.cmake`"
(https://github.com/bitcoin/bitcoin/pull/33379)

💬 fanquake commented on pull request "cmake: Fix regression in `secp256k1.cmake`":
(https://github.com/bitcoin/bitcoin/pull/33379#issuecomment-3291100133)
Backported to 30.x in #33356.

💬 Eunovo commented on pull request "validation: ensure assumevalid is always used during reindex":
(https://github.com/bitcoin/bitcoin/pull/31615#discussion_r2348327967)
I think it still makes sense to account for this scenario since minimumchainwork and assumevalid block are set separately. I'll update the comment to explain that minimumchainwork only needs to be skipped if you set an older assumevalid block without also reducing the minimumchainwork.

📝 vasild opened a pull request: "test: don't throw from the destructor of DebugLogHelper"
(https://github.com/bitcoin/bitcoin/pull/33388)
Throwing an exception from the destructor of a class is a bad practice because the destructor will be called when an object of that type is alive on the stack and another exception is thrown, which will result in "exception during the exception". This would terminate the program without any messages.

Instead print the message to the standard error output and call `std::abort()`.

---

This change is part of https://github.com/bitcoin/bitcoin/pull/26812. It is an improvement on its own, so
...

💬 vasild commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2348333882)
Done in https://github.com/bitcoin/bitcoin/pull/33388

💬 ajtowns commented on pull request "mining: add applySolution() to interface":
(https://github.com/bitcoin/bitcoin/pull/33374#discussion_r2348345510)
An OP_TRUE signet would presumably get both for whatever that's worth

💬 fanquake commented on issue "Assertion hit on shutdown of bitcoin-node with connected mining interface client":
(https://github.com/bitcoin/bitcoin/issues/33387#issuecomment-3291201455)
@Sjors

💬 fanquake commented on pull request "test: Fix CLI_MAX_ARG_SIZE issues":
(https://github.com/bitcoin/bitcoin/pull/33243#issuecomment-3291221804)
Backported to 30.x in #33356.

👍 0xB10C approved a pull request: "depends: systemtap 5.3"
(https://github.com/bitcoin/bitcoin/pull/33373#pullrequestreview-3223791227)
ACK 28efd724b47841a16d0630cec4b59563cda54a81

I agree that the package sha256 hash is `966a360fb73a4b65a8d0b51b389577b3c4f92a327e84aae58682103e8c65a69a`. The diff is minimal and looks fine. The guix build hashes match.

```bash
$ find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
860c39a3db7eb7e8aaf8165cd6834d2fd87afc69b82b20203a013c7ad7e7b057 guix-build-28efd724b478/output/aarch64-linux-gnu/SHA256SUMS.part
841aea1c04fd996bc437a
...

⚠️ ismaelsadeeq opened an issue: "RFC: Bitcoin Core Node `BlockTemplateManager`"
(https://github.com/bitcoin/bitcoin/issues/33389)
Bitcoin Core Node `BlockTemplateManager` main use should be handling block template creation for other components of the node.
It should use the low-level block assembler to do that. This `BlockTemplateManager` should be initialized with a pointer to the mempool and a `ChainstateManager` reference. It should have access to the default block creation options and be instantiated during node startup and destroyed when the node is shutting down.
This `BlockTemplateManager` should be available via t
...

🤔 Eunovo reviewed a pull request: "log: always print initial signature verification state"
(https://github.com/bitcoin/bitcoin/pull/33336#pullrequestreview-3223525430)
Concept ACK https://github.com/bitcoin/bitcoin/pull/33336/commits/63dc937eea297332ec85c1abbcf3b94b8f74aa85
Left some comments.