-
Notifications
You must be signed in to change notification settings - Fork 36
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
Refactored smart contract interactions #476
Conversation
tx = cosign_transaction(tx, args.guardian_service_url, args.guardian_2fa_code) # type: ignore | ||
def _sign_guarded_tx(guardian: Union[IAccount, None], args: Any, tx: Transaction) -> Transaction: | ||
if guardian: | ||
tx.guardian_signature = bytes.fromhex(guardian.sign_transaction(tx)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In Python, I remember sign_transaction()
returns bytes
, not hex-string:
https://github.com/multiversx/mx-sdk-specs/blob/main/accounts/account.md
Is bytes.fromhex()
correct here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here Account.sign_transaction()
returns a string, so yes, bytes.from_hex()
is correct.
multiversx_sdk_cli/cli_contracts.py
Outdated
|
||
return tx | ||
|
||
|
||
def _sign_tx_by_relayer(relayer: Union[IAccount, None], tx: Transaction): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe add an "if relayed" in function name?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
renamed
multiversx_sdk_cli/cli_contracts.py
Outdated
tx.guardian_signature = bytes.fromhex(guardian_account.sign_transaction(tx)) | ||
elif args.guardian: | ||
tx = cosign_transaction(tx, args.guardian_service_url, args.guardian_2fa_code) # type: ignore | ||
def _sign_guarded_tx(guardian: Union[IAccount, None], args: Any, tx: Transaction) -> Transaction: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe add an "if guarded" in function name? Or "guard transaction if necessary".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
renamed
@@ -360,6 +361,58 @@ def prepare_guardian_account(args: Any): | |||
return account | |||
|
|||
|
|||
def load_guardian_account(args: Any) -> Union[Account, None]: | |||
if args.guardian_pem: | |||
return Account(pem_file=args.guardian_pem, pem_index=args.guardian_pem_index) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, now I see, this is the Account
defined here in mxpy, not in sdk-py.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe at some point we can use the one defined in sdk-py?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, will keep in mind for a future refactoring.
No description provided.