matter-labs / foundry-zksync

Fork of Foundry tailored for zkSync environment
Apache License 2.0
296 stars 127 forks source link

non-inline library doesn't work correctly in `forge test` #382

Open wshino opened 5 months ago

wshino commented 5 months ago

Component

Forge

Have you ensured that all of these are up to date?

What version of Foundry are you on?

forge 0.0.2 (13497a5 2024-05-16T00:24:48.304138000Z)

What command(s) is the bug in?

forge test

Operating System

macOS (Apple Silicon)

Describe the bug

I am currently trying to apply the foundry-zksync in the following repositories: https://github.com/zkemail/ether-email-auth

I deployed non-inlinable libraries to zkSync sepolia, so forge build has been succeeded in my local. However, some test cases that references library will cause an EvmError: Revert at the point of referencing library, and the test will fail. For example, the following line fails https://github.com/zkemail/ether-email-auth/blob/feat/foundry-zksync/packages/contracts/src/EmailAuth.sol#L204

Can't we test locally when referring to a library? Or is there any solution of that?

dutterbutter commented 5 months ago

Related to #369

Karrq commented 4 months ago

We are aware of this issue (which came up while working on the functionality), and were able to come up with a workaround.

If you would like to test your functionality before the issue is fixed, you can move away from test and use script instead, like we do in our tests over here, which would allow you to run your tests (now scripts) in an environment with the libraires present.

Basically:

  1. start a local instance of era-test-node
  2. use forge build --zksync to build your contracts, tests and scripts included, and determine the librairies to be deployed
  3. deploy them using forge create --deploy-missing-libraries --zksync --rpc-url <era-node> against your local node
  4. finally, run your scripts forge script ./path/to/script.sol:Script --zksync --rpc-url <era-node> against your local node with the deployed libraries
Karrq commented 4 months ago

We are still working on improving this, but we lack the tools to make this as seamless as with Solidity. In the meantime, we have found an improved way to run tests with libraries, without having to edit your sources.

This is by no means the final solution, and should only be considered a workaround.

The first 3 steps are the same as last time:

  1. start a local instance of era-test-node
  2. use forge build --zksync to build your contracts, tests and scripts included, and determine the librairies to be deployed
  3. deploy them using forge create --deploy-missing-libraries --zksync --rpc-url <era-node> against your local node

You will need to redo the previous steps if/when your local era-test-node shuts down.

Finally, you can run your tests using forge test --zksync --fork-url <era-node>, to have them run against your local node's state, which now includes the deployed libraries.

You can use any other flag as part of your test invocation as usual, just remember to use --fork-url <era-node> so that the test environment will start with the libraries deployed. This also means that if the libraries are already deployed, for example in testnet, you can just specify rpc to fork and update your foundry.toml to use the libraries deployed on that fork.

Please note that if the node restarts you'll need to deploy the libraries again, and, if the addresses were to change, you'll also need to rebuild your contracts so they reference the updated addresses.

hedgar2017 commented 1 month ago

Deploy-time linking has been supported in this pre-release and already tested in Hardhat. We are planning to release a new version of zksolc with it next week. zksolc implements this workflow with a few caveats:

  1. Unlinked bytecode is wrapped into an ELF container. When linking is done, ELF is stripped and only raw EraVM bytecode remains. This means that unlinked and linked bytecode are not equal in size.

  2. The only way to link is to call zksolc --link <inputs> --libraries <libraries>. There is no way to do it with linkReferences, and we're using actual LLVM-based linker facilities (lld, objdump), and it is not feasible to move this logic to such a high-level place like Foundry. Usage:

    zksolc --link bytecode1.hex bytecode2.hex --libraries `test.sol:Test=0x1234....5678` `foo.sol:Foo=0xaaaa....bbbb`

    All libraries will be applied to all bytecode files and will be ignored if they aren't expected.

  3. It is not possible to produce factory dependencies or bytecode hash for unlinked bytecode. Instead, it is provided like in examples above for use cases when it may be needed.

  4. zksolc --link ... writes a small JSON to stdout:

    {
    "linked": ... // what was unlinked now is linked; here are bytecode hashes of the linked files
    "unlinked": ... // what is still unlinked as not all libraries were provided; here are missing libraries
    "ignored": ... // what was already linked and left unchanged; here are bytecode hashes of the unchanged files
    }

    Examples:

    
    hedgarmac@MBP-Oleksandr era-compiler-solidity % zksolc --link output/test.sol/Greeter.zbin | jq .                                                                                
    {
    "ignored": {},
    "linked": {},
    "unlinked": {
    "output/test.sol/Greeter.zbin": [
      "test.sol:GreaterHelper"
    ]
    }
    }

hedgarmac@MBP-Oleksandr era-compiler-solidity % zksolc --link output/test.sol/Greeter.zbin --libraries 'test.sol:GreaterHelper=0x1234567812345678123456781234567812345678' | jq . { "ignored": {}, "linked": { "output/test.sol/Greeter.zbin": "010000d7da40ec0d648357e7f45d87181801afa189ae6ab50d83ede430184f7e" }, "unlinked": {} }

hedgarmac@MBP-Oleksandr era-compiler-solidity % zksolc --link output/test.sol/Greeter.zbin --libraries 'test.sol:GreaterHelper=0x1234567812345678123456781234567812345678' | jq . { "ignored": { "output/test.sol/Greeter.zbin": "010000d7da40ec0d648357e7f45d87181801afa189ae6ab50d83ede430184f7e" }, "linked": {}, "unlinked": {} }



5. Passing a file to `--libraries` is not supported, but let me know if it's needed.
dutterbutter commented 4 weeks ago

@tomimor ^^ worth tracking the above. Once the HH initial implementation is ready to share we can review the flow a bit better.