Bitcoin Core Github
44 subscribers
121K links
Download Telegram
πŸ€” naiyoma reviewed a pull request: "test: add logging to mock external signers"
(https://github.com/bitcoin/bitcoin/pull/32928#pullrequestreview-3035531241)
Concept ACK ->
I attempted to debug -> https://github.com/bitcoin/bitcoin/issues/32855#issuecomment-3090671325 after pulling this branch, and so far I can see some useful logs in `combined.log`

<details>
<summary>mock_external_signer” logs from this <a href="https://github.com/bitcoin/bitcoin/issues/32855#issuecomment-3090671325">diff</a></summary>

grep "mock_external_signer" combined.log
node1 2025-07-19T19:35:11.455260Z [init] [common/args.cpp:850] [logArgsPrefix] Command-line arg:
...
πŸ’¬ naiyoma commented on issue "intermittent timeout in wallet_signer.py : 'createwallet' RPC took longer than 1200.000000 seconds":
(https://github.com/bitcoin/bitcoin/issues/32855#issuecomment-3092562076)
>As stated in that comment, one possible reason is that the close of `err_wr_pipe`, which should send the empty string to stdin for the python process (signer.py) to confirm, could return `-1` but bitcoind didn't check the return result. `subprocess_close(err_wr_pipe);// close child side of pipe, else get stuck in read below`
>
I am not sure about this because on my end this subprocess works okay
```
subprocess_close(err_wr_pipe);// close child side of pipe, else get stuck in read below
fprint
...
βœ… bigshiny90 closed a pull request: "test: Add functional tests for blockreconstructionextratxn"
(https://github.com/bitcoin/bitcoin/pull/33016)
⚠️ totdking opened an issue: "A missing import to the src/chainparamsbase.h"
(https://github.com/bitcoin/bitcoin/issues/33019)
Tried running the make -j $CORES as the bitcoin repo is important to what I'm working with.

The import that is missing is the ```#include <cstdint>```, it gave a plethora of errors.

This is not a bug but i just wanted the community to have a peaceful and seamless integration while building the repo
πŸ’¬ naiyoma commented on issue "intermittent timeout in wallet_signer.py : 'createwallet' RPC took longer than 1200.000000 seconds":
(https://github.com/bitcoin/bitcoin/issues/32855#issuecomment-3092571841)

> Likely the stdin is missing in this particular run and caused `test/functional/mocks/signer.py` to hang.

I agree with this

in `RunCommandParseJSON` ` str_std_in is empty `, I logged this
```
if(str_std_in.empty()) {
LogPrintf("RunCommandParseJSON: No stdin data to send\n");
}
```
logs
```
grep "RunCommandParseJSON" /tmp/bitcoin_func_test_09pw1wgg/node1/regtest/debug.log
2025-07-19T20:52:09.711192Z [httpworker.1] [common/run_command.cpp:29] [RunCommandParseJSON] RunCommand
...
πŸ“ naiyoma opened a pull request: "2025 2/test wallet signer"
(https://github.com/bitcoin/bitcoin/pull/33020)

I noticed that some test cases had been commented out in wallet_signer.py.
This PR uncomments and modifies some of those test cases so that they can pass.

https://github.com/bitcoin/bitcoin/commit/dac74c69491c3787c63b7bf09a3818ea79f9b8a8 -> deleting this assertion since multiple signers is being tested here https://github.com/bitcoin/bitcoin/blob/master/test/functional/wallet_signer.py#L273

https://github.com/bitcoin/bitcoin/commit/1707c905661ef43935eaaeff06e8141f1be5ea07 test when an
...
πŸ€” djs19901 reviewed a pull request: "depends: add missing Darwin objcopy"
(https://github.com/bitcoin/bitcoin/pull/31840#pullrequestreview-3035634094)
bc1q5vef369tqlwztc08rwjjn3y6986ds56z3hfgg7
πŸ“ l0rinc opened a pull request: "test: revive test verifying that `GetCoinsCacheSizeState` switches from OKβ†’LARGEβ†’CRITICAL"
(https://github.com/bitcoin/bitcoin/pull/33021)
After the changes in https://github.com/bitcoin/bitcoin/pull/25325 `getcoinscachesizestate` always end the test early, see:
https://maflcko.github.io/b-c-cov/test_bitcoin.coverage/src/test/validation_flush_tests.cpp.gcov.html

The test revival was extracted from a related PR where it was discovered, see: https://github.com/bitcoin/bitcoin/pull/28531#discussion_r2109417797
πŸ’¬ l0rinc commented on pull request "improve MallocUsage() accuracy":
(https://github.com/bitcoin/bitcoin/pull/28531#discussion_r2217521200)
I have extracted the suggested test to a separate PR: https://github.com/bitcoin/bitcoin/pull/33021
πŸ‘ l0rinc approved a pull request: "log: [refactor] Use info level for init logs"
(https://github.com/bitcoin/bitcoin/pull/32967#pullrequestreview-3035642619)
Concept ACK, thanks for the cleanup.

I personally would prefer doing every single `LogPrintf` -> `LogInfo` inlining here in a scripted diff (so we can get rid of the alias), with follow-up commits fixing the line endings and formatting and stuff.

And I agree that the warn/error split should be done in a separate PR where we can focus on the context (we could probably do a scripted diff for most of the lines that contain warning/error to cover most of those as well)
πŸ’¬ l0rinc commented on pull request "log: [refactor] Use info level for init logs":
(https://github.com/bitcoin/bitcoin/pull/32967#discussion_r2217522134)
Would it be possible to separate the `LogPrintf` inlining from the `__func__` and rewording changes into focused commits?
πŸ’¬ l0rinc commented on pull request "log: [refactor] Use info level for init logs":
(https://github.com/bitcoin/bitcoin/pull/32967#discussion_r2217527057)
I personally would find it simpler if we inlined every single `LogPrintf` call here, removed the [alias](https://github.com/bitcoin/bitcoin/blob/master/src/logging.h#L369-L370) and the [dedicated test](https://github.com/bitcoin/bitcoin/blob/master/src/test/logging_tests.cpp#L131), and do the info/warn/error differentiation in a separate PR.
I know that means we'd be touching the same lines multiple times, but in that case this PR could simply be a scripted diff with alias removal (+ line endin
...
πŸ’¬ l0rinc commented on pull request "[IBD] Tracking PR for speeding up Initial Block Download":
(https://github.com/bitcoin/bitcoin/pull/32043#issuecomment-3092689738)
We're making progress, https://github.com/bitcoin/bitcoin/pull/31144 was just merged! πŸŽ‰
The next ones that need some love are:
- https://github.com/bitcoin/bitcoin/pull/32827
- https://github.com/bitcoin/bitcoin/pull/32497
- https://github.com/bitcoin/bitcoin/pull/32279
πŸ’¬ Eunovo commented on pull request "Silent Payments: Receiving":
(https://github.com/bitcoin/bitcoin/pull/32966#discussion_r2217563506)
Without `sp_keys`, the wallet can't identify which keys are scan and spend keys from the signing provider. Do you have an alternative approach to this?
πŸ’¬ Eunovo commented on pull request "Silent Payments: Receiving":
(https://github.com/bitcoin/bitcoin/pull/32966#discussion_r2217567798)
Done
πŸ’¬ Eunovo commented on pull request "Silent Payments: Receiving":
(https://github.com/bitcoin/bitcoin/pull/32966#discussion_r2217567956)
Fixed.
πŸ’¬ Eunovo commented on pull request "Silent Payments: Receiving":
(https://github.com/bitcoin/bitcoin/pull/32966#discussion_r2217568207)
Done
πŸ’¬ Eunovo commented on pull request "Silent Payments: Receiving":
(https://github.com/bitcoin/bitcoin/pull/32966#discussion_r2217568262)
Done
πŸ’¬ Eunovo commented on pull request "Silent Payments: Receiving":
(https://github.com/bitcoin/bitcoin/pull/32966#discussion_r2217568307)
I've dropped this commit from this branch.
πŸ’¬ Eunovo commented on pull request "Silent Payments: Receiving":
(https://github.com/bitcoin/bitcoin/pull/32966#discussion_r2217568482)
Done