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

Fix get account and get transaction #482

Merged
merged 6 commits into from
Feb 19, 2025
Merged

Fix get account and get transaction #482

merged 6 commits into from
Feb 19, 2025

Conversation

popenta
Copy link
Collaborator

@popenta popenta commented Feb 12, 2025

No description provided.

@popenta popenta self-assigned this Feb 12, 2025
@popenta popenta marked this pull request as draft February 12, 2025 13:30
Base automatically changed from create-validator-wallet to feat/next February 13, 2025 14:31
@popenta popenta marked this pull request as ready for review February 13, 2025 14:32
@@ -45,4 +52,34 @@ def get_account(args: Any):
elif args.username:
print(account.username)
else:
utils.dump_out_json(account.raw)
account = _account_on_network_to_dictionary(account)
Copy link
Contributor

Choose a reason for hiding this comment

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

Linter warning.

@@ -32,6 +37,8 @@ def _add_address_arg(sub: Any):


def get_account(args: Any):
omitted_fields = cli_shared.parse_omit_fields_arg(args)
Copy link
Contributor

Choose a reason for hiding this comment

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

Though, do we still need this command, or can we drop it (since people can simply get the data from the public API)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

as decided, we've dropped this command.

@@ -203,6 +197,9 @@ def send_transaction(args: Any):


def get_transaction(args: Any):
if not args.proxy:
Copy link
Contributor

Choose a reason for hiding this comment

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

Though, do we still need this command, or can we drop it (since people can simply get the data from the public API)?

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 command has been dropped, as well.

Comment on lines 170 to 171
"gas_limit": tx.gas_limit,
"gas_price": tx.gas_price,
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe use the "standard" JSON naming conventions / camel-case.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

@popenta popenta merged commit 6ead3b4 into feat/next Feb 19, 2025
12 checks passed
@popenta popenta deleted the fix-get-account branch February 19, 2025 11:57
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