refactor: Replace remaining binascii method calls#22633
refactor: Replace remaining binascii method calls#22633maflcko merged 1 commit intobitcoin:masterfrom
Conversation
3a2ad3b to
33b8a31
Compare
|
Diff from 3a2ad3b to 33b8a31: Updated Python2 code sample to Python3 and removed Now |
theStack
left a comment
There was a problem hiding this comment.
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.
33b8a31 to
b95d5ad
Compare
|
Diff from 33b8a31 to b95d5ad: addressed comment above. Thanks @theStack for the background reference regarding |
theStack
left a comment
There was a problem hiding this comment.
re-ACK b95d5ad3676a8cc42165ab4d51a2aae7c7b059fb 💯
|
ACK b95d5ad +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 |
|
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 |
Agreed!
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 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 |
b95d5ad to
021daed
Compare
|
Nice catch! Updated. Diff b95d5ad -> 021daed:
- from codecs import encode
...
- self.hash = encode(hash256(r)[::-1], 'hex_codec').decode('ascii')
+ self.hash = hash256(r)[::-1].hex()
... |
|
cc @practicalswift @tryphe given you both reviewed the parent PR. |
|
re-ACK 021daed |
There was a problem hiding this comment.
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.
This PR also tackled the Python code sample in
Just pulled master locally, running both |
|
@Zero-1729 You're right! I grepped from the PR branch (edit: nope, another PR branch 😅) sorry for the noise. |
|
@jonatack No worries, happy to help! 🙏🏾 |
This PR removes the remaining
binasciimethod calls outsidetest/functionalandtest_framework, as pointed out here #22619 (review).Follow-up to #22593 and #22619
Closes #22605