💬 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.

💬 Eunovo commented on pull request "log: always print initial signature verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2348447122)
https://github.com/bitcoin/bitcoin/pull/33336/commits/3ddd4c84af2bf0b5d3e403b71f4750b2326118e1: I think it's best to take out this TODO from the commit. You should also adjust the commit message to focus on what this commit is adding specifically.

💬 Eunovo commented on pull request "log: always print initial signature verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2348463061)
https://github.com/bitcoin/bitcoin/pull/33336/commits/3ddd4c84af2bf0b5d3e403b71f4750b2326118e1: A comment here will also be beneficial.

💬 Eunovo commented on pull request "log: always print initial signature verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2348538334)
https://github.com/bitcoin/bitcoin/pull/33336/commits/63dc937eea297332ec85c1abbcf3b94b8f74aa85:
should be "block not on best header chain"

💬 Eunovo commented on pull request "log: always print initial signature verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2348503453)
https://github.com/bitcoin/bitcoin/pull/33336/commits/6222d8cced4828b83a9dde20b9da9df44848b9bd:
Why not `AssumeValid::DISABLED` instead of `AssumeValid::CHECKED` and `AssumeValid::ENABLED` instead of `AssumeValid:SKIPPED`?
I find the current names to be confusing. I think using `AssumeValid::ENABLED` to indicate that assume_valid is enabled is better.

💬 Eunovo commented on pull request "log: always print initial signature verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2348449770)
https://github.com/bitcoin/bitcoin/pull/33336/commits/3ddd4c84af2bf0b5d3e403b71f4750b2326118e1: This should probably still be set to `3` since the 4th node is not used in this commit.