Skip to content

♻️ Enable expectRevert for internal Functions#298

Merged
pcaversaccio merged 2 commits intomainfrom
test/expectRevert-cheatcode
Jan 28, 2025
Merged

♻️ Enable expectRevert for internal Functions#298
pcaversaccio merged 2 commits intomainfrom
test/expectRevert-cheatcode

Conversation

@pcaversaccio
Copy link
Copy Markdown
Owner

@pcaversaccio pcaversaccio commented Jan 28, 2025

🕓 Changelog

The Foundry PR #9537 has disabled expectRevert for internal functions by default. However, we need this functionality in the ECDSATest contract since we use the internal function to2098Format in various expectRevert test calls:

/**
 * @dev Transforms a standard signature into an EIP-2098
 * compliant signature.
 * @param signature The secp256k1 64/65-bytes signature.
 * @return short The 64-bytes EIP-2098 compliant signature.
 */
function to2098Format(
    bytes memory signature
) internal view returns (bytes memory) {
    if (signature.length != 65) revert InvalidSignatureLength(self);  // <-- We need to enable this `revert`.
    if (uint8(signature[32]) >> 7 == 1) revert InvalidSignatureSValue(self); // <-- We need to enable this `revert`.
    bytes memory short = signature.slice(0, 64);
    uint8 parityBit = uint8(short[32]) | ((uint8(signature[64]) % 27) << 7);
    short[32] = bytes1(parityBit);
    return short;
}

This PR enables expectRevert for internal functions by setting allow_internal_expect_revert = true in the foundry.toml file.

🐶 Cute Animal Picture

image

Signed-off-by: Pascal Marco Caversaccio <pascal.caversaccio@hotmail.ch>
@pcaversaccio pcaversaccio self-assigned this Jan 28, 2025
@pcaversaccio pcaversaccio added dependencies 🔁 Pull requests that update a dependency file refactor/cleanup ♻️ Code refactorings and cleanups labels Jan 28, 2025
@pcaversaccio pcaversaccio added this to the 0.1.1 milestone Jan 28, 2025
Signed-off-by: Pascal Marco Caversaccio <pascal.caversaccio@hotmail.ch>
@pcaversaccio pcaversaccio merged commit 9eeda76 into main Jan 28, 2025
@pcaversaccio pcaversaccio deleted the test/expectRevert-cheatcode branch January 28, 2025 14:26
@zerosnacks
Copy link
Copy Markdown

zerosnacks commented Jan 31, 2025

Hi @pcaversaccio

It is also possible to use the in-line configuration as follows selectively on the failing function tests

/// forge-config: default.allow_internal_expect_revert = true
function test_foo() public {}

This is recommended over using the global configuration option, see:

https://book.getfoundry.sh/cheatcodes/expect-revert#description

@pcaversaccio
Copy link
Copy Markdown
Owner Author

Hi @pcaversaccio

It is also possible to use the in-line configuration as follows selectively on the failing function tests

/// forge-config: default.allow_internal_expect_revert = true
function test_foo() public {}

This is recommended over using the global configuration option, see:

https://book.getfoundry.sh/cheatcodes/expect-revert#description

Thx, I'm aware of this possibility, but I personally prefer to enable expectRevert for internal functions by default.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies 🔁 Pull requests that update a dependency file refactor/cleanup ♻️ Code refactorings and cleanups

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants