contracts icon indicating copy to clipboard operation
contracts copied to clipboard

fix(IdRegistry): added contract validation to setStorageRegistry

Open 0xObsidian opened this issue 1 year ago • 0 comments

Motivation

This PR closes #440 setStorageRegistry was missing validation checks for the new address, which could lead to setting invalid contracts or non-contract addresses as the storage registry.

Change Summary

  • Added zero address check using revert
  • Added contract existence check
  • Used revert with custom errors instead of require() for:
    • Gas efficiency (4-byte selector vs string storage)
    • Better ABI compatibility via custom errors
  • Updated registry tests to align with the current implementation
  • Fixed assumeNotPrecompile issue
  • Ran forge formatting on all modified files in this PR scope

Testing the fix

Fetch this PR branch and from the root:

Add foundry-rs/forge-std

forge install foundry-rs/forge-std

Run test

forge test --match-test testFuzzSetStorageRegistry -vvv

Merge Checklist

Choose all relevant options below by adding an x now or at any time before submitting for review


PR-Codex overview

This PR focuses on improving function signatures by making them more consistent and readable, particularly by changing certain functions to view. It also includes minor refactoring for clarity and better code organization across various test suites and contracts.

Detailed summary

  • Changed several function signatures to view in KeyRegistryTestSuite.sol, IdGatewayTestSuite.sol, and IdRegistryTestSuite.sol.
  • Refactored function parameters for clarity in IdGateway.sol and IdRegistryTestSuite.sol.
  • Added assumptions in testFuzzSetStorageRegistry to ensure valid inputs.
  • Updated the _signDigest function to be pure.
  • Improved readability by breaking long parameter lists into multiple lines in various functions.

✨ Ask PR-Codex anything about this PR by commenting with /codex {your question}

0xObsidian avatar Dec 27 '24 14:12 0xObsidian