Repository navigation
Conversation
keepkeylib.protobuf_json still had Python 2 only code: a bare "import types_pb2" (an implicit relative import, so the module fails to import on Python 3) and the long/unicode builtins. Use a package relative import and int/str, and decode hexlified bytes so pb2json() output can be passed to json.dumps(). FieldDescriptor.label was removed in protobuf 7; use FieldDescriptor.is_repeated when available and fall back to label == LABEL_REPEATED on older protobuf.
Several keepkeyctl commands still used Python 2 only idioms and fail
or print garbage on Python 3:
- ethereum_sign_tx and nano_sign_tx used str.decode('hex') and
bytes.encode('hex'), which raise AttributeError; use bytes.fromhex()
and bytes.hex().
- firmware_update --url called urllib.urlretrieve/urlcleanup, which
only exist as urllib.request.* on Python 3, opened the download in
text mode and compared the first bytes against a str; use
urllib.request, open it in binary mode and compare against bytes.
- ethereum_get_address, ethereum_sign_tx, eos_get_public_key,
get_entropy and encrypt_keyvalue formatted binascii.hexlify() output
(bytes) into strings, producing "0xb'...'" (also in the address used
for the JSON-RPC calls and in the published raw transaction) or bytes
that json.dumps() rejects with -j; use bytes.hex(). sign_message
likewise returned base64 bytes; decode them, and decrypt_keyvalue
returned the decrypted bytes (printed as b'...', TypeError with -j);
return the UTF-8 text encrypt_keyvalue stored, or hex otherwise.
- Drop the dead Python 2 branches in encrypt_keyvalue/decrypt_keyvalue.
Add unit tests that run these handlers against a mocked client.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two commits that remove Python 2 only code that still fails on Python 3.
1. fix(protobuf_json): make the module importable and usable on Python 3
keepkeylib/protobuf_json.pystill contains Python 2 only code, so on any Python 3:and once that is fixed, the
long/unicodenames in the type tables raiseNameError. With protobuf 7 thefield.label == FD.LABEL_REPEATEDchecks also fail, becauseFieldDescriptor.labelwas removed (onlyis_repeatedis left).This commit:
types_pb2relative to the package (from . import types_pb2 as types),int/strinstead oflong/unicode,pb2json()so its result can go straight intojson.dumps()(json2pb()already accepts str forunhexlify),_is_repeated()helper that usesFieldDescriptor.is_repeatedand falls back tolabel == LABEL_REPEATEDon older protobuf, soprotobuf>=3.0.0from setup.py keeps working.Regenerating the bundled
*_pb2.pyfiles for protobuf 7 is separate work and not part of this PR.2. fix(keepkeyctl): replace Python 2 leftovers in command handlers
Several
keepkeyctlcommands fail or print garbage on Python 3:ethereum_sign_txandnano_sign_txusestr.decode('hex')/bytes.encode('hex')and raiseAttributeError: 'str' object has no attribute 'decode'. Nowbytes.fromhex()/bytes.hex().firmware_update --urlcallsurllib.urlretrieve/urllib.urlcleanup(AttributeError: module 'urllib' has no attribute 'urlretrieve'), opens the download in text mode and compares the first bytes against a str. Nowurllib.request.*, binary mode, and a bytes comparison.ethereum_get_address,ethereum_sign_tx,eos_get_public_key,get_entropyandencrypt_keyvalueputbinascii.hexlify()output (bytes) into strings, giving0xb'ab...'. Inethereum_sign_txthat broken string is also thefromaddress for the JSON-RPC calls and the raw transaction passed toeth_sendRawTransaction. With-j, the bare bytes results makejson.dumps()fail. Nowbytes.hex().sign_messagereturned base64 bytes the same way; they are now decoded.encrypt_keyvalue/decrypt_keyvalueare removed.decrypt_keyvaluereturned the decrypted bytes (printed asb'...';-jraised TypeError). It nowreturns the UTF-8 text that
encrypt_keyvaluestored, or hex for anything that is not UTF-8.tests/unit/test_keepkeyctl.pyloads the script and runs these handlers against a mocked client (no device).ethjsonrpcandrlpare stubbed insys.modules, so the test needs no extra dependencies.Testing
protobuf 7.36.2, run from the repo root with
PROTOCOL_BUFFERS_PYTHON_IMPLEMENTATION=python, which the current bundled pb2 files need on protobuf >= 4.protobuf_json: round trip
pb2json()->json.dumps()->json.loads()->json2pb()of aTxInputTypewith repeated uint32, uint64, bytes, enum, a nested message, and repeated bytes. The test asserts that the result equals the original message.keepkeyctl:
PYTHONPATH=. python -B tests/unit/test_keepkeyctl.py(8 tests). I ran it with unittest directly becausetests/conftest.pyimportsrequests.The protobuf_json round trip also passes on 3.12.13 with protobuf 3.20.3, which has no
is_repeatedand so uses the label fallback.