📝 MarcoFalke opened a pull request: "test: Fix TypeError (expected str instance, bytes found) in wait_for_debug_log"
(https://github.com/bitcoin/bitcoin/pull/27280)
null

💬 MarcoFalke commented on pull request "test: Fix TypeError (expected str instance, bytes found) in wait_for_debug_log":
(https://github.com/bitcoin/bitcoin/pull/27280#issuecomment-1476011366)
Can be tested with:

```diff
diff --git a/test/functional/feature_init.py b/test/functional/feature_init.py
index 70a802dc58..1d1c6a64b6 100755
--- a/test/functional/feature_init.py
+++ b/test/functional/feature_init.py
@@ -52,7 +52,7 @@ class InitStressTest(BitcoinTestFramework):
assert_equal(200, node.getblockcount())

lines_to_terminate_after = [
- b'Validating signatures for all blocks',
+ b'.Validating signatures for all blocks',

...

🤔 dergoegge requested changes to a pull request: "Log new headers"
(https://github.com/bitcoin/bitcoin/pull/27278)
Concept ACK

Prefer this over #27276 as well I think.

Maybe add the peer id to the log locations as well?

💬 dergoegge commented on pull request "Log new headers":
(https://github.com/bitcoin/bitcoin/pull/27278#discussion_r1141510303)
I think this `LogPrintf` call isn't safe because the work on the header has not actually been checked, that first happens in `ProcessNewBlockHeaders` below (by "safe" i mean that this location could be used to fill our disk with log messages).

At this point the header is only claiming to have enough work (i.e. `prev_block->nChainWork + CalculateHeadersWork({cmpctblock.header}) >= GetAntiDoSWorkThreshold()`) but the actual PoW check happens later on.

You can move this log location below the
...

💬 fanquake commented on pull request "Log successful AcceptBlockHeader()":
(https://github.com/bitcoin/bitcoin/pull/27276#issuecomment-1476018573)
Closing in favour of #27278.

✅ fanquake closed a pull request: "Log successful AcceptBlockHeader()"
(https://github.com/bitcoin/bitcoin/pull/27276)

👍 alexsio27444 approved a pull request: "Log new headers"
(https://github.com/bitcoin/bitcoin/pull/27278)

💬 josibake commented on pull request "RPC: Fix fund transaction crash when at 0-value, 0-fee":
(https://github.com/bitcoin/bitcoin/pull/27271#discussion_r1141953882)
grammar nit:

```suggestion
// This can only happen if the feerate is 0, the requested destinations are value of 0 (e.g OP_RETURN),
// and there are no pre-selected inputs. This will result in a 0-input transaction, which is consensus-invalid
```

👍 alexsio27444 approved a pull request: "refactor: Add BlockManager getters"
(https://github.com/bitcoin/bitcoin/pull/26900)

👍 alexsio27444 approved a pull request: "net, refactor: net_processing, add `ProcessCompactBlockTxns`"
(https://github.com/bitcoin/bitcoin/pull/26969)

⚠️ fanquake opened an issue: "test: `interface_bitcoin_cli.py --descriptors` failure under `--valgrind`"
(https://github.com/bitcoin/bitcoin/issues/27281)
Seen on a aarch64 Alpine box. Master @ 40e1c4d4024b8ad35f2511b2e10bf80c5531dfde. Binaries compiled with Clang 15.0.7. Valgrind `valgrind-3.21.0.GIT`.

```bash
79/262 - interface_bitcoin_cli.py --descriptors failed, Duration: 34 s

stdout:
2023-03-20T10:34:23.174000Z TestFramework (INFO): PRNG seed is: 6631947331891546746
2023-03-20T10:34:23.175000Z TestFramework (INFO): Initializing test directory /tmp/test_runner_₿_🏃_20230320_101936/interface_bitcoin_cli_187
2023-03-20T10:34:29.054000Z
...

💬 ajtowns commented on pull request "Log new headers":
(https://github.com/bitcoin/bitcoin/pull/27278#discussion_r1141958384)
Could just make this:

```c++
const auto level{(IsIBD() ? BCLog::Level::Info : BCLog::Level::Warning)};
LogPrintf(BCLog::VALIDATION, level, "saw new ...", hash.ToString());
```

(or `None` rather than `Warning` maybe)

💬 fanquake commented on issue "feature_config_args.py failure under `--valgrind`":
(https://github.com/bitcoin/bitcoin/issues/27259#issuecomment-1476035165)
Ok this is timeout related. Going to close for now.

✅ fanquake closed an issue: "feature_config_args.py failure under `--valgrind`"
(https://github.com/bitcoin/bitcoin/issues/27259)

👍 josibake approved a pull request: "RPC: Fix fund transaction crash when at 0-value, 0-fee"
(https://github.com/bitcoin/bitcoin/pull/27271)
ACK https://github.com/bitcoin/bitcoin/pull/27271/commits/d7cc503843769b789dc5f031b4ddc75d555ba980

Left a small grammar nit for the comment. Also, it would be really nice to have the `src/wallet/spend.cpp` change in one commit and the functional test change in a separate commit. Makes it much easier to verify that the functional test actually catches the bug, although not really a big deal here since the change is so small.

To test, I deleted your change to `src/wallet/spend.cpp` and verif
...

💬 josibake commented on pull request "RPC: Fix fund transaction crash when at 0-value, 0-fee":
(https://github.com/bitcoin/bitcoin/pull/27271#discussion_r1141955223)
grammar nit:

```suggestion
// This can only happen if the feerate is 0, the requested destinations are value of 0 (e.g OP_RETURN),
// and there are no pre-selected inputs. This will result in a 0-input transaction, which is consensus-invalid

⚠️ fanquake opened an issue: "test: `wallet_importdescriptors.py --descriptors` failure under `--valgrind` "
(https://github.com/bitcoin/bitcoin/issues/27282)
Seen on a aarch64 Alpine box. Master @ https://github.com/bitcoin/bitcoin/commit/40e1c4d4024b8ad35f2511b2e10bf80c5531dfde. Binaries compiled with Clang 15.0.7. Valgrind `valgrind-3.21.0.GIT`.

We saw some issues with this test recently (#27229), but this looks like a different issue:

```bash
261/262 - wallet_importdescriptors.py --descriptors failed, Duration: 411 s

stdout:
2023-03-20T10:39:03.422000Z TestFramework (INFO): PRNG seed is: 4441145092714460381
2023-03-20T10:39:03.423000Z
...

💬 TheCharlatan commented on pull request "refactor, kernel: Decouple ArgsManager from blockstorage":
(https://github.com/bitcoin/bitcoin/pull/27125#discussion_r1141966979)
> Hmm, that sounds like you are making it harder to remove cs_main.

I don't think this change complicates a refactor of the global locks. `zmqpublishnotifier` relies on `cs_main` being in global scope before and after this change. If the lock is moved into some narrower scope, this would have to be handled for the `zmqpublishnotifier` too - with our without this patch. If I read #27006 correctly, no change is required to make it work with the patch here.

> my brain enjoys the thought of ha
...

💬 willcl-ark commented on pull request "tracing: Only prepare tracepoint arguments when actually tracing":
(https://github.com/bitcoin/bitcoin/pull/26593#discussion_r1141968668)
Why do we need this special-cased 0 arg tracepoint? ISTM that the systemtap variadic supports 0 arg calls:

https://github.com/jav/systemtap/blob/2da355dd02a18bf4f67e2ceeb504b351b4bd5b83/includes/sys/sdt.h#L291-L297

💬 Sjors commented on pull request "Log new headers":
(https://github.com/bitcoin/bitcoin/pull/27278#issuecomment-1476049065)
Retracting ACK because I missed that `CalculateHeadersWork` doesn't _check_ the work, see https://github.com/bitcoin/bitcoin/pull/27278#discussion_r1141510303