Clean exampleContracts folder

NethermindEth / warp

Warp - Bringing Solidity to Starknet at warp speed. Warp is a Solidity to Cairo Compiler, this allows teams to write/migrate Solidity to Cairo for easy onboarding into the StarkNet ecosystem.

https://nethermind.io/warp/

Apache License 2.0

754 stars 70 forks source link

Clean exampleContracts folder #989

Closed julio4 closed 1 year ago

julio4 commented 1 year ago

Fixes #836

What was wrong?

Currently, most of the contracts inside exampleContracts are used for tests. This folder should contain only useful examples for the user.

How was it fixed?

Extracted .sol contracts used in tests from exampleContract folder to a new folder tests/compilation
Renamed yarn script test:examples to test:compilation
Expected test results for contracts in tests/compilation are defined in tests/testing.ts (Default: 'Success')
All contracts in exampleContracts must successfully compile wit both Warp and Starknet-compile. (Tested during tests:compilation)

For contributors

For an important feature, you can write a simple solidity contract in exampleContract to show the usage of this feature.

For all specific test cases, you can add any contracts under tests/compilation and specify the result expectations in the expectedResults map (tests/testing.ts ).

AlejandroLabourdette commented 1 year ago

Hi @julio4, great work. We detected some issues with dependency versions of cairo-lang that, as you see, make our tests to fail. If you merge develop into this branch it should be fixed.

JorikSchellekens commented 1 year ago

Hey @julio4! Amazing. Thank you for cleaning this up a bit. There are many more interesting contracts that should be preserved and cleaned up in the examples folder.

Address sizes require different casts. It would be nice to keep a single file demonstrating and commenting on each case
asserts and reverts have slightly different semantics (the argument must be a literal string in all cases). A commented-out example which doesn't work would be great here
the unsupported cases should be preserved (with comments explaining what isn't supported)
dai is a cool one
include tests/behaviour/contracts/cairoStubs/proxy.sol, a cool example of proxies.

julio4 commented 1 year ago

@JorikSchellekens Maybe I can open a separate issue to clean and add more interesting example contracts ? Additionally, all tests in the example folder are tested and must compile. If we add unsupported cases, should we move them to a dedicated unsupported folder, leave them as comments, or perhaps disable testing for contracts in this example folder? Let me know your thoughts.

rjnrohit commented 1 year ago

Hey , @julio4 thank you for this PR 🙌. But unfortunately I have to close this PR, since I have added some changes in the PR: https://github.com/NethermindEth/warp/pull/1002 inspired from this PR to the cairo-1.0 branch , and eventually cairo-1.0 branch will be merged to the develop branch .

If you have some questions then you ask here. Are you okay with closing this PR?

Anyway thanks for your contribution ! 🎉 Looking forward to see more contributions towards this repo from you 😊

julio4 commented 1 year ago

@rjnrohit Ok I understand.

Can you please share what I could have improved so I can make a better contribution next time ?

rjnrohit commented 1 year ago

@rjnrohit Ok I understand.

Can you please share what I could have improved so I can make a better contribution next time ?

What you have done is brilliant ! I just needed this changes quickly added to cairo-1.0 branch with cairo1 compatibility. So, I followed some of your idea.

For the contribution specifically, as long as you're following this guide :https://github.com/NethermindEth/warp/blob/develop/CONTRIBUTING.md, it should be good for us.

We're eagerly seeking external peoples contributions, so don't hesitate to open a PR. It'll anyway makes WARP better.

julio4 commented 1 year ago

@rjnrohit Ok I understand. Can you please share what I could have improved so I can make a better contribution next time ?

What you have done is brilliant ! I just needed this changes quickly added to cairo-1.0 branch with cairo1 compatibility. So, I followed some of your idea.

For the contribution specifically, as long as you're following this guide :https://github.com/NethermindEth/warp/blob/develop/CONTRIBUTING.md, it should be good for us.

We're eagerly seeking external peoples contributions, so don't hesitate to open a PR. It'll anyway makes WARP better.

Understood! Glad I could help, and I’m looking forward to contribute again!