refactor: Replace remaining binascii method calls #22633

This PR removes the remaining binascii method calls outside test/functional and test_framework, as pointed out here #22619#pullrequestreview-722153458.

Follow-up to #22593 and #22619 Closes #22605

Diff from 3a2ad3b to 33b8a31: Updated Python2 code sample to Python3 and removed binascii.hexlify call.

Now git grep binascii no longer yields any result.

Code-review ACK 33b8a3107fc2d647b2c575f4c081db51f6041e16

It's nice to see that with this PR, there is now only one method left for hex-string/bytes conversions in the functional tests and contrib scripts. Special kudos for also replacing the Python code comment in serfloat_tests.cpp. I tested this directly in the Python interpreter and it works as expected, i.e. the statement yields True.

Diff from 33b8a31 to b95d5ad: addressed comment above.

Thanks @theStack for the background reference regarding contrib/zmq/zmq_sub.py!

re-ACK b95d5ad3676a8cc42165ab4d51a2aae7c7b059fb 💯

ACK https://github.com/bitcoin/bitcoin/pull/22633/commits/b95d5ad3676a8cc42165ab4d51a2aae7c7b059fb

+1 for consistency! did a code review and everything looks good. also ran the standalone scripts where possible and tested a few individual functions (e.g python3 -c 'from rpcauth import generate_salt; print(generate_salt(10))'). copied the python snippet from the comment and ran it, everything working as expected

Concept ACK. Let's be sure that this covers all and is the last PR in the series, it's been dragging on for a while.

I think this is the last PR in this series. No other instances of binascii remain. Unless @theStack has any other suggestions that need to be added?

Let's be sure that this covers all and is the last PR in the series, it's been dragging on for a while.

Agreed!

I think this is the last PR in this series. No other instances of binascii remain. Unless @theStack has any other suggestions that need to be added?

Doing a search in test/function via "git grep -i hex", I unfortunately found yet another variant of converting bin-to-hex that should be fixed (I verified via git grep hex_codec that this is the only place it is used):

diff --git a/test/functional/test_framework/messages.py b/test/functional/test_framework/messages.py
index 6e57107f8..65d90f844 100755
--- a/test/functional/test_framework/messages.py
+++ b/test/functional/test_framework/messages.py
@@ -19,7 +19,6 @@ Classes use __slots__ to ensure extraneous attributes aren't accidentally added
 by tests, compromising their intended effect.
 """
 from base64 import b32decode, b32encode
-from codecs import encode
 import copy
 import hashlib
 from io import BytesIO
@@ -681,7 +680,7 @@ class CBlockHeader:
             r += struct.pack("<I", self.nBits)
             r += struct.pack("<I", self.nNonce)
             self.sha256 = uint256_from_str(hash256(r))
-            self.hash = encode(hash256(r)[::-1], 'hex_codec').decode('ascii')
+            self.hash = hash256(r)[::-1].hex()

     def rehash(self):
         self.sha256 = None

This one is not using binascii (thus not matching the PR title), but should still be tackled in this PR for the reasons laanwj mentioned. After this I'm pretty sure we have them all and #22605 can be closed.

Nice catch! Updated.

Diff b95d5ad -> 021daed:

a/test/functional/test_framework/messages.py

    - from codecs import encode
    ...

    -            self.hash = encode(hash256(r)[::-1], 'hex_codec').decode('ascii')
    +            self.hash = hash256(r)[::-1].hex()

    ...

re-ACK 021daedfa100defad553d69cd3d628aaa5bc7caf

cc @practicalswift @tryphe given you both reviewed the parent PR.

re-ACK https://github.com/bitcoin/bitcoin/pull/22633/commits/021daedfa100defad553d69cd3d628aaa5bc7caf

Posthumous ACK, modulo could have updated the Python example in src/test/serfloat_tests.cpp -- git grep binascii takes you there.

Edit: oops, I ran grep from /src instead of root, there are some other ones, idem for git grep hexlify. Maybe a follow-up if it matters.

Posthumous ACK, modulo could have updated the Python example in src/test/serfloat_tests.cpp -- git grep binascii takes you there.

This PR also tackled the Python code sample in src/test/serfloat_tests.cpp.

Edit: oops, I ran grep from /src instead of root, there are some other ones, idem for git grep hexlify. Maybe a follow-up if it matters.

Just pulled master locally, running both git grep binascii and git grep hexlify in the root shows nothing for me. Also tried it in src and got no results. @jonatack are you still getting any results after f5a406f003a060325128db47552089b3254044e3?

@Zero-1729 You're right! I grepped from the PR branch (edit: nope, another PR branch :sweat_smile:) sorry for the noise.

@jonatack No worries, happy to help! 🙏🏾