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

Unable to run invariant tests using handler pattern #565

Closed
2 tasks done
brotherlymite opened this issue Sep 8, 2024 · 3 comments
Closed
2 tasks done

Unable to run invariant tests using handler pattern #565

brotherlymite opened this issue Sep 8, 2024 · 3 comments

Comments

@brotherlymite
Copy link

Component

Forge

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

  • Foundry
  • Foundryup

What version of Foundry are you on?

forge 0.0.2 (a69c479 2024-09-06T00:32:22.954024000Z)

What command(s) is the bug in?

forge test --zksync

Operating System

macOS (Apple Silicon)

Describe the bug

When running the invariant test, the test seems to be passing but strangely the functions in the handler contract are not being called for some reason.

To replicate, run forge test --match-contract PoolInvariants -vv --show-progress on this branch and you would see the following output in the terminal:

forge test --match-contract PoolInvariants -vv --show-progress

No files changed, compilation skipped
tests/core/pool-invariant/PoolInvariants.t.sol:PoolInvariants
  ↪ Suite result: ok. 2 passed; 0 failed; 0 skipped; finished in 23.04s (44.22s CPU time)

Ran 2 tests for tests/core/pool-invariant/PoolInvariants.t.sol:PoolInvariants
[PASS] invariant_checkHealthFactor() (runs: 25, calls: 25000, reverts: 0)
[PASS] invariant_summary() (runs: 25, calls: 25000, reverts: 0)
Logs:
  Supply Success Calls USDX: 71
  Borrow Success Calls USDX: 62
  Borrow Fail Calls USDX: 43
  Withdraw Success Calls USDX: 70
  Withdraw Fail Calls USDX: 47
  Repay Success Calls USDX: 85
  Repay Fail Calls USDX: 91
  
  Supply Success Calls WETH: 51
  Borrow Success Calls WETH: 55
  Borrow Fail Calls WETH: 22
  Withdraw Success Calls WETH: 47
  Withdraw Fail Calls WETH: 22
  Repay Success Calls WETH: 55
  Repay Fail Calls WETH: 38
  
  Supply Success Calls WBTC: 55
  Borrow Success Calls WBTC: 34
  Borrow Fail Calls WBTC: 14
  Withdraw Success Calls WBTC: 29
  Withdraw Fail Calls WBTC: 27
  Repay Success Calls WBTC: 43
  Repay Fail Calls WBTC: 39

Suite result: ok. 2 passed; 0 failed; 0 skipped; finished in 23.04s (44.22s CPU time)

When the same test is being run in zksync, we see that the handler functions are not being called. To run the same test in zksync and replicate: checkout to this branch, copy .env.examples to .env and run FOUNDRY_PROFILE=zksync forge test --zksync --match-contract PoolInvariants --rpc-url https://mainnet.era.zksync.io -vv --show-progress.
This is the output we are getting when running the same test in zksync:

FOUNDRY_PROFILE=zksync forge test --zksync --match-contract PoolInvariants --rpc-url https://mainnet.era.zksync.io -vv --show-progress

[⠃] Using zksolc-1.5.3

No files changed, compilation skipped
tests/core/pool-invariant/PoolInvariants.t.sol:PoolInvariants
  ↪ Suite result: ok. 2 passed; 0 failed; 0 skipped; finished in 1160.67s (21.18s CPU time)

Ran 2 tests for tests/core/pool-invariant/PoolInvariants.t.sol:PoolInvariants
[PASS] invariant_checkHealthFactor() (runs: 1, calls: 2, reverts: 0)
[PASS] invariant_summary() (runs: 1, calls: 2, reverts: 0)
Logs:
  Supply Success Calls USDX: 0
  Borrow Success Calls USDX: 0
  Borrow Fail Calls USDX: 0
  Withdraw Success Calls USDX: 0
  Withdraw Fail Calls USDX: 0
  Repay Success Calls USDX: 0
  Repay Fail Calls USDX: 0
  
  Supply Success Calls WETH: 0
  Borrow Success Calls WETH: 0
  Borrow Fail Calls WETH: 0
  Withdraw Success Calls WETH: 0
  Withdraw Fail Calls WETH: 0
  Repay Success Calls WETH: 0
  Repay Fail Calls WETH: 0
  
  Supply Success Calls WBTC: 0
  Borrow Success Calls WBTC: 0
  Borrow Fail Calls WBTC: 0
  Withdraw Success Calls WBTC: 0
  Withdraw Fail Calls WBTC: 0
  Repay Success Calls WBTC: 0
  Repay Fail Calls WBTC: 0

Suite result: ok. 2 passed; 0 failed; 0 skipped; finished in 1160.67s (21.18s CPU time)

We see that none of the actions of the invariant test, including Supply, Borrow, Withdraw, Repay are being called for some reason but the same test in normal foundry works fine.

@Karrq
Copy link
Contributor

Karrq commented Sep 11, 2024

Hey @brotherlymite, thanks for the report!
We investigated the issue and found that this behavior can be observed when using the handler pattern.
In particular, when the handler is called it is done thru the zkVM, thus when the handler makes use of cheatcodes, it causes the unexpected behavior you are observing (in this case, the execution is equivalent to a no-op).
EDIT: see next comment

There's no way for us to determine if the target selectors are handler contracts or not, thus it's not possible to determine whether to run it in EVM or zkVM automatically.

We are working on a new feature (#569) which will make calling cheatcodes in external contracts a lot easier to work with, without having to switch zkvm on or off manually.


Unfortunately, there's no workaround available to make it work today, but if you wish to maintain this type of test I can suggest migrating to a fuzz test instead, for example:

struct SummaryInput {
  uint8 tokenIndex;
  uint128 amount;
  uint8 opIdx;
}
  
/// forge-config: zksync.fuzz.runs = 25
/// forge-config: zksync.fuzz.show-logs = true
function testFuzz_summary(SummaryInput[1000] memory inputs) external {
  address(vm).call(abi.encodeWithSignature("zkVm(bool)", false));

  FuzzSelector memory pool = targetSelectors()[0];

  for (uint i = 0; i < inputs.length; i++) {
    SummaryInput memory input = inputs[i];

    uint8 idx = (input.opIdx % uint8(pool.selectors.length));
    bytes4 op = pool.selectors[idx];

    address(s_poolHandler).call(abi.encodeWithSelector(op, input.tokenIndex, input.amount));

  }

  s_poolHandler.summary();
}

You can tweak the runs paramters like usual, and the depth is represented by the array length. In the snippet above I made it match the original (non-zksync) test configuration you reported (please note that show-logs might be undesirable).

Note that the calls to PoolHandler will happen in EVM, thus zkVM must be switched on inside it for the contract under test to be ran inside zkVM. Additionally, it's important to ensure that PoolHandler is either deployed in EVM or be made persistent, otherwise its deployment in setUp will happen in zkVM only, not allowing it to be called from within the test.

@tomimor tomimor changed the title Unable to run invariant tests Unable to run invariant tests using handler pattern Sep 12, 2024
@Karrq
Copy link
Contributor

Karrq commented Sep 16, 2024

Hey @brotherlymite, upon further review we actually determined the issue to be a bit different.
The handler is actually being called via EVM, so cheatcodes would be fine, but the deployment happens in zkEVM (during setUp), thus the contract wouldn't be present in EVM and cause this behavior.

The fix in this case would be to deactivate zkVm before doing the deployment of the handler, which resolves the issue above, and allows invariant tests to run as normal.
Unfortunately, this means we have the opposite problem: non-handler pattern is not working as intended.

Additionally, in the version you originally encountered the issue there's still a bug, in particular when the deployment happening in EVM instead of zkVM, it wouldn't be detected for the invariant executor, but this has been resolved as of #572, therefore an upgrade is required to run invariant tests with the handler pattern.


There are a few more adjustments that need to be made to make the invariant test to run successfully, but these are relatively easy and are just to account for zkVM specifics:

  1. use profile.zksync.invariant.no_zksync_reserved_addresses, this prevents reserved addresses to be used for fuzzed senders
  2. deal ETH to the user to allow the transaction to be ran, otherwise the bootloader will error out because the balance is insufficient
  3. ensure that tx.origin is set correctly - in particular startPrank(user) should change to startPrank(user, user), otherwise the tx originator will be set to the caller of PoolHandler, instead of the pranked sender. If the fuzzed sender is the originator, you'll need to deal ETH to it, for the same reason as dealing to user

Please let me know if you have any more questions about this issue! Thank you

Karrq added a commit that referenced this issue Sep 16, 2024
Karrq added a commit that referenced this issue Sep 20, 2024
* test(zk): add #565 repro

* test(zk:565): fix invariant and use zkVmSkip

* fix(zk): use constant test addr for zkvm skip test

* test(zk): proper basic invariant test

* test(zk:invariant): add direct invariant test

* feat: `InZkVm` utility to detect if contract is running in zkVm

* chore: lints & formatting

* refactor(test:zk): edit runner test opts

* fix(test:zk): pre-select target senders for issue565 w/o handler

* fix(zk): use `get_test_contract_address`

* fix(backend): don't set test contract on each init

* chore: lints

* fix(executor): set_test_contract even w/o setup

* test(zk): filter test to specific contract

* refactor(zk): simplify test contract check

Co-authored-by: Nisheeth Barthwal <[email protected]>

* chore: fmt

---------

Co-authored-by: Nisheeth Barthwal <[email protected]>
@Karrq
Copy link
Contributor

Karrq commented Sep 20, 2024

Hey @brotherlymite! We have fixed the issue with invariant testing as of #581!
We also added 2 tests and improved an existing one to ensure this stays the case in the future, you could make use of them as reference, in particular the reproduction test for this issue!
Please let me know if you still encounter issues!

@Karrq Karrq closed this as completed Oct 10, 2024
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

2 participants