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: scarb library with template account contract #813

Merged
merged 3 commits into from
Oct 26, 2024
Merged

Conversation

ICavlek
Copy link
Contributor

@ICavlek ICavlek commented Oct 14, 2024

In scope of this PR, scarb library has been introduced in test environment. Scarb is a tool that is used for compilation of smart contracts within the starknet ecosystem. This is marked as an initial step for actual account deployment within test cases.
General overview of the introduced changes:

  • Added two test cases, one for sepolia and one for katana
  • Added template account contract
  • Possibility to create and compile multiple account contracts

Additional notable changes:

  • Refactor of test module
  • Added TODOs for additional work

Since the general overview of the test case can be observed with the changes in this PR and future TODOs, please consider the structure and the workflow of the test case within this PR.

@ICavlek ICavlek requested a review from a team October 14, 2024 12:43
@ICavlek ICavlek linked an issue Oct 14, 2024 that may be closed by this pull request
@ICavlek ICavlek force-pushed the ic/scarb_contract branch 3 times, most recently from 5a5fbb6 to e44e531 Compare October 15, 2024 07:15
Copy link
Contributor

@LKozlowski LKozlowski left a comment

Choose a reason for hiding this comment

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

LGTM

To make sure I understand it correctly: We use scarb from within tests to compile account's smart contract then deploy it and test it?

tests/rpc.rs Outdated Show resolved Hide resolved
@ICavlek
Copy link
Contributor Author

ICavlek commented Oct 18, 2024

LGTM

To make sure I understand it correctly: We use scarb from within tests to compile account's smart contract then deploy it and test it?

Yes, scarb is used within the executor. I have created a template where multiple accounts will be deployed on Katana before using the framework on Sepolia. Since the compilation is a slow process (~30 secs in test), it is pushed in separate threads based on the amount of newly deployed accounts.
After that, with starkli introduced also as lib (issue that I am currently working on) we will be able to declare and deploy it on network

Copy link
Collaborator

@sergey-melnychuk sergey-melnychuk left a comment

Choose a reason for hiding this comment

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

LGTM % a few nitpicks

@ICavlek ICavlek force-pushed the ic/scarb_contract branch 3 times, most recently from 35f325e to 6d4967e Compare October 23, 2024 11:35
@ICavlek
Copy link
Contributor Author

ICavlek commented Oct 23, 2024

@sergey-melnychuk @LKozlowski I have updated PR with significant changes and I would like you to have another look at it. Based on the comments, I have reworked it in a way that I have removed executor.rs and added coordinator.rs which will only handle the environment of the contracts.

One additional thing that is not directly connected to this PR, clippy fails in gen.rs (see workflows on commits). I have updated it here so that it can be observed, but I guess that this will be done in a different PR. I will update it here when it is done.

src/gen.rs Outdated Show resolved Hide resolved
tests/rpc.rs Outdated Show resolved Hide resolved
tests/rpc.rs Outdated Show resolved Hide resolved
@sergey-melnychuk sergey-melnychuk self-requested a review October 23, 2024 20:09
Copy link
Collaborator

@sergey-melnychuk sergey-melnychuk left a comment

Choose a reason for hiding this comment

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

Left a few comments about unnecessary complications if a very simple workflows (introducing structs with hard-coded unchangeable properties, when all the process can be described as a composition of simple functions).

tests/rpc.rs Outdated Show resolved Hide resolved
@ICavlek ICavlek force-pushed the ic/scarb_contract branch 2 times, most recently from b03aeda to 9e704b2 Compare October 24, 2024 11:01
@ICavlek
Copy link
Contributor Author

ICavlek commented Oct 24, 2024

@sergey-melnychuk @LKozlowski Updated PR based on comments:


  • Recovered two accidentally deleted tests
  • Removed two template tests that will be added in scope of separate PRs
  • Removed Compiler class and reduced it to single function
  • Removed coordinator.rs, it's Coordinator class and reduced it to single function in utils.rs

@sergey-melnychuk sergey-melnychuk self-requested a review October 25, 2024 07:04
@ICavlek ICavlek changed the title test: scarb library with template test cases test: scarb library with template account contract Oct 25, 2024
@ICavlek ICavlek merged commit bd6c854 into main Oct 26, 2024
7 checks passed
@ICavlek ICavlek deleted the ic/scarb_contract branch October 26, 2024 05:48
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.

feat: scarb support with valid account contracts
3 participants