Skip to content

Refactored delegation transactions#486

Merged
popenta merged 5 commits into
feat/nextfrom
refactoring-delegation-operations
Mar 3, 2025
Merged

Refactored delegation transactions#486
popenta merged 5 commits into
feat/nextfrom
refactoring-delegation-operations

Conversation

@popenta
Copy link
Copy Markdown
Collaborator

@popenta popenta commented Feb 22, 2025

No description provided.

@popenta popenta self-assigned this Feb 22, 2025
@popenta popenta marked this pull request as draft February 22, 2025 17:16
Base automatically changed from args-validation to feat/next February 27, 2025 11:46
@popenta popenta marked this pull request as ready for review February 27, 2025 11:53
Comment thread multiversx_sdk_cli/cli_delegation.py Outdated
Comment on lines +763 to +764
if value < MINIMUM_AMOUNT_TO_DELEGATE:
errors.BadUserInput(f"Value must be greater than {MINIMUM_AMOUNT_TO_DELEGATE} (1 EGLD)")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Check can be dropped 💭 (time-dependent).

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

deleted

Comment thread multiversx_sdk_cli/cli_delegation.py Outdated
Comment on lines +873 to +874
if value < MINIMUM_AMOUNT_TO_DELEGATE:
errors.BadUserInput(f"Value must be greater than {MINIMUM_AMOUNT_TO_DELEGATE} (1 EGLD)")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

As above 💭

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

deleted

from multiversx_sdk_cli.interfaces import IAccount
from multiversx_sdk_cli.validators.validators_file import ValidatorsFile

DELEGATION_MANAGER_SC_ADDRESS_HEX = "000000000000000000010000000000000000000000000000000000000004ffff"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think the constant is defined somewhere else, as well (when used for creating staking providers).

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

it is not defined anywhere else.


def prepare_transaction_for_automatic_activation(self, owner: IAccount, args: Any) -> Transaction:
delegation_contract = Address.new_from_bech32(args.delegation_contract)
def prepare_transaction_for_automatic_activation(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Heads-up: in Barnard, this might change (we'll see at that point):

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Indeed.


return tx

# will be replaced in the future once it's implemented in sdk-py
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍

Comment thread multiversx_sdk_cli/cli_delegation.py Outdated

def add_new_nodes(args: Any):
ensure_arguments_are_provided_and_prepared(args)
validate_transaction_args(args)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

can you extract this into a method because they are used in every method

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

done

sender, nonce = prepare_sender(args)
guardian, guardian_address = prepare_guardian(args)
relayer, relayer_address = prepare_relayer(args)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Also can you extract this into a method maybe something like InitializeDelegationOperations

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

done

@@ -369,66 +390,233 @@ def ensure_arguments_are_provided_and_prepared(args: Any):
cli_shared.prepare_nonce_in_args(args)


Copy link
Copy Markdown

Choose a reason for hiding this comment

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

can you set the nonce on the sender and then use it from there?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yes, done

@popenta popenta merged commit 0bfe46f into feat/next Mar 3, 2025
@popenta popenta deleted the refactoring-delegation-operations branch March 3, 2025 14:18
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