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

Use structs to remove being forced to compile via ir #456

Open
PatrickAlphaC opened this issue May 8, 2024 · 3 comments
Open

Use structs to remove being forced to compile via ir #456

PatrickAlphaC opened this issue May 8, 2024 · 3 comments

Comments

@PatrickAlphaC
Copy link

PatrickAlphaC commented May 8, 2024

As of today, using any of these contracts as a package results in the following issue.

Compiler run failed:
Error: Compiler error (/solidity/libsolidity/codegen/LValue.cpp:55):Stack too deep. Try compiling with `--via-ir` (cli) or the equivalent `viaIR: true` (standard JSON) while enabling the optimizer. Otherwise, try removing local variables.
CompilerError: Stack too deep. Try compiling with `--via-ir` (cli) or the equivalent `viaIR: true` (standard JSON) while enabling the optimizer. Otherwise, try removing local variables.
   --> lib/era-contracts/system-contracts/contracts/libraries/TransactionHelper.sol:133:38:
    |
133 |                 EfficientCall.keccak(_transaction.paymasterInput)

To get around this, you can instead use structs instead of so many variables.

You can only get this error if you try to compile the contracts using foundry, like so:

  1. Replace this with _anything
uint160 constant SYSTEM_CONTRACTS_OFFSET = {{SYSTEM_CONTRACTS_OFFSET}}; // 2^15
  1. Add a foundry.toml
echo -e "[profile.default]\nsrc = \"contracts\"" > foundry.toml
forge build
@PatrickAlphaC
Copy link
Author

PatrickAlphaC commented May 8, 2024

Actually... digging into this more. Seems like the issue goes a bit deeper than I thought. The way this repo compiles the contracts is... a bit odd.

Why?

We have javascript magic like this:

uint160 constant SYSTEM_CONTRACTS_OFFSET = {{SYSTEM_CONTRACTS_OFFSET}}; // 2^15

Which it looks like is fixed when we compile with the precompiles, but it prevents our foundry friends from having a good time, and sort of vendor locks people into Hardhat.

The preprocess-system-contracts.ts seems to do some clever stuff, which is "fine" but it might not be acceptable to use as a package that users use. It might make sense to have a separate repo where users can import the systems contract library so they can mock interacting with the system contracts.

Or, you could have foundry-zksync deal with processing these contracts.

So... we have a few options:

  1. Increase the scope of foundry-zksync to handle this
  2. Split out this repo so there is a repo that mirrors this one but "packagifies" everything so that users can mock interacting with system contracts in their local environments
  3. Reduce the javascript magic

I'm leaning towards number 2, but not sure how others feel

@StanislavBreadless
Copy link
Collaborator

We actually used to have this repo as a mirror of the contracts that can be easily imported. While it is somewhat outdated, we generally ensure backward compatibility, so if the functionality provided by that repo is enough for you, you can use it temporarily. We will soon update it though

@alexhooketh
Copy link

Still not fixed.

@StanislavBreadless is it still safe to use the repo you mentioned?

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

No branches or pull requests

3 participants