Details:
In several contracts (including PerpetualVault), revert statements use custom errors that do not include contextual information. For instance, in PerpetualVault the code currently reverts with a generic Error.InvalidCall() when a caller is unauthorized, whereas it has been suggested to use revert Error.InvalidCall(_gmxLock); (or similar) to include the value of _gmxLock. This lack of contextual data in error messages is consistent across multiple contracts, which can hinder debugging and incident analysis when errors occur.
Root Cause:
The root cause is an oversight in the design of error handling. The custom error definitions and revert statements were implemented without passing contextual parameters (such as state variables) that could help developers or auditors understand the conditions under which the error was triggered. This decision might have been made to save gas but at the cost of reduced clarity in error reporting.
Impact:
Debugging Difficulty: Without additional context in the error messages, developers and support teams may struggle to quickly diagnose and resolve issues when a transaction reverts.
Operational Overhead: The lack of detailed error messages could delay incident resolution in production environments, especially when multiple conditions might cause similar errors.
No Direct Financial Impact: While this issue does not directly expose funds to risk, it reduces the transparency of the contract’s operational state, thereby indirectly affecting maintainability and support.
Recommendation:
Enhance Custom Errors: Update custom error definitions to include relevant parameters that capture key state variables. For example, modify Error.InvalidCall to accept parameters such as the current state of _gmxLock.
Update Revert Statements: Modify the revert calls in all affected contracts to pass the contextual data (e.g., use revert Error.InvalidCall(_gmxLock); instead of the generic call).
Review All Error Messages: Extend this improvement across the entire codebase to ensure consistent and informative error reporting.
Proof of Concept:
Before Modification:
Call an affected function (e.g., one in PerpetualVault) from an unauthorized address.
The function reverts with Error.InvalidCall(), providing no insight into why the call was invalid or the state of _gmxLock.
After Modification:
Update the revert statement to revert Error.InvalidCall(_gmxLock); in the affected function.
Call the same function under the same conditions.
The function now reverts with Error.InvalidCall(<value_of__gmxLock>), supplying the extra context that aids in debugging the failure.
Implementing these changes would improve the clarity of revert messages without compromising the contract’s security or functionality.
Please read the CodeHawks documentation to know which submissions are valid. If you disagree, provide a coded PoC and explain the real likelihood and the detailed impact on the mainnet without any supposition (if, it could, etc) to prove your point.
The contest is live. Earn rewards by submitting a finding.
This is your time to appeal against judgements on your submissions.
Appeals are being carefully reviewed by our judges.