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

Need --account-contract flag when importing IAccount #328

Closed
pscott opened this issue May 20, 2022 · 13 comments
Closed

Need --account-contract flag when importing IAccount #328

pscott opened this issue May 20, 2022 · 13 comments

Comments

@pscott
Copy link
Contributor

pscott commented May 20, 2022

IAccount exopses the __execute__ function which gets the cairo-compiler to yell and error like this:

Only account contracts may have a function named "__execute__". Use --account-contract flag.

        Failed

Unfortunately this means I can't write a contract that includes IAccount without adding the --account-contract flag (which is plain wrong because I'm NOT writing an account contract but simply interacting with one!).

Not sure OZ can do anything about it, I think we need to address this to the StarkWare team but just putting it out there :)

@martriay
Copy link
Contributor

martriay commented May 20, 2022

Agree with this. I don't think this flag makes much sense cc/@bbrandtom. If the idea is to protect users from mis-deploying (not sure if that's a good idea either), then the filter should be on deployment not compilation.

Do consider @JulissaDantes' suggestion of using a language directive.

@ilyalesokhin-starkware
Copy link

ilyalesokhin-starkware commented May 25, 2022

I was unable to reproduce the issue.

What do you mean by IAccount exopses the __execute__?
IAccount only defines an interface, it does not define any @external entry point.

@pscott
Copy link
Contributor Author

pscott commented May 25, 2022

@ilyalesokhin-starkware thanks for having a look at this!

Here's a minimal working (failing) example. I'm trying to compile it with 0.8.2.1 and it's giving me the error I mentioned in the OP.

https://gist.github.com/pscott/aa99849a1e1b53f2eaed3f71de642e8e

@ilyalesokhin-starkware
Copy link

The example compiles on my machine.

Are you sure you didn't change one of the imported files to include an __execute__ entrypoint?

@pscott
Copy link
Contributor Author

pscott commented May 25, 2022

Interesting... I'm using 0.1.0 of openzeppelin-cairo-contracts dependency. Are you running this version too? (installed via pip3 install openzeppelin-cairo-contracts)

I have not added any __execute__ entrypoint.

Maybe someone else can reproduce?

@pscott
Copy link
Contributor Author

pscott commented May 25, 2022

Also it should be noted that I'm using the starknet hardhat plugin and running npx hardhat starknet-compile in order to compile.

@ilyalesokhin-starkware
Copy link

This is related to the #291 and it's already fixed in the main branch of openzeppelin-cairo-contracts.

openzeppelin-cairo-contracts v0.1.0 is broken.

@pscott
Copy link
Contributor Author

pscott commented May 26, 2022

All right so I guess problem solved! So I guess you testing with a local clone of the OZ repo?

@martriay @andrew-fleming do you think OZ could issue a 0.1.1 while waiting for 0.2.0? Would be really nice to be able to use Accounts right now :D

@ilyalesokhin-starkware
Copy link

Your guess is correct, my first attempt to reproduce the issue failed as I fetched the most recent master from the OZ repo.

@martriay
Copy link
Contributor

martriay commented May 26, 2022

All right so I guess problem solved! So I guess you testing with a local clone of the OZ repo?

@martriay @andrew-fleming do you think OZ could issue a 0.1.1 while waiting for 0.2.0? Would be really nice to be able to use Accounts right now :D

No, we're really close to 0.2.0 and it would be a ton of work to cherrypick non-breaking changes to release 0.1.1.

I still think the flag is problematic and I vote for removing it, see OpenZeppelin/nile#112 for example.

@bbrandtom
Copy link

@martriay why do you think the flag is problematic? Why an 'account' directive will be better?

@martriay
Copy link
Contributor

martriay commented Jun 15, 2022

@martriay why do you think the flag is problematic? Why an 'account' directive will be better?

Because you can't just compile all the contracts on a project, and there's no easy nor reliable way to identify them. It's also unclear to me the added value.

A directive I would assume serves the same purpose you intend to give it, without interfering with compilation/tooling and if anything, providing a way for tooling to circumvent the issue.

@martriay
Copy link
Contributor

Closing this since it will not be relevant anymore after the ongoing Cairo 1.0 migration. If you think this is a mistake, feel free to open a new issue.

@github-project-automation github-project-automation bot moved this to ✅ Resolved in Cairo team Feb 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: Resolved
Development

No branches or pull requests

4 participants