Skip to content

Conversation

@cameel
Copy link
Collaborator

@cameel cameel commented Jul 17, 2024

Bug in isoltest discovered by @rodiazet.

functionSignatureFromABI() would attempt to calculate signature for a function even if it had no name in the ABI (which is the case for constructors, fallback and receive). This would lead to a crash.

@cameel cameel self-assigned this Jul 17, 2024
@cameel cameel requested review from nikola-matic and r0qs July 17, 2024 15:24
@cameel cameel force-pushed the fix-isoltest-crash-on-arg-formatting-when-abi-has-unnamed-functions branch from 772e476 to eb713c8 Compare July 17, 2024 15:25
@cameel
Copy link
Collaborator Author

cameel commented Jul 17, 2024

The buggy logic is used only for formatting expectations when a test fails in interactive mode so it went undiscovered until now.

It's not even possible to cover it with a test. It can be easily reproduced, but we would not add such a test since the expectations do not match:

contract C {
    constructor() {}
    function f() external pure returns (bytes32) {}
}
// ----
// constructor()
// f() -> 42

@cameel cameel force-pushed the fix-isoltest-crash-on-arg-formatting-when-abi-has-unnamed-functions branch from eb713c8 to 23ca369 Compare July 18, 2024 15:36
@cameel cameel enabled auto-merge July 18, 2024 15:36
… ABI includes entries without names

- functionSignatureFromABI() would attempt to calculate signature for a function even if it had no name (i.e. was a constructor, fallback or receive). This would lead to a crash.
- The buggy logic is used only for formatting expectations when an test fails in interactive mode.
@cameel cameel force-pushed the fix-isoltest-crash-on-arg-formatting-when-abi-has-unnamed-functions branch from 23ca369 to 1a5335a Compare July 19, 2024 08:54
@cameel cameel merged commit 01a3bb5 into develop Jul 19, 2024
@cameel cameel deleted the fix-isoltest-crash-on-arg-formatting-when-abi-has-unnamed-functions branch July 19, 2024 09:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants