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

Upgrading zksync-ethers to 6.12.0 breaks hardhat chai matchers #200

Closed
mmv08 opened this issue Sep 2, 2024 · 5 comments
Closed

Upgrading zksync-ethers to 6.12.0 breaks hardhat chai matchers #200

mmv08 opened this issue Sep 2, 2024 · 5 comments
Labels
bug Something isn't working

Comments

@mmv08
Copy link

mmv08 commented Sep 2, 2024

🐛 Bug Report for zksync-ethers JavaScript SDK

📝 Description

The following assertion:

            await expect(accessor.simulate(tx.to, tx.value, tx.data, tx.operation)).to.be.revertedWith(
                "SimulateTxAccessor should only be called via delegatecall",
            );

fails with zksync-ethers 6.12.0 with

  1) SimulateTxAccessor
       estimate
         should enforce delegatecall:
     Error: execution reverted: SimulateTxAccessor should only be called via delegatecall (transaction={ "data": "0x1c5fb211000000000000000000000000915148c3a7d97ecf4f741ffaab8263f9d66f2d0c0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000008000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000044484560b2000000000000000000000000e2b8cb53a43a56d4d2ab6131c81bd76b86d3afe5000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000", "from": "0xe2b8cb53a43a56d4d2ab6131c81bd76b86d3afe5", "to": "0x2bec1716df0f0790703f57695649a80cc1b6b119" }, info={ "_error": { "error": { "code": 3, "data": "0x08c379a00000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000003953696d756c61746554784163636573736f722073686f756c64206f6e6c792062652063616c6c6564207669612064656c656761746563616c6c00000000000000", "message": "execution reverted: SimulateTxAccessor should only be called via delegatecall" }, "id": 204, "jsonrpc": "2.0" }, "payload": { "id": 204, "jsonrpc": "2.0", "method": "zks_estimateFee", "params": [ { "data": "0x1c5fb211000000000000000000000000915148c3a7d97ecf4f741ffaab8263f9d66f2d0c0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000008000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000044484560b2000000000000000000000000e2b8cb53a43a56d4d2ab6131c81bd76b86d3afe5000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000", "from": "0xe2b8cb53a43a56d4d2ab6131c81bd76b86d3afe5", "to": "0x2bec1716df0f0790703f57695649a80cc1b6b119" } ] } }, code=3, version=6.13.2)
      at makeError (node_modules/ethers/src.ts/utils/errors.ts:694:21)
      at HardhatZksyncEthersProvider.getRpcError (node_modules/zksync-ethers/src/provider.ts:2390:21)
      at /Users/mmv/projects/safe/safe-contracts/node_modules/ethers/src.ts/providers/provider-jsonrpc.ts:563:45
      at processTicksAndRejections (node:internal/process/task_queues:95:5)

but works with 6.11.2

🔄 Reproduction Steps

  1. Clone the repo & checkout to the branch from this PR
  2. run npm i && npm test:zk, observe the test suite successfully ran
  3. upgrade zksync-ethers to 6.12.0 in package json
  4. repeat step 2, observe the test suite failing
  5. The test from the issue description can be found in test/accessors/SimulateTxAccessor.spec.ts

🤔 Expected Behavior

The test suite passes after the upgrade

😯 Current Behavior

The test suite fails

🖥️ Environment

  • Node version: v20.11.1
  • Operating System & Version: macOS Sonoma 14.6.1 arm64
@mmv08 mmv08 added the bug Something isn't working label Sep 2, 2024
@petarTxFusion
Copy link
Contributor

We have updated how the error handling works since a lot of the custom ZKsync error messages where not displayed correctly with ethers v6. If I am not mistaken in your test its throwing correct error just with additional data, so could you adapt that test to check if error message contains SimulateTxAccessor should only be called via delegatecall?

@mmv08
Copy link
Author

mmv08 commented Sep 2, 2024

We have updated how the error handling works since a lot of the custom ZKsync error messages where not displayed correctly with ethers v6. If I am not mistaken in your test its throwing correct error just with additional data, so could you adapt that test to check if error message contains SimulateTxAccessor should only be called via delegatecall?

The error is not from the assertion. It's coming from the accessor.simulate(tx.to, tx.value, tx.data, tx.operation) invocation.

If it would've been the assertion that throws, the error message would be (example):

 AssertionError: Expected transaction to be reverted with reason 'should only be called via delegatecall', but it reverted with reason 'SimulateTxAccessor should only be called via delegatecall'

Also, I've been tinkering with it and I found that if i remove this overridden getRpcError method, it works again v6.11.2...v6.12.0#diff-e0e6d91455eef50c5793e300039e5cdc7e6d372707172b639564495e4fa0a2f3R2386-R2394

@petarTxFusion
Copy link
Contributor

I have found the issue, the fix should be live today or tomorrow

@mmv08
Copy link
Author

mmv08 commented Sep 2, 2024

I have found the issue, the fix should be live today or tomorrow

Thank you!

@petarTxFusion
Copy link
Contributor

It should work now with version v6.12.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants