-
Notifications
You must be signed in to change notification settings - Fork 37.7k
test: add logging to mock external signers #32928
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. Code Coverage & BenchmarksFor details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32928. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
4f8075b
to
3ff8e66
Compare
Adding mock_signer_log and logging “Started” / “Finished” makes debugging much clearer and great improvement, especially since stdout/stderr can’t be used here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Concept ACK ->
I attempted to debug -> #32855 (comment) after pulling this branch, and so far I can see some useful logs in combined.log
mock_external_signer” logs from this diff
grep "mock_external_signer" combined.log
node1 2025-07-19T19:35:11.455260Z [init] [common/args.cpp:850] [logArgsPrefix] Command-line arg: signer="/usr/bin/python3 /home/ubuntu/Projects/bitcoin/test/functional/test_framework/../mock_external_signer.py"
test 2025-07-19T19:35:11.823136Z TestFramework (DEBUG): -signer=/usr/bin/python3 /home/ubuntu/Projects/bitcoin/test/functional/test_framework/../mock_external_signer.py
node1 2025-07-19T19:35:11.846121Z [httpworker.1] [common/run_command.cpp:29] [RunCommandParseJSON] RunCommandParseJSON: command='/usr/bin/python3 /home/ubuntu/Projects/bitcoin/test/functional/test_framework/../mock_external_signer.py enumerate', stdin_length=0, stdin_empty=true
test 2025-07-19T19:35:11.963244Z TestFramework.mock_external_signer (DEBUG): Started
test 2025-07-19T19:35:11.969415Z TestFramework.mock_external_signer (DEBUG): Finished
node1 2025-07-19T19:35:11.986061Z [httpworker.1] [common/run_command.cpp:29] [RunCommandParseJSON] RunCommandParseJSON: command='/usr/bin/python3 /home/ubuntu/Projects/bitcoin/test/functional/test_framework/../mock_external_signer.py --fingerprint 00000001 --chain regtest getdescriptors --account 0', stdin_length=0, stdin_empty=true
test 2025-07-19T19:35:12.078454Z TestFramework.mock_external_signer (DEBUG): Started
test 2025-07-19T19:35:12.084684Z TestFramework.mock_external_signer (DEBUG): Naiyoma HANGING: stdin hang for getdescriptors
- reuse perform_pre_checks helper - add mock_psbt_path helper
Because stdout and stderr are consumed by the node, we can't use them for logging. Instead have the mocks print directly into test_framework.log, which can then be be retrieved via combine_logs.py.
3ff8e66
to
b1edd6d
Compare
For reference, the issue was something else, and has been fixed now, in the meantime. |
@maflcko updated the PR description. I assume more logging is still useful though. |
Because
stdout
andstderr
are consumed by the node, the mock external signers can't use them for logging.Instead have them print directly into
test_framework.log
, which can then be be retrieved viacombine_logs.py
.In preparation (and addtion) this PR starts with a few refactors:
mock_signer_path
helpers toutil
and reuse them betweenrpc_signer.py
andwallet_signer.py
test/functional/mocks
totest/functional
: this is needed to avoid Python import hell in the next commitstest_framework
by extracting (and reusing)perform_pre_checks
andmock_signer_psbt_path
intoutil
debug.log
styleFormatter
intoutil
so we can use it in the final commitOriginally this PR was intended to aid in debugging #32855, which has been solved since.