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

Tests failing due to foundry recently disabling expectRevert for internal functions #248

Open
r0qs opened this issue Jan 28, 2025 · 1 comment

Comments

@r0qs
Copy link

r0qs commented Jan 28, 2025

What version of PRBMath are you using?

v4.1.0

What version of Solidity are you using?

0.8.29-8c54f4d

Describe the bug

We noticed in our external tests (https://app.circleci.com/pipelines/github/ethereum/solidity/37890/workflows/821e4683-ca57-48b4-a751-8246720aa1db/jobs/1740901/parallel-runs/0/steps/0-108) that some PRB-Math tests started failing with the latest versions of Foundry after foundry-rs/foundry#9537 was merged. The PR disables vm.expectRevert for internal functions by default, causing tests like test_RevertWhen_Positive_GtMaxPermitted to fail with the following error:

[FAIL: call didn't revert at a lower depth than cheatcode call depth] test_RevertWhen_Positive_GtMaxPermitted() (gas: 4033)

The previous behavior can be restored by enabling the allow_internal_expect_revert setting in foundry.toml (false by default):

[default]
allow_internal_expect_revert = true

Or by adding the following comment to affected tests (which is around 92 of them - I didn't check them all though):

/// forge-config: default.allow_internal_expect_revert = true
function test_RevertWhen_Positive_GtMaxPermitted() external whenNotZero {
    SD59x18 x = EXP_MAX_INPUT + sd(1);
    vm.expectRevert(abi.encodeWithSelector(PRBMath_SD59x18_Exp_InputTooBig.selector, x));
    exp(x);
}
@PaulRBerg
Copy link
Owner

Thanks for the report @r0qs. Can you please confirm if the suggestion is to set allow_internal_expect_revert to true in the PRBMath foundry.toml, or in the user's own configuration?

cc @andreivladbrg @smol-ninja — we will likely face the same issue in the Sablier repos when the stable version of Foundry gets this change.

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