Skip to content

refactor: replace require strings with custom errors - #141

Open
pplmaverick wants to merge 1 commit into
tempoxyz:masterfrom
pplmaverick:refactor/custom-errors
Open

refactor: replace require strings with custom errors#141
pplmaverick wants to merge 1 commit into
tempoxyz:masterfrom
pplmaverick:refactor/custom-errors

Conversation

@pplmaverick

Copy link
Copy Markdown

Summary

Replace 4 legacy require(string) statements with custom errors to align
with the existing convention used throughout src/interfaces/.

Changes

File Change
src/sig/SignatureLib.sol require → custom error
src/tx/Eip7702TransactionLib.sol require → custom error
src/tx/Eip1559TransactionLib.sol require → custom error
src/tx/TempoTransactionLib.sol require → custom error

Motivation

  • src/interfaces/ already uses custom errors exclusively
  • Custom errors are more gas-efficient (no string stored in bytecode)
  • No logic changes — behavior is identical

Testing

  • forge build
  • forge fmt --check
  • forge test ✅ (3 existing tests, no regressions)

Comment thread src/sig/SignatureLib.sol
function webAuthnMessageHash(bytes memory webauthnData) internal pure returns (bytes32) {
// First 37 bytes are authenticatorData (no extensions, since the precompile rejects ED).
require(webauthnData.length >= 37, "SignatureLib: webauthnData too short");
if (webauthnData.length < 37) revert WebAuthnDataTooShort();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can still use require and just pass the custom error as the 2nd arg

@pplmaverick

Copy link
Copy Markdown
Author

Thanks for the suggestion! I considered using require(cond, CustomError()),
but this syntax requires solc ≥ 0.8.27 (0.8.26 only supports it with --via-ir).
Since the repo's minimum supported version is 0.8.20, this would break CI on
older compiler jobs.

Happy to switch if you'd like to bump the minimum version, otherwise I'll keep
the current if (...) revert pattern which is compatible across the full range.

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

Successfully merging this pull request may close these issues.

2 participants