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

Allow bech32 address value payload #174

Merged

Conversation

EtienneWallet
Copy link

@EtienneWallet EtienneWallet commented Jan 11, 2025

Context

Currently, bech32 string cannot be directly passed as tranaction argument to the smart-contract factories. Indeed, this leads to an error when setting the payload of an AddressValue

AddressValue().set_payload("erd1qyu5wthldzr8wx5c9ucg8kjagg0jfs53s8nr3zpz3hypefsdd8ssycr6th")

    def set_payload(self, value: Any):
        if isinstance(value, dict):
            value = cast(Dict[str, str], value)
            pubkey = self._extract_pubkey_from_dict(value)
        else:
>           pubkey = bytes(value)
E           TypeError: string argument without an encoding

multiversx_sdk/abi/address_value.py:52: TypeError

Proposed Change

If the set_payload method receives a string, it should try to parse it as bech32 to extract the pubkey

def set_payload(self, value: Any):
        if isinstance(value, dict):
            value = cast(Dict[str, str], value)
            pubkey = self._extract_pubkey_from_dict(value)
        elif isinstance(value, str):
            address = Address.new_from_bech32(value)
            pubkey = address.pubkey
        else:
            pubkey = bytes(value)

This would allow to pass effectively bech32 strings as transaction arguments to the smart-contract factories.

Copy link
Collaborator

@popenta popenta left a comment

Choose a reason for hiding this comment

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

Looks good!

# Simple (from bech32)
address = Address.new_from_bech32("erd1qyu5wthldzr8wx5c9ucg8kjagg0jfs53s8nr3zpz3hypefsdd8ssycr6th")
value = AddressValue()
value.set_payload(address.bech32())
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use to_bech32() instead.

@@ -45,6 +45,9 @@ def set_payload(self, value: Any):
if isinstance(value, dict):
value = cast(Dict[str, str], value)
pubkey = self._extract_pubkey_from_dict(value)
elif isinstance(value, str):
address = Address.new_from_bech32(value)
pubkey = address.pubkey
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd say use get_public_key() to stay consistent throughout the class.

Copy link
Contributor

@andreibancioiu andreibancioiu left a comment

Choose a reason for hiding this comment

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

🎉 Nice!

(the comments from Alex)

@EtienneWallet
Copy link
Author

requested changes applied!

Copy link

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  multiversx_sdk/abi
  address_value.py
  multiversx_sdk/smart_contracts
  smart_contract_controller.py
Project Total  

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

@popenta popenta merged commit 55148d4 into multiversx:feat/next Jan 15, 2025
4 checks passed
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