Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for instantiating address and bytes values from dictionary #80

Merged
merged 4 commits into from
Jul 9, 2024

Conversation

popenta
Copy link
Collaborator

@popenta popenta commented Jul 8, 2024

Previously, the AddresValue used to convert the value of set_payload to bytes. Added support to set payload from a dictionary like bellow:

AddressValue.set_payload(
    {
         "bech32": "erd1qyu5wthldzr8wx5c9ucg8kjagg0jfs53s8nr3zpz3hypefsdd8ssycr6th"
    }
)

or

AddressValue.set_payload(
    {
         "hex": "0139472eff6886771a982f3083da5d421f24c29181e63888228dc81ca60d69e1"
    }
)

BytesValue can now be set from a hex string, like so:

BytesValue.set_payload(
    {
         "hex": "68656c6c6f"
    }
)

Also now, an error is raised if someone tries to convert a dictionary to a list using convert_native_value_to_list.

@popenta popenta self-assigned this Jul 8, 2024
Copy link

github-actions bot commented Jul 8, 2024

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  multiversx_sdk/abi
  address_value.py
  bytes_value.py
  shared.py
Project Total  

This report was generated by python-coverage-comment-action

andreibancioiu
andreibancioiu previously approved these changes Jul 8, 2024
@@ -29,9 +27,20 @@ def decode_top_level(self, data: bytes):
def set_payload(self, value: Any):
if isinstance(value, str):
self.value = bytes(value, "utf-8")
elif isinstance(value, dict):
value = cast(Dict[str, str], value)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe the cast is not necessary, since the linter sees an Any type passed to _extract_value_from_dict and is already forgiving?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The cast is necessary since we have the elif isinstance(value, dict) statement.

if hex_address:
return bytes.fromhex(hex_address)

raise ValueError("cannot extract pubkey from dictionary")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe specify what's needed, in parenthesis e.g. cannot extract pubkey from dictionary (missing "bech32", missing "hex") or something similar.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added

@@ -42,10 +42,26 @@ def _check_pub_key_length(self, pubkey: bytes) -> None:
raise ValueError(f"public key (address) has invalid length: {len(pubkey)}")

def set_payload(self, value: Any):
pubkey = bytes(value)
if isinstance(value, dict):
value = cast(Dict[str, str], value)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If cast can be skipped and the linter is okay with it, let's skip it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The linter complains if we skip the cast.

def _extract_pubkey_from_dict(self, value: Dict[str, str]) -> bytes:
bech32_address = value.get("bech32", None)
if bech32_address:
return Address.new_from_bech32(bech32_address).get_public_key()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here, we reference core in abi - maybe this is the first occasion? However, there should not be any issues with this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not the first time we reference core in abi. It is also used in abi.py and biguint_value.py.

if hex_address:
return bytes.fromhex(hex_address)

raise ValueError("cannot extract pubkey from dictionary")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As above, some details in parenthesis would help the developers.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added

@popenta popenta merged commit f35b2c4 into main Jul 9, 2024
5 checks passed
@popenta popenta deleted the small-abi-enhancements branch July 9, 2024 09:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants