Skip to content

BIP374: remove debug print from test runner #1920

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

Merged
merged 1 commit into from
Aug 10, 2025
Merged

Conversation

strmfos
Copy link
Contributor

@strmfos strmfos commented Aug 9, 2025

Remove leftover debug print statement that was printing private keys(seckey_a_hex) to console during test execution. Improves test output readability and consistency.

@jonatack jonatack changed the title BIP374: remove debug print statement from BIP-374 test runner BIP374: remove debug print from test runner Aug 9, 2025
Copy link
Member

@jonatack jonatack left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but needs approval from a BIP author: @andrewtoth, @RubenSomsen, @theStack WDYT?

@@ -23,7 +23,6 @@
next(reader)
for row in reader:
(index, point_G_hex, seckey_a_hex, point_B_hex, aux_rand_hex, msg_hex, result_str, comment) = row
print(seckey_a_hex)
Copy link
Member

@jonatack jonatack Aug 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test runner output is indeed more clear and consistent after this change.

@jonatack jonatack added the Pending acceptance This BIP modification requires sign-off by the champion of the BIP being modified label Aug 9, 2025
Copy link
Contributor

@theStack theStack left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 1421be4

@jonatack jonatack removed the Pending acceptance This BIP modification requires sign-off by the champion of the BIP being modified label Aug 9, 2025
@jonatack
Copy link
Member

Thanks @theStack 👍

@jonatack jonatack merged commit 4f1359d into bitcoin:master Aug 10, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants