Update README.md

[JulissaDantes]

Fixes #254

Since the directory was changed the README had to be updated accordingly.

[andrew-fleming]

Good catch! Since we added the src directory, it seems that even with the proposed change, compilation will still return errors (since the contracts use openzeppelin as the root with imports). A workaround I found is to cd into src and run nile compile --directory openzeppelin. I'm not sure if that should be included or if we should find a more elegant solution. Thoughts?

Also, we should add a note that an --account_contract flag is required for compiling Account and IAccount. And I think we need to remove src from ReentrancyGuard's import (or importing bools from Cairo's bool.cairo fixes that, too).

[andrew-fleming]

Update! The contracts all compile with nile compile --directory src :)

[JulissaDantes]

Update! The contracts all compile with nile compile --directory src :)

Thanks @andrew-fleming you are right, I've updated the readme accordingly

[martriay]

Also, we should add a note that an --account_contract flag is required for compiling Account and IAccount.

There is no easy way to solve this for the general nile compile case, right? Other than adding extra assumptions such as the one @andrew-fleming proposed before, of assuming that if a contract has Account anywhere in its name, then it should pass the flag automatically. We can consider such an assumption, but Account alone is too broad imo.

And I think we need to remove src from ReentrancyGuard's import (or importing bools from Cairo's bool.cairo fixes that, too).

I can tackle this, but I don't understand how bool.cairo imports are related to ReentrancyGuard's import issues.

[andrew-fleming]

@martriay I agree that just looking for Account is too broad. Maybe something like: if the contract has Account as its suffix?—ergo, Account and IAccount would trigger the flag but Account<something> does not?

I don't understand how bool.cairo imports are related to ReentrancyGuard's import issues.

Importing from bool.cairo eliminates the need for the constants.cairo import in ReentrancyGuard (which is where the issue was with src), but it compiles fine with nile compile --directory src.

[martriay]

@martriay I agree that just looking for Account is too broad. Maybe something like: if the contract has Account as its suffix?—ergo, Account and IAccount would trigger the flag but Account<something> does not?

I think it's worth opening the question to the broader community, in the context of Nile rather than in here.

Importing from bool.cairo eliminates the need for the constants.cairo import in ReentrancyGuard (which is where the issue was with src), but it compiles fine with nile compile --directory src.

Gotcha. I still have issues with vscode though. Is there any easy way to change the target directory? @ericglau

[martriay]

Changes look good. @JulissaDantes please pull the latest main to fix the CI issues.

[ericglau]

I still have issues with vscode though. Is there any easy way to change the target directory?

@martriay I've just updated the VS Code extension to support setting the source dir. You would need to manually set it to src in the VS Code settings:

[martriay]

Amazing @ericglau! Do you think it is worth documenting that somewhere, like CONTRIBUTING.md or too offtopic? I think it's a nice improvement to the Contracts for Cairo contributor toolbelt.

[ericglau]

@martriay I'm planning to let it read the package_dir from setup.cfg (ericglau/cairo-ls#20) so this setting should only be needed as a temporary measure.

[ericglau]

Following up on the above: Cairo LS now autodetects the source directory from setup.cfg so it's no longer necessary to set it manually in VS Code settings (although it's fine to leave it there).