[DRAFT] Test using vault compiled with Hardhat in Forge tests

[joaobrunoah]

Description

Complex contracts, such as BatchRouter and Vault, often trigger 'stack too deep' errors upon modification. These errors, coupled with the need for workarounds, result in difficult-to-understand code.

If we compile the code with the via-ir flag, we can avoid "stack too deep". However, when we develop tests with Forge, they're also compiled with the via-ir flag and take forever to compile. Currently, there's no way to natively use the contracts folder compiled with via-ir and the test folder compiled without it.

To bypass this issue, as mentioned by @elshan-eth, we can import the JSON file that was generated from hardhat/forge build using --via-ir, and with modifications in the foundry.toml file we prevent forge from compiling all contracts from contracts folder when running forge test.

NOTE: This PR breaks the coverage tests in forge, but they're already broken in hardhat since the code is compiled with via-ir too.

Type of change

• Bug fix
• New feature
• Breaking change
• Dependency changes
• Code refactor / cleanup
• Documentation or wording changes
• Other

Checklist:

• The diff is legible and has no extraneous changes
• Complex code has been commented, including external interfaces
• Tests have 100% code coverage
• The base branch is either main, or there's a description of how to merge

[EndymionJkb]

Worked for me! I even deleted the artifacts, node modules, etc., and tried running test:forge only, to make sure hardhat didn't have to be run first. And the dramatic snap file reduction indicates it's functioning.

[jubeira]

Nice! Well let's keep this in mind; maybe let's not merge it yet. It's great to know that it's not that hard to move away from the current pipeline.

If we wanted to have coverage tests, we'd need to deploy the vault in the regular way and compile with a different profile like the project referenced in #539 does right?

[jubeira]

this probably needs to be updated right?

[joaobrunoah]

No.. We need to specify the test profile, so src is set to 'test/foundry' and the Vault binaries are not compiled by the forge test, which would trigger the "Stack too deep" error

[joaobrunoah]

Nice! Well let's keep this in mind; maybe let's not merge it yet. It's great to know that it's not that hard to move away from the current pipeline.

If we wanted to have coverage tests, we'd need to deploy the vault in the regular way and compile with a different profile like the project referenced in #539 does right?

@jubeira We'd need to rewrite VaultMockDeployer, deploying the VaultMock with the actual contract instead of using the compiled json.