Message pack & unpack #10
Conversation
multiversx_sdk/core/message.py
Outdated
|
|
||
|
|
||
| class Message: | ||
| def __init__(self, data: bytes, signature: bytes = b"") -> None: | ||
| def __init__(self, data: bytes, address: IAddress, signature: bytes = b"", version: int = 1) -> None: |
There was a problem hiding this comment.
While the new ordering of the parameters is good and makes perfect sense, let's postpone the breaking change for some time - and make the new address optional, as well (and keep the previous order of the parameters).
There was a problem hiding this comment.
changed the order of the parameters and made address: Optional[IAddress] = None
multiversx_sdk/core/message.py
Outdated
| } | ||
|
|
||
| def unpack(self, packed_message: Dict[str, Any]) -> Message: | ||
| data: str = packed_message["message"] |
There was a problem hiding this comment.
We should handle missing data - possibly allow something to be missing (e.g. address, version).
There was a problem hiding this comment.
now handling missing data, but that shouldn't happen because of the default values.
multiversx_sdk/core/message.py
Outdated
|
|
||
| def unpack(self, packed_message: Dict[str, Any]) -> Message: | ||
| data: str = packed_message["message"] | ||
| if data.startswith("0x") or data.startswith("0X"): |
There was a problem hiding this comment.
Can extract to a private function e.g. trim_hex_prefix or so.
multiversx_sdk/core/message.py
Outdated
| def compute_bytes_for_verifying(self, message: IMessage) -> bytes: | ||
| return self.compute_bytes_for_signing(message) | ||
|
|
||
| def pack(self, message: IMessage) -> Dict[str, Any]: |
There was a problem hiding this comment.
I know here we are in the MessageComputer itself, but maybe we can name the functions pack_message() and unpack_message() (more explicit naming)?
There was a problem hiding this comment.
renamed the methods.
| @@ -1244,15 +1245,15 @@ | |||
| } | |||
| ], | |||
| "source": [ | |||
| "from multiversx_sdk.core import TransactionComputer, MessageComputer\n", | |||
| "from multiversx_sdk.core import MessageComputer, TransactionComputer\n", | |||
There was a problem hiding this comment.
Oh, it's so nice to have the cookbook in the same repository :)
| @@ -29,6 +31,8 @@ class ITransaction(Protocol): | |||
| class IMessage(Protocol): | |||
| data: bytes | |||
| signature: bytes | |||
| address: IAddress | |||
| version: int | |||
There was a problem hiding this comment.
Now that I think about it - there does not seem too much value in having an interface for the message DTO (neither for the transaction DTO, on that matter, but we'll see). Let's think about dropping some of these, if not very useful (e.g. a message DTO is easy to create / instantiate, it's very close to being a "primitive" piece of data).
There was a problem hiding this comment.
Will keep it in mind
| assert message.signature.hex() == "7aff43cd6e3d880a65033bf0a1b16274854fd7dfa9fe5faa7fa9a665ee851afd4c449310f5f1697d348e42d1819eaef69080e33e7652d7393521ed50d7427a0e" | ||
|
|
||
| packed_message = message_computer.pack(message) | ||
| assert packed_message == { |
There was a problem hiding this comment.
Perhaps add a test where the packed message is in the legacy form (with 0x)?
There was a problem hiding this comment.
added a test using the legacy message
multiversx_sdk/core/message.py
Outdated
|
|
||
| address = Address.from_bech32(packed_message["address"]) | ||
| version = packed_message["version"] | ||
| version = packed_message.get("version", 1) |
There was a problem hiding this comment.
Can be a constant (here and for the constructor of Message).
Added functionality to "pack" and "unpack" a message.