DX: Keyreg bytes #521
DX: Keyreg bytes #521
Conversation
jannotti
force-pushed
the
keyreg-bytes
branch
from
2e510fa to
857a48c
Compare
algosdk/transaction.py
Outdated
| votefst (int) | ||
| votelst (int) | ||
| votekd (int) | ||
| type (str) | ||
| lease (byte[32]) | ||
| rekey_to (str) | ||
| nonpart (bool) | ||
| sprfkey (str) | ||
| sprfkey (byte[64]) |
There was a problem hiding this comment.
Expanding the input types to support bytes is a no-brainer, but I'm reluctant to change the attribute types as I view that as a breaking change.
The reality is this SDK uses base64 encoded strings to shuttle bytes around in a lot of places (e.g. public/private keys as well), and it's really in need of an upgrade to bytes across the board.
algosdk/transaction.py
Outdated
| votefst (int) | ||
| votelst (int) | ||
| votekd (int) | ||
| type (str) | ||
| lease (byte[32]) | ||
| rekey_to (str) | ||
| nonpart (bool) | ||
| sprfkey (str) | ||
| sprfkey (byte[64]) | ||
| """ | ||
|
|
||
| def __init__( |
There was a problem hiding this comment.
minor: it'd be nice to annotate the args with Union[bytes, bytearray, str] so it's clearer what types are accepted (though I realize this class does not do any type annotating)
algosdk/transaction.py
Outdated
| if key is None: | ||
| return None | ||
| if isinstance(key, (bytes, bytearray)) and len(key) == size: | ||
| return key |
There was a problem hiding this comment.
I'd want to see bytearray coerced to bytes here if we were keeping the return value of this function in an attribute (for consistency and immutability). Though this isn't applicable if we don't change the attributes
jannotti
force-pushed
the
keyreg-bytes
branch
from
857a48c to
ef2131e
Compare
Previously, these fields were required to be base64 encoded, which seems like a strange requirement. Now they may be bytes or b64 strings. Either way, they are stored in the python object as b64 strings to maintain compatibility with our original decision, regardless of how strange it was. Added a few doc string cleanups.
|
going to close this an re-open with a different strategy |
Allow the various key-ish fields of a keyreg txn to be bytes
Previously, these fields were required to be base64 encoded, which
seems like a strange requirement.
Added a few doc string cleanups.