💬 danielabrozzoni commented on pull request "rpc: Ensure -debug=0/none behaves consistently with -nodebug":
(https://github.com/bitcoin/bitcoin/pull/31767#discussion_r1941344025)
You're right, thanks! Updated with "Logging" as it feels more appropriate to me too

💬 darosior commented on pull request "Double check all block rules in `ConnectBlock`, not only `CheckBlock`":
(https://github.com/bitcoin/bitcoin/pull/31792#discussion_r1941355885)
Done.

💬 Sjors commented on pull request "ci: build multiprocess on most jobs":
(https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2634256382)
Let's see if only ASan remains... https://github.com/bitcoin/bitcoin/actions/runs/13138410284/job/36659308174?pr=30975

💬 maflcko commented on pull request "test: check `scanning` field from `getwalletinfo`":
(https://github.com/bitcoin/bitcoin/pull/31768#issuecomment-2634283850)
lgtm ACK bb0879ddabc8b3a7253bc774d23b842937d18015

💬 maflcko commented on pull request "ci: run in worktrees":
(https://github.com/bitcoin/bitcoin/pull/31787#discussion_r1941383838)
On a second though, I guess if someone wanted to run the CI on an extracted tarball of the repo (git-archive), it would be better to also support that. Not sure if it is as trivial as `git init && git add ./`, so no strong opinion.

💬 darosior commented on pull request "Double check all block rules in `ConnectBlock`, not only `CheckBlock`":
(https://github.com/bitcoin/bitcoin/pull/31792#discussion_r1941384261)
To keep the existing level for this error. I suppose the existing level is debug due to log-filling concerns, maybe with very-low-difficulty headers?

Thanks for pointing this out though, i double checked the other log line in `TestBlockValidity` and there it should have been `LogError`. Fixed.

💬 ryanofsky commented on pull request "ci: build multiprocess on most jobs":
(https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2634296691)
re: https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2634256382

> Let's see if only ASan remains... https://github.com/bitcoin/bitcoin/actions/runs/13138410284/job/36659308174?pr=30975

Following might fix the asan error:

```diff
--- a/src/ipc/CMakeLists.txt
+++ b/src/ipc/CMakeLists.txt
@@ -9,6 +9,7 @@ if (NOT WITH_LIBMULTIPROCESS)
# Apply core_interface compile options to libmultiprocess runtime library.
target_link_libraries(multiprocess PUBLIC $<BUILD_INTERFACE:co
...

💬 ajtowns commented on pull request "Double check all block rules in `ConnectBlock`, not only `CheckBlock`":
(https://github.com/bitcoin/bitcoin/pull/31792#issuecomment-2634312015)
I think what you're doing here is:

* redoing the contextual checks in ConnectBlock
* currently, this could result in errors on startup if we accepted a block with timestamp T, but then our clock went backwards and is now set to a time prior to T-2 hours
* so to avoid that, we move the T+2h check out of the contextual checks into its own special function, and call that from the appropriate places
* once we redo contextual checks in ConnectBlock, `verifychain` rpc can detect contexual er
...

💬 darosior commented on pull request "Double check all block rules in `ConnectBlock`, not only `CheckBlock`":
(https://github.com/bitcoin/bitcoin/pull/31792#discussion_r1941394003)
No, it's to make this commit pass the test on its own. This is because this sub-test restart the node with a different deployment height for Segwit, and its caller then calls `verifychain`. This would fail if we don't restart the node with the default deployment height because `ConnectBlock` would detect unexpected witnesses before activation height. This is essentially what the next commit checks explicitly.

💬 Sjors commented on pull request "ci: build multiprocess on most jobs":
(https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2634356140)
I think that worked, and then found another issue:

```
/home/runner/work/_temp/src/ipc/libmultiprocess/src/mp/gen.cpp:633:26: error: comparison of integers of different signs: 'size_t' (aka 'unsigned long') and 'int' [-Werror,-Wsign-compare]
633 | for (size_t i = 4; i < argc; ++i) {
| ~ ^ ~~~~
1 error generated.
```

💬 theStack commented on pull request "crypto: Use secure_allocator for `AES256_ctx`":
(https://github.com/bitcoin/bitcoin/pull/31774#issuecomment-2634378908)
Concept ACK on clearing out the ctx/iv members

I'm wondering if a minimum-diff fix which simply replaces the `memset` calls in the dtors with `memory_cleanse` in the destructors would be largely sufficient here? In https://github.com/bitcoin/bitcoin/pull/31166#issuecomment-2446055905 one argument for not needing secure allocators was the short-lived nature of the secrets. Looking at the only usage in the wallet, this would imho apply here too (en/decrypting might take a while for larger input
...

📝 maflcko opened a pull request: "ci: Use clang-20 for sanitizer tasks"
(https://github.com/bitcoin/bitcoin/pull/31793)
A new clang version generally comes with bugfixes, new sanitizer features, deprecations, as well as new features.

Upgrade the sanitizer tasks to use the new version.

This was also suggested in https://github.com/bitcoin/bitcoin/pull/31691#issuecomment-2602517116

📝 furszy opened a pull request: "wallet: abandon orphan coinbase txs, and their descendants, during startup"
(https://github.com/bitcoin/bitcoin/pull/31794)
Since #26499, we mark coinbase transactions and their descendants as abandoned when a reorg arises through the "block disconnection" signal handler. However, this does not cover all scenarios; external wallets could contain coinbase transactions from blocks the node has not seen yet, or the user could have replaced the chain with an earlier or different version (one without the coinbase chain).

This affects balance calculation as well as descendants mempool rebroadcast (which shouldn't be rel
...

💬 sipa commented on pull request "crypto: Use secure_allocator for `AES256_ctx`":
(https://github.com/bitcoin/bitcoin/pull/31774#issuecomment-2634496045)
I think the approach here might have an unacceptable performance impact, as it is allocating and deallocating a secure area for every individual key being encrypted/decrypted.

It may be better to:
* Somehow make the `AES256CBC` classes members of `CCrypter`, surviving an individual encryption/decryption (e.g. by adding a reset function that can get called for each encryption/decryption, resetting the CBC state, but letting the key material survive).
* Follow @theStack's suggestion of not lo
...

🤔 mzumsande reviewed a pull request: "Double check all block rules in `ConnectBlock`, not only `CheckBlock`"
(https://github.com/bitcoin/bitcoin/pull/31792#pullrequestreview-2593464262)
> This is fine to do as the expensive check in ContextualCheckBlock() is cached since
https://github.com/bitcoin/bitcoin/commit/1ec6bbeb8d27d31647d1433ccb87b362f6d81f90

As discussed out of band, only the most recent block will be cached in memory and passed to `ActivateBestChain` / `ConnectTip`. During IBD when blocks arrive out of order ABC will frequently connect multiple blocks, which then need to be reloaded from disk during `ConnectTip()` . For these, the `CheckWitnessMalleation` check
...

👍 brunoerg approved a pull request: "test: added additional coverage to waitforblock and waitforblockheight rpc's"
(https://github.com/bitcoin/bitcoin/pull/31784#pullrequestreview-2593526316)
code review ACK 7e0db87d4fff996c086f6e86b62338c98ef30c55

⚠️ embetrix opened an issue: "bitcoind debug log file is growing without limit"
(https://github.com/bitcoin/bitcoin/issues/31795)
### Is there an existing issue for this?

- [x] I have searched the existing issues

### Current behaviour

when setting bitcoind log file in `bitcoin.conf`:

`debuglogfile=/var/run/bitcoin/bitcoind.log`

the file is just growing without a limit

Workaround:

`debuglogfile=/dev/null`


### Expected behaviour

the file should rollback to 0 if a specified limit or predefined limit size is reached i.e :

`debuglogfile_size=16 ` (in MB)

### Steps to reproduce

NA

### Relevant log output

_No res
...

⚠️ embetrix opened an issue: "bitcoind loglevel=info too noisy"
(https://github.com/bitcoin/bitcoin/issues/31796)
### Is there an existing issue for this?

- [x] I have searched the existing issues

### Current behaviour

when setting bitcoind log level in `bitcoin.conf`:

`loglevel=info`

the journactl log are too noisy:


this of course make the journal full and remove all other applications logs after a while.

### Expected behaviour

`loglevel=info` should avoid printing out `UpdateTip`


### Steps to reproduce

NA

### Relevant log output

```
Feb 04 17:23:07 raspberrypi5-c8-bf-64 bitcoind[1470]: 2025-
...

💬 vasild commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#discussion_r1941605053)
Or this, idea from [discussion from another thread](https://github.com/bitcoin/bitcoin/pull/31283#discussion_r1937078288) - return `nullptr` if `TipBlock()` returns no block:

<details>
<summary>[patch] return nullptr from waitNext() if no tip</summary>

```diff
diff --git i/src/node/interfaces.cpp w/src/node/interfaces.cpp
index 1eb66d81c0..9af5e65308 100644
--- i/src/node/interfaces.cpp
+++ w/src/node/interfaces.cpp
@@ -947,35 +947,37 @@ public:
// The latter check is expen
...

💬 pinheadmz commented on issue "bitcoind loglevel=info too noisy":
(https://github.com/bitcoin/bitcoin/issues/31796#issuecomment-2634687921)
> loglevel=info should avoid printing out UpdateTip
> height=464313

You are in initial block download. Once the chain is synced these messages will be reduced to one every ten minutes or so ;-)