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

Test Argent account support #481

Merged
merged 6 commits into from
Jun 4, 2024
Merged

Test Argent account support #481

merged 6 commits into from
Jun 4, 2024

Conversation

FabijanC
Copy link
Contributor

@FabijanC FabijanC commented Jun 3, 2024

Usage related changes

  • None

Development related changes

  • Add Argent account testing
  • The majority of line changes is just refactoring of account testing
  • Extract some constants (related to forking and account class hash)
  • Define OZ and Argent account deployment utility functions
  • Removed functions that extracted common functionality because the common functionality was no longer substantial; the new look is more explicit IMO:
    • can_deploy_new_account_test_body
    • can_declare_deploy_invoke_using_predeployed_test_body

Checklist:

  • Checked out the contribution guidelines
  • Applied formatting - ./scripts/format.sh
  • No linter errors - ./scripts/clippy_check.sh
  • No unused dependencies - ./scripts/check_unused_deps.sh
  • No spelling errors - ./scripts/check_spelling.sh
  • Performed code self-review
  • Rebased to the latest commit of the target branch (or merged it into my branch)
  • Updated the docs if needed - ./website/README.md
  • Linked the issues resolvable by this PR - linking info
  • Updated the tests if needed; all passing - execution info

@FabijanC FabijanC requested a review from mikiw June 3, 2024 15:00
@FabijanC FabijanC changed the title Test argent account Test Argent account support Jun 4, 2024
Copy link
Contributor

@mikiw mikiw left a comment

Choose a reason for hiding this comment

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

We have OZ account artefact built in devnet and we don't need to fork anything to test it and play with it but now to test Argent account we need to fork mainnet.

Shouldn't we just add an Argent account the same way we added OZ account so devnet users can use Argent account straight away?

@mikiw
Copy link
Contributor

mikiw commented Jun 4, 2024

We have OZ account artefact built in devnet and we don't need to fork anything to test it and play with it but now to test Argent account we need to fork mainnet.

Shouldn't we just add an Argent account the same way we added OZ account so devnet users can use Argent account straight away?

ok, since forking is our way to solve it it's ok

@FabijanC FabijanC merged commit 25f995a into main Jun 4, 2024
@FabijanC FabijanC deleted the test-argent-account branch June 4, 2024 08:41
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.

None yet

2 participants